Skip to content

Commit 4ae7cc0

Browse files
aidangarskeaidangarske
committed
Fix fenrir issues 169, 170, 171, 172, 271, 395, 396, 510, 511, 512, 514, 829, 830, 836, 837, 838, 839, 1183, 1184, 1185, 1186, 1187, 1280, 1281 for wolfProvider
1 parent db8342d commit 4ae7cc0

16 files changed

Lines changed: 178 additions & 74 deletions

‎src/wp_aes_aead.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,6 +1688,10 @@ static void *wp_aes_gcm_newctx(WOLFPROV_CTX* provCtx, size_t keyBits)
16881688
*/
16891689
static void wp_aes_gcm_freectx(wp_AeadCtx* ctx)
16901690
{
1691+
OPENSSL_free(ctx->aad);
1692+
#if defined(WP_HAVE_AESGCM) && !defined(WOLFSSL_AESGCM_STREAM)
1693+
OPENSSL_free(ctx->in);
1694+
#endif
16911695
wc_AesFree(&ctx->aes);
16921696
OPENSSL_free(ctx);
16931697
}

‎src/wp_aes_block.c‎

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

867867
if (ok && ctx->pad) {
868868
unsigned char pad;
869+
unsigned char invalid;
870+
unsigned char i;
869871

870872
pad = ctx->buf[AES_BLOCK_SIZE - 1];
871-
if ((pad == 0) || (pad > AES_BLOCK_SIZE)) {
873+
invalid = wp_ct_byte_mask_eq(pad, 0) |
874+
~wp_ct_int_mask_gte(AES_BLOCK_SIZE, (int)pad);
875+
for (i = 0; i < AES_BLOCK_SIZE; i++) {
876+
unsigned char mask = wp_ct_int_mask_gte((int)i,
877+
AES_BLOCK_SIZE - (int)pad);
878+
invalid |= mask & wp_ct_byte_mask_ne(ctx->buf[i], pad);
879+
}
880+
if (invalid) {
872881
ok = 0;
873882
}
874-
if (ok) {
875-
unsigned char len = AES_BLOCK_SIZE;
876-
unsigned char i;
877-
878-
for (i = 0; i < pad; i++) {
879-
if (ctx->buf[--len] != pad) {
880-
return 0;
881-
}
882-
}
883-
ctx->bufSz = len;
883+
else {
884+
ctx->bufSz = AES_BLOCK_SIZE - pad;
884885
}
885886
}
886887

‎src/wp_cmac.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,12 @@ static wp_CmacCtx* wp_cmac_dup(wp_CmacCtx* src)
162162
dst = wp_cmac_new(NULL);
163163
}
164164
if (dst != NULL) {
165-
*dst = *src;
166-
dst->keyLen = 0;
165+
dst->type = src->type;
166+
dst->size = src->size;
167+
dst->expKeySize = src->expKeySize;
167168

168169
if ((src->keyLen != 0) &&
169-
(!wp_cmac_set_key(dst, src->key, src->keyLen, 0))) {
170+
(!wp_cmac_set_key(dst, src->key, src->keyLen, 1))) {
170171
wp_cmac_free(dst);
171172
dst = NULL;
172173
}

‎src/wp_des.c‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -418,10 +418,15 @@ static int wp_des3_block_update(wp_Des3BlockCtx *ctx, unsigned char *out,
418418
int i;
419419
unsigned char off = inLen % DES_BLOCK_SIZE;
420420
unsigned char pad = DES_BLOCK_SIZE - off - 1;
421-
for (i = off; i < DES_BLOCK_SIZE; i++) {
422-
out[inLen - off + i] = pad;
421+
if (outSize < inLen + pad + 1) {
422+
ok = 0;
423+
}
424+
if (ok) {
425+
for (i = off; i < DES_BLOCK_SIZE; i++) {
426+
out[inLen - off + i] = pad;
427+
}
428+
inLen += pad + 1;
423429
}
424-
inLen += pad + 1;
425430
}
426431
if (ctx->bufSz != 0) {
427432
size_t len = DES_BLOCK_SIZE - ctx->bufSz;
@@ -578,21 +583,22 @@ static int wp_des3_block_final_dec(wp_Des3BlockCtx* ctx, unsigned char *out,
578583

579584
if (ok && ctx->pad) {
580585
unsigned char pad;
586+
unsigned char invalid;
587+
unsigned char i;
581588

582589
pad = ctx->buf[DES_BLOCK_SIZE - 1];
583-
if ((pad == 0) || (pad > DES_BLOCK_SIZE)) {
590+
invalid = wp_ct_byte_mask_eq(pad, 0) |
591+
~wp_ct_int_mask_gte(DES_BLOCK_SIZE, (int)pad);
592+
for (i = 0; i < DES_BLOCK_SIZE; i++) {
593+
unsigned char mask = wp_ct_int_mask_gte((int)i,
594+
DES_BLOCK_SIZE - (int)pad);
595+
invalid |= mask & wp_ct_byte_mask_ne(ctx->buf[i], pad);
596+
}
597+
if (invalid) {
584598
ok = 0;
585599
}
586-
if (ok) {
587-
unsigned char len = DES_BLOCK_SIZE;
588-
unsigned char i;
589-
590-
for (i = 0; i < pad; i++) {
591-
if (ctx->buf[--len] != pad) {
592-
return 0;
593-
}
594-
}
595-
ctx->bufSz = len;
600+
else {
601+
ctx->bufSz = DES_BLOCK_SIZE - pad;
596602
}
597603
}
598604

‎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: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -335,28 +335,23 @@ static int wp_drbg_reseed(wp_DrbgCtx* ctx, int predResist,
335335
{
336336
int ok = 1;
337337

338+
int rc;
339+
338340
WOLFPROV_ENTER(WP_LOG_COMP_RNG, "wp_drbg_reseed");
339341

340-
#if 0
341-
/* Calling Hash_DRBG_Instantiate would be better. */
342-
int rc;
343-
rc = wc_RNG_DRBG_Reseed(ctx->rng, entropy, entropyLen);
342+
rc = wc_RNG_DRBG_Reseed(ctx->rng, entropy, (word32)entropyLen);
344343
if (rc != 0) {
344+
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG, "wc_RNG_DRBG_Reseed", rc);
345345
ok = 0;
346346
}
347347
if (ok && (addInLen > 0)) {
348-
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, addInLen);
348+
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, (word32)addInLen);
349349
if (rc != 0) {
350+
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
351+
"wc_RNG_DRBG_Reseed", rc);
350352
ok = 0;
351353
}
352354
}
353-
#else
354-
(void)ctx;
355-
(void)entropy;
356-
(void)entropyLen;
357-
(void)addIn;
358-
(void)addInLen;
359-
#endif
360355

361356
(void)predResist;
362357

@@ -388,6 +383,7 @@ static int wp_drbg_enable_locking(wp_DrbgCtx* ctx)
388383
if (rc != 0) {
389384
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG, "wc_InitMutex", rc);
390385
OPENSSL_free(ctx->mutex);
386+
ctx->mutex = NULL;
391387
ok = 0;
392388
}
393389
}
@@ -547,11 +543,16 @@ static int wp_drbg_set_ctx_params(wp_DrbgCtx* ctx, const OSSL_PARAM params[])
547543
*/
548544
static int wp_drbg_verify_zeroization(wp_DrbgCtx* ctx)
549545
{
546+
int ok;
547+
550548
WOLFPROV_ENTER(WP_LOG_COMP_RNG, "wp_drbg_verify_zeroization");
551549

552-
(void)ctx;
553-
WOLFPROV_LEAVE(WP_LOG_COMP_RNG, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), 1);
554-
return 1;
550+
/* After uninstantiate, ctx->rng is freed (with internal state zeroized
551+
* by wolfSSL) and set to NULL. Verify that cleanup occurred. */
552+
ok = (ctx->rng == NULL);
553+
554+
WOLFPROV_LEAVE(WP_LOG_COMP_RNG, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
555+
return ok;
555556
}
556557

557558
/**

‎src/wp_ecx_kmgmt.c‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,16 +356,65 @@ 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));
364+
int ok = 1;
365+
366366
dst->includePublic = src->includePublic;
367-
dst->hasPub = src->hasPub;
368-
dst->hasPriv = src->hasPriv;
367+
368+
/* Copy public key if available and requested. */
369+
if (ok && src->hasPub &&
370+
((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0)) {
371+
byte buf[64];
372+
word32 len = (word32)sizeof(buf);
373+
int rc = (*src->data->exportPub)((void*)&src->key, buf, &len,
374+
ECX_LITTLE_ENDIAN);
375+
if (rc != 0) {
376+
ok = 0;
377+
}
378+
if (ok) {
379+
rc = (*dst->data->importPub)(buf, len, (void*)&dst->key,
380+
ECX_LITTLE_ENDIAN);
381+
if (rc != 0) {
382+
ok = 0;
383+
}
384+
}
385+
if (ok) {
386+
dst->hasPub = 1;
387+
}
388+
}
389+
/* Copy private key if available and requested. */
390+
if (ok && src->hasPriv &&
391+
((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0)) {
392+
byte buf[64];
393+
word32 len = (word32)sizeof(buf);
394+
int rc = (*src->data->exportPriv)((void*)&src->key, buf, &len);
395+
if (rc != 0) {
396+
ok = 0;
397+
}
398+
if (ok) {
399+
rc = (*dst->data->importPriv)(buf, len, (void*)&dst->key,
400+
ECX_LITTLE_ENDIAN);
401+
if (rc != 0) {
402+
ok = 0;
403+
}
404+
}
405+
if (ok) {
406+
dst->hasPriv = 1;
407+
dst->clamped = src->clamped;
408+
XMEMCPY(dst->unclamped, src->unclamped,
409+
sizeof(src->unclamped));
410+
}
411+
wc_ForceZero(buf, len);
412+
}
413+
414+
if (!ok) {
415+
wp_ecx_free(dst);
416+
dst = NULL;
417+
}
369418
}
370419

371420
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: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,24 @@ 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;
191+
int rc;
193192

194-
if ((src->key != NULL) &&
193+
dst->type = src->type;
194+
dst->size = src->size;
195+
dst->provCtx = src->provCtx;
196+
197+
rc = wc_HmacCopy(&src->hmac, &dst->hmac);
198+
if (rc != 0) {
199+
ok = 0;
200+
}
201+
202+
if (ok && (src->key != NULL) &&
195203
(!wp_hmac_set_key(dst, src->key, src->keyLen, 0))) {
204+
ok = 0;
205+
}
206+
207+
if (!ok) {
196208
wp_hmac_free(dst);
197209
dst = NULL;
198210
}

‎src/wp_mac_kmgmt.c‎

Lines changed: 5 additions & 4 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
@@ -346,7 +347,7 @@ static int wp_mac_match(const wp_Mac* mac1, const wp_Mac* mac2, int selection)
346347
}
347348
if (ok && ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) &&
348349
(mac1->keyLen != MAX_SIZE_T) && ((mac1->keyLen != mac2->keyLen) ||
349-
(XMEMCMP(mac1->key, mac2->key, mac1->keyLen) != 0) ||
350+
(CRYPTO_memcmp(mac1->key, mac2->key, mac1->keyLen) != 0) ||
350351
(XMEMCMP(mac1->cipher, mac2->cipher, WP_MAX_CIPH_NAME_SIZE) != 0))) {
351352
ok = 0;
352353
}

0 commit comments

Comments
 (0)