Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport v3.7.99-ncs1-branch] Fix MCUmgr support for images hashed with sha512 #2262

Merged
merged 3 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions modules/Kconfig.mcuboot
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ config MCUBOOT_BOOTLOADER_NO_DOWNGRADE
MCUBOOT_DOWNGRADE_PREVENTION option enabled.
endif

config MCUBOOT_BOOTLOADER_USES_SHA512
bool "MCUboot uses SHA512 for image hash"
help
MCUboot has been compiled to verify images using SHA512.

endmenu # On board MCUboot operation mode

endif # BOOTLOADER_MCUBOOT
Expand Down
2 changes: 1 addition & 1 deletion samples/subsys/mgmt/mcumgr/smp_svr/prj.conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CONFIG_FLASH_MAP=y

# Some command handlers require a large stack.
CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=2304
CONFIG_MAIN_STACK_SIZE=2048
CONFIG_MAIN_STACK_SIZE=2176

# Ensure an MCUboot-compatible binary is generated.
CONFIG_BOOTLOADER_MCUBOOT=y
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
extern "C" {
#endif

#ifdef CONFIG_MCUBOOT_BOOTLOADER_USES_SHA512
#define IMAGE_TLV_SHA IMAGE_TLV_SHA512
#define IMAGE_SHA_LEN 64
#else
#define IMAGE_TLV_SHA IMAGE_TLV_SHA256
#define IMAGE_SHA_LEN 32
#endif

Check notice on line 27 in subsys/mgmt/mcumgr/grp/img_mgmt/include/mgmt/mcumgr/grp/img_mgmt/img_mgmt_priv.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/mgmt/mcumgr/grp/img_mgmt/include/mgmt/mcumgr/grp/img_mgmt/img_mgmt_priv.h:27 -#define IMAGE_TLV_SHA IMAGE_TLV_SHA512 -#define IMAGE_SHA_LEN 64 +#define IMAGE_TLV_SHA IMAGE_TLV_SHA512 +#define IMAGE_SHA_LEN 64 #else -#define IMAGE_TLV_SHA IMAGE_TLV_SHA256 -#define IMAGE_SHA_LEN 32 +#define IMAGE_TLV_SHA IMAGE_TLV_SHA256 +#define IMAGE_SHA_LEN 32

/**
* @brief Ensures the spare slot (slot 1) is fully erased.
*
Expand Down
12 changes: 6 additions & 6 deletions subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ int img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
if (tlv.it_type == 0xff && tlv.it_len == 0xffff) {
return IMG_MGMT_ERR_INVALID_TLV;
}
if (tlv.it_type != IMAGE_TLV_SHA256 || tlv.it_len != IMAGE_HASH_LEN) {
if (tlv.it_type != IMAGE_TLV_SHA || tlv.it_len != IMAGE_SHA_LEN) {
/* Non-hash TLV. Skip it. */
data_off += sizeof(tlv) + tlv.it_len;
continue;
Expand All @@ -267,10 +267,10 @@ int img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,

data_off += sizeof(tlv);
if (hash != NULL) {
if (data_off + IMAGE_HASH_LEN > data_end) {
if (data_off + IMAGE_SHA_LEN > data_end) {
return IMG_MGMT_ERR_TLV_INVALID_SIZE;
}
rc = img_mgmt_read(image_slot, data_off, hash, IMAGE_HASH_LEN);
rc = img_mgmt_read(image_slot, data_off, hash, IMAGE_SHA_LEN);
if (rc != 0) {
return rc;
}
Expand Down Expand Up @@ -313,13 +313,13 @@ int
img_mgmt_find_by_hash(uint8_t *find, struct image_version *ver)
{
int i;
uint8_t hash[IMAGE_HASH_LEN];
uint8_t hash[IMAGE_SHA_LEN];

for (i = 0; i < 2 * CONFIG_MCUMGR_GRP_IMG_UPDATABLE_IMAGE_NUMBER; i++) {
if (img_mgmt_read_info(i, ver, hash, NULL) != 0) {
continue;
}
if (!memcmp(hash, find, IMAGE_HASH_LEN)) {
if (!memcmp(hash, find, IMAGE_SHA_LEN)) {
return i;
}
}
Expand Down Expand Up @@ -441,7 +441,7 @@ img_mgmt_upload_good_rsp(struct smp_streamer *ctxt)
static int
img_mgmt_upload_log(bool is_first, bool is_last, int status)
{
uint8_t hash[IMAGE_HASH_LEN];
uint8_t hash[IMAGE_SHA_LEN];
const uint8_t *hashp;
int rc;

Expand Down
8 changes: 4 additions & 4 deletions subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt_state.c
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,9 @@
zcbor_state_t *zse = ctxt->writer->zs;
uint32_t flags;
char vers_str[IMG_MGMT_VER_MAX_STR_LEN];
uint8_t hash[IMAGE_HASH_LEN]; /* SHA256 hash */
struct zcbor_string zhash = { .value = hash, .len = IMAGE_HASH_LEN };
uint8_t hash[IMAGE_SHA_LEN];
struct zcbor_string zhash = { .value = hash, .len = IMAGE_SHA_LEN};
struct image_version ver;

Check notice on line 420 in subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt_state.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt_state.c:420 - struct zcbor_string zhash = { .value = hash, .len = IMAGE_SHA_LEN}; + struct zcbor_string zhash = {.value = hash, .len = IMAGE_SHA_LEN};
bool ok;
int rc = img_mgmt_read_info(slot, &ver, hash, &flags);

Expand Down Expand Up @@ -719,14 +719,14 @@
IMG_MGMT_ERR_INVALID_HASH);
goto end;
}
} else if (zhash.len != IMAGE_HASH_LEN) {
} else if (zhash.len != IMAGE_SHA_LEN) {
/* The img_mgmt_find_by_hash does exact length compare
* so just fail here.
*/
ok = smp_add_cmd_err(zse, MGMT_GROUP_ID_IMAGE, IMG_MGMT_ERR_INVALID_HASH);
goto end;
} else {
uint8_t hash[IMAGE_HASH_LEN];
uint8_t hash[IMAGE_SHA_LEN];

memcpy(hash, zhash.value, zhash.len);

Expand Down
Loading