Skip to content

Translate the key wrap metadata trailer #472

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.

Sign up for GitHub

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

Open
yosuke-wolfssl wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Translate the key wrap metadata trailer #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions src/wh_message_nvm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
100 changes: 72 additions & 28 deletions src/wh_server_keystore.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1805,6 +1806,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,
Expand All @@ -1824,20 +1826,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,
Expand Down Expand Up @@ -2012,16 +2018,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) {
Expand Down Expand Up @@ -2055,19 +2063,22 @@ 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 -
WH_KEYWRAP_AES_GCM_HEADER_SIZE - sizeof(*metadata);

/* 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
Expand All @@ -2076,28 +2087,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.
Expand All @@ -2106,12 +2120,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 */

Expand All @@ -2122,7 +2138,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;
Expand Down Expand Up @@ -2472,7 +2507,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;
}

Expand Down Expand Up @@ -3198,8 +3234,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);
Expand Down Expand Up @@ -3300,14 +3336,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);

Expand Down
2 changes: 2 additions & 0 deletions test-refactor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ Not yet migrated (still live in `wolfHSM/test/`):
### Known coverage gaps
- FLASH_LOG backend for server/client-group tests. In `test/`, cert (`whTest_CertRamSim`), crypto (`wh_ClientServer_MemThreadTest`), image-manager (`whTest_ServerImgMgr`), and client/server were each run against both the plain flash and flash-log NVM backends. The refactored server/client-group tests consume a single server context (`wh_test_posix_server.c` hard-codes `WH_NVM_FLASH_CB`), so only the plain flash backend is exercised. Restoring parity means selecting the server backend via `whTest_NvmCfgBackend` and running the server + client groups once per backend (flash, then flash-log) from `wh_test_posix_main.c`. Tracked as a follow-up; the port-specific `whTest_NvmFlashLog` already covers the flash-log NVM object lifecycle directly.

- Cross-endian metadata on the DMA NVM paths. `server/wh_test_keywrap_endian.c` and `client-server/wh_test_keywrap_endian_e2e.c` cover the `whNvmMetadata` trailer the key wrap messages carry, which the server translates via `wh_MessageNvm_TranslateMetadata()`. The DMA NVM paths (`WH_MESSAGE_NVM_ACTION_ADDOBJECTDMA` ingress and `WH_MESSAGE_NVM_ACTION_READDMA` write-back) hand the same struct across untranslated, so a big-endian client against a little-endian server writes byte-reversed `id`, `access`, `flags` and `len` into NVM. Pre-existing and untested in either harness. A fix must translate into a local copy: the struct lives in client DMA memory, so translating in place would write into the client's buffer.

### Other improvements
- Add callback from `wh_Server_HandleRequestMessage` to allow sleep and avoid a busy loop
- Add client-only harness to feed invalid server inputs from the test bench with the goal of expanding coverage.
Loading
Loading