From ae8198ba365d7f04056fd85bbcf2105647132062 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 21 Jul 2026 17:00:34 +0900 Subject: [PATCH] Translate the key wrap metadata trailer --- src/wh_message_nvm.c | 20 +++ src/wh_server_keystore.c | 100 ++++++++++---- .../misc/wh_test_check_struct_padding.c | 2 + test-refactor/misc/wh_test_message_nvm.c | 128 ++++++++++++++++++ test-refactor/wh_test_list.c | 2 + wolfhsm/wh_message_nvm.h | 5 + 6 files changed, 229 insertions(+), 28 deletions(-) create mode 100644 test-refactor/misc/wh_test_message_nvm.c diff --git a/src/wh_message_nvm.c b/src/wh_message_nvm.c index faaf3a0d5..304cdb18c 100644 --- a/src/wh_message_nvm.c +++ b/src/wh_message_nvm.c @@ -36,6 +36,24 @@ #include "wolfhsm/wh_message_nvm.h" +/* Keep in step with wh_MessageNvm_TranslateAddObjectRequest */ +int wh_MessageNvm_TranslateMetadata(uint16_t magic, const whNvmMetadata* src, + whNvmMetadata* dest) +{ + if ((src == NULL) || (dest == NULL)) { + return WH_ERROR_BADARGS; + } + WH_T16(magic, dest, src, id); + WH_T16(magic, dest, src, access); + WH_T16(magic, dest, src, flags); + WH_T16(magic, dest, src, len); + /* Label is just a byte array, no translation needed */ + if (src != dest) { + memcpy(dest->label, src->label, sizeof(dest->label)); + } + return 0; +} + int wh_MessageNvm_TranslateSimpleResponse(uint16_t magic, const whMessageNvm_SimpleResponse* src, whMessageNvm_SimpleResponse* dest) @@ -141,6 +159,8 @@ int wh_MessageNvm_TranslateGetMetadataResponse(uint16_t magic, return 0; } +/* The request mirrors whNvmMetadata: keep in step with + * wh_MessageNvm_TranslateMetadata */ int wh_MessageNvm_TranslateAddObjectRequest(uint16_t magic, const whMessageNvm_AddObjectRequest* src, whMessageNvm_AddObjectRequest* dest) diff --git a/src/wh_server_keystore.c b/src/wh_server_keystore.c index d5385ce75..aa6a3bc2b 100644 --- a/src/wh_server_keystore.c +++ b/src/wh_server_keystore.c @@ -40,6 +40,7 @@ #include "wolfhsm/wh_error.h" #include "wolfhsm/wh_message.h" #include "wolfhsm/wh_message_keystore.h" +#include "wolfhsm/wh_message_nvm.h" /* For wh_MessageNvm_TranslateMetadata */ #include "wolfhsm/wh_utils.h" #include "wolfhsm/wh_server.h" #include "wolfhsm/wh_log.h" @@ -1830,6 +1831,7 @@ static int _AesGcmDataUnwrap(whServerContext* server, uint16_t serverKeyId, #endif /* !NO_AES */ static int _HandleKeyWrapRequest(whServerContext* server, + uint16_t magic, whMessageKeystore_KeyWrapRequest* req, uint8_t* reqData, uint32_t reqDataSz, whMessageKeystore_KeyWrapResponse* resp, @@ -1849,20 +1851,24 @@ static int _HandleKeyWrapRequest(whServerContext* server, return WH_ERROR_BADARGS; } + /* Set before any failure exit: the client checks cipherType before rc */ + resp->cipherType = req->cipherType; + resp->wrappedKeySz = 0; + /* Check if the reqData is big enough to hold the metadata and key */ if (reqDataSz < sizeof(metadata) + req->keySz) { return WH_ERROR_BUFFER_SIZE; } - /* Extract the metadata and key from reqData */ + /* Extract the metadata and key. The metadata trailer arrives in the + * client's byte order, so translate before any field is used */ memcpy(&metadata, reqData, sizeof(metadata)); + ret = wh_MessageNvm_TranslateMetadata(magic, &metadata, &metadata); + if (ret != WH_ERROR_OK) { + return ret; + } memcpy(key, reqData + sizeof(metadata), req->keySz); - /* Ensure the cipher type in the response matches the request */ - resp->cipherType = req->cipherType; - /* Wrapped key size is only passed back to the client on success */ - resp->wrappedKeySz = 0; - /* Ensure the keyId in the wrapped metadata has the wrapped flag set */ if (!WH_KEYID_ISWRAPPED(metadata.id)) { WH_LOG_F(&server->log, WH_LOG_LEVEL_ERROR, @@ -2037,16 +2043,18 @@ _HandleKeyWrapExportRequest(whServerContext* server, } static int _HandleKeyUnwrapAndExportRequest( - whServerContext* server, whMessageKeystore_KeyUnwrapAndExportRequest* req, - uint8_t* reqData, uint32_t reqDataSz, - whMessageKeystore_KeyUnwrapAndExportResponse* resp, uint8_t* respData, - uint32_t respDataSz) + whServerContext* server, uint16_t magic, + whMessageKeystore_KeyUnwrapAndExportRequest* req, uint8_t* reqData, + uint32_t reqDataSz, whMessageKeystore_KeyUnwrapAndExportResponse* resp, + uint8_t* respData, uint32_t respDataSz) { - int ret; + /* Defensive: a case that never assigns ret cannot report success */ + int ret = WH_ERROR_BADARGS; uint8_t* wrappedKey; whNvmMetadata* metadata; uint8_t* key; whKeyId serverKeyId; + uint16_t keySz = 0; if (server == NULL || req == NULL || reqData == NULL || resp == NULL || respData == NULL) { @@ -2080,11 +2088,13 @@ static int _HandleKeyUnwrapAndExportRequest( #ifndef NO_AES #ifdef HAVE_AESGCM case WC_CIPHER_AES_GCM: { - uint16_t keySz; + uint16_t wrappedKeyUser = 0; + uint16_t wrappedKeyType = 0; if (req->wrappedKeySz < WH_KEYWRAP_AES_GCM_HEADER_SIZE + sizeof(*metadata)) { - return WH_ERROR_BADARGS; + ret = WH_ERROR_BADARGS; + break; } keySz = req->wrappedKeySz - @@ -2092,7 +2102,8 @@ static int _HandleKeyUnwrapAndExportRequest( /* Check if the response data can fit the metadata + key */ if (respDataSz < sizeof(*metadata) + keySz) { - return WH_ERROR_BUFFER_SIZE; + ret = WH_ERROR_BUFFER_SIZE; + break; } /* Unwrap the key. The plaintext is handed back to the client, not @@ -2101,28 +2112,31 @@ static int _HandleKeyUnwrapAndExportRequest( /*requireTrustedKek=*/0, wrappedKey, req->wrappedKeySz, metadata, key, keySz); if (ret != WH_ERROR_OK) { - return ret; + break; } /* Dynamic keyId generation for wrapped keys is not allowed */ if (WH_KEYID_IS_UNASSIGNED(metadata->id)) { /* Wrapped keys must use explicit identifiers */ - return WH_ERROR_BADARGS; + ret = WH_ERROR_BADARGS; + break; } /* Extract ownership from unwrapped metadata (preserves original * owner) */ - uint16_t wrappedKeyUser = WH_KEYID_USER(metadata->id); - uint16_t wrappedKeyType = WH_KEYID_TYPE(metadata->id); + wrappedKeyUser = WH_KEYID_USER(metadata->id); + wrappedKeyType = WH_KEYID_TYPE(metadata->id); /* Require explicit wrapped-key encoding */ if (wrappedKeyType != WH_KEYTYPE_WRAPPED) { - return WH_ERROR_ABORTED; + ret = WH_ERROR_ABORTED; + break; } /* Check if the key is exportable */ if (metadata->flags & WH_NVM_FLAGS_NONEXPORTABLE) { - return WH_ERROR_ACCESS; + ret = WH_ERROR_ACCESS; + break; } /* Validate ownership: USER field must match requesting client. @@ -2131,12 +2145,14 @@ static int _HandleKeyUnwrapAndExportRequest( /* Global keys (USER=0) can be exported by any client */ if (wrappedKeyUser != WH_KEYUSER_GLOBAL && wrappedKeyUser != server->comm->client_id) { - return WH_ERROR_ACCESS; + ret = WH_ERROR_ACCESS; + break; } #else /* Without global keys, USER must match requesting client */ if (wrappedKeyUser != server->comm->client_id) { - return WH_ERROR_ACCESS; + ret = WH_ERROR_ACCESS; + break; } #endif /* WOLFHSM_CFG_GLOBAL_KEYS */ @@ -2147,7 +2163,26 @@ static int _HandleKeyUnwrapAndExportRequest( #endif /* !NO_AES */ default: - return WH_ERROR_BADARGS; + ret = WH_ERROR_BADARGS; + break; + } + + if (ret == WH_ERROR_OK) { + /* The blob stores metadata in server order, so every check above ran + * on native values. Convert to client order only on the way out */ + ret = wh_MessageNvm_TranslateMetadata(magic, metadata, metadata); + } + + /* Keyed off the final ret so a failed translation scrubs too. respData is + * the long-lived comm buffer, so wipe the trailer and any decrypted key */ + if (ret != WH_ERROR_OK && respDataSz >= sizeof(*metadata)) { + uint32_t scrubSz = sizeof(*metadata) + keySz; + + if (scrubSz > respDataSz) { + scrubSz = respDataSz; + } + resp->keySz = 0; + wh_Utils_ForceZero(metadata, scrubSz); } return ret; @@ -2497,7 +2532,8 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic, /* validate args, even though these functions are only supposed to be * called by internal functions */ - if ((server == NULL) || (req_packet == NULL) || (out_resp_size == NULL)) { + if ((server == NULL) || (req_packet == NULL) || (resp_packet == NULL) || + (out_resp_size == NULL)) { return WH_ERROR_BADARGS; } @@ -3225,8 +3261,8 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic, if (ret == WH_ERROR_OK) { ret = WH_SERVER_NVM_LOCK(server); if (ret == WH_ERROR_OK) { - ret = _HandleKeyWrapRequest(server, &wrapReq, reqData, - reqDataSz, &wrapResp, + ret = _HandleKeyWrapRequest(server, magic, &wrapReq, + reqData, reqDataSz, &wrapResp, respData, respDataSz); (void)WH_SERVER_NVM_UNLOCK(server); @@ -3327,14 +3363,22 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic, ret = WH_SERVER_NVM_LOCK(server); if (ret == WH_ERROR_OK) { ret = _HandleKeyUnwrapAndExportRequest( - server, &unwrapReq, reqData, reqDataSz, &unwrapResp, - respData, respDataSz); + server, magic, &unwrapReq, reqData, reqDataSz, + &unwrapResp, respData, respDataSz); (void)WH_SERVER_NVM_UNLOCK(server); } /* WH_SERVER_NVM_LOCK() */ } unwrapResp.rc = ret; + /* The size below always counts a trailer, so clear what a failure + * would otherwise ship out of the shared buffer */ + if (ret != WH_ERROR_OK && respDataSz >= sizeof(whNvmMetadata)) { + wh_Utils_ForceZero((uint8_t*)resp_packet + + sizeof(unwrapResp), + sizeof(whNvmMetadata)); + } + (void)wh_MessageKeystore_TranslateKeyUnwrapAndExportResponse( magic, &unwrapResp, resp_packet); diff --git a/test-refactor/misc/wh_test_check_struct_padding.c b/test-refactor/misc/wh_test_check_struct_padding.c index 983b5a087..403c93d1d 100644 --- a/test-refactor/misc/wh_test_check_struct_padding.c +++ b/test-refactor/misc/wh_test_check_struct_padding.c @@ -38,6 +38,8 @@ whMessageCustomCb_Request whMessageCustomCb_Request_test; whMessageCustomCb_Response whMessageCustomCb_Response_test; #include "wolfhsm/wh_message_nvm.h" +/* Raw wire struct: the key wrap trailers memcpy it across the boundary */ +whNvmMetadata whNvmMetadata_test; whMessageNvm_SimpleResponse whMessageNvm_SimpleResponse_test; whMessageNvm_InitRequest whMessageNvm_InitRequest_test; whMessageNvm_InitResponse whMessageNvm_InitResponse_test; diff --git a/test-refactor/misc/wh_test_message_nvm.c b/test-refactor/misc/wh_test_message_nvm.c new file mode 100644 index 000000000..91f03093a --- /dev/null +++ b/test-refactor/misc/wh_test_message_nvm.c @@ -0,0 +1,128 @@ +/* + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfHSM. + * + * wolfHSM is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfHSM is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wolfHSM. If not, see . + */ + +/* Endian-translation coverage for the NVM message structs. A loopback test + * cannot observe peers that disagree on byte order, so these call the + * translation helpers directly with the magic of a foreign-endian peer. */ + +#include "wolfhsm/wh_settings.h" + +#include +#include + +#include "wolfhsm/wh_common.h" +#include "wolfhsm/wh_comm.h" +#include "wolfhsm/wh_error.h" +#include "wolfhsm/wh_message_nvm.h" + +#include "wh_test_common.h" +#include "wh_test_list.h" + +/* Magic of a peer whose byte order is the opposite of ours */ +#define WH_TEST_MAGIC_FOREIGN ((uint16_t)WH_COMM_MAGIC_SWAP) +#define WH_TEST_MAGIC_LOCAL ((uint16_t)WH_COMM_MAGIC_NATIVE) + +/* Non-palindromic field values, and the same values byte-reversed */ +#define WH_TEST_NVM_META_ID 0x0102u +#define WH_TEST_NVM_META_ACCESS 0x1234u +#define WH_TEST_NVM_META_FLAGS 0x5678u +#define WH_TEST_NVM_META_LEN 0x0a0bu + +#define WH_TEST_NVM_META_ID_SWAPPED 0x0201u +#define WH_TEST_NVM_META_ACCESS_SWAPPED 0x3412u +#define WH_TEST_NVM_META_FLAGS_SWAPPED 0x7856u +#define WH_TEST_NVM_META_LEN_SWAPPED 0x0b0au + +/* Fill with distinct byte values so any missed swap is visible */ +static void _whTest_FillPattern(uint8_t* buf, uint32_t len, uint8_t seed) +{ + uint32_t i; + + for (i = 0; i < len; i++) { + buf[i] = (uint8_t)(seed + i); + } +} + +/* The key wrap messages carry this struct raw, so every scalar swaps and the + * label, a plain byte array, must stay put */ +static int _whTest_MessageNvmMetadata(void) +{ + whNvmMetadata src; + whNvmMetadata dest; + whNvmMetadata back; + + memset(&src, 0, sizeof(src)); + memset(&dest, 0, sizeof(dest)); + memset(&back, 0, sizeof(back)); + + src.id = (whNvmId)WH_TEST_NVM_META_ID; + src.access = (whNvmAccess)WH_TEST_NVM_META_ACCESS; + src.flags = (whNvmFlags)WH_TEST_NVM_META_FLAGS; + src.len = (whNvmSize)WH_TEST_NVM_META_LEN; + _whTest_FillPattern(src.label, (uint32_t)sizeof(src.label), 0x70); + + /* A same-endian peer must see the metadata unchanged */ + WH_TEST_RETURN_ON_FAIL( + wh_MessageNvm_TranslateMetadata(WH_TEST_MAGIC_LOCAL, &src, &dest)); + WH_TEST_ASSERT_RETURN(memcmp(&dest, &src, sizeof(src)) == 0); + + /* A cross-endian peer swaps every scalar. The expected values are written + * out rather than derived, so they do not share the code under test */ + WH_TEST_RETURN_ON_FAIL( + wh_MessageNvm_TranslateMetadata(WH_TEST_MAGIC_FOREIGN, &src, &dest)); + WH_TEST_ASSERT_RETURN(dest.id == (whNvmId)WH_TEST_NVM_META_ID_SWAPPED); + WH_TEST_ASSERT_RETURN(dest.access == + (whNvmAccess)WH_TEST_NVM_META_ACCESS_SWAPPED); + WH_TEST_ASSERT_RETURN(dest.flags == + (whNvmFlags)WH_TEST_NVM_META_FLAGS_SWAPPED); + WH_TEST_ASSERT_RETURN(dest.len == (whNvmSize)WH_TEST_NVM_META_LEN_SWAPPED); + WH_TEST_ASSERT_RETURN(memcmp(dest.label, src.label, sizeof(src.label)) == + 0); + + /* Translation is its own inverse */ + WH_TEST_RETURN_ON_FAIL( + wh_MessageNvm_TranslateMetadata(WH_TEST_MAGIC_FOREIGN, &dest, &back)); + WH_TEST_ASSERT_RETURN(memcmp(&back, &src, sizeof(src)) == 0); + + /* In-place translation must give the same result */ + memcpy(&back, &src, sizeof(back)); + WH_TEST_RETURN_ON_FAIL( + wh_MessageNvm_TranslateMetadata(WH_TEST_MAGIC_FOREIGN, &back, &back)); + WH_TEST_ASSERT_RETURN(memcmp(&back, &dest, sizeof(src)) == 0); + + WH_TEST_ASSERT_RETURN(wh_MessageNvm_TranslateMetadata( + WH_TEST_MAGIC_FOREIGN, NULL, &dest) == + WH_ERROR_BADARGS); + WH_TEST_ASSERT_RETURN(wh_MessageNvm_TranslateMetadata( + WH_TEST_MAGIC_FOREIGN, &src, NULL) == + WH_ERROR_BADARGS); + + return WH_TEST_SUCCESS; +} + +int whTest_MessageNvmTranslate(void* ctx) +{ + (void)ctx; + + WH_TEST_PRINT("Testing NVM message translation...\n"); + + WH_TEST_RETURN_ON_FAIL(_whTest_MessageNvmMetadata()); + + return WH_TEST_SUCCESS; +} diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index 1f4fa1f1b..40059708b 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -46,6 +46,7 @@ WH_TEST_DECL(whTest_CryptoAffinity); WH_TEST_DECL(whTest_Dma); WH_TEST_DECL(whTest_HwKeystore); WH_TEST_DECL(whTest_KeystoreReqSize); +WH_TEST_DECL(whTest_MessageNvmTranslate); WH_TEST_DECL(whTest_MultiClient); WH_TEST_DECL(whTest_Lock); WH_TEST_DECL(whTest_Log); @@ -108,6 +109,7 @@ const whTestCase whTestsMisc[] = { { "whTest_Dma", whTest_Dma }, { "whTest_CryptoAffinity", whTest_CryptoAffinity }, { "whTest_KeystoreReqSize", whTest_KeystoreReqSize }, + { "whTest_MessageNvmTranslate", whTest_MessageNvmTranslate }, { "whTest_MultiClient", whTest_MultiClient }, { "whTest_HwKeystore", whTest_HwKeystore }, { "whTest_Lock", whTest_Lock }, diff --git a/wolfhsm/wh_message_nvm.h b/wolfhsm/wh_message_nvm.h index 023158dd4..048953d2b 100644 --- a/wolfhsm/wh_message_nvm.h +++ b/wolfhsm/wh_message_nvm.h @@ -53,6 +53,11 @@ enum WH_MESSAGE_NVM_MAX_ENUM { WH_MESSAGE_NVM_MAX_READ_LEN = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(int32_t), }; +/* Translate a whNvmMetadata carried on the wire as a raw struct, rather than + * flattened into message fields. In-place safe (src may equal dest) */ +int wh_MessageNvm_TranslateMetadata(uint16_t magic, const whNvmMetadata* src, + whNvmMetadata* dest); + /* Simple reusable response message */ typedef struct { int32_t rc;