Skip to content

Commit 7ce068a

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 31278ee commit 7ce068a

11 files changed

Lines changed: 345 additions & 164 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
@@ -39248,6 +39248,47 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3924839248
return ret;
3924939249
}
3925039250

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

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

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

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

4013640260
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)