Refactor: Organize TLS Extensions into TLS 1.2 and 1.3 Modules#5303
Merged
reneme merged 1 commit intorandombit:masterfrom Feb 16, 2026
Merged
Conversation
14 tasks
There was a problem hiding this comment.
Pull request overview
This PR refactors Botan’s TLS stack to separate version-specific TLS extensions (and parts of Client/ServerHello handling) into dedicated TLS 1.2 and TLS 1.3 modules, while keeping cross-version/shared machinery in the common TLS layer. It also adjusts tests and CLI tooling to match the new module boundaries and “shim” message types.
Changes:
- Split TLS extensions into shared (
tls_extensions.h/.cpp) plus version-specific headers/impls (tls_extensions_12.*,tls_extensions_13.*) and make parsing conditional onBOTAN_HAS_TLS_12/13. - Introduce version-agnostic internal hello containers (
tls_messages_internal.h) and useClient_Hello_12_Shim/Server_Hello_12_Shimfor TLS 1.3 downgrade/legacy parsing. - Update TLS tests, vectors, and CLI parsing tools to include and/or depend on the new TLS 1.2/1.3 module headers and shim types.
Locations with semantic (non-move) changes to pay attention to
src/lib/tls/tls_extensions.cpp:make_extension()now conditionally instantiates TLS 1.2 / TLS 1.3 extension objects based onBOTAN_HAS_TLS_12/BOTAN_HAS_TLS_13; otherwise these codes fall through toUnknown_Extension.src/lib/tls/tls_extensions.cpp+src/lib/tls/tls_extensions.h:Server_Name_Indicator::hostname_acceptable_for_sni()moved into the class API (behavior preserved, but now callable externally).src/lib/tls/msg_client_hello.cpp/src/lib/tls/msg_server_hello.cpp+src/lib/tls/tls_magic.h: downgrade markers / HRR marker are now centralized intls_magic.h; server random downgrade signaling is written via astd::spanview of the tail bytes.src/lib/tls/tls13/tls_messages_13.h+src/lib/tls/tls13/msg_client_hello_13.cpp:Client_Hello_13::parse()now returnsClient_Hello_12_Shim(not fullClient_Hello_12). This also means TLS 1.2-specific “fixups” (e.g., fake renegotiation extension insertion based on SCSV) are no longer applied at shim-parse time.src/lib/tls/tls13/msg_client_hello_13.cpp: TLS 1.2 extensions are only added to TLS 1.3 ClientHello construction whenBOTAN_HAS_TLS_12is enabled and policy allows TLS 1.2.src/cli/tls_utils.cpp: CLItls_client_hellois now compiled only when both TLS 1.2 and TLS 1.3 are enabled, and it converts shim parsing into a fullClient_Hello_12by re-parsing the bytes.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/test_tls_rfc8448.cpp | Updates test gating and includes to account for split TLS extension headers and TLS 1.2 dependency. |
| src/tests/test_tls_messages.cpp | Adapts message parsing tests to shim types and updated ServerHello version accessor. |
| src/tests/test_tls_handshake_layer_13.cpp | Updates TLS 1.3 handshake layer test to expect Server_Hello_12_Shim. |
| src/tests/test_tls_cipher_state.cpp | Adds missing standard include required by test code (<map>). |
| src/tests/data/tls_13/client_hello.vec | Updates expected extension-type listing due to shim/extension parsing changes (e.g., no implicit renegotiation ext). |
| src/lib/tls/tls_session.cpp | Adds TLS 1.3 extensions header include for session-related extension use. |
| src/lib/tls/tls_messages_internal.h | New internal header for version-agnostic ClientHello/ServerHello parsing containers and helpers. |
| src/lib/tls/tls_messages.h | Replaces full TLS 1.2 Hello types with shim declarations in common header. |
| src/lib/tls/tls_magic.h | Centralizes downgrade markers and HRR marker values used across modules. |
| src/lib/tls/tls_extensions.h | Removes TLS 1.2/1.3-specific extension class declarations from shared header; annotates version exclusivity in Extension_Code. |
| src/lib/tls/tls_extensions.cpp | Conditionalizes extension instantiation on TLS 1.2/1.3 build flags; moves SNI hostname logic into Server_Name_Indicator. |
| src/lib/tls/tls13/tls_server_impl_13.h | Switches legacy ClientHello handler signature to Client_Hello_12_Shim. |
| src/lib/tls/tls13/tls_server_impl_13.cpp | Includes TLS 1.3 extensions header; updates legacy ClientHello handler signature. |
| src/lib/tls/tls13/tls_messages_13.h | Updates parse variants and handshake message variants to use shim types; adds needed forward decls/includes. |
| src/lib/tls/tls13/tls_handshake_state_13.h | Stores shim message types for TLS 1.2 legacy messages within TLS 1.3 handshake state. |
| src/lib/tls/tls13/tls_handshake_state_13.cpp | Updates store methods to use shim types. |
| src/lib/tls/tls13/tls_extensions_psk.cpp | Switches include to TLS 1.3 specific extensions header. |
| src/lib/tls/tls13/tls_extensions_key_share.cpp | Switches include to TLS 1.3 specific extensions header. |
| src/lib/tls/tls13/tls_extensions_13.h | New public TLS 1.3 extensions header containing TLS 1.3-only extension class declarations. |
| src/lib/tls/tls13/tls_extensions_13.cpp | New TLS 1.3 extensions implementation file (moved from shared extensions). |
| src/lib/tls/tls13/tls_client_impl_13.h | Switches legacy ServerHello handler signature to Server_Hello_12_Shim. |
| src/lib/tls/tls13/tls_client_impl_13.cpp | Includes TLS 1.3 extensions header; updates legacy ServerHello handler signature. |
| src/lib/tls/tls13/tls_channel_impl_13.cpp | Updates type checks to use Client_Hello_12_Shim. |
| src/lib/tls/tls13/msg_session_ticket_13.cpp | Includes TLS 1.3 extensions header after extension split. |
| src/lib/tls/tls13/msg_server_hello_13.cpp | New TLS 1.3 ServerHello/HRR implementation file (moved/split from common). |
| src/lib/tls/tls13/msg_client_hello_13.cpp | New TLS 1.3 ClientHello implementation file (moved/split from common). |
| src/lib/tls/tls13/msg_certificate_req_13.cpp | Includes TLS 1.3 extensions header after extension split. |
| src/lib/tls/tls13/info.txt | Exposes tls_extensions_13.h as a public header for the TLS 1.3 module. |
| src/lib/tls/tls12/tls_server_impl_12.cpp | Adds internal hello container header include needed after refactor. |
| src/lib/tls/tls12/tls_messages_12.h | Reintroduces full TLS 1.2 Client_Hello_12/Server_Hello_12 APIs in TLS 1.2 module header (built on shim). |
| src/lib/tls/tls12/tls_handshake_state.h | Fixes/includes TLS session dependency after header refactor. |
| src/lib/tls/tls12/tls_extensions_12.h | New public TLS 1.2 extensions header containing TLS 1.2-only extension class declarations. |
| src/lib/tls/tls12/tls_extensions_12.cpp | New TLS 1.2 extensions implementation file (moved from shared extensions). |
| src/lib/tls/tls12/msg_server_hello_12.cpp | New TLS 1.2 ServerHello implementation file (moved/split from common). |
| src/lib/tls/tls12/msg_client_hello_12.cpp | New TLS 1.2 ClientHello implementation file (moved/split from common). |
| src/lib/tls/tls12/info.txt | Exposes tls_extensions_12.h as a public header for the TLS 1.2 module. |
| src/lib/tls/msg_server_hello.cpp | Keeps shared ServerHello internals + TLS 1.2 shim; moves TLS 1.2 and TLS 1.3 concrete impls out. |
| src/lib/tls/msg_client_hello.cpp | Keeps shared ClientHello internals + TLS 1.2 shim; moves TLS 1.2 and TLS 1.3 concrete impls out. |
| src/lib/tls/info.txt | Registers new internal header tls_messages_internal.h. |
| src/cli/tls_utils.cpp | Adjusts CLI build guards/includes and adapts parsing to shim + TLS 1.2 re-parse for legacy CH. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cdcd79a to
aea1c62
Compare
924c0c9 to
b998b0b
Compare
This was referenced Feb 12, 2026
Many extensions are understood and semanticaly relevant by both revisions but some are strongly version specific. This moves the lattter ones into their respective tls12 and tls13 modules. The selection roughly follows RFC 8446 Section 4.2: extensions that aren't listed there (anymore) are classified as specific to TLS 1.2, whereas extensions that are introduced by RFC 8446 and that clearly aren't retrofittable, are classified as specific to TLS 1.3. If either TLS 1.2 or TLS 1.3 is disabled at build time, such extensions are not parsed by Botan but their content is captured by the Unknown_Extension class instead.
b998b0b to
2cb874c
Compare
Collaborator
Author
|
Rebased and ready to go after #5293 was merged earlier. |
Collaborator
Author
|
@randombit I can haz green light please? 😅 ... before making it conflict with master again (#5347) |
randombit
approved these changes
Feb 16, 2026
Owner
randombit
left a comment
There was a problem hiding this comment.
Ship it.
Probably want a CI for this (maybe just nightly even?) but ok to merge without
Owner
I had missed that #5318 already has CI setup |
Collaborator
Author
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.
See here for an overview of the related pull requests to disentangle TLS 1.2 and 1.3.
Many extensions are understood and semanticaly relevant by both revisions but some are strongly version specific. This moves the lattter ones into their respective tls12 and tls13 modules. The selection roughly follows RFC 8446 Section 4.2: extensions that aren't listed there (anymore) are classified as specific to TLS 1.2, whereas extensions that are introduced by RFC 8446 and that clearly aren't retrofittable, are classified as specific to TLS 1.3.
If either TLS 1.2 or TLS 1.3 is disabled at build time, such extensions are not parsed by Botan but their content is captured by the Unknown_Extension class instead.
@copilot Again this is mostly moving code around. Please review only the the last commit and put strong empathizes on validating that no relevant implementation changes occurred. Generate a list of locations that didn't just move but where semantic changes were made to the code. This is meant to help a human reviewer with this fairly large patch set.
Pull Request Dependencies