Skip to content

Commit a027bae

Browse files
bigbrettbigbrett
authored
port update fixes (#285)
* fix CMAC DMA message padding, fix types * Fix SHE alignment, fix crypto test key cache leakage * fix leakage of server-side keys when key usage enforcement fails * remove unused debug macro protection * restore old test printf behavior
1 parent 5273e1c commit a027bae

File tree

10 files changed

+293
-316
lines changed

10 files changed

+293
-316
lines changed

‎src/wh_server_crypto.c‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ static int _HandleRsaFunction( whServerContext* ctx, uint16_t magic,
451451
}
452452
}
453453
if (ret != WH_ERROR_OK) {
454-
return ret;
454+
goto cleanup;
455455
}
456456
}
457457
}
@@ -472,6 +472,7 @@ static int _HandleRsaFunction( whServerContext* ctx, uint16_t magic,
472472
/* free the key */
473473
wc_FreeRsaKey(rsa);
474474
}
475+
cleanup:
475476
if (evict != 0) {
476477
/* User requested to evict from cache, even if the call failed */
477478
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
@@ -930,7 +931,7 @@ static int _HandleEccSharedSecret(whServerContext* ctx, uint16_t magic,
930931
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, prv_key_id,
931932
WH_NVM_FLAGS_USAGE_DERIVE);
932933
if (ret != WH_ERROR_OK) {
933-
return ret;
934+
goto cleanup;
934935
}
935936
}
936937

@@ -965,6 +966,7 @@ static int _HandleEccSharedSecret(whServerContext* ctx, uint16_t magic,
965966
}
966967
wc_ecc_free(pub_key);
967968
}
969+
cleanup:
968970
if (evict_pub) {
969971
/* User requested to evict from cache, even if the call failed */
970972
(void)wh_Server_KeystoreEvictKey(ctx, pub_key_id);
@@ -1026,7 +1028,7 @@ static int _HandleEccSign(whServerContext* ctx, uint16_t magic,
10261028
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
10271029
WH_NVM_FLAGS_USAGE_SIGN);
10281030
if (ret != WH_ERROR_OK) {
1029-
return ret;
1031+
goto cleanup;
10301032
}
10311033
}
10321034

@@ -1053,6 +1055,7 @@ static int _HandleEccSign(whServerContext* ctx, uint16_t magic,
10531055
}
10541056
wc_ecc_free(key);
10551057
}
1058+
cleanup:
10561059
if (evict != 0) {
10571060
/* typecasting to void so that not overwrite ret */
10581061
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
@@ -1120,7 +1123,7 @@ static int _HandleEccVerify(whServerContext* ctx, uint16_t magic,
11201123
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
11211124
WH_NVM_FLAGS_USAGE_VERIFY);
11221125
if (ret != WH_ERROR_OK) {
1123-
return ret;
1126+
goto cleanup;
11241127
}
11251128
}
11261129

@@ -1162,6 +1165,8 @@ static int _HandleEccVerify(whServerContext* ctx, uint16_t magic,
11621165
}
11631166
wc_ecc_free(key);
11641167
}
1168+
1169+
cleanup:
11651170
if (evict != 0) {
11661171
/* User requested to evict from cache, even if the call failed */
11671172
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
@@ -1759,7 +1764,7 @@ static int _HandleCurve25519SharedSecret(whServerContext* ctx, uint16_t magic,
17591764
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, prv_key_id,
17601765
WH_NVM_FLAGS_USAGE_DERIVE);
17611766
if (ret != WH_ERROR_OK) {
1762-
return ret;
1767+
goto cleanup;
17631768
}
17641769
}
17651770

@@ -1796,6 +1801,7 @@ static int _HandleCurve25519SharedSecret(whServerContext* ctx, uint16_t magic,
17961801
}
17971802
wc_curve25519_free(priv);
17981803
}
1804+
cleanup:
17991805
if (evict_pub) {
18001806
/* User requested to evict from cache, even if the call failed */
18011807
(void)wh_Server_KeystoreEvictKey(ctx, pub_key_id);
@@ -1943,7 +1949,7 @@ static int _HandleEd25519Sign(whServerContext* ctx, uint16_t magic,
19431949
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
19441950
WH_NVM_FLAGS_USAGE_SIGN);
19451951
if (ret != WH_ERROR_OK) {
1946-
return ret;
1952+
goto cleanup;
19471953
}
19481954
}
19491955

@@ -1970,6 +1976,7 @@ static int _HandleEd25519Sign(whServerContext* ctx, uint16_t magic,
19701976
memcpy(res_sig, sig, sig_len);
19711977
}
19721978

