[PATCH RFC 00/10] image: Reduce the abuse of #ifdefs in image-sig.c
This series is motivated by Simon's bratwurst: [PATCH v2 00/50] image: Reduce #ifdefs and ad-hoc defines in image code
A big problem with current mkimage code, as well as the code it shares with the targets is that it uses a lot of #ifdefs. Some of the #ifdefs are defined based on other macros, or CONFIG_() options.
Simon's approach to fixing this is to extend Kconfig to the host-side of mkimage, and replace ifdefs with "if (CONFIG_IS_ENABLED())" wherever possible. This would resolve most cosmetic issues caused by the ungodly abuse of #ifdefs.
I do not like the aforementioned approach, because I believe it is a band-aid to a much deeper problem. I believe the fundamental problem is the incorrect separation of target code and host code, which has led to the accumulation of crud over the years. I believe in refactoring the current code in order to reduce the need for decision points and branch divergence between the host and target.
In this series I intend to demonstrate a proof-of-concept for achieving this with respect to signing algorithms. I treat the three image_get_*_algo() functions as an interface, and decouple the host and target implementations. This enable a dramatic reduction of #ifdefs decision points in image-sig.c
The existing implementation is mostly suited for the host-side, where it is reused. On the target-side, I implement a linker-list based array of crypto_algo structures, inspired by the DM driver lists.
Two macros are deleted, rsa.h, and ecdsa.h are completely cleaned of #ifdefs, and the new host-side implementation of image-sig.c has zero #ifdefs. This comes at a minimal increase in the noumber of source lines of code.
Only image_get_crypto_algo() is implemented as a linker list in this POC. image_get_checksum_algo() and image_get_padding_algo() would also see healthy benefits.
Alexandru Gagniuc (10): common: Move host-only logic in image-sig.c to separate file common: image-sig.c: Remove host-specific logic and #ifdefs image: Add support for placing crypto_algo in linker lists [UNTESTED] image: Add support for relocating crypto_algos in linker lists image: rsa: Move verification algorithm to a linker list image: image-sig.c: Remove crypto_algos array lib: ecdsa: Remove #ifdefs from ecdsa.h lib: rsa: Remove #ifdefs from rsa.h image: Eliminate IMAGE_ENABLE_VERIFY macro image: Eliminate IMAGE_ENABLE_VERIFY_ECDSA macro
common/image-sig-host.c | 134 ++++++++++++++++++++++++++++++++++++++++ common/image-sig.c | 71 +++++---------------- include/image.h | 13 ++-- include/u-boot/ecdsa.h | 25 -------- include/u-boot/rsa.h | 47 -------------- lib/rsa/rsa-verify.c | 16 +++++ tools/Makefile | 2 +- 7 files changed, 172 insertions(+), 136 deletions(-) create mode 100644 common/image-sig-host.c
image-sig.c is used to map a hash or crypto algorithm name to a handler of that algorithm. There is some similarity between the host and target variants, with the differences worked out by #ifdefs. The purpose of this change is to remove those ifdefs.
First, copy the file to a host-only version, and remove target specific code. Although it looks like we are duplicating code, subsequent patches will change the way target algorithms are searched. Besides we are only duplicating three string to struct mapping functions. This isn't something to fuss about. --- common/image-sig-host.c | 134 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile | 2 +- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 common/image-sig-host.c
diff --git a/common/image-sig-host.c b/common/image-sig-host.c new file mode 100644 index 0000000000..22e9c53c3e --- /dev/null +++ b/common/image-sig-host.c @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2013, Google Inc. + */ + +#include "mkimage.h" +#include <fdt_support.h> +#include <time.h> +#include <linux/libfdt.h> +#include <image.h> +#include <u-boot/ecdsa.h> +#include <u-boot/rsa.h> +#include <u-boot/hash-checksum.h> + + +struct checksum_algo checksum_algos[] = { + { + .name = "sha1", + .checksum_len = SHA1_SUM_LEN, + .der_len = SHA1_DER_LEN, + .der_prefix = sha1_der_prefix, + .calculate_sign = EVP_sha1, + .calculate = hash_calculate, + }, + { + .name = "sha256", + .checksum_len = SHA256_SUM_LEN, + .der_len = SHA256_DER_LEN, + .der_prefix = sha256_der_prefix, + .calculate_sign = EVP_sha256, + .calculate = hash_calculate, + }, + { + .name = "sha384", + .checksum_len = SHA384_SUM_LEN, + .der_len = SHA384_DER_LEN, + .der_prefix = sha384_der_prefix, + .calculate_sign = EVP_sha384, + .calculate = hash_calculate, + }, + { + .name = "sha512", + .checksum_len = SHA512_SUM_LEN, + .der_len = SHA512_DER_LEN, + .der_prefix = sha512_der_prefix, + .calculate_sign = EVP_sha512, + .calculate = hash_calculate, + }, +}; + +struct crypto_algo crypto_algos[] = { + { + .name = "rsa2048", + .key_len = RSA2048_BYTES, + .sign = rsa_sign, + .add_verify_data = rsa_add_verify_data, + .verify = rsa_verify, + }, + { + .name = "rsa4096", + .key_len = RSA4096_BYTES, + .sign = rsa_sign, + .add_verify_data = rsa_add_verify_data, + .verify = rsa_verify, + }, + { + .name = "ecdsa256", + .key_len = ECDSA256_BYTES, + .sign = ecdsa_sign, + .add_verify_data = ecdsa_add_verify_data, + .verify = ecdsa_verify, + }, +}; + +struct padding_algo padding_algos[] = { + { + .name = "pkcs-1.5", + .verify = padding_pkcs_15_verify, + }, + { + .name = "pss", + .verify = padding_pss_verify, + } +}; + +struct checksum_algo *image_get_checksum_algo(const char *full_name) +{ + int i; + const char *name; + + for (i = 0; i < ARRAY_SIZE(checksum_algos); i++) { + name = checksum_algos[i].name; + /* Make sure names match and next char is a comma */ + if (!strncmp(name, full_name, strlen(name)) && + full_name[strlen(name)] == ',') + return &checksum_algos[i]; + } + + return NULL; +} + +struct crypto_algo *image_get_crypto_algo(const char *full_name) +{ + int i; + const char *name; + + /* Move name to after the comma */ + name = strchr(full_name, ','); + if (!name) + return NULL; + name += 1; + + for (i = 0; i < ARRAY_SIZE(crypto_algos); i++) { + if (!strcmp(crypto_algos[i].name, name)) + return &crypto_algos[i]; + } + + return NULL; +} + +struct padding_algo *image_get_padding_algo(const char *name) +{ + int i; + + if (!name) + return NULL; + + for (i = 0; i < ARRAY_SIZE(padding_algos); i++) { + if (!strcmp(padding_algos[i].name, name)) + return &padding_algos[i]; + } + + return NULL; +} diff --git a/tools/Makefile b/tools/Makefile index d020c55d66..6751d9874b 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -58,7 +58,7 @@ hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o common/image-fit-sig.o +FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig-host.o common/image-fit-sig.o FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
# The following files are synced with upstream DTC.
Hi Alexandru,
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
image-sig.c is used to map a hash or crypto algorithm name to a handler of that algorithm. There is some similarity between the host and target variants, with the differences worked out by #ifdefs. The purpose of this change is to remove those ifdefs.
First, copy the file to a host-only version, and remove target specific code. Although it looks like we are duplicating code, subsequent patches will change the way target algorithms are searched. Besides we are only duplicating three string to struct mapping functions. This isn't something to fuss about.
common/image-sig-host.c | 134 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile | 2 +- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 common/image-sig-host.c
Will we never support signing in the board code? So far it is true, but I wonder if it will remain so, as things get more and more complicated. For example, we may want to sign the devicetree (somehow) after fix-ups. The current code structure makes it possible to add signing if needed. If we decided we wanted to sign on the board, how would we refactor things with this approach?
If this is host code, can we move it to tools/ ?
Regards, Simon
On 5/15/21 10:20 AM, Simon Glass wrote:
Hi Alexandru,
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
image-sig.c is used to map a hash or crypto algorithm name to a handler of that algorithm. There is some similarity between the host and target variants, with the differences worked out by #ifdefs. The purpose of this change is to remove those ifdefs.
First, copy the file to a host-only version, and remove target specific code. Although it looks like we are duplicating code, subsequent patches will change the way target algorithms are searched. Besides we are only duplicating three string to struct mapping functions. This isn't something to fuss about.
common/image-sig-host.c | 134 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile | 2 +- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 common/image-sig-host.c
Will we never support signing in the board code? So far it is true, but I wonder if it will remain so, as things get more and more complicated. For example, we may want to sign the devicetree (somehow) after fix-ups. The current code structure makes it possible to add signing if needed. If we decided we wanted to sign on the board, how would we refactor things with this approach?
We'd have the logistics of keeping private keys available to firmware and only to firmware, but those are orthogonal to the problem. Assuming we implemented a device-side *_sign(), then we would add it to the linker list, via the proposed U_BOOT_CRYPTO_ALGO():
int rsa_device_side_sign(...) { if (!CONFIG_IS_ENABLED(RSA_SIGN_ON_DEVICE)) return -EIEIO; return do_rsa_device_side_sign(...); }
U_BOOT_CRYPTO_ALGO(rsa2048) = { .name = "rsa2048", .key_len = RSA2048_BYTES, .verify = rsa_verify, .sign = rsa_device_side_sign, };
If this is host code, can we move it to tools/ ?
Definitely!
Remove any ifdefs in image-sig.c that were previously used to differentiate from the host code. Note that all code dedicated to relocating ->sign() and ->add_verify_data)_ can be safely removed, as signing is not supported target-side.
NOTE that although it appears we are removing ecdsa256 support, this is intentional. ecdsa_verify() is a no-op on the target, and is currently only used by host code.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/image-sig.c | 39 ++------------------------------------- 1 file changed, 2 insertions(+), 37 deletions(-)
diff --git a/common/image-sig.c b/common/image-sig.c index 0f8e592aba..698b044d50 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -3,18 +3,11 @@ * Copyright (c) 2013, Google Inc. */
-#ifdef USE_HOSTCC -#include "mkimage.h" -#include <fdt_support.h> -#include <time.h> -#include <linux/libfdt.h> -#else #include <common.h> #include <log.h> #include <malloc.h> #include <asm/global_data.h> DECLARE_GLOBAL_DATA_PTR; -#endif /* !USE_HOSTCC*/ #include <image.h> #include <u-boot/ecdsa.h> #include <u-boot/rsa.h> @@ -28,9 +21,6 @@ struct checksum_algo checksum_algos[] = { .checksum_len = SHA1_SUM_LEN, .der_len = SHA1_DER_LEN, .der_prefix = sha1_der_prefix, -#if IMAGE_ENABLE_SIGN - .calculate_sign = EVP_sha1, -#endif .calculate = hash_calculate, }, { @@ -38,9 +28,6 @@ struct checksum_algo checksum_algos[] = { .checksum_len = SHA256_SUM_LEN, .der_len = SHA256_DER_LEN, .der_prefix = sha256_der_prefix, -#if IMAGE_ENABLE_SIGN - .calculate_sign = EVP_sha256, -#endif .calculate = hash_calculate, }, #ifdef CONFIG_SHA384 @@ -49,9 +36,6 @@ struct checksum_algo checksum_algos[] = { .checksum_len = SHA384_SUM_LEN, .der_len = SHA384_DER_LEN, .der_prefix = sha384_der_prefix, -#if IMAGE_ENABLE_SIGN - .calculate_sign = EVP_sha384, -#endif .calculate = hash_calculate, }, #endif @@ -61,9 +45,6 @@ struct checksum_algo checksum_algos[] = { .checksum_len = SHA512_SUM_LEN, .der_len = SHA512_DER_LEN, .der_prefix = sha512_der_prefix, -#if IMAGE_ENABLE_SIGN - .calculate_sign = EVP_sha512, -#endif .calculate = hash_calculate, }, #endif @@ -74,24 +55,13 @@ struct crypto_algo crypto_algos[] = { { .name = "rsa2048", .key_len = RSA2048_BYTES, - .sign = rsa_sign, - .add_verify_data = rsa_add_verify_data, .verify = rsa_verify, }, { .name = "rsa4096", .key_len = RSA4096_BYTES, - .sign = rsa_sign, - .add_verify_data = rsa_add_verify_data, .verify = rsa_verify, }, - { - .name = "ecdsa256", - .key_len = ECDSA256_BYTES, - .sign = ecdsa_sign, - .add_verify_data = ecdsa_add_verify_data, - .verify = ecdsa_verify, - }, };
struct padding_algo padding_algos[] = { @@ -112,16 +82,13 @@ struct checksum_algo *image_get_checksum_algo(const char *full_name) int i; const char *name;
-#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC) +#if defined(CONFIG_NEEDS_MANUAL_RELOC) static bool done;
if (!done) { done = true; for (i = 0; i < ARRAY_SIZE(checksum_algos); i++) { checksum_algos[i].name += gd->reloc_off; -#if IMAGE_ENABLE_SIGN - checksum_algos[i].calculate_sign += gd->reloc_off; -#endif checksum_algos[i].calculate += gd->reloc_off; } } @@ -143,15 +110,13 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name) int i; const char *name;
-#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC) +#if defined(CONFIG_NEEDS_MANUAL_RELOC) static bool done;
if (!done) { done = true; for (i = 0; i < ARRAY_SIZE(crypto_algos); i++) { crypto_algos[i].name += gd->reloc_off; - crypto_algos[i].sign += gd->reloc_off; - crypto_algos[i].add_verify_data += gd->reloc_off; crypto_algos[i].verify += gd->reloc_off; } }
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
Remove any ifdefs in image-sig.c that were previously used to differentiate from the host code. Note that all code dedicated to relocating ->sign() and ->add_verify_data)_ can be safely removed, as signing is not supported target-side.
NOTE that although it appears we are removing ecdsa256 support, this is intentional. ecdsa_verify() is a no-op on the target, and is currently only used by host code.
(need to fix that quick!)
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
common/image-sig.c | 39 ++------------------------------------- 1 file changed, 2 insertions(+), 37 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
The purpose of this change is to enable crypto algorithms to be placed in linker lists, rather than be declared as a static array. The goal is to remove the crypto_algos array in a subsequent patch.
Create a new linker list named "cryptos", and search it when image_get_crypto_algo() is invoked.
NOTE that adding support for manual relocation of crypto_algos within linker lists is beyond the scope of this patch.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/image-sig.c | 9 +++++++++ include/image.h | 5 +++++ 2 files changed, 14 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index 698b044d50..2b7e23f99a 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -107,6 +107,7 @@ struct checksum_algo *image_get_checksum_algo(const char *full_name)
struct crypto_algo *image_get_crypto_algo(const char *full_name) { + struct crypto_algo *crypto, *end; int i; const char *name;
@@ -133,6 +134,14 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name) return &crypto_algos[i]; }
+ crypto = ll_entry_start(struct crypto_algo, cryptos); + end = ll_entry_end(struct crypto_algo, cryptos); + for (; crypto < end; crypto++) { + if (!strcmp(crypto->name, name)) + return crypto; + } + + /* Not found */ return NULL; }
diff --git a/include/image.h b/include/image.h index 459685d4d4..1bda4ef725 100644 --- a/include/image.h +++ b/include/image.h @@ -47,6 +47,7 @@ struct fdt_region; #include <lmb.h> #include <asm/u-boot.h> #include <command.h> +#include <linker_lists.h>
/* Take notice of the 'ignore' property for hashes */ #define IMAGE_ENABLE_IGNORE 1 @@ -1362,6 +1363,10 @@ struct crypto_algo { uint8_t *sig, uint sig_len); };
+/* Declare a new U-Boot crypto algorithm handler */ +#define U_BOOT_CRYPTO_ALGO(__name) \ +ll_entry_declare(struct crypto_algo, __name, cryptos) + struct padding_algo { const char *name; int (*verify)(struct image_sign_info *info,
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
The purpose of this change is to enable crypto algorithms to be placed in linker lists, rather than be declared as a static array. The goal is to remove the crypto_algos array in a subsequent patch.
Create a new linker list named "cryptos", and search it when image_get_crypto_algo() is invoked.
NOTE that adding support for manual relocation of crypto_algos within linker lists is beyond the scope of this patch.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
common/image-sig.c | 9 +++++++++ include/image.h | 5 +++++ 2 files changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Function pointers from crypto_algos array are relocated, when NEEDS_MANUAL_RELOC is set. This relocation doesn't happen if the algo is placed in a linker list. Implement this relocation.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/image-sig.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index 2b7e23f99a..b750751144 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -120,6 +120,13 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name) crypto_algos[i].name += gd->reloc_off; crypto_algos[i].verify += gd->reloc_off; } + + crypto = ll_entry_start(struct crypto_algo, cryptos); + end = ll_entry_end(struct crypto_algo, cryptos); + for (; crypto < end; crypto++) { + crypto->name += gd->reloc_off; + crypto->verify += gd->reloc_off; + } } #endif
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
Function pointers from crypto_algos array are relocated, when NEEDS_MANUAL_RELOC is set. This relocation doesn't happen if the algo is placed in a linker list. Implement this relocation.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
common/image-sig.c | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I don't normally bother with this, since it is due to a toolchain failure, that really should have been fixed years ago. But I am sure someone will be happy.
+Michal Simek
On 5/15/21 5:20 PM, Simon Glass wrote:
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
Function pointers from crypto_algos array are relocated, when NEEDS_MANUAL_RELOC is set. This relocation doesn't happen if the algo is placed in a linker list. Implement this relocation.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
common/image-sig.c | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I don't normally bother with this, since it is due to a toolchain failure, that really should have been fixed years ago. But I am sure someone will be happy.
+Michal Simek
It looks good to me. Acked-by: Michal Simek michal.simek@xilinx.com
Thanks, Michal
Move the RSA verification crytpo_algo structure out of the crypto_algos array, and into a linker list.
Although it appears we are adding an #ifdef to rsa-verify.c, the gains outweigh this small inconvenience. This is because rsa_verify() is defined differently based on #ifdefs. This change allows us to have a single definition of rsa_verify().
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/image-sig.c | 9 --------- lib/rsa/rsa-verify.c | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/common/image-sig.c b/common/image-sig.c index b750751144..d3be30289c 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -53,15 +53,6 @@ struct checksum_algo checksum_algos[] = {
struct crypto_algo crypto_algos[] = { { - .name = "rsa2048", - .key_len = RSA2048_BYTES, - .verify = rsa_verify, - }, - { - .name = "rsa4096", - .key_len = RSA4096_BYTES, - .verify = rsa_verify, - }, };
struct padding_algo padding_algos[] = { diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index aee76f42d5..06b0d82e7d 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -571,3 +571,19 @@ int rsa_verify(struct image_sign_info *info,
return rsa_verify_hash(info, hash, sig, sig_len); } + +#ifndef USE_HOSTCC + +U_BOOT_CRYPTO_ALGO(rsa2048) = { + .name = "rsa2048", + .key_len = RSA2048_BYTES, + .verify = rsa_verify, +}; + +U_BOOT_CRYPTO_ALGO(rsa4096) = { + .name = "rsa4096", + .key_len = RSA4096_BYTES, + .verify = rsa_verify, +}; + +#endif
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
Move the RSA verification crytpo_algo structure out of the crypto_algos array, and into a linker list.
Although it appears we are adding an #ifdef to rsa-verify.c, the gains outweigh this small inconvenience. This is because rsa_verify() is defined differently based on #ifdefs. This change allows us to have a single definition of rsa_verify().
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
common/image-sig.c | 9 --------- lib/rsa/rsa-verify.c | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Crytographic algorithms (currently RSA), are stored in linker lists. The crypto_algos array is unused, so remove it, and any logic associated with it.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/image-sig.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/common/image-sig.c b/common/image-sig.c index d3be30289c..6923f0a9e9 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -51,10 +51,6 @@ struct checksum_algo checksum_algos[] = {
};
-struct crypto_algo crypto_algos[] = { - { -}; - struct padding_algo padding_algos[] = { { .name = "pkcs-1.5", @@ -107,10 +103,6 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name)
if (!done) { done = true; - for (i = 0; i < ARRAY_SIZE(crypto_algos); i++) { - crypto_algos[i].name += gd->reloc_off; - crypto_algos[i].verify += gd->reloc_off; - }
crypto = ll_entry_start(struct crypto_algo, cryptos); end = ll_entry_end(struct crypto_algo, cryptos); @@ -127,11 +119,6 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name) return NULL; name += 1;
- for (i = 0; i < ARRAY_SIZE(crypto_algos); i++) { - if (!strcmp(crypto_algos[i].name, name)) - return &crypto_algos[i]; - } - crypto = ll_entry_start(struct crypto_algo, cryptos); end = ll_entry_end(struct crypto_algo, cryptos); for (; crypto < end; crypto++) {
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
Crytographic algorithms (currently RSA), are stored in linker lists. The crypto_algos array is unused, so remove it, and any logic associated with it.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
common/image-sig.c | 13 ------------- 1 file changed, 13 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
It is no longer necessary to implement ecdsa_() functions as no-ops depending on config options. It is merely sufficient to provide the prototypes, as the ecdsa code is no longer linked when unused.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- include/u-boot/ecdsa.h | 25 ------------------------- 1 file changed, 25 deletions(-)
diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h index 979690d966..f6951c7346 100644 --- a/include/u-boot/ecdsa.h +++ b/include/u-boot/ecdsa.h @@ -15,7 +15,6 @@ * @see "struct crypto_algo" * @{ */ -#if IMAGE_ENABLE_SIGN /** * sign() - calculate and return signature for given input data * @@ -49,22 +48,7 @@ int ecdsa_sign(struct image_sign_info *info, const struct image_region region[], * other -ve value on error */ int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest); -#else -static inline -int ecdsa_sign(struct image_sign_info *info, const struct image_region region[], - int region_count, uint8_t **sigp, uint *sig_len) -{ - return -ENXIO; -} - -static inline -int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest) -{ - return -ENXIO; -} -#endif
-#if IMAGE_ENABLE_VERIFY_ECDSA /** * verify() - Verify a signature against some data * @@ -78,15 +62,6 @@ int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest) int ecdsa_verify(struct image_sign_info *info, const struct image_region region[], int region_count, uint8_t *sig, uint sig_len); -#else -static inline -int ecdsa_verify(struct image_sign_info *info, - const struct image_region region[], int region_count, - uint8_t *sig, uint sig_len) -{ - return -ENXIO; -} -#endif /** @} */
#define ECDSA256_BYTES (256 / 8)
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
It is no longer necessary to implement ecdsa_() functions as no-ops depending on config options. It is merely sufficient to provide the prototypes, as the ecdsa code is no longer linked when unused.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
include/u-boot/ecdsa.h | 25 ------------------------- 1 file changed, 25 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
It is no longer necessary to implement rsa_() functions as no-ops depending on config options. It is merely sufficient to provide the prototypes, as the rsa code is no longer linked when unused.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- include/u-boot/rsa.h | 47 -------------------------------------------- 1 file changed, 47 deletions(-)
diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h index bed1c097c2..4ac373cd5e 100644 --- a/include/u-boot/rsa.h +++ b/include/u-boot/rsa.h @@ -31,7 +31,6 @@ struct rsa_public_key {
struct image_sign_info;
-#if IMAGE_ENABLE_SIGN /** * sign() - calculate and return signature for given input data * @@ -66,22 +65,7 @@ int rsa_sign(struct image_sign_info *info, other -ve value on error */ int rsa_add_verify_data(struct image_sign_info *info, void *keydest); -#else -static inline int rsa_sign(struct image_sign_info *info, - const struct image_region region[], int region_count, - uint8_t **sigp, uint *sig_len) -{ - return -ENXIO; -} - -static inline int rsa_add_verify_data(struct image_sign_info *info, - void *keydest) -{ - return -ENXIO; -} -#endif
-#if IMAGE_ENABLE_VERIFY /** * rsa_verify_hash() - Verify a signature against a hash * @@ -124,37 +108,6 @@ int padding_pss_verify(struct image_sign_info *info, uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len); #endif /* CONFIG_FIT_ENABLE_RSASSA_PSS_SUPPORT */ -#else -static inline int rsa_verify_hash(struct image_sign_info *info, - const uint8_t *hash, - uint8_t *sig, uint sig_len) -{ - return -ENXIO; -} - -static inline int rsa_verify(struct image_sign_info *info, - const struct image_region region[], int region_count, - uint8_t *sig, uint sig_len) -{ - return -ENXIO; -} - -static inline int padding_pkcs_15_verify(struct image_sign_info *info, - uint8_t *msg, int msg_len, - const uint8_t *hash, int hash_len) -{ - return -ENXIO; -} - -#ifdef CONFIG_FIT_ENABLE_RSASSA_PSS_SUPPORT -static inline int padding_pss_verify(struct image_sign_info *info, - uint8_t *msg, int msg_len, - const uint8_t *hash, int hash_len) -{ - return -ENXIO; -} -#endif /* CONFIG_FIT_ENABLE_RSASSA_PSS_SUPPORT */ -#endif
#define RSA_DEFAULT_PADDING_NAME "pkcs-1.5"
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
It is no longer necessary to implement rsa_() functions as no-ops depending on config options. It is merely sufficient to provide the prototypes, as the rsa code is no longer linked when unused.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
include/u-boot/rsa.h | 47 -------------------------------------------- 1 file changed, 47 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
This macro is no longer needed for code flow or #ifdefs. Remove it.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- include/image.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/include/image.h b/include/image.h index 1bda4ef725..4a7aa9358f 100644 --- a/include/image.h +++ b/include/image.h @@ -1225,19 +1225,16 @@ int calculate_hash(const void *data, int data_len, const char *algo, #if defined(USE_HOSTCC) # if defined(CONFIG_FIT_SIGNATURE) # define IMAGE_ENABLE_SIGN 1 -# define IMAGE_ENABLE_VERIFY 1 # define IMAGE_ENABLE_VERIFY_ECDSA 1 # define FIT_IMAGE_ENABLE_VERIFY 1 # include <openssl/evp.h> # else # define IMAGE_ENABLE_SIGN 0 -# define IMAGE_ENABLE_VERIFY 0 # define IMAGE_ENABLE_VERIFY_ECDSA 0 # define FIT_IMAGE_ENABLE_VERIFY 0 # endif #else # define IMAGE_ENABLE_SIGN 0 -# define IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(RSA_VERIFY) # define IMAGE_ENABLE_VERIFY_ECDSA 0 # define FIT_IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(FIT_SIGNATURE) #endif @@ -1294,7 +1291,7 @@ struct image_region { int size; };
-#if IMAGE_ENABLE_VERIFY +#if FIT_IMAGE_ENABLE_VERIFY # include <u-boot/hash-checksum.h> #endif struct checksum_algo {
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
This macro is no longer needed for code flow or #ifdefs. Remove it.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
include/image.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
This macro is no longer needed for code flow or #ifdefs. Remove it.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- include/image.h | 3 --- 1 file changed, 3 deletions(-)
diff --git a/include/image.h b/include/image.h index 4a7aa9358f..dd07771480 100644 --- a/include/image.h +++ b/include/image.h @@ -1225,17 +1225,14 @@ int calculate_hash(const void *data, int data_len, const char *algo, #if defined(USE_HOSTCC) # if defined(CONFIG_FIT_SIGNATURE) # define IMAGE_ENABLE_SIGN 1 -# define IMAGE_ENABLE_VERIFY_ECDSA 1 # define FIT_IMAGE_ENABLE_VERIFY 1 # include <openssl/evp.h> # else # define IMAGE_ENABLE_SIGN 0 -# define IMAGE_ENABLE_VERIFY_ECDSA 0 # define FIT_IMAGE_ENABLE_VERIFY 0 # endif #else # define IMAGE_ENABLE_SIGN 0 -# define IMAGE_ENABLE_VERIFY_ECDSA 0 # define FIT_IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(FIT_SIGNATURE) #endif
On Fri, 14 May 2021 at 13:46, Alexandru Gagniuc mr.nuke.me@gmail.com wrote:
This macro is no longer needed for code flow or #ifdefs. Remove it.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com
include/image.h | 3 --- 1 file changed, 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (4)
-
Alex G. -
Alexandru Gagniuc
-
Michal Simek -
Simon Glass