On Thu, 8 Apr 2021 at 16:51, Heinrich Schuchardt <[email protected]> wrote:
> On 08.04.21 12:10, Sughosh Ganu wrote: > > hi Heinrich, > > > > On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt <[email protected] > > <mailto:[email protected]>> wrote: > > > > On 08.04.21 08:53, Sughosh Ganu wrote: > > > hi Simon, > > > > > > On Wed, 7 Apr 2021 at 21:44, Simon Glass <[email protected] > > <mailto:[email protected]> > > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > > > Hi, > > > > > > On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu > > <[email protected] <mailto:[email protected]> > > > <mailto:[email protected] > > <mailto:[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 > > <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html> > > > <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html > > <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 > > > > Hello Sughosh, > > > > Could you, please, explain why there could be a need to use public > keys > > for capsule verification that are not compiled into U-Boot. I cannot > see > > how this would increase security. > > > > > > With the changes that have been made in the Makefile(patch 5/5), the > > public key is now embedded into the platform's dtb, and subsequently > > this dtb is concatenated with the u-boot binary to create a single > > u-boot.bin image. This image can then be verified during the trusted > > boot flow to check against any kind of tampering. This takes care of > > your concern of not having the public key separately on the disk, which > > makes it open to tampering, with the public key now embedded as part of > > the u-boot image. You had suggested embedding the public key as part of > > the u-boot image. I have embedded it within the platform's dtb which is > > part of the u-boot image. This becomes a generic solution which is > > platform and architecture agnostic. I believe concatenating the > > platform's dtb with the u-boot binary is the standard flow for > > production images. > > Embedding the key in the device-tree is fine. I am just trying to > understand why you need the extensibility via a weak function. > This is to provide flexibility for any platform that might have a different mechanism of passing/retrieving the public key. Some platforms might have their dtb passed from an earlier stage firmware, like tf-a. Or there could be a read-only device like a fuse which houses the keys to be used. Having a weak default would allow such platforms to implement a platform specific function to retrieve the public key. So if there is no technical disadvantage of having a weak default, I think keeping this flexibility for platforms would be good. > > > > > I cannot imagine any scenario where you would want to allow switching > > off capsule authentication if it has been built into U-Boot. > > > > > > This is only an additional knob for any user who might want to perform a > > capsule update without authentication -- with this additional knob, the > > user can use the same image for updating a capsule which does not have > > an authentication header. The user would not be required to recompile > > the image to turn off CONFIG_EFI_CAPSULE_AUTHENTICATE config option. But > > if you don't see this necessary, i can remove this additional check. In > > that case, the capsule will be authenticated when > > CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled. > > I would prefer to reduce the number of "knobs" that you have to check > when rolling out secure firmware. > Okay. I will remove this extra check in the next version. Whether the platform authenticates the capsule or not would then depend solely on the config option. -sughosh
