Skip to content

Commit 32502e9

Browse files
douzzerdouzzer
authored
Merge pull request #10102 from Frauschi/zd21460
Various fixes
2 parents 9950923 + 2ae2072 commit 32502e9

File tree

12 files changed

+252
-31
lines changed

12 files changed

+252
-31
lines changed

‎src/tls.c‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16086,11 +16086,18 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX,
1608616086

1608716087
if (serverNameX == NULL && ssl->ctx && ssl->ctx->extensions) {
1608816088
serverNameX = TLSX_Find(ssl->ctx->extensions, TLSX_SERVER_NAME);
16089-
extensions = &ssl->ctx->extensions;
16089+
if (serverNameX != NULL)
16090+
extensions = &ssl->ctx->extensions;
16091+
}
16092+
16093+
/* ECH requires an inner SNI to be present for ClientHelloInner.
16094+
* Without it, fail instead of mutating extension lists. */
16095+
if (serverNameX == NULL) {
16096+
ret = BAD_FUNC_ARG;
1609016097
}
1609116098

1609216099
/* store the inner server name */
16093-
if (serverNameX != NULL) {
16100+
if (ret == 0 && serverNameX != NULL) {
1609416101
char* hostName = ((SNI*)serverNameX->data)->data.host_name;
1609516102
word32 hostNameSz = (word32)XSTRLEN(hostName) + 1;
1609616103

@@ -16101,15 +16108,19 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX,
1610116108
XMEMCPY(serverName, hostName, hostNameSz);
1610216109
}
1610316110

16104-
/* remove the inner server name */
16105-
TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap);
16111+
/* only swap the SNI if one was found; extensions is non-NULL if an
16112+
* SNI entry was found on ssl->extensions or ctx->extensions */
16113+
if (ret == 0 && extensions != NULL) {
16114+
/* remove the inner server name */
16115+
TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap);
1610616116

