Skip to content

Commit 0a6e18a

Browse files
committed
zd/21661: harden X.509 chain validation, session ticket identity binding, and peer cert restore
- x509_str: require CA:TRUE unconditionally in wolfSSL_X509_verify_cert - asn: reject embedded NUL in dNSName / rfc822Name / URI SAN entries - internal: re-verify restored ticket peer cert against trust store with CRL/OCSP checks; clear stale state from session cache on verification failure - tests: update SAN NUL fixtures and add parse-time rejection coverage; add test_tls13_ticket_peer_cert_reverify for CA-removal scenario - ssl_sess: free previous session in wolfSSL_d2i_SSL_SESSION before overwrite - ticket: bind SNI and ALPN into session ticket via compile-time selected hash (TICKET_BINDING_HASH_TYPE); reject resumption on mismatch in both TLS 1.3 and TLS 1.2 paths
1 parent c098e53 commit 0a6e18a

10 files changed

Lines changed: 340 additions & 53 deletions

File tree

src/internal.c

Lines changed: 149 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39207,6 +39207,47 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3920739207
return ret;
3920839208
}
3920939209

39210+
#ifdef HAVE_SNI
39211+
/* Hash the server-selected SNI into dst (TICKET_BINDING_HASH_SZ bytes).
39212+
* Zeros dst when no SNI is present. */
39213+
static int TicketSniHash(WOLFSSL* ssl, byte* dst)
39214+
{
39215+
char* name = NULL;
39216+
word16 nameLen;
39217+
39218+
nameLen = TLSX_SNI_GetRequest(ssl->extensions,
39219+
WOLFSSL_SNI_HOST_NAME,
39220+
(void**)&name, 0);
39221+
if (name != NULL && nameLen > 0) {
39222+
return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)name,
39223+
nameLen, dst, TICKET_BINDING_HASH_SZ);
39224+
}
39225+
39226+
XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);
39227+
return 0;
39228+
}
39229+
#endif
39230+
39231+
#ifdef HAVE_ALPN
39232+
/* Hash the negotiated ALPN protocol into dst (TICKET_BINDING_HASH_SZ
39233+
* bytes). Zeros dst when no ALPN was negotiated. */
39234+
static int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
39235+
{
39236+
char* proto = NULL;
39237+
word16 protoLen = 0;
39238+
39239+
if (TLSX_ALPN_GetRequest(ssl->extensions, (void**)&proto,
39240+
&protoLen) == WOLFSSL_SUCCESS &&
39241+
proto != NULL && protoLen > 0) {
39242+
return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)proto,
39243+
protoLen, dst, TICKET_BINDING_HASH_SZ);
39244+
}
39245+
39246+
XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);
39247+
return 0;
39248+
}
39249+
#endif
39250+
3921039251
/* create a new session ticket, 0 on success
3921139252
* Do any kind of setup in SetupTicket */
3921239253
int CreateTicket(WOLFSSL* ssl)
@@ -39306,6 +39347,19 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3930639347
XMEMCPY(it->sessionCtx, ssl->sessionCtx, ID_LEN);
3930739348
#endif
3930839349

