Skip to content

Commit 0c67d7a

Browse files
authored
Merge pull request #10080 from JeremiahM37/fenrir-issues
Fenrir fixes
2 parents 7fd41b1 + 9c38953 commit 0c67d7a

11 files changed

Lines changed: 102 additions & 32 deletions

File tree

src/internal.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,8 @@ static int ImportKeyState(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
11801180
word16 i, wordCount, wordAdj = 0;
11811181

11821182
/* do window */
1183+
if (idx + OPAQUE16_LEN > len)
1184+
return BUFFER_E;
11831185
ato16(exp + idx, &wordCount);
11841186
idx += OPAQUE16_LEN;
11851187

@@ -1188,6 +1190,8 @@ static int ImportKeyState(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
11881190
wordCount = WOLFSSL_DTLS_WINDOW_WORDS;
11891191
}
11901192

1193+
if (idx + (wordCount * OPAQUE32_LEN) + wordAdj > len)
1194+
return BUFFER_E;
11911195
XMEMSET(keys->peerSeq[0].window, 0xFF, DTLS_SEQ_SZ);
11921196
for (i = 0; i < wordCount; i++) {
11931197
ato32(exp + idx, &keys->peerSeq[0].window[i]);
@@ -1196,6 +1200,9 @@ static int ImportKeyState(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
11961200
idx += wordAdj;
11971201

11981202
/* do prevWindow */
1203+
wordAdj = 0;
1204+
if (idx + OPAQUE16_LEN > len)
1205+
return BUFFER_E;
11991206
ato16(exp + idx, &wordCount);
12001207
idx += OPAQUE16_LEN;
12011208

@@ -1204,6 +1211,8 @@ static int ImportKeyState(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
12041211
wordCount = WOLFSSL_DTLS_WINDOW_WORDS;
12051212
}
12061213

1214+
if (idx + (wordCount * OPAQUE32_LEN) + wordAdj > len)
1215+
return BUFFER_E;
12071216
XMEMSET(keys->peerSeq[0].prevWindow, 0xFF, DTLS_SEQ_SZ);
12081217
for (i = 0; i < wordCount; i++) {
12091218
ato32(exp + idx, &keys->peerSeq[0].prevWindow[i]);

wolfcrypt/src/port/caam/wolfcaam_ecdsa.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ static int wc_CAAM_DevEccSign(const byte* in, int inlen, byte* out,
8787
/* private key */
8888
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(key), pk, keySz) != MP_OKAY)
8989
{
90+
ForceZero(pk, sizeof(pk));
9091
return MP_TO_E;
9192
}
9293

@@ -108,10 +109,12 @@ static int wc_CAAM_DevEccSign(const byte* in, int inlen, byte* out,
108109
mp_free(&mps);
109110
if (ret != 0) {
110111
WOLFSSL_MSG("Issue converting to signature\n");
112+
ForceZero(pk, sizeof(pk));
111113
return -1;
112114
}
113115
}
114116

117+
ForceZero(pk, sizeof(pk));
115118
return ret;
116119
}
117120

@@ -201,6 +204,7 @@ static int wc_CAAM_DevEcdh(ecc_key* private_key, ecc_key* public_key, byte* out,
201204
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(private_key), pk, keySz) !=
202205
MP_OKAY) {
203206
WOLFSSL_MSG("error getting private key buffer");
207+
ForceZero(pk, sizeof(pk));
204208
return MP_TO_E;
205209
}
206210

@@ -209,6 +213,7 @@ static int wc_CAAM_DevEcdh(ecc_key* private_key, ecc_key* public_key, byte* out,
209213
if (ret == 0) {
210214
*outlen = keySz;
211215
}
216+
ForceZero(pk, sizeof(pk));
212217
return ret;
213218
}
214219

@@ -237,10 +242,12 @@ static int wc_CAAM_DevMakeEccKey(WC_RNG* rng, int keySize, ecc_key* key,
237242
ret = wc_DevCryptoEccKeyGen(curveId, blackKey, s, keySize, xy, keySize*2);
238243
if (wc_ecc_import_unsigned(key, xy, xy + keySize, s, curveId) != 0) {
239244
WOLFSSL_MSG("issue importing key");
245+
ForceZero(s, sizeof(s));
240246
return -1;
241247
}
242248
key->blackKey = blackKey;
243249

250+
ForceZero(s, sizeof(s));
244251
(void)rng;
245252
return ret;
246253
}
@@ -340,13 +347,15 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
340347
if (key->blackKey == CAAM_BLACK_KEY_CCM) {
341348
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(key), pk,
342349
keySz + WC_CAAM_MAC_SZ) != MP_OKAY) {
350+
ForceZero(pk, sizeof(pk));
343351
return MP_TO_E;
344352
}
345353
buf[idx].Length = keySz + WC_CAAM_MAC_SZ;
346354
}
347355
else {
348356
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(key), pk, keySz) !=
349357
MP_OKAY) {
358+
ForceZero(pk, sizeof(pk));
350359
return MP_TO_E;
351360
}
352361
buf[idx].Length = keySz;
@@ -376,8 +385,10 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
376385
args[3] = keySz;
377386

