From a554a08a19d301736082978818519e76b9cfd811 Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Wed, 14 May 2025 10:28:07 +0100 Subject: [PATCH 1/6] bootutil: Parse key id for built in keys When MCUBOOT_BUILTIN_KEY is enabled, the key id TLV entry is added to the image. Parse this entry while validating the image to identify the key used to sign the image. This enables future support for scenarios such as multiple built-in keys or multi-signature. Signed-off-by: Maulik Patel Change-Id: Ibe26bc2b09e63350f4214719606a5aa4bc1be93c --- boot/bootutil/include/bootutil/crypto/ecdsa.h | 5 --- boot/bootutil/include/bootutil/image.h | 1 + boot/bootutil/include/bootutil/sign_key.h | 11 +++++ boot/bootutil/src/image_validate.c | 40 ++++++++++++++----- scripts/imgtool/image.py | 24 ++++++++++- 5 files changed, 64 insertions(+), 17 deletions(-) diff --git a/boot/bootutil/include/bootutil/crypto/ecdsa.h b/boot/bootutil/include/bootutil/crypto/ecdsa.h index 8a1463e577..ad65b3c964 100644 --- a/boot/bootutil/include/bootutil/crypto/ecdsa.h +++ b/boot/bootutil/include/bootutil/crypto/ecdsa.h @@ -394,11 +394,6 @@ static inline void bootutil_ecdsa_init(bootutil_ecdsa_context *ctx) ctx->required_algorithm = 0; #else /* !MCUBOOT_BUILTIN_KEY */ - /* The incoming key ID is equal to the image index. The key ID value must be - * shifted (by one in this case) because zero is reserved (PSA_KEY_ID_NULL) - * and considered invalid. - */ - ctx->key_id++; /* Make sure it is not equal to 0. */ #if defined(MCUBOOT_SIGN_EC256) ctx->curve_byte_count = 32; ctx->required_algorithm = PSA_ALG_SHA_256; diff --git a/boot/bootutil/include/bootutil/image.h b/boot/bootutil/include/bootutil/image.h index 57be8b1c45..d1014b091f 100644 --- a/boot/bootutil/include/bootutil/image.h +++ b/boot/bootutil/include/bootutil/image.h @@ -98,6 +98,7 @@ extern "C" { */ #define IMAGE_TLV_KEYHASH 0x01 /* hash of the public key */ #define IMAGE_TLV_PUBKEY 0x02 /* public key */ +#define IMAGE_TLV_KEYID 0x03 /* Key ID */ #define IMAGE_TLV_SHA256 0x10 /* SHA256 of image hdr and body */ #define IMAGE_TLV_SHA384 0x11 /* SHA384 of image hdr and body */ #define IMAGE_TLV_SHA512 0x12 /* SHA512 of image hdr and body */ diff --git a/boot/bootutil/include/bootutil/sign_key.h b/boot/bootutil/include/bootutil/sign_key.h index a5e81d3506..58bfaf5b06 100644 --- a/boot/bootutil/include/bootutil/sign_key.h +++ b/boot/bootutil/include/bootutil/sign_key.h @@ -39,6 +39,17 @@ struct bootutil_key { }; extern const struct bootutil_key bootutil_keys[]; +#ifdef MCUBOOT_BUILTIN_KEY +/** + * Verify that the specified key ID is valid for authenticating the given image. + * + * @param[in] image_index Index of the image to be verified. + * @param[in] key_id Identifier of the key to be verified against the image. + * + * @return 0 if the key ID is valid for the image; nonzero on failure. + */ +int boot_verify_key_id_for_image(uint8_t image_index, uint32_t key_id); +#endif /* MCUBOOT_BUILTIN_KEY */ #else struct bootutil_key { uint8_t *key; diff --git a/boot/bootutil/src/image_validate.c b/boot/bootutil/src/image_validate.c index 589b1c416b..c202c0a8f7 100644 --- a/boot/bootutil/src/image_validate.c +++ b/boot/bootutil/src/image_validate.c @@ -282,12 +282,12 @@ bootutil_img_hash(struct boot_loader_state *state, #if !defined(MCUBOOT_HW_KEY) static int -bootutil_find_key(uint8_t *keyhash, uint8_t keyhash_len) +bootutil_find_key(uint8_t image_index, uint8_t *keyhash, uint8_t keyhash_len) { bootutil_sha_context sha_ctx; int i; const struct bootutil_key *key; - uint8_t hash[IMAGE_HASH_SIZE]; + (void)image_index; BOOT_LOG_DBG("bootutil_find_key"); @@ -347,6 +347,32 @@ bootutil_find_key(uint8_t image_index, uint8_t *key, uint16_t key_len) return -1; } #endif /* !MCUBOOT_HW_KEY */ + +#else +/* For MCUBOOT_BUILTIN_KEY, key id is passed */ +#define EXPECTED_KEY_TLV IMAGE_TLV_KEYID +#define KEY_BUF_SIZE sizeof(int32_t) + +static int bootutil_find_key(uint8_t image_index, uint8_t *key_id_buf, uint8_t key_id_buf_len) +{ + int rc; + FIH_DECLARE(fih_rc, FIH_FAILURE); + + /* Key id is passed */ + assert(key_id_buf_len == sizeof(int32_t)); + int32_t key_id = (((int32_t)key_id_buf[0] << 24) | + ((int32_t)key_id_buf[1] << 16) | + ((int32_t)key_id_buf[2] << 8) | + ((int32_t)key_id_buf[3])); + + /* Check if key id is associated with the image */ + FIH_CALL(boot_verify_key_id_for_image, fih_rc, image_index, key_id); + if (FIH_EQ(fih_rc, FIH_SUCCESS)) { + return key_id; + } + + return -1; +} #endif /* !MCUBOOT_BUILTIN_KEY */ #endif /* EXPECTED_SIG_TLV */ @@ -462,6 +488,7 @@ static int bootutil_check_for_pure(const struct image_header *hdr, static const uint16_t allowed_unprot_tlvs[] = { IMAGE_TLV_KEYHASH, IMAGE_TLV_PUBKEY, + IMAGE_TLV_KEYID, IMAGE_TLV_SHA256, IMAGE_TLV_SHA384, IMAGE_TLV_SHA512, @@ -506,14 +533,7 @@ bootutil_img_validate(struct boot_loader_state *state, uint32_t img_sz; #ifdef EXPECTED_SIG_TLV FIH_DECLARE(valid_signature, FIH_FAILURE); -#ifndef MCUBOOT_BUILTIN_KEY int key_id = -1; -#else - /* Pass a key ID equal to the image index, the underlying crypto library - * is responsible for mapping the image index to a builtin key ID. - */ - int key_id = image_index; -#endif /* !MCUBOOT_BUILTIN_KEY */ #ifdef MCUBOOT_HW_KEY uint8_t key_buf[KEY_BUF_SIZE]; #endif @@ -651,7 +671,7 @@ bootutil_img_validate(struct boot_loader_state *state, if (rc) { goto out; } - key_id = bootutil_find_key(buf, len); + key_id = bootutil_find_key(image_index, buf, len); #else rc = LOAD_IMAGE_DATA(hdr, fap, off, key_buf, len); if (rc) { diff --git a/scripts/imgtool/image.py b/scripts/imgtool/image.py index 542e8f5f88..a16c4f80c3 100644 --- a/scripts/imgtool/image.py +++ b/scripts/imgtool/image.py @@ -76,6 +76,7 @@ TLV_VALUES = { 'KEYHASH': 0x01, 'PUBKEY': 0x02, + 'KEYID': 0x03, 'SHA256': 0x10, 'SHA384': 0x11, 'SHA512': 0x12, @@ -136,13 +137,19 @@ def add(self, kind, payload): """ e = STRUCT_ENDIAN_DICT[self.endian] if isinstance(kind, int): - if not TLV_VENDOR_RES_MIN <= kind <= TLV_VENDOR_RES_MAX: + if kind in TLV_VALUES.values(): + buf = struct.pack(e + 'BBH', kind, 0, len(payload)) + elif TLV_VENDOR_RES_MIN <= kind <= TLV_VENDOR_RES_MAX: + # Custom vendor-reserved tag + buf = struct.pack(e + 'HH', kind, len(payload)) + else: msg = "Invalid custom TLV type value '0x{:04x}', allowed " \ "value should be between 0x{:04x} and 0x{:04x}".format( kind, TLV_VENDOR_RES_MIN, TLV_VENDOR_RES_MAX) raise click.UsageError(msg) - buf = struct.pack(e + 'HH', kind, len(payload)) else: + if kind not in TLV_VALUES: + raise click.UsageError(f"Unknown TLV type string: {kind}") buf = struct.pack(e + 'BBH', TLV_VALUES[kind], 0, len(payload)) self.buf += buf self.buf += payload @@ -641,6 +648,9 @@ def create(self, key, public_key_format, enckey, dependencies=None, print(os.path.basename(__file__) + ': export digest') return + if self.key_ids is not None: + self._add_key_id_tlv_to_unprotected(tlv, self.key_ids[0]) + if key is not None or fixed_sig is not None: if public_key_format == 'hash': tlv.add('KEYHASH', pubbytes) @@ -907,3 +917,13 @@ def verify(imgfile, key): pass tlv_off += TLV_SIZE + tlv_len return VerifyResult.INVALID_SIGNATURE, None, None, None + + def set_key_ids(self, key_ids): + """Set list of key IDs (integers) to be inserted before each signature.""" + self.key_ids = key_ids + + def _add_key_id_tlv_to_unprotected(self, tlv, key_id: int): + """Add a key ID TLV into the *unprotected* TLV area.""" + tag = TLV_VALUES['KEYID'] + value = key_id.to_bytes(4, self.endian) + tlv.add(tag, value) \ No newline at end of file From 083b3efd11f91bb67074eccdf0e1c1fe5239f365 Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Tue, 15 Apr 2025 16:29:38 +0100 Subject: [PATCH 2/6] imgtool: Add support for multiple signatures This patch adds support for multiple signatures to single image. This is useful for scenarios where multiple keys are used to sign images, allowing for greater flexibility and security in the image verification process. The tool command line interface is extended to support multiple signatures. The imgtool test suite is updated to test the new functionality. Change-Id: I285b426671f6ad76472f0a2f8fb3a330f8882c3d Signed-off-by: Maulik Patel --- scripts/imgtool/image.py | 193 ++++++++++++++++++++++++--------------- scripts/imgtool/main.py | 52 +++++++---- 2 files changed, 151 insertions(+), 94 deletions(-) diff --git a/scripts/imgtool/image.py b/scripts/imgtool/image.py index a16c4f80c3..a1b0d8e628 100644 --- a/scripts/imgtool/image.py +++ b/scripts/imgtool/image.py @@ -299,6 +299,7 @@ def __init__(self, version=None, header_size=IMAGE_HEADER_SIZE, self.enctlv_len = 0 self.max_align = max(DEFAULT_MAX_ALIGN, align) if max_align is None else int(max_align) self.non_bootable = non_bootable + self.key_ids = None if self.max_align == DEFAULT_MAX_ALIGN: self.boot_magic = bytes([ @@ -472,7 +473,7 @@ def ecies_hkdf(self, enckey, plainkey, hmac_sha_alg): format=PublicFormat.Raw) return cipherkey, ciphermac, pubk - def create(self, key, public_key_format, enckey, dependencies=None, + def create(self, keys, public_key_format, enckey, dependencies=None, sw_type=None, custom_tlvs=None, compression_tlvs=None, compression_type=None, encrypt_keylen=128, clear=False, fixed_sig=None, pub_key=None, vector_to_sign=None, @@ -480,25 +481,33 @@ def create(self, key, public_key_format, enckey, dependencies=None, dont_encrypt=False): self.enckey = enckey - # key decides on sha, then pub_key; of both are none default is used - check_key = key if key is not None else pub_key + # key decides on sha, then pub_key; if both are none default is used + check_key = keys[0] if keys[0] is not None else pub_key hash_algorithm, hash_tlv = key_and_user_sha_to_alg_and_tlv(check_key, user_sha, is_pure) # Calculate the hash of the public key - if key is not None: - pub = key.get_public_bytes() - sha = hash_algorithm() - sha.update(pub) - pubbytes = sha.digest() - elif pub_key is not None: - if hasattr(pub_key, 'sign'): - print(os.path.basename(__file__) + ": sign the payload") - pub = pub_key.get_public_bytes() - sha = hash_algorithm() - sha.update(pub) - pubbytes = sha.digest() + pub_digests = [] + pub_list = [] + + if keys is None: + if pub_key is not None: + if hasattr(pub_key, 'sign'): + print(os.path.basename(__file__) + ": sign the payload") + pub = pub_key.get_public_bytes() + sha = hash_algorithm() + sha.update(pub) + pubbytes = sha.digest() + else: + pubbytes = bytes(hashlib.sha256().digest_size) else: - pubbytes = bytes(hashlib.sha256().digest_size) + for key in keys or []: + pub = key.get_public_bytes() + sha = hash_algorithm() + sha.update(pub) + pubbytes = sha.digest() + pub_digests.append(pubbytes) + pub_list.append(pub) + protected_tlv_size = 0 @@ -526,10 +535,14 @@ def create(self, key, public_key_format, enckey, dependencies=None, # value later. digest = bytes(hash_algorithm().digest_size) + if pub_digests: + boot_pub_digest = pub_digests[0] + else: + boot_pub_digest = pubbytes # Create CBOR encoded boot record boot_record = create_sw_component_data(sw_type, image_version, hash_tlv, digest, - pubbytes) + boot_pub_digest) protected_tlv_size += TLV_SIZE + len(boot_record) @@ -648,20 +661,30 @@ def create(self, key, public_key_format, enckey, dependencies=None, print(os.path.basename(__file__) + ': export digest') return - if self.key_ids is not None: - self._add_key_id_tlv_to_unprotected(tlv, self.key_ids[0]) + if fixed_sig is not None and keys is not None: + raise click.UsageError("Can not sign using key and provide fixed-signature at the same time") - if key is not None or fixed_sig is not None: - if public_key_format == 'hash': - tlv.add('KEYHASH', pubbytes) - else: - tlv.add('PUBKEY', pub) + if fixed_sig is not None: + tlv.add(pub_key.sig_tlv(), fixed_sig['value']) + self.signatures[0] = fixed_sig['value'] + else: + # Multi-signature handling: iterate through each provided key and sign. + self.signatures = [] + for i, key in enumerate(keys): + # If key IDs are provided, and we have enough for this key, add it first. + if self.key_ids is not None and len(self.key_ids) > i: + # Convert key id (an integer) to 4-byte defined endian bytes. + kid_bytes = self.key_ids[i].to_bytes(4, self.endian) + tlv.add('KEYID', kid_bytes) # Using the TLV tag that corresponds to key IDs. + + if public_key_format == 'hash': + tlv.add('KEYHASH', pub_digests[i]) + else: + tlv.add('PUBKEY', pub_list[i]) - if key is not None and fixed_sig is None: # `sign` expects the full image payload (hashing done # internally), while `sign_digest` expects only the digest # of the payload - if hasattr(key, 'sign'): print(os.path.basename(__file__) + ": sign the payload") sig = key.sign(bytes(self.payload)) @@ -669,12 +692,8 @@ def create(self, key, public_key_format, enckey, dependencies=None, print(os.path.basename(__file__) + ": sign the digest") sig = key.sign_digest(message) tlv.add(key.sig_tlv(), sig) - self.signature = sig - elif fixed_sig is not None and key is None: - tlv.add(pub_key.sig_tlv(), fixed_sig['value']) - self.signature = fixed_sig['value'] - else: - raise click.UsageError("Can not sign using key and provide fixed-signature at the same time") + self.signatures.append(sig) + # At this point the image was hashed + signed, we can remove the # protected TLVs from the payload (will be re-added later) @@ -738,7 +757,7 @@ def get_struct_endian(self): return STRUCT_ENDIAN_DICT[self.endian] def get_signature(self): - return self.signature + return self.signatures def get_infile_data(self): return self.infile_data @@ -848,75 +867,99 @@ def verify(imgfile, key): if magic != IMAGE_MAGIC: return VerifyResult.INVALID_MAGIC, None, None, None + # Locate the first TLV info header tlv_off = header_size + img_size tlv_info = b[tlv_off:tlv_off + TLV_INFO_SIZE] magic, tlv_tot = struct.unpack('HH', tlv_info) + + # If it's the protected-TLV block, skip it if magic == TLV_PROT_INFO_MAGIC: - tlv_off += tlv_tot + tlv_off += TLV_INFO_SIZE + tlv_tot tlv_info = b[tlv_off:tlv_off + TLV_INFO_SIZE] magic, tlv_tot = struct.unpack('HH', tlv_info) if magic != TLV_INFO_MAGIC: return VerifyResult.INVALID_TLV_INFO_MAGIC, None, None, None - # This is set by existence of TLV SIG_PURE - is_pure = False + # Define the unprotected-TLV window + unprot_off = tlv_off + TLV_INFO_SIZE + unprot_end = unprot_off + tlv_tot - prot_tlv_size = tlv_off - hash_region = b[:prot_tlv_size] - tlv_end = tlv_off + tlv_tot - tlv_off += TLV_INFO_SIZE # skip tlv info + # Region up to the start of unprotected TLVs is hashed + prot_tlv_end = unprot_off - TLV_INFO_SIZE + hash_region = b[:prot_tlv_end] - # First scan all TLVs in search of SIG_PURE - while tlv_off < tlv_end: - tlv = b[tlv_off:tlv_off + TLV_SIZE] + # This is set by existence of TLV SIG_PURE + is_pure = False + scan_off = unprot_off + while scan_off < unprot_end: + tlv = b[scan_off:scan_off + TLV_SIZE] tlv_type, _, tlv_len = struct.unpack('BBH', tlv) if tlv_type == TLV_VALUES['SIG_PURE']: is_pure = True break - tlv_off += TLV_SIZE + tlv_len + scan_off += TLV_SIZE + tlv_len + if key is not None and not isinstance(key, list): + key = [key] + + verify_results = [] + scan_off = unprot_off digest = None - tlv_off = prot_tlv_size - tlv_end = tlv_off + tlv_tot - tlv_off += TLV_INFO_SIZE # skip tlv info - while tlv_off < tlv_end: - tlv = b[tlv_off:tlv_off + TLV_SIZE] + prot_tlv_size = unprot_off - TLV_INFO_SIZE + + # Verify hash and signatures + while scan_off < unprot_end: + tlv = b[scan_off:scan_off + TLV_SIZE] tlv_type, _, tlv_len = struct.unpack('BBH', tlv) if is_sha_tlv(tlv_type): - if not tlv_matches_key_type(tlv_type, key): + if not tlv_matches_key_type(tlv_type, key[0]): return VerifyResult.KEY_MISMATCH, None, None, None - off = tlv_off + TLV_SIZE + off = scan_off + TLV_SIZE digest = get_digest(tlv_type, hash_region) - if digest == b[off:off + tlv_len]: - if key is None: - return VerifyResult.OK, version, digest, None - else: - return VerifyResult.INVALID_HASH, None, None, None - elif not is_pure and key is not None and tlv_type == TLV_VALUES[key.sig_tlv()]: - off = tlv_off + TLV_SIZE - tlv_sig = b[off:off + tlv_len] - payload = b[:prot_tlv_size] - try: - if hasattr(key, 'verify'): - key.verify(tlv_sig, payload) - else: - key.verify_digest(tlv_sig, digest) - return VerifyResult.OK, version, digest, None - except InvalidSignature: - # continue to next TLV - pass + if digest != b[off:off + tlv_len]: + verify_results.append(("Digest", "INVALID_HASH")) + + elif not is_pure and key is not None and tlv_type == TLV_VALUES[key[0].sig_tlv()]: + for idx, k in enumerate(key): + if tlv_type == TLV_VALUES[k.sig_tlv()]: + off = scan_off + TLV_SIZE + tlv_sig = b[off:off + tlv_len] + payload = b[:prot_tlv_size] + try: + if hasattr(k, 'verify'): + k.verify(tlv_sig, payload) + else: + k.verify_digest(tlv_sig, digest) + verify_results.append((f"Key {idx}", "OK")) + break + except InvalidSignature: + # continue to next TLV + verify_results.append((f"Key {idx}", "INVALID_SIGNATURE")) + continue + elif is_pure and key is not None and tlv_type in ALLOWED_PURE_SIG_TLVS: - off = tlv_off + TLV_SIZE + # pure signature verification + off = scan_off + TLV_SIZE tlv_sig = b[off:off + tlv_len] + k = key[0] try: - key.verify_digest(tlv_sig, hash_region) + k.verify_digest(tlv_sig, hash_region) return VerifyResult.OK, version, None, tlv_sig except InvalidSignature: - # continue to next TLV - pass - tlv_off += TLV_SIZE + tlv_len - return VerifyResult.INVALID_SIGNATURE, None, None, None + return VerifyResult.INVALID_SIGNATURE, None, None, None + + scan_off += TLV_SIZE + tlv_len + # Now print out the verification results: + for k, result in verify_results: + print(f"{k}: {result}") + + # Decide on a final return (for example, OK only if at least one signature is valid) + if any(result == "OK" for _, result in verify_results): + return VerifyResult.OK, version, digest, None + else: + return VerifyResult.INVALID_SIGNATURE, None, None, None + def set_key_ids(self, key_ids): """Set list of key IDs (integers) to be inserted before each signature.""" diff --git a/scripts/imgtool/main.py b/scripts/imgtool/main.py index 5ff1f8f9f3..3fbb4384e4 100755 --- a/scripts/imgtool/main.py +++ b/scripts/imgtool/main.py @@ -92,12 +92,14 @@ def load_signature(sigfile): signature = base64.b64decode(f.read()) return signature - -def save_signature(sigfile, sig): +def save_signatures(sigfile, sig): with open(sigfile, 'wb') as f: - signature = base64.b64encode(sig) - f.write(signature) - + if isinstance(sig, list): + for s in sig: + encoded = base64.b64encode(s) + f.write(encoded + b'\n') + else: + f.write(base64.b64encode(sig)) def load_key(keyfile): # TODO: better handling of invalid pass-phrase @@ -223,11 +225,14 @@ def getpriv(key, minimal, format): @click.argument('imgfile') -@click.option('-k', '--key', metavar='filename') +@click.option('-k', '--key', multiple=True, metavar='filename') @click.command(help="Check that signed image can be verified by given key") def verify(key, imgfile): - key = load_key(key) if key else None - ret, version, digest, signature = image.Image.verify(imgfile, key) + if key: + keys = [load_key(k) for k in key] + else: + keys = None + ret, version, digest, signature = image.Image.verify(imgfile, keys) if ret == image.VerifyResult.OK: print("Image was correctly validated") print("Image version: {}.{}.{}+{}".format(*version)) @@ -422,7 +427,7 @@ def convert(self, value, param, ctx): @click.option('--public-key-format', type=click.Choice(['hash', 'full']), default='hash', help='In what format to add the public key to ' 'the image manifest: full key or hash of the key.') -@click.option('-k', '--key', metavar='filename') +@click.option('-k', '--key', multiple=True, metavar='filename') @click.option('--fix-sig', metavar='filename', help='fixed signature for the image. It will be used instead of ' 'the signature calculated using the public key') @@ -444,6 +449,8 @@ def convert(self, value, param, ctx): help='send to OUTFILE the payload or payload''s digest instead ' 'of complied image. These data can be used for external image ' 'signing') +@click.option('--key-ids', multiple=True, type=int, required=False, + help='List of integer key IDs for each signature.') @click.command(help='''Create a signed or unsigned image\n INFILE and OUTFILE are parsed as Intel HEX if the params have .hex extension, otherwise binary format is used''') @@ -453,7 +460,7 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, dependencies, load_addr, hex_addr, erased_val, save_enctlv, security_counter, boot_record, custom_tlv, rom_fixed, max_align, clear, fix_sig, fix_sig_pubkey, sig_out, user_sha, hmac_sha, is_pure, - vector_to_sign, non_bootable): + vector_to_sign, non_bootable, key_ids): if confirm: # Confirmed but non-padded images don't make much sense, because @@ -469,21 +476,28 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, non_bootable=non_bootable) compression_tlvs = {} img.load(infile) - key = load_key(key) if key else None + if key: + loaded_keys = [load_key(k) for k in key] + else: + loaded_keys = None + enckey = load_key(encrypt) if encrypt else None if enckey and key: - if ((isinstance(key, keys.ECDSA256P1) and + first_key = loaded_keys[0] + if ((isinstance(first_key, keys.ECDSA256P1) and not isinstance(enckey, keys.ECDSA256P1Public)) - or (isinstance(key, keys.ECDSA384P1) and + or (isinstance(first_key, keys.ECDSA384P1) and not isinstance(enckey, keys.ECDSA384P1Public)) - or (isinstance(key, keys.RSA) and + or (isinstance(first_key, keys.RSA) and not isinstance(enckey, keys.RSAPublic))): # FIXME raise click.UsageError("Signing and encryption must use the same " "type of key") - if pad_sig and hasattr(key, 'pad_sig'): - key.pad_sig = True + if pad_sig and loaded_keys: + for k in loaded_keys: + if hasattr(k, 'pad_sig'): + k.pad_sig = True # Get list of custom protected TLVs from the command-line custom_tlvs = {} @@ -526,7 +540,7 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, 'and forbids sha selection by user.') if compression in ["lzma2", "lzma2armthumb"]: - img.create(key, public_key_format, enckey, dependencies, boot_record, + img.create(loaded_keys, public_key_format, enckey, dependencies, boot_record, custom_tlvs, compression_tlvs, None, int(encrypt_keylen), clear, baked_signature, pub_key, vector_to_sign, user_sha=user_sha, hmac_sha=hmac_sha, is_pure=is_pure, keep_comp_size=False, dont_encrypt=True) @@ -577,14 +591,14 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, is_pure=is_pure, keep_comp_size=keep_comp_size) img = compressed_img else: - img.create(key, public_key_format, enckey, dependencies, boot_record, + img.create(loaded_keys, public_key_format, enckey, dependencies, boot_record, custom_tlvs, compression_tlvs, None, int(encrypt_keylen), clear, baked_signature, pub_key, vector_to_sign, user_sha=user_sha, hmac_sha=hmac_sha, is_pure=is_pure) img.save(outfile, hex_addr) if sig_out is not None: new_signature = img.get_signature() - save_signature(sig_out, new_signature) + save_signatures(sig_out, new_signature) class AliasesGroup(click.Group): From df7225f660eaf2b57cac7c7a6ae818e575a70b57 Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Mon, 12 May 2025 15:23:49 +0100 Subject: [PATCH 3/6] bootutil: Add support for multi-sign of same type This commit adds functionality to the bootutil library to support multiple sign verfication of same type when 'MCUBOOT_BUILTIN_KEY' or 'MCUBOOT_HW_KEY' is enabled. The image_validate.c file is refactored such that: * bootutil_find_key() find the key is moved to a new file bootutil_find_key.c. * bootutil_image_hash() is moved to a new file bootutil_image_hash.c. * bootutil_img_security_cnt() is moved to a new file bootutil_img_security_cnt.c. This allows common validation code to be reused for multiple signatures. All code specific to multi sign is under the option 'MCUBOOT_IMAGE_MULTI_SIG_SUPPORT'. Furthermore, key id type is updated to uint32_t as per PSA crypto spec. Signed-off-by: Maulik Patel Change-Id: I05c97ac385c5816c812c51feb010028df8412fe5 --- boot/bootutil/CMakeLists.txt | 6 +- boot/bootutil/include/bootutil/image.h | 17 + boot/bootutil/include/bootutil/sign_key.h | 27 ++ boot/bootutil/src/bootutil_find_key.c | 137 +++++++ boot/bootutil/src/bootutil_img_hash.c | 198 ++++++++++ boot/bootutil/src/bootutil_img_security_cnt.c | 100 +++++ boot/bootutil/src/bootutil_priv.h | 2 +- boot/bootutil/src/image_ecdsa.c | 4 +- boot/bootutil/src/image_ed25519.c | 2 +- boot/bootutil/src/image_rsa.c | 2 +- boot/bootutil/src/image_validate.c | 361 ++---------------- .../mcuboot_config/mcuboot_config.template.h | 14 + sim/mcuboot-sys/build.rs | 2 + 13 files changed, 537 insertions(+), 335 deletions(-) create mode 100644 boot/bootutil/src/bootutil_find_key.c create mode 100644 boot/bootutil/src/bootutil_img_hash.c create mode 100644 boot/bootutil/src/bootutil_img_security_cnt.c diff --git a/boot/bootutil/CMakeLists.txt b/boot/bootutil/CMakeLists.txt index 3d1b32717e..11aa7b7e4a 100644 --- a/boot/bootutil/CMakeLists.txt +++ b/boot/bootutil/CMakeLists.txt @@ -1,5 +1,5 @@ #------------------------------------------------------------------------------ -# Copyright (c) 2020-2023, Arm Limited. All rights reserved. +# Copyright (c) 2020-2025, Arm Limited. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 # @@ -17,6 +17,9 @@ target_include_directories(bootutil target_sources(bootutil PRIVATE src/boot_record.c + src/bootutil_find_key.c + src/bootutil_img_hash.c + src/bootutil_img_security_cnt.c src/bootutil_misc.c src/bootutil_public.c src/caps.c @@ -33,6 +36,7 @@ target_sources(bootutil src/swap_scratch.c src/tlv.c ) + if(CONFIG_BOOT_RAM_LOAD) target_sources(bootutil PRIVATE diff --git a/boot/bootutil/include/bootutil/image.h b/boot/bootutil/include/bootutil/image.h index d1014b091f..bde4df1f2f 100644 --- a/boot/bootutil/include/bootutil/image.h +++ b/boot/bootutil/include/bootutil/image.h @@ -236,6 +236,23 @@ int32_t bootutil_get_img_security_cnt(struct boot_loader_state *state, int slot, const struct flash_area *fap, uint32_t *img_security_cnt); +#if defined(MCUBOOT_BUILTIN_KEY) +int bootutil_find_key(uint8_t image_index, uint8_t *key_id_buf, uint8_t key_id_buf_len); +#elif defined(MCUBOOT_HW_KEY) +int bootutil_find_key(uint8_t image_index, uint8_t *key, uint16_t key_len); +#else +int bootutil_find_key(uint8_t image_index, uint8_t *keyhash, uint8_t keyhash_len); +#endif + +int bootutil_img_hash(struct boot_loader_state *state, + struct image_header *hdr, const struct flash_area *fap, + uint8_t *tmp_buf, uint32_t tmp_buf_sz, uint8_t *hash_result, + uint8_t *seed, int seed_len +#if defined(MCUBOOT_SWAP_USING_OFFSET) && defined(MCUBOOT_SERIAL_RECOVERY) + , uint32_t start_offset +#endif +); + #ifdef __cplusplus } #endif diff --git a/boot/bootutil/include/bootutil/sign_key.h b/boot/bootutil/include/bootutil/sign_key.h index 58bfaf5b06..22352112f8 100644 --- a/boot/bootutil/include/bootutil/sign_key.h +++ b/boot/bootutil/include/bootutil/sign_key.h @@ -27,6 +27,9 @@ /* mcuboot_config.h is needed for MCUBOOT_HW_KEY to work */ #include "mcuboot_config/mcuboot_config.h" +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT +#include +#endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ #ifdef __cplusplus extern "C" { @@ -62,6 +65,7 @@ extern struct bootutil_key bootutil_keys[]; * Retrieve the hash of the corresponding public key for image authentication. * * @param[in] image_index Index of the image to be authenticated. + * @param[in] key_index Index of the key to be used. * @param[out] public_key_hash Buffer to store the key-hash in. * @param[in,out] key_hash_size As input the size of the buffer. As output * the actual key-hash length. @@ -69,10 +73,33 @@ extern struct bootutil_key bootutil_keys[]; * @return 0 on success; nonzero on failure. */ int boot_retrieve_public_key_hash(uint8_t image_index, + uint8_t key_index, uint8_t *public_key_hash, size_t *key_hash_size); + #endif /* !MCUBOOT_HW_KEY */ +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT +/** + * @brief Checks the key policy for signature verification. + * + * Determines whether a given key might or must be used to sign an image, + * based on the validity of the signature and the key index. Updates the + * provided output parameters to reflect the policy. + * + * @param valid_sig Indicates if the signature is valid. + * @param key The key index to check. + * @param[out] key_might_sign Set to true if the key might be used to sign. + * @param[out] key_must_sign Set to true if the key must be used to sign. + * @param[out] key_must_sign_count Set to the number of keys that must sign. + * + * @return 0 on success, or a negative error code on failure. + */ +int boot_plat_check_key_policy(bool valid_sig, uint32_t key, + bool *key_might_sign, bool *key_must_sign, + uint8_t *key_must_sign_count); +#endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ + extern const int bootutil_key_cnt; #ifdef __cplusplus diff --git a/boot/bootutil/src/bootutil_find_key.c b/boot/bootutil/src/bootutil_find_key.c new file mode 100644 index 0000000000..9d17b3c55e --- /dev/null +++ b/boot/bootutil/src/bootutil_find_key.c @@ -0,0 +1,137 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * Copyright (c) 2017-2019 Linaro LTD + * Copyright (c) 2016-2019 JUUL Labs + * Copyright (c) 2019-2025 Arm Limited + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * Original license: + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include + +#include "bootutil/bootutil_log.h" +#include "bootutil/crypto/sha.h" +#include "bootutil/fault_injection_hardening.h" +#include "bootutil/image.h" +#include "bootutil/sign_key.h" +#include "bootutil_priv.h" +#include "mcuboot_config/mcuboot_config.h" + +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT +#define NUM_OF_KEYS MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE +#else +#define NUM_OF_KEYS 1 +#endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ + +#if defined(MCUBOOT_BUILTIN_KEY) +int bootutil_find_key(uint8_t image_index, uint8_t *key_id_buf, uint8_t key_id_buf_len) +{ + uint32_t key_id; + FIH_DECLARE(fih_rc, FIH_FAILURE); + + BOOT_LOG_DBG("bootutil_find_key: image_index %d", image_index); + + /* Key id is passed */ + assert(key_id_buf_len == sizeof(uint32_t)); + memcpy(&key_id, key_id_buf, sizeof(key_id)); + + /* Check if key id is associated with the image */ + FIH_CALL(boot_verify_key_id_for_image, fih_rc, image_index, key_id); + if (FIH_EQ(fih_rc, FIH_SUCCESS)) { + return (int32_t)key_id; + } + + return -1; +} + +#elif defined(MCUBOOT_HW_KEY) +extern unsigned int pub_key_len; +int bootutil_find_key(uint8_t image_index, uint8_t *key, uint16_t key_len) +{ + bootutil_sha_context sha_ctx; + uint8_t hash[IMAGE_HASH_SIZE]; + uint8_t key_hash[IMAGE_HASH_SIZE]; + size_t key_hash_size = sizeof(key_hash); + int rc; + uint8_t key_index; + FIH_DECLARE(fih_rc, FIH_FAILURE); + + BOOT_LOG_DBG("bootutil_find_key: image_index %d", image_index); + + bootutil_sha_init(&sha_ctx); + bootutil_sha_update(&sha_ctx, key, key_len); + bootutil_sha_finish(&sha_ctx, hash); + bootutil_sha_drop(&sha_ctx); + + for(key_index = 0; key_index < NUM_OF_KEYS; key_index++) { + rc = boot_retrieve_public_key_hash(image_index, key_index, key_hash, &key_hash_size); + if (rc) { + return -1; + } + + /* Adding hardening to avoid this potential attack: + * - Image is signed with an arbitrary key and the corresponding public + * key is added as a TLV field. + * - During public key validation (comparing against key-hash read from + * HW) a fault is injected to accept the public key as valid one. + */ + FIH_CALL(boot_fih_memequal, fih_rc, hash, key_hash, key_hash_size); + if (FIH_EQ(fih_rc, FIH_SUCCESS)) { + BOOT_LOG_INF("Key %d hash found for image %d", key_index, image_index); + bootutil_keys[0].key = key; + pub_key_len = key_len; + return 0; + } + } + BOOT_LOG_ERR("Key hash NOT found for image %d!", image_index); + + return -1; +} + +#else /* !defined MCUBOOT_BUILTIN_KEY && !defined MCUBOOT_HW_KEY */ +int bootutil_find_key(uint8_t image_index, uint8_t *keyhash, uint8_t keyhash_len) +{ + bootutil_sha_context sha_ctx; + int i; + uint8_t hash[IMAGE_HASH_SIZE]; + const struct bootutil_key *key; + (void)image_index; + + BOOT_LOG_DBG("bootutil_find_key"); + + if (keyhash_len > IMAGE_HASH_SIZE) { + return -1; + } + + for (i = 0; i < bootutil_key_cnt; i++) { + key = &bootutil_keys[i]; + bootutil_sha_init(&sha_ctx); + bootutil_sha_update(&sha_ctx, key->key, *key->len); + bootutil_sha_finish(&sha_ctx, hash); + bootutil_sha_drop(&sha_ctx); + if (!memcmp(hash, keyhash, keyhash_len)) { + return i; + } + } + return -1; +} +#endif diff --git a/boot/bootutil/src/bootutil_img_hash.c b/boot/bootutil/src/bootutil_img_hash.c new file mode 100644 index 0000000000..cb8eca48ab --- /dev/null +++ b/boot/bootutil/src/bootutil_img_hash.c @@ -0,0 +1,198 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * Copyright (c) 2017-2019 Linaro LTD + * Copyright (c) 2016-2019 JUUL Labs + * Copyright (c) 2019-2025 Arm Limited + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * Original license: + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include +#include + +#include "bootutil/bootutil_log.h" +#include "bootutil/crypto/sha.h" +#include "bootutil/fault_injection_hardening.h" +#include "bootutil/image.h" +#include "bootutil_priv.h" +#include "mcuboot_config/mcuboot_config.h" + +#ifndef MCUBOOT_SIGN_PURE +/* + * Compute SHA hash over the image. + * (SHA384 if ECDSA-P384 is being used, + * SHA256 otherwise). + */ +int +bootutil_img_hash(struct boot_loader_state *state, + struct image_header *hdr, const struct flash_area *fap, + uint8_t *tmp_buf, uint32_t tmp_buf_sz, uint8_t *hash_result, + uint8_t *seed, int seed_len + ) +{ + bootutil_sha_context sha_ctx; + uint32_t size; + uint16_t hdr_size; + uint32_t blk_off; + uint32_t tlv_off; +#if !defined(MCUBOOT_HASH_STORAGE_DIRECTLY) + int rc; + uint32_t off; + uint32_t blk_sz; +#endif +#ifdef MCUBOOT_HASH_STORAGE_DIRECTLY + uintptr_t base = 0; + int fa_ret; +#endif +#if defined(MCUBOOT_ENC_IMAGES) + struct enc_key_data *enc_state; + int image_index; +#endif +#if defined(MCUBOOT_SWAP_USING_OFFSET) + uint32_t sector_off = 0; +#endif + +#if (BOOT_IMAGE_NUMBER == 1) || !defined(MCUBOOT_ENC_IMAGES) || \ + defined(MCUBOOT_RAM_LOAD) + (void)state; + (void)hdr_size; + (void)blk_off; + (void)tlv_off; +#ifdef MCUBOOT_RAM_LOAD + (void)blk_sz; + (void)off; + (void)rc; + (void)fap; + (void)tmp_buf; + (void)tmp_buf_sz; +#endif +#endif + BOOT_LOG_DBG("bootutil_img_hash"); + +#ifdef MCUBOOT_ENC_IMAGES + if (state == NULL) { + enc_state = NULL; + image_index = 0; + } else { + enc_state = BOOT_CURR_ENC(state); + image_index = BOOT_CURR_IMG(state); + } + + /* Encrypted images only exist in the secondary slot */ + if (MUST_DECRYPT(fap, image_index, hdr) && + !boot_enc_valid(enc_state, 1)) { + BOOT_LOG_DBG("bootutil_img_hash: error encrypted image found in primary slot"); + return -1; + } +#endif + +#if defined(MCUBOOT_SWAP_USING_OFFSET) + /* For swap using offset mode, the image starts in the second sector of the upgrade slot, so + * apply the offset when this is needed + */ + sector_off = boot_get_state_secondary_offset(state, fap); +#endif + + bootutil_sha_init(&sha_ctx); + + /* in some cases (split image) the hash is seeded with data from + * the loader image */ + if (seed && (seed_len > 0)) { + bootutil_sha_update(&sha_ctx, seed, seed_len); + } + + /* Hash is computed over image header and image itself. */ + size = hdr_size = hdr->ih_hdr_size; + size += hdr->ih_img_size; + tlv_off = size; + + /* If protected TLVs are present they are also hashed. */ + size += hdr->ih_protect_tlv_size; + +#ifdef MCUBOOT_HASH_STORAGE_DIRECTLY + /* No chunk loading, storage is mapped to address space and can + * be directly given to hashing function. + */ + fa_ret = flash_device_base(flash_area_get_device_id(fap), &base); + if (fa_ret != 0) { + base = 0; + } + + bootutil_sha_update(&sha_ctx, (void *)(base + flash_area_get_off(fap)), size); +#else /* MCUBOOT_HASH_STORAGE_DIRECTLY */ +#ifdef MCUBOOT_RAM_LOAD + bootutil_sha_update(&sha_ctx, + (void*)(IMAGE_RAM_BASE + hdr->ih_load_addr), + size); +#else + for (off = 0; off < size; off += blk_sz) { + blk_sz = size - off; + if (blk_sz > tmp_buf_sz) { + blk_sz = tmp_buf_sz; + } +#ifdef MCUBOOT_ENC_IMAGES + /* The only data that is encrypted in an image is the payload; + * both header and TLVs (when protected) are not. + */ + if ((off < hdr_size) && ((off + blk_sz) > hdr_size)) { + /* read only the header */ + blk_sz = hdr_size - off; + } + if ((off < tlv_off) && ((off + blk_sz) > tlv_off)) { + /* read only up to the end of the image payload */ + blk_sz = tlv_off - off; + } +#endif +#if defined(MCUBOOT_SWAP_USING_OFFSET) + rc = flash_area_read(fap, off + sector_off, tmp_buf, blk_sz); +#else + rc = flash_area_read(fap, off, tmp_buf, blk_sz); +#endif + if (rc) { + bootutil_sha_drop(&sha_ctx); + BOOT_LOG_DBG("bootutil_img_validate Error %d reading data chunk %p %u %u", + rc, fap, off, blk_sz); + return rc; + } +#ifdef MCUBOOT_ENC_IMAGES + if (MUST_DECRYPT(fap, image_index, hdr)) { + /* Only payload is encrypted (area between header and TLVs) */ + int slot = flash_area_id_to_multi_image_slot(image_index, + flash_area_get_id(fap)); + + if (off >= hdr_size && off < tlv_off) { + blk_off = (off - hdr_size) & 0xf; + boot_enc_decrypt(enc_state, slot, off - hdr_size, + blk_sz, blk_off, tmp_buf); + } + } +#endif + bootutil_sha_update(&sha_ctx, tmp_buf, blk_sz); + } +#endif /* MCUBOOT_RAM_LOAD */ +#endif /* MCUBOOT_HASH_STORAGE_DIRECTLY */ + bootutil_sha_finish(&sha_ctx, hash_result); + bootutil_sha_drop(&sha_ctx); + + return 0; +} +#endif /* !MCUBOOT_SIGN_PURE */ diff --git a/boot/bootutil/src/bootutil_img_security_cnt.c b/boot/bootutil/src/bootutil_img_security_cnt.c new file mode 100644 index 0000000000..c141c56d0b --- /dev/null +++ b/boot/bootutil/src/bootutil_img_security_cnt.c @@ -0,0 +1,100 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * Copyright (c) 2017-2019 Linaro LTD + * Copyright (c) 2016-2019 JUUL Labs + * Copyright (c) 2019-2025 Arm Limited + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * Original license: + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include +#include + +#include "bootutil/image.h" +#include "bootutil/security_cnt.h" +#include "bootutil_priv.h" + +/** + * Reads the value of an image's security counter. + * + * @param state Pointer to the boot state object. + * @param slot Slot of the current image to get the security counter of. + * @param fap Pointer to a description structure of the image's + * flash area. + * @param security_cnt Pointer to store the security counter value. + * + * @return 0 on success; nonzero on failure. + */ +int32_t +bootutil_get_img_security_cnt(struct boot_loader_state *state, int slot, + const struct flash_area *fap, + uint32_t *img_security_cnt) +{ + struct image_tlv_iter it; + uint32_t off; + uint16_t len; + int32_t rc; + + if ((state == NULL) || + (boot_img_hdr(state, slot) == NULL) || + (fap == NULL) || + (img_security_cnt == NULL)) { + /* Invalid parameter. */ + return BOOT_EBADARGS; + } + + /* The security counter TLV is in the protected part of the TLV area. */ + if (boot_img_hdr(state, slot)->ih_protect_tlv_size == 0) { + return BOOT_EBADIMAGE; + } + +#if defined(MCUBOOT_SWAP_USING_OFFSET) + it.start_off = boot_get_state_secondary_offset(state, fap); +#endif + + rc = bootutil_tlv_iter_begin(&it, boot_img_hdr(state, slot), fap, IMAGE_TLV_SEC_CNT, true); + if (rc) { + return rc; + } + + /* Traverse through the protected TLV area to find + * the security counter TLV. + */ + + rc = bootutil_tlv_iter_next(&it, &off, &len, NULL); + if (rc != 0) { + /* Security counter TLV has not been found. */ + return -1; + } + + if (len != sizeof(*img_security_cnt)) { + /* Security counter is not valid. */ + return BOOT_EBADIMAGE; + } + + rc = LOAD_IMAGE_DATA(boot_img_hdr(state, slot), fap, off, img_security_cnt, len); + if (rc != 0) { + return BOOT_EFLASH; + } + + return 0; +} diff --git a/boot/bootutil/src/bootutil_priv.h b/boot/bootutil/src/bootutil_priv.h index b62af146c9..830cbe0c00 100644 --- a/boot/bootutil/src/bootutil_priv.h +++ b/boot/bootutil/src/bootutil_priv.h @@ -305,7 +305,7 @@ struct boot_sector_buffer { * provided signature. */ fih_ret bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, - size_t slen, uint8_t key_id); + size_t slen, uint32_t key_id); /* The function is intended for direct verification of image * against provided signature. diff --git a/boot/bootutil/src/image_ecdsa.c b/boot/bootutil/src/image_ecdsa.c index 30c7d0d0f3..a7020697ce 100644 --- a/boot/bootutil/src/image_ecdsa.c +++ b/boot/bootutil/src/image_ecdsa.c @@ -41,7 +41,7 @@ BOOT_LOG_MODULE_DECLARE(mcuboot); #if !defined(MCUBOOT_BUILTIN_KEY) fih_ret bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, size_t slen, - uint8_t key_id) + uint32_t key_id) { int rc; bootutil_ecdsa_context ctx; @@ -74,7 +74,7 @@ bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, size_t slen, #else /* !MCUBOOT_BUILTIN_KEY */ fih_ret bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, size_t slen, - uint8_t key_id) + uint32_t key_id) { int rc; bootutil_ecdsa_context ctx; diff --git a/boot/bootutil/src/image_ed25519.c b/boot/bootutil/src/image_ed25519.c index 4d83bb3d7c..b469b22611 100644 --- a/boot/bootutil/src/image_ed25519.c +++ b/boot/bootutil/src/image_ed25519.c @@ -149,7 +149,7 @@ bootutil_verify(uint8_t *buf, uint32_t blen, fih_ret bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, size_t slen, - uint8_t key_id) + uint32_t key_id) { FIH_DECLARE(fih_rc, FIH_FAILURE); diff --git a/boot/bootutil/src/image_rsa.c b/boot/bootutil/src/image_rsa.c index 5479b75ebd..f047838e8a 100644 --- a/boot/bootutil/src/image_rsa.c +++ b/boot/bootutil/src/image_rsa.c @@ -262,7 +262,7 @@ bootutil_cmp_rsasig(bootutil_rsa_context *ctx, uint8_t *hash, uint32_t hlen, fih_ret bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, size_t slen, - uint8_t key_id) + uint32_t key_id) { bootutil_rsa_context ctx; int rc; diff --git a/boot/bootutil/src/image_validate.c b/boot/bootutil/src/image_validate.c index c202c0a8f7..27709ad130 100644 --- a/boot/bootutil/src/image_validate.c +++ b/boot/bootutil/src/image_validate.c @@ -61,167 +61,6 @@ BOOT_LOG_MODULE_DECLARE(mcuboot); #include "bootutil_priv.h" -#ifndef MCUBOOT_SIGN_PURE -/* - * Compute SHA hash over the image. - * (SHA384 if ECDSA-P384 is being used, - * SHA256 otherwise). - */ -static int -bootutil_img_hash(struct boot_loader_state *state, - struct image_header *hdr, const struct flash_area *fap, - uint8_t *tmp_buf, uint32_t tmp_buf_sz, uint8_t *hash_result, - uint8_t *seed, int seed_len - ) -{ - bootutil_sha_context sha_ctx; - uint32_t size; - uint16_t hdr_size; - uint32_t blk_off; - uint32_t tlv_off; -#if !defined(MCUBOOT_HASH_STORAGE_DIRECTLY) - int rc; - uint32_t off; - uint32_t blk_sz; -#endif -#ifdef MCUBOOT_HASH_STORAGE_DIRECTLY - uintptr_t base = 0; - int fa_ret; -#endif -#if defined(MCUBOOT_ENC_IMAGES) - struct enc_key_data *enc_state; - int image_index; -#endif -#if defined(MCUBOOT_SWAP_USING_OFFSET) - uint32_t sector_off = 0; -#endif - -#if (BOOT_IMAGE_NUMBER == 1) || !defined(MCUBOOT_ENC_IMAGES) || \ - defined(MCUBOOT_RAM_LOAD) - (void)state; - (void)hdr_size; - (void)blk_off; - (void)tlv_off; -#ifdef MCUBOOT_RAM_LOAD - (void)blk_sz; - (void)off; - (void)rc; - (void)fap; - (void)tmp_buf; - (void)tmp_buf_sz; -#endif -#endif - BOOT_LOG_DBG("bootutil_img_hash"); - -#ifdef MCUBOOT_ENC_IMAGES - if (state == NULL) { - enc_state = NULL; - image_index = 0; - } else { - enc_state = BOOT_CURR_ENC(state); - image_index = BOOT_CURR_IMG(state); - } - - /* Encrypted images only exist in the secondary slot */ - if (MUST_DECRYPT(fap, image_index, hdr) && - !boot_enc_valid(enc_state, 1)) { - BOOT_LOG_DBG("bootutil_img_hash: error encrypted image found in primary slot"); - return -1; - } -#endif - -#if defined(MCUBOOT_SWAP_USING_OFFSET) - /* For swap using offset mode, the image starts in the second sector of the upgrade slot, so - * apply the offset when this is needed - */ - sector_off = boot_get_state_secondary_offset(state, fap); -#endif - - bootutil_sha_init(&sha_ctx); - - /* in some cases (split image) the hash is seeded with data from - * the loader image */ - if (seed && (seed_len > 0)) { - bootutil_sha_update(&sha_ctx, seed, seed_len); - } - - /* Hash is computed over image header and image itself. */ - size = hdr_size = hdr->ih_hdr_size; - size += hdr->ih_img_size; - tlv_off = size; - - /* If protected TLVs are present they are also hashed. */ - size += hdr->ih_protect_tlv_size; - -#ifdef MCUBOOT_HASH_STORAGE_DIRECTLY - /* No chunk loading, storage is mapped to address space and can - * be directly given to hashing function. - */ - fa_ret = flash_device_base(flash_area_get_device_id(fap), &base); - if (fa_ret != 0) { - base = 0; - } - - bootutil_sha_update(&sha_ctx, (void *)(base + flash_area_get_off(fap)), size); -#else /* MCUBOOT_HASH_STORAGE_DIRECTLY */ -#ifdef MCUBOOT_RAM_LOAD - bootutil_sha_update(&sha_ctx, - (void*)(IMAGE_RAM_BASE + hdr->ih_load_addr), - size); -#else - for (off = 0; off < size; off += blk_sz) { - blk_sz = size - off; - if (blk_sz > tmp_buf_sz) { - blk_sz = tmp_buf_sz; - } -#ifdef MCUBOOT_ENC_IMAGES - /* The only data that is encrypted in an image is the payload; - * both header and TLVs (when protected) are not. - */ - if ((off < hdr_size) && ((off + blk_sz) > hdr_size)) { - /* read only the header */ - blk_sz = hdr_size - off; - } - if ((off < tlv_off) && ((off + blk_sz) > tlv_off)) { - /* read only up to the end of the image payload */ - blk_sz = tlv_off - off; - } -#endif -#if defined(MCUBOOT_SWAP_USING_OFFSET) - rc = flash_area_read(fap, off + sector_off, tmp_buf, blk_sz); -#else - rc = flash_area_read(fap, off, tmp_buf, blk_sz); -#endif - if (rc) { - bootutil_sha_drop(&sha_ctx); - BOOT_LOG_DBG("bootutil_img_validate Error %d reading data chunk %p %u %u", - rc, fap, off, blk_sz); - return rc; - } -#ifdef MCUBOOT_ENC_IMAGES - if (MUST_DECRYPT(fap, image_index, hdr)) { - /* Only payload is encrypted (area between header and TLVs) */ - int slot = flash_area_id_to_multi_image_slot(image_index, - flash_area_get_id(fap)); - - if (off >= hdr_size && off < tlv_off) { - blk_off = (off - hdr_size) & 0xf; - boot_enc_decrypt(enc_state, slot, off - hdr_size, - blk_sz, blk_off, tmp_buf); - } - } -#endif - bootutil_sha_update(&sha_ctx, tmp_buf, blk_sz); - } -#endif /* MCUBOOT_RAM_LOAD */ -#endif /* MCUBOOT_HASH_STORAGE_DIRECTLY */ - bootutil_sha_finish(&sha_ctx, hash_result); - bootutil_sha_drop(&sha_ctx); - - return 0; -} -#endif - /* * Currently, we only support being able to verify one type of * signature, because there is a single verification function that we @@ -265,182 +104,24 @@ bootutil_img_hash(struct boot_loader_state *state, #endif #ifdef EXPECTED_SIG_TLV +#ifdef MCUBOOT_BUILTIN_KEY +#define EXPECTED_KEY_TLV IMAGE_TLV_KEYID +#define KEY_BUF_SIZE sizeof(uint32_t) -#if !defined(MCUBOOT_BUILTIN_KEY) -#if !defined(MCUBOOT_HW_KEY) -/* The key TLV contains the hash of the public key. */ -# define EXPECTED_KEY_TLV IMAGE_TLV_KEYHASH -# define KEY_BUF_SIZE IMAGE_HASH_SIZE -#else +#elif defined(MCUBOOT_HW_KEY) /* The key TLV contains the whole public key. * Add a few extra bytes to the key buffer size for encoding and * for public exponent. */ -# define EXPECTED_KEY_TLV IMAGE_TLV_PUBKEY -# define KEY_BUF_SIZE (SIG_BUF_SIZE + 24) -#endif /* !MCUBOOT_HW_KEY */ - -#if !defined(MCUBOOT_HW_KEY) -static int -bootutil_find_key(uint8_t image_index, uint8_t *keyhash, uint8_t keyhash_len) -{ - bootutil_sha_context sha_ctx; - int i; - const struct bootutil_key *key; - (void)image_index; - - BOOT_LOG_DBG("bootutil_find_key"); - - if (keyhash_len > IMAGE_HASH_SIZE) { - return -1; - } - - for (i = 0; i < bootutil_key_cnt; i++) { - key = &bootutil_keys[i]; - bootutil_sha_init(&sha_ctx); - bootutil_sha_update(&sha_ctx, key->key, *key->len); - bootutil_sha_finish(&sha_ctx, hash); - bootutil_sha_drop(&sha_ctx); - if (!memcmp(hash, keyhash, keyhash_len)) { - return i; - } - } - return -1; -} -#else /* !MCUBOOT_HW_KEY */ -extern unsigned int pub_key_len; -static int -bootutil_find_key(uint8_t image_index, uint8_t *key, uint16_t key_len) -{ - bootutil_sha_context sha_ctx; - uint8_t hash[IMAGE_HASH_SIZE]; - uint8_t key_hash[IMAGE_HASH_SIZE]; - size_t key_hash_size = sizeof(key_hash); - int rc; - FIH_DECLARE(fih_rc, FIH_FAILURE); - - BOOT_LOG_DBG("bootutil_find_key: image_index %d", image_index); - - bootutil_sha_init(&sha_ctx); - bootutil_sha_update(&sha_ctx, key, key_len); - bootutil_sha_finish(&sha_ctx, hash); - bootutil_sha_drop(&sha_ctx); - - rc = boot_retrieve_public_key_hash(image_index, key_hash, &key_hash_size); - if (rc) { - return -1; - } - - /* Adding hardening to avoid this potential attack: - * - Image is signed with an arbitrary key and the corresponding public - * key is added as a TLV field. - * - During public key validation (comparing against key-hash read from - * HW) a fault is injected to accept the public key as valid one. - */ - FIH_CALL(boot_fih_memequal, fih_rc, hash, key_hash, key_hash_size); - if (FIH_EQ(fih_rc, FIH_SUCCESS)) { - bootutil_keys[0].key = key; - pub_key_len = key_len; - return 0; - } - - return -1; -} -#endif /* !MCUBOOT_HW_KEY */ +#define EXPECTED_KEY_TLV IMAGE_TLV_PUBKEY +#define KEY_BUF_SIZE (SIG_BUF_SIZE + 24) #else -/* For MCUBOOT_BUILTIN_KEY, key id is passed */ -#define EXPECTED_KEY_TLV IMAGE_TLV_KEYID -#define KEY_BUF_SIZE sizeof(int32_t) - -static int bootutil_find_key(uint8_t image_index, uint8_t *key_id_buf, uint8_t key_id_buf_len) -{ - int rc; - FIH_DECLARE(fih_rc, FIH_FAILURE); - - /* Key id is passed */ - assert(key_id_buf_len == sizeof(int32_t)); - int32_t key_id = (((int32_t)key_id_buf[0] << 24) | - ((int32_t)key_id_buf[1] << 16) | - ((int32_t)key_id_buf[2] << 8) | - ((int32_t)key_id_buf[3])); - - /* Check if key id is associated with the image */ - FIH_CALL(boot_verify_key_id_for_image, fih_rc, image_index, key_id); - if (FIH_EQ(fih_rc, FIH_SUCCESS)) { - return key_id; - } - - return -1; -} -#endif /* !MCUBOOT_BUILTIN_KEY */ -#endif /* EXPECTED_SIG_TLV */ - -/** - * Reads the value of an image's security counter. - * - * @param state Pointer to the boot state object. - * @param slot Slot of the current image to get the security counter of. - * @param fap Pointer to a description structure of the image's - * flash area. - * @param security_cnt Pointer to store the security counter value. - * - * @return 0 on success; nonzero on failure. - */ -int32_t -bootutil_get_img_security_cnt(struct boot_loader_state *state, int slot, - const struct flash_area *fap, - uint32_t *img_security_cnt) -{ - struct image_tlv_iter it; - uint32_t off; - uint16_t len; - int32_t rc; - - if ((state == NULL) || - (boot_img_hdr(state, slot) == NULL) || - (fap == NULL) || - (img_security_cnt == NULL)) { - /* Invalid parameter. */ - return BOOT_EBADARGS; - } - - /* The security counter TLV is in the protected part of the TLV area. */ - if (boot_img_hdr(state, slot)->ih_protect_tlv_size == 0) { - return BOOT_EBADIMAGE; - } - -#if defined(MCUBOOT_SWAP_USING_OFFSET) - it.start_off = boot_get_state_secondary_offset(state, fap); +/* The key TLV contains the hash of the public key. */ +#define EXPECTED_KEY_TLV IMAGE_TLV_KEYHASH +#define KEY_BUF_SIZE IMAGE_HASH_SIZE #endif - - rc = bootutil_tlv_iter_begin(&it, boot_img_hdr(state, slot), fap, IMAGE_TLV_SEC_CNT, true); - if (rc) { - return rc; - } - - /* Traverse through the protected TLV area to find - * the security counter TLV. - */ - - rc = bootutil_tlv_iter_next(&it, &off, &len, NULL); - if (rc != 0) { - /* Security counter TLV has not been found. */ - return -1; - } - - if (len != sizeof(*img_security_cnt)) { - /* Security counter is not valid. */ - return BOOT_EBADIMAGE; - } - - rc = LOAD_IMAGE_DATA(boot_img_hdr(state, slot), fap, off, img_security_cnt, len); - if (rc != 0) { - return BOOT_EFLASH; - } - - return 0; -} +#endif /* EXPECTED_SIG_TLV */ #if defined(MCUBOOT_SIGN_PURE) /* Returns: @@ -551,6 +232,12 @@ bootutil_img_validate(struct boot_loader_state *state, uint32_t img_security_cnt = 0; FIH_DECLARE(security_counter_valid, FIH_FAILURE); #endif +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT + + bool key_must_sign = true; + bool key_might_sign = false; + uint8_t key_must_sign_count = 0; +#endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ BOOT_LOG_DBG("bootutil_img_validate: flash area %p", fap); @@ -706,6 +393,15 @@ bootutil_img_validate(struct boot_loader_state *state, #ifndef MCUBOOT_SIGN_PURE FIH_CALL(bootutil_verify_sig, valid_signature, hash, sizeof(hash), buf, len, key_id); +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT + rc = boot_plat_check_key_policy((valid_signature == 0), key_id, + &key_might_sign, &key_must_sign, + &key_must_sign_count); + if (rc) { + goto out; + } +#endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ + #else /* Directly check signature on the image, by using the mapping of * a device to memory. The pointer is beginning of image in flash, @@ -772,7 +468,14 @@ bootutil_img_validate(struct boot_loader_state *state, rc = FIH_NOT_EQ(valid_signature, FIH_SUCCESS); #endif #ifdef EXPECTED_SIG_TLV +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT + if (FIH_NOT_EQ(key_must_sign, true) || FIH_NOT_EQ(key_might_sign, true)) { + rc = -1; + goto out; + } +#else FIH_SET(fih_rc, valid_signature); +#endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ #endif #ifdef MCUBOOT_HW_ROLLBACK_PROT if (FIH_NOT_EQ(security_counter_valid, FIH_SUCCESS)) { diff --git a/samples/mcuboot_config/mcuboot_config.template.h b/samples/mcuboot_config/mcuboot_config.template.h index ce613bd8c3..4bab323966 100644 --- a/samples/mcuboot_config/mcuboot_config.template.h +++ b/samples/mcuboot_config/mcuboot_config.template.h @@ -33,6 +33,20 @@ /* Uncomment for ECDSA signatures using curve P-256. */ /* #define MCUBOOT_SIGN_EC256 */ +/* + * Multi-signature support + * + * Currently, only ECDSA signatures using curve P-256 and P-384 + * are supported for multi-signature images. + */ + +/* Uncomment to enable verification of images with multiple signatures */ +/* #define MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ +#ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT +/*— How many RoTPK keys per image —*/ +#define MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE 2 +#endif + /* * Public key handling * diff --git a/sim/mcuboot-sys/build.rs b/sim/mcuboot-sys/build.rs index e74a086d00..e180d55e08 100644 --- a/sim/mcuboot-sys/build.rs +++ b/sim/mcuboot-sys/build.rs @@ -448,6 +448,8 @@ fn main() { conf.conf.define("MBEDTLS_CONFIG_FILE", Some("")); } + conf.file("../../boot/bootutil/src/bootutil_find_key.c"); + conf.file("../../boot/bootutil/src/bootutil_img_hash.c"); conf.file("../../boot/bootutil/src/image_validate.c"); if sig_rsa || sig_rsa3072 { conf.file("../../boot/bootutil/src/image_rsa.c"); From 309bf22bea995a53512d85c592192a04ed1c9c4c Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Fri, 16 May 2025 06:56:56 +0100 Subject: [PATCH 4/6] imgtool: Rename key-ids to psa-key-ids Since the key id concept in the PSA specific, rename the variables accordingly. Signed-off-by: Maulik Patel Change-Id: I8a8a5ceba5554211f185cc4045a6081b6d407507 --- scripts/imgtool/image.py | 12 ++++++------ scripts/imgtool/main.py | 8 ++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts/imgtool/image.py b/scripts/imgtool/image.py index a1b0d8e628..7d185b9fba 100644 --- a/scripts/imgtool/image.py +++ b/scripts/imgtool/image.py @@ -277,7 +277,7 @@ def __init__(self, version=None, header_size=IMAGE_HEADER_SIZE, self.image_hash = None self.image_size = None - self.signature = None + self.signatures = None self.version = version or versmod.decode_version("0") self.header_size = header_size self.pad_header = pad_header @@ -299,7 +299,7 @@ def __init__(self, version=None, header_size=IMAGE_HEADER_SIZE, self.enctlv_len = 0 self.max_align = max(DEFAULT_MAX_ALIGN, align) if max_align is None else int(max_align) self.non_bootable = non_bootable - self.key_ids = None + self.psa_key_ids = None if self.max_align == DEFAULT_MAX_ALIGN: self.boot_magic = bytes([ @@ -672,9 +672,9 @@ def create(self, keys, public_key_format, enckey, dependencies=None, self.signatures = [] for i, key in enumerate(keys): # If key IDs are provided, and we have enough for this key, add it first. - if self.key_ids is not None and len(self.key_ids) > i: + if self.psa_key_ids is not None and len(self.psa_key_ids) > i: # Convert key id (an integer) to 4-byte defined endian bytes. - kid_bytes = self.key_ids[i].to_bytes(4, self.endian) + kid_bytes = self.psa_key_ids[i].to_bytes(4, self.endian) tlv.add('KEYID', kid_bytes) # Using the TLV tag that corresponds to key IDs. if public_key_format == 'hash': @@ -961,9 +961,9 @@ def verify(imgfile, key): return VerifyResult.INVALID_SIGNATURE, None, None, None - def set_key_ids(self, key_ids): + def set_key_ids(self, psa_key_ids): """Set list of key IDs (integers) to be inserted before each signature.""" - self.key_ids = key_ids + self.psa_key_ids = psa_key_ids def _add_key_id_tlv_to_unprotected(self, tlv, key_id: int): """Add a key ID TLV into the *unprotected* TLV area.""" diff --git a/scripts/imgtool/main.py b/scripts/imgtool/main.py index 3fbb4384e4..8cc82d3973 100755 --- a/scripts/imgtool/main.py +++ b/scripts/imgtool/main.py @@ -449,7 +449,7 @@ def convert(self, value, param, ctx): help='send to OUTFILE the payload or payload''s digest instead ' 'of complied image. These data can be used for external image ' 'signing') -@click.option('--key-ids', multiple=True, type=int, required=False, +@click.option('--psa-key-ids', multiple=True, type=int, required=False, help='List of integer key IDs for each signature.') @click.command(help='''Create a signed or unsigned image\n INFILE and OUTFILE are parsed as Intel HEX if the params have @@ -460,7 +460,7 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, dependencies, load_addr, hex_addr, erased_val, save_enctlv, security_counter, boot_record, custom_tlv, rom_fixed, max_align, clear, fix_sig, fix_sig_pubkey, sig_out, user_sha, hmac_sha, is_pure, - vector_to_sign, non_bootable, key_ids): + vector_to_sign, non_bootable, psa_key_ids): if confirm: # Confirmed but non-padded images don't make much sense, because @@ -476,6 +476,10 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, non_bootable=non_bootable) compression_tlvs = {} img.load(infile) + # If the user passed any key IDs, apply them here: + if psa_key_ids: + click.echo(f"Signing with PSA key IDs: {psa_key_ids}") + img.set_key_ids(list(psa_key_ids)) if key: loaded_keys = [load_key(k) for k in key] else: From 10b2b13dd80fe0fb6bac18ad1eb09efcb22c47fe Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Fri, 27 Jun 2025 16:58:30 +0100 Subject: [PATCH 5/6] Docs: Add release notes for multi-signature support Signed-off-by: Maulik Patel Change-Id: I12c087cb399d5be0079e2b9d02b57338f492d37c --- docs/release-notes.d/multi-signature-support.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 docs/release-notes.d/multi-signature-support.md diff --git a/docs/release-notes.d/multi-signature-support.md b/docs/release-notes.d/multi-signature-support.md new file mode 100644 index 0000000000..8cfc19f15e --- /dev/null +++ b/docs/release-notes.d/multi-signature-support.md @@ -0,0 +1,2 @@ +- imgtool: added support for multi-signature image generation, enabling firmware images to be signed with multiple keys (e.g. for multi-party signing). +- bootutil: added support for verifying multi-signature TLV entries, allowing the bootloader to validate all signatures on an image during startup. From 5370225256414a4244eff507ff986fe691e0857d Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Thu, 26 Jun 2025 14:53:56 +0100 Subject: [PATCH 6/6] bootutil: imgtool: Fix CI failures For some platorms image_validate.c: In function 'bootutil_img_validate': image_validate.c:358:40: error: 'image_index' undeclared (first use in this function); did you mean 'image_header'? 358 | key_id = bootutil_find_key(image_index, buf, len); | ^~~~~~~~~~~ Resolve imgtool CI errors affecting certain signature verification tests. Change the return type of boot_verify_key_id_for_image to reflect its use as an FIH call. Add new bootutil source files to the Zephyr and espressif CMakeLists.txt files to fix undefined symbols. Update FIH tests to use the latest TFM version. This also requires updating the toolchain version to 14.2 in the Docker image. Signed-off-by: Maulik Patel Change-Id: Ie75e6c533631b2696a4a41d86b64d4009fac0c54 --- boot/bootutil/include/bootutil/sign_key.h | 5 ++- boot/bootutil/src/bootutil_find_key.c | 16 +++++++- boot/bootutil/src/bootutil_img_hash.c | 4 +- boot/bootutil/src/image_validate.c | 2 +- boot/espressif/CMakeLists.txt | 3 ++ boot/zephyr/CMakeLists.txt | 3 ++ ci/fih-tests_run.sh | 3 +- ci/fih_test_docker/execute_test.sh | 50 +++++++++++++++++++++++ scripts/imgtool/image.py | 22 +++++++--- scripts/imgtool/main.py | 11 +++-- sim/mcuboot-sys/build.rs | 1 + 11 files changed, 106 insertions(+), 14 deletions(-) diff --git a/boot/bootutil/include/bootutil/sign_key.h b/boot/bootutil/include/bootutil/sign_key.h index 22352112f8..ce777cef04 100644 --- a/boot/bootutil/include/bootutil/sign_key.h +++ b/boot/bootutil/include/bootutil/sign_key.h @@ -30,6 +30,9 @@ #ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT #include #endif /* MCUBOOT_IMAGE_MULTI_SIG_SUPPORT */ +#ifdef MCUBOOT_BUILTIN_KEY +#include "bootutil/fault_injection_hardening.h" +#endif /* MCUBOOT_BUILTIN_KEY */ #ifdef __cplusplus extern "C" { @@ -51,7 +54,7 @@ extern const struct bootutil_key bootutil_keys[]; * * @return 0 if the key ID is valid for the image; nonzero on failure. */ -int boot_verify_key_id_for_image(uint8_t image_index, uint32_t key_id); +fih_ret boot_verify_key_id_for_image(uint8_t image_index, uint32_t key_id); #endif /* MCUBOOT_BUILTIN_KEY */ #else struct bootutil_key { diff --git a/boot/bootutil/src/bootutil_find_key.c b/boot/bootutil/src/bootutil_find_key.c index 9d17b3c55e..7ada2071fe 100644 --- a/boot/bootutil/src/bootutil_find_key.c +++ b/boot/bootutil/src/bootutil_find_key.c @@ -28,14 +28,27 @@ #include -#include "bootutil/bootutil_log.h" #include "bootutil/crypto/sha.h" #include "bootutil/fault_injection_hardening.h" #include "bootutil/image.h" #include "bootutil/sign_key.h" #include "bootutil_priv.h" #include "mcuboot_config/mcuboot_config.h" +#include "bootutil/bootutil_log.h" + +BOOT_LOG_MODULE_DECLARE(mcuboot); + +#if defined(MCUBOOT_SIGN_RSA) || \ + defined(MCUBOOT_SIGN_EC256) || \ + defined(MCUBOOT_SIGN_EC384) || \ + defined(MCUBOOT_SIGN_EC) || \ + defined(MCUBOOT_SIGN_ED25519) +#define IMAGE_VALIDATION_EXPECTS_KEY +#else + /* no signing, sha256 digest only */ +#endif +#ifdef IMAGE_VALIDATION_EXPECTS_KEY #ifdef MCUBOOT_IMAGE_MULTI_SIG_SUPPORT #define NUM_OF_KEYS MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE #else @@ -135,3 +148,4 @@ int bootutil_find_key(uint8_t image_index, uint8_t *keyhash, uint8_t keyhash_len return -1; } #endif +#endif /* IMAGE_VALIDATION_EXPECTS_KEY */ \ No newline at end of file diff --git a/boot/bootutil/src/bootutil_img_hash.c b/boot/bootutil/src/bootutil_img_hash.c index cb8eca48ab..354001d915 100644 --- a/boot/bootutil/src/bootutil_img_hash.c +++ b/boot/bootutil/src/bootutil_img_hash.c @@ -29,12 +29,14 @@ #include #include -#include "bootutil/bootutil_log.h" #include "bootutil/crypto/sha.h" #include "bootutil/fault_injection_hardening.h" #include "bootutil/image.h" #include "bootutil_priv.h" #include "mcuboot_config/mcuboot_config.h" +#include "bootutil/bootutil_log.h" + +BOOT_LOG_MODULE_DECLARE(mcuboot); #ifndef MCUBOOT_SIGN_PURE /* diff --git a/boot/bootutil/src/image_validate.c b/boot/bootutil/src/image_validate.c index 27709ad130..e49177784b 100644 --- a/boot/bootutil/src/image_validate.c +++ b/boot/bootutil/src/image_validate.c @@ -205,7 +205,7 @@ bootutil_img_validate(struct boot_loader_state *state, int seed_len, uint8_t *out_hash ) { -#if (defined(EXPECTED_KEY_TLV) && defined(MCUBOOT_HW_KEY)) || defined(MCUBOOT_HW_ROLLBACK_PROT) +#if defined(EXPECTED_KEY_TLV) || defined(MCUBOOT_HW_ROLLBACK_PROT) int image_index = (state == NULL ? 0 : BOOT_CURR_IMG(state)); #endif uint32_t off; diff --git a/boot/espressif/CMakeLists.txt b/boot/espressif/CMakeLists.txt index 05358839c1..0fa2759eab 100644 --- a/boot/espressif/CMakeLists.txt +++ b/boot/espressif/CMakeLists.txt @@ -236,6 +236,9 @@ endif() set(bootutil_srcs ${BOOTUTIL_DIR}/src/boot_record.c + ${BOOTUTIL_DIR}/src/bootutil_find_key.c + ${BOOTUTIL_DIR}/src/bootutil_img_hash.c + ${BOOTUTIL_DIR}/src/bootutil_img_security_cnt.c ${BOOTUTIL_DIR}/src/bootutil_misc.c ${BOOTUTIL_DIR}/src/bootutil_public.c ${BOOTUTIL_DIR}/src/caps.c diff --git a/boot/zephyr/CMakeLists.txt b/boot/zephyr/CMakeLists.txt index c601afa67a..8f8ad8e75e 100644 --- a/boot/zephyr/CMakeLists.txt +++ b/boot/zephyr/CMakeLists.txt @@ -105,6 +105,9 @@ endif() # Generic bootutil sources and includes. zephyr_library_include_directories(${BOOT_DIR}/bootutil/include) zephyr_library_sources( + ${BOOT_DIR}/bootutil/src/bootutil_find_key.c + ${BOOT_DIR}/bootutil/src/bootutil_img_hash.c + ${BOOT_DIR}/bootutil/src/bootutil_img_security_cnt.c ${BOOT_DIR}/bootutil/src/image_validate.c ${BOOT_DIR}/bootutil/src/tlv.c ${BOOT_DIR}/bootutil/src/encrypted.c diff --git a/ci/fih-tests_run.sh b/ci/fih-tests_run.sh index 4697c8851d..b9c31046d0 100755 --- a/ci/fih-tests_run.sh +++ b/ci/fih-tests_run.sh @@ -17,13 +17,14 @@ set -e source $(dirname "$0")/fih-tests_version.sh +TFM_TAG="958b54427156e66480489e53df6de085d62aef3a" # Note that we are pulling from a github mirror of these repos, not direct upstream. If the sha # checked out below changes, the mirrors might need to be updated. pushd .. git clone https://github.com/mcu-tools/trusted-firmware-m pushd trusted-firmware-m -git checkout eb8ff0db7d657b77abcd0262d5bf7f38eb1e1cdc +git checkout $TFM_TAG source lib/ext/tf-m-tests/version.txt popd git clone https://github.com/mcu-tools/tf-m-tests.git diff --git a/ci/fih_test_docker/execute_test.sh b/ci/fih_test_docker/execute_test.sh index cc67d846ad..15793d8756 100755 --- a/ci/fih_test_docker/execute_test.sh +++ b/ci/fih_test_docker/execute_test.sh @@ -16,6 +16,52 @@ set -e +# Function to update/install native GCC inside the Docker container +update_native_gcc() { + REQUIRED_MAJOR=12 + INSTALLED_MAJOR=$(gcc -dumpversion | cut -d. -f1 || echo 0) + + if [[ "$INSTALLED_MAJOR" -lt "$REQUIRED_MAJOR" ]]; then + echo "Installing native GCC $REQUIRED_MAJOR..." + apt-get update + apt-get install -y --no-install-recommends gcc-$REQUIRED_MAJOR g++-$REQUIRED_MAJOR \ + cpp-$REQUIRED_MAJOR libgcc-$REQUIRED_MAJOR-dev libstdc++-$REQUIRED_MAJOR-dev + update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-$REQUIRED_MAJOR 60 + update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-$REQUIRED_MAJOR 60 + rm -rf /var/lib/apt/lists/* + else + echo "Native GCC is already version $INSTALLED_MAJOR; skipping installation." + fi +} + +# Function to update/install ARM Embedded GCC inside the Docker container +update_cross_gcc() { + ARM_GCC_URL="https://developer.arm.com/-/media/Files/downloads/gnu/14.2.rel1/binrel/arm-gnu-toolchain-14.2.rel1-x86_64-arm-none-eabi.tar.xz" + TOOLCHAIN_DIR="/opt/arm-gcc" + + # Install prerequisites + echo "Installing prerequisites for ARM toolchain..." + apt-get update + DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ + curl libncurses5 xz-utils file + rm -rf /var/lib/apt/lists/* + + # Download and extract + echo "Downloading and extracting ARM Embedded GCC..." + mkdir -p "$TOOLCHAIN_DIR" + curl -SLf "$ARM_GCC_URL" -o /tmp/arm-gcc.tar.xz + tar -xJf /tmp/arm-gcc.tar.xz -C "$TOOLCHAIN_DIR" --strip-components=1 + rm -f /tmp/arm-gcc.tar.xz + + # Symlink into PATH + echo "Symlinking ARM toolchain into /usr/local/bin..." + ln -sf "$TOOLCHAIN_DIR/bin/"* /usr/local/bin/ +} + +# Ensure we have the proper compiler before running tests +update_native_gcc +update_cross_gcc + source $(dirname "$0")/paths.sh SKIP_SIZE=$1 @@ -23,6 +69,10 @@ BUILD_TYPE=$2 DAMAGE_TYPE=$3 FIH_LEVEL=$4 +# Required for git am to apply patches under TF-M +git config --global user.email "docker@fih-test.com" +git config --global user.name "fih-test docker" + if test -z "$FIH_LEVEL"; then # Use the default level CMAKE_FIH_LEVEL="" diff --git a/scripts/imgtool/image.py b/scripts/imgtool/image.py index 7d185b9fba..96868c3b53 100644 --- a/scripts/imgtool/image.py +++ b/scripts/imgtool/image.py @@ -870,7 +870,12 @@ def verify(imgfile, key): # Locate the first TLV info header tlv_off = header_size + img_size tlv_info = b[tlv_off:tlv_off + TLV_INFO_SIZE] - magic, tlv_tot = struct.unpack('HH', tlv_info) + if len(tlv_info) < TLV_INFO_SIZE: + # no protected block present, jump straight to unprotected + magic = TLV_INFO_MAGIC + tlv_tot = len(b) - tlv_off + else: + magic, tlv_tot = struct.unpack('HH', tlv_info) # If it's the protected-TLV block, skip it if magic == TLV_PROT_INFO_MAGIC: @@ -893,8 +898,12 @@ def verify(imgfile, key): is_pure = False scan_off = unprot_off while scan_off < unprot_end: - tlv = b[scan_off:scan_off + TLV_SIZE] - tlv_type, _, tlv_len = struct.unpack('BBH', tlv) + # if fewer than TLV_SIZE bytes remain, break + if scan_off + TLV_SIZE > len(b): + break + tlv_hdr = b[scan_off:scan_off + TLV_SIZE] + tlv_type, _, tlv_len = struct.unpack('BBH', tlv_hdr) + if tlv_type == TLV_VALUES['SIG_PURE']: is_pure = True break @@ -910,8 +919,11 @@ def verify(imgfile, key): # Verify hash and signatures while scan_off < unprot_end: - tlv = b[scan_off:scan_off + TLV_SIZE] - tlv_type, _, tlv_len = struct.unpack('BBH', tlv) + # stop if not enough bytes for another TLV header + if scan_off + TLV_SIZE > len(b): + break + tlv_hdr = b[scan_off:scan_off + TLV_SIZE] + tlv_type, _, tlv_len = struct.unpack('BBH', tlv_hdr) if is_sha_tlv(tlv_type): if not tlv_matches_key_type(tlv_type, key[0]): return VerifyResult.KEY_MISMATCH, None, None, None diff --git a/scripts/imgtool/main.py b/scripts/imgtool/main.py index 8cc82d3973..1174f2b45e 100755 --- a/scripts/imgtool/main.py +++ b/scripts/imgtool/main.py @@ -576,9 +576,12 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, compression_tlvs["DECOMP_SHA"] = img.image_hash compression_tlvs_size = len(compression_tlvs["DECOMP_SIZE"]) compression_tlvs_size += len(compression_tlvs["DECOMP_SHA"]) - if img.get_signature(): - compression_tlvs["DECOMP_SIGNATURE"] = img.get_signature() - compression_tlvs_size += len(compression_tlvs["DECOMP_SIGNATURE"]) + sigs = img.get_signature() + if sigs: + sig = sigs[0] if isinstance(sigs, list) else sigs + compression_tlvs["DECOMP_SIGNATURE"] = sig + compression_tlvs_size += len(sig) + if (compressed_size + compression_tlvs_size) < uncompressed_size: compression_header = create_lzma2_header( dictsize = comp_default_dictsize, pb = comp_default_pb, @@ -588,7 +591,7 @@ def sign(key, public_key_format, align, version, pad_sig, header_size, keep_comp_size = False; if enckey: keep_comp_size = True - compressed_img.create(key, public_key_format, enckey, + compressed_img.create(loaded_keys, public_key_format, enckey, dependencies, boot_record, custom_tlvs, compression_tlvs, compression, int(encrypt_keylen), clear, baked_signature, pub_key, vector_to_sign, user_sha=user_sha, hmac_sha=hmac_sha, diff --git a/sim/mcuboot-sys/build.rs b/sim/mcuboot-sys/build.rs index e180d55e08..95cb3d3ca6 100644 --- a/sim/mcuboot-sys/build.rs +++ b/sim/mcuboot-sys/build.rs @@ -450,6 +450,7 @@ fn main() { conf.file("../../boot/bootutil/src/bootutil_find_key.c"); conf.file("../../boot/bootutil/src/bootutil_img_hash.c"); + conf.file("../../boot/bootutil/src/bootutil_img_security_cnt.c"); conf.file("../../boot/bootutil/src/image_validate.c"); if sig_rsa || sig_rsa3072 { conf.file("../../boot/bootutil/src/image_rsa.c");