diff mbox series

lib: Move selection of SPL hash algorithms from common/

Message ID 20210322133331.1646575-1-mr.nuke.me@gmail.com
State New
Delegated to: Tom Rini
Headers show
Series lib: Move selection of SPL hash algorithms from common/ | expand

Commit Message

Alex G. March 22, 2021, 1:33 p.m. UTC
When God said, "May there be FIT signature verification in SPL",
Chuck Norris said "SPL image too big". And then there was this patch.

Enabling SPL_FIT_SIGNATURE increased the code size (armv7 platform) by
about 16KiB, just enough to go over the SPL image limit. Of that:
  * .text.sha256_process	3.8 KiB
  * SHA1 implementation         4.4 KiB
Although SHA1 wasn't required, it could not be disabled.

The hash algorithms are implemented in lib/, as is their Kconfig
selection for u-boot main. However, Kconfig selection for SPL is
implemented in common/. To put it mildly, this is inconsistent.
MD5 selection, on the other hand, does not have this problem.

Moving the SPL hash switches to lib/ solves half the problem. They
have to be renamed from SPL_<hash>_SUPPORT to SPL_<hash> to make
them work elegantly with the CONFIG_IS_ENABLED() macro.

The second half of the problem is not referencing the <hash> symbols
when <hash> is disabled. Unfortunately, this requires some more

The above #ifdef problem could be solved in several ways. One way
could be to move the hash handlers to linker lists. This, however,
won't work for userspace tools (mkimage), as they don't implement
custom linker scripts. One could implement a <hash>_register()
function for this case, and manually register all hashes. However,
this is beyond the scope of this patch.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---

This is designed to apply on top of the following series:
 * [PATCH v6 00/11] Add support for ECDSA image signing

 common/hash.c      |  4 ++--
 common/image-sig.c |  8 +++++--
 common/spl/Kconfig | 54 ----------------------------------------------
 include/image.h    | 12 +++++------
 lib/Kconfig        | 39 +++++++++++++++++++++++++++++++++
 lib/Makefile       |  6 +++---
 6 files changed, 56 insertions(+), 67 deletions(-)

Comments

Tom Rini April 22, 2021, 6:09 p.m. UTC | #1
On Mon, Mar 22, 2021 at 08:33:31AM -0500, Alexandru Gagniuc wrote:

> When God said, "May there be FIT signature verification in SPL",
> Chuck Norris said "SPL image too big". And then there was this patch.
> 
> Enabling SPL_FIT_SIGNATURE increased the code size (armv7 platform) by
> about 16KiB, just enough to go over the SPL image limit. Of that:
>   * .text.sha256_process	3.8 KiB
>   * SHA1 implementation         4.4 KiB
> Although SHA1 wasn't required, it could not be disabled.
> 
> The hash algorithms are implemented in lib/, as is their Kconfig
> selection for u-boot main. However, Kconfig selection for SPL is
> implemented in common/. To put it mildly, this is inconsistent.
> MD5 selection, on the other hand, does not have this problem.
> 
> Moving the SPL hash switches to lib/ solves half the problem. They
> have to be renamed from SPL_<hash>_SUPPORT to SPL_<hash> to make
> them work elegantly with the CONFIG_IS_ENABLED() macro.
> 
> The second half of the problem is not referencing the <hash> symbols
> when <hash> is disabled. Unfortunately, this requires some more
> 
> The above #ifdef problem could be solved in several ways. One way
> could be to move the hash handlers to linker lists. This, however,
> won't work for userspace tools (mkimage), as they don't implement
> custom linker scripts. One could implement a <hash>_register()
> function for this case, and manually register all hashes. However,
> this is beyond the scope of this patch.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> 
> This is designed to apply on top of the following series:
>  * [PATCH v6 00/11] Add support for ECDSA image signing
> 
>  common/hash.c      |  4 ++--
>  common/image-sig.c |  8 +++++--
>  common/spl/Kconfig | 54 ----------------------------------------------
>  include/image.h    | 12 +++++------
>  lib/Kconfig        | 39 +++++++++++++++++++++++++++++++++
>  lib/Makefile       |  6 +++---
>  6 files changed, 56 insertions(+), 67 deletions(-)

I like this idea.  As-is, there's a few problems.  socfpga_agilex_vab
and imx8mm_venice now fail to build due to missing sha384 support for
the former and sram overflow for the latter.   ls1046ardb_qspi_spl now
also grows SPL a bit by adding sha1 support.  Can you look in to these
please?  Thanks.
diff mbox series

Patch

diff --git a/common/hash.c b/common/hash.c
index fc64002f73..dbce70e89b 100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -41,7 +41,7 @@  DECLARE_GLOBAL_DATA_PTR;
 
 static void reloc_update(void);
 
