Skip to content

Zero DMA destination buffer on server WRITE_PRE #486

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

Zero DMA destination buffer on server WRITE_PRE #486

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
2 changes: 2 additions & 0 deletions docs/src/5-Features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,8 @@ It is called twice per access — once before, once after — with the `oper` ar

If no callback is registered, the server uses the client address directly as `*serverPtr`, which is the right behavior for a system with a flat shared address space and coherent caches. Ports that need either address translation or cache maintenance supply a callback that handles both; the callback is the single extension point for both concerns.

**The callback must not call into any wolfHSM NVM API.** The NVM and certificate DMA *read* handlers invoke the `WH_DMA_OPER_CLIENT_WRITE_PRE` phase while holding the server's NVM lock, so that a request for an object the client is not permitted to read is refused before any client memory is mapped. That lock is not recursive: a callback that re-enters `wh_Nvm_*` deadlocks under `WOLFHSM_CFG_THREADSAFE`. A port that needs NVM-resident data for address translation — an allowlist or mapping table stored as an NVM object, for example — must read it once at initialization and cache it, rather than looking it up inside the callback. Treat the restriction as applying to every phase: which phases run under the lock is an internal detail and may change.

For platforms where the client buffer is not directly memcpy-able even after address translation — for example, when the only path to client memory is through a hardware FIFO or register window — wolfHSM additionally exposes a `whServerDmaMemCopyCb` callback under `WOLFHSM_CFG_DMA_CUSTOM_CLIENT_COPY`. When registered (via `wh_Server_DmaRegisterMemCopyCb`), this callback replaces the internal `memcpy` between server and client memory entirely, and is the only operation that touches the client side of the transfer.

The same PRE/POST callback model is also available on the **client side** through `wh_Client_DmaRegisterCb`, with an identical `whClientDmaClientMemCb` signature. The client callback is invoked before the request is sent and after the response is received, and is the right place for any work that has to happen in the client's address space before the server is ever told about a buffer — pinning pages, flushing the client's view of a cache line, or substituting the application's pointer with one that lives in a region the server can actually reach. The POSIX shared-memory transport illustrates the last case: an application buffer allocated from the process's ordinary heap is not visible to the server because it lies outside the mapped shared-memory segment, so the transport's client callback (`posixTransportShm_ClientStaticMemDmaCallback`) detects that the supplied address falls outside the DMA region, allocates a bounce buffer inside the shared segment on `*_READ_PRE`/`*_WRITE_PRE`, copies the application data into it for the read direction, and reports the in-segment offset as the address the server should use. The matching POST phase copies any server-written bytes back to the original application buffer and frees the bounce buffer. From the application's perspective the original wolfCrypt call is unchanged; the client callback transparently bridges the gap between the application's address space and the address space the server can address. Client-side and server-side callbacks are independent — a port may register either, both, or neither, depending on which side needs the translation.
Expand Down
69 changes: 45 additions & 24 deletions src/wh_server_cert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,18 +1286,8 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
wh_MessageCert_TranslateReadTrustedDmaRequest(
magic, (whMessageCert_ReadTrustedDmaRequest*)req_packet,
&req);