1979+
cleanup:
19731980
if (evict) {
19741981
/* User requested to evict from cache, even if the call failed */
19751982
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
@@ -2043,7 +2050,7 @@ static int _HandleEd25519Verify(whServerContext* ctx, uint16_t magic,
20432050
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
20442051
WH_NVM_FLAGS_USAGE_VERIFY);
20452052
if (ret != WH_ERROR_OK) {
2046-
return ret;
2053+
goto cleanup;
20472054
}
20482055
}
20492056

@@ -2060,6 +2067,7 @@ static int _HandleEd25519Verify(whServerContext* ctx, uint16_t magic,
20602067
wc_ed25519_free(key);
20612068
}
20622069

2070+
cleanup:
20632071
if (evict != 0) {
20642072
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
20652073
}
@@ -2121,7 +2129,7 @@ static int _HandleEd25519SignDma(whServerContext* ctx, uint16_t magic,
21212129
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
21222130
WH_NVM_FLAGS_USAGE_SIGN);
21232131
if (ret != WH_ERROR_OK) {
2124-
return ret;
2132+
goto cleanup;
21252133
}
21262134
}
21272135

@@ -2168,6 +2176,7 @@ static int _HandleEd25519SignDma(whServerContext* ctx, uint16_t magic,
21682176
ctx, (uintptr_t)req.msg.addr, &msgAddr, req.msg.sz,
21692177
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
21702178

2179+
cleanup:
21712180
if (evict != 0) {
21722181
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
21732182
}
@@ -2227,7 +2236,7 @@ static int _HandleEd25519VerifyDma(whServerContext* ctx, uint16_t magic,
22272236
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
22282237
WH_NVM_FLAGS_USAGE_VERIFY);
22292238
if (ret != WH_ERROR_OK) {
2230-
return ret;
2239+
goto cleanup;
22312240
}
22322241
}
22332242

@@ -2272,6 +2281,7 @@ static int _HandleEd25519VerifyDma(whServerContext* ctx, uint16_t magic,
22722281
ctx, (uintptr_t)req.sig.addr, &sigAddr, req.sig.sz,
22732282
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
22742283

2284+
cleanup:
22752285
if (evict != 0) {
22762286
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
22772287
}
@@ -2974,7 +2984,7 @@ static int _HandleAesGcmDma(whServerContext* ctx, uint16_t magic, uint16_t seq,
29742984
* outKey must be at least AES_MAX_KEY_SIZE bytes. */
29752985
static int _CmacResolveKey(whServerContext* ctx, const uint8_t* requestKey,
29762986
uint32_t requestKeySz, whKeyId clientKeyId,
2977-
uint8_t* outKey, word32* outKeyLen)
2987+
uint8_t* outKey, uint32_t* outKeyLen)
29782988
{
29792989
int ret = WH_ERROR_OK;
29802990

@@ -3052,7 +3062,6 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, uint16_t seq,
30523062
return WH_ERROR_BADARGS;
30533063
}
30543064

3055-
word32 len;
30563065

30573066
/* Setup fixed size fields */
30583067
uint8_t* in =
@@ -3064,20 +3073,21 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, uint16_t seq,
30643073
memset(&res, 0, sizeof(res));
30653074

30663075
uint8_t tmpKey[AES_MAX_KEY_SIZE];
3067-
word32 tmpKeyLen = sizeof(tmpKey);
3076+
uint32_t tmpKeyLen = sizeof(tmpKey);
30683077
Cmac cmac[1];
30693078

30703079
/* Resolve the key to use */
30713080
ret = _CmacResolveKey(ctx, key, req.keySz, req.keyId, tmpKey, &tmpKeyLen);
30723081

30733082
/* Oneshot: input and output are both present */
30743083
if (ret == 0 && req.inSz != 0 && req.outSz != 0) {
3075-
len = req.outSz;
3084+
word32 len = (word32)req.outSz;
30763085

30773086
WH_DEBUG_SERVER_VERBOSE("cmac generate oneshot\n");
30783087

3079-
ret = wc_AesCmacGenerate_ex(cmac, out, &len, in, req.inSz, tmpKey,
3080-
tmpKeyLen, NULL, ctx->crypto->devId);
3088+
ret =
3089+
wc_AesCmacGenerate_ex(cmac, out, &len, in, req.inSz, tmpKey,
3090+
(word32)tmpKeyLen, NULL, ctx->crypto->devId);
30813091

30823092
if (ret == 0) {
30833093
res.outSz = len;
@@ -3113,10 +3123,10 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, uint16_t seq,
31133123

31143124
if (ret == 0 && req.outSz != 0) {
31153125
/* Finalize CMAC operation */
3116-
len = req.outSz;
3126+
word32 len = (word32)req.outSz;
31173127
WH_DEBUG_SERVER_VERBOSE("cmac final len:%d\n", len);
31183128
ret = wc_CmacFinal(cmac, out, &len);
3119-
res.outSz = len;
3129+
res.outSz = (uint32_t)len;
31203130
res.keyId = WH_KEYID_ERASED;
31213131
}
31223132
else if (ret == 0) {
@@ -3642,7 +3652,7 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic,
36423652
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
36433653
WH_NVM_FLAGS_USAGE_SIGN);
36443654
if (ret != WH_ERROR_OK) {
3645-
return ret;
3655+
goto cleanup;
36463656
}
36473657
}
36483658

@@ -3675,6 +3685,7 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic,
36753685
}
36763686
wc_MlDsaKey_Free(key);
36773687
}
3688+
cleanup:
36783689
if (evict != 0) {
36793690
/* User requested to evict from cache, even if the call failed */
36803691
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
@@ -3726,13 +3737,14 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic,
37263737
uint32_t sig_len = req.sigSz;
37273738
byte* req_sig =
37283739
(uint8_t*)(cryptoDataIn) + sizeof(whMessageCrypto_MlDsaVerifyRequest);
3740+
int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_VERIFY_OPTIONS_EVICT);
37293741

