Skip to content

Commit a709b0a

Browse files
bigbrettbigbrett
committed
review: fix tag_len discrepancy, docs, and output args
1 parent 0d3f6c7 commit a709b0a

3 files changed

Lines changed: 260 additions & 25 deletions

File tree

‎docs/draft/async-crypto.md‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -541,10 +541,13 @@ int wh_Client_RngGenerateDma(whClientContext* ctx, uint8_t* out, uint32_t size);
541541
## AES: One-Shot with DMA Support
542542
543543
AES modes (CBC, CTR, ECB, GCM) are all **one-shot** operations — every call
544-
consumes a fixed buffer of input and returns a fixed buffer of output. There
545-
is no streaming state to accumulate across multiple Request/Response pairs
546-
the way SHA does, which makes the async split significantly simpler than
547-
SHA:
544+
consumes a fixed buffer of input and returns a fixed buffer of output in a
545+
single round-trip. There is no client-side partial-block accumulation across
546+
Request/Response pairs the way SHA's `Update` family has, which makes the
547+
async split significantly simpler than SHA. (CBC and CTR still carry
548+
inter-call IV / counter state on the `Aes` struct — see *Mutable state*
549+
below — but that state is updated atomically by the Response, not buffered
550+
across calls.)
548551
549552
- **No partial-block buffering** on the client. The entire plaintext or
550553
ciphertext is handed to one Request and the full result comes back in

‎src/wh_client_crypto.c‎

Lines changed: 80 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,10 @@ int wh_Client_AesCtrRequest(whClientContext* ctx, Aes* aes, int enc,
469469
memcpy(req_iv, (uint8_t*)aes->reg, AES_IV_SIZE);
470470
memcpy(req_tmp, (uint8_t*)aes->tmp, AES_BLOCK_SIZE);
471471

472+
WH_DEBUG_CLIENT_VERBOSE("AesCtr req: enc=%d keylen=%u insz=%u reqsz=%u\n",
473+
enc, (unsigned int)key_len, (unsigned int)len,
474+
(unsigned int)req_len);
475+
472476
return wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO,
473477
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
474478
}
@@ -507,6 +511,9 @@ int wh_Client_AesCtrResponse(whClientContext* ctx, Aes* aes, uint8_t* out,
507511
if (out_size != NULL) {
508512
*out_size = res->sz;
509513
}
514+
WH_DEBUG_CLIENT_VERBOSE("AesCtr res: outsz=%u left=%u\n",
515+
(unsigned int)res->sz,
516+
(unsigned int)res->left);
510517
}
511518
}
512519
return ret;
@@ -624,6 +631,10 @@ int wh_Client_AesCtrDmaRequest(whClientContext* ctx, Aes* aes, int enc,
624631
ctx->dma.asyncCtx.aes.outSz = outAcq ? len : 0;
625632
ctx->dma.asyncCtx.aes.aadSz = 0;
626633

634+
WH_DEBUG_CLIENT_VERBOSE(
635+
"AesCtr DMA req: enc=%d keysz=%u insz=%u reqsz=%u\n", enc,
636+
(unsigned int)req->keySz, (unsigned int)len, (unsigned int)req_len);
637+
627638
ret = wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO_DMA,
628639
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
629640
}
@@ -667,14 +678,15 @@ int wh_Client_AesCtrDmaResponse(whClientContext* ctx, Aes* aes)
667678

668679
if (ret == WH_ERROR_OK) {
669680
ret = _getCryptoResponse(dataPtr, WC_CIPHER_AES_CTR, (uint8_t**)&res);
670-
if (ret >= WH_ERROR_OK) {
681+
if (ret == WH_ERROR_OK) {
671682
uint8_t* res_iv =
672683
(uint8_t*)res + sizeof(whMessageCrypto_AesCtrDmaResponse);
673684
uint8_t* res_tmp = res_iv + AES_IV_SIZE;
674685
aes->left = res->left;
675686
memcpy((uint8_t*)aes->reg, res_iv, AES_IV_SIZE);
676687
memcpy((uint8_t*)aes->tmp, res_tmp, AES_BLOCK_SIZE);
677-
ret = WH_ERROR_OK;
688+
WH_DEBUG_CLIENT_VERBOSE("AesCtr DMA res: left=%u\n",
689+
(unsigned int)res->left);
678690
}
679691
}
680692

@@ -775,6 +787,11 @@ int wh_Client_AesEcbRequest(whClientContext* ctx, Aes* aes, int enc,
775787
memcpy(req_key, (const uint8_t*)(aes->devKey), key_len);
776788
}
777789

790+
WH_DEBUG_CLIENT_VERBOSE(
791+
"AesEcb req: enc=%d keylen=%u insz=%u blocks=%u reqsz=%u\n", enc,
792+
(unsigned int)key_len, (unsigned int)len, (unsigned int)blocks,
793+
(unsigned int)req_len);
794+
778795
return wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO,
779796
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
780797
}
@@ -809,6 +826,8 @@ int wh_Client_AesEcbResponse(whClientContext* ctx, Aes* aes, uint8_t* out,
809826
if (out_size != NULL) {
810827
*out_size = res->sz;
811828
}
829+
WH_DEBUG_CLIENT_VERBOSE("AesEcb res: outsz=%u\n",
830+
(unsigned int)res->sz);
812831
}
813832
}
814833
return ret;
@@ -920,6 +939,10 @@ int wh_Client_AesEcbDmaRequest(whClientContext* ctx, Aes* aes, int enc,
920939
ctx->dma.asyncCtx.aes.outSz = outAcq ? len : 0;
921940
ctx->dma.asyncCtx.aes.aadSz = 0;
922941

942+
WH_DEBUG_CLIENT_VERBOSE(
943+
"AesEcb DMA req: enc=%d keysz=%u insz=%u reqsz=%u\n", enc,
944+
(unsigned int)req->keySz, (unsigned int)len, (unsigned int)req_len);
945+
923946
ret = wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO_DMA,
924947
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
925948
}
@@ -965,8 +988,8 @@ int wh_Client_AesEcbDmaResponse(whClientContext* ctx, Aes* aes)
965988

