[PATCH 1/4] tools: mkeficapsule: add firmwware image signing

Heinrich Schuchardt
2021-05-13T10:40:18Z

On 5/13/21 10:38 AM, AKASHI Takahiro wrote:
> On Thu, May 13, 2021 at 05:18:36PM +0900, Masami Hiramatsu wrote:
>> 2021?5?13?(?) 16:24 AKASHI Takahiro <takahiro.akashi@linaro.org>:
>>
>>>>>>> BTW, IMHO, if u-boot.bin can not find the ESL in the device tree,
>>>>>>> it should skip authentication too.
>>>>>>
>>>>>> In this case the capsule should be rejected (if
>>>>>> CONFIG_EFI_CAPSULE_AUTHENTICATE=y).
>>>>>
>>>>> That's basically right.
>>>>> But as I mentioned in my comment against Sughosh's patch,
>>>>> the authentication process will be enforced only if the capsule has
>>>>> an attribute, IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED.
>>>>>
>>>>
>>>> That would be a security desaster.
>>>
>>> The requirement that I mentioned above is clearly described
>>> in UEFI specification.
>>> If you think that it is a disaster, please discuss the topic
>>> in UEFI Forum first.
>>
>> I confirmed UEFI specification, version 2.7, Section.23.1
>> the last of EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo()
>>
>> -----------------
>> If IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED is supported and clear, then
>> authentication is not required to perform the firmware image operations.
>> -----------------
>
> Thank you for citing this.

This is the fraudulent code:

lib/efi_loader/efi_firmware.c:195

/* Check if the capsule authentication is enabled */
if (env_get("capsule_authentication_enabled"))
image_info[0].attributes_setting |=
IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;

It is not allowable that a user can disable image authentication by
deleting the environment.

Best regards

Heinrich

>
>> Oh, this is really crazy because deciding whether to authenticate the
>> suspicious
>> package or not, depends on whether the package said "please
>> authenticate me" or not. :D
>
> Well, the attributes can been fetched with GetInfo API, but
> how it is managed depends on the implementation of FMP drivers.
>
> As I proposed somewhere else, those attributes should be
> maintained in a separate place (maybe as part of system's policy),
> presumably ESRT or platform-specific internal database?
>
> -Takahiro Akashi
>
>
>> Anyway, since this behavior follows the specification, it should be
>> kept by default,
>> but also IMHO, there should be a CONFIG option to enforce capsule
>> authentication always.
>>
>> Thank you,
>>
>>
>>
>> --
>> Masami Hiramatsu