37303742
/* Validate key usage policy for verification */
37313743
if (!WH_KEYID_ISERASED(key_id)) {
37323744
ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id,
37333745
WH_NVM_FLAGS_USAGE_VERIFY);
37343746
if (ret != WH_ERROR_OK) {
3735-
return ret;
3747+
goto cleanup;
37363748
}
37373749
}
37383750

@@ -3746,8 +3758,7 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic,
37463758
return WH_ERROR_BADARGS;
37473759
}
37483760

3749-
byte* req_hash = req_sig + sig_len;
3750-
int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_VERIFY_OPTIONS_EVICT);
3761+
byte* req_hash = req_sig + sig_len;
37513762

37523763
/* Response message */
37533764
int result = 0;
@@ -3764,6 +3775,7 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic,
37643775
}
37653776
wc_MlDsaKey_Free(key);
37663777
}
3778+
cleanup:
37673779
if (evict != 0) {
37683780
/* User requested to evict from cache, even if the call failed */
37693781
(void)wh_Server_KeystoreEvictKey(ctx, key_id);
@@ -5111,7 +5123,7 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, uint16_t seq,
51115123
void* inAddr = NULL;
51125124

51135125
uint8_t tmpKey[AES_MAX_KEY_SIZE];
5114-
word32 tmpKeyLen = sizeof(tmpKey);
5126+
uint32_t tmpKeyLen = sizeof(tmpKey);
51155127
Cmac cmac[1];
51165128

51175129
/* Attempt oneshot if input and output are both present */
@@ -5137,16 +5149,16 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, uint16_t seq,
51375149
WH_DEBUG_SERVER_VERBOSE("dma cmac generate oneshot\n");
51385150

51395151
ret = wc_AesCmacGenerate_ex(cmac, out, &len, inAddr, req.input.sz,
5140-
tmpKey, tmpKeyLen, NULL,
5152+
tmpKey, (word32)tmpKeyLen, NULL,
51415153
ctx->crypto->devId);
51425154
}
51435155
else if (ret == WH_ERROR_OK) {
51445156
/* HSM-local key via keyId - init then generate */
51455157
WH_DEBUG_SERVER_VERBOSE("dma cmac generate oneshot with keyId:%x\n",
51465158
req.keyId);
51475159

5148-
ret = wc_InitCmac_ex(cmac, tmpKey, tmpKeyLen, WC_CMAC_AES, NULL,
5149-
NULL, ctx->crypto->devId);
5160+
ret = wc_InitCmac_ex(cmac, tmpKey, (word32)tmpKeyLen, WC_CMAC_AES,
5161+
NULL, NULL, ctx->crypto->devId);
51505162

51515163
if (ret == WH_ERROR_OK) {
51525164
ret =
@@ -5171,8 +5183,8 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, uint16_t seq,
51715183

51725184
/* Initialize CMAC context with key (re-derives k1/k2 subkeys) */
51735185
if (ret == 0) {
5174-
ret = wc_InitCmac_ex(cmac, tmpKey, tmpKeyLen, WC_CMAC_AES, NULL,
5175-
NULL, ctx->crypto->devId);
5186+
ret = wc_InitCmac_ex(cmac, tmpKey, (word32)tmpKeyLen, WC_CMAC_AES,
5187+
NULL, NULL, ctx->crypto->devId);
51765188
WH_DEBUG_SERVER_VERBOSE("dma cmac init with keylen:%d ret:%d\n",
51775189
tmpKeyLen, ret);
51785190
}

‎src/wh_server_she.c‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size,
459459
whNvmMetadata meta[1] = {0};
460460
uint32_t she_meta_count = 0;
461461
uint32_t she_meta_flags = 0;
462-
uint32_t* msg_counter_BE;
462+
uint32_t msg_counter_val;
463463

464464
whMessageShe_LoadKeyRequest req = {0};
465465
whMessageShe_LoadKeyResponse resp = {0};
@@ -561,18 +561,18 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size,
561561
sizeof(server->she->uid)) != 0) {
562562
ret = WH_SHE_ERC_KEY_UPDATE_ERROR;
563563
}
564-
/* verify msg_counter_BE is greater than stored value */
565-
msg_counter_BE = (uint32_t*)req.messageTwo;
564+
/* verify msg_counter_val is greater than stored value */
565+
memcpy(&msg_counter_val, req.messageTwo, sizeof(uint32_t));
566566
if (ret == 0 && keyRet != WH_ERROR_NOTFOUND &&
567-
wh_Utils_ntohl(*msg_counter_BE) >> 4 <= she_meta_count) {
567+
wh_Utils_ntohl(msg_counter_val) >> 4 <= she_meta_count) {
568568
ret = WH_SHE_ERC_KEY_UPDATE_ERROR;
569569
}
570570
/* write key with msg_counter_BE */
571571
if (ret == 0) {
572572
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
573573
_PopId(req.messageOne));
574574
she_meta_flags = _PopFlags(req.messageTwo);
575-
she_meta_count = wh_Utils_ntohl(*msg_counter_BE) >> 4;
575+
she_meta_count = wh_Utils_ntohl(msg_counter_val) >> 4;
576576
/* Update the meta label with new values */
577577
wh_She_Meta2Label(she_meta_count, she_meta_flags, meta->label);
578578
meta->len = WH_SHE_KEY_SZ;
@@ -619,8 +619,8 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size,
619619
}
620620
if (ret == 0) {
621621
/* Prepare counter in separate buffer */
622-
msg_counter_BE = (uint32_t*)counter_buffer;
623-
*msg_counter_BE = wh_Utils_htonl(she_meta_count << 4);
622+
msg_counter_val = wh_Utils_htonl(she_meta_count << 4);
623+
memcpy(counter_buffer, &msg_counter_val, sizeof(uint32_t));
624624
counter_buffer[3] |= 0x08;
625625

626626
/* First copy UID into messageFour */
@@ -714,7 +714,7 @@ static int _ExportRamKey(whServerContext* server, uint16_t magic,
714714
uint8_t cmacOutput[AES_BLOCK_SIZE];
715715
uint8_t tmpKey[WH_SHE_KEY_SZ];
716716
whNvmMetadata meta[1];
717-
uint32_t* counter;
717+
uint32_t counter_val;
718718
whMessageShe_ExportRamKeyResponse resp;
719719

720720
/* check if ram key was loaded by CMD_LOAD_PLAIN_KEY */
@@ -750,8 +750,8 @@ static int _ExportRamKey(whServerContext* server, uint16_t magic,
750750
/* set the counter, flags and ram key */
751751
memset(resp.messageTwo, 0, sizeof(resp.messageTwo));
752752
/* set count to 1 */
753-
counter = (uint32_t*)resp.messageTwo;
754-
*counter = (wh_Utils_htonl(1) << 4);
753+
counter_val = (wh_Utils_htonl(1) << 4);
754+
memcpy(resp.messageTwo, &counter_val, sizeof(uint32_t));
755755
keySz = WH_SHE_KEY_SZ;
756756
ret = wh_Server_KeystoreReadKey(
757757
server,
@@ -821,8 +821,9 @@ static int _ExportRamKey(whServerContext* server, uint16_t magic,
821821
if (ret == 0) {
822822
memset(resp.messageFour, 0, sizeof(resp.messageFour));
823823
/* set counter to 1, pad with 1 bit */
824-
counter = (uint32_t*)(resp.messageFour + WH_SHE_KEY_SZ);
825-
*counter = (wh_Utils_htonl(1) << 4);
824+
counter_val = (wh_Utils_htonl(1) << 4);
825+
memcpy(resp.messageFour + WH_SHE_KEY_SZ, &counter_val,
826+
sizeof(uint32_t));
826827
resp.messageFour[WH_SHE_KEY_SZ + 3] |= 0x08;
827828
/* encrypt the new counter */
828829
ret = wc_AesEncryptDirect(server->she->sheAes,

0 commit comments

Comments
 (0)