Skip to content

Commit 3ec0431

Browse files
JeremiahM37JeremiahM37
committed
Fix prefix matches, sizeof errors in string ops
1 parent 259fb1b commit 3ec0431

4 files changed

Lines changed: 44 additions & 29 deletions

File tree

‎src/wp_internal.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -793,23 +793,23 @@ static int wp_EncryptedInfoGet(wp_EncryptedInfo* info, const char* cipherInfo)
793793

794794
/* determine cipher information */
795795
#if !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_128)
796-
if (XSTRNCMP(cipherInfo, kEncTypeAesCbc128, XSTRLEN(kEncTypeAesCbc128)) == 0) {
796+
if (XSTRCMP(cipherInfo, kEncTypeAesCbc128) == 0) {
797797
info->cipherType = WC_CIPHER_AES_CBC;
798798
info->keySz = AES_128_KEY_SIZE;
799799
if (info->ivSz == 0) info->ivSz = AES_IV_SIZE;
800800
}
801801
else
802802
#endif
803803
#if !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_192)
804-
if (XSTRNCMP(cipherInfo, kEncTypeAesCbc192, XSTRLEN(kEncTypeAesCbc192)) == 0) {
804+
if (XSTRCMP(cipherInfo, kEncTypeAesCbc192) == 0) {
805805
info->cipherType = WC_CIPHER_AES_CBC;
806806
info->keySz = AES_192_KEY_SIZE;
807807
if (info->ivSz == 0) info->ivSz = AES_IV_SIZE;
808808
}
809809
else
810810
#endif
811811
#if !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_256)
812-
if (XSTRNCMP(cipherInfo, kEncTypeAesCbc256, XSTRLEN(kEncTypeAesCbc256)) == 0) {
812+
if (XSTRCMP(cipherInfo, kEncTypeAesCbc256) == 0) {
813813
info->cipherType = WC_CIPHER_AES_CBC;
814814
info->keySz = AES_256_KEY_SIZE;
815815
if (info->ivSz == 0) info->ivSz = AES_IV_SIZE;
@@ -1113,7 +1113,7 @@ int wp_read_pem_bio(WOLFPROV_CTX *provctx, OSSL_CORE_BIO *coreBio,
11131113
*len += readLen;
11141114
}
11151115
/* Last line should have footer. */
1116-
if (XMEMCMP(buf, "-----END ", 8) == 0) {
1116+
if (XMEMCMP(buf, "-----END ", 9) == 0) {
11171117
break;
11181118
}
11191119
}

‎src/wp_rsa_asym.c‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -466,28 +466,44 @@ static int wp_rsaa_decrypt(wp_RsaAsymCtx* ctx, unsigned char* out,
466466
if (ok) {
467467
byte mask;
468468
byte negMask;
469-
470-
XMEMSET(out, 0, outSize);
471-
PRIVATE_KEY_UNLOCK();
472-
rc = wc_RsaPrivateDecrypt(in, (word32)inLen, out,
473-
(word32)outSize, wp_rsa_get_key(ctx->rsa));
474-
PRIVATE_KEY_LOCK();
475-
476-
/* Constant time checking of master secret. */
477-
mask = wp_ct_byte_mask_eq(out[0], ctx->clientVersion >> 8);
478-
mask &= wp_ct_byte_mask_eq(out[1], ctx->clientVersion);
479-
if (ctx->negVersion > 0) {
480-
/* Check for negotiated version as well. */
481-
negMask = wp_ct_byte_mask_eq(out[0], ctx->negVersion >> 8);
482-
negMask &= wp_ct_byte_mask_eq(out[1], ctx->negVersion);
483-
mask |= negMask;
484-
}
485-
rc &= (int)(char)mask;
486-
487-
if (rc <= 0) {
488-
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaPrivateDecrypt TLS padding", rc);
469+
byte rand[WOLFSSL_MAX_MASTER_KEY_LENGTH];
470+
int i;
471+
472+
/* Implicit rejection: always generate random fallback
473+
* to prevent Bleichenbacher-style oracle attacks. */
474+
rc = wc_RNG_GenerateBlock(&ctx->rng, rand,
475+
WOLFSSL_MAX_MASTER_KEY_LENGTH);
476+
if (rc != 0) {
489477
ok = 0;
490478
}
479+
if (ok) {
480+
XMEMSET(out, 0, outSize);
481+
PRIVATE_KEY_UNLOCK();
482+
rc = wc_RsaPrivateDecrypt(in, (word32)inLen, out,
483+
(word32)outSize, wp_rsa_get_key(ctx->rsa));
484+
PRIVATE_KEY_LOCK();
485+
486+
/* Constant time checking of master secret. */
487+
mask = wp_ct_byte_mask_eq(out[0],
488+
ctx->clientVersion >> 8);
489+
mask &= wp_ct_byte_mask_eq(out[1], ctx->clientVersion);
490+
if (ctx->negVersion > 0) {
491+
negMask = wp_ct_byte_mask_eq(out[0],
492+
ctx->negVersion >> 8);
493+
negMask &= wp_ct_byte_mask_eq(out[1],
494+
ctx->negVersion);
495+
mask |= negMask;
496+
}
497+
/* Combine decrypt success with version check. */
498+
mask &= wp_ct_int_mask_gte(rc, 1);
499+
500+
/* Constant-time select: real result or random fallback. */
501+
for (i = 0; i < WOLFSSL_MAX_MASTER_KEY_LENGTH; i++) {
502+
out[i] = wp_ct_byte_mask_sel(mask, out[i], rand[i]);
503+
}
504+
OPENSSL_cleanse(rand, sizeof(rand));
505+
rc = WOLFSSL_MAX_MASTER_KEY_LENGTH;
506+
}
491507
}
492508
}
493509
else if (ctx->padMode == RSA_NO_PADDING) {
@@ -551,7 +567,7 @@ static int wp_rsaa_setup_md(wp_RsaAsymCtx* ctx, const char* mdName,
551567
}
552568
else {
553569
OPENSSL_strlcpy(ctx->mgf1MdName, mdName,
554-
sizeof(ctx->oaepMdName));
570+
sizeof(ctx->mgf1MdName));
555571
}
556572
}
557573
}

‎src/wp_rsa_kem.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,7 @@ static int wp_rsakem_set_ctx_params(wp_RsaKemCtx* ctx,
539539
ok = 0;
540540
}
541541
if (ok && (op != NULL)) {
542-
if (XSTRNCMP(OSSL_KEM_PARAM_OPERATION_RSASVE, op,
543-
sizeof(OSSL_KEM_PARAM_OPERATION_RSASVE) - 1) == 0) {
542+
if (XSTRCMP(OSSL_KEM_PARAM_OPERATION_RSASVE, op) == 0) {
544543
ctx->op = WP_RSA_KEM_OP_RSASVE;
545544
}
546545
else {

‎src/wp_rsa_sig.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static int wp_rsa_setup_md(wp_RsaSigCtx* ctx, const char* mdName,
130130
if (ctx->padMode == RSA_PKCS1_PSS_PADDING && ctx->minSaltLen != -1) {
131131
wp_rsa_get_pss_mds(ctx->rsa, &localMdName, NULL);
132132
if (mdName != NULL &&
133-
XSTRNCASECMP(localMdName, mdName, XSTRLEN(localMdName)) != 0) {
133+
XSTRCASECMP(localMdName, mdName) != 0) {
134134
ok = 0;
135135
}
136136
}
@@ -197,7 +197,7 @@ static int wp_rsa_setup_md(wp_RsaSigCtx* ctx, const char* mdName,
197197
}
198198
}
199199
else {
200-
OPENSSL_strlcpy(ctx->mgf1MdName, mdName, sizeof(ctx->mdName));
200+
OPENSSL_strlcpy(ctx->mgf1MdName, mdName, sizeof(ctx->mgf1MdName));
201201
}
202202
}
203203
}

0 commit comments

Comments
 (0)