Skip to content

tls-util: add a rustls connector for tokio-postgres - #39430

Open
jasonhernandez wants to merge 3 commits into
mainfrom
jason/tls-stack-2-rustls-connector
Open

jasonhernandez wants to merge 3 commits into
mainfrom
jason/tls-stack-2-rustls-connector

Conversation

@jasonhernandez

@jasonhernandez jasonhernandez commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Part 2 of the TLS migration stack replacing #36085. #39431 and #39432 build on this.

Moving tokio-postgres connections off OpenSSL needs a rustls connector that verifies servers exactly as make_tls does. Nothing calls it yet.

Description

  • Add make_tls_rustls, MakeRustlsConnect, RustlsConnect and RustlsTlsStream, implementing tokio-postgres's TLS traits.
  • Per sslmode it matches make_tls: no verification for prefer and require without sslrootcert, chain without hostname for verify-ca and require with sslrootcert, chain plus hostname for verify-full. Verifying modes trust native roots plus sslrootcert. Client certs come from sslcert/sslkey.
  • Builds configs with an explicit aws-lc-rs provider, so it never depends on the process default.
  • Channel binding reports none. A follow-up PR adds it.
  • New workspace deps: rustls-native-certs and tokio-rustls, both already in Cargo.lock at the declared versions. No version changes.

Reading guide

  1. make_tls_rustls: sslmode to verifier mapping, roots, client certs.
  2. NoHostnameVerifier and NoVerifier: the only security-sensitive part. Most of their lines just forward calls to rustls.
  3. MakeRustlsConnect, RustlsConnect, RustlsTlsStream: the plumbing tokio-postgres needs.
  4. Tests: ssl_modes_verify_the_same_as_openssl is the one to read.

Verification

Both existing handshake tests now also run through rustls. A new test, ssl_modes_verify_the_same_as_openssl, checks that both connectors accept and reject the same handshakes across sslmodes, trust anchors and hostnames.

🤖 Generated with Claude Code

@jasonhernandez
jasonhernandez force-pushed the jason/tls-stack-2-rustls-connector branch from 0f0a2dc to 95855dc Compare October 27, 2026 07:09
@jasonhernandez
jasonhernandez force-pushed the jason/tls-stack-1-openssl-tests branch from b94323f to 644a36c Compare October 27, 2026 07:09
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]>
Base automatically changed from jason/tls-stack-1-openssl-tests to main October 27, 2026 08:16
Add `make_tls_rustls`, a rustls counterpart to `make_tls` that verifies
the server the same way for every sslmode: no verification for prefer
and for require without sslrootcert, chain but not hostname validation
for verify-ca and for require with sslrootcert, and chain plus hostname
validation for verify-full. Verifying modes trust the platform's native
roots plus sslrootcert, matching openssl's default verify paths.

The connector always uses the aws-lc-rs provider explicitly, so it does
not depend on a process-default provider. Nothing calls it yet.

The handshake tests now run each case against both connectors, and a
new test checks that both accept and reject the same handshakes across
sslmodes, trust anchors and hostnames.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@jasonhernandez
jasonhernandez force-pushed the jason/tls-stack-2-rustls-connector branch from 95855dc to 0017751 Compare October 28, 2026 00:32
Also keep the test TLS server alive across failed connections, so a
broken connection surfaces as a client-side assertion.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@jasonhernandez
jasonhernandez marked this pull request as ready for review October 28, 2026 05:46

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, I left some comments inline.

Comment thread src/tls-util/src/lib.rs
Comment on lines -59 to +74
_ => panic!("unexpected sslmode {:?}", config.get_ssl_mode()),
mode => bail_generic!(format!("unexpected sslmode {mode:?}")),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Comment thread src/tls-util/src/lib.rs
(Some(_), None) => {
bail_generic!("must provide both sslcert and sslkey, but only provided sslcert")
}
(None, None) => builder.with_no_client_auth(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to check (None, None) against the SSL mode? Unless they're synced, we could enable SSL mode, not provide certs, and fall back to no TLS?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no, sslcert/sslkey only control client auth. sslmode alone decides whether TLS is negotiated and how the server is verified, so leaving out client certs never falls back to plaintext.

Comment thread src/tls-util/src/lib.rs Outdated
Comment on lines +307 to +309
Err(rustls::Error::InvalidCertificate(
CertificateError::NotValidForName | CertificateError::NotValidForNameContext { .. },
)) => Ok(ServerCertVerified::assertion()),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I trust you on this, as I don't have the context to say whether this is right or not.

@def-

def- commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. HIGH -- verify-ca with a pinned CA also accepts any publicly trusted certificate

src/tls-util/src/lib.rs:137

make_tls_rustls always adds the image's public root bundle to the trust store, including when sslrootcert is set. In verify-ca and in require with sslrootcert, hostnames are not checked, so any server with any publicly issued certificate (for example a free certificate for an attacker's own domain) passes, while the shipped make_tls accepts only the pinned CA. Once a Postgres caller switches to this connector, an on-path attacker can impersonate an upstream that the user pinned to a private CA.

Details

The comment at :133-136 says the OpenSSL connector also trusts the default verify paths. That holds under cargo, but not in the shipped binaries. OpenSSL is vendored (Cargo.toml:445), so its default paths point at /usr/local/ssl, which does not exist in distroless-prod-base. Nothing in the tree sets SSL_CERT_FILE for environmentd or clusterd. test/aws-glue-schema-registry/mzcompose.py:327 and #39432 both record that vendored OpenSSL misses the image's ca-certificates. rustls_native_certs::load_native_certs() goes through openssl_probe::probe() instead, which finds /etc/ssl/certs/ca-certificates.crt (copied into that image).

