From 182eeefcb439282dfe3320f4a12ab752f313f6fe Mon Sep 17 00:00:00 2001 From: Thirupathaiah Annapureddy Date: Sun, 16 Aug 2020 23:01:09 -0700 Subject: [PATCH 1/9] vboot: add DTB policy for supporting multiple required conf keys Currently FIT image must be signed by all required conf keys. This means Verified Boot fails if there is a signature verification failure using any required key in U-Boot DTB. This patch introduces a new policy in DTB that can be set to any required conf key. This means if verified boot passes with one of the required keys, U-Boot will continue the OS hand off. There were prior attempts to address this: https://lists.denx.de/pipermail/u-boot/2019-April/366047.html The above patch was failing "make tests". https://lists.denx.de/pipermail/u-boot/2020-January/396629.html Signed-off-by: Thirupathaiah Annapureddy Reviewed-by: Simon Glass --- common/image-fit-sig.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c index cc1967109e..5401d9411b 100644 --- a/common/image-fit-sig.c +++ b/common/image-fit-sig.c @@ -416,6 +416,10 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, { int noffset; int sig_node; + int verified = 0; + int reqd_sigs = 0; + bool reqd_policy_all = true; + const char *reqd_mode; /* Work out what we need to verify */ sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME); @@ -425,6 +429,14 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, return 0; } + /* Get required-mode policy property from DTB */ + reqd_mode = fdt_getprop(sig_blob, sig_node, "required-mode", NULL); + if (reqd_mode && !strcmp(reqd_mode, "any")) + reqd_policy_all = false; + + debug("%s: required-mode policy set to '%s'\n", __func__, + reqd_policy_all ? "all" : "any"); + fdt_for_each_subnode(noffset, sig_blob, sig_node) { const char *required; int ret; @@ -433,15 +445,29 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, NULL); if (!required || strcmp(required, "conf")) continue; + + reqd_sigs++; + ret = fit_config_verify_sig(fit, conf_noffset, sig_blob, noffset); if (ret) { - printf("Failed to verify required signature '%s'\n", - fit_get_name(sig_blob, noffset, NULL)); - return ret; + if (reqd_policy_all) { + printf("Failed to verify required signature '%s'\n", + fit_get_name(sig_blob, noffset, NULL)); + return ret; + } + } else { + verified++; + if (!reqd_policy_all) + break; } } + if (reqd_sigs && !verified) { + printf("Failed to verify 'any' of the required signature(s)\n"); + return -EPERM; + } + return 0; } From feaeee8b5ff59477e0372ae7b9a655ecca05b24a Mon Sep 17 00:00:00 2001 From: Thirupathaiah Annapureddy Date: Sun, 16 Aug 2020 23:01:10 -0700 Subject: [PATCH 2/9] test: vboot: add tests for multiple required keys This patch adds vboot tests to verify the support for multiple required keys using new required-mode DTB policy. This patch also fixes existing test where dev key is assumed to be marked as not required, although it is marked as required. Note that this patch re-added sign_fit_norequire(). sign_fit_norequire() was removed as part of the following: commit b008677daf2a ("test: vboot: Fix pylint errors"). This patch leverages sign_fit_norequire() to fix the existing bug. Signed-off-by: Thirupathaiah Annapureddy Reviewed-by: Simon Glass --- test/py/tests/test_vboot.py | 46 +++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 6b998cfd70..e45800d94c 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -126,6 +126,23 @@ def test_vboot(u_boot_console, sha_algo, padding, sign_options, required): cons.log.action('%s: Sign images' % sha_algo) util.run_and_log(cons, args) + def sign_fit_norequire(sha_algo, options): + """Sign the FIT + + Signs the FIT and writes the signature into it. It also writes the + public key into the dtb. It does not mark key as 'required' in dtb. + + Args: + sha_algo: Either 'sha1' or 'sha256', to select the algorithm to + use. + options: Options to provide to mkimage. + """ + args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, fit] + if options: + args += options.split(' ') + cons.log.action('%s: Sign images' % sha_algo) + util.run_and_log(cons, args) + def replace_fit_totalsize(size): """Replace FIT header's totalsize with something greater. @@ -279,15 +296,40 @@ def test_vboot(u_boot_console, sha_algo, padding, sign_options, required): # Build the FIT with dev key (keys NOT required). This adds the # signature into sandbox-u-boot.dtb, NOT marked 'required'. make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) - sign_fit(sha_algo, sign_options) + sign_fit_norequire(sha_algo, sign_options) # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. # Only the prod key is set as 'required'. But FIT we just built has - # a dev signature only (sign_fit() overwrites the FIT). + # a dev signature only (sign_fit_norequire() overwrites the FIT). # Try to boot the FIT with dev key. This FIT should not be accepted by # U-Boot because the prod key is required. run_bootm(sha_algo, 'required key', '', False) + # Build the FIT with dev key (keys required) and sign it. This puts the + # signature into sandbox-u-boot.dtb, marked 'required'. + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) + sign_fit(sha_algo, sign_options) + + # Set the required-mode policy to "any". + # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. + # Both the dev and prod key are set as 'required'. But FIT we just built has + # a dev signature only (sign_fit() overwrites the FIT). + # Try to boot the FIT with dev key. This FIT should be accepted by + # U-Boot because the dev key is required and policy is "any" required key. + util.run_and_log(cons, 'fdtput -t s %s /signature required-mode any' % + (dtb)) + run_bootm(sha_algo, 'multi required key', 'dev+', True) + + # Set the required-mode policy to "all". + # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. + # Both the dev and prod key are set as 'required'. But FIT we just built has + # a dev signature only (sign_fit() overwrites the FIT). + # Try to boot the FIT with dev key. This FIT should not be accepted by + # U-Boot because the prod key is required and policy is "all" required key + util.run_and_log(cons, 'fdtput -t s %s /signature required-mode all' % + (dtb)) + run_bootm(sha_algo, 'multi required key', '', False) + cons = u_boot_console tmpdir = cons.config.result_dir + '/' datadir = cons.config.source_dir + '/test/py/tests/vboot/' From 6a0498a5fd41b58b0c61b34f315771aac0eca0e0 Mon Sep 17 00:00:00 2001 From: Thirupathaiah Annapureddy Date: Sun, 16 Aug 2020 23:01:11 -0700 Subject: [PATCH 3/9] doc: verified-boot: add required-mode information Add documentation about 'required-mode' property in /signature node in U-Boot's control FDT. Signed-off-by: Thirupathaiah Annapureddy Reviewed-by: Simon Glass --- doc/uImage.FIT/signature.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt index d4afd755e9..a3455889ed 100644 --- a/doc/uImage.FIT/signature.txt +++ b/doc/uImage.FIT/signature.txt @@ -386,6 +386,20 @@ that might be used by the target needs to be signed with 'required' keys. This happens automatically as part of a bootm command when FITs are used. +For Signed Configurations, the default verification behavior can be changed by +the following optional property in /signature node in U-Boot's control FDT. + +- required-mode: Valid values are "any" to allow verified boot to succeed if +the selected configuration is signed by any of the 'required' keys, and "all" +to allow verified boot to succeed if the selected configuration is signed by +all of the 'required' keys. + +This property can be added to a binary device tree using fdtput as shown in +below examples:: + + fdtput -t s control.dtb /signature required-mode any + fdtput -t s control.dtb /signature required-mode all + Enabling FIT Verification ------------------------- From 34ca77c1e113d42a63f8ae21b41ec7f9f356c1de Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 20 Aug 2020 19:57:45 +0200 Subject: [PATCH 4/9] lib/hashtable: remove superfluous check We assign first_deleted = 0. There is no need to check its value without any further assignment in between. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- lib/hashtable.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/hashtable.c b/lib/hashtable.c index 4a8c50b4b8..7c08f5c805 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -324,8 +324,7 @@ int hsearch_r(struct env_entry item, enum env_action action, */ unsigned hval2; - if (htab->table[idx].used == USED_DELETED - && !first_deleted) + if (htab->table[idx].used == USED_DELETED) first_deleted = idx; ret = _compare_and_overwrite_entry(item, action, retval, htab, From a6982a6f768bdcf4bd0848ff4dbe68c2fd6599fb Mon Sep 17 00:00:00 2001 From: Philippe Reynes Date: Thu, 17 Sep 2020 15:01:46 +0200 Subject: [PATCH 5/9] fit: cipher: aes: allow to store the IV in the FIT image Binaries may be encrypted in a FIT image with AES. This algo needs a key and an IV (Initialization Vector). The IV is provided in a file (pointer by iv-name-hint in the ITS file) when building the ITB file. This commits adds provide an alternative way to manage the IV. If the property iv-name-hint is not provided in the ITS file, the tool mkimage will generate an random IV and store it in the FIT image. Signed-off-by: Philippe Reynes --- include/image.h | 2 +- include/u-boot/aes.h | 6 +++-- lib/aes/aes-encrypt.c | 22 +++++++++++++--- tools/image-host.c | 61 ++++++++++++++++++++++++++++++++++--------- 4 files changed, 72 insertions(+), 19 deletions(-) diff --git a/include/image.h b/include/image.h index 9a5a87dbf8..10995b8e24 100644 --- a/include/image.h +++ b/include/image.h @@ -1463,7 +1463,7 @@ struct cipher_algo { unsigned char **cipher, int *cipher_len); int (*add_cipher_data)(struct image_cipher_info *info, - void *keydest); + void *keydest, void *fit, int node_noffset); int (*decrypt)(struct image_cipher_info *info, const void *cipher, size_t cipher_len, diff --git a/include/u-boot/aes.h b/include/u-boot/aes.h index 32281041de..acbc50b9e6 100644 --- a/include/u-boot/aes.h +++ b/include/u-boot/aes.h @@ -13,7 +13,8 @@ int image_aes_encrypt(struct image_cipher_info *info, const unsigned char *data, int size, unsigned char **cipher, int *cipher_len); -int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest); +int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest, + void *fit, int node_noffset); #else int image_aes_encrypt(struct image_cipher_info *info, const unsigned char *data, int size, @@ -22,7 +23,8 @@ int image_aes_encrypt(struct image_cipher_info *info, return -ENXIO; } -int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest) +int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest, + void *fit, int node_noffset) { return -ENXIO; } diff --git a/lib/aes/aes-encrypt.c b/lib/aes/aes-encrypt.c index de00a836f6..a6d1720f30 100644 --- a/lib/aes/aes-encrypt.c +++ b/lib/aes/aes-encrypt.c @@ -74,7 +74,8 @@ int image_aes_encrypt(struct image_cipher_info *info, return ret; } -int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest) +int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest, + void *fit, int node_noffset) { int parent, node; char name[128]; @@ -97,8 +98,13 @@ int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest) goto done; /* Either create or overwrite the named key node */ - snprintf(name, sizeof(name), "key-%s-%s-%s", - info->name, info->keyname, info->ivname); + if (info->ivname) + snprintf(name, sizeof(name), "key-%s-%s-%s", + info->name, info->keyname, info->ivname); + else + snprintf(name, sizeof(name), "key-%s-%s", + info->name, info->keyname); + node = fdt_subnode_offset(keydest, parent, name); if (node == -FDT_ERR_NOTFOUND) { node = fdt_add_subnode(keydest, parent, name); @@ -116,9 +122,17 @@ int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest) ret = node; } - if (!ret) + if (ret) + goto done; + + if (info->ivname) + /* Store the IV in the u-boot device tree */ ret = fdt_setprop(keydest, node, "iv", info->iv, info->cipher->iv_len); + else + /* Store the IV in the FIT image */ + ret = fdt_setprop(fit, node_noffset, "iv", + info->iv, info->cipher->iv_len); if (!ret) ret = fdt_setprop(keydest, node, "key", diff --git a/tools/image-host.c b/tools/image-host.c index 3d52593e36..8886beff17 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -320,6 +320,36 @@ err: return ret; } +static int get_random_data(void *data, int size) +{ + unsigned char *tmp = data; + struct timespec date; + int i, ret = 0; + + if (!tmp) { + printf("%s: pointer data is NULL\n", __func__); + ret = -1; + goto out; + } + + ret = clock_gettime(CLOCK_MONOTONIC, &date); + if (ret < 0) { + printf("%s: clock_gettime has failed (err=%d, str=%s)\n", + __func__, ret, strerror(ret)); + goto out; + } + + srand(date.tv_nsec); + + for (i = 0; i < size; i++) { + *tmp = rand() & 0xff; + tmp++; + } + + out: + return ret; +} + static int fit_image_setup_cipher(struct image_cipher_info *info, const char *keydir, void *fit, const char *image_name, int image_noffset, @@ -345,13 +375,13 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, goto out; } - /* Read the IV name */ + /* + * Read the IV name + * + * If this property is not provided then mkimage will generate + * a random IV and store it in the FIT image + */ info->ivname = fdt_getprop(fit, noffset, "iv-name-hint", NULL); - if (!info->ivname) { - printf("Can't get iv name for cipher in image '%s'\n", - image_name); - goto out; - } info->fit = fit; info->node_noffset = noffset; @@ -377,17 +407,23 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, if (ret < 0) goto out; - /* Read the IV in the file */ - snprintf(filename, sizeof(filename), "%s/%s%s", - info->keydir, info->ivname, ".bin"); info->iv = malloc(info->cipher->iv_len); if (!info->iv) { printf("Can't allocate memory for iv\n"); ret = -1; goto out; } - ret = fit_image_read_data(filename, (unsigned char *)info->iv, - info->cipher->iv_len); + + if (info->ivname) { + /* Read the IV in the file */ + snprintf(filename, sizeof(filename), "%s/%s%s", + info->keydir, info->ivname, ".bin"); + ret = fit_image_read_data(filename, (unsigned char *)info->iv, + info->cipher->iv_len); + } else { + /* Generate an ramdom IV */ + ret = get_random_data((void *)info->iv, info->cipher->iv_len); + } out: return ret; @@ -453,9 +489,10 @@ fit_image_process_cipher(const char *keydir, void *keydest, void *fit, * Write the public key into the supplied FDT file; this might fail * several times, since we try signing with successively increasing * size values + * And, if needed, write the iv in the FIT file */ if (keydest) { - ret = info.cipher->add_cipher_data(&info, keydest); + ret = info.cipher->add_cipher_data(&info, keydest, fit, node_noffset); if (ret) { printf("Failed to add verification data for cipher '%s' in image '%s'\n", info.keyname, image_name); From 54ab7cf1dd3f88e124d16c5ef64b0aae4e704ffc Mon Sep 17 00:00:00 2001 From: Philippe Reynes Date: Thu, 17 Sep 2020 15:01:47 +0200 Subject: [PATCH 6/9] fit: cipher: aes: allow to read the IV in the FIT image This commit add the support in u-boot to read the IV in the FIT image instead of u-boot device tree. Signed-off-by: Philippe Reynes --- common/image-cipher.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/common/image-cipher.c b/common/image-cipher.c index 09869f7846..4ca9eec4ef 100644 --- a/common/image-cipher.c +++ b/common/image-cipher.c @@ -94,9 +94,11 @@ static int fit_image_setup_decrypt(struct image_cipher_info *info, return -1; } + info->iv = fdt_getprop(fit, cipher_noffset, "iv", NULL); info->ivname = fdt_getprop(fit, cipher_noffset, "iv-name-hint", NULL); - if (!info->ivname) { - printf("Can't get IV name\n"); + + if (!info->iv && !info->ivname) { + printf("Can't get IV or IV name\n"); return -1; } @@ -120,8 +122,12 @@ static int fit_image_setup_decrypt(struct image_cipher_info *info, * Search the cipher node in the u-boot fdt * the path should be: /cipher/key--- */ - snprintf(node_path, sizeof(node_path), "/%s/key-%s-%s-%s", - FIT_CIPHER_NODENAME, algo_name, info->keyname, info->ivname); + if (info->ivname) + snprintf(node_path, sizeof(node_path), "/%s/key-%s-%s-%s", + FIT_CIPHER_NODENAME, algo_name, info->keyname, info->ivname); + else + snprintf(node_path, sizeof(node_path), "/%s/key-%s-%s", + FIT_CIPHER_NODENAME, algo_name, info->keyname); noffset = fdt_path_offset(fdt, node_path); if (noffset < 0) { @@ -137,10 +143,12 @@ static int fit_image_setup_decrypt(struct image_cipher_info *info, } /* read iv */ - info->iv = fdt_getprop(fdt, noffset, "iv", NULL); if (!info->iv) { - printf("Can't get IV in cipher node '%s'\n", node_path); - return -1; + info->iv = fdt_getprop(fdt, noffset, "iv", NULL); + if (!info->iv) { + printf("Can't get IV in cipher node '%s'\n", node_path); + return -1; + } } return 0; From 167fb1f8dc4bb2b99228c4582a462484ad41fa34 Mon Sep 17 00:00:00 2001 From: Matthieu CASTET Date: Wed, 23 Sep 2020 19:11:44 +0200 Subject: [PATCH 7/9] lib: rsa: check algo match in rsa_verify_with_keynode The algo name should match between the FIT's signature node and the U-Boot's control FDT. If we do not check it, U-Boot's control FDT can expect sha512 hash but nothing will prevent to accept image with sha1 hash if the signature is correct. Signed-off-by: Matthieu CASTET --- lib/rsa/rsa-verify.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index 2057f6819d..b9c800c7dc 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -439,12 +439,17 @@ static int rsa_verify_with_keynode(struct image_sign_info *info, struct key_prop prop; int length; int ret = 0; + const char *algo; if (node < 0) { debug("%s: Skipping invalid node", __func__); return -EBADF; } + algo = fdt_getprop(blob, node, "algo", NULL); + if (strcmp(info->name, algo)) + return -EFAULT; + prop.num_bits = fdtdec_get_int(blob, node, "rsa,num-bits", 0); prop.n0inv = fdtdec_get_int(blob, node, "rsa,n0-inverse", 0); From 3f8808ebaa901ce18a7dfb3432e68e9c3a79f244 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 6 Oct 2020 12:09:45 +0200 Subject: [PATCH 8/9] rsa: fix retrieving public exponent on big-endian systems Commit fdf0819afb (rsa: fix alignment issue when getting public exponent) changed the logic to avoid doing an 8-byte access to a possibly-not-8-byte-aligned address. However, using rsa_convert_big_endian is wrong: That function converts an array of big-endian (32-bit) words with the most significant word first (aka a BE byte array) to an array of cpu-endian words with the least significant word first. While the exponent is indeed _stored_ as a big-endian 64-bit word (two BE words with MSW first), we want to extract it as a cpu-endian 64 bit word. On a little-endian host, swapping the words and byte-swapping each 32-bit word works, because that's the same as byte-swapping the whole 64 bit word. But on a big-endian host, the fdt32_to_cpu are no-ops, but rsa_convert_big_endian() still does the word-swapping, breaking verified boot. To fix that, while still ensuring we don't do unaligned accesses, add a little helper that first memcpy's the bytes to a local fdt64_t, then applies fdt64_to_cpu(). [The name is chosen based on the [bl]eXX_to_cpup in linux/byteorder/generic.h]. Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent") Signed-off-by: Rasmus Villemoes Reviewed-by: Simon Glass --- lib/rsa/rsa-mod-exp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/rsa/rsa-mod-exp.c b/lib/rsa/rsa-mod-exp.c index a437cbe26f..78c688d14c 100644 --- a/lib/rsa/rsa-mod-exp.c +++ b/lib/rsa/rsa-mod-exp.c @@ -25,6 +25,14 @@ #define get_unaligned_be32(a) fdt32_to_cpu(*(uint32_t *)a) #define put_unaligned_be32(a, b) (*(uint32_t *)(b) = cpu_to_fdt32(a)) +static inline uint64_t fdt64_to_cpup(const void *p) +{ + fdt64_t w; + + memcpy(&w, p, sizeof(w)); + return fdt64_to_cpu(w); +} + /* Default public exponent for backward compatibility */ #define RSA_DEFAULT_PUBEXP 65537 @@ -263,8 +271,7 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len, if (!prop->public_exponent) key.exponent = RSA_DEFAULT_PUBEXP; else - rsa_convert_big_endian((uint32_t *)&key.exponent, - prop->public_exponent, 2); + key.exponent = fdt64_to_cpup(prop->public_exponent); if (!key.len || !prop->modulus || !prop->rr) { debug("%s: Missing RSA key info", __func__); From ec71cc34c1cef173d9f656d5cc9a2e698fae28fb Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 8 Oct 2020 20:53:13 +0200 Subject: [PATCH 9/9] lib: rsa: superfluous initialization in rsa_verify() Remove initialization of ret with unused value. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- lib/rsa/rsa-verify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index b9c800c7dc..0ab0f629d0 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -545,7 +545,7 @@ int rsa_verify(struct image_sign_info *info, { /* Reserve memory for maximum checksum-length */ uint8_t hash[info->crypto->key_len]; - int ret = -EACCES; + int ret; /* * Verify that the checksum-length does not exceed the