378387
ret = wc_caamAddAndWait(buf, idx, args, CAAM_ECDSA_SIGN);
379-
if (ret != 0)
388+
if (ret != 0) {
389+
ForceZero(pk, sizeof(pk));
380390
return -1;
391+
}
381392

382393
/* convert signature from raw bytes to signature format */
383394
{
@@ -394,10 +405,12 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
394405
mp_free(&mps);
395406
if (ret != 0) {
396407
WOLFSSL_MSG("Issue converting to signature");
408+
ForceZero(pk, sizeof(pk));
397409
return -1;
398410
}
399411
}
400412

413+
ForceZero(pk, sizeof(pk));
401414
(void)devId;
402415
return MP_OKAY;
403416
}
@@ -610,13 +623,15 @@ int wc_CAAM_Ecdh(ecc_key* private_key, ecc_key* public_key, byte* out,
610623
if (private_key->blackKey == CAAM_BLACK_KEY_CCM) {
611624
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(private_key), pk,
612625
keySz + WC_CAAM_MAC_SZ) != MP_OKAY) {
626+
ForceZero(pk, sizeof(pk));
613627
return MP_TO_E;
614628
}
615629
buf[idx].Length = keySz + WC_CAAM_MAC_SZ;
616630
}
617631
else {
618632
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(private_key), pk,
619633
keySz) != MP_OKAY) {
634+
ForceZero(pk, sizeof(pk));
620635
return MP_TO_E;
621636
}
622637
buf[idx].Length = keySz;
@@ -646,6 +661,7 @@ int wc_CAAM_Ecdh(ecc_key* private_key, ecc_key* public_key, byte* out,
646661
args[2] = ecdsel;
647662
args[3] = keySz;
648663
ret = wc_caamAddAndWait(buf, idx, args, CAAM_ECDSA_ECDH);
664+
ForceZero(pk, sizeof(pk));
649665
(void)devId;
650666
if (ret == 0) {
651667
*outlen = keySz;
@@ -737,20 +753,25 @@ int wc_CAAM_MakeEccKey(WC_RNG* rng, int keySize, ecc_key* key, int curveId,
737753
key->blackKey = (pt[0] << 24) | (pt[1] << 16) | (pt[2] << 8) | pt[3];
738754
if (wc_ecc_import_unsigned(key, xy, xy + keySize, NULL, curveId) != 0) {
739755
WOLFSSL_MSG("issue importing public key");
756+
ForceZero(s, sizeof(s));
740757
return -1;
741758
}
742759
key->partNum = args[2];
760+
ForceZero(s, sizeof(s));
743761
return MP_OKAY;
744762
}
745763
else if (ret == 0) {
746764
if (wc_ecc_import_unsigned(key, xy, xy + keySize,
747765
s, curveId) != 0) {
748766
WOLFSSL_MSG("issue importing key");
767+
ForceZero(s, sizeof(s));
749768
return -1;
750769
}
751770
key->blackKey = args[0];
771+
ForceZero(s, sizeof(s));
752772
return MP_OKAY;
753773
}
774+
ForceZero(s, sizeof(s));
754775
return -1;
755776
}
756777
#endif /* WOLFSSL_KEY_GEN */

wolfcrypt/src/port/caam/wolfcaam_fsl_nxp.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,12 +443,14 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
443443
if (key->blackKey == CAAM_BLACK_KEY_CCM) {
444444
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(key), k,
445445
kSz + WC_CAAM_MAC_SZ) != MP_OKAY) {
446+
ForceZero(k, sizeof(k));
446447
return MP_TO_E;
447448
}
448449
}
449450
else {
450451
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(key), k, kSz) !=
451452
MP_OKAY) {
453+
ForceZero(k, sizeof(k));
452454
return MP_TO_E;
453455
}
454456
}
@@ -457,6 +459,7 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
457459
ecdsel = GetECDSEL(dp->id);
458460
if (ecdsel == 0) {
459461
WOLFSSL_MSG("unknown key type or size");
462+
ForceZero(k, sizeof(k));
460463
return CRYPTOCB_UNAVAILABLE;
461464
}
462465

