Skip to content

Commit 5eb0263

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 - examples/client: increase SESSION_TICKET_LEN fallback from 256 to 2048 to support larger tickets
1 parent 6074a2d commit 5eb0263

12 files changed

Lines changed: 376 additions & 58 deletions

File tree

examples/client/client.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ static int quieter = 0; /* Print fewer messages. This is helpful with overly
155155
#ifdef HAVE_SESSION_TICKET
156156

157157
#ifndef SESSION_TICKET_LEN
158-
#define SESSION_TICKET_LEN 256
158+
#define SESSION_TICKET_LEN 2048
159159
#endif
160160
static int sessionTicketCB(WOLFSSL* ssl,
161161
const unsigned char* ticket, int ticketSz,

src/internal.c

Lines changed: 148 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39249,6 +39249,47 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3924939249
return ret;
3925039250
}
3925139251

39252+
#ifdef HAVE_SNI
39253+
/* Hash the server-selected SNI into dst (TICKET_BINDING_HASH_SZ bytes).
39254+
* Zeros dst when no SNI is present. */
39255+
static int TicketSniHash(WOLFSSL* ssl, byte* dst)
39256+
{
39257+
char* name = NULL;
39258+
word16 nameLen;
39259+
39260+
nameLen = TLSX_SNI_GetRequest(ssl->extensions,
39261+
WOLFSSL_SNI_HOST_NAME,
39262+
(void**)&name, 0);
39263+
if (name != NULL && nameLen > 0) {
39264+
return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)name,
39265+
nameLen, dst, TICKET_BINDING_HASH_SZ);
39266+
}
39267+
39268+
XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);
39269+
return 0;
39270+
}
39271+
#endif
39272+
39273+
#ifdef HAVE_ALPN
39274+
/* Hash the negotiated ALPN protocol into dst (TICKET_BINDING_HASH_SZ
39275+
* bytes). Zeros dst when no ALPN was negotiated. */
39276+
static int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
39277+
{
39278+
char* proto = NULL;
39279+
word16 protoLen = 0;
39280+
39281+
if (TLSX_ALPN_GetRequest(ssl->extensions, (void**)&proto,
39282+
&protoLen) == WOLFSSL_SUCCESS &&
39283+
proto != NULL && protoLen > 0) {
39284+
return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)proto,
39285+
protoLen, dst, TICKET_BINDING_HASH_SZ);
39286+
}
39287+
39288+
XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);
39289+
return 0;
39290+
}
39291+
#endif
39292+
3925239293
/* create a new session ticket, 0 on success
3925339294
* Do any kind of setup in SetupTicket */
3925439295
int CreateTicket(WOLFSSL* ssl)
@@ -39347,6 +39388,18 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3934739388
it->sessionCtxSz = ssl->sessionCtxSz;
3934839389
XMEMCPY(it->sessionCtx, ssl->sessionCtx, ID_LEN);
3934939390
#endif
39391+
#ifdef HAVE_SNI
39392+
ret = TicketSniHash(ssl, it->sniHash);
39393+
if (ret != 0)
39394+
goto error;
39395+
XMEMCPY(ssl->session->sniHash, it->sniHash, TICKET_BINDING_HASH_SZ);
39396+
#endif
39397+
#ifdef HAVE_ALPN
39398+
ret = TicketAlpnHash(ssl, it->alpnHash);
39399+
if (ret != 0)
39400+
goto error;
39401+
XMEMCPY(ssl->session->alpnHash, it->alpnHash, TICKET_BINDING_HASH_SZ);
39402+
#endif
3935039403

3935139404
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
3935239405
!defined(NO_CERT_IN_TICKET)
@@ -39701,6 +39754,28 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3970139754
XMEMCMP(psk->it->sessionCtx, ssl->sessionCtx,
3970239755
ssl->sessionCtxSz) != 0))
3970339756
return WOLFSSL_FATAL_ERROR;
39757+
#endif
39758+
#ifdef HAVE_SNI
39759+
{
39760+
byte curHash[TICKET_BINDING_HASH_SZ];
39761+
if (TicketSniHash((WOLFSSL*)ssl, curHash) != 0 ||
39762+
XMEMCMP(curHash, psk->it->sniHash,
39763+
TICKET_BINDING_HASH_SZ) != 0) {
39764+
WOLFSSL_MSG("Ticket SNI mismatch");
39765+
return WOLFSSL_FATAL_ERROR;
39766+
}
39767+
}
39768+
#endif
39769+
#ifdef HAVE_ALPN
39770+
{
39771+
byte curHash[TICKET_BINDING_HASH_SZ];
39772+
if (TicketAlpnHash((WOLFSSL*)ssl, curHash) != 0 ||
39773+
XMEMCMP(curHash, psk->it->alpnHash,
39774+
TICKET_BINDING_HASH_SZ) != 0) {
39775+
WOLFSSL_MSG("Ticket ALPN mismatch");
39776+
return WOLFSSL_FATAL_ERROR;
39777+
}
39778+
}
3970439779
#endif
3970539780
return 0;
3970639781
}
@@ -39713,36 +39788,54 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3971339788
word16 peerCertLen = 0;
3971439789
ato16(it->peerCertLen, &peerCertLen);
3971539790

