Hi Ian
Apologies for the late reply, I was off.
I have my doubts whether we should support something from the PI spec,
since we are trying to keep things confined to the EFI spec, but I'd
like to hear from Henrich as well. I guess since systemd added it we
should follow if we want to boot distros ....
[...]
> index de57823bd44..934bdd89a69 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2120,7 +2120,8 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
> efi_dp_split_file_path(file_path, &dp, &fp);
> ret = efi_setup_loaded_image(dp, fp, image_obj, &info);
> if (ret == EFI_SUCCESS)
> - ret = efi_load_pe(*image_obj, dest_buffer, source_size, info);
> + ret = efi_load_pe(*image_obj, dest_buffer, source_size, info,
> + file_path, boot_policy);
Why do we need the extra argument here? The file path on the loaded
image protocol isn't enough? If it's not then we can either pass it as
an extra argument to avoid processing DPs again or retrieve it from
the loaded image ptr. I am not sure what I prefer though
Something like
struct efi_handler *handler;
struct efi_device_path *dp1;
ret = efi_search_protocol(loaded_image_info->device_handle,
&efi_guid_device_path, &handler);
dp1 = handler->protocol_interface;
[...]
> +static efi_status_t EFIAPI
> +default_file_authentication(const struct efi_security2_arch_protocol *this,
> + const struct efi_device_path *device_path,
> + void *file_buffer, efi_uintn_t file_size,
> + bool boot_policy)
> +{
> + if (efi_image_authenticate(file_buffer, file_size))
> + return EFI_SUCCESS;
> +
> + return EFI_SECURITY_VIOLATION;
> +}
> +
> +static struct efi_security_arch_protocol efi_security_arch = {
> + .file_authentication_state = default_file_auth_state,
> +};
> +
> +static struct efi_security2_arch_protocol efi_security2_arch = {
> + .file_authentication = default_file_authentication,
> +};
> +
> +/**
> + * efi_security_check_image() - authenticate image via Security2 protocol
> + *
> + * Calls the currently installed FileAuthentication callback on the
> + * EFI_SECURITY2_ARCH_PROTOCOL instance. This may be the default handler
> + * (which delegates to efi_image_authenticate()) or a custom override
> + * installed by an EFI application such as systemd-stub.
> + *
> + * @file_path: device path of the image being loaded
> + * @file_buffer: pointer to the image data
> + * @file_size: size of the image data
> + * @boot_policy: true if image is being loaded as a boot option
> + * Return: EFI_SUCCESS if authenticated, error code otherwise
> + */
> +efi_status_t efi_security_check_image(const struct efi_device_path
> *file_path,
> + void *file_buffer, efi_uintn_t
> file_size,
> + bool boot_policy)
> +{
> + return efi_security2_arch.file_authentication(&efi_security2_arch,
> + file_path, file_buffer,
> + file_size, boot_policy);
Can't a different EFI application install that protocol? We are only
using the local copy here
> +}
> +
> +/**
> + * efi_security_arch_register() - install Security Architecture Protocols
> + *
> + * Installs both EFI_SECURITY_ARCH_PROTOCOL and EFI_SECURITY2_ARCH_PROTOCOL
> + * on the root handle during EFI subsystem initialization.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_security_arch_register(void)
> +{
> + efi_status_t ret;
> +
> + ret = efi_add_protocol(efi_root, &efi_guid_security_arch_protocol,
> + (void *)&efi_security_arch);
> + if (ret != EFI_SUCCESS) {
> + log_err("Cannot install EFI_SECURITY_ARCH_PROTOCOL\n");
> + return ret;
> + }
> +
> + ret = efi_add_protocol(efi_root, &efi_guid_security2_arch_protocol,
> + (void *)&efi_security2_arch);
> + if (ret != EFI_SUCCESS)
> + log_err("Cannot install EFI_SECURITY2_ARCH_PROTOCOL\n");
> +
> + return ret;
> +}
[...]
Thanks
/Ilias
