Skip to content
This repository was archived by the owner on Sep 10, 2024. It is now read-only.

Commit b2e24f3

Browse files
committed
Add configuration for rate-limiting of logins, replacing hardcoded limits
1 parent 9ea77a9 commit b2e24f3

File tree

10 files changed

+317
-16
lines changed

10 files changed

+317
-16
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/cli/src/commands/server.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,18 @@ impl Options {
197197
let activity_tracker = ActivityTracker::new(pool.clone(), Duration::from_secs(60));
198198
let trusted_proxies = config.http.trusted_proxies.clone();
199199

200+
// Build a rate limiter.
201+
// This should not raise an error here as the config should already have been
202+
// validated.
203+
let limiter = Limiter::new(&config.rate_limiting)
204+
.context("rate-limiting configuration is not valid")?;
205+
200206
// Explicitly the config to properly zeroize secret keys
201207
drop(config);
202208

203209
// Listen for SIGHUP
204210
register_sighup(&templates, &activity_tracker)?;
205211

206-
let limiter = Limiter::default();
207-
208212
limiter.start();
209213

210214
let graphql_schema = mas_handlers::graphql_schema(

crates/config/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ rand_chacha = "0.3.1"
3838

3939
indoc = "2.0.5"
4040

41+
governor.workspace = true
42+
4143
mas-jose.workspace = true
4244
mas-keystore.workspace = true
4345
mas-iana.workspace = true

crates/config/src/sections/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ mod http;
2727
mod matrix;
2828
mod passwords;
2929
mod policy;
30+
mod rate_limiting;
3031
mod secrets;
3132
mod telemetry;
3233
mod templates;
@@ -47,6 +48,7 @@ pub use self::{
4748
matrix::MatrixConfig,
4849
passwords::{Algorithm as PasswordAlgorithm, PasswordsConfig},
4950
policy::PolicyConfig,
51+
rate_limiting::RateLimitingConfig,
5052
secrets::SecretsConfig,
5153
telemetry::{
5254
MetricsConfig, MetricsExporterKind, Propagator, TelemetryConfig, TracingConfig,
@@ -103,6 +105,11 @@ pub struct RootConfig {
103105
#[serde(default, skip_serializing_if = "PolicyConfig::is_default")]
104106
pub policy: PolicyConfig,
105107

108+
/// Configuration related to limiting the rate of user actions to prevent
109+
/// abuse
110+
#[serde(default, skip_serializing_if = "RateLimitingConfig::is_default")]
111+
pub rate_limiting: RateLimitingConfig,
112+
106113
/// Configuration related to upstream OAuth providers
107114
#[serde(default, skip_serializing_if = "UpstreamOAuth2Config::is_default")]
108115
pub upstream_oauth2: UpstreamOAuth2Config,
@@ -137,6 +144,7 @@ impl ConfigurationSection for RootConfig {
137144
self.secrets.validate(figment)?;
138145
self.matrix.validate(figment)?;
139146
self.policy.validate(figment)?;
147+
self.rate_limiting.validate(figment)?;
140148
self.upstream_oauth2.validate(figment)?;
141149
self.branding.validate(figment)?;
142150
self.captcha.validate(figment)?;
@@ -168,6 +176,7 @@ impl RootConfig {
168176
secrets: SecretsConfig::generate(&mut rng).await?,
169177
matrix: MatrixConfig::generate(&mut rng),
170178
policy: PolicyConfig::default(),
179+
rate_limiting: RateLimitingConfig::default(),
171180
upstream_oauth2: UpstreamOAuth2Config::default(),
172181
branding: BrandingConfig::default(),
173182
captcha: CaptchaConfig::default(),
@@ -190,6 +199,7 @@ impl RootConfig {
190199
secrets: SecretsConfig::test(),
191200
matrix: MatrixConfig::test(),
192201
policy: PolicyConfig::default(),
202+
rate_limiting: RateLimitingConfig::default(),
193203
upstream_oauth2: UpstreamOAuth2Config::default(),
194204
branding: BrandingConfig::default(),
195205
captcha: CaptchaConfig::default(),
@@ -225,6 +235,9 @@ pub struct AppConfig {
225235
#[serde(default)]
226236
pub policy: PolicyConfig,
227237

238+
#[serde(default)]
239+
pub rate_limiting: RateLimitingConfig,
240+
228241
#[serde(default)]
229242
pub branding: BrandingConfig,
230243

@@ -248,6 +261,7 @@ impl ConfigurationSection for AppConfig {
248261
self.secrets.validate(figment)?;
249262
self.matrix.validate(figment)?;
250263
self.policy.validate(figment)?;
264+
self.rate_limiting.validate(figment)?;
251265
self.branding.validate(figment)?;
252266
self.captcha.validate(figment)?;
253267
self.account.validate(figment)?;
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
// Copyright 2024 The Matrix.org Foundation C.I.C.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use std::{num::NonZero, time::Duration};
16+
17+
use governor::Quota;
18+
use schemars::JsonSchema;
19+
use serde::{de::Error as _, Deserialize, Serialize};
20+
21+
use crate::ConfigurationSection;
22+
23+
/// Configuration related to sending emails
24+
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
25+
pub struct RateLimitingConfig {
26+
/// Login-specific rate limits
27+
#[serde(default)]
28+
pub login: LoginRateLimitingConfig,
29+
}
30+
31+
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
32+
pub struct LoginRateLimitingConfig {
33+
/// Controls how many login attempts are permitted
34+
/// based on source address.
35+
/// This can protect against brute force login attempts.
36+
///
37+
/// Note: this limit also applies to password checks when a user attempts to
38+
/// change their own password.
39+
#[serde(default = "default_login_per_address")]
40+
pub per_address: RateLimiterConfiguration,
41+
/// Controls how many login attempts are permitted
42+
/// based on the account that is being attempted to be logged into.
43+
/// This can protect against a distributed brute force attack
44+
/// but should be set high enough to prevent someone's account being
45+
/// casually locked out.
46+
///
47+
/// Note: this limit also applies to password checks when a user attempts to
48+
/// change their own password.
49+
#[serde(default = "default_login_per_account")]
50+
pub per_account: RateLimiterConfiguration,
51+
}
52+
53+
#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
54+
pub struct RateLimiterConfiguration {
55+
/// A one-off burst of actions that the user can perform
56+
/// in one go without waiting.
57+
/// Replenishes at the rate.
58+
pub burst: u32,
59+
/// How quickly the allowance replenishes, in number of actions per second.
60+
/// Can be fractional to replenish slower.
61+
pub per_second: f64,
62+
}
63+
64+
impl ConfigurationSection for RateLimitingConfig {
65+
const PATH: Option<&'static str> = Some("rate_limiting");
66+
67+
fn validate(&self, figment: &figment::Figment) -> Result<(), figment::Error> {
68+
let metadata = figment.find_metadata(Self::PATH.unwrap());
69+
70+
let error_on_nested_field =
71+
|mut error: figment::error::Error, container: &'static str, field: &'static str| {
72+
error.metadata = metadata.cloned();
73+
error.profile = Some(figment::Profile::Default);
74+
error.path = vec![
75+
Self::PATH.unwrap().to_owned(),
76+
container.to_owned(),
77+
field.to_owned(),
78+
];
79+
error
80+
};
81+
82+
// Check one limiter's configuration for errors
83+
let error_on_limiter =
84+
|limiter: &RateLimiterConfiguration| -> Option<figment::error::Error> {
85+
if limiter.burst == 0 {
86+
return Some(figment::error::Error::custom("`burst` must not be zero, as this would mean the action could never be performed"));
87+
}
88+
89+
let recip = limiter.per_second.recip();
90+
// period must be at least 1 nanosecond according to the governor library
91+
if recip < 1.0e-9 || !recip.is_finite() {
92+
return Some(figment::error::Error::custom(
93+
"`per_second` must be a number that is more than zero and less than 1_000_000_000 (1e9)",
94+
));
95+
}
96+
97+
None
98+
};
99+
100+
if let Some(error) = error_on_limiter(&self.login.per_address) {
101+
return Err(error_on_nested_field(error, "login", "per_address"));
102+
}
103+
if let Some(error) = error_on_limiter(&self.login.per_account) {
104+
return Err(error_on_nested_field(error, "login", "per_account"));
105+
}
106+
107+
Ok(())
108+
}
109+
}
110+
111+
impl RateLimitingConfig {
112+
pub(crate) fn is_default(config: &RateLimitingConfig) -> bool {
113+
config == &Default::default()
114+
}
115+
}
116+
117+
impl RateLimiterConfiguration {
118+
pub fn to_quota(&self) -> Option<Quota> {
119+
let reciprocal = self.per_second.recip();
120+
if !reciprocal.is_finite() {
121+
return None;
122+
}
123+
let burst = NonZero::new(self.burst)?;
124+
Some(Quota::with_period(Duration::from_secs_f64(reciprocal))?.allow_burst(burst))
125+
}
126+
}
127+
128+
fn default_login_per_address() -> RateLimiterConfiguration {
129+
RateLimiterConfiguration {
130+
burst: 3,
131+
per_second: 3.0 / 60.0,
132+
}
133+
}
134+
135+
fn default_login_per_account() -> RateLimiterConfiguration {
136+
RateLimiterConfiguration {
137+
burst: 1800,
138+
per_second: 1800.0 / 3600.0,
139+
}
140+
}
141+
142+
impl Default for RateLimitingConfig {
143+
fn default() -> Self {
144+
RateLimitingConfig {
145+
// registration: default_registration(),
146+
login: Default::default(),
147+
}
148+
}
149+
}
150+
151+
impl Default for LoginRateLimitingConfig {
152+
fn default() -> Self {
153+
LoginRateLimitingConfig {
154+
per_address: default_login_per_address(),
155+
per_account: default_login_per_account(),
156+
}
157+
}
158+
}

crates/handlers/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ headers.workspace = true
8181
ulid.workspace = true
8282

8383
mas-axum-utils.workspace = true
84+
mas-config.workspace = true
8485
mas-data-model.workspace = true
8586
mas-http.workspace = true
8687
mas-i18n.workspace = true

crates/handlers/src/rate_limit.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,11 @@
1414

1515
use std::{net::IpAddr, sync::Arc, time::Duration};
1616

17-
use governor::{clock::QuantaClock, state::keyed::DashMapStateStore, Quota, RateLimiter};
17+
use governor::{clock::QuantaClock, state::keyed::DashMapStateStore, RateLimiter};
18+
use mas_config::RateLimitingConfig;
1819
use mas_data_model::User;
19-
use nonzero_ext::nonzero;
2020
use ulid::Ulid;
2121

22-
const PASSWORD_CHECK_FOR_REQUESTER_QUOTA: Quota = Quota::per_minute(nonzero!(3u32));
23-
const PASSWORD_CHECK_FOR_USER_QUOTA: Quota = Quota::per_hour(nonzero!(1800u32));
24-
2522
#[derive(Debug, Clone, Copy, thiserror::Error)]
2623
pub enum PasswordCheckLimitedError {
2724
#[error("Too many password checks for requester {0}")]
@@ -60,7 +57,7 @@ impl RequesterFingerprint {
6057
}
6158

6259
/// Rate limiters for the different operations
63-
#[derive(Debug, Clone, Default)]
60+
#[derive(Debug, Clone)]
6461
pub struct Limiter {
6562
inner: Arc<LimiterInner>,
6663
}
@@ -73,16 +70,26 @@ struct LimiterInner {
7370
password_check_for_user: KeyedRateLimiter<Ulid>,
7471
}
7572

76-
impl Default for LimiterInner {
77-
fn default() -> Self {
78-
Self {
79-
password_check_for_requester: RateLimiter::keyed(PASSWORD_CHECK_FOR_REQUESTER_QUOTA),
80-
password_check_for_user: RateLimiter::keyed(PASSWORD_CHECK_FOR_USER_QUOTA),
81-
}
73+
impl LimiterInner {
74+
fn new(config: &RateLimitingConfig) -> Option<Self> {
75+
Some(Self {
76+
password_check_for_requester: RateLimiter::keyed(config.login.per_address.to_quota()?),
77+
password_check_for_user: RateLimiter::keyed(config.login.per_account.to_quota()?),
78+
})
8279
}
8380
}
8481

8582
impl Limiter {
83+
/// Creates a new `Limiter` based on a `RateLimitingConfig`.
84+
///
85+
/// If the config is not valid, returns `None`.
86+
/// (This should not happen if the config was validated, though.)
87+
pub fn new(config: &RateLimitingConfig) -> Option<Self> {
88+
Some(Self {
89+
inner: Arc::new(LimiterInner::new(config)?),
90+
})
91+
}
92+
8693
/// Start the rate limiter housekeeping task
8794
///
8895
/// This task will periodically remove old entries from the rate limiters,
@@ -142,7 +149,7 @@ mod tests {
142149
let now = MockClock::default().now();
143150
let mut rng = rand_chacha::ChaChaRng::seed_from_u64(42);
144151

145-
let limiter = Limiter::default();
152+
let limiter = Limiter::new(&Default::default()).unwrap();
146153

147154
// Let's create a lot of requesters to test account-level rate limiting
148155
let requesters: [_; 768] = (0..=255)

crates/handlers/src/test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ impl TestState {
214214
let activity_tracker =
215215
ActivityTracker::new(pool.clone(), std::time::Duration::from_secs(1));
216216

217-
let limiter = Limiter::default();
217+
let limiter = Limiter::new(&Default::default()).unwrap();
218218

219219
Ok(Self {
220220
pool,

0 commit comments

Comments
 (0)