Skip to content

Commit 1461aaf

Browse files
aidangarskeaidangarske
committed
Security fixes: constant-time comparisons, ForceZero, and sizeof fix
 Replace XMEMCMP with ConstantCompare for cryptographic verification to prevent timing attacks in: - wc_PKCS12_verify (pkcs12.c) - wolfSSL_EVP_DigestVerifyFinal HMAC check (evp.c) - wc_SrpVerifyPeersProof (srp.c) - wc_ecc_decrypt HMAC verification (ecc.c) - wc_AesSivEncryptDecrypt SIV comparison (aes.c) - wc_AesKeyUnWrap_ex IV verification (aes.c) - sakke_decapsulate authentication check (sakke.c) Add ForceZero to clear sensitive cryptographic material before freeing: - HPKE: dh, kemContext, sharedSecret, context buffers (hpke.c) - ChaCha20-Poly1305: authKey in Init functions (chacha20_poly1305.c) - ML-KEM: kr and msg buffers in encapsulate/decapsulate (wc_mlkem.c) - ECIES: sharedSecret and keys buffers (ecc.c) - PBKDF1/2: digest and buffer arrays (pwdbased.c) - SRP: digest buffer (srp.c) Fix copy-paste error in ALLOC_ASNSETDATA macro using sizeof(ASNSetData) instead of sizeof(ASNGetData) (asn.c).
1 parent a08efc9 commit 1461aaf

11 files changed

Lines changed: 44 additions & 14 deletions

File tree