/* Process client address */
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
if (resp.rc == WH_ERROR_OK) {
cert_dma_pre_ok = 1;
}
}
if (resp.rc == WH_ERROR_OK) {
/* Deny reading non-exportable or server-only (trusted KEK)
* objects; see the non-DMA path above. */
resp.rc = WH_SERVER_NVM_LOCK(server);
if (resp.rc == WH_ERROR_OK) {
resp.rc = wh_Nvm_GetMetadata(server->nvm, req.id, &meta);
Expand All @@ -1306,25 +1296,56 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
WH_NVM_FLAGS_SERVER_ONLY)) != 0) {
resp.rc = WH_ERROR_ACCESS;
}
else {
/* The callee reports the stored length back into
* cert_len, but SimpleResponse has no field to
* return it, so the client cannot learn it here */
cert_len = req.cert_len;
resp.rc = wh_Server_CertReadTrusted(
server, req.id, cert_data, &cert_len);
}
/* wh_Server_CertReadTrusted() calls the unchecked
* wh_Nvm_Read(), so the check above is the only gate, and
* it precedes the bound and the map. */
if (resp.rc == WH_ERROR_OK &&
req.cert_len > WOLFHSM_CFG_MAX_CERT_SIZE) {
resp.rc = WH_ERROR_BADARGS;
}
if (resp.rc == WH_ERROR_OK) {
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_WRITE_PRE,
(whServerDmaFlags){0});
/* Zero length is a no-op. A NULL mapping cannot be
* zeroed, so fail before pairing a POST. */
if (resp.rc == WH_ERROR_OK && req.cert_len > 0) {
if (cert_data == NULL) {
resp.rc = WH_ERROR_BADARGS;
}
else {
cert_dma_pre_ok = 1;
/* The read fills only meta.len, so zero
* first. */
memset(cert_data, 0, req.cert_len);
}
}
else if (resp.rc == WH_ERROR_OK) {
cert_dma_pre_ok = 1;
}
}
if (resp.rc == WH_ERROR_OK) {
/* The callee reports the stored length back into
* cert_len, but SimpleResponse has no field to
* return it, so the client cannot learn it here */
cert_len = req.cert_len;
resp.rc = wh_Server_CertReadTrusted(
server, req.id, cert_data, &cert_len);
}

(void)WH_SERVER_NVM_UNLOCK(server);
} /* WH_SERVER_NVM_LOCK() */
}
/* Always call POST for successful PRE, regardless of operation
* result */
if (cert_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});

/* Always call POST for successful PRE, regardless of
* operation result. Runs outside the lock: only the PRE has
* to be ordered against the deny check. */
if (cert_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
}
}

/* Convert the response struct */
Expand Down
33 changes: 27 additions & 6 deletions src/wh_server_nvm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,16 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
if (rc == WH_ERROR_OK) {
rc = wh_Nvm_GetMetadata(server->nvm, req.id, &meta);

if (rc == 0) {
/* Refuse before the bound and the map: ACCESS whatever the
* offset, so the length cannot be probed. The non-DMA READ
* deliberately keeps its original ordering. */
if ((meta.flags & (WH_NVM_FLAGS_NONEXPORTABLE |
WH_NVM_FLAGS_SERVER_ONLY)) != 0) {
rc = WH_ERROR_ACCESS;
}
}

if (rc == 0) {
if (req.offset >= meta.len) {
rc = WH_ERROR_BADARGS;
Expand All @@ -479,14 +489,25 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
}
}

/* use unclamped length for DMA address processing in case DMA
* callbacks are sensible to alignment and/or size */
/* Map the full requested length (callbacks may depend on it);
* the allowlist, not the object size, bounds the extent. */
if (rc == 0) {
/* perform platform-specific host address processing */
rc = wh_Server_DmaProcessClientAddress(
server, req.data_hostaddr, &data, req.data_len,
WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
if (rc == 0) {
/* Zero it since the read fills only read_len. A NULL
* mapping cannot be zeroed, so fail rather than POST
* un-zeroed bytes; zero length is a no-op. */
if (rc == 0 && req.data_len > 0) {
if (data == NULL) {
rc = WH_ERROR_BADARGS;
}
else {
data_dma_pre_ok = 1;
memset(data, 0, req.data_len);
}
}
else if (rc == 0) {
data_dma_pre_ok = 1;
}
}
Expand All @@ -495,8 +516,8 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
rc = wh_Nvm_ReadChecked(server->nvm, req.id, req.offset,
read_len, (uint8_t*)data);
}
/* Always call POST for successful PRE, regardless of read
* result */
/* POST inside the lock, matching this handler's original
* bracket. */
if (data_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.data_hostaddr, &data, req.data_len,
Expand Down
60 changes: 60 additions & 0 deletions test-refactor/client-server/wh_test_client_certs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
&& !defined(WOLFHSM_CFG_NO_CRYPTO)

#include <stdint.h>
#include <stddef.h>
#include <string.h>

#include "wolfhsm/wh_error.h"
#include "wolfhsm/wh_client.h"
Expand Down Expand Up @@ -83,9 +85,67 @@ static int _whTest_CertReadTrustedSmallBuffer(whClientContext* ctx)
}


#ifdef WOLFHSM_CFG_DMA

#define CERT_DMA_POISON ((uint8_t)0xA7)

/* File-scope so an oversized request that the server wrongly accepts writes
* into this instead of smashing the sub-test's frame. */
static uint8_t _certDmaOversize[WOLFHSM_CFG_MAX_CERT_SIZE + 1];


/* The server writes the whole requested length once it maps the buffer, so
* the tail past a short certificate must be zeros. A length beyond
* WOLFHSM_CFG_MAX_CERT_SIZE is refused before any mapping happens. */
static int _whTest_CertReadTrustedDmaTailZeroed(whClientContext* ctx)
{
int32_t out_rc = 0;
const whNvmId cert_id = 104;
uint8_t dma_buf[2048];
size_t i;

WH_TEST_ASSERT_RETURN(ROOT_A_CERT_len < sizeof(dma_buf));

WH_TEST_RETURN_ON_FAIL(wh_Client_CertAddTrusted(
ctx, cert_id, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONMODIFIABLE,
NULL, 0, ROOT_A_CERT, ROOT_A_CERT_len, &out_rc));
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);

memset(dma_buf, CERT_DMA_POISON, sizeof(dma_buf));
WH_TEST_RETURN_ON_FAIL(wh_Client_CertReadTrustedDma(
ctx, cert_id, dma_buf, sizeof(dma_buf), &out_rc));
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);

WH_TEST_ASSERT_RETURN(0 == memcmp(dma_buf, ROOT_A_CERT, ROOT_A_CERT_len));
for (i = ROOT_A_CERT_len; i < sizeof(dma_buf); i++) {
WH_TEST_ASSERT_RETURN(dma_buf[i] == 0);
}

/* Past the cap: refused before the map, so the buffer keeps its fill. */
memset(_certDmaOversize, CERT_DMA_POISON, sizeof(_certDmaOversize));
WH_TEST_RETURN_ON_FAIL(wh_Client_CertReadTrustedDma(
ctx, cert_id, _certDmaOversize, sizeof(_certDmaOversize), &out_rc));
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_BADARGS);
for (i = 0; i < sizeof(_certDmaOversize); i++) {
WH_TEST_ASSERT_RETURN(_certDmaOversize[i] == CERT_DMA_POISON);
}

