Add configuration for rate-limiting of logins, replacing hardcoded limits (#3090)

Author: reivilibre

Cargo.lock

@@ -197,14 +197,18 @@ impl Options {
         let activity_tracker = ActivityTracker::new(pool.clone(), Duration::from_secs(60));
         let trusted_proxies = config.http.trusted_proxies.clone();
 
+        // Build a rate limiter.
+        // This should not raise an error here as the config should already have been
+        // validated.
+        let limiter = Limiter::new(&config.rate_limiting)
+            .context("rate-limiting configuration is not valid")?;
+
         // Explicitly the config to properly zeroize secret keys
         drop(config);
 
         // Listen for SIGHUP
         register_sighup(&templates, &activity_tracker)?;
 
-        let limiter = Limiter::default();
-
         limiter.start();
 
         let graphql_schema = mas_handlers::graphql_schema(

crates/cli/src/commands/server.rs

@@ -38,6 +38,8 @@ rand_chacha = "0.3.1"
 
 indoc = "2.0.5"
 
+governor.workspace = true
+
 mas-jose.workspace = true
 mas-keystore.workspace = true
 mas-iana.workspace = true

crates/config/Cargo.toml

@@ -27,6 +27,7 @@ mod http;
 mod matrix;
 mod passwords;
 mod policy;
+mod rate_limiting;
 mod secrets;
 mod telemetry;
 mod templates;
@@ -47,6 +48,7 @@ pub use self::{
     matrix::MatrixConfig,
     passwords::{Algorithm as PasswordAlgorithm, PasswordsConfig},
     policy::PolicyConfig,
+    rate_limiting::RateLimitingConfig,
     secrets::SecretsConfig,
     telemetry::{
         MetricsConfig, MetricsExporterKind, Propagator, TelemetryConfig, TracingConfig,
@@ -103,6 +105,11 @@ pub struct RootConfig {
     #[serde(default, skip_serializing_if = "PolicyConfig::is_default")]
     pub policy: PolicyConfig,
 
+    /// Configuration related to limiting the rate of user actions to prevent
+    /// abuse
+    #[serde(default, skip_serializing_if = "RateLimitingConfig::is_default")]
+    pub rate_limiting: RateLimitingConfig,
+
     /// Configuration related to upstream OAuth providers
     #[serde(default, skip_serializing_if = "UpstreamOAuth2Config::is_default")]
     pub upstream_oauth2: UpstreamOAuth2Config,
@@ -137,6 +144,7 @@ impl ConfigurationSection for RootConfig {
         self.secrets.validate(figment)?;
         self.matrix.validate(figment)?;
         self.policy.validate(figment)?;
+        self.rate_limiting.validate(figment)?;
         self.upstream_oauth2.validate(figment)?;
         self.branding.validate(figment)?;
         self.captcha.validate(figment)?;
@@ -168,6 +176,7 @@ impl RootConfig {
             secrets: SecretsConfig::generate(&mut rng).await?,
             matrix: MatrixConfig::generate(&mut rng),
             policy: PolicyConfig::default(),
+            rate_limiting: RateLimitingConfig::default(),
             upstream_oauth2: UpstreamOAuth2Config::default(),
             branding: BrandingConfig::default(),
             captcha: CaptchaConfig::default(),
@@ -190,6 +199,7 @@ impl RootConfig {
             secrets: SecretsConfig::test(),
             matrix: MatrixConfig::test(),
             policy: PolicyConfig::default(),
+            rate_limiting: RateLimitingConfig::default(),
             upstream_oauth2: UpstreamOAuth2Config::default(),
             branding: BrandingConfig::default(),
             captcha: CaptchaConfig::default(),
@@ -225,6 +235,9 @@ pub struct AppConfig {
     #[serde(default)]
     pub policy: PolicyConfig,
 
+    #[serde(default)]
+    pub rate_limiting: RateLimitingConfig,
+
     #[serde(default)]
     pub branding: BrandingConfig,
 
@@ -248,6 +261,7 @@ impl ConfigurationSection for AppConfig {
         self.secrets.validate(figment)?;
         self.matrix.validate(figment)?;
         self.policy.validate(figment)?;
+        self.rate_limiting.validate(figment)?;
         self.branding.validate(figment)?;
         self.captcha.validate(figment)?;
         self.account.validate(figment)?;

crates/config/src/sections/mod.rs

@@ -0,0 +1,152 @@
+// Copyright 2024 The Matrix.org Foundation C.I.C.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::{num::NonZeroU32, time::Duration};
+
+use governor::Quota;
+use schemars::JsonSchema;
+use serde::{de::Error as _, Deserialize, Serialize};
+
+use crate::ConfigurationSection;
+
+/// Configuration related to sending emails
+#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
+pub struct RateLimitingConfig {
+    /// Login-specific rate limits
+    #[serde(default)]
+    pub login: LoginRateLimitingConfig,
+}
+
+#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
+pub struct LoginRateLimitingConfig {
+    /// Controls how many login attempts are permitted
+    /// based on source address.
+    /// This can protect against brute force login attempts.
+    ///
+    /// Note: this limit also applies to password checks when a user attempts to
+    /// change their own password.
+    #[serde(default = "default_login_per_address")]
+    pub per_address: RateLimiterConfiguration,
+    /// Controls how many login attempts are permitted
+    /// based on the account that is being attempted to be logged into.
+    /// This can protect against a distributed brute force attack
+    /// but should be set high enough to prevent someone's account being
+    /// casually locked out.
+    ///
+    /// Note: this limit also applies to password checks when a user attempts to
+    /// change their own password.
+    #[serde(default = "default_login_per_account")]
+    pub per_account: RateLimiterConfiguration,
+}
+
+#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
+pub struct RateLimiterConfiguration {
+    /// A one-off burst of actions that the user can perform
+    /// in one go without waiting.
+    pub burst: NonZeroU32,
+    /// How quickly the allowance replenishes, in number of actions per second.
+    /// Can be fractional to replenish slower.
+    pub per_second: f64,
+}
+
+impl ConfigurationSection for RateLimitingConfig {
+    const PATH: Option<&'static str> = Some("rate_limiting");
+
+    fn validate(&self, figment: &figment::Figment) -> Result<(), figment::Error> {
+        let metadata = figment.find_metadata(Self::PATH.unwrap());
+
+        let error_on_nested_field =
+            |mut error: figment::error::Error, container: &'static str, field: &'static str| {
+                error.metadata = metadata.cloned();
+                error.profile = Some(figment::Profile::Default);
+                error.path = vec![
+                    Self::PATH.unwrap().to_owned(),
+                    container.to_owned(),
+                    field.to_owned(),
+                ];
+                error
+            };
+
+        // Check one limiter's configuration for errors
+        let error_on_limiter =
+            |limiter: &RateLimiterConfiguration| -> Option<figment::error::Error> {
+                let recip = limiter.per_second.recip();
+                // period must be at least 1 nanosecond according to the governor library
+                if recip < 1.0e-9 || !recip.is_finite() {
+                    return Some(figment::error::Error::custom(
+                        "`per_second` must be a number that is more than zero and less than 1_000_000_000 (1e9)",
+                    ));
+                }
+
+                None
+            };
+
+        if let Some(error) = error_on_limiter(&self.login.per_address) {
+            return Err(error_on_nested_field(error, "login", "per_address"));
+        }
+        if let Some(error) = error_on_limiter(&self.login.per_account) {
+            return Err(error_on_nested_field(error, "login", "per_account"));
+        }
+
+        Ok(())
+    }
+}
+
+impl RateLimitingConfig {
+    pub(crate) fn is_default(config: &RateLimitingConfig) -> bool {
+        config == &RateLimitingConfig::default()
+    }
+}
+
+impl RateLimiterConfiguration {
+    pub fn to_quota(self) -> Option<Quota> {
+        let reciprocal = self.per_second.recip();
+        if !reciprocal.is_finite() {
+            return None;
+        }
+        Some(Quota::with_period(Duration::from_secs_f64(reciprocal))?.allow_burst(self.burst))
+    }
+}
+
+fn default_login_per_address() -> RateLimiterConfiguration {
+    RateLimiterConfiguration {
+        burst: NonZeroU32::new(3).unwrap(),
+        per_second: 3.0 / 60.0,
+    }
+}
+
+fn default_login_per_account() -> RateLimiterConfiguration {
+    RateLimiterConfiguration {
+        burst: NonZeroU32::new(1800).unwrap(),
+        per_second: 1800.0 / 3600.0,
+    }
+}
+
+#[allow(clippy::derivable_impls)] // when we add some top-level ratelimiters this will not be derivable anymore
+impl Default for RateLimitingConfig {
+    fn default() -> Self {
+        RateLimitingConfig {
+            login: LoginRateLimitingConfig::default(),
+        }
+    }
+}
+
+impl Default for LoginRateLimitingConfig {
+    fn default() -> Self {
+        LoginRateLimitingConfig {
+            per_address: default_login_per_address(),
+            per_account: default_login_per_account(),
+        }
+    }
+}

crates/config/src/sections/rate_limiting.rs

@@ -81,6 +81,7 @@ headers.workspace = true
 ulid.workspace = true
 
 mas-axum-utils.workspace = true
+mas-config.workspace = true
 mas-data-model.workspace = true
 mas-http.workspace = true
 mas-i18n.workspace = true

crates/handlers/Cargo.toml

@@ -14,14 +14,11 @@
 
 use std::{net::IpAddr, sync::Arc, time::Duration};
 
-use governor::{clock::QuantaClock, state::keyed::DashMapStateStore, Quota, RateLimiter};
+use governor::{clock::QuantaClock, state::keyed::DashMapStateStore, RateLimiter};
+use mas_config::RateLimitingConfig;
 use mas_data_model::User;
-use nonzero_ext::nonzero;
 use ulid::Ulid;
 
-const PASSWORD_CHECK_FOR_REQUESTER_QUOTA: Quota = Quota::per_minute(nonzero!(3u32));
-const PASSWORD_CHECK_FOR_USER_QUOTA: Quota = Quota::per_hour(nonzero!(1800u32));
-
 #[derive(Debug, Clone, Copy, thiserror::Error)]
 pub enum PasswordCheckLimitedError {
     #[error("Too many password checks for requester {0}")]
@@ -60,7 +57,7 @@ impl RequesterFingerprint {
 }
 
 /// Rate limiters for the different operations
-#[derive(Debug, Clone, Default)]
+#[derive(Debug, Clone)]
 pub struct Limiter {
     inner: Arc<LimiterInner>,
 }
@@ -73,16 +70,27 @@ struct LimiterInner {
     password_check_for_user: KeyedRateLimiter<Ulid>,
 }
 
-impl Default for LimiterInner {
-    fn default() -> Self {
-        Self {
-            password_check_for_requester: RateLimiter::keyed(PASSWORD_CHECK_FOR_REQUESTER_QUOTA),
-            password_check_for_user: RateLimiter::keyed(PASSWORD_CHECK_FOR_USER_QUOTA),
-        }
+impl LimiterInner {
+    fn new(config: &RateLimitingConfig) -> Option<Self> {
+        Some(Self {
+            password_check_for_requester: RateLimiter::keyed(config.login.per_address.to_quota()?),
+            password_check_for_user: RateLimiter::keyed(config.login.per_account.to_quota()?),
+        })
     }
 }
 
 impl Limiter {
+    /// Creates a new `Limiter` based on a `RateLimitingConfig`.
+    ///
+    /// If the config is not valid, returns `None`.
+    /// (This should not happen if the config was validated, though.)
+    #[must_use]
+    pub fn new(config: &RateLimitingConfig) -> Option<Self> {
+        Some(Self {
+            inner: Arc::new(LimiterInner::new(config)?),
+        })
+    }
+
     /// Start the rate limiter housekeeping task
     ///
     /// This task will periodically remove old entries from the rate limiters,
@@ -142,7 +150,7 @@ mod tests {
         let now = MockClock::default().now();
         let mut rng = rand_chacha::ChaChaRng::seed_from_u64(42);
 
-        let limiter = Limiter::default();
+        let limiter = Limiter::new(&RateLimitingConfig::default()).unwrap();
 
         // Let's create a lot of requesters to test account-level rate limiting
         let requesters: [_; 768] = (0..=255)

crates/handlers/src/rate_limit.rs

@@ -37,6 +37,7 @@ use mas_axum_utils::{
     http_client_factory::HttpClientFactory,
     ErrorWrapper,
 };
+use mas_config::RateLimitingConfig;
 use mas_data_model::SiteConfig;
 use mas_i18n::Translator;
 use mas_keystore::{Encrypter, JsonWebKey, JsonWebKeySet, Keystore, PrivateKey};
@@ -214,7 +215,7 @@ impl TestState {
         let activity_tracker =
             ActivityTracker::new(pool.clone(), std::time::Duration::from_secs(1));
 
-        let limiter = Limiter::default();
+        let limiter = Limiter::new(&RateLimitingConfig::default()).unwrap();
 
         Ok(Self {
             pool,