Skip to content

crypto: add rustls TLS foundation alongside openssl - #36085

Closed
jasonhernandez wants to merge 3 commits into
mainfrom
jason/crypto-tls-1-foundation
Closed

jasonhernandez wants to merge 3 commits into
mainfrom
jason/crypto-tls-1-foundation

Conversation

@jasonhernandez

@jasonhernandez jasonhernandez commented Apr 15, 2026 •

Copy link
Copy Markdown
Contributor

Part of the crypto migration from native-tls/OpenSSL to rustls/aws-lc-rs.

Motivation

First PR of the TLS migration stack. Earlier iterations rewrote the shared TLS types in place, which could never pass CI standalone because consumers (balancerd, mz-debug, pgwire) still used the openssl API. This version is additive: the rustls machinery lands next to the openssl paths, and each later PR flips exactly one consumer.

This PR deliberately contains no FIPS machinery. It standardizes rustls on the aws-lc-rs provider, and the FIPS build story lands separately once the TLS stack swap is complete.

Changes

  • ore: new crypto feature installs the aws-lc-rs rustls CryptoProvider process-wide via ctor, so rustls configurations built anywhere in the process pick it up without selecting a provider explicitly. rustls AsyncReady impls added alongside the openssl one.
  • tls-util: adds make_tls_rustls plus a rustls-backed MakeTlsConnect impl for tokio-postgres. Enables mz-ore/crypto, so linking tls-util guarantees the provider is installed. The openssl make_tls and PKCS#12 helpers are untouched.
  • pgwire-common: Conn::Ssl now holds a TlsStream enum (Openssl | rustls Server | rustls Client). The Openssl variant is a migration bridge and is deleted by the final cleanup PR. SNI extraction is unified behind TlsStream::server_name.
  • pgwire, balancerd: mechanical wraps in TlsStream::Openssl only. No behavior change.

Net diff: ~430 insertions, 17 deletions. No consumer changes TLS backend in this PR.

Stack

This PR → #36086 (server-core/pgwire) and #36087 (postgres-util/postgres-client/ccsr) in parallel → #36088 (balancerd), #36089 (environmentd), #36090 (test infra) → final cleanup PR removes the openssl paths and the TlsStream::Openssl variant.

Testing

Existing TLS behavior is unchanged (openssl paths untouched, verified by full workspace build + existing test suite in CI). The rustls machinery gains coverage as each consumer flips in its own PR.

Part of SEC-218. Related to SEC-192.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

@jasonhernandez
jasonhernandez force-pushed the jason/crypto-tls-1-foundation branch 2 times, most recently from 4f06772 to c319014 Compare July 9, 2026 20:58
@jasonhernandez jasonhernandez changed the title crypto: migrate TLS foundation to rustls (tls-util, ore, pgwire-common) crypto: add rustls TLS foundation alongside openssl Jul 9, 2026
Add the rustls/aws-lc-rs building blocks next to the existing openssl
paths so that downstream consumers can migrate one at a time:

- ore: add a `crypto` feature that installs the aws-lc-rs rustls
  CryptoProvider process-wide via ctor, and rustls AsyncReady impls
  alongside the openssl one.
- tls-util: add `make_tls_rustls` and a rustls-backed `MakeTlsConnect`
  implementation for tokio-postgres. The openssl `make_tls` and
  PKCS#12 helpers are unchanged.
- pgwire-common: `Conn::Ssl` now holds a `TlsStream` enum that carries
  either an openssl or a rustls stream. The `Openssl` variant is
  removed once no consumer constructs it anymore.
- pgwire, balancerd: mechanically wrap their openssl streams in
  `TlsStream::Openssl`. Balancerd's SNI extraction moves behind
  `TlsStream::server_name`.

No consumer changes TLS backend in this commit.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Fable 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_018ztDqVCrRgMdtjJxvPWa4B
@jasonhernandez
jasonhernandez force-pushed the jason/crypto-tls-1-foundation branch from c319014 to c7e4283 Compare July 9, 2026 21:48
@jasonhernandez
jasonhernandez marked this pull request as ready for review July 14, 2026 00:04
@jasonhernandez
jasonhernandez requested a review from a team as a code owner July 14, 2026 00:04
@jasonhernandez
jasonhernandez requested a review from antiguru July 14, 2026 00:04

@def- def- 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.

I'll post the QA LLM review verbatim, hope that's ok:

Finding 1 (MEDIUM): make_tls_rustls drops the system/default CA trust store, so verify-full/verify-ca without an explicit CA will fail every connection

File: src/tls-util/src/lib.rs (make_tls_rustls, root-store construction)

