Allow disabling TLS 1.2 at Build Time#5318
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the TLS 1.2 / TLS 1.3 disentangling work to allow building Botan with TLS 1.2 disabled (via --disable-modules=tls12) while retaining TLS 1.3 support, and adds CI coverage for that build configuration.
Changes:
- Add CI target
no_tls12(incl. BoGo shim config) to validate “TLS 1.3 without TLS 1.2” builds. - Refactor TLS internals to better separate TLS 1.2 and TLS 1.3 code paths (ClientHello shim, extension splitting, version selection helpers).
- Adjust tests and tooling to respect
BOTAN_HAS_TLS_12/BOTAN_HAS_TLS_13feature availability.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/unit_tls.cpp | Adjust TLS unit test compilation gates and includes for split TLS modules. |
| src/tests/test_tls_stream_integration.cpp | Conditionally include TLS 1.2-related stream integration configurations. |
| src/tests/test_tls_rfc8448.cpp | Gate RFC8448 test execution on required features and use split extension headers. |
| src/tests/test_tls_messages.cpp | Make TLS message tests compile when TLS 1.2 is disabled; update ClientHello shim type usage. |
| src/tests/test_tls.cpp | Adjust policy text test gating to account for module availability. |
| src/tests/data/tls_13/client_hello.vec | Update vector test data expectations after ClientHello/extension refactor. |
| src/scripts/ci_build.py | Add no_tls12 CI target, configure build flags, and run BoGo with a dedicated shim config. |
| src/scripts/ci/gha_linux_packages.py | Ensure no_tls12 installs required dependencies (e.g., Boost). |
| src/lib/tls/tls_version.h | Move latest TLS/DTLS version selection to .cpp and document “available in this build”. |
| src/lib/tls/tls_version.cpp | Implement build-availability-aware latest TLS/DTLS selection and known-version checks. |
| src/lib/tls/tls_session.cpp | Include TLS 1.3 extension header where needed after refactor. |
| src/lib/tls/tls_server.cpp | Select server impl based on available protocol modules; handle downgrade path conditionally. |
| src/lib/tls/tls_policy.cpp | Make protocol acceptance explicitly build-dependent; unify exception path in version selection. |
| src/lib/tls/tls_messages_internal.h | Introduce internal ClientHello container and hello-random helper declaration. |
| src/lib/tls/tls_messages.h | Replace full TLS 1.2 ClientHello with a TLS 1.2 shim in the public TLS core header. |
| src/lib/tls/tls_magic.cpp | Move handshake-type stringification into a shared TLS “magic” implementation unit. |
| src/lib/tls/tls_extensions.h | Remove TLS 1.2/1.3-specific extension class definitions from the common header; clarify exclusivity. |
| src/lib/tls/tls_extensions.cpp | Parse TLS 1.2/1.3-specific extensions only when the respective module is present. |
| src/lib/tls/tls_client.cpp | Select client impl based on available protocol modules; make downgrade handling conditional. |
| src/lib/tls/tls_alert.h | Annotate deprecated compat enum variants with a removal TODO. |
| src/lib/tls/tls13/tls_server_impl_13.h | Use the TLS 1.2 ClientHello shim type for downgrade indication handling. |
| src/lib/tls/tls13/tls_server_impl_13.cpp | Include TLS 1.3 extensions header; adjust handler signature to shim type. |
| src/lib/tls/tls13/tls_messages_13.h | Update ClientHello parser return type to TLS 1.2 shim variant; add required declarations. |
| src/lib/tls/tls13/tls_handshake_state_13.h | Store TLS 1.2 ClientHello shim in TLS 1.3 handshake state. |
| src/lib/tls/tls13/tls_handshake_state_13.cpp | Implement shim-based storage in handshake state. |
| src/lib/tls/tls13/tls_extensions_psk.cpp | Switch include to TLS 1.3-specific extensions header. |
| src/lib/tls/tls13/tls_extensions_key_share.cpp | Switch include to TLS 1.3-specific extensions header. |
| src/lib/tls/tls13/tls_extensions_13.h | New public header for TLS 1.3-specific extensions. |
| src/lib/tls/tls13/tls_extensions_13.cpp | New implementation unit for TLS 1.3-specific extensions. |
| src/lib/tls/tls13/tls_client_impl_13.cpp | Include TLS 1.3 extensions header after refactor. |
| src/lib/tls/tls13/tls_channel_impl_13.cpp | Adjust includes and use ClientHello shim type in message-type checks. |
| src/lib/tls/tls13/msg_session_ticket_13.cpp | Include TLS 1.3 extensions header after refactor. |
| src/lib/tls/tls13/msg_server_hello_13.cpp | Include TLS 1.3 extensions header after refactor. |
| src/lib/tls/tls13/msg_client_hello_13.cpp | New TLS 1.3 ClientHello implementation unit using the internal container and split extensions. |
| src/lib/tls/tls13/msg_certificate_req_13.cpp | Include TLS 1.3 extensions header after refactor. |
| src/lib/tls/tls13/info.txt | Export TLS 1.3 extensions header; drop implicit dependency on tls12 module. |
| src/lib/tls/tls12/tls_server_impl_12.cpp | Include internal hello-random helper after refactor. |
| src/lib/tls/tls12/tls_messages_12.h | Restore full TLS 1.2 ClientHello in the TLS 1.2 module; adjust namespace structure. |
| src/lib/tls/tls12/tls_handshake_state.h | Add missing TLS session include (needed after refactor). |
| src/lib/tls/tls12/tls_handshake_state.cpp | Remove duplicated handshake-type stringification (moved to tls_magic.cpp). |
| src/lib/tls/tls12/tls_extensions_12.h | New public header for TLS 1.2-specific extensions. |
| src/lib/tls/tls12/tls_extensions_12.cpp | New implementation unit for TLS 1.2-specific extensions. |
| src/lib/tls/tls12/msg_server_hello_12.cpp | Switch include to TLS 1.2-specific extensions header. |
| src/lib/tls/tls12/msg_finished_12.cpp | Remove TLS 1.3-only include guard that’s no longer needed here. |
| src/lib/tls/tls12/msg_client_hello_12.cpp | New TLS 1.2 ClientHello implementation unit moved into tls12 module. |
| src/lib/tls/tls12/info.txt | Export TLS 1.2 extensions header and adjust module dependencies. |
| src/lib/tls/msg_client_hello.cpp | Keep internal ClientHello container + hello-random helper in core TLS, add TLS 1.2 shim constructor. |
| src/lib/tls/info.txt | Adjust core TLS module dependencies after disentangling tls12/tls13. |
| src/editors/vscode/scripts/bogo.py | Add a helper flag to run BoGo with TLS 1.2 disabled config and skip DTLS. |
| src/cli/tls_utils.cpp | Update client-hello parsing CLI command to work with TLS 1.2 shim variant and construct full TLS 1.2 CH when needed. |
| src/bogo_shim/config_no_tls12.json | New BoGo shim config for “TLS 1.3-only” builds. |
| src/bogo_shim/bogo_shim.cpp | Adjust includes after extension refactor; keep CertificateRequest extension tweaking logic. |
| .github/workflows/ci.yml | Add the no_tls12 target to the GitHub Actions matrix and fetch BoringSSL for it. |
Comments suppressed due to low confidence (1)
src/scripts/ci_build.py:945
- In the
no_tls12CI target you disable the TLS 1.2 module, which also removes DTLS 1.2 support. The BoGo runner is only passed-skip-tls12, so DTLS test cases will still run and are likely to fail. Add-skip-dtls(and/or otherwise ensure DTLS tests are disabled) for theno_tls12configuration to match the VSCode helper script and the actual feature set of that build.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a7ba5c3 to
005a803
Compare
005a803 to
496a5fb
Compare
| <requires> | ||
| aead | ||
| aes | ||
| asn1 | ||
| dh | ||
| ecdh | ||
| ecdsa | ||
| gcm | ||
| hmac | ||
| rng | ||
| sha2_32 | ||
| sha2_64 | ||
| tls12 | ||
| x509 | ||
| </requires> |
There was a problem hiding this comment.
Side note (not to be addressed here): it would be actually nice to not hard-depend on specific algorithms here. Specialised applications might have a fairly narrow (and predefined) zoo of algorithms that they actually need for TLS within their ecosystem. Nevertheless, this pulls in much more than actually needed.
For instance, TLS will really need just AEAD, RNG, and perhaps things like HKDF and maybe even SHA. But it could certainly work without Finite-Field Diffie-Hellman, RSA (currently required in TLS 1.2) and GCM in some contexts.
On the other hand, this makes the module selection much harder and frustrating for novice users. Often times, missing modules show up only at runtime and are fairly hard to debug. Or catching all relevant platform acceleration modules is tricky and also a maintenance burden when updating.
Perhaps an additional <recommended> block of optional module dependencies could help with that? So that one could build along the lines of --minimized-build --enable-modules=tls13 --enable-recommended-dependencies.
There was a problem hiding this comment.
But it could certainly work without Finite-Field Diffie-Hellman, RSA (currently required in TLS 1.2) and GCM in some contexts.
Yeah these are a very long standing todos
fc3f52703f doc/todo.rst (Jack Lloyd 2017-02-07 09:46:02 -0500 66) * Make RSA optional at build time
e29024608f doc/todo.rst (Jack Lloyd 2016-08-13 11:13:49 -0400 67) * Make finite field DH optional at build time
TBH for DH at least I was content to leave it be, considering FFDHE is on the docket to be removed entirely in Botan4.
Or catching all relevant platform acceleration modules is tricky and also a maintenance burden when updating.
This should just be fixed where the machine specific implementations are enabled unless specifically disabled by the user.
Perhaps an additional block of optional module dependencies could help with that?
Yeah that would likely be helpful, kind of similar to the if_available block in the module policies. If it's in <recommended> then it's included unless some other setting (eg explicit --disable-modules= or forbidden by module policy) would have prohibited it.
I guess with that the above case of the machine specific implementations could then be resolved via just adding <recommended> blocks to each of the base module info.txt files.
This let's applications disable support for TLS 1.2 at compile time while still being able to use TLS 1.3. Before this wasn't possible. Vice versa, disabling TLS 1.3 while still supporting TLS 1.2 was possible even before this patch.
... this builds the library without support for TLS 1.2 and runs all available tests under that assumption, particularly the BoGo suite.
496a5fb to
050f458
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
See here for an overview of the related pull requests to disentangle TLS 1.2 and 1.3.
Since the integration of TLS 1.3 it was possible to disable it at build time while still being able to use TLS 1.2. However, the opposite wasn't possible. Now applications can build with
--disable-modules=tls12without also loosing support for TLS 1.3. This adds a CI build configuration to exercise this possibility (equivalent to #5309 for TLS 1.3).See #2990 (comment)
Pull Request Dependencies