From f24e01e68dfd9d51e15e63b9c9c75e6e715514e9 Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Wed, 19 Aug 2026 11:03:14 -0400 Subject: [PATCH] fix: ssl_mode=require validates cert against platform trust store build_tls_connector's own doc comment already said require should force TLS "without certificate validation", but needs_cert_validation only matched verify-ca/verify-full -- require fell through to the final with_platform_verifier() fallback, which does validate against the OS trust store, defeating the entire point of require vs. verify-full (the standard require use case is self-signed certs or private CAs the user hasn't configured ssl_ca for). Ported NoCertVerifier from the builtin driver's src-tauri/src/pool_manager.rs::NoCertVerifier -- accepts any certificate unconditionally, including bypassing TLS 1.2/1.3 signature verification, not just chain/hostname checks (more permissive than VerifyCaCertVerifier's chain-only bypass, but that's the builtin's own deliberate choice for this mode, not something to improve on silently). Routed require mode to it, threading client_auth through the same pattern already used by the verify-ca/verify-full branches so mTLS + require still work together. Proved the bug and the fix live against a real self-signed-cert SSL-enabled PostgreSQL instance (separate from the non-SSL fixture used for #43): require failed the TLS handshake before this fix, and now connects successfully. Confirmed no regression in verify-ca/ verify-full (still correctly validate -- existing VerifyCaCertVerifier unit tests unaffected) or require against a non-SSL server (still correctly fails, per #43/#45). Separately found while testing this fix that verify-ca without an explicit ssl_ca file silently falls back to platform-trust validation instead of erroring like the builtin does -- a distinct, smaller discrepancy, filed as #46 and left out of scope here. Fixes #44. --- CHANGELOG.md | 17 +++++++++ src/client.rs | 93 ++++++++++++++++++++++++++++++++++++++++++--- src/client_tests.rs | 44 ++++++++++++++++++++- 3 files changed, 148 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d9203e..d2c4afb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,23 @@ and wired it into `build_pool`. Proved the bug and the fix with a live test against a real non-SSL PostgreSQL instance: `ssl_mode=require` connected successfully before this fix, and now correctly fails. +- `ssl_mode=require` validated the server certificate against the platform + trust store instead of skipping validation entirely. `build_tls_connector`'s + own doc comment already said `require` should force TLS "without + certificate validation," but `needs_cert_validation` only matched + `verify-ca`/`verify-full` — `require` fell through to the final + `with_platform_verifier()` fallback, which does validate against the OS + trust store, defeating the entire point of `require` vs. `verify-full` + (the standard `require` use case is self-signed certs / private CAs the + user hasn't configured `ssl_ca` for). Added `NoCertVerifier`, ported from + the builtin driver's `src-tauri/src/pool_manager.rs::NoCertVerifier` + (accepts any certificate unconditionally — no chain, hostname, or even + TLS 1.2/1.3 signature verification), and routed `require` mode to it. + Proved the bug and the fix live against a real self-signed-cert + SSL-enabled PostgreSQL instance: `require` failed the TLS handshake + before this fix, and now connects successfully; confirmed no regression + in `verify-ca`/`verify-full` (still correctly validate) or `require` + against a non-SSL server (still correctly fails, per the previous entry). ## [1.0.0-beta.7] - 2026-08-17 diff --git a/src/client.rs b/src/client.rs index c5d3517..557f6d6 100644 --- a/src/client.rs +++ b/src/client.rs @@ -441,11 +441,12 @@ fn resolve_ssl_mode(ssl_mode: Option<&str>) -> Option { /// `verify-ca` deliberately skips hostname verification (that's the entire /// distinction from `verify-full` — matches libpq `sslmode=verify-ca` /// semantics, see `VerifyCaCertVerifier` below). `require` forces TLS -/// without certificate validation (matches the builtin driver's `require` -/// behavior — see `src-tauri/src/pool_manager.rs`). When `ssl_cert`/ -/// `ssl_key` are both supplied, presents them as a client certificate for -/// servers requiring mTLS (e.g. Google Cloud SQL) — matches the builtin -/// driver's `build_postgres_tls_connector` client-auth handling. +/// without any certificate validation at all (matches the builtin driver's +/// `require` behavior — see `src-tauri/src/pool_manager.rs::NoCertVerifier`). +/// When `ssl_cert`/`ssl_key` are both supplied, presents them as a client +/// certificate for servers requiring mTLS (e.g. Google Cloud SQL) — +/// matches the builtin driver's `build_postgres_tls_connector` client-auth +/// handling. fn build_tls_connector(params: &ConnectionParams) -> Result { use rustls_platform_verifier::BuilderVerifierExt; @@ -506,6 +507,19 @@ fn build_tls_connector(params: &ConnectionParams) -> Result builder + .with_client_auth_cert(certs, key) + .map_err(|e| format!("Failed to configure client certificate: {e}")), + None => Ok(builder.with_no_client_auth()), + }; + } + let builder = rustls::ClientConfig::builder() .with_platform_verifier() .map_err(|e| format!("Failed to build platform TLS verifier: {e}"))?; @@ -598,6 +612,75 @@ impl rustls::client::danger::ServerCertVerifier for VerifyCaCertVerifier { } } +/// Accepts any server certificate unconditionally — no chain validation, +/// no hostname check, not even TLS 1.2/1.3 signature verification. Used +/// for `require` mode, which forces TLS (encryption) without validating +/// who's on the other end — the standard use case is self-signed certs or +/// private CAs the user hasn't configured `ssl_ca` for. Matches the +/// builtin driver's `src-tauri/src/pool_manager.rs::NoCertVerifier` +/// exactly, including its signature-check bypass (more permissive than +/// `VerifyCaCertVerifier` above, which still validates the chain) — this +/// is the builtin's own deliberate choice for this mode, not something to +/// improve on silently. +#[derive(Debug)] +struct NoCertVerifier { + supported: rustls::crypto::WebPkiSupportedAlgorithms, +} + +impl NoCertVerifier { + fn new() -> Self { + let provider = match rustls::crypto::CryptoProvider::get_default() { + Some(provider) => provider.clone(), + None => { + let provider = rustls::crypto::ring::default_provider(); + let supported = provider.signature_verification_algorithms; + // Ignore the error from losing an install race — another + // caller's install still leaves a usable default installed. + let _ = provider.install_default(); + return Self { supported }; + } + }; + Self { + supported: provider.signature_verification_algorithms, + } + } +} + +impl rustls::client::danger::ServerCertVerifier for NoCertVerifier { + fn verify_server_cert( + &self, + _end_entity: &rustls::pki_types::CertificateDer<'_>, + _intermediates: &[rustls::pki_types::CertificateDer<'_>], + _server_name: &rustls::pki_types::ServerName<'_>, + _ocsp_response: &[u8], + _now: rustls::pki_types::UnixTime, + ) -> Result { + Ok(rustls::client::danger::ServerCertVerified::assertion()) + } + + fn verify_tls12_signature( + &self, + _message: &[u8], + _cert: &rustls::pki_types::CertificateDer<'_>, + _dss: &rustls::DigitallySignedStruct, + ) -> Result { + Ok(rustls::client::danger::HandshakeSignatureValid::assertion()) + } + + fn verify_tls13_signature( + &self, + _message: &[u8], + _cert: &rustls::pki_types::CertificateDer<'_>, + _dss: &rustls::DigitallySignedStruct, + ) -> Result { + Ok(rustls::client::danger::HandshakeSignatureValid::assertion()) + } + + fn supported_verify_schemes(&self) -> Vec { + self.supported.supported_schemes() + } +} + /// Load root certificates from a PEM file (used for `ssl_ca`-pinned /// `verify-ca`/`verify-full` connections). fn load_roots_from_pem(path: &str) -> Result { diff --git a/src/client_tests.rs b/src/client_tests.rs index 0955887..43c3164 100644 --- a/src/client_tests.rs +++ b/src/client_tests.rs @@ -5,7 +5,8 @@ use tokio::sync::Mutex; use super::{ build_tls_connector, cleanup_idle_pools, connection_key, get_or_create_pool, - load_client_cert_from_pem, load_roots_from_pem, resolve_ssl_mode, VerifyCaCertVerifier, POOLS, + load_client_cert_from_pem, load_roots_from_pem, resolve_ssl_mode, NoCertVerifier, + VerifyCaCertVerifier, POOLS, }; use crate::models::ConnectionParams; use deadpool_postgres::SslMode; @@ -628,3 +629,44 @@ fn resolve_ssl_mode_leaves_unset_or_unknown_values_unmapped() { assert_eq!(resolve_ssl_mode(None), None); assert_eq!(resolve_ssl_mode(Some("bogus")), None); } + +// Coverage for #44: build_tls_connector's `require` branch fell through to +// with_platform_verifier(), which DOES validate the server cert against the +// OS trust store — contradicting the function's own doc comment ("require +// forces TLS without certificate validation") and the builtin's actual +// behavior (NoCertVerifier: no validation at all for this mode). Unlike +// VerifyCaCertVerifier (which still skips hostname but validates the +// chain), NoCertVerifier accepts anything — not even a hostname check — +// matching the builtin's own NoCertVerifier exactly. + +#[test] +fn no_cert_verifier_accepts_a_cert_with_no_matching_hostname_or_chain() { + use rustls::client::danger::ServerCertVerifier; + use rustls::pki_types::{pem::PemObject, CertificateDer, ServerName, UnixTime}; + + let verifier = NoCertVerifier::new(); + + let end_entity: CertificateDer = + CertificateDer::pem_slice_iter(FIXTURE_SERVER_CERT_PEM.as_bytes()) + .next() + .unwrap() + .unwrap(); + // Deliberately mismatched hostname vs. the cert's CN/SAN + // (cert-hostname.example) — proves this verifier doesn't even do the + // hostname check VerifyCaCertVerifier skips deliberately; it does no + // checking of any kind. + let server_name = ServerName::try_from("totally-unrelated-hostname.internal").unwrap(); + + let result = verifier.verify_server_cert(&end_entity, &[], &server_name, &[], UnixTime::now()); + assert!( + result.is_ok(), + "require mode must accept any certificate, matching the builtin's NoCertVerifier" + ); +} + +#[test] +fn build_tls_connector_require_builds_successfully_with_no_ssl_ca() { + let params = params_with_ssl("require"); + build_tls_connector(¶ms) + .expect("require mode must build a connector without needing ssl_ca set"); +}