Skip to content

Commit 5995395

Browse files
jackctj117jackctj117
committed
Harden DMA security: fix resource leaks, validate SHA contexts, and enforce default-deny allow list
Fix DMA POST resource leaks (bugs #1566-#1574): - Ensure DMA POST is always called after successful PRE in ADDOBJECTDMA, READDMA, ADDTRUSTED_DMA, READTRUSTED_DMA, VERIFY_DMA, and VERIFY_ACERT_DMA handlers using tracking flags Validate SHA DMA contexts from untrusted client memory (bug #2009): - Add buffLen >= block_size validation in all four SHA DMA handlers - Validate hashType in SHA512 DMA handler Change DMA allow list default to fail-closed (bug #2279): - Return WH_ERROR_ACCESS when no allow list is registered instead of permitting all operations - Remove erroneous server-side allow list check in CopyFromClient/CopyToClient (server buffers are trusted; client addresses are validated separately) Update tests for new DMA security defaults: - Register server and client DMA allow lists in stress, multiclient, and clientserver tests - Update DMA unit test to expect fail-closed behavior - Add client-address deny test for allow list enforcement
1 parent 29c2888 commit 5995395

10 files changed

Lines changed: 311 additions & 135 deletions

‎src/wh_dma.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ static int _checkMemOperAgainstAllowList(const whDmaAddrAllowList* allowlist,
9999
return rc;
100100
}
101101

102-
/* If no allowlist is registered, anything goes */
102+
/* If no allowlist is registered, deny all operations (fail-closed) */
103103
if (allowlist == NULL) {
104-
return WH_ERROR_OK;
104+
return WH_ERROR_ACCESS;
105105
}
106106

107107
/* If a read/write operation is requested, check the transformed address

‎src/wh_server_cert.c‎

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -556,9 +556,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
556556

557557
#ifdef WOLFHSM_CFG_DMA
558558
case WH_MESSAGE_CERT_ACTION_ADDTRUSTED_DMA: {
559-
whMessageCert_AddTrustedDmaRequest req = {0};
560-
whMessageCert_SimpleResponse resp = {0};
561-
void* cert_data = NULL;
559+
whMessageCert_AddTrustedDmaRequest req = {0};
560+
whMessageCert_SimpleResponse resp = {0};
561+
void* cert_data = NULL;
562+
int cert_dma_pre_ok = 0;
562563

563564
if (req_size != sizeof(req)) {
564565
/* Request is malformed */
@@ -574,6 +575,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
574575
resp.rc = wh_Server_DmaProcessClientAddress(
575576
server, req.cert_addr, &cert_data, req.cert_len,
576577
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
578+
if (resp.rc == WH_ERROR_OK) {
579+
cert_dma_pre_ok = 1;
580+
}
577581
}
578582
if (resp.rc == WH_ERROR_OK) {
579583
/* Process the add trusted action */
@@ -586,9 +590,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
586590
(void)WH_SERVER_NVM_UNLOCK(server);
587591
} /* WH_SERVER_NVM_LOCK() */
588592
}
589-
if (resp.rc == WH_ERROR_OK) {
590-
/* Post-process client address */
591-
resp.rc = wh_Server_DmaProcessClientAddress(
593+
/* Always call POST for successful PRE, regardless of operation
594+
* result */
595+
if (cert_dma_pre_ok) {
596+
(void)wh_Server_DmaProcessClientAddress(
592597
server, req.cert_addr, &cert_data, req.cert_len,
593598
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
594599
}
@@ -600,11 +605,12 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
600605
}; break;
601606

602607
case WH_MESSAGE_CERT_ACTION_READTRUSTED_DMA: {
603-
whMessageCert_ReadTrustedDmaRequest req = {0};
604-
whMessageCert_SimpleResponse resp = {0};
605-
void* cert_data = NULL;
608+
whMessageCert_ReadTrustedDmaRequest req = {0};
609+
whMessageCert_SimpleResponse resp = {0};
610+
void* cert_data = NULL;
606611
uint32_t cert_len;
607612
whNvmMetadata meta;
613+
int cert_dma_pre_ok = 0;
608614

609615
if (req_size != sizeof(req)) {
610616
/* Request is malformed */
@@ -620,6 +626,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
620626
resp.rc = wh_Server_DmaProcessClientAddress(
621627
server, req.cert_addr, &cert_data, req.cert_len,
622628
WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
629+
if (resp.rc == WH_ERROR_OK) {
630+
cert_dma_pre_ok = 1;
631+
}
623632
}
624633
if (resp.rc == WH_ERROR_OK) {
625634
/* Check metadata to see if the certificate is non-exportable */
@@ -641,10 +650,11 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
641650
(void)WH_SERVER_NVM_UNLOCK(server);
642651
} /* WH_SERVER_NVM_LOCK() */
643652
}
644-
if (resp.rc == WH_ERROR_OK) {
645-
/* Post-process client address */
646-
resp.rc = wh_Server_DmaProcessClientAddress(
647-
server, req.cert_addr, &cert_data, cert_len,
653+
/* Always call POST for successful PRE, regardless of operation
654+
* result */
655+
if (cert_dma_pre_ok) {
656+
(void)wh_Server_DmaProcessClientAddress(
657+
server, req.cert_addr, &cert_data, req.cert_len,
648658
WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
649659
}
650660

@@ -655,10 +665,11 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
655665
}; break;
656666

657667
case WH_MESSAGE_CERT_ACTION_VERIFY_DMA: {
658-
whMessageCert_VerifyDmaRequest req = {0};
659-
whMessageCert_VerifyDmaResponse resp = {0};
660-
void* cert_data = NULL;
661-
whKeyId keyId = WH_KEYID_ERASED;
668+
whMessageCert_VerifyDmaRequest req = {0};
669+
whMessageCert_VerifyDmaResponse resp = {0};
670+
void* cert_data = NULL;
671+
whKeyId keyId = WH_KEYID_ERASED;
672+
int cert_dma_pre_ok = 0;
662673

663674
if (req_size != sizeof(req)) {
664675
/* Request is malformed */
@@ -677,6 +688,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
677688
resp.rc = wh_Server_DmaProcessClientAddress(
678689
server, req.cert_addr, &cert_data, req.cert_len,
679690
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
691+
if (resp.rc == WH_ERROR_OK) {
692+
cert_dma_pre_ok = 1;
693+
}
680694
}
681695
if (resp.rc == WH_ERROR_OK) {
682696
resp.rc = WH_SERVER_NVM_LOCK(server);
@@ -693,9 +707,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
693707
(void)WH_SERVER_NVM_UNLOCK(server);
694708
} /* WH_SERVER_NVM_LOCK() */
695709
}
696-
if (resp.rc == WH_ERROR_OK) {
697-
/* Post-process client address */
698-
resp.rc = wh_Server_DmaProcessClientAddress(
710+
/* Always call POST for successful PRE, regardless of operation
711+
* result */
712+
if (cert_dma_pre_ok) {
713+
(void)wh_Server_DmaProcessClientAddress(
699714
server, req.cert_addr, &cert_data, req.cert_len,
700715
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
701716
}
@@ -766,9 +781,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
766781
#if defined(WOLFHSM_CFG_DMA)
767782
case WH_MESSAGE_CERT_ACTION_VERIFY_ACERT_DMA: {
768783
/* Acert verify request uses standard cert verify request struct */
769-
whMessageCert_VerifyDmaRequest req = {0};
770-
whMessageCert_SimpleResponse resp = {0};
771-
void* cert_data = NULL;
784+
whMessageCert_VerifyDmaRequest req = {0};
785+
whMessageCert_SimpleResponse resp = {0};
786+
void* cert_data = NULL;
787+
int cert_dma_pre_ok = 0;
772788

773789
if (req_size != sizeof(req)) {
774790
/* Request is malformed */
@@ -783,6 +799,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
783799
rc = wh_Server_DmaProcessClientAddress(
784800
server, req.cert_addr, &cert_data, req.cert_len,
785801
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
802+
if (rc == WH_ERROR_OK) {
803+
cert_dma_pre_ok = 1;
804+
}
786805
}
787806
if (rc == WH_ERROR_OK) {
788807
/* Process the verify action */
@@ -805,9 +824,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
805824
resp.rc = rc;
806825
}
807826
}
808-
if (rc == WH_ERROR_OK) {
809-
/* Post-process client address */
810-
rc = wh_Server_DmaProcessClientAddress(
827+
/* Always call POST for successful PRE, regardless of operation
828+
* result */
829+
if (cert_dma_pre_ok) {
830+
(void)wh_Server_DmaProcessClientAddress(
811831
server, req.cert_addr, &cert_data, req.cert_len,
812832
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
813833
}

‎src/wh_server_crypto.c‎

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4784,11 +4784,17 @@ static int _HandleSha256Dma(whServerContext* ctx, uint16_t magic, int devId,
47844784
res.dmaAddrStatus.badAddr = req.state;
47854785
}
47864786
else {
4787-
/* Save the client devId to be restored later, when the context is
4788-
* copied back into client memory. */
4789-
clientDevId = sha256->devId;
4790-
/* overwrite the devId to that of the server for local crypto */
4791-
sha256->devId = devId;
4787+
/* Validate buffLen from untrusted context */
4788+
if (sha256->buffLen >= WC_SHA256_BLOCK_SIZE) {
4789+
ret = WH_ERROR_BADARGS;
4790+
}
4791+
else {
4792+
/* Save the client devId to be restored later, when the context
4793+
* is copied back into client memory. */
4794+
clientDevId = sha256->devId;
4795+
/* overwrite the devId to that of the server for local crypto */
4796+
sha256->devId = devId;
4797+
}
47924798
}
47934799
}
47944800

@@ -4906,11 +4912,17 @@ static int _HandleSha224Dma(whServerContext* ctx, uint16_t magic, int devId,
49064912
res.dmaAddrStatus.badAddr = req.state;
49074913
}
49084914
else {
4909-
/* Save the client devId to be restored later, when the context is
4910-
* copied back into client memory. */
4911-
clientDevId = sha224->devId;
4912-
/* overwrite the devId to that of the server for local crypto */
4913-
sha224->devId = devId;
4915+
/* Validate buffLen from untrusted context */
4916+
if (sha224->buffLen >= WC_SHA224_BLOCK_SIZE) {
4917+
ret = WH_ERROR_BADARGS;
4918+
}
4919+
else {
4920+
/* Save the client devId to be restored later, when the context
4921+
* is copied back into client memory. */
4922+
clientDevId = sha224->devId;
4923+
/* overwrite the devId to that of the server for local crypto */
4924+
sha224->devId = devId;
4925+
}
49144926
}
49154927
}
49164928

@@ -5028,11 +5040,17 @@ static int _HandleSha384Dma(whServerContext* ctx, uint16_t magic, int devId,
50285040
res.dmaAddrStatus.badAddr = req.state;
50295041
}
50305042
else {
5031-
/* Save the client devId to be restored later, when the context is
5032-
* copied back into client memory. */
5033-
clientDevId = sha384->devId;
5034-
/* overwrite the devId to that of the server for local crypto */
5035-
sha384->devId = devId;
5043+
/* Validate buffLen from untrusted context */
5044+
if (sha384->buffLen >= WC_SHA384_BLOCK_SIZE) {
5045+
ret = WH_ERROR_BADARGS;
5046+
}
5047+
else {
5048+
/* Save the client devId to be restored later, when the context
5049+
* is copied back into client memory. */
5050+
clientDevId = sha384->devId;
5051+
/* overwrite the devId to that of the server for local crypto */
5052+
sha384->devId = devId;
5053+
}
50365054
}
50375055
}
50385056

@@ -5150,13 +5168,25 @@ static int _HandleSha512Dma(whServerContext* ctx, uint16_t magic, int devId,
51505168
res.dmaAddrStatus.badAddr = req.state;
51515169
}
51525170
else {
5153-
/* Save the client devId to be restored later, when the context is
5154-
* copied back into client memory. */
5155-
clientDevId = sha512->devId;
5156-
/* overwrite the devId to that of the server for local crypto */
5157-
sha512->devId = devId;
5158-
/* retrieve hash Type to handle 512, 512-224, or 512-256 */
5159-
hashType = sha512->hashType;
5171+
/* Validate buffLen from untrusted context */
5172+
if (sha512->buffLen >= WC_SHA512_BLOCK_SIZE) {
5173+
ret = WH_ERROR_BADARGS;
5174+
}
5175+
else {
5176+
/* Save the client devId to be restored later, when the context
5177+
* is copied back into client memory. */
5178+
clientDevId = sha512->devId;
5179+
/* overwrite the devId to that of the server for local crypto */
5180+
sha512->devId = devId;
5181+
/* retrieve hash Type to handle 512, 512-224, or 512-256 */
5182+
hashType = sha512->hashType;
5183+
/* Validate hashType from untrusted context */
5184+
if (hashType != WC_HASH_TYPE_SHA512 &&
5185+
hashType != WC_HASH_TYPE_SHA512_224 &&
5186+
hashType != WC_HASH_TYPE_SHA512_256) {
5187+
ret = WH_ERROR_BADARGS;
5188+
}
5189+
}
51605190
}
51615191
}
51625192

‎src/wh_server_dma.c‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,7 @@ int whServerDma_CopyFromClient(struct whServerContext_t* server,
131131
return WH_ERROR_BADARGS;
132132
}
133133

134-
/* Check the server address against the allow list */
135-
rc = wh_Dma_CheckMemOperAgainstAllowList(server->dma.dmaAddrAllowList,
136-
WH_DMA_OPER_CLIENT_READ_PRE,
137-
serverPtr, len);
138-
if (rc != WH_ERROR_OK) {
139-
return rc;
140-
}
141-
142-
/* Process the client address pre-read */
134+
/* Process the client address pre-read (includes allow list check) */
143135
rc = wh_Server_DmaProcessClientAddress(
144136
server, clientAddr, &transformedAddr, len, WH_DMA_OPER_CLIENT_READ_PRE,
145137
flags);
@@ -185,15 +177,7 @@ int whServerDma_CopyToClient(struct whServerContext_t* server,
185177
return WH_ERROR_BADARGS;
186178
}
187179

188-
/* Check the server address against the allow list */
189-
rc = wh_Dma_CheckMemOperAgainstAllowList(server->dma.dmaAddrAllowList,
190-
WH_DMA_OPER_CLIENT_WRITE_PRE,
191-
serverPtr, len);
192-
if (rc != WH_ERROR_OK) {
193-
return rc;
194-
}
195-
196-
/* Process the client address pre-write */
180+
/* Process the client address pre-write (includes allow list check) */
197181
rc = wh_Server_DmaProcessClientAddress(server, clientAddr, &transformedAddr,
198182
len, WH_DMA_OPER_CLIENT_WRITE_PRE,
199183
flags);

‎src/wh_server_nvm.c‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,8 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
349349
whMessageNvm_SimpleResponse resp = {0};
350350
void* metadata = NULL;
351351
void* data = NULL;
352+
int metadata_dma_pre_ok = 0;
353+
int data_dma_pre_ok = 0;
352354

353355
if (req_size != sizeof(req)) {
354356
/* Request is malformed */
@@ -363,11 +365,17 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
363365
resp.rc = wh_Server_DmaProcessClientAddress(
364366
server, req.metadata_hostaddr, &metadata, sizeof(whNvmMetadata),
365367
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
368+
if (resp.rc == 0) {
369+
metadata_dma_pre_ok = 1;
370+
}
366371
}
367372
if (resp.rc == 0) {
368373
resp.rc = wh_Server_DmaProcessClientAddress(
369374
server, req.data_hostaddr, &data, req.data_len,
370375
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
376+
if (resp.rc == 0) {
377+
data_dma_pre_ok = 1;
378+
}
371379
}
372380
if (resp.rc == 0) {
373381
rc = WH_SERVER_NVM_LOCK(server);
@@ -381,14 +389,15 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
381389
} /* WH_SERVER_NVM_LOCK() */
382390
resp.rc = rc;
383391
}
384-
if (resp.rc == 0) {
385-
/* perform platform-specific host address processing */
386-
resp.rc = wh_Server_DmaProcessClientAddress(
392+
/* Always call POST for successful PREs, regardless of operation
393+
* result */
394+
if (metadata_dma_pre_ok) {
395+
(void)wh_Server_DmaProcessClientAddress(
387396
server, req.metadata_hostaddr, &metadata, sizeof(whNvmMetadata),
388397
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
389398
}
390-
if (resp.rc == 0) {
391-
resp.rc = wh_Server_DmaProcessClientAddress(
399+
if (data_dma_pre_ok) {
400+
(void)wh_Server_DmaProcessClientAddress(
392401
server, req.data_hostaddr, &data, req.data_len,
393402
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
394403
}
@@ -405,6 +414,7 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
405414
whNvmMetadata meta = {0};
406415
whNvmSize read_len = 0;
407416
void* data = NULL;
417+
int data_dma_pre_ok = 0;
408418

409419
if (req_size != sizeof(req)) {
410420
/* Request is malformed */
@@ -441,15 +451,19 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
441451
rc = wh_Server_DmaProcessClientAddress(
442452
server, req.data_hostaddr, &data, req.data_len,
443453
WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
454+
if (rc == 0) {
455+
data_dma_pre_ok = 1;
456+
}
444457
}
445458
if (rc == 0) {
446459
/* Process the Read action */
447460
rc = wh_Nvm_ReadChecked(server->nvm, req.id, req.offset,
448461
read_len, (uint8_t*)data);
449462
}
450-
if (rc == 0) {
451-
/* perform platform-specific host address processing */
452-
rc = wh_Server_DmaProcessClientAddress(
463+
/* Always call POST for successful PRE, regardless of read
464+
* result */
465+
if (data_dma_pre_ok) {
466+
(void)wh_Server_DmaProcessClientAddress(
453467
server, req.data_hostaddr, &data, req.data_len,
454468
WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
455469
}

0 commit comments

Comments
 (0)