diff --git a/proxy/src/auth/backend.rs b/proxy/src/auth/backend.rs index 25db6f2e6d..ca034a5e05 100644 --- a/proxy/src/auth/backend.rs +++ b/proxy/src/auth/backend.rs @@ -11,7 +11,7 @@ use crate::{ provider::{CachedNodeInfo, ConsoleReqExtra}, Api, }, - stream, url, + scram, stream, url, }; use futures::TryFutureExt; use std::borrow::Cow; @@ -106,6 +106,23 @@ impl<'a, T, E> BackendType<'a, Result> { } } +impl console::AuthInfo { + /// Either it's our way ([SCRAM](crate::scram)) or the highway :) + /// But seriously, we don't aim to support anything but SCRAM for now. + fn scram_or_goodbye(self) -> auth::Result { + match self { + Self::Md5(_) => { + info!("auth endpoint chooses MD5"); + Err(auth::AuthError::bad_auth_method("MD5")) + } + Self::Scram(secret) => { + info!("auth endpoint chooses SCRAM"); + Ok(secret) + } + } + } +} + /// True to its name, this function encapsulates our current auth trade-offs. /// Here, we choose the appropriate auth flow based on circumstances. async fn auth_quirks( diff --git a/proxy/src/auth/backend/classic.rs b/proxy/src/auth/backend/classic.rs index 51d73ffcad..0d89faea19 100644 --- a/proxy/src/auth/backend/classic.rs +++ b/proxy/src/auth/backend/classic.rs @@ -2,7 +2,7 @@ use super::AuthSuccess; use crate::{ auth::{self, AuthFlow, ClientCredentials}, compute, - console::{self, AuthInfo, CachedNodeInfo, ConsoleReqExtra}, + console::{self, CachedNodeInfo, ConsoleReqExtra}, sasl, scram, stream::PqStream, }; @@ -37,31 +37,16 @@ async fn do_scram( Ok(keys) } -pub(super) async fn authenticate( +pub async fn authenticate( api: &impl console::Api, extra: &ConsoleReqExtra<'_>, creds: &ClientCredentials<'_>, client: &mut PqStream, ) -> auth::Result> { - info!("fetching user's authentication info"); - let info = api.get_auth_info(extra, creds).await?.unwrap_or_else(|| { - // If we don't have an authentication secret, we mock one to - // prevent malicious probing (possible due to missing protocol steps). - // This mocked secret will never lead to successful authentication. - info!("authentication info not found, mocking it"); - AuthInfo::Scram(scram::ServerSecret::mock(creds.user, rand::random())) - }); + let info = console::get_auth_info(api, extra, creds).await?; - let scram_keys = match info { - AuthInfo::Md5(_) => { - info!("auth endpoint chooses MD5"); - return Err(auth::AuthError::bad_auth_method("MD5")); - } - AuthInfo::Scram(secret) => { - info!("auth endpoint chooses SCRAM"); - do_scram(secret, creds, client).await? - } - }; + let secret = info.scram_or_goodbye()?; + let scram_keys = do_scram(secret, creds, client).await?; let mut node = api.wake_compute(extra, creds).await?; node.config.auth_keys(AuthKeys::ScramSha256(scram_keys)); diff --git a/proxy/src/auth/backend/hacks.rs b/proxy/src/auth/backend/hacks.rs index f710581cb2..b09e18cfe5 100644 --- a/proxy/src/auth/backend/hacks.rs +++ b/proxy/src/auth/backend/hacks.rs @@ -5,11 +5,33 @@ use crate::{ self, provider::{CachedNodeInfo, ConsoleReqExtra}, }, - stream, + stream::PqStream, }; use tokio::io::{AsyncRead, AsyncWrite}; use tracing::{info, warn}; +/// Wake the compute node, but only if the password is valid. +async fn get_compute( + api: &impl console::Api, + extra: &ConsoleReqExtra<'_>, + creds: &mut ClientCredentials<'_>, + password: Vec, +) -> auth::Result { + // TODO: this will slow down both "hacks" below; we probably need a cache. + let info = console::get_auth_info(api, extra, creds).await?; + + let secret = info.scram_or_goodbye()?; + if !secret.matches_password(&password) { + info!("our obscure magic indicates that the password doesn't match"); + return Err(auth::AuthError::auth_failed(creds.user)); + } + + let mut node = api.wake_compute(extra, creds).await?; + node.config.password(password); + + Ok(node) +} + /// Compared to [SCRAM](crate::scram), cleartext password auth saves /// one round trip and *expensive* computations (>= 4096 HMAC iterations). /// These properties are benefical for serverless JS workers, so we @@ -18,7 +40,7 @@ pub async fn cleartext_hack( api: &impl console::Api, extra: &ConsoleReqExtra<'_>, creds: &mut ClientCredentials<'_>, - client: &mut stream::PqStream, + client: &mut PqStream, ) -> auth::Result> { warn!("cleartext auth flow override is enabled, proceeding"); let password = AuthFlow::new(client) @@ -27,8 +49,7 @@ pub async fn cleartext_hack( .authenticate() .await?; - let mut node = api.wake_compute(extra, creds).await?; - node.config.password(password); + let node = get_compute(api, extra, creds, password).await?; // Report tentative success; compute node will check the password anyway. Ok(AuthSuccess { @@ -43,7 +64,7 @@ pub async fn password_hack( api: &impl console::Api, extra: &ConsoleReqExtra<'_>, creds: &mut ClientCredentials<'_>, - client: &mut stream::PqStream, + client: &mut PqStream, ) -> auth::Result> { warn!("project not specified, resorting to the password hack auth flow"); let payload = AuthFlow::new(client) @@ -55,8 +76,8 @@ pub async fn password_hack( info!(project = &payload.project, "received missing parameter"); creds.project = Some(payload.project.into()); - let mut node = api.wake_compute(extra, creds).await?; - node.config.password(payload.password); + let password = payload.password.into(); + let node = get_compute(api, extra, creds, password).await?; // Report tentative success; compute node will check the password anyway. Ok(AuthSuccess { diff --git a/proxy/src/console.rs b/proxy/src/console.rs index 1f3ef99555..20179a2fd2 100644 --- a/proxy/src/console.rs +++ b/proxy/src/console.rs @@ -6,7 +6,9 @@ pub mod messages; /// Wrappers for console APIs and their mocks. pub mod provider; -pub use provider::{errors, Api, AuthInfo, CachedNodeInfo, ConsoleReqExtra, NodeInfo}; +pub use provider::{ + errors, get_auth_info, Api, AuthInfo, CachedNodeInfo, ConsoleReqExtra, NodeInfo, +}; /// Various cache-related types. pub mod caches { diff --git a/proxy/src/console/provider.rs b/proxy/src/console/provider.rs index 80cd94d483..5b22b67df0 100644 --- a/proxy/src/console/provider.rs +++ b/proxy/src/console/provider.rs @@ -9,6 +9,7 @@ use crate::{ }; use async_trait::async_trait; use std::sync::Arc; +use tracing::info; pub mod errors { use crate::{ @@ -175,6 +176,12 @@ pub struct NodeInfo { pub type NodeInfoCache = TimedLru, NodeInfo>; pub type CachedNodeInfo = timed_lru::Cached<&'static NodeInfoCache>; +/// Various caches for [`console`]. +pub struct ApiCaches { + /// Cache for the `wake_compute` API method. + pub node_info: NodeInfoCache, +} + /// This will allocate per each call, but the http requests alone /// already require a few allocations, so it should be fine. #[async_trait] @@ -194,8 +201,21 @@ pub trait Api { ) -> Result; } -/// Various caches for [`console`]. -pub struct ApiCaches { - /// Cache for the `wake_compute` API method. - pub node_info: NodeInfoCache, +/// A more insightful version of [`Api::get_auth_info`] which +/// knows what to do when we get [`None`] instead of [`AuthInfo`]. +pub async fn get_auth_info( + api: &impl Api, + extra: &ConsoleReqExtra<'_>, + creds: &ClientCredentials<'_>, +) -> Result { + info!("fetching user's authentication info"); + let info = api.get_auth_info(extra, creds).await?.unwrap_or_else(|| { + // If we don't have an authentication secret, we mock one to + // prevent malicious probing (possible due to missing protocol steps). + // This mocked secret will never lead to successful authentication. + info!("authentication info not found, mocking it"); + AuthInfo::Scram(scram::ServerSecret::mock(creds.user, rand::random())) + }); + + Ok(info) } diff --git a/proxy/src/proxy/tests.rs b/proxy/src/proxy/tests.rs index ed429df421..6158828b6c 100644 --- a/proxy/src/proxy/tests.rs +++ b/proxy/src/proxy/tests.rs @@ -92,10 +92,10 @@ impl TestAuth for NoAuth {} struct Scram(scram::ServerSecret); impl Scram { - fn new(password: &str) -> anyhow::Result { + fn new(password: &[u8]) -> anyhow::Result { let salt = rand::random::<[u8; 16]>(); - let secret = scram::ServerSecret::build(password, &salt, 256) - .context("failed to generate scram secret")?; + let secret = scram::ServerSecret::build(password, &salt, 256); + Ok(Scram(secret)) } @@ -230,11 +230,11 @@ async fn keepalive_is_inherited() -> anyhow::Result<()> { } #[rstest] -#[case("password_foo")] -#[case("pwd-bar")] -#[case("")] +#[case(b"password_foo")] +#[case(b"pwd-bar")] +#[case(b"")] #[tokio::test] -async fn scram_auth_good(#[case] password: &str) -> anyhow::Result<()> { +async fn scram_auth_good(#[case] password: &[u8]) -> anyhow::Result<()> { let (client, server) = tokio::io::duplex(1024); let (client_config, server_config) = diff --git a/proxy/src/scram.rs b/proxy/src/scram.rs index 7cc4191435..a28f52ed2d 100644 --- a/proxy/src/scram.rs +++ b/proxy/src/scram.rs @@ -12,7 +12,6 @@ mod messages; mod secret; mod signature; -#[cfg(test)] mod password; pub use exchange::Exchange; diff --git a/proxy/src/scram/exchange.rs b/proxy/src/scram/exchange.rs index 882769a70d..556cc36752 100644 --- a/proxy/src/scram/exchange.rs +++ b/proxy/src/scram/exchange.rs @@ -73,7 +73,7 @@ impl sasl::Mechanism for Exchange<'_> { let server_first_message = client_first_message.build_server_first_message( &(self.nonce)(), - &self.secret.salt_base64, + &self.secret.salt, self.secret.iterations, ); let msg = server_first_message.as_str().to_owned(); diff --git a/proxy/src/scram/messages.rs b/proxy/src/scram/messages.rs index 05855e74df..ba63fb57c6 100644 --- a/proxy/src/scram/messages.rs +++ b/proxy/src/scram/messages.rs @@ -75,19 +75,27 @@ impl<'a> ClientFirstMessage<'a> { pub fn build_server_first_message( &self, nonce: &[u8; SCRAM_RAW_NONCE_LEN], - salt_base64: &str, + salt: &[u8], iterations: u32, ) -> OwnedServerFirstMessage { use std::fmt::Write; let mut message = String::new(); + + // Write base64-encoded combined nonce. write!(&mut message, "r={}", self.nonce).unwrap(); base64::encode_config_buf(nonce, base64::STANDARD, &mut message); let combined_nonce = 2..message.len(); - write!(&mut message, ",s={},i={}", salt_base64, iterations).unwrap(); + + // Write base64-encoded salt. + write!(&mut message, ",s=").unwrap(); + base64::encode_config_buf(salt, base64::STANDARD, &mut message); + + // Write number of iterations. + write!(&mut message, ",i={iterations}").unwrap(); // This design guarantees that it's impossible to create a - // server-first-message without receiving a client-first-message + // server-first-message without receiving a client-first-message. OwnedServerFirstMessage { message, nonce: combined_nonce, @@ -229,4 +237,49 @@ mod tests { "SRpfsIVS4Gk11w1LqQ4QvCUBZYQmqXNSDEcHqbQ3CHI=" ); } + + #[test] + fn build_server_messages() { + let input = "n,,n=pepe,r=t8JwklwKecDLwSsA72rHmVju"; + let client_first_message = ClientFirstMessage::parse(&input).unwrap(); + + let nonce = [0; 18]; + let salt = [1, 2, 3]; + let iterations = 4096; + let server_first_message = + client_first_message.build_server_first_message(&nonce, &salt, iterations); + + assert_eq!( + server_first_message.message, + "r=t8JwklwKecDLwSsA72rHmVjuAAAAAAAAAAAAAAAAAAAAAAAA,s=AQID,i=4096" + ); + assert_eq!( + server_first_message.nonce(), + "t8JwklwKecDLwSsA72rHmVjuAAAAAAAAAAAAAAAAAAAAAAAA" + ); + + let input = [ + "c=eSws", + "r=iiYEfS3rOgn8S3rtpSdrOsHtPLWvIkdgmHxA0hf3JNOAG4dU", + "p=SRpfsIVS4Gk11w1LqQ4QvCUBZYQmqXNSDEcHqbQ3CHI=", + ] + .join(","); + + let client_final_message = ClientFinalMessage::parse(&input).unwrap(); + + let signature_builder = SignatureBuilder { + client_first_message_bare: client_first_message.bare, + server_first_message: server_first_message.as_str(), + client_final_message_without_proof: &client_final_message.without_proof, + }; + + let server_key = ScramKey::default(); + let server_final_message = + client_final_message.build_server_final_message(signature_builder, &server_key); + + assert_eq!( + server_final_message, + "v=XEL4X1vy5LnqIgOo4hOjm7zd1Ceyo9+nBUE+/zVnqLE=" + ); + } } diff --git a/proxy/src/scram/password.rs b/proxy/src/scram/password.rs index 656780d853..9723e5af39 100644 --- a/proxy/src/scram/password.rs +++ b/proxy/src/scram/password.rs @@ -1,6 +1,7 @@ //! Password hashing routines. use super::key::ScramKey; +use tracing::warn; pub const SALTED_PASSWORD_LEN: usize = 32; @@ -13,7 +14,12 @@ pub struct SaltedPassword { impl SaltedPassword { /// See `scram-common.c : scram_SaltedPassword` for details. /// Further reading: (see `PBKDF2`). + /// TODO: implement proper password normalization required by the RFC! pub fn new(password: &[u8], salt: &[u8], iterations: u32) -> SaltedPassword { + if !password.is_ascii() { + warn!("found non-ascii symbols in password! salted password might be broken"); + } + let one = 1_u32.to_be_bytes(); // magic let mut current = super::hmac_sha256(password, [salt, &one]); @@ -30,6 +36,7 @@ impl SaltedPassword { } /// Derive `ClientKey` from a salted hashed password. + #[cfg(test)] pub fn client_key(&self) -> ScramKey { super::hmac_sha256(&self.bytes, [b"Client Key".as_ref()]).into() } diff --git a/proxy/src/scram/secret.rs b/proxy/src/scram/secret.rs index 424beccec9..a37c9310cc 100644 --- a/proxy/src/scram/secret.rs +++ b/proxy/src/scram/secret.rs @@ -1,15 +1,14 @@ //! Tools for SCRAM server secret management. -use super::base64_decode_array; -use super::key::ScramKey; +use super::{base64_decode_array, key::ScramKey, password::SaltedPassword}; -/// Server secret is produced from [password](super::password::SaltedPassword) +/// Server secret is produced from [password](SaltedPassword) /// and is used throughout the authentication process. pub struct ServerSecret { /// Number of iterations for `PBKDF2` function. pub iterations: u32, /// Salt used to hash user's password. - pub salt_base64: String, + pub salt: Vec, /// Hashed `ClientKey`. pub stored_key: ScramKey, /// Used by client to verify server's signature. @@ -30,7 +29,7 @@ impl ServerSecret { let secret = ServerSecret { iterations: iterations.parse().ok()?, - salt_base64: salt.to_owned(), + salt: base64::decode(salt).ok()?, stored_key: base64_decode_array(stored_key)?.into(), server_key: base64_decode_array(server_key)?.into(), doomed: false, @@ -48,31 +47,31 @@ impl ServerSecret { Self { iterations: 4096, - salt_base64: base64::encode(mocked_salt), + salt: mocked_salt.into(), stored_key: ScramKey::default(), server_key: ScramKey::default(), doomed: true, } } + /// Check if this secret was derived from the given password. + pub fn matches_password(&self, password: &[u8]) -> bool { + let password = SaltedPassword::new(password, &self.salt, self.iterations); + self.server_key == password.server_key() + } + /// Build a new server secret from the prerequisites. - /// XXX: We only use this function in tests. #[cfg(test)] - pub fn build(password: &str, salt: &[u8], iterations: u32) -> Option { - // TODO: implement proper password normalization required by the RFC - if !password.is_ascii() { - return None; - } + pub fn build(password: &[u8], salt: &[u8], iterations: u32) -> Self { + let password = SaltedPassword::new(password, salt, iterations); - let password = super::password::SaltedPassword::new(password.as_bytes(), salt, iterations); - - Some(Self { + Self { iterations, - salt_base64: base64::encode(salt), + salt: salt.into(), stored_key: password.client_key().sha256(), server_key: password.server_key(), doomed: false, - }) + } } } @@ -87,17 +86,11 @@ mod tests { let stored_key = "D5h6KTMBlUvDJk2Y8ELfC1Sjtc6k9YHjRyuRZyBNJns="; let server_key = "Pi3QHbcluX//NDfVkKlFl88GGzlJ5LkyPwcdlN/QBvI="; - let secret = format!( - "SCRAM-SHA-256${iterations}:{salt}${stored_key}:{server_key}", - iterations = iterations, - salt = salt, - stored_key = stored_key, - server_key = server_key, - ); + let secret = format!("SCRAM-SHA-256${iterations}:{salt}${stored_key}:{server_key}"); let parsed = ServerSecret::parse(&secret).unwrap(); assert_eq!(parsed.iterations, iterations); - assert_eq!(parsed.salt_base64, salt); + assert_eq!(base64::encode(parsed.salt), salt); assert_eq!(base64::encode(parsed.stored_key), stored_key); assert_eq!(base64::encode(parsed.server_key), server_key); @@ -106,9 +99,9 @@ mod tests { #[test] fn build_scram_secret() { let salt = b"salt"; - let secret = ServerSecret::build("password", salt, 4096).unwrap(); + let secret = ServerSecret::build(b"password", salt, 4096); assert_eq!(secret.iterations, 4096); - assert_eq!(secret.salt_base64, base64::encode(salt)); + assert_eq!(secret.salt, salt); assert_eq!( base64::encode(secret.stored_key.as_ref()), "lF4cRm/Jky763CN4HtxdHnjV4Q8AWTNlKvGmEFFU8IQ=" @@ -118,4 +111,12 @@ mod tests { "ub8OgRsftnk2ccDMOt7ffHXNcikRkQkq1lh4xaAqrSw=" ); } + + #[test] + fn secret_match_password() { + let password = b"password"; + let secret = ServerSecret::build(password, b"salt", 2); + assert!(secret.matches_password(password)); + assert!(!secret.matches_password(b"different")); + } }