The tests can't see the difference because cargo exports SSL_CERT_FILE=/usr/lib/ssl/cert.pem and SSL_CERT_DIR=/usr/lib/ssl/certs to every child process, so OpenSSL loads the system bundle under cargo test/nextest. I cleared both variables, set sslrootcert to a fresh private CA, and handshook with s3.amazonaws.com:443 as host my-private-db.internal:

  • require: openssl Err("unable to get local issuer certificate"), rustls Ok(())
  • verify-ca: openssl Err(...), rustls Ok(())

libpq rejects this combination for the same reason: sslrootcert=system with any mode weaker than verify-full is an error.

Fix: when sslrootcert is set, build the root store from it alone. Load native roots only when no sslrootcert is given, and only for verify-full, so a chain check without a hostname check never runs against public roots. Add a test that clears SSL_CERT_FILE/SSL_CERT_DIR before comparing the connectors, or the equivalence test keeps running against the cargo environment.

2. MEDIUM -- Postgres-style self-signed and CN-only server certificates are rejected

src/tls-util/src/lib.rs:150

webpki rejects end-entity certificates with basicConstraints CA:TRUE and ignores the subject CN, and OpenSSL accepts both. The certificate from PostgreSQL's documented self-signed recipe therefore fails every verifying mode, and CN-only server certificates fail verify-full. Sources that connect today would stop connecting once callers move to make_tls_rustls.

Details

PostgreSQL's documented self-signed recipe is openssl req -new -x509 -subj "/CN=dbhost...", used as both server cert and sslrootcert. With the default openssl.cnf it produces a v3 certificate with CA:TRUE and no SAN (checked with OpenSSL 3.5). webpki returns CaUsedAsEndEntity for it (rustls-webpki verify_cert.rs:449). NoHostnameVerifier only maps name errors, so the handshake fails. Probe against an OpenSSL test server using such a certificate as sslrootcert, host localhost:

  • require, verify-ca, verify-full: openssl Ok(()), rustls Err(... CaUsedAsEndEntity)

The documented CA-signed recipe (openssl x509 -req with no extfile) gives a server certificate with only a CN. OpenSSL's X509_check_host falls back to the CN when no DNS SAN is present, and webpki checks SANs only:

  • CN-only leaf, verify-ca: both Ok(())
  • CN-only leaf, verify-full: openssl Ok(()), rustls Err("certificate not valid for name \"localhost\" ... subjectAltName")

ssl_modes_verify_the_same_as_openssl only uses CA-issued leaves with a DNS SAN, so it passes despite these differences. Either handle these certificate shapes, for example by trusting an exact sslrootcert match for a self-signed leaf, or document the stricter behavior and surface it before callers switch.

With an sslrootcert, the rustls connector now trusts only that file. Platform
roots are used only for verify-full without an sslrootcert, so a publicly
issued certificate can no longer pass verify-ca or require against a pinned
CA. verify-ca without an sslrootcert is an error.

A server presenting an exact sslrootcert entry is trusted directly, which
accepts PostgreSQL's self-signed CA:TRUE recipe, and verify-full falls back to
the subject CN when a certificate has no DNS subjectAltName, as OpenSSL does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@jasonhernandez

Copy link
Copy Markdown
Contributor Author

fixed both in dfe0bfe:

  1. with an sslrootcert, only that file is trusted. platform roots are used only for verify-full without an sslrootcert, and verify-ca without one is an error. sslrootcert_excludes_default_store covers it.
  2. a server presenting an exact sslrootcert entry is trusted directly (validity still checked), and verify-full falls back to the CN when there's no DNS SAN. postgres_recipe_certs_verify_the_same_as_openssl checks both shapes against openssl. wildcard CNs aren't matched, which is stricter than openssl.

@def-

def- commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- The CN fallback skips name constraints, so a constrained CA can issue a verify-full certificate for any host

src/tls-util/src/lib.rs:349

common_name_matches accepts a leaf whose CN equals the host after webpki has validated the chain, but webpki never checks the subject CN against DNS name constraints. A CA constrained to one domain can therefore issue a CN-only leaf for any hostname, and make_tls_rustls accepts it under verify-full. OpenSSL rejects that certificate, so the fallback undoes the protection that name-constrained CAs depend on.

Details

webpki applies nameConstraints only to SAN entries and the subject as a directoryName (rustls-webpki 0.103 subject_name/mod.rs, check_name_constraints). Its comment there says it does not support CN-IDs. OpenSSL checks the CN of a leaf with no DNS SAN against DNS constraints (NAME_CONSTRAINTS_check_CN in x509_vfy.c, "If EE certificate check commonName too"). This affects both roots the connector can trust: an enterprise sslrootcert with a constrained issuing CA, and the platform bundle used by verify-full with no sslrootcert, which can include technically constrained subordinate CAs held outside a CA.

I checked this at dfe0bfe with a chain of root, then an intermediate with nameConstraints=critical,permitted;DNS:allowed.example, then a leaf with CN=victim.example and no SAN. The test used verify-full and host victim.example:

  • sslrootcert = root: openssl Err("permitted subtree violation"), rustls Ok
  • no sslrootcert, SSL_CERT_FILE = root: openssl Err("permitted subtree violation"), rustls Ok
  • control, leaf CN=db.allowed.example: both Ok
  • control, leaf with SAN DNS:victim.example: both Err (rustls NameConstraintViolation)

Fix: skip the CN fallback when the server's intermediates or any trust anchor carry a nameConstraints extension. Webpki does not expose which path it chose, and treating any constraint as disqualifying is safe. Alternatively, check the CN against those DNS subtrees yourself. The pinned branch is not affected, because no issuer sits above a pinned certificate. A case like the first one above, added to postgres_recipe_certs_verify_the_same_as_openssl, would cover this.

This branch has not been deployed

No deployments
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.

3 participants