39716-
if (peerCertLen > 0 && peerCertLen <= MAX_TICKET_PEER_CERT_SZ) {
39791+
/* Clear any peer cert state that may have been copied from the session
39792+
* cache by wolfSSL_DupSession before we got here. */
39793+
FreeX509(&ssl->peerCert);
39794+
InitX509(&ssl->peerCert, 0, ssl->heap);
3971739795
#ifdef SESSION_CERTS
39718-
/* Clear existing chain and add the peer certificate */
39719-
ssl->session->chain.count = 0;
39720-
AddSessionCertToChain(&ssl->session->chain,
39721-
it->peerCert, peerCertLen);
39796+
ssl->session->chain.count = 0;
3972239797
#endif
39723-
/* Also decode into ssl->peerCert for direct access */
39724-
{
39725-
int ret;
39726-
DecodedCert* dCert;
39727-
39728-
dCert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), ssl->heap,
39729-
DYNAMIC_TYPE_DCERT);
39730-
if (dCert != NULL) {
39731-
InitDecodedCert(dCert, it->peerCert, peerCertLen, ssl->heap);
39732-
ret = ParseCertRelative(dCert, CERT_TYPE, 0, NULL, NULL);
39733-
if (ret == 0) {
39798+
39799+
if (peerCertLen > 0 && peerCertLen <= MAX_TICKET_PEER_CERT_SZ) {
39800+
int ret;
39801+
DecodedCert* dCert;
39802+
39803+
dCert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), ssl->heap,
39804+
DYNAMIC_TYPE_DCERT);
39805+
if (dCert != NULL) {
39806+
int verify = ssl->options.verifyPeer ? VERIFY : NO_VERIFY;
39807+
InitDecodedCert(dCert, it->peerCert, peerCertLen, ssl->heap);
39808+
/* Re-verify against the current trust store so that CA
39809+
* removal since ticket issue is enforced. */
39810+
ret = ParseCertRelative(dCert, CERT_TYPE, verify,
39811+
SSL_CM(ssl), NULL);
39812+
#ifdef HAVE_OCSP
39813+
/* ParseCertRelative does not check revocation status.
39814+
* Run OCSP if the CertManager has it enabled. */
39815+
if (ret == 0 && SSL_CM(ssl)->ocspEnabled) {
39816+
ret = CheckCertOCSP_ex(SSL_CM(ssl)->ocsp, dCert, ssl);
39817+
}
39818+
#endif
39819+
#ifdef HAVE_CRL
39820+
if (ret == 0 && SSL_CM(ssl)->crlEnabled) {
39821+
ret = CheckCertCRL(SSL_CM(ssl)->crl, dCert);
39822+
}
39823+
#endif
39824+
if (ret == 0) {
39825+
#ifdef SESSION_CERTS
39826+
AddSessionCertToChain(&ssl->session->chain,
39827+
it->peerCert, peerCertLen);
39828+
#endif
39829+
FreeX509(&ssl->peerCert);
39830+
InitX509(&ssl->peerCert, 0, ssl->heap);
39831+
ret = CopyDecodedToX509(&ssl->peerCert, dCert);
39832+
if (ret != 0) {
3973439833
FreeX509(&ssl->peerCert);
3973539834
InitX509(&ssl->peerCert, 0, ssl->heap);
39736-
ret = CopyDecodedToX509(&ssl->peerCert, dCert);
39737-
if (ret != 0) {
39738-
/* Failed to copy - clear peerCert */
39739-
FreeX509(&ssl->peerCert);
39740-
InitX509(&ssl->peerCert, 0, ssl->heap);
39741-
}
3974239835
}
39743-
FreeDecodedCert(dCert);
39744-
XFREE(dCert, ssl->heap, DYNAMIC_TYPE_DCERT);
3974539836
}
39837+
FreeDecodedCert(dCert);
39838+
XFREE(dCert, ssl->heap, DYNAMIC_TYPE_DCERT);
3974639839
}
3974739840
}
3974839841
}
@@ -39888,6 +39981,12 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3988839981
it->sessionCtxSz = sess->sessionCtxSz;
3988939982
XMEMCPY(it->sessionCtx, sess->sessionCtx, sess->sessionCtxSz);
3989039983
#endif
39984+
#ifdef HAVE_SNI
39985+
XMEMCPY(it->sniHash, sess->sniHash, TICKET_BINDING_HASH_SZ);
39986+
#endif
39987+
#ifdef HAVE_ALPN
39988+
XMEMCPY(it->alpnHash, sess->alpnHash, TICKET_BINDING_HASH_SZ);
39989+
#endif
3989139990
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
3989239991
defined(SESSION_CERTS) && !defined(NO_CERT_IN_TICKET)
3989339992
/* Store peer certificate from session chain */
@@ -40132,6 +40231,31 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
4013240231
goto cleanup;
4013340232
}
4013440233

40234+
#ifdef HAVE_SNI
40235+
{
40236+
byte curHash[TICKET_BINDING_HASH_SZ];
40237+
if (TicketSniHash(ssl, curHash) != 0 ||
40238+
XMEMCMP(curHash, it->sniHash,
40239+
TICKET_BINDING_HASH_SZ) != 0) {
40240+
WOLFSSL_MSG("Ticket SNI mismatch");
40241+
decryptRet = WOLFSSL_TICKET_RET_REJECT;
40242+
goto cleanup;
40243+
}
40244+
}
40245+
#endif
40246+
#ifdef HAVE_ALPN
40247+
{
40248+
byte curHash[TICKET_BINDING_HASH_SZ];
40249+
if (TicketAlpnHash(ssl, curHash) != 0 ||
40250+
XMEMCMP(curHash, it->alpnHash,
40251+
TICKET_BINDING_HASH_SZ) != 0) {
40252+
WOLFSSL_MSG("Ticket ALPN mismatch");
40253+
decryptRet = WOLFSSL_TICKET_RET_REJECT;
40254+
goto cleanup;
40255+
}
40256+
}
40257+
#endif
40258+
4013540259
DoClientTicketFinalize(ssl, it, NULL);
4013640260

4013740261
cleanup:

src/ssl_sess.c

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

30713071
if (sess != NULL) {
3072+
wolfSSL_FreeSession(NULL, *sess);
30723073
*sess = s;
30733074
}
30743075

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)