zd/21661: harden X.509 chain validation, session ticket identity binding, and peer cert restore#10279
Open
julek-wolfssl wants to merge 4 commits intowolfSSL:masterfrom
Open
zd/21661: harden X.509 chain validation, session ticket identity binding, and peer cert restore#10279julek-wolfssl wants to merge 4 commits intowolfSSL:masterfrom
julek-wolfssl wants to merge 4 commits intowolfSSL:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens X.509 and resumption security in wolfSSL by (1) enforcing stricter intermediate CA requirements during chain building, (2) rejecting SAN IA5String entries with embedded NULs, and (3) re-checking peer certificates restored from session tickets against the current trust store.
Changes:
- Reject embedded NUL bytes in SAN
dNSName,rfc822Name, anduniformResourceIdentifierduring ASN.1 decode. - Enforce
CA:TRUEfor non-trusted issuers used as intermediates inwolfSSL_X509_verify_cert. - Re-parse/re-verify the peer certificate embedded in a session ticket when restoring
ssl->peerCert, and adjust tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
wolfcrypt/src/asn.c |
Adds embedded-NUL rejection for selected IA5String SAN GeneralName types. |
src/x509_str.c |
Makes intermediate CA:TRUE enforcement unconditional (with callback override only under OpenSSL/QT builds). |
src/internal.c |
Re-verifies ticket-restored peer cert against current CertManager when verifyPeer is enabled. |
tests/api/test_asn.c |
Adds/adjusts unit coverage to ensure NUL in dNSName SAN is rejected. |
tests/api/test_ossl_x509.c |
Updates malformed SAN test vector away from embedded-NUL to remain loadable under the new decoder rules. |
tests/test-fails.conf |
Removes CLI “expected failure” cases for SAN-with-NUL certs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
julek-wolfssl
changed the title
julek-wolfssl
force-pushed
the
zd/21661
branch
5 times, most recently
from
47abfb5 to
aa9d6e2
Compare
…ing, 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
When verify_cb returned WOLFSSL_SUCCESS to suppress X509_V_ERR_INVALID_CA for a non-CA issuer, control skipped X509StoreVerifyCert and the leaf signature was never checked. Drop the else so signature verification runs on every issuer.
WOLFSSL_X509_V_ERR_INVALID_CA was 24 while X509_V_ERR_INVALID_CA from the OpenSSL-compat header is 79. In OPENSSL_COEXIST builds the literal X509_V_ERR_INVALID_CA macro is suppressed to avoid clashing with real OpenSSL, so referencing it from src/x509_str.c failed to compile. Move WOLFSSL_X509_V_ERR_INVALID_CA to 79 so the wolfSSL native code matches the OpenSSL value, bump WC_OSSL_V509_V_ERR_MAX to 80, and use the WOLFSSL_-prefixed constant in wolfSSL_X509_verify_cert. Extend error_test()'s missing-value table to cover the new gaps (24 and 65-78). Also skip test_tls13_ticket_peer_cert_reverify under WOLFSSL_NO_DEF_TICKET_ENC_CB, since without a ticket encryption callback the server never emits a NewSessionTicket and the test's resumption step cannot run.
The early checks in DoClientTicketCheck and DoClientTicket ran before the corresponding extensions were parsed, so the computed current hash was zero while the ticket's stored hash was non-zero, rejecting valid resumptions in the nginx, haproxy, grpc and CPython integration tests. * TLS 1.3: DoTls13ClientHello processes pre_shared_key before ALPN_Select, so TLSX_ALPN_GetRequest returned WOLFSSL_ALPN_NOT_FOUND. * TLS 1.2: ClientHello extensions are parsed in wire order; clients that put SessionTicket before server_name / ALPN hit the same problem with both bindings. Consolidate the verification into a single VerifyTicketBinding() function, called once on the server after ALPN_Select (in both DoTls13ClientHello and DoClientHello). DoClientTicketFinalize copies the ticket's stored bindings onto ssl->session so the deferred check has the values to compare. The early per-call sites are removed. VerifyTicketBinding returns WOLFSSL_FATAL_ERROR on mismatch; the caller currently aborts the handshake. Behaviour on mismatch (error vs fallback to a fresh handshake) can be revisited from this single point.
Member
Author
|
retest this please |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
with CRL/OCSP checks; clear stale state from session cache on
verification failure
add test_tls13_ticket_peer_cert_reverify for CA-removal scenario
overwrite
selected hash (TICKET_BINDING_HASH_TYPE); reject resumption on
mismatch in both TLS 1.3 and TLS 1.2 paths