boot: SHA512 verification #1967

michalek-no · 2024-05-28T08:35:06Z

adds TLV and Kconfig to decouple verification from other options.

boot/bootutil/include/bootutil/crypto/sha.h

nordicjm · 2024-07-25T07:52:12Z

boot/zephyr/Kconfig

+
+config BOOT_SIGNATURE_TYPE_NONE_SHA512
+	bool "No signature; use only sha512 check"


Not really sure what the point of this change is? The hash would only be for detecting bit flips, the chance of having a bit flip and a sha256 hash still matching is basically impossible, so not sure why allowing a sha512 for this would improve anything?

In general yes, but it's security requirement we got.

AFAIK BOOT_SIGNATURE_TYPE_NONE was introduced for giving SW evaluation (for development testing) value. It is not intended to use in any real product. If we are not going to use BOOT_SIGNATURE_TYPE_NONE_SHA512 for anything like this - then it is not needed.

de-nordic · 2024-08-29T16:16:44Z

Sorry, got to add own sign off on the thing because I just added one line.

nordicjm · 2024-09-02T15:16:55Z

boot/zephyr/Kconfig

+config BOOT_USE_PSA_CRYPTO
+	bool
+	# Hidden option
+	default n


default n is never needed

Not addressed

nordicjm · 2024-09-02T15:18:01Z

boot/zephyr/Kconfig

+	  Hash algorithm used for image verification. Selection
+	  here may be limited by other configurations, like for
+	  example selected cryptographic signature.
+	default BOOT_IMG_HASH_ALG_SHA256 if BOOT_IMG_HASH_ALG_SHA256_ALLOW


default goes above help text, help text is always last. In addition the list of options seems reversed, this means if you supported sha512 and sha256, you would always default to sha256

nordicjm · 2024-09-02T15:18:38Z

boot/zephyr/Kconfig

+config BOOT_IMG_HASH_ALG_SHA512
+	bool "SHA512"
+	depends on BOOT_IMG_HASH_ALG_SHA512_ALLOW
+	depends on BOOT_USE_PSA_CRYPT


this should be part of the depends on for the allow part

ok will move to allow.

Moved it back, I have wasted too much time fighting with conflicts in Kconfig.

OK, fixed it finally.

nordicjm · 2024-09-02T15:19:53Z

boot/zephyr/Kconfig

 	select BOOT_USE_TINYCRYPT
+	select BOOT_IMG_HASH_ALG_SHA256_ALLOW
+	select BOOT_IMG_HASH_ALG_SHA512_ALLOW


Don't see the point in sha512 here, this means you have no signature, so you could use a crc16 really, sha512 from sha256 adds nothing in this configuration

Actually there is significant difference between sha and crc in this context, although I admit that with the sha256 vs sha512 not so much, without signature verification.
But, because there is always one, user may have hardware accelerator that only supports sha512 calculation, which is often the case with hardware with ed25519 accelerators, as the sha512 is the sha of choice for that signature.

Such hardware will often not have the sha256, so, even if ed25519 is not used in the end, the hashing of image will be faster when sha512 is used.

But that's the point, this is only used either for development where you want nil security or whereby you have a project that has nil security, it doesn't really have any real world use for a product, so even if it is slower, that doesn't matter, this is working fine on an nrf51 which is a cortex m0 from over a decade ago, I don't see the speed of verifying this ever being a problem for it's intended purposes, especially on more modern architectures. Plus it has absolutely no support in MCUmgr tools

imgtool support is here: #2048, it is based on older version of this PR.
MCUmgr support could be added at some point, and it could also use the sha512 coprocessor on devices that have it.

Crypto engines often have better power characteristics than doing the same hashing on CPU, and the reduction of time may be significant.
In my experimentation with 32kB image, I had following results:

by chunks with read to mem by flash_read - 5 to 6ms

by chunks, but with direct access to storage, crypto engine doing the DMA - 3ms

by whole image at once, crypto doing DMA - 0ms

I probably should repeat the experiment with ticks or some smaller units, but this already show improvement.

Of course, we can restrict usage of sha512 to ed25519 kinda prehashing, but still we will have sha512 TLV added.

Yes, sha512 TLV support is fine but to me no signature mode is just for development or "I don't care about security" and a few extra ms speed there is not of concern, it should be limited to sha256, then use sha512 when using a signature that needs it. The sha256 is baseline "no thrills" and everything else is good security

Fine, I will move the line to some limbo PR.

nordicjm · 2024-09-05T10:14:07Z

boot/bootutil/include/bootutil/crypto/sha.h

@@ -131,7 +138,6 @@ static inline int bootutil_sha_drop(bootutil_sha_context *ctx)
    (void)ctx;
    return 0;
 }
-


unrelated? Can we add it back?

nordicjm

Minor nit.