The openssl make_tls builds its connector with
SslConnector::builder(SslMethod::tls_client()), which calls
set_default_verify_paths() internally and therefore trusts the process/system
default CA bundle. In verify-full (or verify-ca) mode with no explicit
sslrootcert, a server presenting a publicly/system-trusted certificate
verifies successfully. This matches libpq: with no ~/.postgresql/root.crt,
libpq falls back to the system CA store.

make_tls_rustls instead starts from rustls::RootCertStore::empty() and only
ever add()s certificates from config.get_ssl_root_cert(). There is no
rustls-native-certs / webpki-roots fallback (confirmed: neither crate is a
workspace dependency). So when verify_mode == true and no CA was supplied, the
root store is empty and the default WebPkiServerVerifier rejects every
server certificate — the TLS handshake can never succeed.

This is reachable via a real, user-configurable path. For CREATE CONNECTION ... TO POSTGRES, src/sql/.../ddl/connection.rs:445-457 maps SSL MODE 'verify-full' / 'verify-ca' to SslMode::VerifyFull / VerifyCa and, unlike
the SQL Server path, does not require SSL CERTIFICATE AUTHORITY
(tls_root_cert may be None). That config flows through
storage-types/connections.rs → mz-postgres-util → make_tls today. When
#36087 swaps that call to make_tls_rustls, any such connection that relied on
system/public CA trust silently starts failing at handshake time (source
ingestion / CSR outage), with no test to catch it (testdrive PG fixtures always
pin an explicit CA, and CI has no publicly-trusted-cert server).

It is fail-closed (a broken connection, not a verification bypass), so this is an
availability/correctness regression rather than a security hole. But it defeats
the function's stated contract of being "the rustls equivalent of make_tls",
and — unlike the related hostname deviation — it is not tracked anywhere
(see the note below).

Suggested fix: seed the root store from the platform trust store before
adding the user CA, matching openssl's set_default_verify_paths(). e.g. add
rustls-native-certs and, when verify_mode, load
rustls_native_certs::load_native_certs() into root_store in addition to any
sslrootcert entries (or fall back to webpki-roots). Add a regression case for
verify-full/verify-ca with no explicit CA against a system-trusted endpoint.

Related, already-tracked: verify-ca collapses into verify-full

For verify-ca (and require+CA), openssl uses (PEER, verify_hostname=false)
— it validates the chain but skips the hostname. make_tls_rustls folds both
VerifyCa and VerifyFull into verify_mode = true and uses rustls's standard
verifier, which always checks the hostname. So verify-ca behaves like
verify-full (stricter than libpq), breaking connections whose cert name does
not match the connection host (e.g. connecting by IP or internal name).

