Skip to content

25.3.8-fips: restrict TLS 1.3 ciphersuites to FIPS-approved algorithms#1562

Open
mkmkme wants to merge 2 commits intoreleases/25.3.8-fipsfrom
mkmkme/fips/ciphersuites
Open

25.3.8-fips: restrict TLS 1.3 ciphersuites to FIPS-approved algorithms#1562
mkmkme wants to merge 2 commits intoreleases/25.3.8-fipsfrom
mkmkme/fips/ciphersuites

Conversation

@mkmkme
Copy link
Collaborator

@mkmkme mkmkme commented Mar 20, 2026

SSL_CTX_set_cipher_list() only controls TLS 1.2 and below. TLS 1.3 ciphersuites are a separate namespace controlled by SSL_CTX_set_ciphersuites(), which was never called. This meant all TLS 1.3 ciphersuites were allowed by default, including non-FIPS-approved ones like TLS_CHACHA20_POLY1305_SHA256.

Add a new "cipherSuites" parameter (mirroring the existing "cipherList") that calls SSL_CTX_set_ciphersuites(). The default is set to "TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256", which are the only two FIPS-approved TLS 1.3 ciphersuites. This can be overridden via the XML config element under openSSL.server/client.

The parameter is plumbed through all three SSL context creation paths: SSLManager (general), TLSHandler (native TCP), and PostgreSQLHandler (PostgreSQL wire protocol).

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

restrict TLS 1.3 ciphersuites to FIPS-approved algorithms

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

mkmkme added 2 commits March 20, 2026 14:09
SSL_CTX_set_cipher_list() only controls TLS 1.2 and below. TLS 1.3
ciphersuites are a separate namespace controlled by
SSL_CTX_set_ciphersuites(), which was never called. This meant all
TLS 1.3 ciphersuites were allowed by default, including non-FIPS-approved
ones like TLS_CHACHA20_POLY1305_SHA256.

Add a new "cipherSuites" parameter (mirroring the existing "cipherList")
that calls SSL_CTX_set_ciphersuites(). The default is set to
"TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256", which are the only
two FIPS-approved TLS 1.3 ciphersuites. This can be overridden via the
<cipherSuites> XML config element under openSSL.server/client.

The parameter is plumbed through all three SSL context creation paths:
SSLManager (general), TLSHandler (native TCP), and PostgreSQLHandler
(PostgreSQL wire protocol).
@mkmkme
Copy link
Collaborator Author

mkmkme commented Mar 20, 2026

AI audit note: This review comment was generated by AI (gpt-5.3-codex).

Audit update for PR #1562 (restrict TLS 1.3 ciphersuites for FIPS; AWS-LC 2.0 FIPS / FIPS 140-3 context):

Confirmed defects

Low: Cipher configuration OpenSSL calls in Context::init ignore return values

  • Impact: A bad cipherList (TLS ≤1.2) or cipherSuites (TLS 1.3) string can be rejected by the SSL library while context creation still succeeds; the process only discovers the problem later (handshake / client errors), and compliance-focused deployments cannot rely on "fail on bad crypto config" at init time.
  • Anchor: base/poco/NetSSL_OpenSSL/src/Context.cpp / Context::initSSL_CTX_set_cipher_list (pre-existing) and SSL_CTX_set_ciphersuites (added by this PR); neither return value is checked.
  • Trigger: Non-empty but invalid or unmatchable cipher strings in config / Context::Params.
  • Why defect: The library APIs signal failure explicitly; swallowing failures breaks the implied contract that configured policy was applied.
  • Fix direction (short): Check both return values and throw SSLContextException (or equivalent) with Utility::getLastError() on failure; keep TLS 1.2 and 1.3 handling symmetric.
  • Regression test direction (short): Integration or unit test that supplies an invalid cipherList / cipherSuites and expects deterministic startup failure (or a single well-defined error path).

Note: This is not treated as a TLS-1.3-only or AWS-LC–specific regression: the same pattern already existed for cipherList. Severity is Low because typical FIPS/AWS-LC builds bound default algorithms more tightly than generic OpenSSL, so "wrong fallback" is less often a raw non-FIPS algorithm exposure; the main harm is silent misconfiguration and weaker assurability.

Coverage summary

  • Scope reviewed: PR 25.3.8-fips: restrict TLS 1.3 ciphersuites to FIPS-approved algorithms #1562 full diff (Poco Context::{Params,init}, SSLManager config keys/defaults, TLSHandler, PostgreSQLHandler); tip including doc fix; OpenSSL 3.x SSL_CTX_set_ciphersuites / set_ciphersuites failure semantics; AWS-LC in-tree ssl_cipher.cc / SSL_CTX_set_ciphersuites for failure/empty-list behavior vs stock OpenSSL.
  • Categories failed: Configuration → OpenSSL API error propagation (partial: one Low, symmetric TLS 1.2/1.3).
  • Categories passed: TLS 1.3 suite control wired where only cipher_list applied before; defaults match stated AES-GCM-only FIPS intent; config plumbing consistent across SSLManager, native TCP TLS, and PostgreSQL SSL; Params default path; documentation corrected for default cipherSuites semantics; no High/Medium FIPS-boundary or concurrency issues identified in this scope.
  • Fault-category completion (abbreviated): Config parsing / defaults — Executed, pass. OpenSSL setter error contract — Executed, Low finding (unchecked returns, shared with pre-existing cipher_list). Cross-stack (AWS-LC vs OpenSSL) failure atomicity — Executed, pass for High (no confirmed crypto escape specific to this PR on AWS-LC FIPS). Concurrency / shared state — Not applicable (init path). Integration (Keeper-style partial Params) — Executed, pass (defaults supply cipherSuites).
  • Assumptions/limits: No runtime handshake against live AWS-LC FIPS binary in this review; behavior of explicit empty <cipherSuites/> in XML not exhaustively validated against Poco getString semantics.

@DimensionWieldr
Copy link
Collaborator

DimensionWieldr commented Mar 20, 2026

Looks good overall. The FIPS ciphersuite selection is correct.

An additional issue found from AI Audit (super minor):

One minor nit: the XML example in `SSLManager.h` (line 81) lists the ciphersuites in a different order than the actual default used everywhere else:

- XML example: `TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384` (128 first)
- Actual defaults in `Context.cpp`, `SSLManager.cpp`, and the prose doc: `TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256` (256 first)

Since ciphersuite order implies preference, it would be good to make the XML example match the code defaults (AES-256 first).

The regression tests that verify that CH server only accepts FIPS 140-3 compatible secure connections on a given port (using the openssl s_client utility) are now passing thanks to the changes in this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants