Successful authentication requests should not count when rate limiting

This commit is contained in:
mdecimus
2023-08-07 16:41:52 +02:00
parent 5346beb975
commit 39cbb946f7
10 changed files with 119 additions and 49 deletions

View File

@@ -100,7 +100,7 @@ impl<T: AsyncRead> Session<T> {
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<T: AsyncRead> Session<T> {
// 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

View File

@@ -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

View File

@@ -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<AccessToken> {
let mut principal = self
pub async fn authenticate_plain(
&self,
username: &str,
secret: &str,
remote_addr: &RemoteAddress,
) -> Option<AccessToken> {
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();
}

View File

@@ -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);

View File

@@ -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)

View File

@@ -72,9 +72,9 @@ impl JMAP {
})
}
pub fn get_anonymous_limiter(&self, addr: RemoteAddress) -> Arc<Mutex<AnonymousLimiter>> {
pub fn get_anonymous_limiter(&self, addr: &RemoteAddress) -> Arc<Mutex<AnonymousLimiter>> {
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()

View File

@@ -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<Option<EncryptionParams>, Cow<str>> {
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" {

View File

@@ -76,7 +76,7 @@ impl<T: AsyncRead + AsyncWrite + IsTls> Session<T> {
};
// 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<T: AsyncRead + AsyncWrite + IsTls> Session<T> {
// 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

View File

@@ -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

View File

@@ -64,7 +64,7 @@ pub async fn test(server: Arc<JMAP>, 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<JMAP>, 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()