| Message ID | 20210514194602.598322-2-mr.nuke.me@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | image: Reduce the abuse of #ifdefs in image-sig.c | expand |
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!
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.