Skip to content

Commit

Permalink
Removed authentication rate limit (unnecessary since there is fail2ban)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdecimus
mdecimus committed Dec 27, 2024
1 parent 7a905ca commit c7499ab
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 144 deletions.
4 changes: 0 additions & 4 deletions crates/common/src/config/jmap/settings.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ pub struct JmapConfig {

pub session_cache_ttl: Duration,
pub rate_authenticated: Option<Rate>,
pub rate_authenticate_req: Option<Rate>,
pub rate_anonymous: Option<Rate>,

pub event_source_throttle: Duration,
Expand Down Expand Up @@ -305,9 +304,6 @@ impl JmapConfig {
rate_authenticated: config
.property_or_default::<Option<Rate>>("jmap.rate-limit.account", "1000/1m")
.unwrap_or_default(),
rate_authenticate_req: config
.property_or_default::<Option<Rate>>("authentication.rate-limit", "10/1m")
.unwrap_or_default(),
rate_anonymous: config
.property_or_default::<Option<Rate>>("jmap.rate-limit.anonymous", "100/1m")
.unwrap_or_default(),
Expand Down
7 changes: 3 additions & 4 deletions crates/common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ pub const KV_RATE_LIMIT_LOITER: u8 = 4;
pub const KV_RATE_LIMIT_AUTH: u8 = 5;
pub const KV_RATE_LIMIT_HASH: u8 = 6;
pub const KV_RATE_LIMIT_CONTACT: u8 = 7;
pub const KV_RATE_LIMIT_JMAP: u8 = 8;
pub const KV_RATE_LIMIT_JMAP_AUTH: u8 = 9;
pub const KV_RATE_LIMIT_HTTP_ANONYM: u8 = 10;
pub const KV_RATE_LIMIT_IMAP: u8 = 11;
pub const KV_RATE_LIMIT_HTTP_AUTHENTICATED: u8 = 8;
pub const KV_RATE_LIMIT_HTTP_ANONYMOUS: u8 = 9;
pub const KV_RATE_LIMIT_IMAP: u8 = 10;
pub const KV_REPUTATION_IP: u8 = 12;
pub const KV_REPUTATION_FROM: u8 = 13;
pub const KV_REPUTATION_DOMAIN: u8 = 14;
Expand Down
7 changes: 0 additions & 7 deletions crates/imap/src/op/authenticate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use imap_proto::{
receiver::{self, Request},
Command, ResponseCode, StatusResponse,
};
use jmap::auth::rate_limit::RateLimiter;
use mail_parser::decoders::base64::base64_decode;
use mail_send::Credentials;
use std::sync::Arc;
Expand Down Expand Up @@ -76,12 +75,6 @@ impl<T: SessionStream> Session<T> {
credentials: Credentials<String>,
tag: String,
) -> trc::Result<()> {
// Throttle authentication requests
self.server
.is_auth_allowed_soft(&self.remote_addr)
.await
.map_err(|err| err.id(tag.clone()))?;

// Authenticate
let access_token = self
.server
Expand Down
74 changes: 55 additions & 19 deletions crates/jmap/src/api/http.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,15 @@ impl ParseHttp for Server {
}
("oauth-authorization-server", &Method::GET) => {
// Limit anonymous requests
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_oauth_metadata(req, session).await;
}
("openid-configuration", &Method::GET) => {
// Limit anonymous requests
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_oidc_metadata(req, session).await;
}
Expand All @@ -258,6 +260,10 @@ impl ParseHttp for Server {
}
}
("mta-sts.txt", &Method::GET) => {
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return if let Some(policy) = self.build_mta_sts_policy() {
Ok(Resource::new("text/plain", policy.to_string().into_bytes())
.into_http_response())
Expand All @@ -266,12 +272,20 @@ impl ParseHttp for Server {
};
}
("mail-v1.xml", &Method::GET) => {
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_autoconfig_request(&req).await;
}
("autoconfig", &Method::GET) => {
if path.next().unwrap_or_default() == "mail"
&& path.next().unwrap_or_default() == "config-v1.1.xml"
{
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_autoconfig_request(&req).await;
}
}
Expand All @@ -282,12 +296,14 @@ impl ParseHttp for Server {
},
"auth" => match (path.next().unwrap_or_default(), req.method()) {
("device", &Method::POST) => {
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_device_auth(&mut req, session).await;
}
("token", &Method::POST) => {
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_token_request(&mut req, session).await;
}
Expand All @@ -314,7 +330,8 @@ impl ParseHttp for Server {
}
("jwks.json", &Method::GET) => {
// Limit anonymous requests
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return Ok(self.core.oauth.oidc_jwks.clone().into_http_response());
}
Expand Down Expand Up @@ -408,13 +425,21 @@ impl ParseHttp for Server {
if req.method() == Method::GET
&& path.next().unwrap_or_default() == "config-v1.1.xml"
{
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self.handle_autoconfig_request(&req).await;
}
}
"autodiscover" => {
if req.method() == Method::POST
&& path.next().unwrap_or_default() == "autodiscover.xml"
{
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return self
.handle_autodiscover_request(
fetch_body(&mut req, 8192, session.session_id).await,
Expand All @@ -423,27 +448,37 @@ impl ParseHttp for Server {
}
}
"robots.txt" => {
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return Ok(
Resource::new("text/plain", b"User-agent: *\nDisallow: /\n".to_vec())
.into_http_response(),
);
}
"healthz" => match path.next().unwrap_or_default() {
"live" => {
return Ok(StatusCode::OK.into_http_response());
}
"ready" => {
return Ok({
if !self.core.storage.data.is_none() {
StatusCode::OK
} else {
StatusCode::SERVICE_UNAVAILABLE
"healthz" => {
// Limit anonymous requests
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

match path.next().unwrap_or_default() {
"live" => {
return Ok(StatusCode::OK.into_http_response());
}
"ready" => {
return Ok({
if !self.core.storage.data.is_none() {
StatusCode::OK
} else {
StatusCode::SERVICE_UNAVAILABLE
}
}
.into_http_response());
}
.into_http_response());
_ => (),
}
_ => (),
},
}
"metrics" => match path.next().unwrap_or_default() {
"prometheus" => {
if let Some(prometheus) = &self.core.metrics.prometheus {
Expand Down Expand Up @@ -508,7 +543,8 @@ impl ParseHttp for Server {
if let Some(form) = &self.core.network.contact_form {
match *req.method() {
Method::POST => {
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

let form_data =
FormData::from_request(&mut req, form.max_size, session.session_id)
Expand Down
7 changes: 4 additions & 3 deletions crates/jmap/src/api/management/stores.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,10 @@ impl ManageStore for Server {
Some("rate-auth") => vec![KV_RATE_LIMIT_AUTH].into(),
Some("rate-hash") => vec![KV_RATE_LIMIT_HASH].into(),
Some("rate-contact") => vec![KV_RATE_LIMIT_CONTACT].into(),
Some("rate-jmap") => vec![KV_RATE_LIMIT_JMAP].into(),
Some("rate-jmap-auth") => vec![KV_RATE_LIMIT_JMAP_AUTH].into(),
Some("rate-http-anonymous") => vec![KV_RATE_LIMIT_HTTP_ANONYM].into(),
Some("rate-http-authenticated") => {
vec![KV_RATE_LIMIT_HTTP_AUTHENTICATED].into()
}
Some("rate-http-anonymous") => vec![KV_RATE_LIMIT_HTTP_ANONYMOUS].into(),
Some("rate-imap") => vec![KV_RATE_LIMIT_IMAP].into(),
Some("reputation-ip") => vec![KV_REPUTATION_IP].into(),
Some("reputation-from") => vec![KV_REPUTATION_FROM].into(),
Expand Down
27 changes: 9 additions & 18 deletions crates/jmap/src/auth/authenticate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ impl Authenticator for Server {
self.get_cached_access_token(account_id).await?
} else {
let credentials = if mechanism.eq_ignore_ascii_case("basic") {
// Throttle authentication requests
self.is_auth_allowed_soft(&session.remote_ip).await?;

// Decode the base64 encoded credentials
decode_plain_auth(token).ok_or_else(|| {
trc::AuthEvent::Error
Expand All @@ -54,7 +51,8 @@ impl Authenticator for Server {
})?
} else if mechanism.eq_ignore_ascii_case("bearer") {
// Enforce anonymous rate limit
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

decode_bearer_token(token, allow_api_access).ok_or_else(|| {
trc::AuthEvent::Error
Expand All @@ -65,7 +63,8 @@ impl Authenticator for Server {
})?
} else {
// Enforce anonymous rate limit
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

return Err(trc::AuthEvent::Error
.into_err()
Expand All @@ -75,35 +74,27 @@ impl Authenticator for Server {
};

// Authenticate
let access_token = match self
let access_token = self
.authenticate(&AuthRequest::from_credentials(
credentials,
session.session_id,
session.remote_ip,
))
.await
{
Ok(access_token) => access_token,
Err(err) => {
if err.matches(trc::EventType::Auth(trc::AuthEvent::Failed)) {
let _ = self.is_auth_allowed_hard(&session.remote_ip).await;
}
return Err(err);
}
};
.await?;

// Cache session
self.cache_session(token.to_string(), &access_token);
access_token
};

// Enforce authenticated rate limit
self.is_account_allowed(&access_token)
self.is_http_authenticated_request_allowed(&access_token)
.await
.map(|in_flight| (in_flight, access_token))
} else {
// Enforce anonymous rate limit
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;

Err(trc::AuthEvent::Failed
.into_err()
Expand Down
3 changes: 2 additions & 1 deletion crates/jmap/src/auth/oauth/registration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ impl ClientRegistrationHandler for Server {
// Validate permissions
access_token.assert_has_permission(Permission::OauthClientRegistration)?;
} else {
self.is_anonymous_allowed(&session.remote_ip).await?;
self.is_http_anonymous_request_allowed(&session.remote_ip)
.await?;
}

// Parse request
Expand Down
Loading

0 comments on commit c7499ab

Please sign in to comment.