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. 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
