AP_CheckFirmware: allow an unsigned bootloader to boot a signed firmware

this is important to provide an upgrade path for AP_Periph from
unsigned to signed. It means a bootloader with no public keys can
still check the board ID and CRCs of the signed firmware

libraries/AP_CheckFirmware/AP_CheckFirmware.cpp

@@ -16,17 +16,37 @@
 
 const struct ap_secure_data public_keys __attribute__((section(".ecc_raw")));
 
+/*
+  return true if all public keys are zero. We allow boot of an
+  unsigned firmware in that case
+ */
+static bool all_zero_public_keys(void)
+{
+    /*
+      look over all public keys, if one matches then we are OK
+     */
+    const uint8_t zero_key[AP_PUBLIC_KEY_LEN] {};
+    for (const auto &public_key : public_keys.public_key) {
+        if (memcmp(public_key.key, zero_key, AP_PUBLIC_KEY_LEN) != 0) {
+            return false;
+        }
+    }
+    return true;
+}
+
 /*
   check a signature against bootloader keys
  */
-static check_fw_result_t check_firmware_signature(const app_descriptor *ad,
+static check_fw_result_t check_firmware_signature(const app_descriptor_signed *ad,
                                                   const uint8_t *flash1, uint32_t len1,
                                                   const uint8_t *flash2, uint32_t len2)
 {
+    if (all_zero_public_keys()) {
+        return check_fw_result_t::CHECK_FW_OK;
+    }
+
     // 8 byte signature version
     static const uint64_t sig_version = 30437LLU;
-    const uint8_t zero_key[AP_PUBLIC_KEY_LEN] {};
-
     if (ad->signature_length != 72) {
         return check_fw_result_t::FAIL_REASON_BAD_FIRMWARE_SIGNATURE;
     }
@@ -34,16 +54,10 @@ static check_fw_result_t check_firmware_signature(const app_descriptor *ad,
         return check_fw_result_t::FAIL_REASON_BAD_FIRMWARE_SIGNATURE;
     }
 
-    bool all_zero_keys = true;
     /*
       look over all public keys, if one matches then we are OK
      */
     for (const auto &public_key : public_keys.public_key) {
-        if (memcmp(public_key.key, zero_key, AP_PUBLIC_KEY_LEN) == 0) {
-            continue;
-        }
-        all_zero_keys = false;
-
         crypto_check_ctx ctx {};
         crypto_check_ctx_abstract *actx = (crypto_check_ctx_abstract*)&ctx;
         crypto_check_init(actx, &ad->signature[sizeof(sig_version)], public_key.key);
@@ -56,11 +70,6 @@ static check_fw_result_t check_firmware_signature(const app_descriptor *ad,
         }
     }
 
-    if (all_zero_keys) {
-        // bootloader is unlocked if it has no public keys
-        return check_fw_result_t::CHECK_FW_OK;
-    }
-
     // none of the public keys matched
     return check_fw_result_t::FAIL_REASON_VERIFICATION;
 }
@@ -69,16 +78,12 @@ static check_fw_result_t check_firmware_signature(const app_descriptor *ad,
 /*
   check firmware CRC and board ID to see if it matches
  */
-check_fw_result_t check_good_firmware(void)
+static check_fw_result_t check_good_firmware_signed(void)
 {
-#if AP_SIGNED_FIRMWARE
+    const uint8_t sig[8] = AP_APP_DESCRIPTOR_SIGNATURE_SIGNED;
-    const uint8_t sig[8] = { 0x41, 0xa3, 0xe5, 0xf2, 0x65, 0x69, 0x92, 0x07 };
-#else
-    const uint8_t sig[8] = { 0x40, 0xa2, 0xe4, 0xf1, 0x64, 0x68, 0x91, 0x06 };
-#endif
     const uint8_t *flash1 = (const uint8_t *)(FLASH_LOAD_ADDRESS + (FLASH_BOOTLOADER_LOAD_KB + APP_START_OFFSET_KB)*1024);
     const uint32_t flash_size = (BOARD_FLASH_SIZE - (FLASH_BOOTLOADER_LOAD_KB + APP_START_OFFSET_KB))*1024;
-    const app_descriptor *ad = (const app_descriptor *)memmem(flash1, flash_size-sizeof(app_descriptor), sig, sizeof(sig));
+    const app_descriptor_signed *ad = (const app_descriptor_signed *)memmem(flash1, flash_size-sizeof(app_descriptor_signed), sig, sizeof(sig));
     if (ad == nullptr) {
         // no application signature
         return check_fw_result_t::FAIL_REASON_NO_APP_SIG;
@@ -98,7 +103,7 @@ check_fw_result_t check_good_firmware(void)
     }
 
     const uint8_t *flash2 = (const uint8_t *)&ad->version_major;
-    const uint8_t desc_len = offsetof(app_descriptor, version_major) - offsetof(app_descriptor, image_crc1);
+    const uint8_t desc_len = offsetof(app_descriptor_signed, version_major) - offsetof(app_descriptor_signed, image_crc1);
     const uint32_t len1 = ((const uint8_t *)&ad->image_crc1) - flash1;
 
     if ((len1 + desc_len) > ad->image_size) {
@@ -120,6 +125,79 @@ check_fw_result_t check_good_firmware(void)
 
     return ret;
 }
+
+/*
+  check firmware CRC and board ID to see if it matches, using unsigned
+  signature
+ */
+static check_fw_result_t check_good_firmware_unsigned(void)
+{
+    const uint8_t sig[8] = AP_APP_DESCRIPTOR_SIGNATURE_UNSIGNED;
+    const uint8_t *flash1 = (const uint8_t *)(FLASH_LOAD_ADDRESS + (FLASH_BOOTLOADER_LOAD_KB + APP_START_OFFSET_KB)*1024);
+    const uint32_t flash_size = (BOARD_FLASH_SIZE - (FLASH_BOOTLOADER_LOAD_KB + APP_START_OFFSET_KB))*1024;
+    const app_descriptor_unsigned *ad = (const app_descriptor_unsigned *)memmem(flash1, flash_size-sizeof(app_descriptor_unsigned), sig, sizeof(sig));
+    if (ad == nullptr) {
+        // no application signature
+        return check_fw_result_t::FAIL_REASON_NO_APP_SIG;
+    }
+    // check length
+    if (ad->image_size > flash_size) {
+        return check_fw_result_t::FAIL_REASON_BAD_LENGTH_APP;
+    }
+
+    bool id_ok = (ad->board_id == APJ_BOARD_ID);
+#ifdef ALT_BOARD_ID
+    id_ok |= (ad->board_id == ALT_BOARD_ID);
+#endif
+
+    if (!id_ok) {
+        return check_fw_result_t::FAIL_REASON_BAD_BOARD_ID;
+    }
+
+    const uint8_t *flash2 = (const uint8_t *)&ad->version_major;
+    const uint8_t desc_len = offsetof(app_descriptor_unsigned, version_major) - offsetof(app_descriptor_unsigned, image_crc1);
+    const uint32_t len1 = ((const uint8_t *)&ad->image_crc1) - flash1;
+
+    if ((len1 + desc_len) > ad->image_size) {
+        return check_fw_result_t::FAIL_REASON_BAD_LENGTH_DESCRIPTOR;
+    }
+
+    const uint32_t len2 = ad->image_size - (len1 + desc_len);
+    uint32_t crc1 = crc32_small(0, flash1, len1);
+    uint32_t crc2 = crc32_small(0, flash2, len2);
+    if (crc1 != ad->image_crc1 || crc2 != ad->image_crc2) {
+        return check_fw_result_t::FAIL_REASON_BAD_CRC;
+    }
+
+    return check_fw_result_t::CHECK_FW_OK;
+}
+
+check_fw_result_t check_good_firmware(void)
+{
+#if AP_SIGNED_FIRMWARE
+    // allow unsigned format if we have no public keys. This allows
+    // for use of SECURE_COMMAND to remove all public keys and then
+    // load of unsigned firmware
+    const auto ret = check_good_firmware_signed();
+    if (ret != check_fw_result_t::CHECK_FW_OK &&
+        all_zero_public_keys() &&
+        check_good_firmware_unsigned() == check_fw_result_t::CHECK_FW_OK) {
+        return check_fw_result_t::CHECK_FW_OK;
+    }
+    return ret;
+#else
+    const auto ret = check_good_firmware_unsigned();
+    if (ret != check_fw_result_t::CHECK_FW_OK) {
+        // allow for signed format, not checking public keys. This
+        // allows for booting of a signed firmware with an unsigned
+        // bootloader, which allows for bootstrapping a system up from
+        // unsigned to signed
+        return check_good_firmware_signed();
+    }
+    return ret;
+#endif
+}
+
 #endif // HAL_BOOTLOADER_BUILD
 
 #if !defined(HAL_BOOTLOADER_BUILD)
@@ -128,11 +206,11 @@ extern const AP_HAL::HAL &hal;
 /*
   declare constant app_descriptor in flash
  */
-extern const struct app_descriptor app_descriptor;
+extern const app_descriptor_t app_descriptor;
 #if CONFIG_HAL_BOARD == HAL_BOARD_CHIBIOS
-const struct app_descriptor app_descriptor __attribute__((section(".app_descriptor")));
+const app_descriptor_t app_descriptor __attribute__((section(".app_descriptor")));
 #else
-const struct app_descriptor app_descriptor;
+const app_descriptor_t app_descriptor;
 #endif
 
 /*

libraries/AP_CheckFirmware/AP_CheckFirmware.h

@@ -47,13 +47,16 @@ enum class check_fw_result_t : uint8_t {
  by the bootloader to confirm that the firmware is not corrupt and is
  suitable for this board. The build dependent values in this structure
  are filled in by set_app_descriptor() in the waf build
+
+ Note that we need to define both structures to make it possible to
+ boot a signed firmware using a bootloader setup for unsigned
  */
-struct app_descriptor {
+
-#if AP_SIGNED_FIRMWARE
+#define AP_APP_DESCRIPTOR_SIGNATURE_SIGNED   { 0x41, 0xa3, 0xe5, 0xf2, 0x65, 0x69, 0x92, 0x07 }
-    uint8_t sig[8] = { 0x41, 0xa3, 0xe5, 0xf2, 0x65, 0x69, 0x92, 0x07 };
+#define AP_APP_DESCRIPTOR_SIGNATURE_UNSIGNED { 0x40, 0xa2, 0xe4, 0xf1, 0x64, 0x68, 0x91, 0x06 }
-#else
+
-    uint8_t sig[8] = { 0x40, 0xa2, 0xe4, 0xf1, 0x64, 0x68, 0x91, 0x06 };
+struct app_descriptor_unsigned {
-#endif
+    uint8_t sig[8] = AP_APP_DESCRIPTOR_SIGNATURE_UNSIGNED;
     // crc1 is the crc32 from firmware start to start of image_crc1
     uint32_t image_crc1 = 0;
     // crc2 is the crc32 from the start of version_major to the end of the firmware
@@ -62,11 +65,6 @@ struct app_descriptor {
     uint32_t image_size = 0;
     uint32_t git_hash = 0;
 
-#if AP_SIGNED_FIRMWARE
-    // firmware signature
-    uint32_t signature_length = 0;
-    uint8_t signature[72] = {};
-#endif
     // software version number
     uint8_t  version_major = APP_FW_MAJOR;
     uint8_t version_minor = APP_FW_MINOR;
@@ -74,16 +72,42 @@ struct app_descriptor {
     // with high byte in HardwareVersion.major and low byte in HardwareVersion.minor
     uint16_t  board_id = APJ_BOARD_ID;
     uint8_t reserved[8] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+};
 
+struct app_descriptor_signed {
+    uint8_t sig[8] = AP_APP_DESCRIPTOR_SIGNATURE_SIGNED;
+    // crc1 is the crc32 from firmware start to start of image_crc1
+    uint32_t image_crc1 = 0;
+    // crc2 is the crc32 from the start of version_major to the end of the firmware
+    uint32_t image_crc2 = 0;
+    // total size of firmware image in bytes
+    uint32_t image_size = 0;
+    uint32_t git_hash = 0;
+
+    // firmware signature
+    uint32_t signature_length = 0;
+    uint8_t signature[72] = {};
+
+    // software version number
+    uint8_t  version_major = APP_FW_MAJOR;
+    uint8_t version_minor = APP_FW_MINOR;
+    // APJ_BOARD_ID (hardware version). This is also used in CAN NodeInfo
+    // with high byte in HardwareVersion.major and low byte in HardwareVersion.minor
+    uint16_t  board_id = APJ_BOARD_ID;
+    uint8_t reserved[8] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 };
 
 #if AP_SIGNED_FIRMWARE
-#define APP_DESCRIPTOR_TOTAL_LENGTH (36+72+4)
+typedef struct app_descriptor_signed app_descriptor_t;
 #else
-#define APP_DESCRIPTOR_TOTAL_LENGTH 36
+typedef struct app_descriptor_unsigned app_descriptor_t;
 #endif
 
-static_assert(sizeof(app_descriptor) == APP_DESCRIPTOR_TOTAL_LENGTH, "app_descriptor incorrect length");
+#define APP_DESCRIPTOR_UNSIGNED_TOTAL_LENGTH 36
+#define APP_DESCRIPTOR_SIGNED_TOTAL_LENGTH (APP_DESCRIPTOR_UNSIGNED_TOTAL_LENGTH+72+4)
+
+static_assert(sizeof(app_descriptor_unsigned) == APP_DESCRIPTOR_UNSIGNED_TOTAL_LENGTH, "app_descriptor_unsigned incorrect length");
+static_assert(sizeof(app_descriptor_signed) == APP_DESCRIPTOR_SIGNED_TOTAL_LENGTH, "app_descriptor_signed incorrect length");
 
 #if AP_SIGNED_FIRMWARE
 
@@ -104,13 +128,14 @@ check_fw_result_t check_good_firmware(void);
 #else
 void check_firmware_print(void);
 
-#ifdef HAL_GCS_ENABLED
 class AP_CheckFirmware {
 public:
+#if HAL_GCS_ENABLED
     // handle a message from the GCS. This is static as we don't have an AP_CheckFirmware object
     static void handle_msg(mavlink_channel_t chan, const mavlink_message_t &msg);
     static void handle_secure_command(mavlink_channel_t chan, const mavlink_secure_command_t &pkt);
     static bool check_signature(const mavlink_secure_command_t &pkt);
+#endif
     static const struct ap_secure_data *find_public_keys(void);
 
     /*
@@ -138,9 +163,10 @@ public:
     static bool check_signed_bootloader(const uint8_t *fw, uint32_t fw_size);
 
 private:
+#if HAL_GCS_ENABLED
     static uint8_t session_key[8];
-};
 #endif
+};
 
 #endif // HAL_BOOTLOADER_BUILD
 

libraries/AP_CheckFirmware/AP_CheckFirmware_secure_command.cpp

@@ -11,31 +11,6 @@
 
 extern const AP_HAL::HAL &hal;
 
-uint8_t AP_CheckFirmware::session_key[8];
-
-/*
-  make a session key
- */
-static void make_session_key(uint8_t key[8])
-{
-    struct {
-        uint32_t time_us;
-        uint8_t unique_id[12];
-        uint16_t rand1;
-        uint16_t rand2;
-    } data {};
-    static_assert(sizeof(data) % 4 == 0, "data must be multiple of 4 bytes");
-
-    // get data which will not apply on a different board, and includes some randomness
-    uint8_t uid_len = 12;
-    hal.util->get_system_id_unformatted(data.unique_id, uid_len);
-    data.time_us = AP_HAL::micros();
-    data.rand1 = get_random16();
-    data.rand2 = get_random16();
-    const uint64_t c64 = crc_crc64((const uint32_t *)&data, sizeof(data)/sizeof(uint32_t));
-    memcpy(key, (uint8_t *)&c64, 8);
-}
-
 /*
   find public keys in bootloader, or return NULL if signature not found
 
@@ -54,6 +29,24 @@ const struct ap_secure_data *AP_CheckFirmware::find_public_keys(void)
 #endif
 }
 
+/*
+  return true if all keys are zeros
+ */
+bool AP_CheckFirmware::all_zero_keys(const struct ap_secure_data *sec_data)
+{
+    const uint8_t zero_key[AP_PUBLIC_KEY_LEN] {};
+    /*
+      look over all public keys, if one matches then we are OK
+     */
+    for (const auto &public_key : sec_data->public_key) {
+        if (memcmp(public_key.key, zero_key, AP_PUBLIC_KEY_LEN) != 0) {
+            return false;
+        }
+    }
+    return true;
+}
+
+
 #if CONFIG_HAL_BOARD == HAL_BOARD_CHIBIOS
 /*
   return true if 1k of data is all 0xff (empty flash)
@@ -134,6 +127,33 @@ AP_CheckFirmware::bl_data *AP_CheckFirmware::read_bootloader(void)
 #endif
 }
 
+#if HAL_GCS_ENABLED
+uint8_t AP_CheckFirmware::session_key[8];
+
+/*
+  make a session key
+ */
+static void make_session_key(uint8_t key[8])
+{
+    struct {
+        uint32_t time_us;
+        uint8_t unique_id[12];
+        uint16_t rand1;
+        uint16_t rand2;
+    } data {};
+    static_assert(sizeof(data) % 4 == 0, "data must be multiple of 4 bytes");
+
+    // get data which will not apply on a different board, and includes some randomness
+    uint8_t uid_len = 12;
+    hal.util->get_system_id_unformatted(data.unique_id, uid_len);
+    data.time_us = AP_HAL::micros();
+    data.rand1 = get_random16();
+    data.rand2 = get_random16();
+    const uint64_t c64 = crc_crc64((const uint32_t *)&data, sizeof(data)/sizeof(uint32_t));
+    memcpy(key, (uint8_t *)&c64, 8);
+}
+
+
 /*
   write bootloader from memory
  */
@@ -163,23 +183,6 @@ bool AP_CheckFirmware::write_bootloader(const struct bl_data *bld)
 #endif
 }
 
-/*
-  return true if all keys are zeros
- */
-bool AP_CheckFirmware::all_zero_keys(const struct ap_secure_data *sec_data)
-{
-    const uint8_t zero_key[AP_PUBLIC_KEY_LEN] {};
-    /*
-      look over all public keys, if one matches then we are OK
-     */
-    for (const auto &public_key : sec_data->public_key) {
-        if (memcmp(public_key.key, zero_key, AP_PUBLIC_KEY_LEN) != 0) {
-            return false;
-        }
-    }
-    return true;
-}
-
 /*
   check signature in a command against bootloader public keys
  */
@@ -394,6 +397,8 @@ void AP_CheckFirmware::handle_msg(mavlink_channel_t chan, const mavlink_message_
     }
 }
 
+#endif // HAL_GCS_ENABLED
+
 /*
   check that a bootloader is OK to flash. We don't want to allow
   flashing of a bootloader unless we either have no public keys setup