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

Commit 244f8f5

Browse files
authored
Add configuration for rate-limiting of logins, replacing hardcoded limits (#3090)
1 parent 1bdad26 commit 244f8f5

File tree

10 files changed

+313
-16
lines changed

10 files changed

+313
-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: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
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::NonZeroU32, 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+
pub burst: NonZeroU32,
58+
/// How quickly the allowance replenishes, in number of actions per second.
59+
/// Can be fractional to replenish slower.
60+
pub per_second: f64,
61+
}
62+
63+
impl ConfigurationSection for RateLimitingConfig {
64+
const PATH: Option<&'static str> = Some("rate_limiting");
65+
66+
fn validate(&self, figment: &figment::Figment) -> Result<(), figment::Error> {
67+
let metadata = figment.find_metadata(Self::PATH.unwrap());
68+
69+
let error_on_nested_field =
70+
|mut error: figment::error::Error, container: &'static str, field: &'static str| {
71+
error.metadata = metadata.cloned();
72+
error.profile = Some(figment::Profile::Default);
73+
error.path = vec![
74+
Self::PATH.unwrap().to_owned(),
75+
container.to_owned(),
76+
field.to_owned(),
77+
];
78+
error
79+
};
80+
81+
// Check one limiter's configuration for errors
82+
let error_on_limiter =
83+
|limiter: &RateLimiterConfiguration| -> Option<figment::error::Error> {
84+
let recip = limiter.per_second.recip();
85+
// period must be at least 1 nanosecond according to the governor library
86+
if recip < 1.0e-9 || !recip.is_finite() {
87+
return Some(figment::error::Error::custom(
88+
"`per_second` must be a number that is more than zero and less than 1_000_000_000 (1e9)",
89+
));
90+
}
91+
92+
None
93+
};
94+
95+
if let Some(error) = error_on_limiter(&self.login.per_address) {
96+
return Err(error_on_nested_field(error, "login", "per_address"));
97+
}
98+
if let Some(error) = error_on_limiter(&self.login.per_account) {
99+
return Err(error_on_nested_field(error, "login", "per_account"));
100+
}
101+
102+
Ok(())
103+
}
104+
}
105+
106+
impl RateLimitingConfig {
107+
pub(crate) fn is_default(config: &RateLimitingConfig) -> bool {
108+
config == &RateLimitingConfig::default()
109+
}
110+
}
111+
112+
impl RateLimiterConfiguration {
113+
pub fn to_quota(self) -> Option<Quota> {
114+
let reciprocal = self.per_second.recip();
115+
if !reciprocal.is_finite() {
116+
return None;
117+
}
118+
Some(Quota::with_period(Duration::from_secs_f64(reciprocal))?.allow_burst(self.burst))
119+
}
120+
}
121+
122+
fn default_login_per_address() -> RateLimiterConfiguration {
123+
RateLimiterConfiguration {
124+
burst: NonZeroU32::new(3).unwrap(),
125+
per_second: 3.0 / 60.0,
126+
}
127+
}
128+
129+
fn default_login_per_account() -> RateLimiterConfiguration {
130+
RateLimiterConfiguration {
131+
burst: NonZeroU32::new(1800).unwrap(),
132+
per_second: 1800.0 / 3600.0,
133+
}
134+
}
135+
136+
#[allow(clippy::derivable_impls)] // when we add some top-level ratelimiters this will not be derivable anymore
137+
impl Default for RateLimitingConfig {
138+
fn default() -> Self {
139+
RateLimitingConfig {
140+
login: LoginRateLimitingConfig::default(),
141+
}
142+
}
143+
}
144+
145+
impl Default for LoginRateLimitingConfig {
146+
fn default() -> Self {
147+
LoginRateLimitingConfig {
148+
per_address: default_login_per_address(),
149+
per_account: default_login_per_account(),
150+
}
151+
}
152+
}

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: 21 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,27 @@ 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+
#[must_use]
88+
pub fn new(config: &RateLimitingConfig) -> Option<Self> {
89+
Some(Self {
90+
inner: Arc::new(LimiterInner::new(config)?),
91+
})
92+
}
93+
8694
/// Start the rate limiter housekeeping task
8795
///
8896
/// This task will periodically remove old entries from the rate limiters,
@@ -142,7 +150,7 @@ mod tests {
142150
let now = MockClock::default().now();
143151
let mut rng = rand_chacha::ChaChaRng::seed_from_u64(42);
144152

145-
let limiter = Limiter::default();
153+
let limiter = Limiter::new(&RateLimitingConfig::default()).unwrap();
146154

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

crates/handlers/src/test_utils.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use mas_axum_utils::{
3737
http_client_factory::HttpClientFactory,
3838
ErrorWrapper,
3939
};
40+
use mas_config::RateLimitingConfig;
4041
use mas_data_model::SiteConfig;
4142
use mas_i18n::Translator;
4243
use mas_keystore::{Encrypter, JsonWebKey, JsonWebKeySet, Keystore, PrivateKey};
@@ -214,7 +215,7 @@ impl TestState {
214215
let activity_tracker =
215216
ActivityTracker::new(pool.clone(), std::time::Duration::from_secs(1));
216217

217-
let limiter = Limiter::default();
218+
let limiter = Limiter::new(&RateLimitingConfig::default()).unwrap();
218219

219220
Ok(Self {
220221
pool,

0 commit comments

Comments
 (0)