From 39595d36befa335c2351f8b1af348060e2d75aec Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 3 Sep 2022 12:21:51 +1000 Subject: [PATCH] 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 --- .../AP_CheckFirmware/AP_CheckFirmware.cpp | 128 ++++++++++++++---- libraries/AP_CheckFirmware/AP_CheckFirmware.h | 58 +++++--- .../AP_CheckFirmware_secure_command.cpp | 89 ++++++------ 3 files changed, 192 insertions(+), 83 deletions(-) diff --git a/libraries/AP_CheckFirmware/AP_CheckFirmware.cpp b/libraries/AP_CheckFirmware/AP_CheckFirmware.cpp index 237d149300..ffd2c3cb45 100644 --- a/libraries/AP_CheckFirmware/AP_CheckFirmware.cpp +++ b/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] = { 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 sig[8] = AP_APP_DESCRIPTOR_SIGNATURE_SIGNED; 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 /* diff --git a/libraries/AP_CheckFirmware/AP_CheckFirmware.h b/libraries/AP_CheckFirmware/AP_CheckFirmware.h index f161a0563e..cf0837d20c 100644 --- a/libraries/AP_CheckFirmware/AP_CheckFirmware.h +++ b/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 - uint8_t sig[8] = { 0x41, 0xa3, 0xe5, 0xf2, 0x65, 0x69, 0x92, 0x07 }; -#else - uint8_t sig[8] = { 0x40, 0xa2, 0xe4, 0xf1, 0x64, 0x68, 0x91, 0x06 }; -#endif + +#define AP_APP_DESCRIPTOR_SIGNATURE_SIGNED { 0x41, 0xa3, 0xe5, 0xf2, 0x65, 0x69, 0x92, 0x07 } +#define AP_APP_DESCRIPTOR_SIGNATURE_UNSIGNED { 0x40, 0xa2, 0xe4, 0xf1, 0x64, 0x68, 0x91, 0x06 } + +struct app_descriptor_unsigned { + 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 diff --git a/libraries/AP_CheckFirmware/AP_CheckFirmware_secure_command.cpp b/libraries/AP_CheckFirmware/AP_CheckFirmware_secure_command.cpp index 1a9a59b905..d6578d8c5a 100644 --- a/libraries/AP_CheckFirmware/AP_CheckFirmware_secure_command.cpp +++ b/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