@@ -469,6 +472,7 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
469472
break;
470473
default:
471474
WOLFSSL_MSG("unknown/unsupported key type");
475+
ForceZero(k, sizeof(k));
472476
return BAD_FUNC_ARG;
473477
}
474478

@@ -508,10 +512,12 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,
508512
mp_free(&mps);
509513
if (ret != 0) {
510514
WOLFSSL_MSG("Issue converting to signature");
515+
ForceZero(k, sizeof(k));
511516
return -1;
512517
}
513518
}
514519

520+
ForceZero(k, sizeof(k));
515521
return ret;
516522
}
517523

@@ -697,22 +703,26 @@ int wc_CAAM_Ecdh(ecc_key* private_key, ecc_key* public_key, byte* out,
697703
if (private_key->blackKey == CAAM_BLACK_KEY_CCM) {
698704
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(private_key), k,
699705
keySz + WC_CAAM_MAC_SZ) != MP_OKAY) {
706+
ForceZero(k, sizeof(k));
700707
return MP_TO_E;
701708
}
702709
}
703710
else {
704711
if (mp_to_unsigned_bin_len(wc_ecc_key_get_priv(private_key), k, keySz)
705712
!= MP_OKAY) {
713+
ForceZero(k, sizeof(k));
706714
return MP_TO_E;
707715
}
708716
}
709717

710718
if (*outlen < (word32)keySz) {
719+
ForceZero(k, sizeof(k));
711720
return -1;
712721
}
713722