39350+
#ifdef HAVE_SNI
39351+
ret = TicketSniHash(ssl, it->sniHash);
39352+
if (ret != 0)
39353+
goto error;
39354+
XMEMCPY(ssl->session->sniHash, it->sniHash, TICKET_BINDING_HASH_SZ);
39355+
#endif
39356+
#ifdef HAVE_ALPN
39357+
ret = TicketAlpnHash(ssl, it->alpnHash);
39358+
if (ret != 0)
39359+
goto error;
39360+
XMEMCPY(ssl->session->alpnHash, it->alpnHash, TICKET_BINDING_HASH_SZ);
39361+
#endif
39362+
3930939363
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
3931039364
!defined(NO_CERT_IN_TICKET)
3931139365
/* Store peer certificate in ticket for session resumption.
@@ -39659,6 +39713,28 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3965939713
XMEMCMP(psk->it->sessionCtx, ssl->sessionCtx,
3966039714
ssl->sessionCtxSz) != 0))
3966139715
return WOLFSSL_FATAL_ERROR;
39716+
#endif
39717+
#ifdef HAVE_SNI
39718+
{
39719+
byte curHash[TICKET_BINDING_HASH_SZ];
39720+
if (TicketSniHash((WOLFSSL*)ssl, curHash) != 0 ||
39721+
XMEMCMP(curHash, psk->it->sniHash,
39722+
TICKET_BINDING_HASH_SZ) != 0) {
39723+
WOLFSSL_MSG("Ticket SNI mismatch");
39724+
return WOLFSSL_FATAL_ERROR;
39725+
}
39726+
}
39727+
#endif
39728+
#ifdef HAVE_ALPN
39729+
{
39730+
byte curHash[TICKET_BINDING_HASH_SZ];
39731+
if (TicketAlpnHash((WOLFSSL*)ssl, curHash) != 0 ||
39732+
XMEMCMP(curHash, psk->it->alpnHash,
39733+
TICKET_BINDING_HASH_SZ) != 0) {
39734+
WOLFSSL_MSG("Ticket ALPN mismatch");
39735+
return WOLFSSL_FATAL_ERROR;
39736+
}
39737+
}
3966239738
#endif
3966339739
return 0;
3966439740
}
@@ -39671,36 +39747,54 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3967139747
word16 peerCertLen = 0;
3967239748
ato16(it->peerCertLen, &peerCertLen);
3967339749

39674-
if (peerCertLen > 0 && peerCertLen <= MAX_TICKET_PEER_CERT_SZ) {
39750+
/* Clear any peer cert state that may have been copied from the session
39751+
* cache by wolfSSL_DupSession before we got here. */
39752+
FreeX509(&ssl->peerCert);
39753+
InitX509(&ssl->peerCert, 0, ssl->heap);
3967539754
#ifdef SESSION_CERTS
39676-
/* Clear existing chain and add the peer certificate */
39677-
ssl->session->chain.count = 0;
39678-
AddSessionCertToChain(&ssl->session->chain,
39679-
it->peerCert, peerCertLen);
39755+
ssl->session->chain.count = 0;
3968039756
#endif
39681-
/* Also decode into ssl->peerCert for direct access */
39682-
{
39683-
int ret;
39684-
DecodedCert* dCert;
39685-
39686-
dCert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), ssl->heap,
39687-
DYNAMIC_TYPE_DCERT);
39688-
if (dCert != NULL) {
39689-
InitDecodedCert(dCert, it->peerCert, peerCertLen, ssl->heap);
39690-
ret = ParseCertRelative(dCert, CERT_TYPE, 0, NULL, NULL);
39691-
if (ret == 0) {
39757+
39758+
if (peerCertLen > 0 && peerCertLen <= MAX_TICKET_PEER_CERT_SZ) {
39759+
int ret;
39760+
DecodedCert* dCert;
39761+
39762+
dCert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), ssl->heap,
39763+
DYNAMIC_TYPE_DCERT);
39764+
if (dCert != NULL) {
39765+
int verify = ssl->options.verifyPeer ? VERIFY : NO_VERIFY;
39766+
InitDecodedCert(dCert, it->peerCert, peerCertLen, ssl->heap);
39767+
/* Re-verify against the current trust store so that CA
39768+
* removal since ticket issue is enforced. */
39769+
ret = ParseCertRelative(dCert, CERT_TYPE, verify,
39770+
SSL_CM(ssl), NULL);
39771+
#ifdef HAVE_OCSP
39772+
/* ParseCertRelative does not check revocation status.
39773+
* Run OCSP if the CertManager has it enabled. */
39774+
if (ret == 0 && SSL_CM(ssl)->ocspEnabled) {
39775+
ret = CheckCertOCSP_ex(SSL_CM(ssl)->ocsp, dCert, ssl);
39776+
}
39777+
#endif
39778+
#ifdef HAVE_CRL
39779+
if (ret == 0 && SSL_CM(ssl)->crlEnabled) {
39780+
ret = CheckCertCRL(SSL_CM(ssl)->crl, dCert);
39781+
}
39782+
#endif
39783+
if (ret == 0) {
39784+
#ifdef SESSION_CERTS
39785+
AddSessionCertToChain(&ssl->session->chain,
39786+
it->peerCert, peerCertLen);
39787+
#endif
39788+
FreeX509(&ssl->peerCert);
39789+
InitX509(&ssl->peerCert, 0, ssl->heap);
39790+
ret = CopyDecodedToX509(&ssl->peerCert, dCert);
39791+
if (ret != 0) {
3969239792
FreeX509(&ssl->peerCert);
3969339793
InitX509(&ssl->peerCert, 0, ssl->heap);
39694-
ret = CopyDecodedToX509(&ssl->peerCert, dCert);
39695-
if (ret != 0) {
39696-
/* Failed to copy - clear peerCert */
39697-
FreeX509(&ssl->peerCert);
39698-
InitX509(&ssl->peerCert, 0, ssl->heap);
39699-
}
3970039794
}
39701-
FreeDecodedCert(dCert);
39702-
XFREE(dCert, ssl->heap, DYNAMIC_TYPE_DCERT);
3970339795
}
39796+
FreeDecodedCert(dCert);
39797+
XFREE(dCert, ssl->heap, DYNAMIC_TYPE_DCERT);
3970439798
}
3970539799
}
3970639800
}
@@ -39846,6 +39940,12 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3984639940
it->sessionCtxSz = sess->sessionCtxSz;
3984739941
XMEMCPY(it->sessionCtx, sess->sessionCtx, sess->sessionCtxSz);
3984839942
#endif
39943+
#ifdef HAVE_SNI
39944+
XMEMCPY(it->sniHash, sess->sniHash, TICKET_BINDING_HASH_SZ);
39945+
#endif
39946+
#ifdef HAVE_ALPN
39947+
XMEMCPY(it->alpnHash, sess->alpnHash, TICKET_BINDING_HASH_SZ);
39948+
#endif
3984939949
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
3985039950
defined(SESSION_CERTS) && !defined(NO_CERT_IN_TICKET)
3985139951
/* Store peer certificate from session chain */
@@ -40090,6 +40190,31 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
4009040190
goto cleanup;
4009140191
}
4009240192

