fix(core): change logic of vendor header comparison

Previously we checked whether the current vendor header and
the new vendor header are the same by comparing the embedded keyset.

What originally looked like a good idea is not that good, because
this disallows us from ever changing the vendor header signing keys
without causing erasure of the storage during the version update.

This commit fixes that by changing the logic to comparing just the
vendor string.

Change of function names is purely cosmetic:
* vendor_keys_hash -> vendor_header_hash
* check_vendor_keys_lock -> check_vendor_header_lock
pull/2046/head
Pavol Rusnak 3 years ago committed by Andrew Kozlik
parent 572f3eda20
commit 35d40cc164

@ -0,0 +1 @@
Update logic of vendor header comparison.

@ -191,9 +191,9 @@ secbool load_vendor_header_keys(const uint8_t *const data,
BOOTLOADER_KEYS, vhdr);
}
static secbool check_vendor_keys_lock(const vendor_header *const vhdr) {
static secbool check_vendor_header_lock(const vendor_header *const vhdr) {
uint8_t lock[FLASH_OTP_BLOCK_SIZE];
ensure(flash_otp_read(FLASH_OTP_BLOCK_VENDOR_KEYS_LOCK, 0, lock,
ensure(flash_otp_read(FLASH_OTP_BLOCK_VENDOR_HEADER_LOCK, 0, lock,
FLASH_OTP_BLOCK_SIZE),
NULL);
if (0 ==
@ -204,7 +204,7 @@ static secbool check_vendor_keys_lock(const vendor_header *const vhdr) {
return sectrue;
}
uint8_t hash[32];
vendor_keys_hash(vhdr, hash);
vendor_header_hash(vhdr, hash);
return sectrue * (0 == memcmp(lock, hash, 32));
}
@ -269,7 +269,7 @@ int main(void) {
secbool firmware_present =
load_vendor_header_keys((const uint8_t *)FIRMWARE_START, &vhdr);
if (sectrue == firmware_present) {
firmware_present = check_vendor_keys_lock(&vhdr);
firmware_present = check_vendor_header_lock(&vhdr);
}
if (sectrue == firmware_present) {
firmware_present = load_image_header(
@ -328,7 +328,7 @@ int main(void) {
ensure(load_vendor_header_keys((const uint8_t *)FIRMWARE_START, &vhdr),
"invalid vendor header");
ensure(check_vendor_keys_lock(&vhdr), "unauthorized vendor keys");
ensure(check_vendor_header_lock(&vhdr), "unauthorized vendor keys");
ensure(load_image_header((const uint8_t *)(FIRMWARE_START + vhdr.hdrlen),
FIRMWARE_IMAGE_MAGIC, FIRMWARE_IMAGE_MAXSIZE,

@ -295,7 +295,7 @@ static void send_msg_features(uint8_t iface_num,
MSG_SEND_ASSIGN_VALUE(fw_patch, ((hdr->version >> 16) & 0xFF));
MSG_SEND_ASSIGN_STRING_LEN(fw_vendor, vhdr->vstr, vhdr->vstr_len);
uint8_t hash[32];
vendor_keys_hash(vhdr, hash);
vendor_header_hash(vhdr, hash);
MSG_SEND_ASSIGN_BYTES(fw_vendor_keys, hash, 32);
} else {
MSG_SEND_ASSIGN_VALUE(firmware_present, false);
@ -446,8 +446,8 @@ static void detect_installation(vendor_header *current_vhdr,
return;
}
uint8_t hash1[32], hash2[32];
vendor_keys_hash(new_vhdr, hash1);
vendor_keys_hash(current_vhdr, hash2);
vendor_header_hash(new_vhdr, hash1);
vendor_header_hash(current_vhdr, hash2);
if (0 != memcmp(hash1, hash2, 32)) {
return;
}

@ -0,0 +1 @@
Update logic of vendor header comparison.

@ -169,9 +169,9 @@ secbool load_vendor_header_keys(const uint8_t *const data,
BOOTLOADER_KEYS, vhdr);
}
static secbool check_vendor_keys_lock(const vendor_header *const vhdr) {
static secbool check_vendor_header_lock(const vendor_header *const vhdr) {
uint8_t lock[FLASH_OTP_BLOCK_SIZE];
ensure(flash_otp_read(FLASH_OTP_BLOCK_VENDOR_KEYS_LOCK, 0, lock,
ensure(flash_otp_read(FLASH_OTP_BLOCK_VENDOR_HEADER_LOCK, 0, lock,
FLASH_OTP_BLOCK_SIZE),
NULL);
if (0 ==
@ -182,7 +182,7 @@ static secbool check_vendor_keys_lock(const vendor_header *const vhdr) {
return sectrue;
}
uint8_t hash[32];
vendor_keys_hash(vhdr, hash);
vendor_header_hash(vhdr, hash);
return sectrue * (0 == memcmp(lock, hash, 32));
}
@ -235,7 +235,7 @@ int main(void) {
secbool firmware_present =
load_vendor_header_keys((const uint8_t *)FIRMWARE_START, &vhdr);
if (sectrue == firmware_present) {
firmware_present = check_vendor_keys_lock(&vhdr);
firmware_present = check_vendor_header_lock(&vhdr);
}
if (sectrue == firmware_present) {
firmware_present = load_image_header(
@ -263,7 +263,7 @@ int main(void) {
ensure(load_vendor_header_keys((const uint8_t *)FIRMWARE_START, &vhdr),
"invalid vendor header");
ensure(check_vendor_keys_lock(&vhdr), "unauthorized vendor keys");
ensure(check_vendor_header_lock(&vhdr), "unauthorized vendor keys");
ensure(load_image_header((const uint8_t *)(FIRMWARE_START + vhdr.hdrlen),
FIRMWARE_IMAGE_MAGIC, FIRMWARE_IMAGE_MAXSIZE,

@ -295,7 +295,7 @@ static void send_msg_features(uint8_t iface_num,
MSG_SEND_ASSIGN_VALUE(fw_patch, ((hdr->version >> 16) & 0xFF));
MSG_SEND_ASSIGN_STRING_LEN(fw_vendor, vhdr->vstr, vhdr->vstr_len);
uint8_t hash[32];
vendor_keys_hash(vhdr, hash);
vendor_header_hash(vhdr, hash);
MSG_SEND_ASSIGN_BYTES(fw_vendor_keys, hash, 32);
} else {
MSG_SEND_ASSIGN_VALUE(firmware_present, false);
@ -446,8 +446,8 @@ static void detect_installation(vendor_header *current_vhdr,
return;
}
uint8_t hash1[32], hash2[32];
vendor_keys_hash(new_vhdr, hash1);
vendor_keys_hash(current_vhdr, hash2);
vendor_header_hash(new_vhdr, hash1);
vendor_header_hash(current_vhdr, hash2);
if (0 != memcmp(hash1, hash2, 32)) {
return;
}

@ -110,7 +110,7 @@ secbool __wur flash_write_word(uint8_t sector, uint32_t offset, uint32_t data);
// OTP blocks allocation
#define FLASH_OTP_BLOCK_BATCH 0
#define FLASH_OTP_BLOCK_BOOTLOADER_VERSION 1
#define FLASH_OTP_BLOCK_VENDOR_KEYS_LOCK 2
#define FLASH_OTP_BLOCK_VENDOR_HEADER_LOCK 2
#define FLASH_OTP_BLOCK_RANDOMNESS 3
secbool __wur flash_otp_read(uint8_t block, uint8_t offset, uint8_t *data,

@ -161,22 +161,11 @@ secbool load_vendor_header(const uint8_t *const data, uint8_t key_m,
*(const ed25519_signature *)vhdr->sig));
}
void vendor_keys_hash(const vendor_header *const vhdr, uint8_t *hash) {
void vendor_header_hash(const vendor_header *const vhdr, uint8_t *hash) {
BLAKE2S_CTX ctx;
blake2s_Init(&ctx, BLAKE2S_DIGEST_LENGTH);
blake2s_Update(&ctx, &(vhdr->vsig_m), sizeof(vhdr->vsig_m));
blake2s_Update(&ctx, &(vhdr->vsig_n), sizeof(vhdr->vsig_n));
for (int i = 0; i < MAX_VENDOR_PUBLIC_KEYS; i++) {
if (vhdr->vpub[i] != 0) {
blake2s_Update(&ctx, vhdr->vpub[i], 32);
} else {
blake2s_Update(
&ctx,
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
32);
}
}
blake2s_Update(&ctx, vhdr->vstr, vhdr->vstr_len);
blake2s_Update(&ctx, "Trezor Vendor Header", 20);
blake2s_Final(&ctx, hash, BLAKE2S_DIGEST_LENGTH);
}

@ -87,7 +87,7 @@ secbool __wur load_vendor_header(const uint8_t *const data, uint8_t key_m,
uint8_t key_n, const uint8_t *const *keys,
vendor_header *const vhdr);
void vendor_keys_hash(const vendor_header *const vhdr, uint8_t *hash);
void vendor_header_hash(const vendor_header *const vhdr, uint8_t *hash);
secbool __wur check_single_hash(const uint8_t *const hash,
const uint8_t *const data, int len);

Loading…
Cancel
Save