Skip to content

Refactor TLS SSL context configuration#7974

Open
achamayou with Copilot wants to merge 17 commits into
mainfrom
copilot/refactor-ssl-context-usage
Open

Refactor TLS SSL context configuration#7974
achamayou with Copilot wants to merge 17 commits into
mainfrom
copilot/refactor-ssl-context-usage

Conversation

Copilot AI commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Closes #3461.

TLS connections configured both SSL_CTX and SSL directly, even though SSL inherits connection options from its context. This refactors setup so each connection’s SSL_CTX is fully configured before creating the per-connection SSL.

  • Context-first TLS setup

    • Delay SSL creation until after certificate, verification, and ALPN configuration is applied to SSL_CTX.
    • Keep connection-specific setup, such as SNI, on SSL.
  • Clear certificate configuration phases

    • Split certificate application into explicit context and connection phases:
      cert->configure_context(cfg);
      create_ssl();
      cert->configure_connection(get_ssl());
  • Safer OpenSSL handling

    • Add checks for touched OpenSSL calls and allocations.
    • Use RAII during BIO setup until ownership is transferred to OpenSSL.
  • Focused coverage

    • Add a TLS unit test confirming verification mode is inherited by the created SSL connection.

Copilot AI and others added 8 commits June 24, 2026 21:03
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor use of SSL context objects Refactor TLS SSL context configuration Jun 24, 2026
Copilot AI requested a review from achamayou June 24, 2026 21:15
@achamayou

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts in commit 766754b2b. Targeted validation passed with cmake --build build --target tls_test and ./build/tls_test.

achamayou and others added 3 commits June 29, 2026 18:19
Configure certificates, trust stores, and context modes before the SSL BIO creates its connection, then apply connection-specific settings before the handshake.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@achamayou
achamayou marked this pull request as ready for review July 15, 2026 07:41
@achamayou
achamayou requested a review from a team as a code owner July 15, 2026 07:41
Copilot AI review requested due to automatic review settings July 15, 2026 07:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the TLS setup flow so SSL_CTX is fully configured before constructing the per-connection SSL, aligning configuration with OpenSSL inheritance semantics and clarifying which settings are context-wide vs connection-specific.

Changes:

  • Split certificate configuration into Cert::configure_context(SSL_CTX*) and Cert::configure_connection(SSL*), and update callers accordingly.
  • Delay SSL creation in ccf::tls::Context until after SSL_CTX configuration is complete; add RAII protection for BIO allocation until ownership transfer.
  • Add a TLS unit test ensuring a connection’s SSL inherits verification mode from its configured SSL_CTX.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/tls/context.h Defers SSL creation until after SSL_CTX setup; improves OpenSSL allocation safety around BIO ownership transfer.
src/tls/cert.h Splits cert configuration into context vs connection phases (e.g., verification/certs vs SNI).
src/tls/client.h Updates TLS test client to configure context first, then create/configure the connection.
src/tls/server.h Updates TLS test server to configure context (including ALPN) before creating the connection SSL.
src/clients/tls_client.h Refactors client TLS context creation to apply SSL_CTX config before BIO/SSL creation; keeps connection-only config on SSL.
src/tls/test/main.cpp Updates existing cert verification test and adds inheritance-focused test coverage.

Comment thread src/tls/cert.h Outdated
Comment thread src/tls/cert.h Outdated
Comment thread src/tls/test/main.cpp
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@achamayou

Copy link
Copy Markdown
Member

@copilot fix the code format/syntax

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>

Copilot AI commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix the code format/syntax

Fixed in the latest commit. The previous code review suggestions accidentally dropped the opening braces { from configure_context and configure_connection — those are now restored.

Copilot AI requested a review from achamayou July 16, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor use of SSL context objects

3 participants