[PATCH 0/5] Fix FIT hash algos in SPL (Fixes v2021.10-rc2)
Simon and I recently worked on killing a bunch extra definitions. One of the side-effects is that certain hash algorithms won't work in SPL when used in the context of FIT verification.
For example, in FIT verification, CONFIG_IS_ENABLED(SHA256) is used (good), but there is no corresponding CONFIG_SPL_SHA256 (bad). This will always be false for SPL, hence certain "hash" algos are broken.
This series resolves the selection by replacing the broken selection with hash_lookup_algo(), which does not have the aforementioned problem. This at the very least allows 'algo = "sha256"' FIT nodes to work in SPL.
This series does not attempt to add individual SHA/CRC/MD5 configs for SPL. Hash algo selection for SPL has been problematic even before. This series is meant as an emergency fix, so it does not attempt to tackle general refactoring issues.
Alexandru Gagniuc (5): common: Remove unused CONFIG_FIT_SHAxxx selectors lib: Drop SHA512_ALGO in lieu of SHA512 common/spl: Drop SPL_HASH_SUPPORT in favor of SPL_HASH image: Drop if/elseif hash selection in calculate_hash() image: Drop IMAGE_ENABLE_{MD5, CRC32} #defines
arch/arm/mach-socfpga/Kconfig | 2 +- common/Kconfig.boot | 32 ++--------- common/Makefile | 3 +- common/image-fit.c | 45 +++++++--------- common/spl/Kconfig | 54 +------------------ ...s1021atwr_sdcard_ifc_SECURE_BOOT_defconfig | 2 +- configs/ls1043ardb_nand_SECURE_BOOT_defconfig | 2 +- .../ls1043ardb_sdcard_SECURE_BOOT_defconfig | 2 +- .../ls1046ardb_sdcard_SECURE_BOOT_defconfig | 2 +- ...1088ardb_sdcard_qspi_SECURE_BOOT_defconfig | 2 +- configs/mt8516_pumpkin_defconfig | 2 +- include/configs/xilinx_zynqmp.h | 2 +- include/image.h | 23 -------- lib/Kconfig | 12 ++--- lib/Makefile | 2 +- lib/crypt/Kconfig | 2 +- lib/efi_loader/Kconfig | 2 +- lib/sha512.c | 2 - 18 files changed, 39 insertions(+), 154 deletions(-)
Originally CONFIG_FIT_SHAxxx enabled specific SHA algos for and only for has_calculate() in common/image-fit.c. However, since commit 14f061dcb1 ("image: Drop IMAGE_ENABLE_SHAxxx"), the correct selector was changed to CONFIG_SHAxxx.
The extra "_FIT_" variants are neither used, nor needed. Remove them. One defconfig disables FIT_SHA256, which is now changed to 'SHA256'.
Note that SHA selection in SPL is broken for this exact reason. There is no corresponding SPL_SHAxxx. Fixing this is is beyond the scope of this change.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/Kconfig.boot | 28 --------------------- common/spl/Kconfig | 42 -------------------------------- configs/mt8516_pumpkin_defconfig | 2 +- include/image.h | 3 --- 4 files changed, 1 insertion(+), 74 deletions(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 0d4c38402c..2399d5849e 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -35,34 +35,6 @@ config FIT_EXTERNAL_OFFSET could be put in the hole between data payload and fit image header, such as CSF data on i.MX platform.
-config FIT_SHA256 - bool "Support SHA256 checksum of FIT image contents" - default y - select SHA256 - help - Enable this to support SHA256 checksum of FIT image contents. A - SHA256 checksum is a 256-bit (32-byte) hash value used to check that - the image contents have not been corrupted. - -config FIT_SHA384 - bool "Support SHA384 checksum of FIT image contents" - default n - select SHA384 - help - Enable this to support SHA384 checksum of FIT image contents. 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 FIT_SHA512 - bool "Support SHA512 checksum of FIT image contents" - default n - select SHA512 - help - Enable this to support SHA512 checksum of FIT image contents. A - SHA512 checksum is a 512-bit (64-byte) hash value used to check that - the image contents have not been corrupted. - config FIT_FULL_CHECK bool "Do a full check of the FIT before using it" default y diff --git a/common/spl/Kconfig b/common/spl/Kconfig index c155a3b5fc..d69d1fa5f7 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -439,48 +439,6 @@ config SPL_MD5 applications where images may be changed maliciously, you should consider SHA256 or SHA384.
-config SPL_FIT_SHA1 - 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_FIT_SHA256 - 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_FIT_SHA384 - 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_FIT_SHA512 - 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/configs/mt8516_pumpkin_defconfig b/configs/mt8516_pumpkin_defconfig index 0a6c1fccae..1478b01716 100644 --- a/configs/mt8516_pumpkin_defconfig +++ b/configs/mt8516_pumpkin_defconfig @@ -13,7 +13,7 @@ CONFIG_DEBUG_UART_CLOCK=26000000 # CONFIG_PSCI_RESET is not set CONFIG_DEBUG_UART=y CONFIG_FIT=y -# CONFIG_FIT_SHA256 is not set +# CONFIG_SHA256 is not set # CONFIG_ARCH_FIXUP_FDT_MEMORY is not set CONFIG_DEFAULT_FDT_FILE="mt8516-pumpkin" # CONFIG_DISPLAY_BOARDINFO is not set diff --git a/include/image.h b/include/image.h index e20f0b69d5..489b220eba 100644 --- a/include/image.h +++ b/include/image.h @@ -31,9 +31,6 @@ struct fdt_region; #define IMAGE_ENABLE_OF_LIBFDT 1 #define CONFIG_FIT_VERBOSE 1 /* enable fit_format_{error,warning}() */ #define CONFIG_FIT_RSASSA_PSS 1 -#define CONFIG_FIT_SHA256 -#define CONFIG_FIT_SHA384 -#define CONFIG_FIT_SHA512 #define CONFIG_SHA1 #define CONFIG_SHA256 #define CONFIG_SHA384
SHA512_ALGO was used as a "either SHA512 or SHA384", although the implementations of these two algorithms share a majority of code.
From a Kconfig interface perspective, it makes sense to present two
distinct options. This requires #ifdefing out the SHA512 implementation from sha512.c. The latter doesn't make any sense.
It's reasonable to say in Kconfig that SHA384 depends on SHA512, and seems to be the more polite way to handle the selection.
Thus, automatically select SHA512 when SHA384 is enabled.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- arch/arm/mach-socfpga/Kconfig | 2 +- lib/Kconfig | 12 ++++-------- lib/Makefile | 2 +- lib/crypt/Kconfig | 2 +- lib/efi_loader/Kconfig | 2 +- lib/sha512.c | 2 -- 6 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig index 0c35406232..5d95530292 100644 --- a/arch/arm/mach-socfpga/Kconfig +++ b/arch/arm/mach-socfpga/Kconfig @@ -11,7 +11,7 @@ config SOCFPGA_SECURE_VAB_AUTH depends on TARGET_SOCFPGA_AGILEX select FIT_IMAGE_POST_PROCESS select SHA384 - select SHA512_ALGO + select SHA512 select SPL_FIT_IMAGE_POST_PROCESS help All images loaded from FIT will be authenticated by Secure Device diff --git a/lib/Kconfig b/lib/Kconfig index c535147aea..48565a4169 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -375,14 +375,9 @@ config SHA256 The SHA256 algorithm produces a 256-bit (32-byte) hash value (digest).
-config SHA512_ALGO - bool "Enable SHA512 algorithm" - help - This option enables support of internal SHA512 algorithm.
config SHA512 bool "Enable SHA512 support" - depends on SHA512_ALGO help This option enables support of hashing using SHA512 algorithm. The hash is calculated in software. @@ -391,10 +386,11 @@ config SHA512
config SHA384 bool "Enable SHA384 support" - depends on SHA512_ALGO + select SHA512 help This option enables support of hashing using SHA384 algorithm. - The hash is calculated in software. + The hash is calculated in software. This is also selects SHA512, + because these implementations share the bulk of the code.. The SHA384 algorithm produces a 384-bit (48-byte) hash value (digest).
@@ -409,7 +405,7 @@ if SHA_HW_ACCEL
config SHA512_HW_ACCEL bool "Enable hardware acceleration for SHA512" - depends on SHA512_ALGO + depends on SHA512 help This option enables hardware acceleration for the SHA384 and SHA512 hashing algorithms. This affects the 'hash' command and also the diff --git a/lib/Makefile b/lib/Makefile index 8ba745faa0..6aa48ca3d5 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -65,7 +65,7 @@ obj-$(CONFIG_$(SPL_)RSA) += rsa/ obj-$(CONFIG_HASH) += hash-checksum.o obj-$(CONFIG_SHA1) += sha1.o obj-$(CONFIG_SHA256) += sha256.o -obj-$(CONFIG_SHA512_ALGO) += sha512.o +obj-$(CONFIG_SHA512) += sha512.o obj-$(CONFIG_CRYPT_PW) += crypt/
obj-$(CONFIG_$(SPL_)ZLIB) += zlib/ diff --git a/lib/crypt/Kconfig b/lib/crypt/Kconfig index 5495ae8d4c..6a50029642 100644 --- a/lib/crypt/Kconfig +++ b/lib/crypt/Kconfig @@ -20,7 +20,7 @@ config CRYPT_PW_SHA256 config CRYPT_PW_SHA512 bool "Provide sha512crypt" select SHA512 - select SHA512_ALGO + select SHA512 help Enables support for the sha512crypt password-hashing algorithm. The prefix is "$6$". diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index dacc3b5881..08463251cd 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -323,7 +323,7 @@ config EFI_TCG2_PROTOCOL depends on TPM_V2 select SHA1 select SHA256 - select SHA512_ALGO + select SHA512 select SHA384 select SHA512 select HASH diff --git a/lib/sha512.c b/lib/sha512.c index 35f31e3dc5..a421f249ba 100644 --- a/lib/sha512.c +++ b/lib/sha512.c @@ -320,7 +320,6 @@ void sha384_csum_wd(const unsigned char *input, unsigned int ilen,
#endif
-#if defined(CONFIG_SHA512) void sha512_starts(sha512_context * ctx) { ctx->state[0] = SHA512_H0; @@ -381,4 +380,3 @@ void sha512_csum_wd(const unsigned char *input, unsigned int ilen,
sha512_finish(&ctx, output); } -#endif
Both these configs exist. Stick to using CONFIG_SPL_HASH, and drop all references to CONFIG_SPL_HASH_SUPPORT.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/Kconfig.boot | 2 +- common/Makefile | 3 +-- common/spl/Kconfig | 12 +----------- configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig | 2 +- configs/ls1043ardb_nand_SECURE_BOOT_defconfig | 2 +- configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig | 2 +- configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig | 2 +- configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig | 2 +- include/configs/xilinx_zynqmp.h | 2 +- 9 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 2399d5849e..314f1e50a0 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -157,7 +157,7 @@ config SPL_FIT_SIGNATURE select FIT_SIGNATURE select SPL_FIT select SPL_CRYPTO - select SPL_HASH_SUPPORT + select SPL_HASH imply SPL_RSA imply SPL_RSA_VERIFY select SPL_IMAGE_SIGN_INFO diff --git a/common/Makefile b/common/Makefile index 9063ed9391..592f340f1b 100644 --- a/common/Makefile +++ b/common/Makefile @@ -8,7 +8,7 @@ ifndef CONFIG_SPL_BUILD obj-y += init/ obj-y += main.o obj-y += exports.o -obj-$(CONFIG_HASH) += hash.o +obj-$(CONFIG_$(SPL_)HASH) += hash.o obj-$(CONFIG_HUSH_PARSER) += cli_hush.o obj-$(CONFIG_AUTOBOOT) += autoboot.o
@@ -66,7 +66,6 @@ ifdef CONFIG_SPL_BUILD ifdef CONFIG_SPL_DFU obj-$(CONFIG_DFU_OVER_USB) += dfu.o endif -obj-$(CONFIG_SPL_HASH_SUPPORT) += hash.o obj-$(CONFIG_TPL_HASH_SUPPORT) += hash.o obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o obj-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o diff --git a/common/spl/Kconfig b/common/spl/Kconfig index d69d1fa5f7..c75c8aaf08 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -477,16 +477,6 @@ config SPL_CRYPTO this option to build the drivers in drivers/crypto as part of an SPL build.
-config SPL_HASH_SUPPORT - bool "Support hashing drivers" - select SHA1 - select SHA256 - help - Enable hashing drivers in SPL. These drivers can be used to - accelerate secure boot processing in secure applications. Enable - this option to build system-specific drivers for hash acceleration - as part of an SPL build. - config TPL_HASH_SUPPORT bool "Support hashing drivers in TPL" depends on TPL @@ -1193,7 +1183,7 @@ config SPL_USB_ETHER
config SPL_DFU bool "Support DFU (Device Firmware Upgrade)" - select SPL_HASH_SUPPORT + select SPL_HASH select SPL_DFU_NO_RESET depends on SPL_RAM_SUPPORT help diff --git a/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig b/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig index 78196e6485..c5a6819a38 100644 --- a/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig +++ b/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig @@ -31,7 +31,7 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0xe8 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH_SUPPORT=y +CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_I2C=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y diff --git a/configs/ls1043ardb_nand_SECURE_BOOT_defconfig b/configs/ls1043ardb_nand_SECURE_BOOT_defconfig index 3736445d47..93f6b2a668 100644 --- a/configs/ls1043ardb_nand_SECURE_BOOT_defconfig +++ b/configs/ls1043ardb_nand_SECURE_BOOT_defconfig @@ -27,7 +27,7 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0xf0 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH_SUPPORT=y +CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y CONFIG_SPL_NAND_SUPPORT=y diff --git a/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig b/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig index b879a0c361..71c33ca463 100644 --- a/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig +++ b/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig @@ -27,7 +27,7 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x110 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH_SUPPORT=y +CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y CONFIG_SPL_WATCHDOG=y diff --git a/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig b/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig index c46d0dbedd..9d7ff790e0 100644 --- a/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig +++ b/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig @@ -27,7 +27,7 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x110 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH_SUPPORT=y +CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_I2C=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y diff --git a/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig b/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig index 96d44799fa..62da4ecd71 100644 --- a/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig +++ b/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig @@ -33,7 +33,7 @@ CONFIG_MISC_INIT_R=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x8b0 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH_SUPPORT=y +CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_I2C=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h index 262154cdff..42758ba758 100644 --- a/include/configs/xilinx_zynqmp.h +++ b/include/configs/xilinx_zynqmp.h @@ -258,7 +258,7 @@
#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_DFU) # define CONFIG_SPL_ENV_SUPPORT -# define CONFIG_SPL_HASH_SUPPORT +# define CONFIG_SPL_HASH # define CONFIG_ENV_MAX_ENTRIES 10 #endif
On Mon, Aug 23, 2021 at 07:53:18PM -0500, Alexandru Gagniuc wrote:
Both these configs exist. Stick to using CONFIG_SPL_HASH, and drop all references to CONFIG_SPL_HASH_SUPPORT.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
This isn't quite enough and causes a bunch of platforms like ls1043ardb_sdcard_SECURE_BOOT to blow up. I'm adding the following and will update the commit message as that fixes at least the first set of problems I've observed:
diff --git a/board/freescale/common/Kconfig b/board/freescale/common/Kconfig index ab9c14ae8858..35a6115e5e43 100644 --- a/board/freescale/common/Kconfig +++ b/board/freescale/common/Kconfig @@ -4,6 +4,7 @@ config CHAIN_OF_TRUST imply CMD_HASH if ARM select FSL_CAAM select SPL_BOARD_INIT if (ARM && SPL) + select SPL_HASH if (ARM && SPL) select SHA_HW_ACCEL select SHA_PROG_HW_ACCEL select ENV_IS_NOWHERE diff --git a/common/Makefile b/common/Makefile index 592f340f1b6f..ae0430c35fe4 100644 --- a/common/Makefile +++ b/common/Makefile @@ -8,7 +8,6 @@ ifndef CONFIG_SPL_BUILD obj-y += init/ obj-y += main.o obj-y += exports.o -obj-$(CONFIG_$(SPL_)HASH) += hash.o obj-$(CONFIG_HUSH_PARSER) += cli_hush.o obj-$(CONFIG_AUTOBOOT) += autoboot.o
@@ -66,7 +65,6 @@ ifdef CONFIG_SPL_BUILD ifdef CONFIG_SPL_DFU obj-$(CONFIG_DFU_OVER_USB) += dfu.o endif -obj-$(CONFIG_TPL_HASH_SUPPORT) += hash.o obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o obj-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o @@ -104,6 +102,7 @@ endif endif
obj-y += image.o +obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o obj-$(CONFIG_ANDROID_AB) += android_ab.o obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o image-android-dt.o obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o diff --git a/common/spl/Kconfig b/common/spl/Kconfig index c75c8aaf0884..29a46c47877a 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -477,17 +477,6 @@ config SPL_CRYPTO this option to build the drivers in drivers/crypto as part of an SPL build.
-config TPL_HASH_SUPPORT - bool "Support hashing drivers in TPL" - depends on TPL - select SHA1 - select SHA256 - help - Enable hashing drivers in SPL. These drivers can be used to - accelerate secure boot processing in secure applications. Enable - this option to build system-specific drivers for hash acceleration - as part of an SPL build. - config SPL_DMA bool "Support DMA drivers" help diff --git a/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig b/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig index c5a6819a3849..f9d551c6a89b 100644 --- a/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig +++ b/configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig @@ -31,7 +31,6 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0xe8 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_I2C=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y diff --git a/configs/ls1043ardb_nand_SECURE_BOOT_defconfig b/configs/ls1043ardb_nand_SECURE_BOOT_defconfig index 93f6b2a6683f..2733ca83587d 100644 --- a/configs/ls1043ardb_nand_SECURE_BOOT_defconfig +++ b/configs/ls1043ardb_nand_SECURE_BOOT_defconfig @@ -27,7 +27,6 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0xf0 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y CONFIG_SPL_NAND_SUPPORT=y diff --git a/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig b/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig index 71c33ca4634e..392ef1cbd559 100644 --- a/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig +++ b/configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig @@ -27,7 +27,6 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x110 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y CONFIG_SPL_WATCHDOG=y diff --git a/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig b/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig index 9d7ff790e0f5..3d5783aa260f 100644 --- a/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig +++ b/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig @@ -27,7 +27,6 @@ CONFIG_SPL_FSL_PBL=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x110 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_I2C=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y diff --git a/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig b/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig index 62da4ecd7173..0d94027ccb9e 100644 --- a/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig +++ b/configs/ls1088ardb_sdcard_qspi_SECURE_BOOT_defconfig @@ -33,7 +33,6 @@ CONFIG_MISC_INIT_R=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x8b0 CONFIG_SPL_CRYPTO=y -CONFIG_SPL_HASH=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_I2C=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y diff --git a/lib/Makefile b/lib/Makefile index 6aa48ca3d507..93be86c34a01 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -87,7 +87,7 @@ endif
ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16.o -obj-$(CONFIG_$(SPL_TPL_)HASH_SUPPORT) += crc16.o +obj-$(CONFIG_$(SPL_TPL_)HASH) += crc16.o obj-y += net_utils.o endif obj-$(CONFIG_ADDR_MAP) += addr_map.o
On Mon, Aug 23, 2021 at 07:53:18PM -0500, Alexandru Gagniuc wrote:
Both these configs exist. Stick to using CONFIG_SPL_HASH, and drop all references to CONFIG_SPL_HASH_SUPPORT.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
In addition to what I sent already this also needs: diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig index 7c42c75afbe2..d5533b49699b 100644 --- a/cmd/mvebu/Kconfig +++ b/cmd/mvebu/Kconfig @@ -3,6 +3,7 @@ depends on ARCH_MVEBU
config CMD_MVEBU_BUBT bool "bubt" + select SHA256 default n help bubt - Burn a u-boot image to flash
On Mon, Aug 23, 2021 at 07:53:18PM -0500, Alexandru Gagniuc wrote:
Both these configs exist. Stick to using CONFIG_SPL_HASH, and drop all references to CONFIG_SPL_HASH_SUPPORT.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
[snip]
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index d69d1fa5f7..c75c8aaf08 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -477,16 +477,6 @@ config SPL_CRYPTO this option to build the drivers in drivers/crypto as part of an SPL build.
-config SPL_HASH_SUPPORT
- bool "Support hashing drivers"
- select SHA1
- select SHA256
- help
Enable hashing drivers in SPL. These drivers can be used toaccelerate secure boot processing in secure applications. Enablethis option to build system-specific drivers for hash accelerationas part of an SPL build.
There's a lot more work to be done around platforms that enabled sha1/sha256 this way and confirming that it was actually unusable code. For example, (as it was where I stopped paging at in less just now) ls1043aqds_nor_ddr3 drops sha256 and md5 algorithm support, I suspect because of this hunk, but could be some other part of the series. Using buildman's --show-sizes --bloat can be really handy to see when a patch drops out a bunch of code, or adds it in.
calculate_hash() would try to select the appropriate hashing function by a if/elseif contruct. But that is exactly why hash_lookup_algo() exists, so use it instead.
This does mean that we now have to 'select HASH' to make sure we get the hash_lookup_algo() symbol. However, the change makes sense because even basic FITs will have to deal with "hash" nodes.
My only concern is that the 'select SPL_HASH' might cause some platform to grow above its SPL size allowance
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/Kconfig.boot | 2 ++ common/image-fit.c | 45 +++++++++++++++++++-------------------------- 2 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 314f1e50a0..c2d6c89372 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -13,6 +13,7 @@ config FIT bool "Support Flattened Image Tree" select MD5 select SHA1 + select HASH help This option allows you to boot the new uImage structure, Flattened Image Tree. FIT is formally a FDT, which can include @@ -133,6 +134,7 @@ if SPL config SPL_FIT bool "Support Flattened Image Tree within SPL" depends on SPL + select SPL_HASH select SPL_OF_LIBFDT
config SPL_FIT_PRINT diff --git a/common/image-fit.c b/common/image-fit.c index aff4670be3..92d9141bcd 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1193,6 +1193,12 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) return 0; }
+static void crc32_uimage_fixup(void *value) +{ + /* TODO: In C, this type punning is undefined behavior: */ + *((uint32_t *)value) = cpu_to_uimage(*((uint32_t *)value)); +} + /** * calculate_hash - calculate and return hash for provided input data * @data: pointer to the input data @@ -1211,37 +1217,24 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) * 0, on success * -1, when algo is unsupported */ -int calculate_hash(const void *data, int data_len, const char *algo, +int calculate_hash(const void *data, int data_len, const char *name, uint8_t *value, int *value_len) { - if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) { - *((uint32_t *)value) = crc32_wd(0, data, data_len, - CHUNKSZ_CRC32); - *((uint32_t *)value) = cpu_to_uimage(*((uint32_t *)value)); - *value_len = 4; - } else if (CONFIG_IS_ENABLED(SHA1) && strcmp(algo, "sha1") == 0) { - sha1_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA1); - *value_len = 20; - } else if (CONFIG_IS_ENABLED(SHA256) && strcmp(algo, "sha256") == 0) { - sha256_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA256); - *value_len = SHA256_SUM_LEN; - } else if (CONFIG_IS_ENABLED(SHA384) && strcmp(algo, "sha384") == 0) { - sha384_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA384); - *value_len = SHA384_SUM_LEN; - } else if (CONFIG_IS_ENABLED(SHA512) && strcmp(algo, "sha512") == 0) { - sha512_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA512); - *value_len = SHA512_SUM_LEN; - } else if (IMAGE_ENABLE_MD5 && strcmp(algo, "md5") == 0) { - md5_wd((unsigned char *)data, data_len, value, CHUNKSZ_MD5); - *value_len = 16; - } else { + struct hash_algo *algo; + int ret; + + ret = hash_lookup_algo(name, &algo); + if (ret < 0) { debug("Unsupported hash alogrithm\n"); return -1; } + + algo->hash_func_ws(data, data_len, value, algo->chunk_size); + *value_len = algo->digest_size; + + if (!strcmp(name, "crc32")) + crc32_uimage_fixup(value); + return 0; }
On Mon, Aug 23, 2021 at 07:53:19PM -0500, Alexandru Gagniuc wrote:
calculate_hash() would try to select the appropriate hashing function by a if/elseif contruct. But that is exactly why hash_lookup_algo() exists, so use it instead.
This does mean that we now have to 'select HASH' to make sure we get the hash_lookup_algo() symbol. However, the change makes sense because even basic FITs will have to deal with "hash" nodes.
My only concern is that the 'select SPL_HASH' might cause some platform to grow above its SPL size allowance
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
Two problems. The easy to fix problem is that we need: commit 7dbac59f6655e928d11708948f34e779b03bb3d2 Author: Alexandru Gagniuc mr.nuke.me@gmail.com Date: Mon Aug 23 19:53:19 2021 -0500
image: Drop if/elseif hash selection in calculate_hash()
calculate_hash() would try to select the appropriate hashing function by a if/elseif contruct. But that is exactly why hash_lookup_algo() exists, so use it instead.
This does mean that we now have to 'select HASH' to make sure we get the hash_lookup_algo() symbol. However, the change makes sense because even basic FITs will have to deal with "hash" nodes.
My only concern is that the 'select SPL_HASH' might cause some platform to grow above its SPL size allowance
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com [trini: Add SPL_CRYPTO to FSL_CAAM if ARM && SPL] Signed-off-by: Tom Rini trini@konsulko.com
diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig index 1f5dfb94bb81..f91753aa0f3d 100644 --- a/drivers/crypto/fsl/Kconfig +++ b/drivers/crypto/fsl/Kconfig @@ -1,6 +1,7 @@ config FSL_CAAM bool "Freescale Crypto Driver Support" select SHA_HW_ACCEL + select SPL_CRYPTO if (ARM && SPL) imply CMD_HASH help Enables the Freescale's Cryptographic Accelerator and Assurance
to fix a handful of platforms. The harder to fix problem is that changing how the algorithm is selected seems to break mkimage on for example xilinx_zynqmp_virt. Please fix and resubmit, and run it all past CI as well. Thanks!
These are no longer used, so drop them.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- include/image.h | 20 -------------------- 1 file changed, 20 deletions(-)
diff --git a/include/image.h b/include/image.h index 489b220eba..2d057d445c 100644 --- a/include/image.h +++ b/include/image.h @@ -59,26 +59,6 @@ struct fdt_region; #include <hash.h> #include <linux/libfdt.h> #include <fdt_support.h> -# ifdef CONFIG_SPL_BUILD -# ifdef CONFIG_SPL_CRC32 -# define IMAGE_ENABLE_CRC32 1 -# endif -# ifdef CONFIG_SPL_MD5 -# define IMAGE_ENABLE_MD5 1 -# endif -# else -# define IMAGE_ENABLE_CRC32 1 -# define IMAGE_ENABLE_MD5 1 -# endif - -#ifndef IMAGE_ENABLE_CRC32 -#define IMAGE_ENABLE_CRC32 0 -#endif - -#ifndef IMAGE_ENABLE_MD5 -#define IMAGE_ENABLE_MD5 0 -#endif - #endif /* IMAGE_ENABLE_FIT */
#ifdef CONFIG_SYS_BOOT_GET_CMDLINE
participants (2)
-
Alexandru Gagniuc -
Tom Rini