diff --git a/crates/imap/src/op/authenticate.rs b/crates/imap/src/op/authenticate.rs index 82d64f05..2f6df14c 100644 --- a/crates/imap/src/op/authenticate.rs +++ b/crates/imap/src/op/authenticate.rs @@ -100,7 +100,7 @@ impl Session { tag: String, ) -> crate::Result<()> { // Throttle authentication requests - if self.jmap.is_auth_allowed(self.remote_addr.clone()).is_err() { + if self.jmap.is_auth_allowed_soft(&self.remote_addr).is_err() { self.write_bytes( StatusResponse::bye("Too many authentication requests from this IP address.") .into_bytes(), @@ -116,7 +116,9 @@ impl Session { // Authenticate let access_token = match credentials { Credentials::Plain { username, secret } | Credentials::XOauth2 { username, secret } => { - self.jmap.authenticate_plain(&username, &secret).await + self.jmap + .authenticate_plain(&username, &secret, &self.remote_addr) + .await } Credentials::OAuthBearer { token } => { match self diff --git a/crates/jmap/src/api/http.rs b/crates/jmap/src/api/http.rs index 219e3558..2f07c94a 100644 --- a/crates/jmap/src/api/http.rs +++ b/crates/jmap/src/api/http.rs @@ -185,7 +185,7 @@ pub async fn parse_jmap_request( ("oauth-authorization-server", &Method::GET) => { let remote_addr = jmap.build_remote_addr(&req, remote_ip); // Limit anonymous requests - return match jmap.is_anonymous_allowed(remote_addr) { + return match jmap.is_anonymous_allowed(&remote_addr) { Ok(_) => { JsonResponse::new(OAuthMetadata::new(&instance.data)).into_http_response() } @@ -199,37 +199,43 @@ pub async fn parse_jmap_request( match (path.next().unwrap_or(""), req.method()) { ("", &Method::GET) => { - return match jmap.is_anonymous_allowed(remote_addr) { + return match jmap.is_anonymous_allowed(&remote_addr) { Ok(_) => jmap.handle_user_device_auth(&mut req).await, Err(err) => err.into_http_response(), } } ("", &Method::POST) => { - return match jmap.is_auth_allowed(remote_addr) { - Ok(_) => jmap.handle_user_device_auth_post(&mut req).await, + return match jmap.is_auth_allowed_soft(&remote_addr) { + Ok(_) => { + jmap.handle_user_device_auth_post(&mut req, &remote_addr) + .await + } Err(err) => err.into_http_response(), } } ("code", &Method::GET) => { - return match jmap.is_anonymous_allowed(remote_addr) { + return match jmap.is_anonymous_allowed(&remote_addr) { Ok(_) => jmap.handle_user_code_auth(&mut req).await, Err(err) => err.into_http_response(), } } ("code", &Method::POST) => { - return match jmap.is_auth_allowed(remote_addr) { - Ok(_) => jmap.handle_user_code_auth_post(&mut req).await, + return match jmap.is_auth_allowed_soft(&remote_addr) { + Ok(_) => { + jmap.handle_user_code_auth_post(&mut req, &remote_addr) + .await + } Err(err) => err.into_http_response(), } } ("device", &Method::POST) => { - return match jmap.is_anonymous_allowed(remote_addr) { + return match jmap.is_anonymous_allowed(&remote_addr) { Ok(_) => jmap.handle_device_auth(&mut req, instance).await, Err(err) => err.into_http_response(), } } ("token", &Method::POST) => { - return match jmap.is_anonymous_allowed(remote_addr) { + return match jmap.is_anonymous_allowed(&remote_addr) { Ok(_) => jmap.handle_token_request(&mut req).await, Err(err) => err.into_http_response(), } @@ -237,18 +243,22 @@ pub async fn parse_jmap_request( _ => (), } } - "crypto" if jmap.config.encrypt => match *req.method() { - Method::GET => { - return jmap.handle_crypto_update(&mut req).await; - } - Method::POST => { - return match jmap.is_auth_allowed(jmap.build_remote_addr(&req, remote_ip)) { - Ok(_) => jmap.handle_crypto_update(&mut req).await, - Err(err) => err.into_http_response(), + "crypto" if jmap.config.encrypt => { + let remote_addr = jmap.build_remote_addr(&req, remote_ip); + + match *req.method() { + Method::GET => { + return jmap.handle_crypto_update(&mut req, &remote_addr).await; } + Method::POST => { + return match jmap.is_auth_allowed_soft(&remote_addr) { + Ok(_) => jmap.handle_crypto_update(&mut req, &remote_addr).await, + Err(err) => err.into_http_response(), + } + } + _ => (), } - _ => (), - }, + } "admin" => { // Make sure the user is a superuser diff --git a/crates/jmap/src/auth/authenticate.rs b/crates/jmap/src/auth/authenticate.rs index a01e0e46..36ea0e00 100644 --- a/crates/jmap/src/auth/authenticate.rs +++ b/crates/jmap/src/auth/authenticate.rs @@ -62,7 +62,7 @@ impl JMAP { let addr = self.build_remote_addr(req, remote_ip); if mechanism.eq_ignore_ascii_case("basic") { // Enforce rate limit for authentication requests - self.is_auth_allowed(addr)?; + self.is_auth_allowed_soft(&addr)?; // Decode the base64 encoded credentials if let Some((account, secret)) = base64_decode(token.as_bytes()) @@ -73,7 +73,7 @@ impl JMAP { }) }) { - self.authenticate_plain(&account, &secret).await + self.authenticate_plain(&account, &secret, &addr).await } else { tracing::debug!( context = "authenticate_headers", @@ -84,7 +84,7 @@ impl JMAP { } } else if mechanism.eq_ignore_ascii_case("bearer") { // Enforce anonymous rate limit for bearer auth requests - self.is_anonymous_allowed(addr)?; + self.is_anonymous_allowed(&addr)?; match self.validate_access_token("access_token", &token).await { Ok((account_id, _, _)) => self.get_access_token(account_id).await, @@ -99,7 +99,7 @@ impl JMAP { } } else { // Enforce anonymous rate limit - self.is_anonymous_allowed(addr)?; + self.is_anonymous_allowed(&addr)?; None } .map(|access_token| { @@ -118,7 +118,7 @@ impl JMAP { } } else { // Enforce anonymous rate limit - self.is_anonymous_allowed(self.build_remote_addr(req, remote_ip))?; + self.is_anonymous_allowed(&self.build_remote_addr(req, remote_ip))?; Ok(None) } @@ -266,15 +266,30 @@ impl JMAP { } } - pub async fn authenticate_plain(&self, username: &str, secret: &str) -> Option { - let mut principal = self + pub async fn authenticate_plain( + &self, + username: &str, + secret: &str, + remote_addr: &RemoteAddress, + ) -> Option { + let mut principal = match self .directory .authenticate(&Credentials::Plain { username: username.to_string(), secret: secret.to_string(), }) .await - .ok()??; + { + Ok(Some(principal)) => principal, + Ok(None) => { + let _ = self.is_auth_allowed_hard(remote_addr); + return None; + } + Err(_) => { + return None; + } + }; + if !principal.has_name() { principal.name = username.to_string(); } diff --git a/crates/jmap/src/auth/oauth/device_auth.rs b/crates/jmap/src/auth/oauth/device_auth.rs index c96b8aeb..2c1f4082 100644 --- a/crates/jmap/src/auth/oauth/device_auth.rs +++ b/crates/jmap/src/auth/oauth/device_auth.rs @@ -35,9 +35,12 @@ use utils::{listener::ServerInstance, map::ttl_dashmap::TtlMap}; use crate::{ api::{http::ToHttpResponse, HtmlResponse, HttpRequest, HttpResponse, JsonResponse}, - auth::oauth::{ - MAX_POST_LEN, OAUTH_HTML_ERROR, OAUTH_HTML_LOGIN_HEADER_FAILED, OAUTH_HTML_LOGIN_SUCCESS, - STATUS_AUTHORIZED, + auth::{ + oauth::{ + MAX_POST_LEN, OAUTH_HTML_ERROR, OAUTH_HTML_LOGIN_HEADER_FAILED, + OAUTH_HTML_LOGIN_SUCCESS, STATUS_AUTHORIZED, + }, + rate_limit::RemoteAddress, }, JMAP, }; @@ -148,7 +151,11 @@ impl JMAP { } // Handles POST request from the device authorization form - pub async fn handle_user_device_auth_post(&self, req: &mut HttpRequest) -> HttpResponse { + pub async fn handle_user_device_auth_post( + &self, + req: &mut HttpRequest, + remote_addr: &RemoteAddress, + ) -> HttpResponse { // Parse form let fields = match FormData::from_request(req, MAX_POST_LEN).await { Ok(fields) => fields, @@ -170,7 +177,7 @@ impl JMAP { { if let (Some(email), Some(password)) = (fields.get("email"), fields.get("password")) { - if let Some(id) = self.authenticate_plain(email, password).await { + if let Some(id) = self.authenticate_plain(email, password, remote_addr).await { oauth .account_id .store(id.primary_id(), atomic::Ordering::Relaxed); diff --git a/crates/jmap/src/auth/oauth/user_code.rs b/crates/jmap/src/auth/oauth/user_code.rs index 2404b206..e62792d8 100644 --- a/crates/jmap/src/auth/oauth/user_code.rs +++ b/crates/jmap/src/auth/oauth/user_code.rs @@ -37,6 +37,7 @@ use utils::map::ttl_dashmap::TtlMap; use crate::{ api::{http::ToHttpResponse, HtmlResponse, HttpRequest, HttpResponse}, + auth::rate_limit::RemoteAddress, JMAP, }; @@ -107,7 +108,11 @@ impl JMAP { } // Handles POST request from the code authorization form - pub async fn handle_user_code_auth_post(&self, req: &mut HttpRequest) -> HttpResponse { + pub async fn handle_user_code_auth_post( + &self, + req: &mut HttpRequest, + remote_addr: &RemoteAddress, + ) -> HttpResponse { // Parse form let params = match FormData::from_request(req, MAX_POST_LEN).await { Ok(params) => params, @@ -132,7 +137,8 @@ impl JMAP { // Authenticate user if let (Some(email), Some(password)) = (params.get("email"), params.get("password")) { - if let Some(access_token) = self.authenticate_plain(email, password).await { + if let Some(access_token) = self.authenticate_plain(email, password, remote_addr).await + { // Generate client code let client_code = thread_rng() .sample_iter(Alphanumeric) diff --git a/crates/jmap/src/auth/rate_limit.rs b/crates/jmap/src/auth/rate_limit.rs index 4fa31c78..88a3e699 100644 --- a/crates/jmap/src/auth/rate_limit.rs +++ b/crates/jmap/src/auth/rate_limit.rs @@ -72,9 +72,9 @@ impl JMAP { }) } - pub fn get_anonymous_limiter(&self, addr: RemoteAddress) -> Arc> { + pub fn get_anonymous_limiter(&self, addr: &RemoteAddress) -> Arc> { self.rate_limit_unauth - .get(&addr) + .get(addr) .map(|limiter| limiter.clone()) .unwrap_or_else(|| { let limiter = Arc::new(Mutex::new(AnonymousLimiter { @@ -87,7 +87,7 @@ impl JMAP { self.config.rate_authenticate_req.period, ), })); - self.rate_limit_unauth.insert(addr, limiter.clone()); + self.rate_limit_unauth.insert(addr.clone(), limiter.clone()); limiter }) } @@ -111,7 +111,7 @@ impl JMAP { } } - pub fn is_anonymous_allowed(&self, addr: RemoteAddress) -> Result<(), RequestError> { + pub fn is_anonymous_allowed(&self, addr: &RemoteAddress) -> Result<(), RequestError> { if self .get_anonymous_limiter(addr) .lock() @@ -139,7 +139,16 @@ impl JMAP { } } - pub fn is_auth_allowed(&self, addr: RemoteAddress) -> Result<(), RequestError> { + pub fn is_auth_allowed_soft(&self, addr: &RemoteAddress) -> Result<(), RequestError> { + match self.rate_limit_unauth.get(addr) { + Some(limiter) if !limiter.lock().auth_limiter.is_allowed_soft() => { + Err(RequestError::too_many_auth_attempts()) + } + _ => Ok(()), + } + } + + pub fn is_auth_allowed_hard(&self, addr: &RemoteAddress) -> Result<(), RequestError> { if self .get_anonymous_limiter(addr) .lock() diff --git a/crates/jmap/src/email/crypto.rs b/crates/jmap/src/email/crypto.rs index 0c102a9c..9e06ce78 100644 --- a/crates/jmap/src/email/crypto.rs +++ b/crates/jmap/src/email/crypto.rs @@ -45,7 +45,7 @@ use store::{ use crate::{ api::{http::ToHttpResponse, HtmlResponse, HttpRequest, HttpResponse}, - auth::oauth::FormData, + auth::{oauth::FormData, rate_limit::RemoteAddress}, JMAP, }; @@ -538,7 +538,11 @@ impl ToBitmaps for &EncryptionParams { impl JMAP { // Code authorization flow, handles an authorization request - pub async fn handle_crypto_update(&self, req: &mut HttpRequest) -> HttpResponse { + pub async fn handle_crypto_update( + &self, + req: &mut HttpRequest, + remote_addr: &RemoteAddress, + ) -> HttpResponse { let mut response = String::with_capacity( CRYPT_HTML_HEADER.len() + CRYPT_HTML_FOOTER.len() + CRYPT_HTML_FORM.len(), ); @@ -552,7 +556,7 @@ impl JMAP { Err(err) => return err, }; - match self.validate_form(form).await { + match self.validate_form(form, remote_addr).await { Ok(Some(params)) => { response.push_str( &CRYPT_HTML_SUCCESS @@ -586,6 +590,7 @@ impl JMAP { async fn validate_form( &self, mut form: FormData, + remote_addr: &RemoteAddress, ) -> Result, Cow> { let certificate = form.remove_bytes("certificate"); if let (Some(email), Some(password), Some(encryption)) = ( @@ -603,7 +608,7 @@ impl JMAP { // Authenticate let token = self - .authenticate_plain(email, password) + .authenticate_plain(email, password, remote_addr) .await .ok_or_else(|| Cow::from("Invalid login or password"))?; if encryption != "disable" { diff --git a/crates/managesieve/src/op/authenticate.rs b/crates/managesieve/src/op/authenticate.rs index 19a8a6f6..83a812e1 100644 --- a/crates/managesieve/src/op/authenticate.rs +++ b/crates/managesieve/src/op/authenticate.rs @@ -76,7 +76,7 @@ impl Session { }; // Throttle authentication requests - if self.jmap.is_auth_allowed(self.remote_addr.clone()).is_err() { + if self.jmap.is_auth_allowed_soft(&self.remote_addr).is_err() { tracing::debug!(parent: &self.span, event = "disconnect", "Too many authentication attempts, disconnecting.", @@ -89,7 +89,9 @@ impl Session { // Authenticate let access_token = match credentials { Credentials::Plain { username, secret } | Credentials::XOauth2 { username, secret } => { - self.jmap.authenticate_plain(&username, &secret).await + self.jmap + .authenticate_plain(&username, &secret, &self.remote_addr) + .await } Credentials::OAuthBearer { token } => { match self diff --git a/crates/utils/src/listener/limiter.rs b/crates/utils/src/listener/limiter.rs index 5a760b1a..e21689aa 100644 --- a/crates/utils/src/listener/limiter.rs +++ b/crates/utils/src/listener/limiter.rs @@ -79,6 +79,10 @@ impl RateLimiter { } } + pub fn is_allowed_soft(&self) -> bool { + self.tokens >= 1 || self.last_refill.elapsed() >= self.max_interval + } + pub fn retry_at(&self) -> Instant { Instant::now() + (self diff --git a/tests/src/jmap/auth_limits.rs b/tests/src/jmap/auth_limits.rs index 4f3c7689..b6a13077 100644 --- a/tests/src/jmap/auth_limits.rs +++ b/tests/src/jmap/auth_limits.rs @@ -64,7 +64,7 @@ pub async fn test(server: Arc, admin_client: &mut Client) { .await, Err(jmap_client::Error::Problem(err)) if err.status() == Some(401))); - // Requests should be rate limited + // Invalid authentication requests should be rate limited let mut n_401 = 0; let mut n_429 = 0; for n in 0..110 { @@ -96,7 +96,17 @@ pub async fn test(server: Arc, admin_client: &mut Client) { } // Limit should be restored after 1 second - tokio::time::sleep(Duration::from_secs(1)).await; + tokio::time::sleep(Duration::from_millis(1500)).await; + + // Valid authentication requests should not be rate limited + for _ in 0..110 { + Client::new() + .credentials(Credentials::basic("jdoe@example.com", "12345")) + .accept_invalid_certs(true) + .connect("https://127.0.0.1:8899") + .await + .unwrap(); + } // Login with the correct credentials let client = Client::new()