Move key_id from header into TLV

Remove the key_id field from the image header.  There are two problems
with this field.  First, it is only an integer offset, and so causes an
unnecessarily tight coupling between the particular keys built into the
bootloader, and the key that is used to sign.  Second, it makes the
key_id part of the image header, which is included in the signature.
This makes it impossible to later sign the image with a different
signature.

Instead of the key-id, add a TLV KEYHASH entry.  This will hold the
SHA256 of the public key that the signature is against.  Each signature
placed in the TLV should be preceeded by this entry to indicate the
public key used.

The signature check will check each signature, and if the KEYHASH is
known and the signature type is supported, it will be checked.  As long
as at least one signature is considered valid, the image will be
considered signed.  This also allows the image to be signed with
multiple signatures to support having different devices with possibly
different keys compiled into the bootloaders.

Based on work by Marko Kiiskila <marko@runtime.io>

Signed-off-by: Marko Kiiskila <marko@runtime.io>
Signed-off-by: David Brown <david.brown@linaro.org>

Author: d3zd3z

boot/bootutil/include/bootutil/image.h

@@ -37,12 +37,7 @@ struct flash_area;
  * Image header flags.
  */
 #define IMAGE_F_PIC                      0x00000001 /* Not supported. */
-#define IMAGE_F_SHA256                   0x00000002 /* Hash TLV is present */
-#define IMAGE_F_PKCS15_RSA2048_SHA256    0x00000004 /* PKCS15 w/RSA and SHA */
-#define IMAGE_F_ECDSA224_SHA256          0x00000008 /* ECDSA224 over SHA256 */
 #define IMAGE_F_NON_BOOTABLE             0x00000010 /* Split image app. */
-#define IMAGE_F_ECDSA256_SHA256          0x00000020 /* ECDSA256 over SHA256 */
-#define IMAGE_F_PKCS1_PSS_RSA2048_SHA256 0x00000040 /* PKCS1 PSS */
 
 /*
  * ECSDA224 is with NIST P-224
@@ -52,6 +47,7 @@ struct flash_area;
 /*
  * Image trailer TLV types.
  */
+#define IMAGE_TLV_KEYHASH           0x01   /* hash of the public key */
 #define IMAGE_TLV_SHA256            0x10   /* SHA256 of image hdr and body */
 #define IMAGE_TLV_RSA2048           0x20   /* RSA2048 of hash output */
 #define IMAGE_TLV_ECDSA224          0x21   /* ECDSA of hash output */
