From f9f40fa41dd8fd201fda36d29adc5ae3e9376da8 Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Wed, 22 Feb 2023 22:34:13 +0300 Subject: [PATCH] [proxy] Introduce SniParams for creds parsing --- proxy/src/auth.rs | 2 +- proxy/src/auth/backend.rs | 2 + proxy/src/auth/credentials.rs | 84 +++++++++++++++++++++++------------ proxy/src/proxy.rs | 20 ++++++--- 4 files changed, 72 insertions(+), 36 deletions(-) diff --git a/proxy/src/auth.rs b/proxy/src/auth.rs index dfea84953b..41b5a2e97b 100644 --- a/proxy/src/auth.rs +++ b/proxy/src/auth.rs @@ -3,7 +3,7 @@ pub mod backend; pub use backend::BackendType; -mod credentials; +pub mod credentials; pub use credentials::ClientCredentials; mod password_hack; diff --git a/proxy/src/auth/backend.rs b/proxy/src/auth/backend.rs index 3c20f6d10e..25db6f2e6d 100644 --- a/proxy/src/auth/backend.rs +++ b/proxy/src/auth/backend.rs @@ -183,7 +183,9 @@ impl BackendType<'_, ClientCredentials<'_>> { info!("user successfully authenticated"); Ok(res) } +} +impl BackendType<'_, ClientCredentials<'_>> { /// When applicable, wake the compute node, gaining its connection info in the process. /// The link auth flow doesn't support this, so we return [`None`] in that case. pub async fn wake_compute( diff --git a/proxy/src/auth/credentials.rs b/proxy/src/auth/credentials.rs index c556c33197..3251d37145 100644 --- a/proxy/src/auth/credentials.rs +++ b/proxy/src/auth/credentials.rs @@ -31,12 +31,22 @@ pub enum ClientCredsParseError { impl UserFacingError for ClientCredsParseError {} +/// eSNI parameters which might contain endpoint/project name. +#[derive(Default)] +pub struct SniParams<'a> { + /// Server Name Indication (TLS jargon). + pub sni: Option<&'a str>, + /// Common Name from a TLS certificate. + pub common_name: Option<&'a str>, +} + /// Various client credentials which we use for authentication. /// Note that we don't store any kind of client key or password here. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ClientCredentials<'a> { + /// Name of postgres role. pub user: &'a str, - // TODO: this is a severe misnomer! We should think of a new name ASAP. + /// Also known as endpoint in the console. pub project: Option>, } @@ -49,18 +59,17 @@ impl ClientCredentials<'_> { impl<'a> ClientCredentials<'a> { pub fn parse( - params: &'a StartupMessageParams, - sni: Option<&str>, - common_name: Option<&str>, + startup_params: &'a StartupMessageParams, + &SniParams { sni, common_name }: &SniParams<'_>, ) -> Result { use ClientCredsParseError::*; // Some parameters are stored in the startup message. - let get_param = |key| params.get(key).ok_or(MissingKey(key)); + let get_param = |key| startup_params.get(key).ok_or(MissingKey(key)); let user = get_param("user")?; // Project name might be passed via PG's command-line options. - let project_option = params.options_raw().and_then(|mut options| { + let project_option = startup_params.options_raw().and_then(|mut options| { options .find_map(|opt| opt.strip_prefix("project=")) .map(Cow::Borrowed) @@ -122,7 +131,9 @@ mod tests { // According to postgresql, only `user` should be required. let options = StartupMessageParams::new([("user", "john_doe")]); - let creds = ClientCredentials::parse(&options, None, None)?; + let sni = SniParams::default(); + + let creds = ClientCredentials::parse(&options, &sni)?; assert_eq!(creds.user, "john_doe"); assert_eq!(creds.project, None); @@ -131,13 +142,15 @@ mod tests { #[test] fn parse_excessive() -> anyhow::Result<()> { - let options = StartupMessageParams::new([ + let startup = StartupMessageParams::new([ ("user", "john_doe"), ("database", "world"), // should be ignored ("foo", "bar"), // should be ignored ]); - let creds = ClientCredentials::parse(&options, None, None)?; + let sni = SniParams::default(); + + let creds = ClientCredentials::parse(&startup, &sni)?; assert_eq!(creds.user, "john_doe"); assert_eq!(creds.project, None); @@ -146,12 +159,14 @@ mod tests { #[test] fn parse_project_from_sni() -> anyhow::Result<()> { - let options = StartupMessageParams::new([("user", "john_doe")]); + let startup = StartupMessageParams::new([("user", "john_doe")]); - let sni = Some("foo.localhost"); - let common_name = Some("localhost"); + let sni = SniParams { + sni: Some("foo.localhost"), + common_name: Some("localhost"), + }; - let creds = ClientCredentials::parse(&options, sni, common_name)?; + let creds = ClientCredentials::parse(&startup, &sni)?; assert_eq!(creds.user, "john_doe"); assert_eq!(creds.project.as_deref(), Some("foo")); @@ -160,12 +175,14 @@ mod tests { #[test] fn parse_project_from_options() -> anyhow::Result<()> { - let options = StartupMessageParams::new([ + let startup = StartupMessageParams::new([ ("user", "john_doe"), ("options", "-ckey=1 project=bar -c geqo=off"), ]); - let creds = ClientCredentials::parse(&options, None, None)?; + let sni = SniParams::default(); + + let creds = ClientCredentials::parse(&startup, &sni)?; assert_eq!(creds.user, "john_doe"); assert_eq!(creds.project.as_deref(), Some("bar")); @@ -174,12 +191,17 @@ mod tests { #[test] fn parse_projects_identical() -> anyhow::Result<()> { - let options = StartupMessageParams::new([("user", "john_doe"), ("options", "project=baz")]); + let startup = StartupMessageParams::new([ + ("user", "john_doe"), + ("options", "project=baz"), // fmt + ]); - let sni = Some("baz.localhost"); - let common_name = Some("localhost"); + let sni = SniParams { + sni: Some("baz.localhost"), + common_name: Some("localhost"), + }; - let creds = ClientCredentials::parse(&options, sni, common_name)?; + let creds = ClientCredentials::parse(&startup, &sni)?; assert_eq!(creds.user, "john_doe"); assert_eq!(creds.project.as_deref(), Some("baz")); @@ -188,13 +210,17 @@ mod tests { #[test] fn parse_projects_different() { - let options = - StartupMessageParams::new([("user", "john_doe"), ("options", "project=first")]); + let startup = StartupMessageParams::new([ + ("user", "john_doe"), + ("options", "project=first"), // fmt + ]); - let sni = Some("second.localhost"); - let common_name = Some("localhost"); + let sni = SniParams { + sni: Some("second.localhost"), + common_name: Some("localhost"), + }; - let err = ClientCredentials::parse(&options, sni, common_name).expect_err("should fail"); + let err = ClientCredentials::parse(&startup, &sni).expect_err("should fail"); match err { InconsistentProjectNames { domain, option } => { assert_eq!(option, "first"); @@ -206,12 +232,14 @@ mod tests { #[test] fn parse_inconsistent_sni() { - let options = StartupMessageParams::new([("user", "john_doe")]); + let startup = StartupMessageParams::new([("user", "john_doe")]); - let sni = Some("project.localhost"); - let common_name = Some("example.com"); + let sni = SniParams { + sni: Some("project.localhost"), + common_name: Some("example.com"), + }; - let err = ClientCredentials::parse(&options, sni, common_name).expect_err("should fail"); + let err = ClientCredentials::parse(&startup, &sni).expect_err("should fail"); match err { InconsistentSni { sni, cn } => { assert_eq!(sni, "project.localhost"); diff --git a/proxy/src/proxy.rs b/proxy/src/proxy.rs index 9642047812..6383dc6cfc 100644 --- a/proxy/src/proxy.rs +++ b/proxy/src/proxy.rs @@ -2,7 +2,7 @@ mod tests; use crate::{ - auth::{self, backend::AuthSuccess}, + auth::{self, backend::AuthSuccess, credentials}, cancellation::{self, CancelMap}, compute::{self, PostgresConnection}, config::{ProxyConfig, TlsConfig}, @@ -112,7 +112,6 @@ pub async fn handle_ws_client( } let tls = config.tls_config.as_ref(); - let hostname = hostname.as_deref(); // TLS is None here, because the connection is already encrypted. let do_handshake = handshake(stream, None, cancel_map); @@ -121,13 +120,17 @@ pub async fn handle_ws_client( None => return Ok(()), // it's a cancellation request }; + let sni = credentials::SniParams { + sni: hostname.as_deref(), + common_name: tls.and_then(|tls| tls.common_name.as_deref()), + }; + // Extract credentials which we're going to use for auth. let creds = { - let common_name = tls.and_then(|tls| tls.common_name.as_deref()); let result = config .auth_backend .as_ref() - .map(|_| auth::ClientCredentials::parse(¶ms, hostname, common_name)) + .map(|_| auth::ClientCredentials::parse(¶ms, &sni)) .transpose(); async { result }.or_else(|e| stream.throw_error(e)).await? @@ -159,14 +162,17 @@ async fn handle_client( None => return Ok(()), // it's a cancellation request }; + let sni = credentials::SniParams { + sni: stream.get_ref().sni_hostname(), + common_name: tls.and_then(|tls| tls.common_name.as_deref()), + }; + // Extract credentials which we're going to use for auth. let creds = { - let sni = stream.get_ref().sni_hostname(); - let common_name = tls.and_then(|tls| tls.common_name.as_deref()); let result = config .auth_backend .as_ref() - .map(|_| auth::ClientCredentials::parse(¶ms, sni, common_name)) + .map(|_| auth::ClientCredentials::parse(¶ms, &sni)) .transpose(); async { result }.or_else(|e| stream.throw_error(e)).await?