Repository navigation
tls-util: add a rustls connector for tokio-postgres - #39430
jasonhernandez wants to merge 3 commits into
Conversation
0f0a2dc to
95855dc
Compare
b94323f to
644a36c
Compare
### 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]>
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]>
95855dc to
0017751
Compare
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]>
antiguru
left a comment
There was a problem hiding this comment.
Looks good, I left some comments inline.
| _ => panic!("unexpected sslmode {:?}", config.get_ssl_mode()), | ||
| mode => bail_generic!(format!("unexpected sslmode {mode:?}")), |
| (Some(_), None) => { | ||
| bail_generic!("must provide both sslcert and sslkey, but only provided sslcert") | ||
| } | ||
| (None, None) => builder.with_no_client_auth(), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| Err(rustls::Error::InvalidCertificate( | ||
| CertificateError::NotValidForName | CertificateError::NotValidForNameContext { .. }, | ||
| )) => Ok(ServerCertVerified::assertion()), |
There was a problem hiding this comment.
I trust you on this, as I don't have the context to say whether this is right or not.
QA LLM Review1. HIGH -- verify-ca with a pinned CA also accepts any publicly trusted certificate
DetailsThe comment at :133-136 says the OpenSSL connector also trusts the default verify paths. That holds under The tests can't see the difference because cargo exports
libpq rejects this combination for the same reason: Fix: when 2. MEDIUM -- Postgres-style self-signed and CN-only server certificates are rejected
webpki rejects end-entity certificates with DetailsPostgreSQL's documented self-signed recipe is
The documented CA-signed recipe (
|
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]>
|
fixed both in dfe0bfe:
|
QA LLM Review1. MEDIUM -- The CN fallback skips name constraints, so a constrained CA can issue a verify-full certificate for any host
Detailswebpki applies I checked this at dfe0bfe with a chain of root, then an intermediate with
Fix: skip the CN fallback when the server's intermediates or any trust anchor carry a |
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_tlsdoes. Nothing calls it yet.Description
make_tls_rustls,MakeRustlsConnect,RustlsConnectandRustlsTlsStream, implementing tokio-postgres's TLS traits.make_tls: no verification forpreferandrequirewithoutsslrootcert, chain without hostname forverify-caandrequirewithsslrootcert, chain plus hostname forverify-full. Verifying modes trust native roots plussslrootcert. Client certs come fromsslcert/sslkey.rustls-native-certsandtokio-rustls, both already inCargo.lockat the declared versions. No version changes.Reading guide
make_tls_rustls: sslmode to verifier mapping, roots, client certs.NoHostnameVerifierandNoVerifier: the only security-sensitive part. Most of their lines just forward calls to rustls.MakeRustlsConnect,RustlsConnect,RustlsTlsStream: the plumbing tokio-postgres needs.ssl_modes_verify_the_same_as_opensslis 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