nordicjm · 2024-09-05T10:15:07Z

boot/zephyr/Kconfig

+	# Hidden option
+	help
+	  Use PSA crypt for supporting cryptography functions


Use same style as below e.g.

help Hidden option set if using PSA crypt for cryptography functionality

nordicjm · 2024-09-05T10:15:24Z

boot/zephyr/Kconfig

+	  Hidden option to set by configurations that allow SHA256
+
+config BOOT_IMG_HASH_ALG_SHA384_ALLOW
+	bool
+	help
+	  Hidden option to set by configurations that allow SHA384
+
+config BOOT_IMG_HASH_ALG_SHA512_ALLOW
+	bool
+	depends on BOOT_USE_PSA_CRYPTO
+	help
+	  Hidden option to set by configurations that allow SHA512


Hidden option to set by

boot/zephyr/Kconfig

adds TLV and Kconfig to decouple verification from other options. Signed-off-by: Mateusz Michalek <[email protected]> Signed-off-by: Dominik Ermel <[email protected]>

adds TLV and Kconfig to decouple verification from other options. Upstream PR: mcu-tools/mcuboot#1967 Signed-off-by: Mateusz Michalek <[email protected]> Signed-off-by: Dominik Ermel <[email protected]>

d3zd3z · 2024-09-09T22:22:58Z

Do we want this or #2048?

de-nordic · 2024-09-12T10:51:43Z

Do we want this or #2048?

~~Yes,~~ this is the verification of SHA512 the #2048 adds. This is MCUboot code, the 2048 is imgtool change.

adds TLV and Kconfig to decouple verification from other options. Upstream PR: mcu-tools#1967 Signed-off-by: Mateusz Michalek <[email protected]> Signed-off-by: Dominik Ermel <[email protected]>

nvlsianpu self-requested a review May 28, 2024 13:50

nvlsianpu added the area: core Affects core functionality label May 28, 2024

nvlsianpu reviewed May 28, 2024

View reviewed changes

boot/bootutil/include/bootutil/crypto/sha.h Show resolved Hide resolved

michalek-no closed this Jun 18, 2024

michalek-no reopened this Jun 26, 2024

michalek-no force-pushed the mb-psa-patch branch 2 times, most recently from 4153784 to 9de733a Compare June 26, 2024 12:13

michalek-no marked this pull request as ready for review June 26, 2024 12:13

michalek-no changed the title ~~wip~~ boot: SHA512 verification Jun 26, 2024

michalek-no force-pushed the mb-psa-patch branch from 9de733a to 2e6d3ea Compare June 26, 2024 12:32

de-nordic requested review from nordicjm and d3zd3z July 24, 2024 11:50

nordicjm reviewed Jul 25, 2024

View reviewed changes

de-nordic mentioned this pull request Aug 23, 2024

imgtool: Support for SHA512 for ED25519 usage #2048

Merged

de-nordic force-pushed the mb-psa-patch branch 2 times, most recently from 4b1d4b8 to 4bbfc04 Compare August 29, 2024 16:15

de-nordic requested review from nvlsianpu and nordicjm August 29, 2024 16:16

de-nordic force-pushed the mb-psa-patch branch from 4bbfc04 to 65116e0 Compare August 30, 2024 16:34

nordicjm reviewed Sep 2, 2024

View reviewed changes

de-nordic force-pushed the mb-psa-patch branch 6 times, most recently from 1a3a943 to f75fb90 Compare September 5, 2024 09:54

nordicjm reviewed Sep 5, 2024

View reviewed changes

boot: SHA512 verification

ea7d667

adds TLV and Kconfig to decouple verification from other options. Signed-off-by: Mateusz Michalek <[email protected]> Signed-off-by: Dominik Ermel <[email protected]>

de-nordic force-pushed the mb-psa-patch branch from f75fb90 to ea7d667 Compare September 5, 2024 11:47

nordicjm approved these changes Sep 5, 2024

View reviewed changes

de-nordic self-requested a review September 13, 2024 15:32

nvlsianpu approved these changes Sep 18, 2024

View reviewed changes

nvlsianpu merged commit 41df52e into mcu-tools:main Sep 18, 2024
58 checks passed


		config BOOT_SIGNATURE_TYPE_NONE_SHA512
		bool "No signature; use only sha512 check"

boot: SHA512 verification #1967

boot: SHA512 verification #1967

Uh oh!

Conversation

michalek-no commented May 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nordicjm Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

de-nordic commented Aug 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nordicjm Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nordicjm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

d3zd3z commented Sep 9, 2024

Uh oh!

de-nordic commented Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michalek-no commented May 28, 2024 •

edited

Loading

nordicjm Jul 25, 2024 •

edited

Loading

nordicjm Sep 3, 2024 •

edited

Loading

de-nordic commented Sep 12, 2024 •

edited

Loading