WH_TEST_RETURN_ON_FAIL(
wh_Client_CertEraseTrusted(ctx, cert_id, &out_rc));
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);

return WH_ERROR_OK;
}

#endif /* WOLFHSM_CFG_DMA */


int whTest_ClientCerts(whClientContext* ctx)
{
WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedSmallBuffer(ctx));
#ifdef WOLFHSM_CFG_DMA
WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedDmaTailZeroed(ctx));
#endif

return WH_ERROR_OK;
}
Expand Down
147 changes: 147 additions & 0 deletions test-refactor/client-server/wh_test_crypto_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,148 @@ int whTest_CryptoAesKeyUsagePolicies(whClientContext* ctx)
return ret;
}

#if defined(WOLFHSM_CFG_DMA) && \
(defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER) || \
defined(HAVE_AES_ECB))
/* One buffer as both input and output must match out-of-place. Also fails if
* a zero-fill moves into wh_Server_DmaProcessClientAddress(): in and out map
* to one address here, so a central memset would erase the plaintext. */
static int whTest_CryptoAesDmaInPlace(whClientContext* ctx)
{
/* The wh_Client_Aes*Dma entry points are called directly, so the devId
* only has to be a valid one -- it does not select the DMA path here. */
int devId = WH_CLIENT_DEVID(ctx);
int ret = 0;
Aes aes[1];
uint8_t inplace[AES_BLOCK_SIZE * 2];
uint8_t refcipher[AES_BLOCK_SIZE * 2];
const uint8_t key[] = {0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6,
0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c};
#if defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER)
const uint8_t iv[AES_BLOCK_SIZE] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
0x0c, 0x0d, 0x0e, 0x0f};
#endif
const uint8_t plainIn[AES_BLOCK_SIZE * 2] = {
0x6b, 0xc1, 0xbe, 0xe2, 0x2e, 0x40, 0x9f, 0x96, 0xe9, 0x3d, 0x7e,
0x11, 0x73, 0x93, 0x17, 0x2a, 0xae, 0x2d, 0x8a, 0x57, 0x1e, 0x03,
0xac, 0x9c, 0x9e, 0xb7, 0x6f, 0xac, 0x45, 0xaf, 0x8e, 0x51};