966989
if (ret == WH_ERROR_OK) {
967990
ret = _getCryptoResponse(dataPtr, WC_CIPHER_AES_ECB, (uint8_t**)&res);
968-
if (ret >= 0) {
969-
ret = WH_ERROR_OK;
991+
if (ret == WH_ERROR_OK) {
992+
WH_DEBUG_CLIENT_VERBOSE("AesEcb DMA res: ok\n");
970993
}
971994
}
972995

@@ -1083,6 +1106,11 @@ int wh_Client_AesCbcRequest(whClientContext* ctx, Aes* aes, int enc,
10831106
memcpy(req_iv, iv, iv_len);
10841107
}
10851108

1109+
WH_DEBUG_CLIENT_VERBOSE(
1110+
"AesCbc req: enc=%d keylen=%u ivsz=%u insz=%u reqsz=%u\n", enc,
1111+
(unsigned int)key_len, (unsigned int)iv_len, (unsigned int)len,
1112+
(unsigned int)req_len);
1113+
10861114
return wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO,
10871115
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
10881116
}
@@ -1119,6 +1147,8 @@ int wh_Client_AesCbcResponse(whClientContext* ctx, Aes* aes, uint8_t* out,
11191147
if (out_size != NULL) {
11201148
*out_size = res->sz;
11211149
}
1150+
WH_DEBUG_CLIENT_VERBOSE("AesCbc res: outsz=%u\n",
1151+
(unsigned int)res->sz);
11221152
}
11231153
}
11241154

@@ -1237,6 +1267,10 @@ int wh_Client_AesCbcDmaRequest(whClientContext* ctx, Aes* aes, int enc,
12371267
ctx->dma.asyncCtx.aes.outSz = outAcq ? len : 0;
12381268
ctx->dma.asyncCtx.aes.aadSz = 0;
12391269

1270+
WH_DEBUG_CLIENT_VERBOSE(
1271+
"AesCbc DMA req: enc=%d keysz=%u insz=%u reqsz=%u\n", enc,
1272+
(unsigned int)req->keySz, (unsigned int)len, (unsigned int)req_len);
1273+
12401274
ret = wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO_DMA,
12411275
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
12421276
}
@@ -1280,11 +1314,11 @@ int wh_Client_AesCbcDmaResponse(whClientContext* ctx, Aes* aes)
12801314

