F-7054: sign: fail when the hybrid secondary key cannot be loaded

main() checked load_key() for the primary key but not for the hybrid
secondary key, and load_key() left *pubkey/*pubkey_sz untouched (or
dangling, after the ED25519/ED448 free(*pubkey)) on its failure paths.

With a missing or undecodable secondary key file the sign tool therefore
either silently emitted a manifest with no secondary public key hashed,
dereferenced a freed pubkey buffer and double-freed it, or crashed on the
uninitialized pubkey_sz2 stack value.

Clear *pubkey/*pubkey_sz on every load_key() failure path, initialize
pubkey_sz2, and exit(1) when the secondary key fails to load.

Add tools/unit-tests/unit-sign-hybrid-keyload, covering the missing-file
and decode-failure contracts of load_key() plus the end-to-end exit
status of the sign tool.
pull/842/head
Daniele Lacamera 2026-08-04 08:13:36 +02:00
parent 0e53cac36c
commit 675a927fa2
3 changed files with 187 additions and 3 deletions

View File

@ -639,6 +639,8 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz,
/* open and load key buffer */
*key_buffer = NULL;
*pubkey = NULL;
*pubkey_sz = 0;
if (secondary) {
key_file = CMD.secondary_key_file;
sign = CMD.secondary_sign;
@ -722,8 +724,10 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz,
wc_ed25519_free(&key.ed);
}
if (ret != 0)
if (ret != 0) {
free(*pubkey);
*pubkey = NULL;
}
/* break if we succeed or are not using auto */
if (ret == 0 || sign != SIGN_AUTO) {
@ -789,8 +793,10 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz,
wc_ed448_free(&key.ed4);
}
if (ret != 0)
if (ret != 0) {
free(*pubkey);
*pubkey = NULL;
}
/* break if we succeed or are not using auto */
if (ret == 0 || sign != SIGN_AUTO) {
@ -1051,6 +1057,11 @@ failure:
zero_and_free(*key_buffer, *key_buffer_sz);
*key_buffer = NULL;
}
if (*pubkey != NULL) {
free(*pubkey);
*pubkey = NULL;
}
*pubkey_sz = 0;
return NULL;
}
@ -3728,9 +3739,12 @@ int main(int argc, char** argv)
if (CMD.hybrid) {
uint8_t *kbuf2 = NULL;
uint8_t *pubkey2 = NULL;
uint32_t pubkey_sz2;
uint32_t pubkey_sz2 = 0;
DEBUG_PRINT("Loading secondary key\n");
kbuf2 = load_key(&key_buffer2, &key_buffer_sz2, &pubkey2, &pubkey_sz2, 1);
if (!kbuf2) {
exit(1);
}
printf("Creating hybrid signature\n");
make_hybrid_header(pubkey, pubkey_sz, CMD.image_file, CMD.output_image_file,
pubkey2, pubkey_sz2);

View File

@ -61,6 +61,7 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128
unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-store-sbrk \
unit-tpm-blob unit-policy-create unit-policy-sign unit-rot-auth unit-sdhci-response-bits \
unit-sdhci-disk-unaligned unit-sign-encrypted-output \
unit-sign-hybrid-keyload \
unit-keygen-xmss-params
TESTS+=unit-tpm-check-rot-auth
TESTS+=unit-tpm-api-names
@ -323,6 +324,14 @@ unit-sign-encrypted-output: ../../include/target.h unit-sign-encrypted-output.c
-ffunction-sections -fdata-sections \
$(LDFLAGS) -Wl,--gc-sections
unit-sign-hybrid-keyload: ../../include/target.h unit-sign-hybrid-keyload.c \
$(KEYTOOLS_SIGN_SRCS)
gcc -o $@ $^ -I../keytools $(CFLAGS) -DML_DSA_LEVEL=2 -DDELTA_UPDATES \
-D"LMS_LEVELS=1" -D"LMS_HEIGHT=10" -D"LMS_WINTERNITZ=8" \
-DWOLFBOOT_XMSS_PARAMS=\"XMSS-SHA2_10_256\" \
-ffunction-sections -fdata-sections \
$(LDFLAGS) -Wl,--gc-sections
unit-keygen-xmss-params: ../../include/target.h unit-keygen-xmss-params.c
gcc -o $@ $^ -I../keytools $(CFLAGS) -DML_DSA_LEVEL=2 \
-D"LMS_LEVELS=1" -D"LMS_HEIGHT=10" -D"LMS_WINTERNITZ=8" \

View File

@ -0,0 +1,161 @@
/* unit-sign-hybrid-keyload.c
*
* Unit test for sign tool secondary (hybrid) key load error handling.
*/
#include <check.h>
#include <limits.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#define WOLFBOOT_HASH_SHA256
#define IMAGE_HEADER_SIZE 512
#define main wolfboot_sign_main
#include "../keytools/sign.c"
#undef main
static const char missing_key[] = "/nonexistent/wolfboot-secondary-key.der";
static int write_file(const char *path, const void *buf, size_t len)
{
FILE *f = fopen(path, "wb");
size_t written;
if (f == NULL) {
return -1;
}
written = fwrite(buf, 1, len, f);
fclose(f);
return written == len ? 0 : -1;
}
static void reset_cmd_defaults(void)
{
memset(&CMD, 0, sizeof(CMD));
CMD.sign = NO_SIGN;
CMD.hash_algo = HASH_SHA256;
CMD.partition_id = HDR_IMG_TYPE_APP;
CMD.header_sz = IMAGE_HEADER_SIZE;
CMD.fw_version = "1";
CMD.no_ts = 1;
}
/* load_key() must not hand back an unset public key when the key file
* cannot be opened at all. */
START_TEST(test_load_key_clears_pubkey_when_file_missing)
{
uint8_t *key_buffer = NULL;
uint32_t key_buffer_sz = 0;
uint8_t sentinel = 0xA5;
uint8_t *pubkey = &sentinel;
uint32_t pubkey_sz = 0xDEADBEEFU;
reset_cmd_defaults();
CMD.hybrid = 1;
CMD.secondary_sign = SIGN_ML_DSA;
CMD.secondary_key_file = missing_key;
ck_assert_ptr_null(load_key(&key_buffer, &key_buffer_sz, &pubkey,
&pubkey_sz, 1));
ck_assert_ptr_null(pubkey);
ck_assert_uint_eq(pubkey_sz, 0);
}
END_TEST
/* load_key() must not hand back a dangling public key pointer when the key
* file is readable but cannot be decoded. */
START_TEST(test_load_key_clears_pubkey_when_decode_fails)
{
char tempdir[] = "/tmp/wolfboot-sign-XXXXXX";
char key_path[PATH_MAX];
uint8_t garbage[7];
uint8_t *key_buffer = NULL;
uint32_t key_buffer_sz = 0;
uint8_t *pubkey = NULL;
uint32_t pubkey_sz = 0;
ck_assert_ptr_nonnull(mkdtemp(tempdir));
snprintf(key_path, sizeof(key_path), "%s/secondary.der", tempdir);
memset(garbage, 0x5A, sizeof(garbage));
ck_assert_int_eq(write_file(key_path, garbage, sizeof(garbage)), 0);
reset_cmd_defaults();
CMD.hybrid = 1;
CMD.secondary_sign = SIGN_ED25519;
CMD.secondary_key_file = key_path;
ck_assert_ptr_null(load_key(&key_buffer, &key_buffer_sz, &pubkey,
&pubkey_sz, 1));
ck_assert_ptr_null(pubkey);
ck_assert_uint_eq(pubkey_sz, 0);
unlink(key_path);
rmdir(tempdir);
}
END_TEST
/* The sign tool must fail when the hybrid secondary key cannot be loaded,
* instead of building a manifest out of an unset secondary public key. */
START_TEST(test_sign_main_fails_when_secondary_key_missing)
{
char tempdir[] = "/tmp/wolfboot-sign-XXXXXX";
char image_path[PATH_MAX];
char key_path[PATH_MAX];
uint8_t image_buf[] = { 0x01, 0x02, 0x03, 0x04 };
uint8_t raw_pubkey[64]; /* ECC256 raw Qx + Qy */
char *argv[8];
ck_assert_ptr_nonnull(mkdtemp(tempdir));
snprintf(image_path, sizeof(image_path), "%s/image.bin", tempdir);
snprintf(key_path, sizeof(key_path), "%s/ecc256.raw", tempdir);
memset(raw_pubkey, 0x11, sizeof(raw_pubkey));
ck_assert_int_eq(write_file(image_path, image_buf, sizeof(image_buf)), 0);
ck_assert_int_eq(write_file(key_path, raw_pubkey, sizeof(raw_pubkey)), 0);
argv[0] = "sign";
argv[1] = "--sha-only";
argv[2] = "--ecc256";
argv[3] = "--ml_dsa";
argv[4] = image_path;
argv[5] = key_path;
argv[6] = (char *)missing_key;
argv[7] = "1";
exit(wolfboot_sign_main(8, argv));
}
END_TEST
Suite *wolfboot_suite(void)
{
Suite *s = suite_create("sign-hybrid-keyload");
TCase *tcase = tcase_create("load-key");
tcase_add_test(tcase, test_load_key_clears_pubkey_when_file_missing);
tcase_add_test(tcase, test_load_key_clears_pubkey_when_decode_fails);
tcase_add_exit_test(tcase, test_sign_main_fails_when_secondary_key_missing,
1);
suite_add_tcase(s, tcase);
return s;
}
int main(void)
{
int failed;
Suite *s = wolfboot_suite();
SRunner *runner = srunner_create(s);
srunner_run_all(runner, CK_NORMAL);
failed = srunner_ntests_failed(runner);
srunner_free(runner);
return failed == 0 ? 0 : 1;
}