-#if defined(CONFIG_SHA1) && !defined(CONFIG_SHA_PROG_HW_ACCEL)
+#if IMAGE_ENABLE_SHA1 && !defined(CONFIG_SHA_PROG_HW_ACCEL)
 static int hash_init_sha1(struct hash_algo *algo, void **ctxp)
 {
 	sha1_context *ctx = malloc(sizeof(sha1_context));
@@ -213,7 +213,7 @@  static int hash_finish_crc32(struct hash_algo *algo, void *ctx, void *dest_buf,
  * Note that algorithm names must be in lower case.
  */
 static struct hash_algo hash_algo[] = {
-#ifdef CONFIG_SHA1
+#if IMAGE_ENABLE_SHA1
 	{
 		.name 		= "sha1",
 		.digest_size	= SHA1_SUM_LEN,
diff --git a/common/image-sig.c b/common/image-sig.c
index 0f8e592aba..dbef978bef 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -23,6 +23,7 @@  DECLARE_GLOBAL_DATA_PTR;
 #define IMAGE_MAX_HASHED_NODES		100
 
 struct checksum_algo checksum_algos[] = {
+#if IMAGE_ENABLE_SHA1
 	{
 		.name = "sha1",
 		.checksum_len = SHA1_SUM_LEN,
@@ -33,6 +34,8 @@  struct checksum_algo checksum_algos[] = {
 #endif
 		.calculate = hash_calculate,
 	},
+#endif
+#if IMAGE_ENABLE_SHA256
 	{
 		.name = "sha256",
 		.checksum_len = SHA256_SUM_LEN,
@@ -43,7 +46,8 @@  struct checksum_algo checksum_algos[] = {
 #endif
 		.calculate = hash_calculate,
 	},
-#ifdef CONFIG_SHA384
+#endif
+#if IMAGE_ENABLE_SHA384
 	{
 		.name = "sha384",
 		.checksum_len = SHA384_SUM_LEN,
@@ -55,7 +59,7 @@  struct checksum_algo checksum_algos[] = {
 		.calculate = hash_calculate,
 	},
 #endif
-#ifdef CONFIG_SHA512
+#if IMAGE_ENABLE_SHA512
 	{
 		.name = "sha512",
 		.checksum_len = SHA512_SUM_LEN,
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 774541c02b..85c542e0e0 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -412,60 +412,6 @@  config SPL_CRC32_SUPPORT
 	  for detected accidental image corruption. For secure applications you
 	  should consider SHA1 or SHA256.
 
-config SPL_MD5_SUPPORT
-	bool "Support MD5"
-	depends on SPL_FIT
-	help
-	  Enable this to support MD5 in FIT images within SPL. An MD5
-	  checksum is a 128-bit hash value used to check that the image
-	  contents have not been corrupted. Note that MD5 is not considered
-	  secure as it is possible (with a brute-force attack) to adjust the
-	  image while still retaining the same MD5 hash value. For secure
-	  applications where images may be changed maliciously, you should
-	  consider SHA256 or SHA384.
-
-config SPL_SHA1_SUPPORT
-	bool "Support SHA1"
-	depends on SPL_FIT
-	select SHA1
-	help
-	  Enable this to support SHA1 in FIT images within SPL. A SHA1
-	  checksum is a 160-bit (20-byte) hash value used to check that the
-	  image contents have not been corrupted or maliciously altered.
-	  While SHA1 is fairly secure it is coming to the end of its life
-	  due to the expanding computing power available to brute-force
-	  attacks. For more security, consider SHA256 or SHA384.
-
-config SPL_SHA256_SUPPORT
-	bool "Support SHA256"
-	depends on SPL_FIT
-	select SHA256
-	help
-	  Enable this to support SHA256 in FIT images within SPL. A SHA256
-	  checksum is a 256-bit (32-byte) hash value used to check that the
-	  image contents have not been corrupted.
-
-config SPL_SHA384_SUPPORT
-	bool "Support SHA384"
-	depends on SPL_FIT
-	select SHA384
-	select SHA512_ALGO
-	help
-	  Enable this to support SHA384 in FIT images within SPL. A SHA384
-	  checksum is a 384-bit (48-byte) hash value used to check that the
-	  image contents have not been corrupted. Use this for the highest
-	  security.
-
-config SPL_SHA512_SUPPORT
-	bool "Support SHA512"
-	depends on SPL_FIT
-	select SHA512
-	select SHA512_ALGO
-	help
-	  Enable this to support SHA512 in FIT images within SPL. A SHA512
-	  checksum is a 512-bit (64-byte) hash value used to check that the
-	  image contents have not been corrupted.
-
 config SPL_FIT_IMAGE_TINY
 	bool "Remove functionality from SPL FIT loading to reduce size"
 	depends on SPL_FIT
diff --git a/include/image.h b/include/image.h
index b5bcf08e61..f85e935f0c 100644
--- a/include/image.h
+++ b/include/image.h
@@ -62,13 +62,13 @@  struct fdt_region;
 #include <linux/libfdt.h>
 #include <fdt_support.h>
 # ifdef CONFIG_SPL_BUILD
-#  ifdef CONFIG_SPL_CRC32_SUPPORT
+#  ifdef CONFIG_SPL_CRC32
 #   define IMAGE_ENABLE_CRC32	1
 #  endif
-#  ifdef CONFIG_SPL_MD5_SUPPORT
+#  ifdef CONFIG_SPL_MD5
 #   define IMAGE_ENABLE_MD5	1
 #  endif
-#  ifdef CONFIG_SPL_SHA1_SUPPORT
+#  ifdef CONFIG_SPL_SHA1
 #   define IMAGE_ENABLE_SHA1	1
 #  endif
 # else
@@ -90,21 +90,21 @@  struct fdt_region;
 #endif
 
 #if defined(CONFIG_FIT_ENABLE_SHA256_SUPPORT) || \
-	defined(CONFIG_SPL_SHA256_SUPPORT)
+	defined(CONFIG_SPL_SHA256)
 #define IMAGE_ENABLE_SHA256	1
 #else
 #define IMAGE_ENABLE_SHA256	0
 #endif
 
 #if defined(CONFIG_FIT_ENABLE_SHA384_SUPPORT) || \
-	defined(CONFIG_SPL_SHA384_SUPPORT)
+	defined(CONFIG_SPL_SHA384)
 #define IMAGE_ENABLE_SHA384	1
 #else
 #define IMAGE_ENABLE_SHA384	0
 #endif
 
 #if defined(CONFIG_FIT_ENABLE_SHA512_SUPPORT) || \
-	defined(CONFIG_SPL_SHA512_SUPPORT)
+	defined(CONFIG_SPL_SHA512_ALGO)
 #define IMAGE_ENABLE_SHA512	1
 #else
 #define IMAGE_ENABLE_SHA512	0
diff --git a/lib/Kconfig b/lib/Kconfig
index 7288340614..8222120cf2 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -357,6 +357,18 @@  config SHA1
 	  The SHA1 algorithm produces a 160-bit (20-byte) hash value
 	  (digest).
 
+ config SPL_SHA1
+	bool "Support SHA1 in SPL"
+	default y if SHA1
+	help
+	  Enable this to support SHA1 in FIT images within SPL. A SHA1
+	  checksum is a 160-bit (20-byte) hash value used to check that the
+	  image contents have not been corrupted or maliciously altered.
+	  While SHA1 is fairly secure it is coming to the end of its life
+	  due to the expanding computing power available to brute-force
+	  attacks. For more security, consider SHA256 or SHA384.
+
+
 config SHA256
 	bool "Enable SHA256 support"
 	help
@@ -365,6 +377,14 @@  config SHA256
 	  The SHA256 algorithm produces a 256-bit (32-byte) hash value
 	  (digest).
 
+config SPL_SHA256
+	bool "Support SHA256 in SPL"
+	default y if SHA256
+	help
+	  Enable this to support SHA256 in FIT images within SPL. A SHA256
+	  checksum is a 256-bit (32-byte) hash value used to check that the
+	  image contents have not been corrupted.
+
 config SHA512_ALGO
 	bool "Enable SHA512 algorithm"
 	help
@@ -379,6 +399,15 @@  config SHA512
 	  The SHA512 algorithm produces a 512-bit (64-byte) hash value
 	  (digest).
 
+config SPL_SHA512_ALGO
+	bool "Support SHA512 in SPL"
+	default y if SHA512
+	help
+	  Enable this to support SHA512 in FIT images within SPL. A SHA512
+	  checksum is a 512-bit (64-byte) hash value used to check that the
+	  image contents have not been corrupted.
+
+
 config SHA384
 	bool "Enable SHA384 support"
 	depends on SHA512_ALGO
@@ -388,6 +417,16 @@  config SHA384
 	  The SHA384 algorithm produces a 384-bit (48-byte) hash value
 	  (digest).
 
+config SPL_SHA384
+	bool "Support SHA384 in SPL"
+	depends on SPL_SHA512_ALGO
+	default y if SHA384
+	help
+	  Enable this to support SHA384 in FIT images within SPL. A SHA384
+	  checksum is a 384-bit (48-byte) hash value used to check that the
+	  image contents have not been corrupted. Use this for the highest
+	  security.
+
 config SHA_HW_ACCEL
 	bool "Enable hashing using hardware"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index 1d4b7d3aad..382a537709 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -61,9 +61,9 @@  obj-$(CONFIG_$(SPL_)ACPIGEN) += acpi/
 obj-$(CONFIG_$(SPL_)MD5) += md5.o
 obj-$(CONFIG_$(SPL_)RSA) += rsa/
 obj-$(CONFIG_FIT_SIGNATURE) += hash-checksum.o
-obj-$(CONFIG_SHA1) += sha1.o
-obj-$(CONFIG_SHA256) += sha256.o
-obj-$(CONFIG_SHA512_ALGO) += sha512.o
+obj-$(CONFIG_$(SPL_)SHA1) += sha1.o
+obj-$(CONFIG_$(SPL_)SHA256) += sha256.o
+obj-$(CONFIG_$(SPL_)SHA512_ALGO) += sha512.o
 
 obj-$(CONFIG_$(SPL_)ZLIB) += zlib/
 obj-$(CONFIG_$(SPL_)ZSTD) += zstd/