‎wolfcrypt/src/aes.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14643,7 +14643,7 @@ int wc_AesKeyUnWrap_ex(Aes *aes, const byte* in, word32 inSz, byte* out,
1464314643
return ret;
1464414644

1464514645
/* verify IV */
14646-
if (XMEMCMP(tmp, expIv, KEYWRAP_BLOCK_SIZE) != 0)
14646+
if (ConstantCompare(tmp, expIv, KEYWRAP_BLOCK_SIZE) != 0)
1464714647
return BAD_KEYWRAP_IV_E;
1464814648

1464914649
return (int)(inSz - KEYWRAP_BLOCK_SIZE);
@@ -16303,7 +16303,7 @@ static WARN_UNUSED_RESULT int AesSivCipher(
1630316303
WOLFSSL_MSG("S2V failed.");
1630416304
}
1630516305

16306-
if (XMEMCMP(siv, sivTmp, WC_AES_BLOCK_SIZE) != 0) {
16306+
if (ConstantCompare(siv, sivTmp, WC_AES_BLOCK_SIZE) != 0) {
1630716307
WOLFSSL_MSG("Computed SIV doesn't match received SIV.");
1630816308
ret = AES_SIV_AUTH_E;
1630916309
}

‎wolfcrypt/src/asn.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ static word32 SizeASNLength(word32 length)
487487
#define ALLOC_ASNSETDATA(name, cnt, err, heap) \
488488
do { \
489489
if ((err) == 0) { \
490-
(name) = (ASNSetData*)XMALLOC(sizeof(ASNGetData) * (cnt), (heap), \
490+
(name) = (ASNSetData*)XMALLOC(sizeof(ASNSetData) * (cnt), (heap), \
491491
DYNAMIC_TYPE_TMP_BUFFER); \
492492
if ((name) == NULL) { \
493493
(err) = MEMORY_E; \

‎wolfcrypt/src/chacha20_poly1305.c‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ int wc_ChaCha20Poly1305_Init(ChaChaPoly_Aead* aead,
187187
aead->state = CHACHA20_POLY1305_STATE_READY;
188188
}
189189

190+
ForceZero(authKey, sizeof(authKey));
191+
190192
return ret;
191193
}
192194

@@ -332,25 +334,30 @@ int wc_XChaCha20Poly1305_Init(
332334
/* Create the Poly1305 key */
333335
if ((ret = wc_Chacha_Process(&aead->chacha, authKey, authKey,
334336
(word32)sizeof authKey)) < 0)
335-
return ret;
337+
goto out;
336338
/* advance to start of the next ChaCha block. */
337339
wc_Chacha_purge_current_block(&aead->chacha);
338340

339341
/* Initialize Poly1305 context */
340342
if ((ret = wc_Poly1305SetKey(&aead->poly, authKey,
341343
(word32)sizeof authKey)) < 0)
342-
return ret;
344+
goto out;
343345

344346
if ((ret = wc_Poly1305Update(&aead->poly, ad, (word32)ad_len)) < 0)
345-
return ret;
347+
goto out;
346348

347349
if ((ret = wc_Poly1305_Pad(&aead->poly, (word32)ad_len)) < 0)
348-
return ret;
350+
goto out;
349351

350352
aead->isEncrypt = isEncrypt ? 1 : 0;
351353
aead->state = CHACHA20_POLY1305_STATE_AAD;
352354

353-
return 0;
355+
ret = 0;
356+
357+
out:
358+
ForceZero(authKey, sizeof(authKey));
359+
360+
return ret;
354361
}
355362

356363
static WC_INLINE int wc_XChaCha20Poly1305_crypt_oneshot(

‎wolfcrypt/src/ecc.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14484,6 +14484,8 @@ int wc_ecc_encrypt_ex(ecc_key* privKey, ecc_key* pubKey, const byte* msg,
1448414484

1448514485
RESTORE_VECTOR_REGISTERS();
1448614486

14487+
ForceZero(sharedSecret, sharedSz);
14488+
ForceZero(keys, (word32)keysLen);
1448714489
WC_FREE_VAR_EX(sharedSecret, ctx->heap, DYNAMIC_TYPE_ECC_BUFFER);
1448814490
WC_FREE_VAR_EX(keys, ctx->heap, DYNAMIC_TYPE_ECC_BUFFER);
1448914491

@@ -14778,8 +14780,8 @@ int wc_ecc_decrypt(ecc_key* privKey, ecc_key* pubKey, const byte* msg,
1477814780

1477914781
if (ret == 0)
1478014782
ret = wc_HmacFinal(hmac, verify);
14781-
if ((ret == 0) && (XMEMCMP(verify, msg + msgSz - digestSz,
14782-
digestSz) != 0)) {
14783+
if ((ret == 0) && (ConstantCompare(verify, msg + msgSz - digestSz,
14784+
(int)digestSz) != 0)) {
1478314785
ret = HASH_TYPE_E;
1478414786
WOLFSSL_MSG("ECC Decrypt HMAC Check failed!");
1478514787
}
@@ -14882,6 +14884,8 @@ int wc_ecc_decrypt(ecc_key* privKey, ecc_key* pubKey, const byte* msg,
1488214884
if (pubKey == peerKey)
1488314885
wc_ecc_free(peerKey);
1488414886
#endif
14887+
ForceZero(sharedSecret, sharedSz);
14888+
ForceZero(keys, (word32)keysLen);
1488514889
#ifdef WOLFSSL_SMALL_STACK
1488614890
#ifndef WOLFSSL_ECIES_OLD
1488714891
XFREE(peerKey, ctx->heap, DYNAMIC_TYPE_ECC_BUFFER);

‎wolfcrypt/src/evp.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4962,7 +4962,7 @@ int wolfSSL_EVP_DigestVerifyFinal(WOLFSSL_EVP_MD_CTX *ctx,
49624962

49634963
if (ctx->isHMAC) {
49644964
/* Check HMAC result matches the signature. */
4965-
if (XMEMCMP(sig, digest, (size_t)siglen) == 0)
4965+
if (ConstantCompare(sig, digest, (int)siglen) == 0)
49664966
return WOLFSSL_SUCCESS;
49674967
return WOLFSSL_FAILURE;
49684968
}

‎wolfcrypt/src/hpke.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,8 @@ static int wc_HpkeEncap(Hpke* hpke, void* ephemeralKey, void* receiverKey,
796796
hpke->Npk * 2, sharedSecret);
797797
}
798798

799+
ForceZero(dh, hpke->Ndh);
800+
ForceZero(kemContext, hpke->Npk * 2);
799801
WC_FREE_VAR_EX(dh, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
800802
WC_FREE_VAR_EX(kemContext, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
801803

@@ -827,6 +829,7 @@ static int wc_HpkeSetupBaseSender(Hpke* hpke, HpkeBaseContext* context,
827829
infoSz);
828830
}
829831

832+
ForceZero(sharedSecret, hpke->Nsecret);
830833
WC_FREE_VAR_EX(sharedSecret, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
831834

832835
return ret;
@@ -914,6 +917,7 @@ int wc_HpkeSealBase(Hpke* hpke, void* ephemeralKey, void* receiverKey,
914917

915918
PRIVATE_KEY_LOCK();
916919

920+
ForceZero(context, sizeof(HpkeBaseContext));
917921
WC_FREE_VAR_EX(context, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
918922

919923
return ret;
@@ -1032,6 +1036,7 @@ static int wc_HpkeDecap(Hpke* hpke, void* receiverKey, const byte* pubKey,
10321036
hpke->Npk * 2, sharedSecret);
10331037
}
10341038

1039+
ForceZero(dh, hpke->Ndh);
10351040
WC_FREE_VAR_EX(dh, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
10361041
WC_FREE_VAR_EX(kemContext, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
10371042

@@ -1058,6 +1063,7 @@ static int wc_HpkeSetupBaseReceiver(Hpke* hpke, HpkeBaseContext* context,
10581063
infoSz);
10591064
}
10601065

1066+
ForceZero(sharedSecret, hpke->Nsecret);
10611067
WC_FREE_VAR_EX(sharedSecret, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
10621068

10631069
return ret;
@@ -1144,6 +1150,7 @@ int wc_HpkeOpenBase(Hpke* hpke, void* receiverKey, const byte* pubKey,
11441150

11451151
PRIVATE_KEY_LOCK();
11461152

1153+
ForceZero(context, sizeof(HpkeBaseContext));
11471154
WC_FREE_VAR_EX(context, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
11481155

11491156
return ret;

‎wolfcrypt/src/pkcs12.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,9 @@ static int wc_PKCS12_verify(WC_PKCS12* pkcs12, byte* data, word32 dataSz,
637637
}
638638
#endif
639639

640-
return XMEMCMP(digest, mac->digest, mac->digestSz);
640+
if (ConstantCompare(digest, mac->digest, (int)mac->digestSz) != 0)
641+
return MAC_CMP_FAILED_E;
642+
return 0;
641643
}
642644

643645
int wc_PKCS12_verify_ex(WC_PKCS12* pkcs12, const byte* psw, word32 pswSz)

‎wolfcrypt/src/pwdbased.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ int wc_PBKDF1_ex(byte* key, int keyLen, byte* iv, int ivLen,
152152

153153
WC_FREE_VAR_EX(hash, heap, DYNAMIC_TYPE_HASHCTX);
154154

155+
ForceZero(digest, sizeof(digest));
156+
155157
if (err != 0)
156158
return err;
157159

@@ -294,6 +296,7 @@ int wc_PBKDF2_ex(byte* output, const byte* passwd, int pLen, const byte* salt,
294296
wc_HmacFree(hmac);
295297
}
296298

299+
ForceZero(buffer, hLen);
297300
WC_FREE_VAR_EX(buffer, heap, DYNAMIC_TYPE_TMP_BUFFER);
298301
WC_FREE_VAR_EX(hmac, heap, DYNAMIC_TYPE_HMAC);
299302

‎wolfcrypt/src/sakke.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6941,7 +6941,7 @@ int wc_DeriveSakkeSSV(SakkeKey* key, enum wc_HashType hashType, byte* ssv,
69416941

69426942
err = sakke_compute_point_r(key, key->id, key->idSz, ri, n, test);
69436943
}
6944-
if ((err == 0) && (XMEMCMP(auth, test, (size_t)(2 * n + 1)) != 0)) {
6944+
if ((err == 0) && (ConstantCompare(auth, test, (int)(2 * n + 1)) != 0)) {
69456945
err = SAKKE_VERIFY_FAIL_E;
69466946
}
69476947

‎wolfcrypt/src/srp.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -994,9 +994,11 @@ int wc_SrpVerifyPeersProof(Srp* srp, byte* proof, word32 size)
994994
if (!r) r = SrpHashUpdate(&srp->server_proof, srp->key, srp->keySz);
995995
}
996996

997-
if (!r && XMEMCMP(proof, digest, size) != 0)
997+
if (!r && ConstantCompare(proof, digest, (int)size) != 0)
998998
r = SRP_VERIFY_E;
999999

1000+
ForceZero(digest, sizeof(digest));
1001+
10001002
return r;
10011003
}
10021004

0 commit comments

Comments
 (0)