[PATCH 0/5] Add support for embedding public key in platform's dtb
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(-)
Enable building of the crypto helper functions used during capsule authentication by selecting IMAGE_SIGN_INFO.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org ---
This was not detected when support for capsule auth was added to the qemu arm64 platform. This is because the platform includes CONFIG_FIT_SIGNATURE which selects IMAGE_SIGN_INFO.
lib/efi_loader/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e44f004f3f..0b99d7c774 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -173,6 +173,7 @@ config EFI_CAPSULE_AUTHENTICATE select X509_CERTIFICATE_PARSER select PKCS7_MESSAGE_PARSER select PKCS7_VERIFY + select IMAGE_SIGN_INFO default n help Select this option if you want to enable capsule
On 07.04.21 13:53, Sughosh Ganu wrote:
Enable building of the crypto helper functions used during capsule authentication by selecting IMAGE_SIGN_INFO.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
This was not detected when support for capsule auth was added to the qemu arm64 platform. This is because the platform includes CONFIG_FIT_SIGNATURE which selects IMAGE_SIGN_INFO.
lib/efi_loader/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e44f004f3f..0b99d7c774 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -173,6 +173,7 @@ config EFI_CAPSULE_AUTHENTICATE select X509_CERTIFICATE_PARSER select PKCS7_MESSAGE_PARSER select PKCS7_VERIFY
- select IMAGE_SIGN_INFO default n help Select this option if you want to enable capsule
Add config options EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE which are to be used for embedding the public key to be used for capsule authentication into the platform's device tree.
The embedding of the public key would take place during the platform build process.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- lib/efi_loader/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 0b99d7c774..de3083a979 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -179,6 +179,21 @@ config EFI_CAPSULE_AUTHENTICATE Select this option if you want to enable capsule authentication
+config EFI_PKEY_DTB_EMBED + bool "Embed the public key in the Device Tree" + default n + depends on EFI_CAPSULE_AUTHENTICATE + help + Select this option if the public key used for capsule + authentication is to be embedded into the platform's + device tree. + +config EFI_PKEY_FILE + string "Public Key esl file to be embedded into the Device Tree" + help + Specify the absolute path of the public key esl file that is + to be embedded in the platform's device tree. + config EFI_CAPSULE_FIRMWARE_FIT bool "FMP driver for FIT image" depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add config options EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE which are to be used for embedding the public key to be used for capsule authentication into the platform's device tree.
The embedding of the public key would take place during the platform build process.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
lib/efi_loader/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 0b99d7c774..de3083a979 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -179,6 +179,21 @@ config EFI_CAPSULE_AUTHENTICATE Select this option if you want to enable capsule authentication
+config EFI_PKEY_DTB_EMBED
- bool "Embed the public key in the Device Tree"
- default n
"default n" has no effect. So this line is superfluous.
But shouldn't this be default=y and tested via Python tests?
- depends on EFI_CAPSULE_AUTHENTICATE
- help
Select this option if the public key used for capsuleauthentication is to be embedded into the platform'sdevice tree.+config EFI_PKEY_FILE
- string "Public Key esl file to be embedded into the Device Tree"
Please, provide a default value (e.g. "eficapsule.esl") that we can rely on in our Python tests.
Best regards
Heinrich
- help
Specify the absolute path of the public key esl file that isto be embedded in the platform's device tree.- config EFI_CAPSULE_FIRMWARE_FIT bool "FMP driver for FIT image" depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
Define a weak function which checks if the environment variable capsule_authentication_enabled has been set, for enabling capsule authentication. Other platforms might have a different mechanism to determine this, and would then define their own platform specific function.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- board/emulation/common/qemu_capsule.c | 6 ------ lib/efi_loader/efi_capsule.c | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c index 5cb461d52b..6b8a87022a 100644 --- a/board/emulation/common/qemu_capsule.c +++ b/board/emulation/common/qemu_capsule.c @@ -41,9 +41,3 @@ int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
return 0; } - -bool efi_capsule_auth_enabled(void) -{ - return env_get("capsule_authentication_enabled") != NULL ? - true : false; -} diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 0cfff0daf7..1423b675c8 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -218,6 +218,12 @@ __weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) return 0; }
+__weak bool efi_capsule_auth_enabled(void) +{ + return env_get("capsule_authentication_enabled") ? + true : false; +} + efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size, void **image, efi_uintn_t *image_size) {
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Define a weak function which checks if the environment variable capsule_authentication_enabled has been set, for enabling capsule authentication. Other platforms might have a different mechanism to determine this, and would then define their own platform specific function.
I cannot yet understand the concrete need for such a function.
Is there any Linaro customer who actually needs it? What is the use case?
Best regards
Heinrich
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
board/emulation/common/qemu_capsule.c | 6 ------ lib/efi_loader/efi_capsule.c | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c index 5cb461d52b..6b8a87022a 100644 --- a/board/emulation/common/qemu_capsule.c +++ b/board/emulation/common/qemu_capsule.c @@ -41,9 +41,3 @@ int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
return 0; }
-bool efi_capsule_auth_enabled(void) -{
- return env_get("capsule_authentication_enabled") != NULL ?
true : false;-} diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 0cfff0daf7..1423b675c8 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -218,6 +218,12 @@ __weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) return 0; }
+__weak bool efi_capsule_auth_enabled(void) +{
- return env_get("capsule_authentication_enabled") ?
true : false;+}
- efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size, void **image, efi_uintn_t *image_size) {
On Fri, 9 Apr 2021 at 01:22, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Define a weak function which checks if the environment variable capsule_authentication_enabled has been set, for enabling capsule authentication. Other platforms might have a different mechanism to determine this, and would then define their own platform specific function.
I cannot yet understand the concrete need for such a function.
Is there any Linaro customer who actually needs it? What is the use case?
This point was discussed in another mail thread[1]. I will be dropping the additional knob, so this weak function will go away.
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2021-April/446694.html
Best regards
Heinrich
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
board/emulation/common/qemu_capsule.c | 6 ------ lib/efi_loader/efi_capsule.c | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/board/emulation/common/qemu_capsule.c
b/board/emulation/common/qemu_capsule.c
index 5cb461d52b..6b8a87022a 100644 --- a/board/emulation/common/qemu_capsule.c +++ b/board/emulation/common/qemu_capsule.c @@ -41,9 +41,3 @@ int efi_get_public_key_data(void **pkey, efi_uintn_t
*pkey_len)
return 0;}
-bool efi_capsule_auth_enabled(void) -{
return env_get("capsule_authentication_enabled") != NULL ?true : false;-} diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 0cfff0daf7..1423b675c8 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -218,6 +218,12 @@ __weak int efi_get_public_key_data(void **pkey,
efi_uintn_t *pkey_len)
return 0;}
+__weak bool efi_capsule_auth_enabled(void) +{
return env_get("capsule_authentication_enabled") ?true : false;+}
- efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t
capsule_size,
void **image, efi_uintn_t*image_size)
{
Define a weak function which would be used in the scenario where the public key is stored on the platform's dtb. This dtb is concatenated with the u-boot binary during the build process. Platforms which have a different mechanism for getting the public key would define their own platform specific function.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- lib/efi_loader/efi_capsule.c | 38 ++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1423b675c8..fc5e1c0856 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -14,10 +14,13 @@ #include <mapmem.h> #include <sort.h>
+#include <asm/global_data.h> #include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h>
+DECLARE_GLOBAL_DATA_PTR; + const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; static const efi_guid_t efi_guid_firmware_management_capsule_id = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; @@ -210,11 +213,38 @@ const efi_guid_t efi_guid_capsule_root_cert_guid =
__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) { - /* The platform is supposed to provide - * a method for getting the public key - * stored in the form of efi signature - * list + /* + * This is a function for retrieving the public key from the + * platform's device tree. The platform's device tree has been + * concatenated with the u-boot binary. + * If a platform has a different mechanism to get the public + * key, it can define it's own function. */ + const void *fdt_blob = gd->fdt_blob; + const void *blob; + const char *cnode_name = "capsule-key"; + const char *snode_name = "signature"; + int sig_node; + int len; + + sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name); + if (sig_node < 0) { + EFI_PRINT("Unable to get signature node offset\n"); + return -FDT_ERR_NOTFOUND; + } + + blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len); + + if (!blob || len < 0) { + EFI_PRINT("Unable to get capsule-key value\n"); + *pkey = NULL; + *pkey_len = 0; + return -FDT_ERR_NOTFOUND; + } + + *pkey = (void *)blob; + *pkey_len = len; + return 0; }
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Define a weak function which would be used in the scenario where the public key is stored on the platform's dtb. This dtb is concatenated with the u-boot binary during the build process. Platforms which have a different mechanism for getting the public key would define their own platform specific function.
Storing the public key in U-Boot's dtb is reasonable. But what is the use case for a weak function?
Best regards
Heinrich
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
lib/efi_loader/efi_capsule.c | 38 ++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1423b675c8..fc5e1c0856 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -14,10 +14,13 @@ #include <mapmem.h> #include <sort.h>
+#include <asm/global_data.h> #include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h>
+DECLARE_GLOBAL_DATA_PTR;
- const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; static const efi_guid_t efi_guid_firmware_management_capsule_id = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
@@ -210,11 +213,38 @@ const efi_guid_t efi_guid_capsule_root_cert_guid =
__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) {
- /* The platform is supposed to provide
* a method for getting the public key* stored in the form of efi signature* list
- /*
* This is a function for retrieving the public key from the* platform's device tree. The platform's device tree has been* concatenated with the u-boot binary.* If a platform has a different mechanism to get the public* key, it can define it's own function.- const void *fdt_blob = gd->fdt_blob;
- const void *blob;
- const char *cnode_name = "capsule-key";
- const char *snode_name = "signature";
- int sig_node;
- int len;
- sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
- if (sig_node < 0) {
EFI_PRINT("Unable to get signature node offset\n");return -FDT_ERR_NOTFOUND;- }
- blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
- if (!blob || len < 0) {
EFI_PRINT("Unable to get capsule-key value\n");*pkey = NULL;*pkey_len = 0;return -FDT_ERR_NOTFOUND;- }
- *pkey = (void *)blob;
- *pkey_len = len;
- return 0; }
On Fri, 9 Apr 2021 at 01:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Define a weak function which would be used in the scenario where the public key is stored on the platform's dtb. This dtb is concatenated with the u-boot binary during the build process. Platforms which have a different mechanism for getting the public key would define their own platform specific function.
Storing the public key in U-Boot's dtb is reasonable. But what is the use case for a weak function?
This point was discussed in another mail thread[1].
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2021-April/446694.html
Best regards
Heinrich
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
lib/efi_loader/efi_capsule.c | 38 ++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1423b675c8..fc5e1c0856 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -14,10 +14,13 @@ #include <mapmem.h> #include <sort.h>
+#include <asm/global_data.h> #include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h>
+DECLARE_GLOBAL_DATA_PTR;
- const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; static const efi_guid_t efi_guid_firmware_management_capsule_id = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
@@ -210,11 +213,38 @@ const efi_guid_t efi_guid_capsule_root_cert_guid =
__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) {
/* The platform is supposed to provide* a method for getting the public key* stored in the form of efi signature* list
/** This is a function for retrieving the public key from the* platform's device tree. The platform's device tree has been* concatenated with the u-boot binary.* If a platform has a different mechanism to get the public* key, it can define it's own function. */const void *fdt_blob = gd->fdt_blob;const void *blob;const char *cnode_name = "capsule-key";const char *snode_name = "signature";int sig_node;int len;sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);if (sig_node < 0) {EFI_PRINT("Unable to get signature node offset\n");return -FDT_ERR_NOTFOUND;}blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);if (!blob || len < 0) {EFI_PRINT("Unable to get capsule-key value\n");*pkey = NULL;*pkey_len = 0;return -FDT_ERR_NOTFOUND;}*pkey = (void *)blob;*pkey_len = len;return 0;
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \ + -D $@ + cfg: u-boot.cfg
quiet_cmd_cfgcheck = CFGCHK $2 @@ -1104,8 +1108,14 @@ endif PHONY += dtbs dtbs: dts/dt.dtb @: +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE)$(CONFIG_EFI_PKEY_DTB_EMBED),yy) +dts/dt.dtb: u-boot tools + $(Q)$(MAKE) $(build)=dts dtbs + $(call cmd,mkeficapsule) +else dts/dt.dtb: u-boot $(Q)$(MAKE) $(build)=dts dtbs +endif
quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \
- -D $@
tools/mkeficapsule --help does neither show a parameter -K nor a parameter -D. Please, update tools/mkeficapsule.c before using these. A man-page for mkeficapsule in doc/usage/ would be helpful.
$ tools/mkeficapsule --help Usage: mkeficapsule [options] <output file> Options: --fit <fit image> new FIT image file --raw <raw image> new raw image file --index <index> update image index --instance <instance> update hardware instance --public-key <key file> public key esl file --dtb <dtb file> dtb file --overlay the dtb file is an overlay --help print a help message
Best regards
Heinrich
cfg: u-boot.cfg
quiet_cmd_cfgcheck = CFGCHK $2 @@ -1104,8 +1108,14 @@ endif PHONY += dtbs dtbs: dts/dt.dtb @: +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE)$(CONFIG_EFI_PKEY_DTB_EMBED),yy) +dts/dt.dtb: u-boot tools
- $(Q)$(MAKE) $(build)=dts dtbs
- $(call cmd,mkeficapsule)
+else dts/dt.dtb: u-boot $(Q)$(MAKE) $(build)=dts dtbs +endif
quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
On Thu, Apr 08, 2021 at 09:58:17PM +0200, Heinrich Schuchardt wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \
- -D $@
tools/mkeficapsule --help does neither show a parameter -K nor a parameter -D.
This clearly shows that the feature with -K/-D has nothing to do with creating a capsule file. Two totally different things in one place (command). And the dtb overlay operation can be achieved by using standard commands.
I believe that the feature should be removed from mkeficapsule.
-Takahiro Akashi
Please, update tools/mkeficapsule.c before using these. A man-page for mkeficapsule in doc/usage/ would be helpful.
$ tools/mkeficapsule --help Usage: mkeficapsule [options] <output file> Options: --fit <fit image> new FIT image file --raw <raw image> new raw image file --index <index> update image index --instance <instance> update hardware instance --public-key <key file> public key esl file --dtb <dtb file> dtb file --overlay the dtb file is an overlay --help print a help message
Best regards
Heinrich
cfg: u-boot.cfg
quiet_cmd_cfgcheck = CFGCHK $2 @@ -1104,8 +1108,14 @@ endif PHONY += dtbs dtbs: dts/dt.dtb @: +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE)$(CONFIG_EFI_PKEY_DTB_EMBED),yy) +dts/dt.dtb: u-boot tools
- $(Q)$(MAKE) $(build)=dts dtbs
- $(call cmd,mkeficapsule)
+else dts/dt.dtb: u-boot $(Q)$(MAKE) $(build)=dts dtbs +endif
quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
2021年4月28日(水) 14:44 AKASHI Takahiro takahiro.akashi@linaro.org:
On Thu, Apr 08, 2021 at 09:58:17PM +0200, Heinrich Schuchardt wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \
- -D $@
tools/mkeficapsule --help does neither show a parameter -K nor a parameter -D.
This clearly shows that the feature with -K/-D has nothing to do with creating a capsule file. Two totally different things in one place (command). And the dtb overlay operation can be achieved by using standard commands.
If I understand correctly, we need the following steps, 1. prepare the key for signing 2. make dtb overlay from that key 3. sign the capsule with the key
And Sughosh's implementation is using mkeficapsule for 2 and 3. Takahiro pointed that mkeficapsule is only for 3 because of its name and avoid confusion.
Is that correct?
What would you think about changing the tool name? E.g.
For step 2. capsuletool dtb --public-key pubkey [--overlay] target.dtb
For step 3. capsuletool capsule --raw u-boot.bin --index 1 --public-key pubkey u-boot.cap
Then we can expand it for inspection, verify etc.
Thank you,
I believe that the feature should be removed from mkeficapsule.
-Takahiro Akashi
Please, update tools/mkeficapsule.c before using these. A man-page for mkeficapsule in doc/usage/ would be helpful.
$ tools/mkeficapsule --help Usage: mkeficapsule [options] <output file> Options: --fit <fit image> new FIT image file --raw <raw image> new raw image file --index <index> update image index --instance <instance> update hardware instance --public-key <key file> public key esl file --dtb <dtb file> dtb file --overlay the dtb file is an overlay --help print a help message
Best regards
Heinrich
cfg: u-boot.cfg
quiet_cmd_cfgcheck = CFGCHK $2 @@ -1104,8 +1108,14 @@ endif PHONY += dtbs dtbs: dts/dt.dtb @: +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE)$(CONFIG_EFI_PKEY_DTB_EMBED),yy) +dts/dt.dtb: u-boot tools
- $(Q)$(MAKE) $(build)=dts dtbs
- $(call cmd,mkeficapsule)
+else dts/dt.dtb: u-boot $(Q)$(MAKE) $(build)=dts dtbs +endif
quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
On Wed, Apr 28, 2021 at 03:31:36PM +0900, Masami Hiramatsu wrote:
2021年4月28日(水) 14:44 AKASHI Takahiro takahiro.akashi@linaro.org:
On Thu, Apr 08, 2021 at 09:58:17PM +0200, Heinrich Schuchardt wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \
- -D $@
tools/mkeficapsule --help does neither show a parameter -K nor a parameter -D.
This clearly shows that the feature with -K/-D has nothing to do with creating a capsule file. Two totally different things in one place (command). And the dtb overlay operation can be achieved by using standard commands.
If I understand correctly, we need the following steps,
- prepare the key for signing
- make dtb overlay from that key
- sign the capsule with the key
And Sughosh's implementation is using mkeficapsule for 2 and 3. Takahiro pointed that mkeficapsule is only for 3 because of its name and avoid confusion.
Is that correct?
What would you think about changing the tool name? E.g.
For step 2. capsuletool dtb --public-key pubkey [--overlay] target.dtb
My point is: as this command line shows, it has nothing to do with a capsule file. It simply deals with dtb blob for overlaying. (So 'capsuletool' is not appropriate.)
-Takahiro Akashi
For step 3. capsuletool capsule --raw u-boot.bin --index 1 --public-key pubkey u-boot.cap
Then we can expand it for inspection, verify etc.
Thank you,
I believe that the feature should be removed from mkeficapsule.
-Takahiro Akashi
Please, update tools/mkeficapsule.c before using these. A man-page for mkeficapsule in doc/usage/ would be helpful.
$ tools/mkeficapsule --help Usage: mkeficapsule [options] <output file> Options: --fit <fit image> new FIT image file --raw <raw image> new raw image file --index <index> update image index --instance <instance> update hardware instance --public-key <key file> public key esl file --dtb <dtb file> dtb file --overlay the dtb file is an overlay --help print a help message
Best regards
Heinrich
cfg: u-boot.cfg
quiet_cmd_cfgcheck = CFGCHK $2 @@ -1104,8 +1108,14 @@ endif PHONY += dtbs dtbs: dts/dt.dtb @: +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE)$(CONFIG_EFI_PKEY_DTB_EMBED),yy) +dts/dt.dtb: u-boot tools
- $(Q)$(MAKE) $(build)=dts dtbs
- $(call cmd,mkeficapsule)
+else dts/dt.dtb: u-boot $(Q)$(MAKE) $(build)=dts dtbs +endif
quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
-- Masami Hiramatsu
Hi,
2021年5月7日(金) 17:15 AKASHI Takahiro takahiro.akashi@linaro.org:
On Wed, Apr 28, 2021 at 03:31:36PM +0900, Masami Hiramatsu wrote:
2021年4月28日(水) 14:44 AKASHI Takahiro takahiro.akashi@linaro.org:
On Thu, Apr 08, 2021 at 09:58:17PM +0200, Heinrich Schuchardt wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \
- -D $@
tools/mkeficapsule --help does neither show a parameter -K nor a parameter -D.
This clearly shows that the feature with -K/-D has nothing to do with creating a capsule file. Two totally different things in one place (command). And the dtb overlay operation can be achieved by using standard commands.
If I understand correctly, we need the following steps,
- prepare the key for signing
- make dtb overlay from that key
- sign the capsule with the key
And Sughosh's implementation is using mkeficapsule for 2 and 3. Takahiro pointed that mkeficapsule is only for 3 because of its name and avoid confusion.
Is that correct?
What would you think about changing the tool name? E.g.
For step 2. capsuletool dtb --public-key pubkey [--overlay] target.dtb
My point is: as this command line shows, it has nothing to do with a capsule file. It simply deals with dtb blob for overlaying. (So 'capsuletool' is not appropriate.)
But if the capsuletool provide the devicetree template for the capsule something like test/py/tests/test_efi_capsule/pubkey.dts, we can say it is related to the capsule, because the dts is obviously for capsule. What would you think?
Thank you,
-Takahiro Akashi
For step 3. capsuletool capsule --raw u-boot.bin --index 1 --public-key pubkey u-boot.cap
Then we can expand it for inspection, verify etc.
Thank you,
I believe that the feature should be removed from mkeficapsule.
-Takahiro Akashi
Please, update tools/mkeficapsule.c before using these. A man-page for mkeficapsule in doc/usage/ would be helpful.
$ tools/mkeficapsule --help Usage: mkeficapsule [options] <output file> Options: --fit <fit image> new FIT image file --raw <raw image> new raw image file --index <index> update image index --instance <instance> update hardware instance --public-key <key file> public key esl file --dtb <dtb file> dtb file --overlay the dtb file is an overlay --help print a help message
Best regards
Heinrich
cfg: u-boot.cfg
quiet_cmd_cfgcheck = CFGCHK $2 @@ -1104,8 +1108,14 @@ endif PHONY += dtbs dtbs: dts/dt.dtb @: +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE)$(CONFIG_EFI_PKEY_DTB_EMBED),yy) +dts/dt.dtb: u-boot tools
- $(Q)$(MAKE) $(build)=dts dtbs
- $(call cmd,mkeficapsule)
+else dts/dt.dtb: u-boot $(Q)$(MAKE) $(build)=dts dtbs +endif
quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
-- Masami Hiramatsu
-- Masami Hiramatsu
2021年5月7日(金) 18:57 Masami Hiramatsu masami.hiramatsu@linaro.org:
Hi,
2021年5月7日(金) 17:15 AKASHI Takahiro takahiro.akashi@linaro.org:
On Wed, Apr 28, 2021 at 03:31:36PM +0900, Masami Hiramatsu wrote:
2021年4月28日(水) 14:44 AKASHI Takahiro takahiro.akashi@linaro.org:
On Thu, Apr 08, 2021 at 09:58:17PM +0200, Heinrich Schuchardt wrote:
On 4/7/21 1:53 PM, Sughosh Ganu wrote:
Add provision for embedding the public key used for capsule authentication in the platform's dtb. This is done by invoking the mkeficapsule utility which puts the public key in the efi signature list(esl) format into the dtb.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Makefile b/Makefile index 193aa4d1c9..0d50c6a805 100644 --- a/Makefile +++ b/Makefile @@ -1010,6 +1010,10 @@ cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@
+quiet_cmd_mkeficapsule = MKEFICAPSULE $@ +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule -K $(CONFIG_EFI_PKEY_FILE) \
- -D $@
tools/mkeficapsule --help does neither show a parameter -K nor a parameter -D.
This clearly shows that the feature with -K/-D has nothing to do with creating a capsule file. Two totally different things in one place (command). And the dtb overlay operation can be achieved by using standard commands.
If I understand correctly, we need the following steps,
- prepare the key for signing
- make dtb overlay from that key
- sign the capsule with the key
And Sughosh's implementation is using mkeficapsule for 2 and 3. Takahiro pointed that mkeficapsule is only for 3 because of its name and avoid confusion.
Is that correct?
What would you think about changing the tool name? E.g.
For step 2. capsuletool dtb --public-key pubkey [--overlay] target.dtb
My point is: as this command line shows, it has nothing to do with a capsule file. It simply deals with dtb blob for overlaying. (So 'capsuletool' is not appropriate.)
But if the capsuletool provide the devicetree template for the capsule something like test/py/tests/test_efi_capsule/pubkey.dts, we can say it is related to the capsule, because the dts is obviously for capsule. What would you think?
Ah, wait. I misunderstood. It seems that the efi_get_public_key_data() is platform dependent. Thus isn't it hard to provide a unified tool to embed the key data into the dtb because it is usable for some platform but not usable for others?
Thank you,
Hi,
On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu sughosh.ganu@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.
Regards, Simon
hi Simon,
On Wed, 7 Apr 2021 at 21:44, Simon Glass sjg@chromium.org wrote:
Hi,
On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu sughosh.ganu@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
On 08.04.21 08:53, Sughosh Ganu wrote:
hi Simon,
On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org mailto:sjg@chromium.org> wrote:
Hi, On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org <mailto:sughosh.ganu@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 <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.
I cannot imagine any scenario where you would want to allow switching off capsule authentication if it has been built into U-Boot.
Best regards
Heinrich
hi Heinrich,
On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 08.04.21 08:53, Sughosh Ganu wrote:
hi Simon,
On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org mailto:sjg@chromium.org> wrote:
Hi, On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org <mailto:sughosh.ganu@linaro.org>> wrote: > > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO configoption
> when capsule authentication is enabled. > > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED andEFI_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, whichcan
> be used as a default mechanism for checking if capsuleauthentication
> 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> > > 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'sdtb
> > 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.
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.
-sughosh
On 08.04.21 12:10, Sughosh Ganu wrote:
hi Heinrich,
On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt <xypron.glpk@gmx.de mailto:xypron.glpk@gmx.de> wrote:
On 08.04.21 08:53, Sughosh Ganu wrote: > hi Simon, > > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org> > <mailto:sjg@chromium.org <mailto:sjg@chromium.org>>> wrote: > > Hi, > > On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org <mailto:sughosh.ganu@linaro.org> > <mailto:sughosh.ganu@linaro.org <mailto:sughosh.ganu@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 <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.
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.
Best regards
Heinrich
On Thu, 8 Apr 2021 at 16:51, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 08.04.21 12:10, Sughosh Ganu wrote:
hi Heinrich,
On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt <xypron.glpk@gmx.de mailto:xypron.glpk@gmx.de> wrote:
On 08.04.21 08:53, Sughosh Ganu wrote: > hi Simon, > > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org> > <mailto:sjg@chromium.org <mailto:sjg@chromium.org>>> wrote: > > Hi, > > On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org <mailto:sughosh.ganu@linaro.org> > <mailto:sughosh.ganu@linaro.org <mailto:sughosh.ganu@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 DK2board.
> > > > [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 publickey
> 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 theother 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 ifcapsule
> 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 thoseplatforms
> -- having a weak function would mean that we are not required to repeat > the same functionality for every platform that uses the samemechanism
> for extracting the public key. However, there would be platformswhere
> 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 publickeys
for capsule verification that are not compiled into U-Boot. I cannotsee
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
Hi Sughosh,
On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 7 Apr 2021 at 21:44, Simon Glass sjg@chromium.org wrote:
Hi,
On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu sughosh.ganu@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.
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
hi Simon,
On Fri, 9 Apr 2021 at 05:26, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 7 Apr 2021 at 21:44, Simon Glass sjg@chromium.org wrote:
Hi,
On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu sughosh.ganu@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 dtbefi_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
Hi Sughosh,
On Fri, 9 Apr 2021 at 23:27, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Fri, 9 Apr 2021 at 05:26, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 7 Apr 2021 at 21:44, Simon Glass sjg@chromium.org wrote:
Hi,
On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu sughosh.ganu@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
participants (5)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Masami Hiramatsu -
Simon Glass -
Sughosh Ganu