#ifdef HAVE_AES_CBC
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKey(aes, key, sizeof(key), iv, AES_ENCRYPTION);
if (ret == 0) {
ret = wh_Client_AesCbcDma(ctx, aes, 1, plainIn, sizeof(plainIn),
refcipher);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKey(aes, key, sizeof(key), iv, AES_ENCRYPTION);
if (ret == 0) {
memcpy(inplace, plainIn, sizeof(plainIn));
ret = wh_Client_AesCbcDma(ctx, aes, 1, inplace, sizeof(inplace),
inplace);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0 && memcmp(inplace, refcipher, sizeof(refcipher)) != 0) {
WH_ERROR_PRINT("AES-CBC DMA in-place != out-of-place\n");
ret = -1;
}
/* Decrypt is the harder in-place direction: the recovered plaintext below
* proves each ciphertext block survived to serve as the next block's IV. */
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKey(aes, key, sizeof(key), iv, AES_DECRYPTION);
if (ret == 0) {
memcpy(inplace, refcipher, sizeof(refcipher));
ret = wh_Client_AesCbcDma(ctx, aes, 0, inplace, sizeof(inplace),
inplace);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0 && memcmp(inplace, plainIn, sizeof(plainIn)) != 0) {
WH_ERROR_PRINT("AES-CBC DMA in-place decrypt != plaintext\n");
ret = -1;
}
#endif /* HAVE_AES_CBC */

#ifdef WOLFSSL_AES_COUNTER
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION);
if (ret == 0) {
ret = wh_Client_AesCtrDma(ctx, aes, 1, plainIn, sizeof(plainIn),
refcipher);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION);
if (ret == 0) {
memcpy(inplace, plainIn, sizeof(plainIn));
ret = wh_Client_AesCtrDma(ctx, aes, 1, inplace, sizeof(inplace),
inplace);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0 && memcmp(inplace, refcipher, sizeof(refcipher)) != 0) {
WH_ERROR_PRINT("AES-CTR DMA in-place != out-of-place\n");
ret = -1;
}
#endif /* WOLFSSL_AES_COUNTER */

#ifdef HAVE_AES_ECB
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKey(aes, key, sizeof(key), NULL, AES_ENCRYPTION);
if (ret == 0) {
ret = wh_Client_AesEcbDma(ctx, aes, 1, plainIn, sizeof(plainIn),
refcipher);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0) {
ret = wc_AesInit(aes, NULL, devId);
if (ret == 0) {
ret = wc_AesSetKey(aes, key, sizeof(key), NULL, AES_ENCRYPTION);
if (ret == 0) {
memcpy(inplace, plainIn, sizeof(plainIn));
ret = wh_Client_AesEcbDma(ctx, aes, 1, inplace, sizeof(inplace),
inplace);
}
(void)wc_AesFree(aes);
}
}
if (ret == 0 && memcmp(inplace, refcipher, sizeof(refcipher)) != 0) {
WH_ERROR_PRINT("AES-ECB DMA in-place != out-of-place\n");
ret = -1;
}
#endif /* HAVE_AES_ECB */

if (ret == 0) {
WH_TEST_PRINT("AES DMA in-place aliasing DEVID=0x%X SUCCESS\n", devId);
}
return ret;
}
#endif /* WOLFHSM_CFG_DMA && AES (CBC|CTR|ECB) */

int whTest_Crypto_Aes(whClientContext* ctx)
{
int i;
Expand All @@ -1123,6 +1265,11 @@ int whTest_Crypto_Aes(whClientContext* ctx)
#endif
#ifdef WOLFSSL_AES_COUNTER
WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesCtrLeftOob(ctx));
#endif
#if defined(WOLFHSM_CFG_DMA) && \
(defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER) || \
defined(HAVE_AES_ECB))
WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesDmaInPlace(ctx));
#endif
/* TODO: port legacy AES async + DMA-async coverage (comm-buffer & DMA, round-trip & KAT) -- the only remaining legacy crypto parity gap; deferred to follow-up PR. */
return 0;
Expand Down
Loading
Loading