diff --git a/.gitignore b/.gitignore index ae120f7c65..f1a8800c92 100644 --- a/.gitignore +++ b/.gitignore @@ -170,6 +170,7 @@ tools/unit-tests/unit-image-sha384 tools/unit-tests/unit-store-sbrk tools/unit-tests/unit-tpm-blob tools/unit-tests/unit-update-disk +tools/unit-tests/unit-policy-sign diff --git a/src/boot_arm32.c b/src/boot_arm32.c index 9a999dda8f..e7d1f47319 100644 --- a/src/boot_arm32.c +++ b/src/boot_arm32.c @@ -86,7 +86,7 @@ void RAMFUNCTION do_boot(const uint32_t *app_offset) #ifdef RAM_CODE #define AIRCR *(volatile uint32_t *)(0xE000ED0C) -#define AIRCR_VKEY (0r05FA << 16) +#define AIRCR_VKEY (0x05FA << 16) #define AIRCR_SYSRESETREQ (1 << 2) void RAMFUNCTION arch_reboot(void) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index 2f35d4649d..121b01d4d5 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -436,9 +436,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, void wolfPKCS11_Store_Close(void* store) { struct store_handle *handle = store; - /* This removes all flags (including STORE_FLAGS_OPEN) */ - handle->flags = 0; - handle->hdr = NULL; + memset(handle, 0, sizeof(*handle)); } int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) diff --git a/src/psa_store.c b/src/psa_store.c index a2ed5a701b..a5e7adb4ce 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -442,9 +442,7 @@ int wolfPSA_Store_OpenSz(int type, unsigned long id1, unsigned long id2, int rea void wolfPSA_Store_Close(void* store) { struct store_handle *handle = store; - /* This removes all flags (including STORE_FLAGS_OPEN) */ - handle->flags = 0; - handle->hdr = NULL; + memset(handle, 0, sizeof(*handle)); } int wolfPSA_Store_Read(void* store, unsigned char* buffer, int len) diff --git a/src/update_disk.c b/src/update_disk.c index 3fb45554b6..6d1b7d0514 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -221,6 +221,11 @@ static void disk_crypto_clear(void) ForceZero(disk_encrypt_nonce, sizeof(disk_encrypt_nonce)); } +static void disk_decrypted_header_clear(uint8_t *hdr) +{ + ForceZero(hdr, IMAGE_HEADER_SIZE); +} + #endif /* DISK_ENCRYPT */ extern int wolfBoot_get_dts_size(void *dts_addr); @@ -267,12 +272,14 @@ void RAMFUNCTION wolfBoot_start(void) #ifdef DISK_ENCRYPT /* Initialize encryption - this sets up the cipher with key from storage */ if (wolfBoot_initialize_encryption() != 0) { + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); wolfBoot_printf("Error initializing encryption\r\n"); wolfBoot_panic(); } /* Retrieve encryption key and nonce for disk decryption */ if (wolfBoot_get_encrypt_key(disk_encrypt_key, disk_encrypt_nonce) != 0) { + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); wolfBoot_printf("Error getting encryption key\r\n"); wolfBoot_panic(); @@ -283,6 +290,7 @@ void RAMFUNCTION wolfBoot_start(void) ret = disk_init(BOOT_DISK); if (ret != 0) { #ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); #endif wolfBoot_panic(); @@ -290,6 +298,7 @@ void RAMFUNCTION wolfBoot_start(void) if (disk_open(BOOT_DISK) < 0) { #ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); #endif wolfBoot_printf("Error opening disk %d\r\n", BOOT_DISK); @@ -328,6 +337,7 @@ void RAMFUNCTION wolfBoot_start(void) if ((pB_ver == 0) && (pA_ver == 0)) { #ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); #endif wolfBoot_printf("No valid OS image found in either partition %d or %d\r\n", @@ -433,6 +443,7 @@ void RAMFUNCTION wolfBoot_start(void) wolfBoot_printf("Decrypting image..."); BENCHMARK_START(); if ((IMAGE_HEADER_SIZE % ENCRYPT_BLOCK_SIZE) != 0) { + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); wolfBoot_printf("Encrypted disk images require aligned header size\r\n"); wolfBoot_panic(); @@ -482,6 +493,7 @@ void RAMFUNCTION wolfBoot_start(void) if (failures) { #ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); #endif wolfBoot_printf("Unable to find a valid partition!\r\n"); @@ -542,6 +554,7 @@ void RAMFUNCTION wolfBoot_start(void) wolfBoot_hook_boot(&os_image); #endif #ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); disk_crypto_clear(); #endif do_boot((uint32_t*)load_address diff --git a/src/update_flash.c b/src/update_flash.c index 5d3ed29169..c32c3002fa 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -1241,7 +1241,6 @@ int wolfBoot_unlock_disk(void) ret = wolfBoot_get_random(secret, secretSz); if (ret == 0) { wolfBoot_printf("Creating new secret (%d bytes)\n", secretSz); - wolfBoot_print_hexstr(secret, secretSz, 0); /* seal new secret */ ret = wolfBoot_seal(pubkey_hint, policy, policySz, nvIndex, @@ -1265,7 +1264,6 @@ int wolfBoot_unlock_disk(void) } wolfBoot_printf("Secret Check %d bytes\n", secretCheckSz); - wolfBoot_print_hexstr(secretCheck, secretCheckSz, 0); TPM2_ForceZero(secretCheck, sizeof(secretCheck)); } } @@ -1273,7 +1271,6 @@ int wolfBoot_unlock_disk(void) if (ret == 0) { wolfBoot_printf("Secret %d bytes\n", secretSz); - wolfBoot_print_hexstr(secret, secretSz, 0); /* TODO: Unlock disk */ diff --git a/src/x86/ahci.c b/src/x86/ahci.c index f00c7029de..ff5a7a42a2 100644 --- a/src/x86/ahci.c +++ b/src/x86/ahci.c @@ -281,7 +281,6 @@ static int sata_create_and_seal_unlock_secret(const uint8_t *pubkey_hint, ret = sata_get_random_base64(secret, secret_size); if (ret == 0) { wolfBoot_printf("Creating new secret (%d bytes)\r\n", *secret_size); - wolfBoot_printf("%s\r\n", secret); /* seal new secret */ ret = wolfBoot_seal(pubkey_hint, policy, policy_size, @@ -305,14 +304,11 @@ static int sata_create_and_seal_unlock_secret(const uint8_t *pubkey_hint, } wolfBoot_printf("Secret Check %d bytes\n", secret_check_sz); - wolfBoot_printf("%s\r\n", secret_check); TPM2_ForceZero(secret_check, sizeof(secret_check)); } - if (ret == 0) { + if (ret == 0) wolfBoot_printf("Secret %d bytes\n", *secret_size); - wolfBoot_printf("%s\r\n", secret); - } return ret; } @@ -414,9 +410,6 @@ int sata_unlock_disk(int drv, int freeze) r = sata_get_unlock_secret(secret, &secret_size); if (r != 0) return r; -#ifdef TARGET_x86_fsp_qemu - wolfBoot_printf("DISK LOCK SECRET: %s\r\n", secret); -#endif ata_st = ata_security_get_state(drv); wolfBoot_printf("ATA: Security state SEC%d\r\n", ata_st); #if defined(TARGET_x86_fsp_qemu) diff --git a/tools/elf-parser/elf-parser.c b/tools/elf-parser/elf-parser.c index cdf580f031..e50973c067 100644 --- a/tools/elf-parser/elf-parser.c +++ b/tools/elf-parser/elf-parser.c @@ -67,7 +67,9 @@ int main(int argc, char *argv[]) ret = -1; } } - fclose(f); + if (f != NULL) { + fclose(f); + } if (ret == 0) { ret = elf_load_image_mmu(image, (uint32_t)imageSz, &entry, NULL); diff --git a/tools/fdt-parser/fdt-parser.c b/tools/fdt-parser/fdt-parser.c index c33c96d5ea..cc1c3080ab 100644 --- a/tools/fdt-parser/fdt-parser.c +++ b/tools/fdt-parser/fdt-parser.c @@ -192,10 +192,18 @@ static int fdt_test(void* fdt) off = fdt_node_offset_by_compatible(fdt, -1, "fsl,qman-portal"); while (off != -FDT_ERR_NOTFOUND) { const int *ci = fdt_getprop(fdt, off, "cell-index", NULL); + uint32_t portal_idx; uint32_t liodns[2]; if (!ci) break; - i = fdt32_to_cpu(*ci); + portal_idx = fdt32_to_cpu(*ci); + if (portal_idx >= QMAN_NUM_PORTALS) { + printf("FDT: Invalid qman-portal cell-index %u at %d\n", + portal_idx, off); + ret = -FDT_ERR_BADSTRUCTURE; + goto exit; + } + i = (int)portal_idx; liodns[0] = qp_info[i].dliodn; liodns[1] = qp_info[i].fliodn; diff --git a/tools/tpm/policy_sign.c b/tools/tpm/policy_sign.c index f64c929671..80ee12d182 100644 --- a/tools/tpm/policy_sign.c +++ b/tools/tpm/policy_sign.c @@ -287,7 +287,7 @@ int policy_sign(int argc, char *argv[]) pcrDigestSz = -1; else pcrDigestSz = hexToByte(hashHexStr, pcrDigest, hashHexStrlen); - if (pcrDigestSz <= 0) { + if ((int)pcrDigestSz <= 0) { fprintf(stderr, "Invalid PCR hash length\n"); usage(); return -1; @@ -300,7 +300,7 @@ int policy_sign(int argc, char *argv[]) digestSz = -1; else digestSz = hexToByte(hashHexStr, digest, hashHexStrlen); - if (digestSz <= 0) { + if ((int)digestSz <= 0) { fprintf(stderr, "Invalid Policy Digest hash length\n"); usage(); return -1; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 667820d9f1..333845c259 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -49,7 +49,7 @@ TESTS:=unit-parser unit-extflash unit-string unit-spi-flash unit-aes128 \ unit-update-flash-enc unit-update-ram unit-pkcs11_store unit-psa_store unit-disk \ unit-update-disk unit-multiboot unit-boot-x86-fsp unit-qspi-flash unit-tpm-rsa-exp \ unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-store-sbrk \ - unit-tpm-blob + unit-tpm-blob unit-policy-sign all: $(TESTS) @@ -132,6 +132,13 @@ unit-tpm-blob: ../../include/target.h unit-tpm-blob.c -DWOLFBOOT_HASH_SHA256 \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +unit-policy-sign: ../../include/target.h unit-policy-sign.c \ + $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/memory.c + gcc -o $@ $^ -I../tpm $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ + -DWOLFTPM_USER_SETTINGS -DWOLFBOOT_SIGN_ECC256 -DWOLFBOOT_HASH_SHA256 \ + -DHAVE_ECC_KEY_IMPORT \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + unit-store-sbrk: unit-store-sbrk.c ../../src/store_sbrk.c gcc -o $@ $^ $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 2cf501d7fd..4c5c9b6a4f 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -318,6 +318,39 @@ START_TEST(test_cross_sector_write_preserves_length) } END_TEST +START_TEST(test_close_clears_handle_state) +{ + const int type = DYNAMIC_TYPE_RSA; + const CK_ULONG id_tok = 17; + const CK_ULONG id_obj = 21; + void *store = NULL; + struct store_handle *handle; + int ret; + + ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store); + ck_assert_int_eq(ret, 0); + ck_assert_ptr_nonnull(store); + + handle = store; + ck_assert_ptr_nonnull(handle->buffer); + ck_assert_ptr_nonnull(handle->hdr); + ck_assert_uint_ne(handle->in_buffer_offset, 0); + + wolfPKCS11_Store_Close(store); + + ck_assert_uint_eq(handle->flags, 0); + ck_assert_uint_eq(handle->pos, 0); + ck_assert_ptr_null(handle->buffer); + ck_assert_ptr_null(handle->hdr); + ck_assert_uint_eq(handle->in_buffer_offset, 0); +} +END_TEST + START_TEST(test_delete_object_ignores_metadata_prefix) { const int32_t type = DYNAMIC_TYPE_RSA; @@ -356,12 +389,15 @@ Suite *wolfboot_suite(void) TCase* tcase_store_and_load_objs = tcase_create("store_and_load_objs"); TCase* tcase_cross_sector_write = tcase_create("cross_sector_write"); + TCase* tcase_close = tcase_create("close_state"); TCase* tcase_delete_object = tcase_create("delete_object"); tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs); tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length); + tcase_add_test(tcase_close, test_close_clears_handle_state); tcase_add_test(tcase_delete_object, test_delete_object_ignores_metadata_prefix); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); + suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete_object); return s; } diff --git a/tools/unit-tests/unit-policy-sign.c b/tools/unit-tests/unit-policy-sign.c new file mode 100644 index 0000000000..aec5f1d02a --- /dev/null +++ b/tools/unit-tests/unit-policy-sign.c @@ -0,0 +1,230 @@ +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include "tpm.h" + +#ifdef wc_InitRng +#undef wc_InitRng +#endif +#ifdef wc_FreeRng +#undef wc_FreeRng +#endif + +static int policy_pcr_make_calls; +static int policy_ref_make_calls; +static int hash_digest_size_calls; +static int rng_init_calls; + +int wolfTPM2_PolicyPCRMake(TPM_ALG_ID pcrAlg, byte* pcrArray, word32 pcrArraySz, + const byte* pcrDigest, word32 pcrDigestSz, byte* digest, word32* digestSz) +{ + (void)pcrAlg; + (void)pcrArray; + (void)pcrArraySz; + (void)pcrDigest; + (void)pcrDigestSz; + (void)digest; + (void)digestSz; + policy_pcr_make_calls++; + return -200; +} + +int wolfTPM2_PolicyRefMake(TPM_ALG_ID pcrAlg, byte* digest, word32* digestSz, + const byte* policyRef, word32 policyRefSz) +{ + (void)pcrAlg; + (void)digest; + (void)digestSz; + (void)policyRef; + (void)policyRefSz; + policy_ref_make_calls++; + return -201; +} + +int TPM2_GetHashDigestSize(TPMI_ALG_HASH hashAlg) +{ + (void)hashAlg; + hash_digest_size_calls++; + return 32; +} + +const char* TPM2_GetAlgName(TPM_ALG_ID alg) +{ + (void)alg; + return "SHA256"; +} + +const char* wolfTPM2_GetRCString(int rc) +{ + (void)rc; + return "stub"; +} + +int wc_InitRng(WC_RNG* rng) +{ + (void)rng; + rng_init_calls++; + return -202; +} + +int wc_FreeRng(WC_RNG* rng) +{ + (void)rng; + return 0; +} + +int wc_ecc_init(ecc_key* key) +{ + (void)key; + return 0; +} + +int wc_ecc_free(ecc_key* key) +{ + (void)key; + return 0; +} + +int wc_ecc_import_unsigned(ecc_key* key, const byte* qx, const byte* qy, + const byte* d, int curve_id) +{ + (void)key; + (void)qx; + (void)qy; + (void)d; + (void)curve_id; + return 0; +} + +int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng, ecc_key* key, + mp_int* r, mp_int* s) +{ + (void)in; + (void)inlen; + (void)rng; + (void)key; + (void)r; + (void)s; + return 0; +} + +int mp_init_multi(mp_int* mp1, mp_int* mp2, mp_int* mp3, mp_int* mp4, + mp_int* mp5, mp_int* mp6) +{ + (void)mp1; + (void)mp2; + (void)mp3; + (void)mp4; + (void)mp5; + (void)mp6; + return 0; +} + +int sp_unsigned_bin_size(const sp_int* a) +{ + (void)a; + return 0; +} + +int sp_to_unsigned_bin(const sp_int* a, byte* out) +{ + (void)a; + (void)out; + return 0; +} + +void mp_clear(mp_int* a) +{ + (void)a; +} + +#define main policy_sign_tool_main +#include "../tpm/policy_sign.c" +#undef main + +static void setup(void) +{ + policy_pcr_make_calls = 0; + policy_ref_make_calls = 0; + hash_digest_size_calls = 0; + rng_init_calls = 0; +} + +static void make_oversized_hex_arg(char* dst, size_t dst_sz, const char* prefix) +{ + size_t prefix_len = strlen(prefix); + size_t hex_len = (WC_MAX_DIGEST_SIZE * 2U) + 2U; + + ck_assert_uint_gt(dst_sz, prefix_len + hex_len); + memcpy(dst, prefix, prefix_len); + memset(dst + prefix_len, 'a', hex_len); + dst[prefix_len + hex_len] = '\0'; +} + +START_TEST(test_policy_sign_rejects_oversized_pcr_digest) +{ + char arg[sizeof("-pcrdigest=") + (WC_MAX_DIGEST_SIZE * 2) + 3]; + char* argv[] = { (char*)"policy_sign", arg }; + int rc; + + make_oversized_hex_arg(arg, sizeof(arg), "-pcrdigest="); + rc = policy_sign(2, argv); + + ck_assert_int_eq(rc, -1); + ck_assert_int_eq(hash_digest_size_calls, 0); + ck_assert_int_eq(policy_pcr_make_calls, 0); + ck_assert_int_eq(policy_ref_make_calls, 0); + ck_assert_int_eq(rng_init_calls, 0); +} +END_TEST + +START_TEST(test_policy_sign_rejects_invalid_policy_digest_hex) +{ + char arg[] = "-policydigest=zz"; + char* argv[] = { (char*)"policy_sign", arg }; + int rc; + + rc = policy_sign(2, argv); + + ck_assert_int_eq(rc, -1); + ck_assert_int_eq(hash_digest_size_calls, 0); + ck_assert_int_eq(policy_pcr_make_calls, 0); + ck_assert_int_eq(policy_ref_make_calls, 0); + ck_assert_int_eq(rng_init_calls, 0); +} +END_TEST + +static Suite* policy_sign_suite(void) +{ + Suite* s; + TCase* tc; + + s = suite_create("policy_sign"); + tc = tcase_create("argument_validation"); + tcase_add_checked_fixture(tc, setup, NULL); + tcase_add_test(tc, test_policy_sign_rejects_oversized_pcr_digest); + tcase_add_test(tc, test_policy_sign_rejects_invalid_policy_digest_hex); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + Suite* s; + SRunner* sr; + int failed; + + s = policy_sign_suite(); + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index 91f8f9ebb3..92acbf9a01 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -90,6 +90,39 @@ START_TEST(test_cross_sector_write_preserves_length) } END_TEST +START_TEST(test_close_clears_handle_state) +{ + enum { type = WOLFPSA_STORE_KEY }; + const unsigned long id1 = 17; + const unsigned long id2 = 21; + void *store = NULL; + struct store_handle *handle; + int ret; + + ret = mmap_file("/tmp/wolfboot-unit-psa-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + ret = wolfPSA_Store_Open(type, id1, id2, 0, &store); + ck_assert_int_eq(ret, 0); + ck_assert_ptr_nonnull(store); + + handle = store; + ck_assert_ptr_nonnull(handle->buffer); + ck_assert_ptr_nonnull(handle->hdr); + ck_assert_uint_ne(handle->in_buffer_offset, 0); + + wolfPSA_Store_Close(store); + + ck_assert_uint_eq(handle->flags, 0); + ck_assert_uint_eq(handle->pos, 0); + ck_assert_ptr_null(handle->buffer); + ck_assert_ptr_null(handle->hdr); + ck_assert_uint_eq(handle->in_buffer_offset, 0); +} +END_TEST + START_TEST(test_delete_object_ignores_metadata_prefix) { enum { type = WOLFPSA_STORE_KEY }; @@ -125,11 +158,14 @@ Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot-psa-store"); TCase *tcase_write = tcase_create("cross_sector_write"); + TCase *tcase_close = tcase_create("close_state"); TCase *tcase_delete = tcase_create("delete_object"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); + tcase_add_test(tcase_close, test_close_clears_handle_state); tcase_add_test(tcase_delete, test_delete_object_ignores_metadata_prefix); suite_add_tcase(s, tcase_write); + suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); return s; }