diff --git a/CHANGELOG.md b/CHANGELOG.md index d1929e3..fa78d2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,21 @@ staged, so this was a parity miss during extraction, not a later upstream change. Folded all four TLS params into `connection_key`, matching the builtin's TLS-param keying. +- `ssl_mode=verify-ca` incorrectly enforced hostname verification — + `build_tls_connector` wrapped `rustls::client::WebPkiServerVerifier` for + `verify-ca`, whose `verify_server_cert` unconditionally checks the + hostname with no way to opt out, making `verify-ca` behave identically to + `verify-full`. `verify-ca` is supposed to validate the certificate chain + but skip hostname verification — that's the entire distinction from + `verify-full` (matches libpq `sslmode=verify-ca` semantics). Added a + dedicated `VerifyCaCertVerifier`, ported from the builtin driver's + `src-tauri/src/pool_manager.rs`, using + `rustls::client::verify_server_cert_signed_by_trust_anchor` directly + instead. Proved the bug and the fix with a chain-valid cert whose + hostname deliberately doesn't match the connection target: rejected + before this fix, accepted after (while a CA-untrusted cert is still + correctly rejected, and `verify-full` still correctly rejects the + hostname mismatch). ## [1.0.0-beta.7] - 2026-08-17 diff --git a/src/client.rs b/src/client.rs index c6f73f7..d3af4c5 100644 --- a/src/client.rs +++ b/src/client.rs @@ -419,12 +419,15 @@ fn needs_tls(params: &ConnectionParams) -> bool { /// Build a rustls ClientConfig. `verify-ca`/`verify-full` validate the /// server's certificate chain — against a caller-supplied CA bundle -/// (`ssl_ca`) when present, or the platform trust store otherwise. `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. +/// (`ssl_ca`) when present, or the platform trust store otherwise. +/// `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. fn build_tls_connector(params: &ConnectionParams) -> Result { use rustls_platform_verifier::BuilderVerifierExt; @@ -457,6 +460,18 @@ 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 verifier = rustls::client::WebPkiServerVerifier::builder(std::sync::Arc::new(roots)) .build() @@ -484,6 +499,87 @@ fn build_tls_connector(params: &ConnectionParams) -> Result, + supported: rustls::crypto::WebPkiSupportedAlgorithms, +} + +impl VerifyCaCertVerifier { + fn new(roots: rustls::RootCertStore) -> Result { + 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 Ok(Self { + roots: std::sync::Arc::new(roots), + supported, + }); + } + }; + Ok(Self { + roots: std::sync::Arc::new(roots), + supported: provider.signature_verification_algorithms, + }) + } +} + +impl rustls::client::danger::ServerCertVerifier for VerifyCaCertVerifier { + 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 { + let cert = rustls::server::ParsedCertificate::try_from(end_entity)?; + rustls::client::verify_server_cert_signed_by_trust_anchor( + &cert, + &self.roots, + intermediates, + now, + self.supported.all, + )?; + Ok(rustls::client::danger::ServerCertVerified::assertion()) + } + + fn verify_tls12_signature( + &self, + message: &[u8], + cert: &rustls::pki_types::CertificateDer<'_>, + dss: &rustls::DigitallySignedStruct, + ) -> Result { + rustls::crypto::verify_tls12_signature(message, cert, dss, &self.supported) + } + + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &rustls::pki_types::CertificateDer<'_>, + dss: &rustls::DigitallySignedStruct, + ) -> Result { + rustls::crypto::verify_tls13_signature(message, cert, dss, &self.supported) + } + + 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 198fd7f..8b97beb 100644 --- a/src/client_tests.rs +++ b/src/client_tests.rs @@ -5,7 +5,7 @@ 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, POOLS, + load_client_cert_from_pem, load_roots_from_pem, VerifyCaCertVerifier, POOLS, }; use crate::models::ConnectionParams; @@ -307,6 +307,50 @@ yORfscWKlsDf+tv4Zb2jYQ== -----END PRIVATE KEY----- "; +// Self-signed CA + a server leaf cert it issued for CN/SAN +// `cert-hostname.example` (100-year validity, generated via `openssl req +// -x509`/`openssl x509 -req`) — used to prove `verify-ca` validates the +// chain but must NOT reject a hostname mismatch (that's what distinguishes +// it from `verify-full`; see issue #38). +const FIXTURE_CA_CERT_PEM: &str = "-----BEGIN CERTIFICATE----- +MIICyjCCAbKgAwIBAgIJAMUP8ld3x16kMA0GCSqGSIb3DQEBCwUAMBkxFzAVBgNV +BAMMDlRlc3QgUGFyaXR5IENBMCAXDTI2MDgxOTEzMDMxMVoYDzIxMjYwNzI2MTMw +MzExWjAZMRcwFQYDVQQDDA5UZXN0IFBhcml0eSBDQTCCASIwDQYJKoZIhvcNAQEB +BQADggEPADCCAQoCggEBANO7mxf1C4m0QXiWFez1OIwX0SLM35IRrpIEki4yIMlA +3c1SGzuXchNROu0QEz1A0WJosZHZNR1LRwxYpk6x9G7h/ojlzlpEYkyEE1y/i0VV +q5m+VxzARr54YcEny18tAFmKV73HeIRDrrM2k+L5GAObtCGJpsmkJQV87OMXrRWy +WsASq4hZS1/rFd+mwyUeVp5U7o8LkFMp+Kxc4UOUtXu5EAda7x26H3ARXtpYgUdX +l07k8SZ7jNR1immup/FVUpuCualtguEH5dLfmi4d+nAakIktGz61TSUQtSrPTFJH +c7c8UYG0ogqk1RZiNxMnSFjhDCgKblrPXq4EUloAiy8CAwEAAaMTMBEwDwYDVR0T +AQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAOB2iKUnYvx6BhczwsULe8ASU +wJeN2u6uj0EwxPpcAacDb7ccz0/muD5GyPFs2f7DkiYOg7aI6XVmWIIb0hN4gvwy ++480mQj4+y51ZqtPDykXkhyUerrU/ZRrP/AgpjtTw1tJ2I2xQWFLHtyb55zKguxW +r8sWnR/o6ZqaMQk6mcAQUkmOAUoTONPhrZD6LyKgxEC4/SN4TullHzPyaA0Grn1d +NdpSUD6Q5w8D7iXzeWS14pGG9JBKoa/f3/RBSXIqO3HsMttltNbUXpQE/uT+LDZe +yYrwHa9ZAVl3uV5ToMwfbGRK58IILIEDmmSkq97YoYAunqUcC/nAu9SmVQMteQ== +-----END CERTIFICATE----- +"; + +const FIXTURE_SERVER_CERT_PEM: &str = "-----BEGIN CERTIFICATE----- +MIIC4jCCAcqgAwIBAgIJAPgkrGMQ7ApvMA0GCSqGSIb3DQEBCwUAMBkxFzAVBgNV +BAMMDlRlc3QgUGFyaXR5IENBMCAXDTI2MDgxOTEzMDMxMVoYDzIxMjYwNzI2MTMw +MzExWjAgMR4wHAYDVQQDDBVjZXJ0LWhvc3RuYW1lLmV4YW1wbGUwggEiMA0GCSqG +SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDClp6Zk39KW/t8Pg2EzKn+OEOxL7WlxMCS +T1Je/FWZLHxV2EoyBLBQDvLsbREJnqxoAZ2bYd2Lq8YJWEP/SqnjWFpyPrLxIIb4 +HJzI7sqENF6fZpcLLo7hCnVHzWkWOPB1p3QOFYrot1Pnf51Qdw7QkqOp4J2GR3Zf +TbwCSZt2Kkn30IisFzp6Gl/FHPqKEnm6PK9RYtXOCtDjY4tECrSIpQIhkFE/CytI +sq71boNUBL5bpCIQkqVIvMT1pd5b2VRe1p15mOE3QsBBMu9SFk5FSljS/n3NrIQK +AimBqRY5Hx+Se/FpGFADVjzEF4YaalLRHGjVjzaoNVhgcCsWsjwVAgMBAAGjJDAi +MCAGA1UdEQQZMBeCFWNlcnQtaG9zdG5hbWUuZXhhbXBsZTANBgkqhkiG9w0BAQsF +AAOCAQEAKJseOSTJEdEq+pGXPNHsXrizteuF7WlFYBr3Cup/MO4b+nMkJarzmbqp +MFZk3l/3nssmCSlJ4lXDToWadkTkvFF6+IbSENS25OE5v01BNDYpx3P0eKrJ45G2 +/FWtAQr/yqVuCdPJSlaeTnn9RsPwZuH/RjWcH7tASPtAy1QaBclTZlX4jz2LzvoJ +QSukREcPVQohSKwAM4pU+YNa7ktLFa0APCQcQ8pks9rY98vxxib0FQNr5kh3ZcCT +GEWslhtuCliQjxnE4A1E8PxtF6I6huONjlJ0Z/uQrZc1B5uJf0dsPlAb0Y3Op5R+ +NV3jgQrSEPlHBW/Z+aJ152TvH0IG4w== +-----END CERTIFICATE----- +"; + fn params_with_ssl(ssl_mode: &str) -> ConnectionParams { ConnectionParams { driver: Some("postgres-plugin".to_string()), @@ -444,3 +488,107 @@ fn build_tls_connector_errors_when_ssl_key_is_set_without_ssl_cert() { "unexpected error message: {err}" ); } + +/// Constructs a `VerifyCaCertVerifier` from `FIXTURE_CA_CERT_PEM` and runs +/// it against a leaf cert issued by that CA (`FIXTURE_SERVER_CERT_PEM`), +/// checked against a hostname that deliberately does not match the leaf's +/// CN/SAN (`cert-hostname.example`). Returns the verifier's +/// `verify_server_cert` result so tests can assert accept-vs-reject +/// without a live TLS server. +fn probe_verify_ca_verifier_against_mismatched_hostname() -> Result<(), rustls::Error> { + use rustls::client::danger::ServerCertVerifier; + use rustls::pki_types::{pem::PemObject, CertificateDer, ServerName, UnixTime}; + + let mut roots = rustls::RootCertStore::empty(); + for cert in CertificateDer::pem_slice_iter(FIXTURE_CA_CERT_PEM.as_bytes()) { + roots.add(cert.unwrap()).unwrap(); + } + let verifier = VerifyCaCertVerifier::new(roots).expect("verifier should build from valid CA"); + + let end_entity: CertificateDer = + CertificateDer::pem_slice_iter(FIXTURE_SERVER_CERT_PEM.as_bytes()) + .next() + .unwrap() + .unwrap(); + let server_name = ServerName::try_from("totally-unrelated-hostname.internal").unwrap(); + + verifier + .verify_server_cert(&end_entity, &[], &server_name, &[], UnixTime::now()) + .map(|_| ()) +} + +#[test] +fn verify_ca_cert_verifier_accepts_a_chain_valid_cert_with_mismatched_hostname() { + probe_verify_ca_verifier_against_mismatched_hostname().expect( + "verify-ca must validate the chain but skip hostname verification — \ + that's the entire distinction from verify-full (matches libpq semantics)", + ); +} + +#[test] +fn verify_ca_cert_verifier_still_rejects_a_cert_from_an_untrusted_ca() { + use rustls::client::danger::ServerCertVerifier; + use rustls::pki_types::{pem::PemObject, CertificateDer, ServerName, UnixTime}; + + // Empty root store: the fixture server cert's issuer is not in it, so + // chain validation itself (not hostname) must reject this. + let untrusted_ca_pem = FIXTURE_CERT_PEM; // unrelated CA from load_roots_from_pem's fixtures above + let mut roots = rustls::RootCertStore::empty(); + for cert in CertificateDer::pem_slice_iter(untrusted_ca_pem.as_bytes()) { + roots.add(cert.unwrap()).unwrap(); + } + let verifier = VerifyCaCertVerifier::new(roots).expect("verifier should build"); + + let end_entity: CertificateDer = + CertificateDer::pem_slice_iter(FIXTURE_SERVER_CERT_PEM.as_bytes()) + .next() + .unwrap() + .unwrap(); + let server_name = ServerName::try_from("cert-hostname.example").unwrap(); + + let result = verifier.verify_server_cert(&end_entity, &[], &server_name, &[], UnixTime::now()); + assert!( + result.is_err(), + "a cert signed by a CA not in the root store must still be rejected" + ); +} + +#[test] +fn build_tls_connector_verify_ca_uses_the_hostname_skipping_verifier() { + let ca_path = write_temp_file(FIXTURE_CA_CERT_PEM); + let mut params = params_with_ssl("verify-ca"); + params.ssl_ca = Some(ca_path.to_str().unwrap().to_string()); + + let result = build_tls_connector(¶ms); + std::fs::remove_file(&ca_path).ok(); + + result.expect("verify-ca with a valid ssl_ca file should build a connector successfully"); +} + +// Regression coverage for a merge-time bug (#38's VerifyCaCertVerifier +// branch initially dropped #35's client_auth entirely, always calling +// .with_no_client_auth() regardless of ssl_cert/ssl_key): verify-ca must +// actually attach the client cert, not silently ignore it. Checking +// `has_certs()` distinguishes "attached" from merely "built successfully" +// — the buggy version also built successfully, just with no client cert. +#[test] +fn build_tls_connector_verify_ca_attaches_a_configured_client_cert() { + let ca_path = write_temp_file(FIXTURE_CA_CERT_PEM); + let cert_path = write_temp_file(FIXTURE_CLIENT_CERT_PEM); + let key_path = write_temp_file(FIXTURE_CLIENT_KEY_PEM); + + let mut params = params_with_ssl("verify-ca"); + params.ssl_ca = Some(ca_path.to_str().unwrap().to_string()); + params.ssl_cert = Some(cert_path.to_str().unwrap().to_string()); + params.ssl_key = Some(key_path.to_str().unwrap().to_string()); + + let config = build_tls_connector(¶ms).expect("connector should build"); + std::fs::remove_file(&ca_path).ok(); + std::fs::remove_file(&cert_path).ok(); + std::fs::remove_file(&key_path).ok(); + + assert!( + config.client_auth_cert_resolver.has_certs(), + "verify-ca must attach the configured client cert, not silently drop it" + ); +}