From bbd2e9574fb07d9c202f97163bab8b922240ced0 Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Mon, 1 Jun 2026 12:24:06 -0700 Subject: [PATCH] zephyr: imgtool: sim: support multiple keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zephyr: support multiple signing keys CONFIG_BOOT_SIGNATURE_KEY_FILE accepts a semicolon-separated list of PEMs. The first entry may be a keypair or public-only PEM — only the public bytes are ever embedded in the bootloader binary; subsequent entries must be public-only and are validated at CMake configure time. This enables the prod/dev custody model: a development bootloader can boot both production-signed and development-signed images, while production bootloaders embed only the production public key. imgtool: add --name-suffix to getpub and getpubhash Emits suffixed C/Rust symbol names so more than one key of the same signature type can be embedded without collisions. imgtool: add keyinfo subcommand Reports whether a PEM is a keypair (private) or public-only. Used by the Zephyr build to enforce that verification-only entries past the first are public-only PEMs. sim: tests for multiple ed25519 keys Add the sig-second-key feature and multi_key test scenarios covering single-key and dual-key builds. Documentation updated in docs/readme-zephyr.md, docs/signed_images.md, and docs/imgtool.md. Closes #2700. Signed-off-by: JP Hutchins --- .github/workflows/sim.yaml | 1 + boot/zephyr/CMakeLists.txt | 87 +++++---- boot/zephyr/Kconfig | 37 +++- boot/zephyr/keys.c | 47 +++-- boot/zephyr/sample.yaml | 8 + docs/imgtool.md | 24 +++ docs/readme-zephyr.md | 42 ++++- docs/signed_images.md | 12 ++ root-ed25519-2-pub.pem | 3 + root-ed25519-2.pem | 3 + root-ed25519-unknown.pem | 3 + scripts/imgtool/keys/general.py | 18 +- scripts/imgtool/main.py | 67 ++++++- scripts/tests/test_commands.py | 1 + scripts/tests/test_keys.py | 235 +++++++++++++++++++++++++ sim/Cargo.toml | 1 + sim/mcuboot-sys/Cargo.toml | 6 + sim/mcuboot-sys/build.rs | 8 + sim/mcuboot-sys/csupport/keys.c | 19 +- sim/src/ed25519_pub_key_2-rs.txt | 9 + sim/src/ed25519_pub_key_unknown-rs.txt | 9 + sim/src/image.rs | 112 ++++++++++-- sim/src/lib.rs | 2 +- sim/src/tlv.rs | 41 ++++- sim/tests/core.rs | 60 +++++++ 25 files changed, 766 insertions(+), 89 deletions(-) create mode 100644 root-ed25519-2-pub.pem create mode 100644 root-ed25519-2.pem create mode 100644 root-ed25519-unknown.pem create mode 100644 sim/src/ed25519_pub_key_2-rs.txt create mode 100644 sim/src/ed25519_pub_key_unknown-rs.txt diff --git a/.github/workflows/sim.yaml b/.github/workflows/sim.yaml index db91c01fd3..fcfde2f345 100644 --- a/.github/workflows/sim.yaml +++ b/.github/workflows/sim.yaml @@ -44,6 +44,7 @@ jobs: - "sig-rsa validate-primary-slot ram-load multiimage" - "sig-rsa validate-primary-slot direct-xip multiimage" - "sig-ecdsa hw-rollback-protection multiimage" + - "sig-ed25519 sig-second-key" - "sig-ecdsa-psa,sig-ecdsa-psa sig-p384,sig-ecdsa-psa swap-move bootstrap max-align-16" - "sig-ecdsa-psa enc-ec256 max-align-16, sig-ecdsa-psa enc-ec256 swap-offset validate-primary-slot max-align-16" - "ram-load enc-aes256-kw multiimage" diff --git a/boot/zephyr/CMakeLists.txt b/boot/zephyr/CMakeLists.txt index 1315737286..76975e4903 100644 --- a/boot/zephyr/CMakeLists.txt +++ b/boot/zephyr/CMakeLists.txt @@ -316,17 +316,6 @@ endif() if(NOT CONFIG_BOOT_SIGNATURE_TYPE_NONE AND NOT CONFIG_BOOT_BUILTIN_KEY AND NOT CONFIG_BOOT_SIGNATURE_KEY_FILE STREQUAL "") - set(key_file "${CONFIG_BOOT_SIGNATURE_KEY_FILE}") - string(CONFIGURE "${key_file}" key_file) - - if(IS_ABSOLUTE ${key_file}) - set(signing_key_file ${key_file}) - elseif(EXISTS ${APPLICATION_CONFIG_DIR}/${key_file}) - set(signing_key_file ${APPLICATION_CONFIG_DIR}/${key_file}) - else() - set(signing_key_file ${MCUBOOT_DIR}/${key_file}) - endif() - message("MCUBoot bootloader key file: ${signing_key_file}") set(mcuboot_default_signature_files ${MCUBOOT_DIR}/root-ec-p256-pkcs8.pem @@ -338,26 +327,64 @@ if(NOT CONFIG_BOOT_SIGNATURE_TYPE_NONE AND NOT CONFIG_BOOT_BUILTIN_KEY AND NOT ${MCUBOOT_DIR}/root-ec-p256.pem ) - # Emit a warning if using one of the default MCUboot key files - if(${signing_key_file} IN_LIST mcuboot_default_signature_files) - message(WARNING "WARNING: Using default MCUboot signing key file, this file is for debug use only and is not secure!") - endif() + # Keys are comma-separated in CONFIG_BOOT_SIGNATURE_KEY_FILE (semicolons do + # not survive sysbuild); convert to a CMake list (semicolon-separated) here. + string(REPLACE "," ";" mcuboot_key_files "${CONFIG_BOOT_SIGNATURE_KEY_FILE}") + list(LENGTH mcuboot_key_files mcuboot_sign_key_count) - set(generated_pubkey ${ZEPHYR_BINARY_DIR}/autogen-pubkey.c) - add_custom_command( - OUTPUT ${generated_pubkey} - COMMAND - ${PYTHON_EXECUTABLE} - ${MCUBOOT_DIR}/scripts/imgtool.py - getpub - -k - ${signing_key_file} - > ${generated_pubkey} - DEPENDS ${signing_key_file} - ) - target_sources(app PRIVATE - ${generated_pubkey} - ) + set(key_index 0) + foreach(raw_key_path IN LISTS mcuboot_key_files) + string(CONFIGURE "${raw_key_path}" key_path) + + if(IS_ABSOLUTE ${key_path}) + set(resolved_key_path ${key_path}) + elseif(EXISTS ${APPLICATION_CONFIG_DIR}/${key_path}) + set(resolved_key_path ${APPLICATION_CONFIG_DIR}/${key_path}) + else() + set(resolved_key_path ${MCUBOOT_DIR}/${key_path}) + endif() + + if(key_index EQUAL 0) + set(signing_key_file ${resolved_key_path}) + set(name_suffix_arg "") + set(generated_pubkey ${ZEPHYR_BINARY_DIR}/autogen-pubkey.c) + set(keyinfo_check_command "") + message("MCUBoot bootloader key file: ${resolved_key_path}") + if(${resolved_key_path} IN_LIST mcuboot_default_signature_files) + message(WARNING "WARNING: Using default MCUboot signing key file, this file is for debug use only and is not secure!") + endif() + else() + set(name_suffix_arg "--name-suffix" "_${key_index}") + set(generated_pubkey ${ZEPHYR_BINARY_DIR}/autogen-pubkey-${key_index}.c) + set(keyinfo_check_command + COMMAND ${PYTHON_EXECUTABLE} ${MCUBOOT_DIR}/scripts/imgtool.py + keyinfo --key ${resolved_key_path} --require public + ) + message("MCUBoot bootloader verification key file #${key_index}: ${resolved_key_path}") + if(${resolved_key_path} IN_LIST mcuboot_default_signature_files) + message(WARNING "WARNING: Using default MCUboot signing key file for verification key #${key_index}, this file is for debug use only and is not secure!") + endif() + endif() + + add_custom_command( + OUTPUT ${generated_pubkey} + ${keyinfo_check_command} + COMMAND + ${PYTHON_EXECUTABLE} + ${MCUBOOT_DIR}/scripts/imgtool.py + getpub + -k + ${resolved_key_path} + ${name_suffix_arg} + > ${generated_pubkey} + DEPENDS ${resolved_key_path} + ) + target_sources(app PRIVATE ${generated_pubkey}) + math(EXPR key_index "${key_index} + 1") + endforeach() + target_compile_definitions(app PRIVATE MCUBOOT_SIGN_KEY_COUNT=${mcuboot_sign_key_count}) +elseif(CONFIG_BOOT_BUILTIN_KEY) + target_compile_definitions(app PRIVATE MCUBOOT_SIGN_KEY_COUNT=1) endif() if(CONFIG_BOOT_ENCRYPTION_KEY_FILE AND NOT CONFIG_BOOT_ENCRYPTION_KEY_FILE STREQUAL "") diff --git a/boot/zephyr/Kconfig b/boot/zephyr/Kconfig index 3374df8da3..27c799ba56 100644 --- a/boot/zephyr/Kconfig +++ b/boot/zephyr/Kconfig @@ -463,7 +463,7 @@ config BOOT_BYPASS_KEY_MATCH MCUboot code and boot time. config BOOT_SIGNATURE_KEY_FILE - string "PEM key file" + string "PEM key file (or comma-separated list)" depends on !BOOT_SIGNATURE_TYPE_NONE default "root-ec-p256.pem" if BOOT_SIGNATURE_TYPE_ECDSA_P256 default "root-ed25519.pem" if BOOT_SIGNATURE_TYPE_ED25519 @@ -472,13 +472,34 @@ config BOOT_SIGNATURE_KEY_FILE default "" depends on !BOOT_BUILTIN_KEY help - You can use either absolute or relative path. - In case relative path is used, the build system assumes that it starts - from the APPLICATION_CONFIG_DIR directory. If the key file is not there, the build - system uses relative path that starts from the MCUBoot repository root directory. - The key file will be parsed by imgtool's getpub command and a .c source - with the public key information will be written in a format expected by - MCUboot. + Path to a signing/verification key PEM, or a comma-separated + list of PEMs (e.g. "prod_pub.pem,dev_pub.pem") to embed multiple + verification keys in the bootloader. Only the public-key bytes are + ever embedded in the bootloader image regardless of which form is + passed in. + + The first entry may be either a keypair PEM or a public-only PEM. + A keypair is required only if the same file is also used with + `imgtool sign`; a public-only PEM is sufficient (and preferred) + when image signing is performed elsewhere with the private half + held under separate custody. When a private key is used, only + the public half is embedded in the bootloader. + + Subsequent entries (positions past the first) must be public-only + PEMs; the build will fail at CMake time otherwise. This guards the + intended workflow: a development bootloader that accepts both + production-signed images (verified against the prod public key, + whose private half stays under release-team custody) and + development-signed images (verified against the dev public key). + All entries must use the same BOOT_SIGNATURE_TYPE. Multi-key mode + is mutually exclusive with BOOT_HW_KEY, BOOT_BUILTIN_KEY, and + BOOT_BYPASS_KEY_MATCH. + + Each entry can be an absolute or relative path. Relative paths are + resolved first against APPLICATION_CONFIG_DIR, then against the + MCUboot repository root. Each file is parsed by imgtool's getpub + command and a .c source with the public key information is written + in a format expected by MCUboot. Note: In configuration files, escaped CMake variables can be used to refer to paths e.g. \${CMAKE_CURRENT_LIST_DIR} will allow referencing a file in that directory, these diff --git a/boot/zephyr/keys.c b/boot/zephyr/keys.c index ab403ddc38..3f84da1c37 100644 --- a/boot/zephyr/keys.c +++ b/boot/zephyr/keys.c @@ -27,20 +27,40 @@ * provides via the compiler command line). */ #include +#include #if !defined(MCUBOOT_HW_KEY) #if defined(MCUBOOT_SIGN_RSA) || defined(MCUBOOT_SIGN_EC256) || defined(MCUBOOT_SIGN_ED25519) #define HAVE_KEYS + +#ifndef MCUBOOT_SIGN_KEY_COUNT +#error "MCUBOOT_SIGN_KEY_COUNT must be defined by the build system" +#endif + +#define _BOOT_KEY_CAT(a, b) a##b +#define BOOT_KEY_CAT(a, b) _BOOT_KEY_CAT(a, b) + #if defined(MCUBOOT_SIGN_RSA) -extern const unsigned char rsa_pub_key[]; -extern unsigned int rsa_pub_key_len; +# define BOOT_KEY_PRIMARY rsa_pub_key #elif defined(MCUBOOT_SIGN_EC256) -extern const unsigned char ecdsa_pub_key[]; -extern unsigned int ecdsa_pub_key_len; +# define BOOT_KEY_PRIMARY ecdsa_pub_key #elif defined(MCUBOOT_SIGN_ED25519) -extern const unsigned char ed25519_pub_key[]; -extern unsigned int ed25519_pub_key_len; +# define BOOT_KEY_PRIMARY ed25519_pub_key #endif + +#define BOOT_KEY_NAME(N) BOOT_KEY_CAT(BOOT_KEY_PRIMARY, BOOT_KEY_CAT(_, N)) + +#define BOOT_KEY_DECL_AT(i, _) \ + extern const unsigned char BOOT_KEY_NAME(UTIL_INC(i))[]; \ + extern unsigned int BOOT_KEY_CAT(BOOT_KEY_NAME(UTIL_INC(i)), _len); + +#define BOOT_KEY_ENTRY_AT(i, _) \ + { .key = BOOT_KEY_NAME(UTIL_INC(i)), \ + .len = &BOOT_KEY_CAT(BOOT_KEY_NAME(UTIL_INC(i)), _len) }, + +extern const unsigned char BOOT_KEY_PRIMARY[]; +extern unsigned int BOOT_KEY_CAT(BOOT_KEY_PRIMARY, _len); +LISTIFY(UTIL_DEC(MCUBOOT_SIGN_KEY_COUNT), BOOT_KEY_DECL_AT, ()) #endif /* @@ -51,19 +71,12 @@ extern unsigned int ed25519_pub_key_len; #if defined(HAVE_KEYS) const struct bootutil_key bootutil_keys[] = { { -#if defined(MCUBOOT_SIGN_RSA) - .key = rsa_pub_key, - .len = &rsa_pub_key_len, -#elif defined(MCUBOOT_SIGN_EC256) - .key = ecdsa_pub_key, - .len = &ecdsa_pub_key_len, -#elif defined(MCUBOOT_SIGN_ED25519) - .key = ed25519_pub_key, - .len = &ed25519_pub_key_len, -#endif + .key = BOOT_KEY_PRIMARY, + .len = &BOOT_KEY_CAT(BOOT_KEY_PRIMARY, _len), }, + LISTIFY(UTIL_DEC(MCUBOOT_SIGN_KEY_COUNT), BOOT_KEY_ENTRY_AT, ()) }; -const int bootutil_key_cnt = 1; +const int bootutil_key_cnt = sizeof(bootutil_keys) / sizeof(bootutil_keys[0]); #endif /* HAVE_KEYS */ #else unsigned int pub_key_len; diff --git a/boot/zephyr/sample.yaml b/boot/zephyr/sample.yaml index 5e50a44d8f..c2ae5a1b4f 100644 --- a/boot/zephyr/sample.yaml +++ b/boot/zephyr/sample.yaml @@ -110,6 +110,14 @@ tests: integration_platforms: - nrf52840dk/nrf52840 tags: bootloader_mcuboot + sample.bootloader.mcuboot.two_signing_keys: + extra_configs: + - CONFIG_BOOT_SIGNATURE_TYPE_ED25519=y + - CONFIG_BOOT_SIGNATURE_KEY_FILE="root-ed25519.pem,root-ed25519-2-pub.pem" + platform_allow: nrf52840dk/nrf52840 + integration_platforms: + - nrf52840dk/nrf52840 + tags: bootloader_mcuboot sample.bootloader.mcuboot.runtime_source.hooks: extra_args: EXTRA_CONF_FILE=../../samples/runtime-source/zephyr/sample.conf TEST_RUNTIME_SOURCE_HOOKS=y diff --git a/docs/imgtool.md b/docs/imgtool.md index 068996b907..dfc59e2e16 100644 --- a/docs/imgtool.md +++ b/docs/imgtool.md @@ -51,6 +51,30 @@ exported public-key PEM. You can replace or insert the emitted code into the key file. However, when the `MCUBOOT_HW_KEY` config option is enabled, this last step is unnecessary and can be skipped. +When embedding more than one signing-verification key in the same image +(for example, a Zephyr build with a multi-key +`CONFIG_BOOT_SIGNATURE_KEY_FILE` list), pass `--name-suffix` to +distinguish the emitted symbol names: + + ./scripts/imgtool.py getpub -k dev-key.pem --name-suffix _2 + +emits `_pub_key_2[]` and `_pub_key_2_len` (the +same suffix is applied by `getpubhash` for the lang-c encoding). The +option is accepted only for the `lang-c` / `lang-rust` encodings; using +it with `--encoding pem` or `--encoding raw` is rejected. + +## [Inspecting key kind](#inspecting-key-kind) + +For build-system use, `imgtool keyinfo` reports whether a PEM contains +private material (`private`) or only public material (`public`): + + ./scripts/imgtool.py keyinfo -k some-key.pem + +Pair with `--require private` or `--require public` to exit non-zero +when the kind does not match. The Zephyr port uses this to enforce that +every verification-only key passed via `CONFIG_BOOT_SIGNATURE_KEY_FILE` +past the first entry is a public-only PEM. + ## [Signing images](#signing-images) Image signing takes an image in binary or Intel Hex format intended for the diff --git a/docs/readme-zephyr.md b/docs/readme-zephyr.md index 4a6d01b9cf..66328c5c5e 100644 --- a/docs/readme-zephyr.md +++ b/docs/readme-zephyr.md @@ -163,8 +163,46 @@ by a release team and only an exported public key is provided to bootloader builders. Signing images (`imgtool sign`) requires the private key. -Currently, the Zephyr RTOS port limits its support to one keypair at the time, -although MCUboot's key management infrastructure supports multiple keypairs. +The Zephyr port supports embedding multiple verification keys in the +bootloader. `CONFIG_BOOT_SIGNATURE_KEY_FILE` accepts a single PEM path or +a comma-separated list, e.g. +`"prod_pubkey.pem,dev_pubkey.pem"` or +`"\${CMAKE_CURRENT_LIST_DIR}/prod_pubkey.pem,\${CMAKE_CURRENT_LIST_DIR}/dev_pubkey.pem"`. +Only the public-key bytes are ever embedded in the bootloader image, +regardless of which form is passed in. Up to eight total keys are +supported by the Zephyr port. All entries must use the same +`BOOT_SIGNATURE_TYPE`, and multi-key mode is mutually exclusive with +`BOOT_HW_KEY`, `BOOT_BUILTIN_KEY`, and `BOOT_BYPASS_KEY_MATCH`. + +The first entry **may** be a keypair PEM or a public-only PEM. A +keypair is required only if the same file is also used with +`imgtool sign`; otherwise a public-only PEM is sufficient — and +preferred, since private material embedded in the bootloader image is +recoverable from flash. Subsequent entries (positions past the first) +**must** be public-only PEMs; the build is rejected at CMake time +otherwise (via `imgtool keyinfo --require public`). + +### Custody model + +The feature is for separating signing custody from verification custody. + +| Key | Custody | Distribution | Blast radius if lost | +| ------------------------- | ------------------------------------------ | ----------------------------------------------------------------- | --------------------------------------------------------------------- | +| Production private | HSM, signing ceremony, release team only | Never leaves the HSM | Catastrophic: attacker can sign images that boot on the prod fleet | +| Production public | N/A (public material) | Embedded in dev bootloaders so dev units can verify prod images | None: public by design | +| Development private | Loosely held by engineers | On dev workstations / dev signing infra | Bounded: only authorizes firmware on non-deployed dev hardware | + +Production bootloaders should embed only the production public key, so +that production units boot only production-signed images. Development +bootloaders embed both the production public key and the development +public key, so a dev unit can boot a production-signed image (verified +against the prod public key) without re-signing, while still allowing +engineers to flash development-signed images. + +The bootloader's existing key-matching logic (`bootutil_find_key()`) +hashes the image's KEYHASH TLV against every embedded key and accepts +the first match. There is no per-key behaviour: multi-key mode is purely +about accepting more than one valid signer. Once MCUboot is built, this new keypair file (`mykey.pem` in this example) can be used to sign images. diff --git a/docs/signed_images.md b/docs/signed_images.md index 41a4d119f4..c9394c0dd3 100644 --- a/docs/signed_images.md +++ b/docs/signed_images.md @@ -33,6 +33,18 @@ be useful when you want to prevent production units from booting development images, but want development units to be able to boot both production images and development images. +On Zephyr, `CONFIG_BOOT_SIGNATURE_KEY_FILE` accepts a comma-separated +list of PEMs. Only the public-key bytes are embedded regardless of which +form is passed in. The first entry may be a keypair PEM (needed only if +the same file is also fed to `imgtool sign`) or a public-only PEM +(preferred when signing happens elsewhere); subsequent entries must be +public-only. The intended use is to separate signing custody (a +production private key, held only by a release team) from verification +custody (the production public key, embedded in development bootloaders +so dev units can boot prod-signed images). See +[readme-zephyr.md](readme-zephyr.md) for the threat-model table and a +worked example. + For an alternative solution when the public key(s) doesn't need to be included in the bootloader, see the [design](design.md) document. diff --git a/root-ed25519-2-pub.pem b/root-ed25519-2-pub.pem new file mode 100644 index 0000000000..683505fe90 --- /dev/null +++ b/root-ed25519-2-pub.pem @@ -0,0 +1,3 @@ +-----BEGIN PUBLIC KEY----- +MCowBQYDK2VwAyEAw7zfMHZOH120DYkuDQ6rBJwwBk55qO2293kuRpom2nc= +-----END PUBLIC KEY----- diff --git a/root-ed25519-2.pem b/root-ed25519-2.pem new file mode 100644 index 0000000000..79976b9d43 --- /dev/null +++ b/root-ed25519-2.pem @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEICZVk44tV7KC3eJ+Qokha0aULNUVqDp9iR0cKjpqcO4D +-----END PRIVATE KEY----- diff --git a/root-ed25519-unknown.pem b/root-ed25519-unknown.pem new file mode 100644 index 0000000000..7a4891887b --- /dev/null +++ b/root-ed25519-unknown.pem @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIGHiQXOA1EyKdOzovW9M2d5tP/fDC0i1ByV80WJGMrqN +-----END PRIVATE KEY----- diff --git a/scripts/imgtool/keys/general.py b/scripts/imgtool/keys/general.py index a87ae302b9..cc70e113c5 100644 --- a/scripts/imgtool/keys/general.py +++ b/scripts/imgtool/keys/general.py @@ -86,28 +86,28 @@ def _emit_raw(self, encoded_bytes, file): # raw binary data, can be for example io.BytesIO file.write(encoded_bytes) - def emit_c_public(self, file=sys.stdout): + def emit_c_public(self, file=sys.stdout, name_suffix: str = ""): self._emit( - header=f"const unsigned char {self.shortname()}_pub_key[] = {{" + header=f"const unsigned char {self.shortname()}_pub_key{name_suffix}[] = {{" , trailer="};", encoded_bytes=self.get_public_bytes(), indent=" ", - len_format=f"const unsigned int {self.shortname()}_pub_key_len = {{}};" + len_format=f"const unsigned int {self.shortname()}_pub_key{name_suffix}_len = {{}};" , file=file) - def emit_c_public_hash(self, file=sys.stdout): + def emit_c_public_hash(self, file=sys.stdout, name_suffix: str = ""): digest = Hash(SHA256()) digest.update(self.get_public_bytes()) self._emit( - header=f"const unsigned char {self.shortname()}_pub_key_hash[] = {{" + header=f"const unsigned char {self.shortname()}_pub_key_hash{name_suffix}[] = {{" , trailer="};", encoded_bytes=digest.finalize(), indent=" ", - len_format=f"const unsigned int {self.shortname()}_pub_key_hash_len = {{}};" - , + len_format=("const unsigned int " + f"{self.shortname()}_pub_key_hash{name_suffix}_len = {{}};"), file=file) def emit_raw_public(self, file=sys.stdout): @@ -118,9 +118,9 @@ def emit_raw_public_hash(self, file=sys.stdout): digest.update(self.get_public_bytes()) self._emit_raw(digest.finalize(), file=file) - def emit_rust_public(self, file=sys.stdout): + def emit_rust_public(self, file=sys.stdout, name_suffix: str = ""): self._emit( - header=f"static {self.shortname().upper()}_PUB_KEY: &[u8] = &[" + header=f"static {self.shortname().upper()}_PUB_KEY{name_suffix.upper()}: &[u8] = &[" , trailer="];", encoded_bytes=self.get_public_bytes(), diff --git a/scripts/imgtool/main.py b/scripts/imgtool/main.py index fc569536b3..a9d9acbc43 100755 --- a/scripts/imgtool/main.py +++ b/scripts/imgtool/main.py @@ -74,6 +74,17 @@ def gen_x25519(keyfile, passwd): valid_langs = ['c', 'rust'] valid_hash_encodings = ['lang-c', 'raw'] valid_encodings = ['lang-c', 'lang-rust', 'pem', 'raw'] + + +def _validate_name_suffix(ctx: click.Context, param: click.Parameter, value: str) -> str: + if value and not re.match(r"^[A-Za-z0-9_]*$", value): + raise click.BadParameter( + f"{value!r} must contain only [A-Za-z0-9_]; it is appended " + f"directly to a C/Rust identifier." + ) + return value + + keygens = { 'rsa-2048': gen_rsa2048, 'rsa-3072': gen_rsa3072, @@ -141,18 +152,28 @@ def keygen(type, key, password): @click.option('-e', '--encoding', metavar='encoding', type=click.Choice(valid_encodings), help='Valid encodings: {}'.format(', '.join(valid_encodings))) +@click.option('--name-suffix', 'name_suffix', metavar='SUFFIX', default='', + callback=_validate_name_suffix, + help='Append SUFFIX to the emitted C/Rust symbol names ' + '(e.g. `--name-suffix _2` emits `rsa_pub_key_2` / ' + '`rsa_pub_key_2_len`). Useful when embedding multiple ' + 'signing keys in the same image. Ignored for PEM/raw ' + 'encodings (those emit no identifiers).') @click.option('-k', '--key', metavar='filename', required=True) @click.option('-o', '--output', metavar='output', required=False, help='Specify the output file\'s name. \ The stdout is used if it is not provided.') @click.command(help='Dump public key from keypair') -def getpub(key, encoding, lang, output): +def getpub(key, encoding, lang, output, name_suffix): if encoding and lang: raise click.UsageError('Please use only one of `--encoding/-e` or `--lang/-l`') elif not encoding and not lang: # Preserve old behavior defaulting to `c`. If `lang` is removed, # `default=valid_encodings[0]` should be added to `-e` param. lang = valid_langs[0] + if name_suffix and (encoding in ('pem', 'raw')): + raise click.UsageError( + '`--name-suffix` is only meaningful for lang-c / lang-rust encodings') key = load_key(key) if not output: @@ -160,9 +181,9 @@ def getpub(key, encoding, lang, output): if key is None: print("Invalid passphrase") elif lang == 'c' or encoding == 'lang-c': - key.emit_c_public(file=output) + key.emit_c_public(file=output, name_suffix=name_suffix) elif lang == 'rust' or encoding == 'lang-rust': - key.emit_rust_public(file=output) + key.emit_rust_public(file=output, name_suffix=name_suffix) elif encoding == 'pem': key.emit_public_pem(file=output) elif encoding == 'raw': @@ -177,14 +198,21 @@ def getpub(key, encoding, lang, output): 'Default value is {}.' .format(', '.join(valid_hash_encodings), valid_hash_encodings[0])) +@click.option('--name-suffix', 'name_suffix', metavar='SUFFIX', default='', + callback=_validate_name_suffix, + help='Append SUFFIX to the emitted C symbol names (lang-c ' + 'encoding only). Ignored for raw encoding.') @click.option('-k', '--key', metavar='filename', required=True) @click.option('-o', '--output', metavar='output', required=False, help='Specify the output file\'s name. \ The stdout is used if it is not provided.') @click.command(help='Dump the SHA256 hash of the public key') -def getpubhash(key, output, encoding): +def getpubhash(key, output, encoding, name_suffix): if not encoding: encoding = valid_hash_encodings[0] + if name_suffix and encoding == 'raw': + raise click.UsageError( + '`--name-suffix` is only meaningful for the lang-c encoding') key = load_key(key) if not output: @@ -192,13 +220,41 @@ def getpubhash(key, output, encoding): if key is None: print("Invalid passphrase") elif encoding == 'lang-c': - key.emit_c_public_hash(file=output) + key.emit_c_public_hash(file=output, name_suffix=name_suffix) elif encoding == 'raw': key.emit_raw_public_hash(file=output) else: raise click.UsageError() +@click.option('--require', 'require', type=click.Choice(['private', 'public']), + default=None, + help='Exit non-zero if the key kind does not match REQUIRE. ' + 'Without this option, keyinfo always exits 0 and prints ' + 'the detected kind on stdout.') +@click.option('-k', '--key', metavar='filename', required=True) +@click.command(help='Print whether KEY is a keypair PEM (`private`) or a ' + 'public-only PEM (`public`). Intended for build-system ' + 'use: pair with `--require` to gate the build on the ' + 'expected key kind.') +def keyinfo(key, require): + loaded = keys.load(key) + if loaded is None: + raise click.UsageError( + f"Cannot inspect {key}: key is password-protected or unreadable. " + f"keyinfo runs non-interactively and does not prompt for a " + f"passphrase." + ) + kind = ('private' + if isinstance(loaded, (keys.PayloadSigner, keys.DigestSigner)) + else 'public') + click.echo(kind) + if require is not None and kind != require: + raise click.UsageError( + f"Key {key} is {kind}, but {require} was required." + ) + + @click.option('--minimal', default=False, is_flag=True, help='Reduce the size of the dumped private key to include only ' 'the minimum amount of data required to decrypt. This ' @@ -642,6 +698,7 @@ def imgtool(): imgtool.add_command(getpub) imgtool.add_command(getpubhash) imgtool.add_command(getpriv) +imgtool.add_command(keyinfo) imgtool.add_command(verify) imgtool.add_command(sign) imgtool.add_command(version) diff --git a/scripts/tests/test_commands.py b/scripts/tests/test_commands.py index 4ef794e256..d3a1609db4 100644 --- a/scripts/tests/test_commands.py +++ b/scripts/tests/test_commands.py @@ -28,6 +28,7 @@ "getpub", "getpubhash", "keygen", + "keyinfo", "sign", "verify", "version", diff --git a/scripts/tests/test_keys.py b/scripts/tests/test_keys.py index b0b42828c5..a0a0dc3ef9 100644 --- a/scripts/tests/test_keys.py +++ b/scripts/tests/test_keys.py @@ -285,6 +285,241 @@ def test_getpubhash_from_pub_only_pem( assert from_priv.read_bytes() == from_pub.read_bytes() +KEY_SHORTNAMES = { + "rsa-2048": "rsa", + "rsa-3072": "rsa", + "ecdsa-p256": "ecdsa", + "ecdsa-p384": "ecdsap384", + "ed25519": "ed25519", + "x25519": "x25519", +} +"""Map from keygen key_type names to the shortname() each key class emits, +which is the prefix used in the autogenerated C/Rust symbol names.""" + + +@pytest.mark.parametrize("key_type", KEY_TYPES) +def test_getpub_name_suffix_c(key_type, tmp_path_persistent): + """`--name-suffix` appends to lang-c symbol names.""" + runner = CliRunner() + + gen_key = tmp_name(tmp_path_persistent, key_type, GEN_KEY_EXT) + pub_key = tmp_name(tmp_path_persistent, key_type, + PUB_KEY_EXT + ".suffix.c") + suffix = "_dev" + + result = runner.invoke( + imgtool, + [ + "getpub", "--key", str(gen_key), + "--output", str(pub_key), + "--encoding", "lang-c", + "--name-suffix", suffix, + ], + ) + assert result.exit_code == 0 + content = pub_key.read_text() + short = KEY_SHORTNAMES[key_type] + assert f"{short}_pub_key{suffix}[]" in content + assert f"{short}_pub_key{suffix}_len" in content + # The un-suffixed names must not appear — otherwise linking two keys of + # the same type in the same image would fail. + assert f"{short}_pub_key[]" not in content + assert f"{short}_pub_key_len" not in content + + +@pytest.mark.parametrize("key_type", KEY_TYPES) +def test_getpub_name_suffix_rust(key_type, tmp_path_persistent): + """`--name-suffix` appends to lang-rust symbol names (uppercased).""" + runner = CliRunner() + + gen_key = tmp_name(tmp_path_persistent, key_type, GEN_KEY_EXT) + pub_key = tmp_name(tmp_path_persistent, key_type, + PUB_KEY_EXT + ".suffix.rs") + suffix = "_dev" + + result = runner.invoke( + imgtool, + [ + "getpub", "--key", str(gen_key), + "--output", str(pub_key), + "--encoding", "lang-rust", + "--name-suffix", suffix, + ], + ) + assert result.exit_code == 0 + content = pub_key.read_text() + short = KEY_SHORTNAMES[key_type].upper() + assert f"{short}_PUB_KEY{suffix.upper()}" in content + + +@pytest.mark.parametrize("encoding", ["pem", "raw"]) +def test_getpub_name_suffix_rejected(encoding, tmp_path_persistent): + """`--name-suffix` must be rejected for encodings without symbol names.""" + runner = CliRunner() + + gen_key = tmp_name(tmp_path_persistent, "ed25519", GEN_KEY_EXT) + out = tmp_name(tmp_path_persistent, "ed25519", + PUB_KEY_EXT + ".reject." + encoding) + + result = runner.invoke( + imgtool, + [ + "getpub", "--key", str(gen_key), + "--output", str(out), + "--encoding", encoding, + "--name-suffix", "_2", + ], + ) + assert result.exit_code != 0 + assert "name-suffix" in result.output + + +@pytest.mark.parametrize( + "bad_suffix", + ["-dev", "foo/bar", "a b", "@2", "key+", " ", "_2-x"], +) +def test_getpub_name_suffix_invalid_chars_rejected( + bad_suffix: str, tmp_path_persistent: Path, +) -> None: + """Suffixes that would produce invalid C/Rust identifiers are rejected.""" + runner = CliRunner() + gen_key = tmp_name(tmp_path_persistent, "ed25519", GEN_KEY_EXT) + + result = runner.invoke( + imgtool, + [ + "getpub", "--key", str(gen_key), + "--encoding", "lang-c", + "--name-suffix", bad_suffix, + ], + ) + assert result.exit_code != 0 + assert "A-Za-z0-9_" in result.output + + +@pytest.mark.parametrize( + "bad_suffix", + ["-dev", "foo/bar", "a b", "@2", "key+", " "], +) +def test_getpubhash_name_suffix_invalid_chars_rejected( + bad_suffix: str, tmp_path_persistent: Path, +) -> None: + """getpubhash rejects suffixes with non-identifier chars too.""" + runner = CliRunner() + gen_key = tmp_name(tmp_path_persistent, "ed25519", GEN_KEY_EXT) + + result = runner.invoke( + imgtool, + [ + "getpubhash", "--key", str(gen_key), + "--encoding", "lang-c", + "--name-suffix", bad_suffix, + ], + ) + assert result.exit_code != 0 + assert "A-Za-z0-9_" in result.output + + +@pytest.mark.parametrize("key_type", KEY_TYPES) +def test_getpubhash_name_suffix_c(key_type, tmp_path_persistent): + """`--name-suffix` appends to getpubhash lang-c symbol names.""" + runner = CliRunner() + + gen_key = tmp_name(tmp_path_persistent, key_type, GEN_KEY_EXT) + pub_hash = tmp_name(tmp_path_persistent, key_type, + PUB_KEY_HASH_EXT + ".suffix.c") + suffix = "_dev" + + result = runner.invoke( + imgtool, + [ + "getpubhash", "--key", str(gen_key), + "--output", str(pub_hash), + "--encoding", "lang-c", + "--name-suffix", suffix, + ], + ) + assert result.exit_code == 0 + content = pub_hash.read_text() + short = KEY_SHORTNAMES[key_type] + assert f"{short}_pub_key_hash{suffix}[]" in content + assert f"{short}_pub_key_hash{suffix}_len" in content + + +def test_getpubhash_name_suffix_rejects_raw(tmp_path_persistent): + """`--name-suffix` must be rejected for getpubhash raw encoding.""" + runner = CliRunner() + + gen_key = tmp_name(tmp_path_persistent, "ed25519", GEN_KEY_EXT) + out = tmp_name(tmp_path_persistent, "ed25519", + PUB_KEY_HASH_EXT + ".reject.raw") + + result = runner.invoke( + imgtool, + [ + "getpubhash", "--key", str(gen_key), + "--output", str(out), + "--encoding", "raw", + "--name-suffix", "_2", + ], + ) + assert result.exit_code != 0 + assert "name-suffix" in result.output + + +@pytest.mark.parametrize("key_type", KEY_TYPES) +def test_keyinfo_reports_private(key_type, tmp_path_persistent): + """keyinfo prints `private` for a keypair PEM.""" + runner = CliRunner() + gen_key = tmp_name(tmp_path_persistent, key_type, GEN_KEY_EXT) + + result = runner.invoke(imgtool, ["keyinfo", "--key", str(gen_key)]) + assert result.exit_code == 0 + assert result.output.strip() == "private" + + +@pytest.mark.parametrize("key_type", KEY_TYPES) +def test_keyinfo_reports_public(key_type, tmp_path_persistent): + """keyinfo prints `public` for a public-only PEM.""" + runner = CliRunner() + gen_key = tmp_name(tmp_path_persistent, key_type, GEN_KEY_EXT) + pub_only_pem = tmp_name(tmp_path_persistent, key_type, PUB_ONLY_PEM_EXT) + _ensure_pub_only_pem(runner, gen_key, pub_only_pem) + + result = runner.invoke(imgtool, ["keyinfo", "--key", str(pub_only_pem)]) + assert result.exit_code == 0 + assert result.output.strip() == "public" + + +def test_keyinfo_require_public_rejects_private(tmp_path_persistent): + """keyinfo --require=public fails on a keypair PEM.""" + runner = CliRunner() + gen_key = tmp_name(tmp_path_persistent, "ed25519", GEN_KEY_EXT) + + result = runner.invoke( + imgtool, + ["keyinfo", "--key", str(gen_key), "--require", "public"], + ) + assert result.exit_code != 0 + assert "private" in result.output + assert "public was required" in result.output + + +def test_keyinfo_require_private_rejects_public(tmp_path_persistent): + """keyinfo --require=private fails on a public-only PEM.""" + runner = CliRunner() + gen_key = tmp_name(tmp_path_persistent, "ed25519", GEN_KEY_EXT) + pub_only_pem = tmp_name(tmp_path_persistent, "ed25519", PUB_ONLY_PEM_EXT) + _ensure_pub_only_pem(runner, gen_key, pub_only_pem) + + result = runner.invoke( + imgtool, + ["keyinfo", "--key", str(pub_only_pem), "--require", "private"], + ) + assert result.exit_code != 0 + assert "public" in result.output + assert "private was required" in result.output + @pytest.mark.parametrize("key_type", KEY_TYPES) def test_sign_verify(key_type, tmp_path_persistent): diff --git a/sim/Cargo.toml b/sim/Cargo.toml index e582ac2ffc..95bc1895b9 100644 --- a/sim/Cargo.toml +++ b/sim/Cargo.toml @@ -14,6 +14,7 @@ sig-ecdsa-mbedtls = ["mcuboot-sys/sig-ecdsa-mbedtls"] sig-ecdsa-psa = ["mcuboot-sys/sig-ecdsa-psa", "mcuboot-sys/psa-crypto-api"] sig-p384 = ["mcuboot-sys/sig-p384"] sig-ed25519 = ["mcuboot-sys/sig-ed25519"] +sig-second-key = ["mcuboot-sys/sig-second-key"] overwrite-only = ["mcuboot-sys/overwrite-only"] swap-offset = ["mcuboot-sys/swap-offset"] swap-move = ["mcuboot-sys/swap-move"] diff --git a/sim/mcuboot-sys/Cargo.toml b/sim/mcuboot-sys/Cargo.toml index afca7dcd35..d76f59614c 100644 --- a/sim/mcuboot-sys/Cargo.toml +++ b/sim/mcuboot-sys/Cargo.toml @@ -33,6 +33,12 @@ sig-p384 = [] # Verify ED25519 signatures. sig-ed25519 = [] +# Embed a second signing-verification key into the bootloader. When this +# feature is enabled the simulator's bootutil_keys[] exposes two keys and +# images signed with either are accepted. Currently only meaningful with +# `sig-ed25519` — support for other signature types can be added as needed. +sig-second-key = [] + # Overwrite only upgrade overwrite-only = [] diff --git a/sim/mcuboot-sys/build.rs b/sim/mcuboot-sys/build.rs index 8a7ae06d1f..0f02cfde4f 100644 --- a/sim/mcuboot-sys/build.rs +++ b/sim/mcuboot-sys/build.rs @@ -18,6 +18,7 @@ fn main() { let sig_ecdsa_psa = env::var("CARGO_FEATURE_SIG_ECDSA_PSA").is_ok(); let sig_p384 = env::var("CARGO_FEATURE_SIG_P384").is_ok(); let sig_ed25519 = env::var("CARGO_FEATURE_SIG_ED25519").is_ok(); + let sig_second_key = env::var("CARGO_FEATURE_SIG_SECOND_KEY").is_ok(); let overwrite_only = env::var("CARGO_FEATURE_OVERWRITE_ONLY").is_ok(); let swap_move = env::var("CARGO_FEATURE_SWAP_MOVE").is_ok(); let swap_offset = env::var("CARGO_FEATURE_SWAP_OFFSET").is_ok(); @@ -61,6 +62,13 @@ fn main() { conf.conf.define("MCUBOOT_IMAGE_NUMBER", Some(if multiimage { "2" } else { "1" })); + if sig_second_key { + if !sig_ed25519 { + panic!("sig-second-key currently only supports sig-ed25519 in the simulator"); + } + conf.conf.define("MCUBOOT_SIGN_KEY_2", None); + } + if downgrade_prevention && !overwrite_only { panic!("Downgrade prevention requires overwrite only"); } diff --git a/sim/mcuboot-sys/csupport/keys.c b/sim/mcuboot-sys/csupport/keys.c index 82a746ba02..8b1271df6a 100644 --- a/sim/mcuboot-sys/csupport/keys.c +++ b/sim/mcuboot-sys/csupport/keys.c @@ -155,6 +155,17 @@ const unsigned char root_pub_der[] = { 0x20, 0xff, 0xb4, 0xe0, }; const unsigned int root_pub_der_len = 44; +#if defined(MCUBOOT_SIGN_KEY_2) +const unsigned char root_pub_der_2[] = { + 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, + 0x70, 0x03, 0x21, 0x00, 0xc3, 0xbc, 0xdf, 0x30, + 0x76, 0x4e, 0x1f, 0x5d, 0xb4, 0x0d, 0x89, 0x2e, + 0x0d, 0x0e, 0xab, 0x04, 0x9c, 0x30, 0x06, 0x4e, + 0x79, 0xa8, 0xed, 0xb6, 0xf7, 0x79, 0x2e, 0x46, + 0x9a, 0x26, 0xda, 0x77, +}; +const unsigned int root_pub_der_2_len = 44; +#endif #endif #if defined(HAVE_KEYS) @@ -163,8 +174,14 @@ const struct bootutil_key bootutil_keys[] = { .key = root_pub_der, .len = &root_pub_der_len, }, +#if defined(MCUBOOT_SIGN_KEY_2) + { + .key = root_pub_der_2, + .len = &root_pub_der_2_len, + }, +#endif }; -const int bootutil_key_cnt = 1; +const int bootutil_key_cnt = sizeof(bootutil_keys) / sizeof(bootutil_keys[0]); #endif #if defined(MCUBOOT_ENCRYPT_RSA) diff --git a/sim/src/ed25519_pub_key_2-rs.txt b/sim/src/ed25519_pub_key_2-rs.txt new file mode 100644 index 0000000000..800603f84f --- /dev/null +++ b/sim/src/ed25519_pub_key_2-rs.txt @@ -0,0 +1,9 @@ +/* Autogenerated by imgtool.py, do not edit. */ +static ED25519_PUB_KEY_2: &[u8] = &[ + 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, + 0x70, 0x03, 0x21, 0x00, 0xc3, 0xbc, 0xdf, 0x30, + 0x76, 0x4e, 0x1f, 0x5d, 0xb4, 0x0d, 0x89, 0x2e, + 0x0d, 0x0e, 0xab, 0x04, 0x9c, 0x30, 0x06, 0x4e, + 0x79, 0xa8, 0xed, 0xb6, 0xf7, 0x79, 0x2e, 0x46, + 0x9a, 0x26, 0xda, 0x77, +]; diff --git a/sim/src/ed25519_pub_key_unknown-rs.txt b/sim/src/ed25519_pub_key_unknown-rs.txt new file mode 100644 index 0000000000..5150794f86 --- /dev/null +++ b/sim/src/ed25519_pub_key_unknown-rs.txt @@ -0,0 +1,9 @@ +/* Autogenerated by imgtool.py, do not edit. */ +static ED25519_PUB_KEY_UNKNOWN: &[u8] = &[ + 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, + 0x70, 0x03, 0x21, 0x00, 0xd2, 0x39, 0xea, 0xd5, + 0x35, 0x88, 0xb0, 0xf5, 0xb2, 0x43, 0x3e, 0x4b, + 0x2b, 0xf2, 0x76, 0x19, 0x0b, 0x86, 0x87, 0x91, + 0x5c, 0xb0, 0x13, 0x30, 0xcd, 0x40, 0xd0, 0xc7, + 0x36, 0x67, 0x11, 0x95, +]; diff --git a/sim/src/image.rs b/sim/src/image.rs index 60c0deb846..ae9ff2b0f8 100644 --- a/sim/src/image.rs +++ b/sim/src/image.rs @@ -50,7 +50,7 @@ use crate::depends::{ PairDep, UpgradeInfo, }; -use crate::tlv::{ManifestGen, TlvGen, TlvFlags}; +use crate::tlv::{ManifestGen, SigningKey, TlvGen, TlvFlags}; use crate::utils::align_up; use typenum::{U32, U16}; @@ -221,6 +221,17 @@ impl ImagesBuilder { /// Construct an `Images` that doesn't expect an upgrade to happen. pub fn make_no_upgrade_image(self, deps: &DepTest, img_manipulation: ImageManipulation) -> Images { + self.make_no_upgrade_image_with_key(deps, img_manipulation, SigningKey::Primary) + } + + /// Like `make_no_upgrade_image`, but signs every installed image with the + /// given signing key. Used by the multi-key test matrix. + pub fn make_no_upgrade_image_with_key( + self, + deps: &DepTest, + img_manipulation: ImageManipulation, + signing_key: SigningKey, + ) -> Images { let num_images = self.num_images(); let mut flash = self.flash; let ram = self.ram.clone(); // TODO: Avoid this clone. @@ -234,21 +245,21 @@ impl ImagesBuilder { let (primaries,upgrades) = if img_manipulation == ImageManipulation::CorruptHigherVersionImage && !higher_version_corrupted { higher_version_corrupted = true; - let prim = install_image(&mut flash, &self.areadesc, &slots, 0, - maximal(42784), &ram, &*dep, ImageManipulation::None, Some(0)); + let prim = install_image_with_key(&mut flash, &self.areadesc, &slots, 0, + maximal(42784), &ram, &*dep, ImageManipulation::None, Some(0), signing_key); let upgr = match deps.depends[image_num] { DepType::NoUpgrade => install_no_image(), - _ => install_image(&mut flash, &self.areadesc, &slots, 1, - maximal(46928), &ram, &*dep, ImageManipulation::BadSignature, Some(1)) + _ => install_image_with_key(&mut flash, &self.areadesc, &slots, 1, + maximal(46928), &ram, &*dep, ImageManipulation::BadSignature, Some(1), signing_key) }; (prim, upgr) } else { - let prim = install_image(&mut flash, &self.areadesc, &slots, 0, - maximal(42784), &ram, &*dep, img_manipulation, Some(0)); + let prim = install_image_with_key(&mut flash, &self.areadesc, &slots, 0, + maximal(42784), &ram, &*dep, img_manipulation, Some(0), signing_key); let upgr = match deps.depends[image_num] { DepType::NoUpgrade => install_no_image(), - _ => install_image(&mut flash, &self.areadesc, &slots, 1, - maximal(46928), &ram, &*dep, img_manipulation, Some(1)) + _ => install_image_with_key(&mut flash, &self.areadesc, &slots, 1, + maximal(46928), &ram, &*dep, img_manipulation, Some(1), signing_key) }; (prim, upgr) }; @@ -320,6 +331,53 @@ impl ImagesBuilder { } } + /// Install a valid primary-key-signed image in slot 0 and a secondary + /// image signed with `secondary_key` in slot 1. Paired with + /// `run_signfail_upgrade` to assert that a build unaware of the secondary + /// key correctly refuses to upgrade to it. + pub fn make_secondary_slot_image_with_key(self, secondary_key: SigningKey) -> Images { + let mut flash = self.flash; + let ram = self.ram.clone(); + let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| { + let dep = BoringDep::new(image_num, &NO_DEPS); + let primaries = install_image_with_key( + &mut flash, + &self.areadesc, + &slots, + 0, + maximal(32_784), + &ram, + &dep, + ImageManipulation::None, + Some(0), + SigningKey::Primary, + ); + let upgrades = install_image_with_key( + &mut flash, + &self.areadesc, + &slots, + 1, + maximal(41_928), + &ram, + &dep, + ImageManipulation::None, + Some(1), + secondary_key, + ); + OneImage { + slots, + primaries, + upgrades, + }}).collect(); + Images { + flash, + areadesc: self.areadesc, + images, + total_count: None, + ram: self.ram, + } + } + pub fn make_oversized_secondary_slot_image(self) -> Images { let mut bad_flash = self.flash; let ram = self.ram.clone(); // TODO: Avoid this clone. @@ -1910,12 +1968,38 @@ fn compute_largest_image_size(dev: &dyn Flash, areadesc: &AreaDesc, slots: &[Slo fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slots: &[SlotInfo], slot_ind: usize, len: ImageSize, ram: &RamData, deps: &dyn Depender, img_manipulation: ImageManipulation, security_counter:Option) -> ImageData { + install_image_with_key( + flash, + areadesc, + slots, + slot_ind, + len, + ram, + deps, + img_manipulation, + security_counter, + SigningKey::Primary, + ) +} + +fn install_image_with_key( + flash: &mut SimMultiFlash, + areadesc: &AreaDesc, + slots: &[SlotInfo], + slot_ind: usize, + len: ImageSize, + ram: &RamData, + deps: &dyn Depender, + img_manipulation: ImageManipulation, + security_counter: Option, + signing_key: SigningKey, +) -> ImageData { let slot = &slots[slot_ind]; let mut offset = slot.base_off; let dev_id = slot.dev_id; let dev = flash.get_mut(&dev_id).unwrap(); - let mut tlv: Box = Box::new(make_tlv()); + let mut tlv: Box = Box::new(make_tlv(signing_key)); if Caps::SwapUsingOffset.present() && slot_ind == 1 { let sector_size = dev.sector_iter().next().unwrap().size as usize; @@ -2134,10 +2218,10 @@ fn install_no_image() -> ImageData { /// Construct a TLV generator based on how MCUboot is currently configured. The returned /// ManifestGen will generate the appropriate entries based on this configuration. -fn make_tlv() -> TlvGen { +fn make_tlv(signing_key: SigningKey) -> TlvGen { let aes_key_size = if Caps::Aes256.present() { 256 } else { 128 }; - if Caps::EncKw.present() { + let tlv = if Caps::EncKw.present() { if Caps::RSA2048.present() { TlvGen::new_rsa_kw(aes_key_size) } else if Caps::EcdsaP256.present() { @@ -2178,7 +2262,9 @@ fn make_tlv() -> TlvGen { } else { TlvGen::new_hash_only() } - } + }; + + tlv.with_signing_key(signing_key) } impl ImageData { diff --git a/sim/src/lib.rs b/sim/src/lib.rs index 34fd6fc3c2..e227e70cbc 100644 --- a/sim/src/lib.rs +++ b/sim/src/lib.rs @@ -15,7 +15,7 @@ use serde_derive::Deserialize; mod caps; mod depends; mod image; -mod tlv; +pub mod tlv; mod utils; pub mod testlog; diff --git a/sim/src/tlv.rs b/sim/src/tlv.rs index d076eb2379..f1263d9a39 100644 --- a/sim/src/tlv.rs +++ b/sim/src/tlv.rs @@ -120,6 +120,23 @@ pub trait ManifestGen { fn set_ignore_ram_load_flag(&mut self); } +/// Selects which signing key to use when generating the TLV signature. +/// The default (`Primary`) preserves existing single-key test behaviour. +#[derive(Debug, Copy, Clone, Default, Eq, PartialEq)] +pub enum SigningKey { + /// Sign with the primary (and only, in single-key builds) signing key. + #[default] + Primary, + /// Sign with a distinct secondary key. The bootloader must have been + /// built with `MCUBOOT_SIGN_KEY_2` (via the `sig-second-key` Cargo + /// feature) and embed both public keys; otherwise the signed image is + /// expected to be rejected. + Secondary, + /// Sign with a key the bootloader does not know about. Used to assert + /// that multi-key builds still reject images signed by any third key. + Unknown, +} + #[derive(Debug, Default)] pub struct TlvGen { flags: u32, @@ -132,6 +149,8 @@ pub struct TlvGen { security_cnt: Option, /// Ignore RAM_LOAD flag ignore_ram_load_flag: bool, + /// Which signing key to use. + signing_key: SigningKey, } #[derive(Debug)] @@ -141,6 +160,14 @@ struct Dependency { } impl TlvGen { + /// Builder: select which signing key the generator will use. Has no + /// effect on non-signing TLV kinds. + #[allow(dead_code)] + pub fn with_signing_key(mut self, key: SigningKey) -> Self { + self.signing_key = key; + self + } + /// Construct a new tlv generator that will only contain a hash of the data. #[allow(dead_code)] pub fn new_hash_only() -> TlvGen { @@ -646,7 +673,13 @@ impl ManifestGen for TlvGen { } if self.kinds.contains(&TlvKinds::ED25519) { - let keyhash = digest::digest(&digest::SHA256, ED25519_PUB_KEY); + let (pem_bytes, pub_key): (&[u8], &[u8]) = match self.signing_key { + SigningKey::Primary => (include_bytes!("../../root-ed25519.pem"), ED25519_PUB_KEY), + SigningKey::Secondary => (include_bytes!("../../root-ed25519-2.pem"), ED25519_PUB_KEY_2), + SigningKey::Unknown => (include_bytes!("../../root-ed25519-unknown.pem"), ED25519_PUB_KEY_UNKNOWN), + }; + + let keyhash = digest::digest(&digest::SHA256, pub_key); let keyhash = keyhash.as_ref(); assert!(keyhash.len() == 32); @@ -658,11 +691,11 @@ impl ManifestGen for TlvGen { let hash = hash.as_ref(); assert!(hash.len() == 32); - let key_bytes = pem::parse(include_bytes!("../../root-ed25519.pem").as_ref()).unwrap(); + let key_bytes = pem::parse(pem_bytes).unwrap(); assert_eq!(key_bytes.tag, "PRIVATE KEY"); let key_pair = Ed25519KeyPair::from_seed_and_public_key( - &key_bytes.contents[16..48], &ED25519_PUB_KEY[12..44]).unwrap(); + &key_bytes.contents[16..48], &pub_key[12..44]).unwrap(); let signature = key_pair.sign(&hash); result.write_u16::(TlvKinds::ED25519 as u16).unwrap(); @@ -861,3 +894,5 @@ include!("rsa_pub_key-rs.txt"); include!("rsa3072_pub_key-rs.txt"); include!("ecdsa_pub_key-rs.txt"); include!("ed25519_pub_key-rs.txt"); +include!("ed25519_pub_key_2-rs.txt"); +include!("ed25519_pub_key_unknown-rs.txt"); diff --git a/sim/tests/core.rs b/sim/tests/core.rs index b7be7530d4..6843ec8f89 100644 --- a/sim/tests/core.rs +++ b/sim/tests/core.rs @@ -86,6 +86,66 @@ sim_test!(ram_load_corrupt_higher_version_image, make_no_upgrade_image(&NO_DEPS, sim_test!(hw_prot_missing_security_cnt, make_image_with_security_counter(None), run_hw_rollback_prot()); sim_test!(hw_prot_failed_security_cnt_check, make_image_with_security_counter(Some(0)), run_hw_rollback_prot()); +#[cfg(feature = "sig-ed25519")] +mod multi_key { + //! Multi-signing-key matrix. + //! + //! These tests only run when the simulator was built with `sig-ed25519` — + //! `sig-second-key` alone is meaningless without a supported signature + //! type, and build.rs panics if the pairing is missing. The whole module + //! is dropped on incompatible feature combinations rather than silently + //! turning into a no-op. + + use super::*; + use bootsim::tlv::SigningKey; + + // With a single-key build, the image signed with the primary key must + // boot and upgrade normally. With a two-key build (sig-second-key on) + // this exercises the iteration-finds-first-match path in + // bootutil_find_key(). + test_shell!(signed_primary_key_boots, r, { + let image = r.make_no_upgrade_image_with_key( + &NO_DEPS, + ImageManipulation::None, + SigningKey::Primary, + ); + dump_image(&image, "signed_primary_key_boots"); + assert!(!image.run_norevert_newimage()); + }); + + // With `sig-second-key`, the bootloader embeds a second verification + // key; an image signed with that key must boot. Without the feature, + // this test is dropped (see cfg-gated module). + #[cfg(feature = "sig-second-key")] + test_shell!(signed_secondary_key_boots, r, { + let image = r.make_no_upgrade_image_with_key( + &NO_DEPS, + ImageManipulation::None, + SigningKey::Secondary, + ); + dump_image(&image, "signed_secondary_key_boots"); + assert!(!image.run_norevert_newimage()); + }); + + // A third ("unknown") key must always be rejected, regardless of + // whether the bootloader was built single- or multi-key. + test_shell!(signed_unknown_key_rejected, r, { + let image = r.make_secondary_slot_image_with_key(SigningKey::Unknown); + dump_image(&image, "signed_unknown_key_rejected"); + assert!(!image.run_signfail_upgrade()); + }); + + // Regression guard: a single-key build must reject images signed with + // the secondary key (since it doesn't embed it). Only valid when + // sig-second-key is OFF. + #[cfg(not(feature = "sig-second-key"))] + test_shell!(single_key_build_rejects_secondary_key_image, r, { + let image = r.make_secondary_slot_image_with_key(SigningKey::Secondary); + dump_image(&image, "single_key_build_rejects_secondary_key_image"); + assert!(!image.run_signfail_upgrade()); + }); +} + #[cfg(feature = "multiimage")] sim_test!(ram_load_overlapping_images_same_base, make_no_upgrade_image(&NO_DEPS, ImageManipulation::OverlapImages(true)), run_ram_load_boot_with_result(false)); #[cfg(feature = "multiimage")]