hi Simon, On Fri, 9 Apr 2021 at 05:26, Simon Glass <[email protected]> wrote:
> Hi Sughosh, > > On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu <[email protected]> wrote: > > > > hi Simon, > > > > On Wed, 7 Apr 2021 at 21:44, Simon Glass <[email protected]> wrote: > >> > >> Hi, > >> > >> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <[email protected]> > 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. -sughosh > > Perhaps it is time to consider a 'hook' system, with a command to let > us see what hooks are active for any particular event? > > Regards, > Simon >