40193+
#ifdef HAVE_SNI
40194+
{
40195+
byte curHash[TICKET_BINDING_HASH_SZ];
40196+
if (TicketSniHash(ssl, curHash) != 0 ||
40197+
XMEMCMP(curHash, it->sniHash,
40198+
TICKET_BINDING_HASH_SZ) != 0) {
40199+
WOLFSSL_MSG("Ticket SNI mismatch");
40200+
decryptRet = WOLFSSL_TICKET_RET_REJECT;
40201+
goto cleanup;
40202+
}
40203+
}
40204+
#endif
40205+
#ifdef HAVE_ALPN
40206+
{
40207+
byte curHash[TICKET_BINDING_HASH_SZ];
40208+
if (TicketAlpnHash(ssl, curHash) != 0 ||
40209+
XMEMCMP(curHash, it->alpnHash,
40210+
TICKET_BINDING_HASH_SZ) != 0) {
40211+
WOLFSSL_MSG("Ticket ALPN mismatch");
40212+
decryptRet = WOLFSSL_TICKET_RET_REJECT;
40213+
goto cleanup;
40214+
}
40215+
}
40216+
#endif
40217+
4009340218
DoClientTicketFinalize(ssl, it, NULL);
4009440219

4009540220
cleanup:

src/ssl_sess.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,6 +3057,7 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess,
30573057
(void)idx;
30583058

30593059
if (sess != NULL) {
3060+
wolfSSL_FreeSession(NULL, *sess);
30603061
*sess = s;
30613062
}
30623063

src/x509_str.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -705,28 +705,26 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
705705