16107-
/* set the public name as the server name */
16108-
if ((ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME,
16109-
((WOLFSSL_ECH*)echX->data)->echConfig->publicName,
16110-
XSTRLEN(((WOLFSSL_ECH*)echX->data)->echConfig->publicName),
16111-
ssl->heap)) == WOLFSSL_SUCCESS)
16112-
ret = 0;
16117+
/* set the public name as the server name */
16118+
if ((ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME,
16119+
((WOLFSSL_ECH*)echX->data)->echConfig->publicName,
16120+
XSTRLEN(((WOLFSSL_ECH*)echX->data)->echConfig->publicName),
16121+
ssl->heap)) == WOLFSSL_SUCCESS)
16122+
ret = 0;
16123+
}
1611316124
}
1611416125
*pServerNameX = serverNameX;
1611516126
*pExtensions = extensions;
@@ -16122,10 +16133,12 @@ static int TLSX_EchRestoreSNI(WOLFSSL* ssl, char* serverName,
1612216133
{
1612316134
int ret = 0;
1612416135

16125-
if (serverNameX != NULL) {
16126-
/* remove the public name SNI */
16136+
/* always remove the publicName SNI we injected, regardless of whether
16137+
* there was a prior inner SNI to restore */
16138+
if (extensions != NULL)
1612716139
TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap);
1612816140

16141+
if (serverNameX != NULL) {
1612916142
/* restore the inner server name */
1613016143
ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME,
1613116144
serverName, XSTRLEN(serverName), ssl->heap);

‎src/x509_str.c‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,11 @@ static void SetupStoreCtxError_ex(WOLFSSL_X509_STORE_CTX* ctx, int ret,
318318
{
319319
int error = GetX509Error(ret);
320320

321+
/* Do not overwrite a previously recorded error with success; preserve
322+
* the worst-seen error across the chain walk. */
323+
if (error == 0 && ctx->error != 0)
324+
return;
325+
321326
wolfSSL_X509_STORE_CTX_set_error(ctx, error);
322327
wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth);
323328
}
@@ -635,9 +640,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
635640
if (ctx->store->verify_cb) {
636641
ret = ctx->store->verify_cb(0, ctx);
637642
if (ret != WOLFSSL_SUCCESS) {
643+
ret = WOLFSSL_FAILURE;
638644
goto exit;
639645
}
640646
}
647+
else {
648+
ret = WOLFSSL_FAILURE;
649+
goto exit;
650+
}
641651
} else
642652
#endif
643653
{
@@ -2174,4 +2184,3 @@ int wolfSSL_X509_STORE_set1_param(WOLFSSL_X509_STORE *ctx,
21742184
#endif /* !WOLFCRYPT_ONLY */
21752185

21762186
#endif /* !WOLFSSL_X509_STORE_INCLUDED */
2177-

‎tests/api/test_evp_cipher.c‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,6 +1915,7 @@ int test_wolfssl_EVP_chacha20_poly1305(void)
19151915
byte cipherText[sizeof(plainText)];
19161916
byte decryptedText[sizeof(plainText)];
19171917
byte tag[CHACHA20_POLY1305_AEAD_AUTHTAG_SIZE];
1918+
byte badTag[CHACHA20_POLY1305_AEAD_AUTHTAG_SIZE];
19181919
EVP_CIPHER_CTX* ctx = NULL;
19191920
int outSz;
19201921

@@ -1979,6 +1980,28 @@ int test_wolfssl_EVP_chacha20_poly1305(void)
19791980
EVP_CIPHER_CTX_free(ctx);
19801981
ctx = NULL;
19811982

1983+
/* Negative test: forged (all-zero) tag must be rejected. */
1984+
XMEMSET(badTag, 0, sizeof(badTag));
1985+
ExpectNotNull((ctx = EVP_CIPHER_CTX_new()));
1986+
ExpectIntEQ(EVP_DecryptInit_ex(ctx, EVP_chacha20_poly1305(), NULL,
1987+
NULL, NULL), WOLFSSL_SUCCESS);
1988+
ExpectIntEQ(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_IVLEN,
1989+
CHACHA20_POLY1305_AEAD_IV_SIZE, NULL), WOLFSSL_SUCCESS);
1990+
ExpectIntEQ(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG,
1991+
CHACHA20_POLY1305_AEAD_AUTHTAG_SIZE, badTag),
1992+
WOLFSSL_SUCCESS);
1993+
ExpectIntEQ(EVP_DecryptInit_ex(ctx, NULL, NULL, key, iv),
1994+
WOLFSSL_SUCCESS);
1995+
ExpectIntEQ(EVP_DecryptUpdate(ctx, NULL, &outSz, aad, sizeof(aad)),
1996+
WOLFSSL_SUCCESS);
1997+
ExpectIntEQ(EVP_DecryptUpdate(ctx, decryptedText, &outSz, cipherText,
1998+
sizeof(cipherText)), WOLFSSL_SUCCESS);
1999+
/* EVP_DecryptFinal_ex MUST return failure on tag mismatch */
2000+
ExpectIntNE(EVP_DecryptFinal_ex(ctx, decryptedText, &outSz),
2001+
WOLFSSL_SUCCESS);
2002+
EVP_CIPHER_CTX_free(ctx);
2003+
ctx = NULL;
2004+
19822005
/* Test partial Inits. CipherInit() allow setting of key and iv
19832006
* in separate calls. */
19842007
ExpectNotNull((ctx = EVP_CIPHER_CTX_new()));

‎tests/api/test_ossl_x509_str.c‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,56 @@ int test_X509_STORE_InvalidCa(void)
10741074
ExpectIntEQ(X509_STORE_CTX_init(ctx, str, cert, untrusted), 1);
10751075
ExpectIntEQ(X509_verify_cert(ctx), 1);
10761076
ExpectIntEQ(last_errcode, X509_V_ERR_INVALID_CA);
1077+
/* Defense in depth: ctx->error must not be clobbered back to X509_V_OK
1078+
* by the later successful verification of the intermediate against the
1079+
* trusted root. The worst-seen error must persist. */
1080+
ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_INVALID_CA);
1081+
1082+
X509_free(cert);
1083+
X509_STORE_free(str);
1084+
X509_STORE_CTX_free(ctx);
1085+
sk_X509_pop_free(untrusted, NULL);
1086+
#endif
1087+
return EXPECT_RESULT();
1088+
}
1089+
1090+
int test_X509_STORE_InvalidCa_NoCallback(void)
1091+
{
1092+
EXPECT_DECLS;
1093+
#if defined(OPENSSL_ALL) && !defined(NO_RSA) && !defined(NO_FILESYSTEM)
1094+
const char* filename = "./certs/intermediate/ca_false_intermediate/"
1095+
"test_int_not_cacert.pem";
1096+
const char* srvfile = "./certs/intermediate/ca_false_intermediate/"
1097+
"test_sign_bynoca_srv.pem";
1098+
X509_STORE_CTX* ctx = NULL;
1099+
X509_STORE* str = NULL;
1100+
XFILE fp = XBADFILE;
1101+
X509* cert = NULL;
1102+
STACK_OF(X509)* untrusted = NULL;
1103+
1104+
ExpectTrue((fp = XFOPEN(srvfile, "rb"))
1105+
!= XBADFILE);
1106+
ExpectNotNull(cert = PEM_read_X509(fp, 0, 0, 0 ));
1107+
if (fp != XBADFILE) {
1108+
XFCLOSE(fp);
1109+
fp = XBADFILE;
1110+
}
1111+
1112+
ExpectNotNull(str = X509_STORE_new());
1113+
ExpectNotNull(ctx = X509_STORE_CTX_new());
1114+
ExpectNotNull(untrusted = sk_X509_new_null());
1115+
1116+
/* Create cert chain stack with an intermediate that is CA:FALSE. */
1117+
ExpectIntEQ(test_X509_STORE_untrusted_load_cert_to_stack(filename,
1118+
untrusted), TEST_SUCCESS);
1119+
1120+
ExpectIntEQ(X509_STORE_load_locations(str,
1121+
"./certs/intermediate/ca_false_intermediate/test_ca.pem",
1122+
NULL), 1);
1123+
ExpectIntEQ(X509_STORE_CTX_init(ctx, str, cert, untrusted), 1);
1124+
/* No verify callback: verification must fail on CA:FALSE issuer. */
1125+
ExpectIntNE(X509_verify_cert(ctx), 1);
1126+
ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_INVALID_CA);
10771127