@@ -71,7 +67,7 @@ struct image_version {
 struct image_header {
     uint32_t ih_magic;
     uint16_t ih_tlv_size; /* Combined size of trailing TLVs (bytes). */
-    uint8_t  ih_key_id;   /* Which key image is signed with (0xff=unsigned). */
+    uint8_t  _pad0;
     uint8_t  _pad1;
     uint16_t ih_hdr_size; /* Size of image header (bytes). */
     uint16_t _pad2;

boot/bootutil/src/image_validate.c

@@ -87,6 +87,53 @@ bootutil_img_hash(struct image_header *hdr, const struct flash_area *fap,
     return 0;
 }
 
+/*
+ * Currently, we only support being able to verify one type of
+ * signature, because there is a single verification function that we
+ * call.  List the type of TLV we are expecting.  If we aren't
+ * configured for any signature, don't define this macro.
+ */
+#if defined(MCUBOOT_SIGN_RSA)
+#    define EXPECTED_SIG_TLV IMAGE_TLV_RSA2048
+#    define EXPECTED_SIG_LEN(x) ((x) == 256) /* 2048 bits */
+#    if defined(MCUBOOT_SIGN_EC) || defined(MCUBOOT_SIGN_EC256)
+#        error "Multiple signature types not yet supported"
+#    endif
+#elif defined(MCUBOOT_SIGN_EC)
+#    define EXPECTED_SIG_TLV IMAGE_TLV_ECDSA224
+#    define EXPECTED_SIG_LEN(x) ((x) >= 64) /* oids + 2 * 28 bytes */
+#    if defined(MCUBOOT_SIGN_EC256)
+#        error "Multiple signature types not yet supported"
+#    endif
+#elif defined(MCUBOOT_SIGN_EC256)
+#    define EXPECTED_SIG_TLV IMAGE_TLV_ECDSA256
+#    define EXPECTED_SIG_LEN(x) ((x) >= 72) /* oids + 2 * 32 bytes */
+#endif
+
+#ifdef EXPECTED_SIG_TLV
+static int
+bootutil_find_key(uint8_t *keyhash, uint8_t keyhash_len)
+{
+    bootutil_sha256_context sha256_ctx;
+    int i;
+    const struct bootutil_key *key;
+    uint8_t hash[32];
+
+    assert(keyhash_len == 32);
+
+    for (i = 0; i < bootutil_key_cnt; i++) {
+        key = &bootutil_keys[i];
+        bootutil_sha256_init(&sha256_ctx);
+        bootutil_sha256_update(&sha256_ctx, key->key, *key->len);
+        bootutil_sha256_finish(&sha256_ctx, hash);
+        if (!memcmp(hash, keyhash, keyhash_len)) {
+            return i;
+        }
+    }
+    return -1;
+}
+#endif
+
 /*
  * Verify the integrity of the image.
  * Return non-zero if image could not be validated/does not validate.
@@ -98,42 +145,16 @@ bootutil_img_validate(struct image_header *hdr, const struct flash_area *fap,
 {
     uint32_t off;
     uint32_t size;
-    uint32_t sha_off = 0;
-#if defined(MCUBOOT_SIGN_RSA) || defined(MCUBOOT_SIGN_EC) || \
-    defined(MCUBOOT_SIGN_EC256)
-    uint32_t sig_off = 0;
-    uint32_t sig_len = 0;
+    int sha256_valid = 0;
+#ifdef EXPECTED_SIG_TLV
+    int valid_signature = 0;
+    int key_id = -1;
 #endif
     struct image_tlv tlv;
     uint8_t buf[256];
     uint8_t hash[32];
     int rc;
 
-#ifdef MCUBOOT_SIGN_RSA
-#ifdef MCUBOOT_RSA_PKCS1_15
-    if ((hdr->ih_flags & IMAGE_F_PKCS15_RSA2048_SHA256) == 0) {
-        return -1;
-    }
-#else
-    if ((hdr->ih_flags & IMAGE_F_PKCS1_PSS_RSA2048_SHA256) == 0) {
-        return -1;
-    }
-#endif
-#endif
-#ifdef MCUBOOT_SIGN_EC
-    if ((hdr->ih_flags & IMAGE_F_ECDSA224_SHA256) == 0) {
-        return -1;
-    }
-#endif
-#ifdef MCUBOOT_SIGN_EC256
-    if ((hdr->ih_flags & IMAGE_F_ECDSA256_SHA256) == 0) {
-        return -1;
-    }
-#endif
-    if ((hdr->ih_flags & IMAGE_F_SHA256) == 0) {
-        return -1;
-    }
-
     rc = bootutil_img_hash(hdr, fap, tmp_buf, tmp_buf_sz, hash,
                            seed, seed_len);
     if (rc) {
@@ -148,81 +169,81 @@ bootutil_img_validate(struct image_header *hdr, const struct flash_area *fap,
     off = hdr->ih_img_size + hdr->ih_hdr_size;
     size = off + hdr->ih_tlv_size;
 
+    /*
+     * Traverse through all of the TLVs, performing any checks we know
+     * and are able to do.
+     */
     for (; off < size; off += sizeof(tlv) + tlv.it_len) {
         rc = flash_area_read(fap, off, &tlv, sizeof tlv);
         if (rc) {
             return rc;
         }
+
         if (tlv.it_type == IMAGE_TLV_SHA256) {
+            /*
+             * Verify the SHA256 image hash.  This must always be
+             * present.
+             */
             if (tlv.it_len != sizeof(hash)) {
                 return -1;
             }
-            sha_off = off + sizeof(tlv);
-        }
-#ifdef MCUBOOT_SIGN_RSA
-        if (tlv.it_type == IMAGE_TLV_RSA2048) {
-            if (tlv.it_len != 256) { /* 2048 bits */
+            rc = flash_area_read(fap, off + sizeof(tlv), buf, sizeof hash);
+            if (rc) {
+                    return rc;
+            }
+            if (memcmp(hash, buf, sizeof(hash))) {
+                    return -1;
+            }
+
+            sha256_valid = 1;
+#ifdef EXPECTED_SIG_TLV
+        } else if (tlv.it_type == IMAGE_TLV_KEYHASH) {
+            /*
+             * Determine which key we should be checking.
+             */
+            if (tlv.it_len > 32) {
                 return -1;
             }
-            sig_off = off + sizeof(tlv);
-            sig_len = tlv.it_len;
-        }
-#endif
-#ifdef MCUBOOT_SIGN_EC
-        if (tlv.it_type == IMAGE_TLV_ECDSA224) {
-            if (tlv.it_len < 64) { /* oids + 2 * 28 bytes */
+            rc = flash_area_read(fap, off + sizeof tlv, buf, tlv.it_len);
+            if (rc) {
+                return rc;
+            }
+            key_id = bootutil_find_key(buf, tlv.it_len);
+            /*
+             * The key may not be found, which is acceptable.  There
+             * can be multiple signatures, each preceded by a key.
+             */
+        } else if (tlv.it_type == EXPECTED_SIG_TLV) {
+            /* Ignore this signature if it is out of bounds. */
+            if (key_id < 0 || key_id >= bootutil_key_cnt) {
+                key_id = -1;
+                continue;
+            }
+            if (!EXPECTED_SIG_LEN(tlv.it_len) || tlv.it_len > sizeof(buf)) {
                 return -1;
             }
-            sig_off = off + sizeof(tlv);
-            sig_len = tlv.it_len;
-        }
-#endif
-#ifdef MCUBOOT_SIGN_EC256
-        if (tlv.it_type == IMAGE_TLV_ECDSA256) {
-            if (tlv.it_len < 72) { /* oids + 2 * 32 bytes */
+            rc = flash_area_read(fap, off + sizeof(tlv), buf, tlv.it_len);
+            if (rc) {
                 return -1;
             }
-            sig_off = off + sizeof(tlv);
-            sig_len = tlv.it_len;
-        }
+            rc = bootutil_verify_sig(hash, sizeof(hash), buf, tlv.it_len, key_id);
+            if (rc == 0) {
+                valid_signature = 1;
+            }
+            key_id = -1;
 #endif
-    }
-    if (hdr->ih_flags & IMAGE_F_SHA256) {
-        if (!sha_off) {
-            /*
-             * Header said there should be hash TLV, no TLV found.
-             */
-            return -1;
-        }
-        rc = flash_area_read(fap, sha_off, buf, sizeof hash);
-        if (rc) {
-            return rc;
-        }
-        if (memcmp(hash, buf, sizeof(hash))) {
-            return -1;
         }
     }
-#if defined(MCUBOOT_SIGN_RSA) || defined(MCUBOOT_SIGN_EC) || \
-    defined(MCUBOOT_SIGN_EC256)
-    if (!sig_off) {
-        /*
-         * Header said there should be PKCS1.v5 signature, no TLV
-         * found.
-         */
-        return -1;
-    }
-    rc = flash_area_read(fap, sig_off, buf, sig_len);
-    if (rc) {
-        return -1;
-    }
 
-    if (hdr->ih_key_id >= bootutil_key_cnt) {
+    if (!sha256_valid) {
         return -1;
     }
-    rc = bootutil_verify_sig(hash, sizeof(hash), buf, sig_len, hdr->ih_key_id);
-    if (rc) {
+
+#ifdef EXPECTED_SIG_TLV
+    if (!valid_signature) {
         return -1;
     }
 #endif
+
     return 0;
 }

scripts/imgtool/image.py

@@ -12,14 +12,10 @@
 # Image header flags.
 IMAGE_F = {
         'PIC':                   0x0000001,
-        'SHA256':                0x0000002,
-        'PKCS15_RSA2048_SHA256': 0x0000004,
-        'ECDSA224_SHA256':       0x0000008,
-        'NON_BOOTABLE':          0x0000010,
-        'ECDSA256_SHA256':       0x0000020,
-        'PKCS1_PSS_RSA2048_SHA256': 0x0000040, }
+        'NON_BOOTABLE':          0x0000010, }
 
 TLV_VALUES = {
+        'KEYHASH': 0x01,
         'SHA256': 0x10,
         'RSA2048': 0x20,
         'ECDSA224': 0x21,
@@ -106,6 +102,12 @@ def sign(self, key):
         tlv.add('SHA256', digest)
 
         if key is not None:
+            pub = key.get_public_bytes()
+            sha = hashlib.sha256()
+            sha.update(pub)
+            pubbytes = sha.digest()
+            tlv.add('KEYHASH', pubbytes)
+
             sig = key.sign(self.payload)
             tlv.add(key.sig_tlv(), sig)
 
@@ -120,10 +122,9 @@ def add_header(self, key):
         flags = 0
         tlvsz = 0
         if key is not None:
-            flags |= IMAGE_F[key.sig_type()]
             tlvsz += TLV_HEADER_SIZE + key.sig_len()
 
-        flags |= IMAGE_F['SHA256']
+        tlvsz += 4 + hashlib.sha256().digest_size
         tlvsz += 4 + hashlib.sha256().digest_size
 
         fmt = ('<' +

sim/Cargo.lock

@@ -21,7 +21,6 @@ log = "0.3"
 env_logger = "0.4"
 simflash = { path = "simflash" }
 mcuboot-sys = { path = "mcuboot-sys" }
-bitflags = "0.9"
 ring = { version = "0.11", features = ["rsa_signing"] }
 untrusted = "0.5"
 pem = "0.4"

sim/Cargo.toml

@@ -1,7 +1,6 @@
 #[macro_use] extern crate log;
 extern crate ring;
 extern crate env_logger;
-#[macro_use] extern crate bitflags;
 extern crate docopt;
 extern crate libc;
 extern crate pem;

sim/src/main.rs

@@ -0,0 +1,37 @@
+/* Autogenerated by imgtool.py, do not edit. */
+static RSA_PUB_KEY: &'static [u8] = &[
+    0x30, 0x82, 0x01, 0x0a, 0x02, 0x82, 0x01, 0x01,
+    0x00, 0xd1, 0x06, 0x08, 0x1a, 0x18, 0x44, 0x2c,
+    0x18, 0xe8, 0xfb, 0xfd, 0xf7, 0x0d, 0xa3, 0x4f,
+    0x1f, 0xbb, 0xee, 0x5e, 0xf9, 0xaa, 0xd2, 0x4b,
+    0x18, 0xd3, 0x5a, 0xe9, 0x6d, 0x18, 0x80, 0x19,
+    0xf9, 0xf0, 0x9c, 0x34, 0x1b, 0xcb, 0xf3, 0xbc,
+    0x74, 0xdb, 0x42, 0xe7, 0x8c, 0x7f, 0x10, 0x53,
+    0x7e, 0x43, 0x5e, 0x0d, 0x57, 0x2c, 0x44, 0xd1,
+    0x67, 0x08, 0x0f, 0x0d, 0xbb, 0x5c, 0xee, 0xec,
+    0xb3, 0x99, 0xdf, 0xe0, 0x4d, 0x84, 0x0b, 0xaa,
+    0x77, 0x41, 0x60, 0xed, 0x15, 0x28, 0x49, 0xa7,
+    0x01, 0xb4, 0x3c, 0x10, 0xe6, 0x69, 0x8c, 0x2f,
+    0x5f, 0xac, 0x41, 0x4d, 0x9e, 0x5c, 0x14, 0xdf,
+    0xf2, 0xf8, 0xcf, 0x3d, 0x1e, 0x6f, 0xe7, 0x5b,
+    0xba, 0xb4, 0xa9, 0xc8, 0x88, 0x7e, 0x47, 0x3c,
+    0x94, 0xc3, 0x77, 0x67, 0x54, 0x4b, 0xaa, 0x8d,
+    0x38, 0x35, 0xca, 0x62, 0x61, 0x7e, 0xb7, 0xe1,
+    0x15, 0xdb, 0x77, 0x73, 0xd4, 0xbe, 0x7b, 0x72,
+    0x21, 0x89, 0x69, 0x24, 0xfb, 0xf8, 0x65, 0x6e,
+    0x64, 0x3e, 0xc8, 0x0e, 0xd7, 0x85, 0xd5, 0x5c,
+    0x4a, 0xe4, 0x53, 0x0d, 0x2f, 0xff, 0xb7, 0xfd,
+    0xf3, 0x13, 0x39, 0x83, 0x3f, 0xa3, 0xae, 0xd2,
+    0x0f, 0xa7, 0x6a, 0x9d, 0xf9, 0xfe, 0xb8, 0xce,
+    0xfa, 0x2a, 0xbe, 0xaf, 0xb8, 0xe0, 0xfa, 0x82,
+    0x37, 0x54, 0xf4, 0x3e, 0xe1, 0x2b, 0xd0, 0xd3,
+    0x08, 0x58, 0x18, 0xf6, 0x5e, 0x4c, 0xc8, 0x88,
+    0x81, 0x31, 0xad, 0x5f, 0xb0, 0x82, 0x17, 0xf2,
+    0x8a, 0x69, 0x27, 0x23, 0xf3, 0xab, 0x87, 0x3e,
+    0x93, 0x1a, 0x1d, 0xfe, 0xe8, 0xf8, 0x1a, 0x24,
+    0x66, 0x59, 0xf8, 0x1c, 0xab, 0xdc, 0xce, 0x68,
+    0x1b, 0x66, 0x64, 0x35, 0xec, 0xfa, 0x0d, 0x11,
+    0x9d, 0xaf, 0x5c, 0x3a, 0xa7, 0xd1, 0x67, 0xc6,
+    0x47, 0xef, 0xb1, 0x4b, 0x2c, 0x62, 0xe1, 0xd1,
+    0xc9, 0x02, 0x03, 0x01, 0x00, 0x01,
+];