diff --git a/.schema/pgdog.schema.json b/.schema/pgdog.schema.json index 6b048264e..eb9bf4095 100644 --- a/.schema/pgdog.schema.json +++ b/.schema/pgdog.schema.json @@ -109,7 +109,6 @@ "tls_certificate": null, "tls_client_ca_certificate": null, "tls_client_required": false, - "tls_client_validate_cn": false, "tls_private_key": null, "tls_server_ca_certificate": null, "tls_verify": "prefer", @@ -1126,11 +1125,6 @@ "type": "boolean", "default": false }, - "tls_client_validate_cn": { - "description": "Validate that the CN of the certificate matches the user's name.\nThis is part of our mTLS implementation. The certificate authority can\nissue certificates that PgDog will validate for both authenticity and authentication.", - "type": "boolean", - "default": false - }, "tls_private_key": { "description": "Path to the TLS private key PgDog will use to setup TLS connections with clients.\n\nhttps://docs.pgdog.dev/configuration/pgdog.toml/general/#tls_private_key", "type": [ diff --git a/integration/tls/pgdog.toml b/integration/tls/pgdog.toml index bf03d5d03..d9fe942d1 100644 --- a/integration/tls/pgdog.toml +++ b/integration/tls/pgdog.toml @@ -2,7 +2,6 @@ tls_certificate = "integration/tls/server.crt" tls_private_key = "integration/tls/server.key" tls_client_ca_certificate = "integration/tls/ca.crt" -tls_client_validate_cn = true [[databases]] name = "pgdog" diff --git a/pgdog-config/src/general.rs b/pgdog-config/src/general.rs index 420e20743..26aef743f 100644 --- a/pgdog-config/src/general.rs +++ b/pgdog-config/src/general.rs @@ -242,12 +242,6 @@ pub struct General { /// https://docs.pgdog.dev/configuration/pgdog.toml/general/#tls_client_ca_certificate pub tls_client_ca_certificate: Option, - /// Validate that the CN of the certificate matches the user's name. - /// This is part of our mTLS implementation. The certificate authority can - /// issue certificates that PgDog will validate for both authenticity and authentication. - #[serde(default)] - pub tls_client_validate_cn: bool, - /// How long to wait for active clients to finish transactions when shutting down. /// /// _Default:_ `60000` @@ -801,7 +795,6 @@ impl Default for General { tls_verify: Self::default_tls_verify(), tls_server_ca_certificate: Self::tls_server_ca_certificate(), tls_client_ca_certificate: Self::tls_client_ca_certificate(), - tls_client_validate_cn: bool::default(), shutdown_timeout: Self::default_shutdown_timeout(), shutdown_termination_timeout: Self::default_shutdown_termination_timeout(), broadcast_address: Self::broadcast_address(), diff --git a/pgdog/src/backend/databases.rs b/pgdog/src/backend/databases.rs index 0fb47b498..764c609ae 100644 --- a/pgdog/src/backend/databases.rs +++ b/pgdog/src/backend/databases.rs @@ -458,7 +458,7 @@ impl Databases { // Launch all clusters for cluster in self.all().values() { - if cluster.passwords().is_empty() && !cluster.mutual_tls() { + if cluster.passwords().is_empty() && cluster.identity().is_none() { warn!( r#"disabling pool for user "{}" and database "{}", password not set"#, cluster.user(), diff --git a/pgdog/src/backend/pool/cluster.rs b/pgdog/src/backend/pool/cluster.rs index 5a35c0951..d8421212a 100644 --- a/pgdog/src/backend/pool/cluster.rs +++ b/pgdog/src/backend/pool/cluster.rs @@ -86,7 +86,6 @@ pub struct Cluster { resharding_replication_retry_max_attempts: usize, resharding_replication_retry_min_delay: Duration, regex_parser: RegexParser, - mutual_tls: bool, identity: Option, } @@ -174,7 +173,6 @@ pub struct ClusterConfig<'a> { pub resharding_replication_retry_min_delay: u64, pub regex_parser_limit: usize, pub pub_sub_enabled: bool, - pub mutual_tls: bool, pub identity: &'a Option, } @@ -238,7 +236,6 @@ impl<'a> ClusterConfig<'a> { resharding_replication_retry_min_delay: general.resharding_replication_retry_min_delay, regex_parser_limit: general.regex_parser_limit, pub_sub_enabled: general.pub_sub_enabled(), - mutual_tls: config.general.tls_client_validate_cn, identity: &user.identity, } } @@ -285,7 +282,6 @@ impl Cluster { resharding_replication_retry_min_delay, regex_parser_limit, pub_sub_enabled, - mutual_tls, identity, } = config; @@ -346,7 +342,6 @@ impl Cluster { resharding_replication_retry_min_delay, ), regex_parser: RegexParser::new(regex_parser_limit, query_parser), - mutual_tls, identity: identity.clone(), } } @@ -540,10 +535,6 @@ impl Cluster { &self.multi_tenant } - pub fn mutual_tls(&self) -> bool { - self.mutual_tls - } - /// Get replication configuration for this cluster. pub fn replication_sharding_config(&self) -> Option { self.replication_sharding diff --git a/pgdog/src/frontend/client/mod.rs b/pgdog/src/frontend/client/mod.rs index dc7064893..c9cf58ec1 100644 --- a/pgdog/src/frontend/client/mod.rs +++ b/pgdog/src/frontend/client/mod.rs @@ -134,6 +134,61 @@ impl Client { } } + /// Authenticate a client against the configured password(s) using the + /// requested authentication method. + /// + /// Returns `false` if no passwords are configured or the credentials the + /// client provided don't match. + async fn check_password( + stream: &mut Stream, + user: &str, + auth_type: &AuthType, + passwords: &[PasswordKind], + ) -> Result { + if passwords.is_empty() { + return Ok(false); + } + + let ok = match auth_type { + AuthType::Md5 => { + let md5 = md5::Client::new( + user, + &passwords.iter().map(|s| s.to_string()).collect::>(), + ); + stream.send_flush(&md5.challenge()).await?; + let password = Password::from_bytes(stream.read().await?.to_bytes()?)?; + if let Password::PasswordMessage { response } = password { + md5.check(&response) + } else { + false + } + } + + AuthType::Scram => { + stream.send_flush(&Authentication::scram()).await?; + + let scram = Server::new(passwords); + let res = scram.handle(stream).await; + matches!(res, Ok(true)) + } + + AuthType::Plain => { + stream + .send_flush(&Authentication::ClearTextPassword) + .await?; + let response = stream.read().await?; + let response = Password::from_bytes(response.to_bytes()?)?; + passwords + .iter() + .any(|p| Some(p.as_str()) == response.password()) + } + + AuthType::Trust => true, + }; + + Ok(ok) + } + /// Create new frontend client from the given TCP stream. async fn login( mut stream: Stream, @@ -153,7 +208,6 @@ impl Client { let admin_password = &config.config.admin.password; let auth_type = &config.config.general.auth_type; let passthrough = config.config.general.passthrough_auth(); - let validate_cn = config.config.general.tls_client_validate_cn; let id = BackendKeyData::new_client(protocol_version); let comms = ClientComms::new(&id); @@ -180,62 +234,26 @@ impl Client { } else { false } - } else if validate_cn { - // This checks that the certificate CN (common name) - // matches the user identity exactly. If the client is not connecting with TLS, - // this will fail. - // - // This is part of our mTLS implementation. - // - stream.tls_cn() == databases::databases().identity((user, database)) + } else if admin { + // The admin database is virtual and never present in the cluster + // map, so authenticate directly against the configured admin password. + let passwords = [PasswordKind::Plain(admin_password.clone())]; + Self::check_password(&mut stream, user, auth_type, &passwords).await? } else { - let passwords = if admin { - Some(vec![PasswordKind::Plain(admin_password.clone())]) - } else { - databases::databases() - .passwords((user, database)) - .map(|p| p.to_vec()) - }; - - if let Some(passwords) = passwords { - match auth_type { - AuthType::Md5 => { - let md5 = md5::Client::new( - user, - &passwords.iter().map(|s| s.to_string()).collect::>(), - ); - stream.send_flush(&md5.challenge()).await?; - let password = Password::from_bytes(stream.read().await?.to_bytes()?)?; - if let Password::PasswordMessage { response } = password { - md5.check(&response) - } else { - false - } - } - - AuthType::Scram => { - stream.send_flush(&Authentication::scram()).await?; - - let scram = Server::new(&passwords); - let res = scram.handle(&mut stream).await; - matches!(res, Ok(true)) + match databases::databases().cluster((user, database)) { + Ok(cluster) => { + if let Some(identity) = cluster.identity() { + // mTLS authentication: the client certificate identity + // must match the configured user identity. + stream.tls_identity() == Some(identity) + } else { + // Password authentication. + Self::check_password(&mut stream, user, auth_type, cluster.passwords()) + .await? } - - AuthType::Plain => { - stream - .send_flush(&Authentication::ClearTextPassword) - .await?; - let response = stream.read().await?; - let response = Password::from_bytes(response.to_bytes()?)?; - passwords - .iter() - .any(|p| Some(p.as_str()) == response.password()) - } - - AuthType::Trust => true, } - } else { - false + + Err(_) => false, } }; diff --git a/pgdog/src/frontend/listener.rs b/pgdog/src/frontend/listener.rs index 65dc6bc4c..dbc314b21 100644 --- a/pgdog/src/frontend/listener.rs +++ b/pgdog/src/frontend/listener.rs @@ -8,7 +8,7 @@ use crate::backend::databases::{databases, reload, shutdown}; use crate::config::config; use crate::frontend::client::query_engine::two_pc::Manager; use crate::net::messages::{hello::SslReply, NegotiateProtocolVersion, Startup}; -use crate::net::tls::{acceptor, peer_cn}; +use crate::net::tls::{acceptor, peer_identity}; use crate::net::{self, tweak, Stream}; use crate::sighup::Sighup; use tokio::net::{TcpListener, TcpStream}; @@ -197,11 +197,11 @@ impl Listener { return Ok(()); } }; - let tls_cn = peer_cn(cipher.get_ref().1); + let tls_identity = peer_identity(cipher.get_ref().1); stream = Stream::tls( tokio_rustls::TlsStream::Server(cipher), config.config.memory.net_buffer, - tls_cn, + tls_identity, ); } else { stream.send_flush(&SslReply::No).await?; diff --git a/pgdog/src/net/stream.rs b/pgdog/src/net/stream.rs index 467e6ac7d..bd8644b07 100644 --- a/pgdog/src/net/stream.rs +++ b/pgdog/src/net/stream.rs @@ -32,7 +32,7 @@ pub struct Stream { inner: StreamInner, io_in_progress: bool, capacity: usize, - tls_cn: Option, + tls_identity: Option, } impl AsyncRead for Stream { @@ -101,7 +101,7 @@ impl Stream { inner: StreamInner::Plain(BufStream::with_capacity(capacity, capacity, stream)), io_in_progress: false, capacity, - tls_cn: None, + tls_identity: None, } } @@ -109,13 +109,13 @@ impl Stream { pub fn tls( stream: tokio_rustls::TlsStream, capacity: usize, - tls_cn: Option, + tls_identity: Option, ) -> Self { Self { inner: StreamInner::Tls(BufStream::with_capacity(capacity, capacity, stream)), io_in_progress: false, capacity, - tls_cn, + tls_identity, } } @@ -125,13 +125,14 @@ impl Stream { inner: StreamInner::DevNull, io_in_progress: false, capacity: 0, - tls_cn: None, + tls_identity: None, } } - /// Get the Common Name (CN) from the client's TLS certificate, if any. - pub fn tls_cn(&self) -> Option<&str> { - self.tls_cn.as_deref() + /// Get the hostname identity (SAN dNSName, falling back to Subject CN) + /// from the client's TLS certificate, if any. + pub fn tls_identity(&self) -> Option<&str> { + self.tls_identity.as_deref() } /// This is a TLS stream. diff --git a/pgdog/src/net/tls.rs b/pgdog/src/net/tls.rs index f1c91375b..aef47954a 100644 --- a/pgdog/src/net/tls.rs +++ b/pgdog/src/net/tls.rs @@ -77,16 +77,33 @@ pub fn acceptor() -> Option> { ACCEPTOR.load_full() } -/// Extract the Common Name (CN) from the peer's TLS certificate, if present. -pub fn peer_cn(conn: &ServerConnection) -> Option { - cn_from_certs(conn.peer_certificates()?) +/// Extract the hostname identity from the peer's TLS certificate, if present. +pub fn peer_identity(conn: &ServerConnection) -> Option { + identity_from_certs(conn.peer_certificates()?) } -/// Extract the Common Name (CN) from the first certificate in the chain. -pub(crate) fn cn_from_certs(certs: &[CertificateDer<'_>]) -> Option { +/// Extract a hostname identity from the first certificate in the chain. +/// +/// Prefers the first `dNSName` in the Subject Alternative Name extension and +/// falls back to the Subject CN. RFC 6125 deprecated CN for hostname identity, +/// and modern certificates often publish identity only via SAN. +pub(crate) fn identity_from_certs(certs: &[CertificateDer<'_>]) -> Option { use x509_parser::certificate::X509Certificate; + use x509_parser::extensions::GeneralName; + let cert_der = certs.first()?; let (_, cert) = X509Certificate::from_der(cert_der).ok()?; + + if let Ok(Some(san)) = cert.subject_alternative_name() { + let dns_name = san.value.general_names.iter().find_map(|gn| match gn { + GeneralName::DNSName(name) => Some((*name).to_string()), + _ => None, + }); + if dns_name.is_some() { + return dns_name; + } + } + let cn = cert .subject() .iter_common_name() @@ -693,20 +710,49 @@ mod tests { } #[test] - fn cn_from_test_cert() { + fn identity_from_test_cert() { let pem = include_str!("../../tests/tls/cert.pem"); let certs: Vec> = rustls_pki_types::CertificateDer::pem_slice_iter(pem.as_bytes()) .collect::, _>>() .expect("parse PEM"); - let cn = cn_from_certs(&certs); - assert_eq!(cn.as_deref(), Some("CommonNameOrHostname")); + let identity = identity_from_certs(&certs); + assert_eq!(identity.as_deref(), Some("CommonNameOrHostname")); + } + + #[test] + fn identity_from_empty_certs() { + assert_eq!(identity_from_certs(&[]), None); + } + + #[test] + fn san_dns_preferred_over_cn() { + let pem = include_str!("../../tests/tls/cert_with_san.pem"); + let certs: Vec> = + rustls_pki_types::CertificateDer::pem_slice_iter(pem.as_bytes()) + .collect::, _>>() + .expect("parse PEM"); + + // Subject CN is "fallback-cn.example" but SAN dNSName comes first. + assert_eq!( + identity_from_certs(&certs).as_deref(), + Some("primary.san.example") + ); } #[test] - fn cn_from_empty_certs() { - assert_eq!(cn_from_certs(&[]), None); + fn san_dns_used_when_no_cn() { + let pem = include_str!("../../tests/tls/cert_san_only.pem"); + let certs: Vec> = + rustls_pki_types::CertificateDer::pem_slice_iter(pem.as_bytes()) + .collect::, _>>() + .expect("parse PEM"); + + assert_eq!( + identity_from_certs(&certs).as_deref(), + Some("only-via-san.example") + ); } #[test] diff --git a/pgdog/tests/tls/cert_san_only.pem b/pgdog/tests/tls/cert_san_only.pem new file mode 100644 index 000000000..38d331789 --- /dev/null +++ b/pgdog/tests/tls/cert_san_only.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDLjCCAhagAwIBAgIUEIwuQgRT10uprrx+bwnm491mGx4wDQYJKoZIhvcNAQEL +BQAwFjEUMBIGA1UECgwLTm9DTkV4YW1wbGUwHhcNMjYwNTI2MTcwOTQzWhcNMzYw +NTIzMTcwOTQzWjAWMRQwEgYDVQQKDAtOb0NORXhhbXBsZTCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAJ8fr3/OKf5Rv/E3mxUodxd0yBL4QWdTArUMQC1T +nZRWO8v4LWVQT+BkDG259V6KjJmIi1Vt01rukA61WuwINg4W7B7ZvEauOxMfedWf +RvyX1NqyogKZDMXeHRAXUX+/0Y3qwjDbeVW9POwPYKUdj/d7RP38IA/Az9bZKlxi +MYYV+SS3nZnYMsgHD81aX7LWmr3eGJ6Pe91dY6nz7ZaqyoU1dndn9sPQcCEGw/h2 +m73wv0Z5exEfgq6aUi4tQwN7HCf6GJMHrdTaeC4wOt5zNhWzHu2gj/4LWZTG63A8 +jYBVUjwOnhp25dJrA5aZosEJZt6p3RLTcp2CwYK9dgWxye8CAwEAAaN0MHIwHQYD +VR0OBBYEFEnIYKwQEkY32NfBwJ+C3S4qxsX7MB8GA1UdIwQYMBaAFEnIYKwQEkY3 +2NfBwJ+C3S4qxsX7MA8GA1UdEwEB/wQFMAMBAf8wHwYDVR0RBBgwFoIUb25seS12 +aWEtc2FuLmV4YW1wbGUwDQYJKoZIhvcNAQELBQADggEBAErE31L8yHPO6wEpAbog +ogZNCY6fpRpQ+SDdYN8ict4jUNHSmAnP813VjCK+J+Rz4ZY/yxBs0Yl1TsF39IoV +0uFLinXtd5HuwHm0kCJAhmWC/vz3deXXrIEaeoNxa3eH4C8Y5EJZoO0q8MQ+W79r +wuEOJC8fOpmgMvZuRpqfI4Z1kQtok6zL+Bxj4k0tMCmYdflWNfiDFuFxZKlkYHpe +ni6hKnaL55Tva1OgJ+DCFu32rw+6OFH8fl2B68PG89W9IydgkkTiR532mptBmMhS +Jip+4Pmy1cTwfBUPN0fXprdCfPlzY8mZ7vi/tlLRsLHopqLO19plWj20Yxsb5FEZ +2kE= +-----END CERTIFICATE----- diff --git a/pgdog/tests/tls/cert_with_san.pem b/pgdog/tests/tls/cert_with_san.pem new file mode 100644 index 000000000..410d49277 --- /dev/null +++ b/pgdog/tests/tls/cert_with_san.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDVjCCAj6gAwIBAgIUcuhi+K+4eS8ZpG/ehiKHgItkLH4wDQYJKoZIhvcNAQEL +BQAwHjEcMBoGA1UEAwwTZmFsbGJhY2stY24uZXhhbXBsZTAeFw0yNjA1MjYxNzA5 +MzRaFw0zNjA1MjMxNzA5MzRaMB4xHDAaBgNVBAMME2ZhbGxiYWNrLWNuLmV4YW1w +bGUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQD17MqrZZDF8HO5p7tJ +hgnX5P0Tq/ZcxKEuwRh4A5pxW4SK7MRNNrELXDQfpk8qJ6WbJz9cneaFXxUikR3P +vXk/RsoPVmKNgqRM6HQbK5z+YShnFiFG7hhgtXfJ35ig3cvl3deeU+ap/yzv62Pj +AIwBGwlIDu+q3IjaAdirmcCuCGEBz6k88r66CBYcAWIQ8DupFj7xrjETKZ/cFVj6 +rcIGxfeEBblcz27hVx4spj/+QfHDUjuU3974ymF4x3eNFtO7GSDpW2LfcZqmp6D4 ++5aUoygxTrMCQ+4urCqoo491oW1zSwfc/xvjagGOMbLz0TcJOci76mFxS5FG9GOw +/XxVAgMBAAGjgYswgYgwHQYDVR0OBBYEFC68PvmK5szZ9qjLexMBedvQR9EwMB8G +A1UdIwQYMBaAFC68PvmK5szZ9qjLexMBedvQR9EwMA8GA1UdEwEB/wQFMAMBAf8w +NQYDVR0RBC4wLIITcHJpbWFyeS5zYW4uZXhhbXBsZYIVc2Vjb25kYXJ5LnNhbi5l +eGFtcGxlMA0GCSqGSIb3DQEBCwUAA4IBAQB4u+H2p0Mtsrhkv2ftuMUu8WwdrdxB +QlP9EjkJR/k4vPf1hXHVWrBB6iwQKtQ3Nn4/ZOxE6RjNISlf0hZpbF65elij2WU3 +GR+hLBP6wDZeRFvjR/+NfimYCas7SRDd3GJLAIbOaVMl4CA/vxJY9S/DvwOvJdJL +OtBQwFKj/AeAskNA81iRr4XotrB+Q4NojR/v1wqWMOQ79rBFYOZnm7AvmLS8n5jv +PeZeNlFmakpjcfUlUOylBhIhvIXH6OcukaB+Krcx9OXLLMqBjkIa0ECKXwU7LXiH +kJu/4s/f5Xbw8+2D0dzsifeiWS2fyjzWxKqXeW2d9JiswfXusOl41t2I +-----END CERTIFICATE-----