10781128
X509_free(cert);
10791129
X509_STORE_free(str);
@@ -1793,4 +1843,3 @@ int test_X509_STORE_No_SSL_CTX(void)
17931843
#endif
17941844
return EXPECT_RESULT();
17951845
}
1796-

‎tests/api/test_ossl_x509_str.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ int test_wolfSSL_X509_STORE_CTX(void);
3131
int test_wolfSSL_X509_STORE_CTX_ex(void);
3232
int test_X509_STORE_untrusted(void);
3333
int test_X509_STORE_InvalidCa(void);
34+
int test_X509_STORE_InvalidCa_NoCallback(void);
3435
int test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup(void);
3536
int test_wolfSSL_X509_STORE_CTX_get_issuer(void);
3637
int test_wolfSSL_X509_STORE_set_flags(void);
@@ -51,6 +52,7 @@ int test_X509_STORE_No_SSL_CTX(void);
5152
TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_STORE_CTX_ex), \
5253
TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_untrusted), \
5354
TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa), \
55+
TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa_NoCallback), \
5456
TEST_DECL_GROUP("ossl_x509_store", \
5557
test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup), \
5658
TEST_DECL_GROUP("ossl_x509_store", \

‎wolfcrypt/src/aes.c‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10217,8 +10217,9 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
1021710217
/* If the sz is non-zero, both in and out must be set. If sz is 0,
1021810218
* in and out are don't cares, as this is is the GMAC case. */
1021910219
if (aes == NULL || iv == NULL || (sz != 0 && (in == NULL || out == NULL)) ||
10220-
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE || authTagSz == 0 ||
10221-
ivSz == 0 || ((authInSz > 0) && (authIn == NULL)))
10220+
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE ||
10221+
authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ || ivSz == 0 ||
10222+
((authInSz > 0) && (authIn == NULL)))
1022210223
{
1022310224
return BAD_FUNC_ARG;
1022410225
}
@@ -10781,8 +10782,8 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
1078110782
/* If the sz is non-zero, both in and out must be set. If sz is 0,
1078210783
* in and out are don't cares, as this is is the GMAC case. */
1078310784
if (aes == NULL || iv == NULL || (sz != 0 && (in == NULL || out == NULL)) ||
10784-
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE || authTagSz == 0 ||
10785-
ivSz == 0) {
10785+
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE ||
10786+
authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ || ivSz == 0) {
1078610787

1078710788
return BAD_FUNC_ARG;
1078810789
}
@@ -12473,7 +12474,7 @@ int wc_AesGcmEncryptFinal(Aes* aes, byte* authTag, word32 authTagSz)
1247312474

