Hi Sughosh,

On Fri, 9 Apr 2021 at 23:27, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>
> hi Simon,
>
> On Fri, 9 Apr 2021 at 05:26, Simon Glass <s...@chromium.org> wrote:
>>
>> Hi Sughosh,
>>
>> On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>> >
>> > hi Simon,
>> >
>> > On Wed, 7 Apr 2021 at 21:44, Simon Glass <s...@chromium.org> wrote:
>> >>
>> >> Hi,
>> >>
>> >> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>> >> >
>> >> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option
>> >> > when capsule authentication is enabled.
>> >> >
>> >> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE
>> >> > which are used for enabling embedding of the public key in the dtb,
>> >> > and specifying the esl file name.
>> >> >
>> >> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can
>> >> > be used as a default mechanism for checking if capsule authentication
>> >> > has been enabled.
>> >> >
>> >> > Patch 4 adds a default weak function for retrieving the public key
>> >> > from the platform's dtb.
>> >> >
>> >> > Patch 5 adds the functionality to embed the esl file into the
>> >> > platform's dtb during the platform build.
>> >> >
>> >> > I have tested this functionality on the STM32MP157C DK2 board.
>> >> >
>> >> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html
>> >> >
>> >> > Sughosh Ganu (5):
>> >> >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule
>> >> >     authentication is enabled
>> >> >   efi_loader: Kconfig: Add symbols for embedding the public key into the
>> >> >     platform's dtb
>> >> >   efi_capsule: Add a weak function to check whether capsule
>> >> >     authentication is enabled
>> >> >   efi_capsule: Add a weak function to get the public key needed for
>> >> >     capsule authentication
>> >> >   Makefile: Add provision for embedding public key in platform's dtb
>> >> >
>> >> >  Makefile                              | 10 ++++++
>> >> >  board/emulation/common/qemu_capsule.c |  6 ----
>> >> >  lib/efi_loader/Kconfig                | 16 ++++++++++
>> >> >  lib/efi_loader/efi_capsule.c          | 44 ++++++++++++++++++++++++---
>> >> >  4 files changed, 66 insertions(+), 10 deletions(-)
>> >> >
>> >> > --
>> >> > 2.17.1
>> >> >
>> >>
>> >> We need to rethink the use of weak functions for this sort of thing,
>> >> or we will end up with an unnavigable mess at some point. If we need
>> >> to adjust the flow of boot, let's adjust the flow rather than adding
>> >> hooks everywhere.
>> >
>> >
>> > There are two weak functions being added. One is for retrieving the public 
>> > key to be used for the capsule authentication, and the other is for 
>> > checking for whether capsule authentication has been enabled. The reason 
>> > why a weak function is needed is because platforms can have other 
>> > mechanisms for retrieval of the public key or for testing if capsule 
>> > authentication has been enabled.
>> >
>> > If we consider the case of public key retrieval, the majority of platforms 
>> > would be built with the device tree concatenated with the u-boot binary. 
>> > The weak function would cater to all of those platforms -- having a weak 
>> > function would mean that we are not required to repeat the same 
>> > functionality for every platform that uses the same mechanism for 
>> > extracting the public key. However, there would be platforms where the 
>> > public key is obtained through a different mechanism which is platform 
>> > specific. Those platforms would have to define their own function to get 
>> > the public key. Same for checking whether capsule authentication feature 
>> > has been enabled or not.
>> >
>> > -sughosh
>>
>> Yes, I get it, but if this is to be a critical feature and part of the
>> grand new design for verified boot using UEFI, surely we should be
>> building a new front door, not digging a tunnel in the bathroom :-)
>>
>> We can either use drivers with driver model, or perhaps have a Kconfig
>> that enables calling the function (so we get a link error if not
>> provided). Or if there will be more than one handler, a linker_list.
>
>
> I will use the Kconfig symbol for selecting the function to use for 
> retrieving the public key. So, in the current scenario, the function would be 
> under the CONFIG_EFI_PKEY_DTB_EMBED symbol. This should cater to the majority 
> of the cases. If some platform wants to use a different method to get the 
> public key, it would need to define it's own function.

I wonder why another method would be needed, but if it, then someone
can turn your thing into a choice, I suppose.

Regards,
Simon

Reply via email to