12811315
if (ret == WH_ERROR_OK) {
12821316
ret = _getCryptoResponse(dataPtr, WC_CIPHER_AES_CBC, (uint8_t**)&res);
1283-
if (ret >= WH_ERROR_OK) {
1317+
if (ret == WH_ERROR_OK) {
12841318
uint8_t* res_iv =
12851319
(uint8_t*)res + sizeof(whMessageCrypto_AesCbcDmaResponse);
12861320
memcpy((uint8_t*)aes->reg, res_iv, AES_IV_SIZE);
1287-
ret = WH_ERROR_OK;
1321+
WH_DEBUG_CLIENT_VERBOSE("AesCbc DMA res: ok\n");
12881322
}
12891323
}
12901324

@@ -1409,6 +1443,12 @@ int wh_Client_AesGcmRequest(whClientContext* ctx, Aes* aes, int enc,
14091443
memcpy(req_tag, dec_tag, tag_len);
14101444
}
14111445

1446+
WH_DEBUG_CLIENT_VERBOSE(
1447+
"AesGcm req: enc=%d keylen=%u ivsz=%u insz=%u authinsz=%u tagsz=%u "
1448+
"reqsz=%u\n",
1449+
enc, (unsigned int)key_len, (unsigned int)iv_len, (unsigned int)len,
1450+
(unsigned int)authin_len, (unsigned int)tag_len, (unsigned int)req_len);
1451+
14121452
return wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO,
14131453
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
14141454
}
@@ -1426,10 +1466,12 @@ int wh_Client_AesGcmResponse(whClientContext* ctx, Aes* aes, uint8_t* out,
14261466

14271467
(void)aes;
14281468

1429-
/* out may be NULL (e.g. GMAC: tag-only, no plaintext). If provided the
1430-
* response payload is copied to it after validating that the server's
1431-
* reported size fits within out_capacity. */
1432-
if (ctx == NULL || (out == NULL && out_capacity > 0)) {
1469+
/* out may be NULL (e.g. GMAC: tag-only, no plaintext). The downstream
1470+
* copy guards on `out != NULL && res->sz > 0`, so out_capacity is
1471+
* checked only when out is actually used. Rejecting `out == NULL &&
1472+
* out_capacity > 0` here would leave a stale response in the comm
1473+
* queue after Request already sent. */
1474+
if (ctx == NULL) {
14331475
return WH_ERROR_BADARGS;
14341476
}
14351477

@@ -1441,7 +1483,7 @@ int wh_Client_AesGcmResponse(whClientContext* ctx, Aes* aes, uint8_t* out,
14411483
ret = wh_Client_RecvResponse(ctx, &group, &action, &res_len, dataPtr);
14421484
if (ret == WH_ERROR_OK) {
14431485
ret = _getCryptoResponse(dataPtr, WC_CIPHER_AES_GCM, (uint8_t**)&res);
1444-
if (ret >= WH_ERROR_OK) {
1486+
if (ret == WH_ERROR_OK) {
14451487
uint8_t* res_out = (uint8_t*)(res + 1);
14461488
uint8_t* res_tag = res_out + res->sz;
14471489

@@ -1454,11 +1496,15 @@ int wh_Client_AesGcmResponse(whClientContext* ctx, Aes* aes, uint8_t* out,
14541496
if (out_size != NULL) {
14551497
*out_size = res->sz;
14561498
}
1457-
if (enc_tag != NULL && res->authTagSz > 0 &&
1458-
res->authTagSz <= tag_len) {
1499+
if (enc_tag != NULL && res->authTagSz > 0) {
1500+
if (res->authTagSz > tag_len) {
1501+
return WH_ERROR_ABORTED;
1502+
}
14591503
memcpy(enc_tag, res_tag, res->authTagSz);
14601504
}
1461-
ret = WH_ERROR_OK;
1505+
WH_DEBUG_CLIENT_VERBOSE("AesGcm res: outsz=%u tagsz=%u\n",
1506+
(unsigned int)res->sz,
1507+
(unsigned int)res->authTagSz);
14621508
}
14631509
}
14641510
return ret;
@@ -1600,6 +1646,13 @@ int wh_Client_AesGcmDmaRequest(whClientContext* ctx, Aes* aes, int enc,
16001646
ctx->dma.asyncCtx.aes.aadClientAddr = (uintptr_t)authin;
16011647
ctx->dma.asyncCtx.aes.aadSz = aadAcq ? authin_len : 0;
16021648

1649+
WH_DEBUG_CLIENT_VERBOSE(
1650+
"AesGcm DMA req: enc=%d keysz=%u ivsz=%u insz=%u authinsz=%u "
1651+
"tagsz=%u reqsz=%u\n",
1652+
enc, (unsigned int)req->keySz, (unsigned int)iv_len,
1653+
(unsigned int)len, (unsigned int)authin_len, (unsigned int)tag_len,
1654+
(unsigned int)req_len);
1655+
16031656
ret = wh_Client_SendRequest(ctx, WH_MESSAGE_GROUP_CRYPTO_DMA,
16041657
WC_ALGO_TYPE_CIPHER, req_len, dataPtr);
16051658
}
@@ -1651,14 +1704,20 @@ int wh_Client_AesGcmDmaResponse(whClientContext* ctx, Aes* aes,
16511704

16521705
if (ret == WH_ERROR_OK) {
16531706
ret = _getCryptoResponse(dataPtr, WC_CIPHER_AES_GCM, (uint8_t**)&res);
1654-
if (ret >= WH_ERROR_OK) {
1655-
if (enc_tag != NULL && res->authTagSz > 0 &&
1656-
res->authTagSz <= tag_len) {
1657-
uint8_t* res_tag =
1658-
(uint8_t*)res + sizeof(whMessageCrypto_AesGcmDmaResponse);
1659-
memcpy(enc_tag, res_tag, res->authTagSz);
1707+
if (ret == WH_ERROR_OK) {
1708+
if (enc_tag != NULL && res->authTagSz > 0) {
1709+
if (res->authTagSz > tag_len) {
1710+
ret = WH_ERROR_ABORTED;
1711+
}
1712+
else {
1713+
uint8_t* res_tag =
1714+
(uint8_t*)res +
1715+
sizeof(whMessageCrypto_AesGcmDmaResponse);
1716+
memcpy(enc_tag, res_tag, res->authTagSz);
1717+
}
16601718
}
1661-
ret = WH_ERROR_OK;
1719+
WH_DEBUG_CLIENT_VERBOSE("AesGcm DMA res: tagsz=%u\n",
1720+
(unsigned int)res->authTagSz);
16621721
}
16631722
}
16641723

0 commit comments

Comments
 (0)