[U-Boot] [PATCH] fit: skip signature verification if board request
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org --- common/image-sig.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index d9f712f..f3d1252 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -151,6 +151,11 @@ struct image_region *fit_region_make_list(const void *fit, return region; }
+int __attribute__((weak)) fit_board_skip_sig_verification(void) +{ + return 0; +} + static int fit_image_setup_verify(struct image_sign_info *info, const void *fit, int noffset, int required_keynode, char **err_msgp) @@ -188,6 +193,12 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data, uint8_t *fit_value; int fit_value_len;
+ /* Skip verification if board says that */ + if (fit_board_skip_sig_verification()) { + printf("signature check skipped\n"); + return 0; + } + *err_msgp = NULL; if (fit_image_setup_verify(&info, fit, noffset, required_keynode, err_msgp)) @@ -438,6 +449,12 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, int noffset; int sig_node;
+ /* Skip verification if board says that */ + if (fit_board_skip_sig_verification()) { + printf("signature check skipped\n"); + return 0; + } + /* Work out what we need to verify */ sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME); if (sig_node < 0) {
On Wed, Apr 11, 2018 at 11:13:05PM +0800, Jun Nie wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org
common/image-sig.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index d9f712f..f3d1252 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -151,6 +151,11 @@ struct image_region *fit_region_make_list(const void *fit, return region; }
+int __attribute__((weak)) fit_board_skip_sig_verification(void) +{
- return 0;
+}
static int fit_image_setup_verify(struct image_sign_info *info, const void *fit, int noffset, int required_keynode, char **err_msgp) @@ -188,6 +193,12 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data, uint8_t *fit_value; int fit_value_len;
- /* Skip verification if board says that */
- if (fit_board_skip_sig_verification()) {
printf("signature check skipped\n");return 0;- }
- *err_msgp = NULL; if (fit_image_setup_verify(&info, fit, noffset, required_keynode, err_msgp))
@@ -438,6 +449,12 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, int noffset; int sig_node;
- /* Skip verification if board says that */
- if (fit_board_skip_sig_verification()) {
printf("signature check skipped\n");return 0;- }
- /* Work out what we need to verify */ sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME); if (sig_node < 0) {
I'm not sure I like the concept here. Wouldn't this make it easier to break in to a secure setup with some binary editing? Or is that really no worse than today? Also, can you please follow up with an implementation of fit_board_skip_sig_verification? Thanks!
2018-04-12 21:53 GMT+08:00 Tom Rini trini@konsulko.com:
On Wed, Apr 11, 2018 at 11:13:05PM +0800, Jun Nie wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org
I'm not sure I like the concept here. Wouldn't this make it easier to break in to a secure setup with some binary editing? Or is that really no worse than today? Also, can you please follow up with an implementation of fit_board_skip_sig_verification? Thanks!
-- Tom
SoC boot ROM shall verify signature of SPL before running. Binary editing shall make signature invalid. If it is possible to run an edited SPL, the secure boot is already hacked and this patch does not make it worse. I surely will post implementation, which is just reading some register to get fuse value for lock status. I post this patch to get opinions from community first.
Jun
Hi,
On 11 April 2018 at 09:13, Jun Nie jun.nie@linaro.org wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org
common/image-sig.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index d9f712f..f3d1252 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -151,6 +151,11 @@ struct image_region *fit_region_make_list(const void *fit, return region; }
+int __attribute__((weak)) fit_board_skip_sig_verification(void) +{
return 0;+}
static int fit_image_setup_verify(struct image_sign_info *info, const void *fit, int noffset, int required_keynode, char **err_msgp) @@ -188,6 +193,12 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data, uint8_t *fit_value; int fit_value_len;
/* Skip verification if board says that */if (fit_board_skip_sig_verification()) {printf("signature check skipped\n");return 0;}
Instead of a weak function can you please add a parameter to this function (perhaps a flags word?) and a add test for this case to the test?
Regards, Simon
2018-04-13 1:24 GMT+08:00 Simon Glass sjg@chromium.org:
Hi,
On 11 April 2018 at 09:13, Jun Nie jun.nie@linaro.org wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org
Instead of a weak function can you please add a parameter to this function (perhaps a flags word?) and a add test for this case to the test?
Regards, Simon
Sure, I can add a parameter to the function. But not sure what's the usage of it in your mind. Maybe "int flag" is enough for you?
Do you mean add implementation of this function for a specific platform for "add test"?
Best Regards, Jun
Hi Jun,
On 13 April 2018 at 04:05, Jun Nie jun.nie@linaro.org wrote:
2018-04-13 1:24 GMT+08:00 Simon Glass sjg@chromium.org:
Hi,
On 11 April 2018 at 09:13, Jun Nie jun.nie@linaro.org wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org
Instead of a weak function can you please add a parameter to this function (perhaps a flags word?) and a add test for this case to the test?
Regards, Simon
Sure, I can add a parameter to the function. But not sure what's the usage of it in your mind. Maybe "int flag" is enough for you?
Yes a flag is fine if you define an enum in the header file for that function. You will likely need to pass the flag around a few other functions.
Do you mean add implementation of this function for a specific platform for "add test"?
See test/py/tests/test_vboot.py which you should be able to modify for your case.
Regards, Simon
2018-04-17 3:06 GMT+08:00 Simon Glass sjg@chromium.org:
Hi Jun,
On 13 April 2018 at 04:05, Jun Nie jun.nie@linaro.org wrote:
2018-04-13 1:24 GMT+08:00 Simon Glass sjg@chromium.org:
Hi,
On 11 April 2018 at 09:13, Jun Nie jun.nie@linaro.org wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Signed-off-by: Jun Nie jun.nie@linaro.org
Instead of a weak function can you please add a parameter to this function (perhaps a flags word?) and a add test for this case to the test?
Regards, Simon
Sure, I can add a parameter to the function. But not sure what's the usage of it in your mind. Maybe "int flag" is enough for you?
Yes a flag is fine if you define an enum in the header file for that function. You will likely need to pass the flag around a few other functions.
I am still unclear on what the usage of this flag. I mean what variable from fit_image_setup_verify() and fit_config_verify_required_sigs() to be feed to this function as a flag. Maybe a void pointer is better for different platform to pass context data and cost to specific data structure. For example, pass data of image that to be verified. fit_board_skip_sig_verification(const void *data)
Do you mean add implementation of this function for a specific platform for "add test"?
See test/py/tests/test_vboot.py which you should be able to modify for your case.
Will check this file for more modification and test.
Thank you!
Regards, Simon
Hi,
On Wed, Apr 11, 2018 at 11:13:05PM +0800, Jun Nie wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Isn't it what the environment variable `verify` is made for?
i.e. setting verify=no will skip checks and boot an image even though it isn't signed or hash/signature does not match.
I may be missing some context here, so please ignore if it's not what you're after.
BTW, I saw that you were speaking of reading the lock fuse to decide whether to check the signature or not. I'd like to have at least a bypass option for this as it would be horribly tedious for debugging/development purposes. E.g. I want to be able to boot from an unverified U-Boot binary a signed (and checked) fitImage so that I can validate everything works as it should before locking down the bootloader.
Regards, Quentin
Signed-off-by: Jun Nie jun.nie@linaro.org
common/image-sig.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index d9f712f..f3d1252 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -151,6 +151,11 @@ struct image_region *fit_region_make_list(const void *fit, return region; }
+int __attribute__((weak)) fit_board_skip_sig_verification(void) +{
- return 0;
+}
static int fit_image_setup_verify(struct image_sign_info *info, const void *fit, int noffset, int required_keynode, char **err_msgp) @@ -188,6 +193,12 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data, uint8_t *fit_value; int fit_value_len;
- /* Skip verification if board says that */
- if (fit_board_skip_sig_verification()) {
printf("signature check skipped\n");return 0;- }
- *err_msgp = NULL; if (fit_image_setup_verify(&info, fit, noffset, required_keynode, err_msgp))
@@ -438,6 +449,12 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, int noffset; int sig_node;
- /* Skip verification if board says that */
- if (fit_board_skip_sig_verification()) {
printf("signature check skipped\n");return 0;- }
- /* Work out what we need to verify */ sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME); if (sig_node < 0) {
-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
2018-04-17 20:06 GMT+08:00 Quentin Schulz quentin.schulz@bootlin.com:
Hi,
On Wed, Apr 11, 2018 at 11:13:05PM +0800, Jun Nie wrote:
It may be unnecessary to check signature on unlocked board. Get the hint from platform specific code to support secure boot and non-secure boot with the same binary, so that boot is not blocked if board is not locked and has no key for signature verification.
Isn't it what the environment variable `verify` is made for?
i.e. setting verify=no will skip checks and boot an image even though it isn't signed or hash/signature does not match.
I may be missing some context here, so please ignore if it's not what you're after.
Thanks for pointing me for this. I check code and find that this variable does not cover all signature verification cases, such as fit_image_verify(). There is no variable in SPL neither, I suppose.
BTW, I saw that you were speaking of reading the lock fuse to decide whether to check the signature or not. I'd like to have at least a bypass option for this as it would be horribly tedious for debugging/development purposes. E.g. I want to be able to boot from an unverified U-Boot binary a signed (and checked) fitImage so that I can validate everything works as it should before locking down the bootloader.
For this case, it is OK if you do not detect lock fuse value and use default weak function to indicate no skipping.
Regards, Quentin
Signed-off-by: Jun Nie jun.nie@linaro.org
common/image-sig.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index d9f712f..f3d1252 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -151,6 +151,11 @@ struct image_region *fit_region_make_list(const void *fit, return region; }
+int __attribute__((weak)) fit_board_skip_sig_verification(void) +{
return 0;+}
static int fit_image_setup_verify(struct image_sign_info *info, const void *fit, int noffset, int required_keynode, char **err_msgp) @@ -188,6 +193,12 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data, uint8_t *fit_value; int fit_value_len;
/* Skip verification if board says that */if (fit_board_skip_sig_verification()) {printf("signature check skipped\n");return 0;}*err_msgp = NULL; if (fit_image_setup_verify(&info, fit, noffset, required_keynode, err_msgp))@@ -438,6 +449,12 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, int noffset; int sig_node;
/* Skip verification if board says that */if (fit_board_skip_sig_verification()) {printf("signature check skipped\n");return 0;}/* Work out what we need to verify */ sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME); if (sig_node < 0) {-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (4)
-
Jun Nie -
Quentin Schulz -
Simon Glass -
Tom Rini