This deviation is already known and tracked in Linear SEC-192 ("rustls's
standard verifier always checks the hostname, so sslmode=verify-ca currently
behaves like verify-full ... Either add a custom verifier that skips the server
name check or document the stricter behavior as intended"), so I am not
re-reporting it — noting it only because the same root-store construction is
where both the tracked hostname issue and the untracked system-CA issue in
Finding 1 live, and both should be fixed together when the verifier is reworked.


Finding 2 (LOW): rustls client stream reports no channel binding, dropping SCRAM-SHA-256-PLUS

File: src/tls-util/src/lib.rs — RustlsTlsStream::channel_binding returns
ChannelBinding::none().

The openssl path (postgres-openssl) supplies tls-server-end-point channel
binding, so tokio-postgres negotiates SCRAM-SHA-256-PLUS when the upstream
offers it. The rustls stream returns ChannelBinding::none(), so channel binding
is never available and the client always falls back to plain SCRAM-SHA-256.

Impact is limited: no consumer sets channel_binding=require (confirmed — the
only channel_binding references in-tree are the server-side pgwire SCRAM
decoder, unrelated), so tokio-postgres runs with the default prefer, where the
loss is a silent no-op rather than a failure, and an active MITM can already
strip the -PLUS offer under prefer. Still, in sslmode=require (no cert
verification — the recommended Materialize PG source mode, since prefer is
intentionally disallowed) channel binding was the only MITM defense the openssl
path offered against a network attacker, and this removes it. Not tracked in
SEC-192. Worth restoring tls-server-end-point (rustls exposes the peer cert via
TlsStream::get_ref().1.peer_certificates()) when the client flip lands, or at
least recording the gap.

jasonhernandez and others added 2 commits July 16, 2026 11:05
Address review findings on the rustls connector:

Seed the rustls root store with the platform's native certificates when
verifying, matching the openssl connector whose builder trusts the
system default CA paths. Without this, verify-full/verify-ca with no
explicit sslrootcert rejected every server certificate.

Implement RFC 5929 tls-server-end-point channel binding for the rustls
stream, so SCRAM-SHA-256-PLUS stays available after the migration off
postgres-openssl. Signature algorithms without a well-defined hash
(Ed25519, RSASSA-PSS) report no channel binding, matching openssl.
Includes parity tests against openssl's X509_digest.

Co-Authored-By: Claude Fable 5 <[email protected]>
Add hermetic end-to-end tests driving both TLS connectors over real
handshakes against an in-process tokio-rustls server: verify-full with
no explicit sslrootcert trusting a CA installed via SSL_CERT_FILE
(guarding the native root loading in make_tls_rustls), and client
certificate presentation against a server that requires one.

Add a Nightly-only test that completes a verify-full handshake with
s3.amazonaws.com through the rustls connector, validating the platform
trust store against a public CA end to end. The openssl connector is
excluded there: vendored openssl compiles its default trust store path
to /usr/local/ssl, which does not exist at runtime, so it cannot
validate public CAs without SSL_CERT_FILE. Wired into Nightly via a
--tls-external mode on the cargo-test composition.

Co-Authored-By: Claude Fable 5 <[email protected]>
@jasonhernandez
jasonhernandez requested a review from a team as a code owner July 16, 2026 22:13
@jasonhernandez

Copy link
Copy Markdown
Contributor Author

@def- I made some fixes and added some more test coverage (I would suggest we consider keeping this through the openssl -> rustls migration then possibly dropping / shrinking it). Unfortunately that makes this PR bigger. I could refactor the tests out into another PR but would appreciate some brief feedback on this general test approach.

Claude did point out that a live TLS test against s3.amazonaws.com would make our tests depend on AWS's availability ;).

antiguru added a commit that referenced this pull request Sep 30, 2026
mz-aws-util was the last user of hyper 0.14 in the workspace. It had
stayed there because the AWS SDK's hyper 1 client offers only rustls and
s2n-tls as TLS providers, and its hook for wrapping a custom TLS
connector is crate-private. Native TLS therefore meant either staying on
hyper 0.14 or writing our own SDK HTTP client, including the SDK's
timeout handling and retry error classification.

This PR takes the SDK's own hyper 1 client with rustls on the aws-lc-rs
provider. That matches the direction of the
[SEC-218](https://linear.app/materializeinc/issue/SEC-218) migration
from OpenSSL to rustls on aws-lc-rs (#36085), and aws-lc-rs is already
in the tree through jsonwebtoken and launchdarkly. The client trusts the
system's root certificates. `http_client_with_resolver` passes
`MzAwsResolver` to `build_with_resolver`, so connections to AWS
endpoints still reject non-global addresses when enforcement is on.

This drops hyper 0.14, hyper-tls 0.5, h2 0.3, http-body 0.4, socket2 0.5
and the direct aws-smithy-runtime dependency, along with their deny.toml
skips and the RUSTSEC-2026-0258 ignore for h2 0.3. The resolver tests
now go through `ResolveDns`. Locally, both clients listed a public S3
bucket over TLS.

Until the rest of the
[SEC-218](https://linear.app/materializeinc/issue/SEC-218) stack lands,
AWS traffic uses rustls while other TLS consumers stay on OpenSSL. FIPS
is out of scope here, as in #36085.

Posted by Claude Code.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <[email protected]>
@jasonhernandez

Copy link
Copy Markdown
Contributor Author

Closing in favor of a smaller stack: #39428 (OpenSSL handshake tests) → #39430 (rustls Postgres connector) → #39431 (channel binding) and #39432 (nightly public-CA check), plus #39433 (pgwire-common TlsStream bridge, based on main). The mz_ore::crypto provider module is dropped: binaries install aws-lc-rs explicitly and tls-util passes the provider directly.

jasonhernandez added a commit that referenced this pull request Oct 27, 2026
### Motivation

First PR in the TLS migration stack replacing #36085: #39428 (this,
OpenSSL handshake tests) → #39430 (rustls Postgres connector) → #39431
(channel binding) and #39432 (nightly public-CA check). #39433
(pgwire-common `TlsStream` bridge) is independent, based on main.

These tests pin down today's OpenSSL TLS client behavior in
`mz-tls-util` ahead of the rustls migration, which will extend each test
with a rustls leg. Split out of #36085.

### Description

- `verify_full_trusts_default_store`: with `sslmode=verify-full` and no
`sslrootcert`, `make_tls` trusts a CA from the default store
(`SSL_CERT_FILE`).
- `client_certs_accepted`: `make_tls` presents the configured client
certificate to a server that requires one.
- Harness: a throwaway `TestCa`, a blocking OpenSSL `SslAcceptor` server
on a background thread, and connect/assert helpers.
- `tempfile` and `tokio` added as dev-dependencies (both existing
workspace deps). No production code changes.

### Verification

Ran the tests repeatedly under nextest. Confirmed each fails when its
property is broken (no `SSL_CERT_FILE`, no client cert).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Jason Hernandez <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
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