714723
status = CAAM_ECC_ECDH(CAAM, &hndl, k, keySz, qxy, keySz*2, out, keySz,
715724
ecdsel, enc);
725+
ForceZero(k, sizeof(k));
716726
if (status == kStatus_Success) {
717727
*outlen = keySz;
718728
return MP_OKAY;
@@ -762,17 +772,22 @@ int wc_CAAM_MakeEccKey(WC_RNG* rng, int keySize, ecc_key* key, int curveId,
762772
return CRYPTOCB_UNAVAILABLE;
763773
}
764774

765-
if (key->blackKey == CAAM_BLACK_KEY_ECB) {
775+
switch (key->blackKey) {
776+
case CAAM_BLACK_KEY_ECB:
766777
enc = CAAM_PKHA_ENC_PRI_AESECB;
767-
}
768-
769-
if (key->blackKey == 0) {
778+
break;
779+
case 0:
770780
#ifdef WOLFSSL_CAAM_NO_BLACK_KEY
771781
enc = 0;
772782
#else
773783
key->blackKey = CAAM_BLACK_KEY_ECB;
774784
enc = CAAM_PKHA_ENC_PRI_AESECB;
775785
#endif
786+
break;
787+
default:
788+
WOLFSSL_MSG("unknown/unsupported key type");
789+
ForceZero(k, sizeof(k));
790+
return BAD_FUNC_ARG;
776791
}
777792

778793
status = CAAM_ECC_Keygen(CAAM, &hndl, k, &kSz, xy, &xySz, ecdsel,
@@ -787,6 +802,7 @@ int wc_CAAM_MakeEccKey(WC_RNG* rng, int keySize, ecc_key* key, int curveId,
787802
ret = -1;
788803
}
789804

805+
ForceZero(k, sizeof(k));
790806
return ret;
791807
}
792808
#endif /* WOLFSSL_KEY_GEN */

wolfcrypt/src/port/devcrypto/devcrypto_rsa.c

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,10 @@ static int _PrivateOperation(const byte* in, word32 inlen, byte* out,
156156
byte* u = NULL;
157157
byte* n = NULL;
158158
word32 dSz, pSz, qSz, dpSz = 0, dqSz = 0, uSz = 0, nSz;
159+
word32 dAllocSz;
159160

160161
dev = &key->ctx;
161-
dSz = nSz = wc_RsaEncryptSize(key);
162+
dAllocSz = dSz = nSz = wc_RsaEncryptSize(key);
162163
pSz = qSz = nSz / 2;
163164
if (outlen < dSz) {
164165
WOLFSSL_MSG("Output buffer is too small");
@@ -196,7 +197,7 @@ static int _PrivateOperation(const byte* in, word32 inlen, byte* out,
196197
if (!key->blackKey) { /* @TODO unexpected results with black key CRT form */
197198
if (ret == 0 && dpSz > 0) {
198199
dSz = 0; nSz = 0;
199-
dq = (byte*)XMALLOC(dpSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
200+
dq = (byte*)XMALLOC(dqSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
200201
dp = (byte*)XMALLOC(dpSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
201202
u = (byte*)XMALLOC(uSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
202203
if (dq == NULL || dp == NULL || u == NULL) {
@@ -237,12 +238,12 @@ static int _PrivateOperation(const byte* in, word32 inlen, byte* out,
237238
}
238239
}
239240

240-
XFREE(d, NULL, DYNAMIC_TYPE_TMP_BUFFER);
241-
XFREE(p, NULL, DYNAMIC_TYPE_TMP_BUFFER);
242-
XFREE(q, NULL, DYNAMIC_TYPE_TMP_BUFFER);
243-
XFREE(dp, NULL, DYNAMIC_TYPE_TMP_BUFFER);
244-
XFREE(dq, NULL, DYNAMIC_TYPE_TMP_BUFFER);
245-
XFREE(u, NULL, DYNAMIC_TYPE_TMP_BUFFER);
241+
if (d) { ForceZero(d, dAllocSz); XFREE(d, NULL, DYNAMIC_TYPE_TMP_BUFFER); }
242+
if (p) { ForceZero(p, pSz); XFREE(p, NULL, DYNAMIC_TYPE_TMP_BUFFER); }
243+
if (q) { ForceZero(q, qSz); XFREE(q, NULL, DYNAMIC_TYPE_TMP_BUFFER); }
244+
if (dp) { ForceZero(dp, dpSz); XFREE(dp, NULL, DYNAMIC_TYPE_TMP_BUFFER); }
245+
if (dq) { ForceZero(dq, dqSz); XFREE(dq, NULL, DYNAMIC_TYPE_TMP_BUFFER); }
246+
if (u) { ForceZero(u, uSz); XFREE(u, NULL, DYNAMIC_TYPE_TMP_BUFFER); }
246247
XFREE(n, NULL, DYNAMIC_TYPE_TMP_BUFFER);
247248

248249
wc_DevCryptoFree(dev);
@@ -540,13 +541,13 @@ int wc_DevCrypto_MakeRsaKey(RsaKey* key, int size, long e, WC_RNG* rng)
540541
#endif
541542
}
542543

543-
XFREE(p, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
544-
XFREE(q, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
545-
XFREE(dp, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
546-
XFREE(dq, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
547-
XFREE(c, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
544+
if (p) { ForceZero(p, pSz); XFREE(p, key->heap, DYNAMIC_TYPE_TMP_BUFFER); }
545+
if (q) { ForceZero(q, qSz); XFREE(q, key->heap, DYNAMIC_TYPE_TMP_BUFFER); }
546+
if (dp) { ForceZero(dp, dpSz); XFREE(dp, key->heap, DYNAMIC_TYPE_TMP_BUFFER); }
547+
if (dq) { ForceZero(dq, dqSz); XFREE(dq, key->heap, DYNAMIC_TYPE_TMP_BUFFER); }
548+
if (c) { ForceZero(c, cSz); XFREE(c, key->heap, DYNAMIC_TYPE_TMP_BUFFER); }
548549
XFREE(n, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
549-
XFREE(d, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
550+
if (d) { ForceZero(d, dSz); XFREE(d, key->heap, DYNAMIC_TYPE_TMP_BUFFER); }
550551

551552
(void)rng;
552553
return ret;

wolfcrypt/src/port/kcapi/kcapi_dh.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,10 @@ static int KcapiDh_SetPrivKey(DhKey* key)
127127
}
128128
}
129129

130-
XFREE(priv, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
130+
if (priv != NULL) {
131+
ForceZero(priv, len);
132+
XFREE(priv, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
133+
}
131134
return ret;
132135
}
133136
#endif

wolfcrypt/src/port/kcapi/kcapi_ecc.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ int KcapiEcc_LoadKey(ecc_key* key, byte* pubkey_raw, word32* pubkey_sz,
120120
if (ret == 0) {
121121
ret = kcapi_kpp_setkey(key->handle, priv, keySz);
122122
}
123+
ForceZero(priv, sizeof(priv));
123124
}
124125
else {
125126
/* generate new ephemeral key */
@@ -241,6 +242,7 @@ int KcapiEcc_SharedSecret(ecc_key* private_key, ecc_key* public_key, byte* out,
241242
ret = 0;
242243
}
243244
}
245+
ForceZero(priv, sizeof(priv));
244246
}
245247
if (ret == 0) {
246248
#ifdef KCAPI_USE_XMALLOC
@@ -317,6 +319,7 @@ static int KcapiEcc_SetPrivKey(ecc_key* key)
317319
}
318320
}
319321

322+
ForceZero(priv, sizeof(priv));
320323
return ret;
321324
}
322325

0 commit comments

Comments
 (0)