706706
/* We found our issuer in the non-trusted cert list, add it
707707
* to the CM and verify the current cert against it */
708-
#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT)
709-
/* OpenSSL doesn't allow the cert as CA if it is not CA:TRUE for
710-
* intermediate certs.
711-
*/
708+
/* RFC 5280 4.2.1.9: reject non-CA issuer. */
712709
if (!issuer->isCa) {
713-
/* error depth is current depth + 1 */
714710
SetupStoreCtxError_ex(ctx, X509_V_ERR_INVALID_CA,
715711
(ctx->chain) ? (int)(ctx->chain->num + 1) : 1);
712+
#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT)
716713
if (ctx->store->verify_cb) {
717714
ret = ctx->store->verify_cb(0, ctx);
718715
if (ret != WOLFSSL_SUCCESS) {
719716
ret = WOLFSSL_FAILURE;
720717
goto exit;
721718
}
722719
}
723-
else {
720+
else
721+
#endif
722+
{
724723
ret = WOLFSSL_FAILURE;
725724
goto exit;
726725
}
727-
} else
728-
#endif
729-
{
726+
}
727+
else {
730728
ret = X509StoreAddCa(ctx->store, issuer,
731729
WOLFSSL_TEMP_CA);
732730
if (ret != WOLFSSL_SUCCESS) {

tests/api/test_asn.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,7 @@ int test_DecodeAltNames_length_underflow(void)
969969
0x06, 0x03, 0x55, 0x1d, 0x13, 0x01, 0x01, 0xff, 0x04, 0x02, 0x30, 0x00,
970970
/* SAN extension: correct SEQUENCE length 0x06 */
971971
0x30, 0x0f, 0x06, 0x03, 0x55, 0x1d, 0x11, 0x04, 0x08, 0x30, 0x06, 0x82,
972-
0x04, 0x61, 0x2a, 0x00, 0x2a, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e,
972+
0x04, 0x61, 0x2a, 0x62, 0x2a, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e,
973973
0x04, 0x16, 0x04, 0x14, 0x92, 0x6a, 0x1e, 0x52, 0x3a, 0x1a, 0x57, 0x9f,
974974
0xc9, 0x82, 0x9a, 0xce, 0xc8, 0xc0, 0xa9, 0x51, 0x9d, 0x2f, 0xc7, 0x72,
975975
0x30, 0x1f, 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04, 0x18, 0x30, 0x16, 0x80,
@@ -1025,6 +1025,16 @@ int test_DecodeAltNames_length_underflow(void)
10251025
WC_NO_ERR_TRACE(ASN_PARSE_E));
10261026
wc_FreeDecodedCert(&cert);
10271027

1028+
/* NUL in dNSName SAN must be rejected per RFC 5280 4.2.1.6. */
1029+
XMEMCPY(bad_san_cert, good_san_cert, sizeof(good_san_cert));
1030+
bad_san_cert[SAN_SEQ_LEN_OFFSET + 5] = 0x00;
1031+
1032+
wc_InitDecodedCert(&cert, bad_san_cert, (word32)sizeof(bad_san_cert),
1033+
NULL);
1034+
ExpectIntEQ(wc_ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL),
1035+
WC_NO_ERR_TRACE(ASN_PARSE_E));
1036+
wc_FreeDecodedCert(&cert);
1037+
10281038
#endif /* !NO_CERTS && !NO_RSA && !NO_ASN */
10291039
return EXPECT_RESULT();
10301040
}

tests/api/test_ossl_x509.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ int test_wolfSSL_X509_bad_altname(void)
11361136
0xf5, 0xe5, 0x09, 0x02, 0x01, 0x03, 0xa3, 0x61, 0x30, 0x5f, 0x30, 0x0c,
11371137
0x06, 0x03, 0x55, 0x1d, 0x13, 0x01, 0x01, 0xff, 0x04, 0x02, 0x30, 0x00,
11381138
0x30, 0x0f, 0x06, 0x03, 0x55, 0x1d, 0x11, 0x04, 0x08, 0x30, 0x06, 0x82,
1139-
0x04, 0x61, 0x2a, 0x00, 0x2a, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e,
1139+
0x04, 0x61, 0x2a, 0x62, 0x2a, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e,
11401140
0x04, 0x16, 0x04, 0x14, 0x92, 0x6a, 0x1e, 0x52, 0x3a, 0x1a, 0x57, 0x9f,
11411141
0xc9, 0x82, 0x9a, 0xce, 0xc8, 0xc0, 0xa9, 0x51, 0x9d, 0x2f, 0xc7, 0x72,
11421142
0x30, 0x1f, 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04, 0x18, 0x30, 0x16, 0x80,
@@ -1175,8 +1175,7 @@ int test_wolfSSL_X509_bad_altname(void)
11751175
ExpectNotNull(x509 = wolfSSL_X509_load_certificate_buffer(
11761176
malformed_alt_name_cert, certSize, SSL_FILETYPE_ASN1));
11771177

1178-
/* malformed_alt_name_cert has a malformed alternative
1179-
* name of "a*\0*". Ensure that it does not match "aaaaa" */
1178+
/* SAN "a*b*" must not match "aaaaa" under any wildcard flag. */
11801179
ExpectIntNE(wolfSSL_X509_check_host(x509, name, nameLen,
11811180
WOLFSSL_ALWAYS_CHECK_SUBJECT, NULL), 1);
11821181

0 commit comments

Comments
 (0)