Fully disabled TLS 1.2 - #11324
Open
Frauschi wants to merge 5 commits into
Open
Conversation
|
Can one of the admins verify this patch? |
Frauschi
force-pushed
the
tls12-disable
branch
2 times, most recently
from
September 1, 2026 08:01
745195a to
2635169
Compare
Contributor
Author
|
Jenkins retest this please |
SparkiDev
previously approved these changes
Sep 2, 2026
wolfSSL-Fenrir-bot
previously requested changes
Sep 2, 2026
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11324
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Frauschi
commented
Sep 2, 2026
Frauschi
commented
Sep 2, 2026
Frauschi
force-pushed
the
tls12-disable
branch
from
September 2, 2026 11:22
2635169 to
febd515
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11324
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
wolfSSL-Fenrir-bot
dismissed
their stale review
September 2, 2026 11:57
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
Frauschi
force-pushed
the
tls12-disable
branch
from
September 2, 2026 12:15
febd515 to
2f109e4
Compare
philljj
requested changes
Sep 4, 2026
philljj
left a comment
philljj
left a comment
Contributor
There was a problem hiding this comment.
merge conflict in src/sniffer.c
test_tls13_downgrade_sentinel() builds a TLS 1.2 server, and a TLS 1.1 one when old TLS is enabled, to make the peer produce the downgrade sentinel a TLS 1.3 client has to reject. Its guard did not mention WOLFSSL_NO_TLS12, so a build without TLS 1.2 failed to compile the unit tests at wolfTLSv1_2_server_method(). There is nothing to downgrade to in such a build, so require TLS 1.2 for the whole test. It still runs everywhere it did before. The status_request_v2 block at the end of test_TLSX_CSR_parse() builds a TLS 1.2 server context the same way and was guarded only on the extension. RFC 6961 is defined for TLS 1.2 and below, so require TLS 1.2 there too. test_tls13_bounds.c has the opposite problem. Its file-level guard requires TLS 1.2 for the legacy-version tests, but the four test_tls13_mutual_auth_* entry points that call test_tls13b_mutual_auth_round() do not, so a build without TLS 1.2 could not link them. Those handshakes are pure TLS 1.3, so the helper moves into its own block without the TLS 1.2 condition rather than being skipped, and now runs in such a build. The key-type condition on the new block keeps it from going unused where none of the four callers compile.
The option set ENABLED_TLSV12 to no, which the summary printed and a couple of derived settings read, but nothing defined WOLFSSL_NO_TLS12, so every line of the TLS 1.2 implementation was still compiled and still reachable. The one place configure.ac adds that define sits in the FIPS lean-aesgcm bundle and is guarded by "TLS 1.2 is still on and the user did not ask for it", which is false exactly when --disable-tlsv12 was passed. Only the CMake build honoured the switch. Define it once the bundles have had their say, so --enable-tinytls13 and the other bundles that turn the version off get it as well. The case guard keeps the FIPS bundles from adding it twice. Add three os-check entries for the configurations this makes reachable: --disable-tlsv12 on its own, the same with DTLS 1.3, and the same with the sniffer. The one existing entry that passes the option, dtls13-client-minimal, already defined WOLFSSL_NO_TLS12 through CPPFLAGS and strips most of the crypto along with it, so it never covered the option's own effect. Compiling the version out reaches code that assumed it was always there. ProcessCSR_ex parses one certificate_status message and TLS 1.3 reads the chain's per-certificate entries through it, so it moves out of the version guard; only its TLS 1.2 wrapper stays behind. BuildMessage's connection-ID size is read by the TLS 1.2 record path alone, so it is declared with it. The tests and examples that hard-code a TLS 1.2 method, or a helper that only those call, are skipped without the version. CMake rejected three of the four combinations configure does but let WOLFSSL_OLD_TLS=yes through, defined NO_OLD_TLS behind the user's back and reported the option as still on. Reject it there too, and drop the NO_OLD_TLS the block used to add, which the old-TLS option itself now always supplies.
examples/benchmark/dtls_bench.c calls wolfSSL_accept and wolfSSL_connect unconditionally, so a build that compiles either side out fails to link: Undefined symbols: "_wolfSSL_accept", referenced from: _main in dtls_bench.o That is the os-check dtls13-client-minimal entry, which passes CPPFLAGS=-DNO_WOLFSSL_SERVER; os-check.yml does not pass --disable-examples, so the benchmark is always built. Compile each DTLS leg out with its own side, the way examples/server/server.c does, and report the missing side from main(). pick_method() and set_mtu() serve only those two legs and go with them, and the send sink is used by the client leg alone, so none of them are left unused. The plain UDP legs need neither side and keep working.
myEccKeyGen() generated the key share straight into the library's key object, and myEccSharedSecret() is handed only the peer's key, so a TLS 1.3 client had no private key left to reach and every handshake ended with: wolfSSL_connect error -170, ECC input argument wrong type, invalid input TEST_PK_PRIVKEY already kept the key on the application's side, which is what a real PK-callback application does anyway; it just was not the default. Keep it whenever the connection is TLS or DTLS v1.3 as well, and have the shared secret callback pick the key by the same question its key gen asked, not by a flag key gen has not set yet. The union holding it stops being TEST_PK_PRIVKEY-only, and its comment stops naming TLS v1.2 as the version that needs it. Neither example zeroed its PkCbInfo, so hasKeyGen started as stack garbage. Only the TEST_PK_PRIVKEY paths read it before, which hid that; zero it in both. scripts/pkcallbacks.test only ever ran the build's default version, and that is TLS v1.2 wherever it is compiled in, so none of the above was covered. Run the default and then each version the build has.
The chain OCSP loops suppress the responder errors that mean "no status to staple", so that a responder problem cannot take the handshake down with it. OCSP_INVALID_STATUS was missing from that list, and it is what CheckOcspRequest() returns when it cannot reach the responder at all - the most ordinary failure of the set. A TLS 1.3 server built with WOLFSSL_TLS_OCSP_MULTI therefore dropped every handshake whenever the responder for any intermediate was down, having already obtained the leaf status the peer actually asked about. Add it to the three chain lists. The leaf list in CreateOcspResponse() keeps failing hard, and says why: that is the certificate the peer asked about, and test_ocsp_callback_fails() pins the behaviour. OCSP_CERT_REVOKED stays fatal everywhere, chain included. Found through testsuite's OCSP responder test, which starts its responder for a single request and so cannot answer the chain request TLS 1.3 adds.
Frauschi
force-pushed
the
tls12-disable
branch
from
September 6, 2026 09:31
2f109e4 to
532d42b
Compare
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
5 participants
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.
--disable-tlsv12never compiled TLS 1.2 out. The option setENABLED_TLSV12=no, which the configure summary printed and a couple of derived settings read, but nothing definedWOLFSSL_NO_TLS12, so the whole TLS 1.2 implementation was still built and a peer could still negotiate it. The single placeconfigure.acadds that define sits in the FIPS lean-aesgcm bundle, behind a condition that is false exactly when the user asked for the option, so no autotools configuration ever reached it. Only the CMakeWOLFSSL_TLSV12=nopath honoured the switch.This makes the option do what it says, and fixes what that exposed.
Behavioral change
--disable-tlsv12now definesWOLFSSL_NO_TLS12. A build that passes it and still expects to negotiate TLS 1.2 has to stop passing it. The define is added after the bundles have had their say, so--enable-tinytls13and the other bundles that turn the version off pick it up too.Because the pre-TLS-1.3 handshake is now actually compiled out, configure rejects the four combinations that depend on it rather than producing a build that cannot work:
--enable-oldtls, since TLS 1.0 and 1.1 use the TLS 1.2 handshake--enable-mcast, which rides on DTLS 1.2 and its NULL cipher suite--enable-dtls13, for the same reason on the datagram sideCMake rejects the same four for
-DWOLFSSL_TLSV12=no. It previously accepted-DWOLFSSL_OLD_TLS=yesalongside it and definedNO_OLD_TLSanyway, so the reported option and the build disagreed.The sniffer, the examples and the test suite now pick their code paths by version where they used to assume TLS 1.2 was present. Four
no-tlsv12-*entries were added to.github/configs/os-check-linux.jsoncovering the option on its own axes: OCSP stapling plus the OpenSSL compatibility layer, status_request_v2 without status_request, DTLS 1.3 with connection ID and session tickets, and the sniffer.Three bugs this uncovered
These are independent of the TLS 1.2 work and are kept as their own commits, so they can be split out if preferred. All three reproduce on master.
PK callbacks could not complete a TLS 1.3 handshake.
myEccKeyGen()generated the key share straight into the library'secc_key, whilemyEccSharedSecret()is handed only the peer's key on the client side, so the client had no private key left to reach:wolfSSL_connect error -170, ECC input argument wrong type. The example callbacks now keep the private half application-side for TLS and DTLS 1.3, which is whatTEST_PK_PRIVKEYalready modelled and what a real PK-callback application has to do anyway.scripts/pkcallbacks.testpassed no-v, so it always negotiated TLS 1.2 and never covered this; it now runs the build's default plus every version the build reports.Chain OCSP stapling failed the handshake when a responder was unreachable. The chain loops suppress the responder errors that mean "no status to staple", but
OCSP_INVALID_STATUSwas missing from that set, and it is whatCheckOcspRequest()returns when it cannot reach the responder at all. A TLS 1.3 server built withWOLFSSL_TLS_OCSP_MULTItherefore dropped every handshake whenever the responder for any intermediate was down, having already obtained the leaf status the peer asked about. The leaf path inCreateOcspResponse()deliberately still fails hard, andOCSP_CERT_REVOKEDstays fatal everywhere.dtls_benchdid not build without one side.examples/benchmark/dtls_bench.ccalledwolfSSL_accept()andwolfSSL_connect()unconditionally, so aNO_WOLFSSL_SERVERbuild failed to link. That is the existingdtls13-client-minimalos-check entry, which passesCPPFLAGS=-DNO_WOLFSSL_SERVERand gets the benchmark built becauseos-check.ymldoes not pass--disable-examples.Testing
Built and
make checked clean (0 failures) on macOS/arm64 with clang, each with-pedantic -Wdeclaration-after-statement -Wnull-dereference -Wno-overlength-strings -Werror:--disable-tlsv12 --enable-ocspstapling --enable-opensslextra--enable-tls13 --disable-tlsv12 --enable-ocspstapling2(-Wall -Wextra -Werror)--disable-tlsv12 --enable-dtls --enable-dtls13 --enable-dtlscid --enable-session-ticket--enable-sniffer --disable-tlsv12snifftestreportstls_v13withouttls_v12and decodes the ecc, dh and hrr pcaps--enable-tinytls13--enable-all --disable-mcast --disable-oldtls --disable-tlsv12--enable-all --disable-mcast --disable-oldtls--enable-all --enable-pkcallbacks, with and without-DTEST_PK_PRIVKEYRejection paths checked by hand: configure errors on
--disable-tlsv12with each of--enable-oldtls,--disable-tls13,--enable-mcastand--enable-dtlswithout dtls13; CMakeFATAL_ERRORs onWOLFSSL_OLD_TLS=yes,WOLFSSL_TLS13=no,WOLFSSL_MCAST=yesandWOLFSSL_DTLS=yeswithoutWOLFSSL_DTLS13, and accepts the valid combinations.For the PK-callback fix specifically: the new
-v 4leg ofscripts/pkcallbacks.testfails against pre-fix binaries and passes after, and the HelloRetryRequest re-keygen path (server -Y) and a two-curve client both complete. For the OCSP fix, a revoked server certificate is still rejected with-360 OCSP Cert revoked, so the suppression does not mask revocation.