1247412475
/* Check validity of parameters. */
1247512476
if ((aes == NULL) || (authTag == NULL) || (authTagSz > WC_AES_BLOCK_SIZE) ||
12476-
(authTagSz == 0)) {
12477+
(authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ)) {
1247712478
ret = BAD_FUNC_ARG;
1247812479
}
1247912480

@@ -16377,9 +16378,7 @@ int wc_local_CmacUpdateAes(struct Cmac *cmac, const byte* in, word32 inSz) {
1637716378
in += add;
1637816379

1637916380
if (cmac->bufferSz == WC_AES_BLOCK_SIZE && inSz != 0) {
16380-
if (cmac->totalSz != 0) {
16381-
xorbuf(cmac->buffer, cmac->digest, WC_AES_BLOCK_SIZE);
16382-
}
16381+
xorbuf(cmac->buffer, cmac->digest, WC_AES_BLOCK_SIZE);
1638316382
ret = AesEncrypt_preFetchOpt(aes, cmac->buffer,
1638416383
cmac->digest, &did_prefetches);
1638516384
if (ret == 0) {

‎wolfcrypt/src/cmac.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,7 @@ int wc_CmacUpdate(Cmac* cmac, const byte* in, word32 inSz)
238238
inSz -= add;
239239

240240
if (cmac->bufferSz == WC_AES_BLOCK_SIZE && inSz != 0) {
241-
if (cmac->totalSz != 0) {
242-
xorbuf(cmac->buffer, cmac->digest, WC_AES_BLOCK_SIZE);
243-
}
241+
xorbuf(cmac->buffer, cmac->digest, WC_AES_BLOCK_SIZE);
244242
wc_AesEncryptDirect(&cmac->aes, cmac->digest,
245243
cmac->buffer);
246244
cmac->totalSz += WC_AES_BLOCK_SIZE;

‎wolfcrypt/src/eccsi.c‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2159,6 +2159,18 @@ static int eccsi_calc_j(EccsiKey* key, const mp_int* hem, const byte* sig,
21592159
if (err == 0) {
21602160
err = eccsi_decode_sig_s(key, sig, sigSz, s);
21612161
}
2162+
/* Validate s is in [1, q-1]: reject zero or out-of-range second signature
2163+
* component. With s=0, [s](...) yields the point at infinity whose
2164+
* affine x-coordinate is 0, making the final mp_cmp(0,0) accept any
2165+
* forged signature. */
2166+
if (err == 0) {
2167+
if (mp_iszero(s)) {
2168+
err = MP_ZERO_E;
2169+
}
2170+
else if (mp_cmp(s, &key->params.order) != MP_LT) {
2171+
err = ECC_OUT_OF_RANGE_E;
2172+
}
2173+
}
21622174
/* [s]( [HE]G + [r]Y ) */
21632175
if (err == 0) {
21642176
err = eccsi_mulmod_point(key, s, j, j, 1);
@@ -2238,6 +2250,19 @@ int wc_VerifyEccsiHash(EccsiKey* key, enum wc_HashType hashType,
22382250
err = mp_montgomery_setup(&params->prime, &mp);
22392251
}
22402252

2253+
/* Validate r is in [1, q-1]: reject zero or out-of-range first signature
2254+
* component before any scalar multiplication takes place.
2255+
* Without this check, r=0 causes J_x=0 and the final mp_cmp(0,0)==MP_EQ
2256+
* comparison accepts the forged signature unconditionally. */
2257+
if (err == 0) {
2258+
if (mp_iszero(r)) {
2259+
err = MP_ZERO_E;
2260+
}
2261+
else if (mp_cmp(r, &params->order) != MP_LT) {
2262+
err = ECC_OUT_OF_RANGE_E;
2263+
}
2264+
}
2265+
22412266
/* Step 1: Validate PVT is on curve */
22422267
if (err == 0) {
22432268
err = wc_ecc_is_point(pvt, &params->a, &params->b, &params->prime);
@@ -2273,6 +2298,16 @@ int wc_VerifyEccsiHash(EccsiKey* key, enum wc_HashType hashType,
22732298
key->params.haveBase = 0;
22742299
}
22752300

2301+
/* Defense-in-depth: reject J = point at infinity before the final
2302+
* comparison. Catches any future path that might reach this point
2303+
* with a neutral-element result (e.g. s = 0 mod q for a non-zero
2304+
* encoded s). */
2305+
if (err == 0) {
2306+
if (wc_ecc_point_is_at_infinity(j)) {
2307+
err = ECC_INF_E;
2308+
}
2309+
}
2310+
22762311
/* Step 6: Jx fitting, compare with r */
22772312
if (err == 0) {
22782313
jx = &key->tmp;

‎wolfcrypt/src/evp.c‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,16 +1505,33 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, unsigned char *out,
15051505
* HAVE_FIPS_VERSION >= 2 */
15061506
#if defined(HAVE_CHACHA) && defined(HAVE_POLY1305)
15071507
case WC_CHACHA20_POLY1305_TYPE:
1508+
{
1509+
byte computedTag[CHACHA20_POLY1305_AEAD_AUTHTAG_SIZE];
1510+
if (!ctx->enc) {
1511+
/* Save the expected tag before _Final() overwrites
1512+
* ctx->authTag */
1513+
XMEMCPY(computedTag, ctx->authTag, sizeof(computedTag));
1514+
}
15081515
if (wc_ChaCha20Poly1305_Final(&ctx->cipher.chachaPoly,
15091516
ctx->authTag) != 0) {
15101517
WOLFSSL_MSG("wc_ChaCha20Poly1305_Final failed");
15111518
return WOLFSSL_FAILURE;
15121519
}
1513-
else {
1514-
*outl = 0;
1515-
return WOLFSSL_SUCCESS;
1520+
if (!ctx->enc) {
1521+
/* ctx->authTag now holds computed tag; computedTag holds
1522+
* expected */
1523+
int tagErr = wc_ChaCha20Poly1305_CheckTag(computedTag,
1524+
ctx->authTag);
1525+
ForceZero(computedTag, sizeof(computedTag));
1526+
if (tagErr != 0) {
1527+
WOLFSSL_MSG("ChaCha20-Poly1305 tag mismatch");
1528+
return WOLFSSL_FAILURE;
1529+
}
15161530
}
1517-
break;
1531+
*outl = 0;
1532+
return WOLFSSL_SUCCESS;
1533+
}
1534+
break;
15181535
#endif
15191536
#ifdef WOLFSSL_SM4_GCM
15201537
case WC_SM4_GCM_TYPE:
@@ -3704,6 +3721,10 @@ int wolfSSL_EVP_PKEY_keygen_init(WOLFSSL_EVP_PKEY_CTX *ctx)
37043721
return WOLFSSL_SUCCESS;
37053722
}
37063723

3724+
#ifdef HAVE_ECC
3725+
static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key);
3726+
#endif
3727+
37073728
int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx,
37083729
WOLFSSL_EVP_PKEY **ppkey)
37093730
{
@@ -3758,6 +3779,8 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx,
37583779
ret = wolfSSL_EC_KEY_generate_key(pkey->ecc);
37593780
if (ret == WOLFSSL_SUCCESS) {
37603781
pkey->ownEcc = 1;
3782+
if (ECC_populate_EVP_PKEY(pkey, pkey->ecc) != WOLFSSL_SUCCESS)
3783+
ret = WOLFSSL_FAILURE;
37613784
}
37623785
}
37633786
break;
@@ -9516,7 +9539,15 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key)
95169539
else
95179540
#endif /* HAVE_PKCS8 */
95189541
{
9519-
if (ecc->type == ECC_PRIVATEKEY_ONLY) {
9542+
if (ecc->type == ECC_PRIVATEKEY_ONLY ||
9543+
(ecc->type == ECC_PRIVATEKEY &&
9544+
mp_iszero(ecc->pubkey.x))) {
9545+
/* Reconstruct public key from private scalar. This covers
9546+
* both ECC_PRIVATEKEY_ONLY keys and ECC_PRIVATEKEY keys whose
9547+
* public-key point was never populated (e.g. when only
9548+
* EC_KEY_set_private_key was called, SetECKeyInternal copies
9549+
* the zero-initialized pub_key point and marks the type as
9550+
* ECC_PRIVATEKEY, leaving pubkey.x == 0). */
95209551
if (wc_ecc_make_pub(ecc, NULL) != MP_OKAY) {
95219552
return WOLFSSL_FAILURE;
95229553
}

0 commit comments

Comments
 (0)