Skip to content

Commit d8ae61d

Browse files
ColtonWilleyColtonWilley
authored
Merge pull request #377 from aidangarske/fenrir-fixes
Fix Fenrir Issues in wolfProvider extend test suite coverage
2 parents 3de3a16 + c29ac26 commit d8ae61d

24 files changed

Lines changed: 1239 additions & 88 deletions

‎src/wp_aes_aead.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,6 +1690,10 @@ static void *wp_aes_gcm_newctx(WOLFPROV_CTX* provCtx, size_t keyBits)
16901690
*/
16911691
static void wp_aes_gcm_freectx(wp_AeadCtx* ctx)
16921692
{
1693+
OPENSSL_free(ctx->aad);
1694+
#if defined(WP_HAVE_AESGCM) && !defined(WOLFSSL_AESGCM_STREAM)
1695+
OPENSSL_free(ctx->in);
1696+
#endif
16931697
wc_AesFree(&ctx->aes);
16941698
OPENSSL_free(ctx);
16951699
}

‎src/wp_aes_block.c‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -899,21 +899,22 @@ static int wp_aes_block_final_dec(wp_AesBlockCtx* ctx, unsigned char *out,
899899

900900
if (ok && ctx->pad) {
901901
unsigned char pad;
902+
unsigned char invalid;
903+
unsigned char i;
902904

903905
pad = ctx->buf[AES_BLOCK_SIZE - 1];
904-
if ((pad == 0) || (pad > AES_BLOCK_SIZE)) {
906+
invalid = wp_ct_byte_mask_eq(pad, 0) |
907+
~wp_ct_int_mask_gte(AES_BLOCK_SIZE, (int)pad);
908+
for (i = 0; i < AES_BLOCK_SIZE; i++) {
909+
unsigned char mask = wp_ct_int_mask_gte((int)i,
910+
AES_BLOCK_SIZE - (int)pad);
911+
invalid |= mask & wp_ct_byte_mask_ne(ctx->buf[i], pad);
912+
}
913+
if (invalid) {
905914
ok = 0;
906915
}
907-
if (ok) {
908-
unsigned char len = AES_BLOCK_SIZE;
909-
unsigned char i;
910-
911-
for (i = 0; i < pad; i++) {
912-
if (ctx->buf[--len] != pad) {
913-
return 0;
914-
}
915-
}
916-
ctx->bufSz = len;
916+
else {
917+
ctx->bufSz = AES_BLOCK_SIZE - pad;
917918
}
918919
}
919920

‎src/wp_cmac.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,16 @@ static wp_CmacCtx* wp_cmac_dup(wp_CmacCtx* src)
165165
dst = wp_cmac_new(NULL);
166166
}
167167
if (dst != NULL) {
168-
*dst = *src;
169-
dst->keyLen = 0;
170-
171-
if ((src->keyLen != 0) &&
172-
(!wp_cmac_set_key(dst, src->key, src->keyLen, 0))) {
168+
/* Copy the entire context to preserve in-progress CMAC state. */
169+
XMEMCPY(&dst->cmac, &src->cmac, sizeof(Cmac));
170+
dst->type = src->type;
171+
dst->size = src->size;
172+
dst->expKeySize = src->expKeySize;
173+
if (src->keyLen <= sizeof(dst->key)) {
174+
XMEMCPY(dst->key, src->key, src->keyLen);
175+
dst->keyLen = src->keyLen;
176+
}
177+
else {
173178
wp_cmac_free(dst);
174179
dst = NULL;
175180
}

‎src/wp_des.c‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -427,10 +427,15 @@ static int wp_des3_block_update(wp_Des3BlockCtx *ctx, unsigned char *out,
427427
int i;
428428
unsigned char off = inLen % DES_BLOCK_SIZE;
429429
unsigned char pad = DES_BLOCK_SIZE - off - 1;
430-
for (i = off; i < DES_BLOCK_SIZE; i++) {
431-
out[inLen - off + i] = pad;
430+
if (outSize < inLen + pad + 1) {
431+
ok = 0;
432+
}
433+
if (ok) {
434+
for (i = off; i < DES_BLOCK_SIZE; i++) {
435+
out[inLen - off + i] = pad;
436+
}
437+
inLen += pad + 1;
432438
}
433-
inLen += pad + 1;
434439
}
435440
if (ctx->bufSz != 0) {
436441
size_t len = DES_BLOCK_SIZE - ctx->bufSz;
@@ -587,21 +592,22 @@ static int wp_des3_block_final_dec(wp_Des3BlockCtx* ctx, unsigned char *out,
587592

588593
if (ok && ctx->pad) {
589594
unsigned char pad;
595+
unsigned char invalid;
596+
unsigned char i;
590597

591598
pad = ctx->buf[DES_BLOCK_SIZE - 1];
592-
if ((pad == 0) || (pad > DES_BLOCK_SIZE)) {
599+
invalid = wp_ct_byte_mask_eq(pad, 0) |
600+
~wp_ct_int_mask_gte(DES_BLOCK_SIZE, (int)pad);
601+
for (i = 0; i < DES_BLOCK_SIZE; i++) {
602+
unsigned char mask = wp_ct_int_mask_gte((int)i,
603+
DES_BLOCK_SIZE - (int)pad);
604+
invalid |= mask & wp_ct_byte_mask_ne(ctx->buf[i], pad);
605+
}
606+
if (invalid) {
593607
ok = 0;
594608
}
595-
if (ok) {
596-
unsigned char len = DES_BLOCK_SIZE;
597-
unsigned char i;
598-
599-
for (i = 0; i < pad; i++) {
600-
if (ctx->buf[--len] != pad) {
601-
return 0;
602-
}
603-
}
604-
ctx->bufSz = len;
609+
else {
610+
ctx->bufSz = DES_BLOCK_SIZE - pad;
605611
}
606612
}
607613

‎src/wp_dh_kmgmt.c‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ void wp_dh_free(wp_Dh* dh)
453453
if (cnt == 0) {
454454
/* No more references to this object. */
455455
OPENSSL_free(dh->pub);
456-
OPENSSL_free(dh->priv);
456+
OPENSSL_clear_free(dh->priv, dh->privSz);
457457
#ifndef WP_SINGLE_THREADED
458458
wc_FreeMutex(&dh->mutex);
459459
#endif
@@ -730,6 +730,9 @@ static int wp_dh_get_params_encoded_public_key(wp_Dh* dh, OSSL_PARAM params[])
730730
if (p->data_size < outLen) {
731731
ok = 0;
732732
}
733+
if (ok && (dh->pubSz > outLen)) {
734+
ok = 0;
735+
}
733736
if (ok) {
734737
unsigned char* data = p->data;
735738
size_t padSz = outLen - dh->pubSz;
@@ -863,16 +866,6 @@ static int wp_dh_get_params(wp_Dh* dh, OSSL_PARAM params[])
863866
}
864867
}
865868
}
866-
if (ok) {
867-
/* Only call if we haven't already handled OSSL_PKEY_PARAM_PRIV_KEY */
868-
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_PRIV_KEY);
869-
if (p == NULL || p->data != NULL) {
870-
if (!wp_params_set_octet_string_be(params, OSSL_PKEY_PARAM_PRIV_KEY,
871-
dh->priv, dh->privSz)) {
872-
ok = 0;
873-
}
874-
}
875-
}
876869
if (ok && (!wp_dh_get_params_encoded_public_key(dh, params))) {
877870
ok = 0;
878871
}

‎src/wp_drbg.c‎

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -337,29 +337,53 @@ static int wp_drbg_reseed(wp_DrbgCtx* ctx, int predResist,
337337
const unsigned char* addIn, size_t addInLen)
338338
{
339339
int ok = 1;
340+
int rc;
341+
unsigned char *seed = NULL;
342+
size_t seedLen = 0;
340343

341344
WOLFPROV_ENTER(WP_LOG_COMP_RNG, "wp_drbg_reseed");
342345

343-
#if 0
344-
/* Calling Hash_DRBG_Instantiate would be better. */
345-
int rc;
346-
rc = wc_RNG_DRBG_Reseed(ctx->rng, entropy, entropyLen);
347-
if (rc != 0) {
348-
ok = 0;
346+
/* If no entropy provided, get fresh entropy from the OS source. */
347+
if (entropy == NULL || entropyLen == 0) {
348+
seedLen = 48;
349+
seed = OPENSSL_malloc(seedLen);
350+
if (seed == NULL) {
351+
ok = 0;
352+
}
353+
if (ok) {
354+
OS_Seed osSeed;
355+
rc = wc_GenerateSeed(&osSeed, seed, (word32)seedLen);
356+
if (rc != 0) {
357+
ok = 0;
358+
}
359+
else {
360+
entropy = seed;
361+
entropyLen = seedLen;
362+
}
363+
}
349364
}
350-
if (ok && (addInLen > 0)) {
351-
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, addInLen);
365+
366+
if (ok && entropy != NULL && entropyLen > 0) {
367+
rc = wc_RNG_DRBG_Reseed(ctx->rng, entropy, (word32)entropyLen);
352368
if (rc != 0) {
369+
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
370+
"wc_RNG_DRBG_Reseed", rc);
371+
ok = 0;
372+
}
373+
}
374+
if (ok && (addInLen > 0) && (addIn != NULL)) {
375+
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, (word32)addInLen);
376+
if (rc != 0) {
377+
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
378+
"wc_RNG_DRBG_Reseed", rc);
353379
ok = 0;
354380
}
355381
}
356-
#else
357-
(void)ctx;
358-
(void)entropy;
359-
(void)entropyLen;
360-
(void)addIn;
361-
(void)addInLen;
362-
#endif
382+
383+
/* Securely clear and free locally allocated seed buffer. */
384+
if (seed != NULL) {
385+
OPENSSL_clear_free(seed, seedLen);
386+
}
363387

364388
(void)predResist;
365389

@@ -391,6 +415,7 @@ static int wp_drbg_enable_locking(wp_DrbgCtx* ctx)
391415
if (rc != 0) {
392416
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG, "wc_InitMutex", rc);
393417
OPENSSL_free(ctx->mutex);
418+
ctx->mutex = NULL;
394419
ok = 0;
395420
}
396421
}
@@ -550,11 +575,16 @@ static int wp_drbg_set_ctx_params(wp_DrbgCtx* ctx, const OSSL_PARAM params[])
550575
*/
551576
static int wp_drbg_verify_zeroization(wp_DrbgCtx* ctx)
552577
{
578+
int ok;
579+
553580
WOLFPROV_ENTER(WP_LOG_COMP_RNG, "wp_drbg_verify_zeroization");
554581

555-
(void)ctx;
556-
WOLFPROV_LEAVE(WP_LOG_COMP_RNG, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), 1);
557-
return 1;
582+
/* After uninstantiate, ctx->rng is freed (with internal state zeroized
583+
* by wolfSSL) and set to NULL. Verify that cleanup occurred. */
584+
ok = (ctx->rng == NULL);
585+
586+
WOLFPROV_LEAVE(WP_LOG_COMP_RNG, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
587+
return ok;
558588
}
559589

560590
/**

‎src/wp_ecx_kmgmt.c‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,16 +356,46 @@ static wp_Ecx* wp_ecx_dup(const wp_Ecx* src, int selection)
356356
{
357357
wp_Ecx* dst = NULL;
358358

359-
(void)selection;
360359
if (wolfssl_prov_is_running()) {
361360
/* Create a new ecx object. */
362361
dst = wp_ecx_new(src->provCtx, src->data);
363362
}
364363
if (dst != NULL) {
365-
XMEMCPY(&dst->key, &src->key, sizeof(src->key));
366364
dst->includePublic = src->includePublic;
367-
dst->hasPub = src->hasPub;
368-
dst->hasPriv = src->hasPriv;
365+
366+
/* Copy the full key union to preserve internal wolfSSL state.
367+
* Private material is zeroized below if not selected. */
368+
XMEMCPY(&dst->key, &src->key, sizeof(src->key));
369+
370+
/* Set public key flag if available and requested. */
371+
if (src->hasPub &&
372+
((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0)) {
373+
dst->hasPub = 1;
374+
}
375+
/* Set private key flag if available and requested. */
376+
if (src->hasPriv &&
377+
((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0)) {
378+
dst->hasPriv = 1;
379+
dst->clamped = src->clamped;
380+
XMEMCPY(dst->unclamped, src->unclamped, sizeof(src->unclamped));
381+
}
382+
else {
383+
/* Private key not selected — re-import only public key to
384+
* ensure no private material remains in the dst key object. */
385+
if (dst->hasPub) {
386+
byte buf[64];
387+
word32 len = (word32)sizeof(buf);
388+
int rc = (*src->data->exportPub)((void*)&src->key, buf, &len,
389+
ECX_LITTLE_ENDIAN);
390+
if (rc == 0) {
391+
/* Re-init key and import only public part. */
392+
(*dst->data->freeKey)((void*)&dst->key);
393+
(*dst->data->initKey)((void*)&dst->key);
394+
(*dst->data->importPub)(buf, len, (void*)&dst->key,
395+
ECX_LITTLE_ENDIAN);
396+
}
397+
}
398+
}
369399
}
370400

371401
return dst;

‎src/wp_hkdf.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,9 @@ static int wp_kdf_tls1_3_derive(wp_HkdfCtx* ctx, unsigned char* key,
716716
ok = 0;
717717
}
718718
}
719+
else {
720+
ok = 0;
721+
}
719722
}
720723

721724
WOLFPROV_LEAVE(WP_LOG_COMP_HKDF, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);

‎src/wp_hmac.c‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,22 @@ static wp_HmacCtx* wp_hmac_dup(wp_HmacCtx* src)
187187
dst = wp_hmac_new(src->provCtx);
188188
}
189189
if (dst != NULL) {
190-
*dst = *src;
191-
dst->key = NULL;
192-
dst->keyLen = 0;
190+
int ok = 1;
193191

194-
if ((src->key != NULL) &&
192+
dst->type = src->type;
193+
dst->size = src->size;
194+
dst->provCtx = src->provCtx;
195+
196+
/* Copy the Hmac struct directly to preserve in-progress state.
197+
* wc_HmacCopy is not available in all wolfSSL versions. */
198+
XMEMCPY(&dst->hmac, &src->hmac, sizeof(Hmac));
199+
200+
if (ok && (src->key != NULL) &&
195201
(!wp_hmac_set_key(dst, src->key, src->keyLen, 0))) {
202+
ok = 0;
203+
}
204+
205+
if (!ok) {
196206
wp_hmac_free(dst);
197207
dst = NULL;
198208
}

‎src/wp_mac_kmgmt.c‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,11 @@ void wp_mac_free(wp_Mac* mac)
223223
int rc;
224224

225225
rc = wc_LockMutex(&mac->mutex);
226-
cnt = --mac->refCnt;
227-
if (rc == 0) {
228-
wc_UnLockMutex(&mac->mutex);
226+
if (rc != 0) {
227+
return;
229228
}
229+
cnt = --mac->refCnt;
230+
wc_UnLockMutex(&mac->mutex);
230231
#else
231232
cnt = --mac->refCnt;
232233
#endif
@@ -318,6 +319,10 @@ static int wp_mac_has(const wp_Mac* mac, int selection)
318319
if (mac == NULL) {
319320
ok = 0;
320321
}
322+
if (ok && ((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0)) {
323+
/* MAC keys do not have a public key component. */
324+
ok = 0;
325+
}
321326
if (ok && ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0)) {
322327
ok &= mac->key != NULL;
323328
}
@@ -344,11 +349,13 @@ static int wp_mac_match(const wp_Mac* mac1, const wp_Mac* mac2, int selection)
344349
if (!wolfssl_prov_is_running()) {
345350
ok = 0;
346351
}
347-
if (ok && ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) &&
348-
(mac1->keyLen != MAX_SIZE_T) && ((mac1->keyLen != mac2->keyLen) ||
349-
(XMEMCMP(mac1->key, mac2->key, mac1->keyLen) != 0) ||
350-
(XMEMCMP(mac1->cipher, mac2->cipher, WP_MAX_CIPH_NAME_SIZE) != 0))) {
351-
ok = 0;
352+
if (ok && ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0)) {
353+
if ((mac1->keyLen == MAX_SIZE_T) || (mac2->keyLen == MAX_SIZE_T) ||
354+
(mac1->keyLen != mac2->keyLen) ||
355+
(CRYPTO_memcmp(mac1->key, mac2->key, mac1->keyLen) != 0) ||
356+
(XMEMCMP(mac1->cipher, mac2->cipher, WP_MAX_CIPH_NAME_SIZE) != 0)) {
357+
ok = 0;
358+
}
352359
}
353360

354361
WOLFPROV_LEAVE(WP_LOG_COMP_MAC, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);

0 commit comments

Comments
 (0)