Skip to content

Commit 318cd62

Browse files
SparkiDevSparkiDev
authored
Merge pull request #10231 from JeremiahM37/fenrir-issues-3
Fix PEM input validation and zeroize sensitive key buffers
2 parents 460463a + e182645 commit 318cd62

4 files changed

Lines changed: 36 additions & 3 deletions

File tree

‎tests/api.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11872,6 +11872,10 @@ static int test_wc_CertPemToDer(void)
1187211872
(int)cert_dersz, CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
1187311873
ExpectIntEQ(wc_CertPemToDer(cert_buf, (int)cert_sz, cert_der, -1,
1187411874
CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
11875+
ExpectIntEQ(wc_CertPemToDer(cert_buf, -1, cert_der, (int)cert_dersz,
11876+
CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
11877+
ExpectIntEQ(wc_CertPemToDer(cert_buf, 0, cert_der, (int)cert_dersz,
11878+
CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
1187511879

1187611880
if (cert_der != NULL)
1187711881
free(cert_der);
@@ -11928,6 +11932,12 @@ static int test_wc_KeyPemToDer(void)
1192811932
ExpectIntEQ(wc_KeyPemToDer(cert_buf, cert_sz, (byte*)&cert_der, 0, ""),
1192911933
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
1193011934

11935+
/* Bad arg: negative or zero pemSz */
11936+
ExpectIntEQ(wc_KeyPemToDer(cert_buf, -1, (byte*)&cert_der, cert_sz, ""),
11937+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
11938+
ExpectIntEQ(wc_KeyPemToDer(cert_buf, 0, (byte*)&cert_der, cert_sz, ""),
11939+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
11940+
1193111941
/* Test normal operation */
1193211942
cert_dersz = cert_sz; /* DER will be smaller than PEM */
1193311943
ExpectNotNull(cert_der = (byte*)malloc((size_t)cert_dersz));
@@ -11971,6 +11981,10 @@ static int test_wc_PubKeyPemToDer(void)
1197111981
ExpectIntEQ(load_file(key, &cert_buf, &cert_sz), 0);
1197211982
cert_dersz = cert_sz; /* DER will be smaller than PEM */
1197311983
ExpectNotNull(cert_der = (byte*)malloc(cert_dersz));
11984+
ExpectIntEQ(wc_PubKeyPemToDer(cert_buf, -1, cert_der, (int)cert_dersz),
11985+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
11986+
ExpectIntEQ(wc_PubKeyPemToDer(cert_buf, 0, cert_der, (int)cert_dersz),
11987+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
1197411988
ExpectIntGE(wc_PubKeyPemToDer(cert_buf, (int)cert_sz, cert_der,
1197511989
(int)cert_dersz), 0);
1197611990
if (cert_der != NULL) {

‎wolfcrypt/src/asn.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24381,7 +24381,7 @@ int wc_KeyPemToDer(const unsigned char* pem, int pemSz,
2438124381

2438224382
WOLFSSL_ENTER("wc_KeyPemToDer");
2438324383

24384-
if (pem == NULL || (buff != NULL && buffSz <= 0)) {
24384+
if (pem == NULL || (buff != NULL && buffSz <= 0) || pemSz <= 0) {
2438524385
WOLFSSL_MSG("Bad pem der args");
2438624386
return BAD_FUNC_ARG;
2438724387
}
@@ -24432,7 +24432,7 @@ int wc_CertPemToDer(const unsigned char* pem, int pemSz,
2443224432

2443324433
WOLFSSL_ENTER("wc_CertPemToDer");
2443424434

24435-
if (pem == NULL || buff == NULL || buffSz <= 0) {
24435+
if (pem == NULL || buff == NULL || buffSz <= 0 || pemSz <= 0) {
2443624436
WOLFSSL_MSG("Bad pem der args");
2443724437
return BAD_FUNC_ARG;
2443824438
}
@@ -24479,7 +24479,7 @@ int wc_PubKeyPemToDer(const unsigned char* pem, int pemSz,
2447924479

2448024480
WOLFSSL_ENTER("wc_PubKeyPemToDer");
2448124481

24482-
if (pem == NULL || (buff != NULL && buffSz <= 0)) {
24482+
if (pem == NULL || (buff != NULL && buffSz <= 0) || pemSz <= 0) {
2448324483
WOLFSSL_MSG("Bad pem der args");
2448424484
return BAD_FUNC_ARG;
2448524485
}

‎wolfcrypt/src/ecc.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7669,6 +7669,11 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
76697669
/* 3.2 c. Set K = 0x00 0x00 ... */
76707670
XMEMSET(K, 0x00, KSz);
76717671

7672+
#ifdef WOLFSSL_CHECK_MEM_ZERO
7673+
wc_MemZero_Add("wc_ecc_gen_deterministic_k K", K, KSz);
7674+
wc_MemZero_Add("wc_ecc_gen_deterministic_k V", V, VSz);
7675+
#endif
7676+
76727677
if (ret == 0) {
76737678
ret = mp_init(z1); /* always init z1 and free z1 */
76747679
}
@@ -7811,6 +7816,8 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
78117816
}
78127817

78137818
ForceZero(x, MAX_ECC_BYTES);
7819+
ForceZero(K, WC_MAX_DIGEST_SIZE);
7820+
ForceZero(V, WC_MAX_DIGEST_SIZE);
78147821
#ifdef WOLFSSL_SMALL_STACK
78157822
XFREE(z1, heap, DYNAMIC_TYPE_ECC_BUFFER);
78167823
XFREE(x, heap, DYNAMIC_TYPE_PRIVATE_KEY);
@@ -7819,6 +7826,8 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
78197826
XFREE(h1, heap, DYNAMIC_TYPE_DIGEST);
78207827
#elif defined(WOLFSSL_CHECK_MEM_ZERO)
78217828
wc_MemZero_Check(x, MAX_ECC_BYTES);
7829+
wc_MemZero_Check(K, WC_MAX_DIGEST_SIZE);
7830+
wc_MemZero_Check(V, WC_MAX_DIGEST_SIZE);
78227831
#endif
78237832

78247833
return ret;

‎wolfcrypt/src/pkcs12.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1976,6 +1976,7 @@ static int wc_PKCS12_create_key_bag(WC_PKCS12* pkcs12, WC_RNG* rng,
19761976
word32 sz;
19771977
word32 i;
19781978
word32 tmpSz;
1979+
word32 tmpAllocSz;
19791980
int ret;
19801981

19811982
/* get max size for shrouded key */
@@ -2014,6 +2015,7 @@ static int wc_PKCS12_create_key_bag(WC_PKCS12* pkcs12, WC_RNG* rng,
20142015
}
20152016

20162017
/* shroud key */
2018+
tmpAllocSz = length;
20172019
tmp = (byte*)XMALLOC(length, heap, DYNAMIC_TYPE_TMP_BUFFER);
20182020
if (tmp == NULL) {
20192021
return MEMORY_E;
@@ -2022,11 +2024,13 @@ static int wc_PKCS12_create_key_bag(WC_PKCS12* pkcs12, WC_RNG* rng,
20222024
ret = wc_PKCS12_shroud_key(pkcs12, rng, tmp, &length, key, keySz,
20232025
algo, pass, passSz, iter);
20242026
if (ret < 0) {
2027+
ForceZero(tmp, tmpAllocSz);
20252028
XFREE(tmp, heap, DYNAMIC_TYPE_TMP_BUFFER);
20262029
return ret;
20272030
}
20282031
length = (word32)ret;
20292032
XMEMCPY(out + idx, tmp, (size_t)length);
2033+
ForceZero(tmp, tmpAllocSz);
20302034
XFREE(tmp, heap, DYNAMIC_TYPE_TMP_BUFFER);
20312035
totalSz += length;
20322036

@@ -2357,6 +2361,7 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
23572361
{
23582362
byte* keyBuf;
23592363
word32 keyBufSz = 0;
2364+
word32 keyBufAllocSz = 0;
23602365
byte* keyCi = NULL;
23612366
word32 tmpSz;
23622367
int ret;
@@ -2406,6 +2411,7 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
24062411

24072412
/* account for sequence around bag */
24082413
keyBufSz += MAX_SEQ_SZ;
2414+
keyBufAllocSz = keyBufSz;
24092415
keyBuf = (byte*)XMALLOC(keyBufSz, heap, DYNAMIC_TYPE_TMP_BUFFER);
24102416
if (keyBuf == NULL) {
24112417
WOLFSSL_MSG("Memory error creating keyBuf buffer");
@@ -2415,6 +2421,7 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
24152421
ret = wc_PKCS12_create_key_bag(pkcs12, rng, keyBuf + MAX_SEQ_SZ, &keyBufSz,
24162422
key, keySz, algo, iter, pass, (int)passSz);
24172423
if (ret < 0) {
2424+
ForceZero(keyBuf, keyBufAllocSz);
24182425
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
24192426
WOLFSSL_MSG("Error creating key bag");
24202427
return NULL;
@@ -2437,18 +2444,21 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
24372444
ret = wc_PKCS12_encrypt_content(pkcs12, rng, NULL, keyCiSz,
24382445
NULL, keyBufSz, algo, pass, (int)passSz, iter, WC_PKCS12_DATA);
24392446
if (ret != WC_NO_ERR_TRACE(LENGTH_ONLY_E)) {
2447+
ForceZero(keyBuf, keyBufAllocSz);
24402448
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
24412449
WOLFSSL_MSG("Error getting key encrypt content size");
24422450
return NULL;
24432451
}
24442452
keyCi = (byte*)XMALLOC(*keyCiSz, heap, DYNAMIC_TYPE_TMP_BUFFER);
24452453
if (keyCi == NULL) {
2454+
ForceZero(keyBuf, keyBufAllocSz);
24462455
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
24472456
return NULL;
24482457
}
24492458

24502459
ret = wc_PKCS12_encrypt_content(pkcs12, rng, keyCi, keyCiSz,
24512460
keyBuf, keyBufSz, algo, pass, (int)passSz, iter, WC_PKCS12_DATA);
2461+
ForceZero(keyBuf, keyBufAllocSz);
24522462
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
24532463
if (ret < 0 ) {
24542464
XFREE(keyCi, heap, DYNAMIC_TYPE_TMP_BUFFER);

0 commit comments

Comments
 (0)