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

Rate-limit password-based login attempts #3013

Merged
merged 2 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ features = ["derive"]
version = "0.10.19"
features = ["env", "yaml", "test"]

# Rate-limiting
[workspace.dependencies.governor]
version = "0.6.3"

# HTTP headers
[workspace.dependencies.headers]
version = "0.4.0"
Expand Down Expand Up @@ -164,6 +168,10 @@ features = [
[workspace.dependencies.minijinja]
version = "2.1.0"

# Utilities to deal with non-zero values
[workspace.dependencies.nonzero_ext]
version = "0.3.0"

# Random values
[workspace.dependencies.rand]
version = "0.8.5"
Expand Down
32 changes: 31 additions & 1 deletion crates/cli/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use ipnetwork::IpNetwork;
use mas_data_model::SiteConfig;
use mas_handlers::{
passwords::PasswordManager, ActivityTracker, BoundActivityTracker, CookieManager, ErrorWrapper,
GraphQLSchema, HttpClientFactory, MetadataCache,
GraphQLSchema, HttpClientFactory, Limiter, MetadataCache, RequesterFingerprint,
};
use mas_i18n::Translator;
use mas_keystore::{Encrypter, Keystore};
Expand Down Expand Up @@ -57,6 +57,7 @@ pub struct AppState {
pub site_config: SiteConfig,
pub activity_tracker: ActivityTracker,
pub trusted_proxies: Vec<IpNetwork>,
pub limiter: Limiter,
pub conn_acquisition_histogram: Option<Histogram<u64>>,
}

Expand Down Expand Up @@ -210,6 +211,12 @@ impl FromRef<AppState> for SiteConfig {
}
}

impl FromRef<AppState> for Limiter {
fn from_ref(input: &AppState) -> Self {
input.limiter.clone()
}
}

impl FromRef<AppState> for BoxHomeserverConnection {
fn from_ref(input: &AppState) -> Self {
Box::new(input.homeserver_connection.clone())
Expand Down Expand Up @@ -326,12 +333,35 @@ impl FromRequestParts<AppState> for BoundActivityTracker {
parts: &mut axum::http::request::Parts,
state: &AppState,
) -> Result<Self, Self::Rejection> {
// TODO: we may infer the IP twice, for the activity tracker and the limiter
let ip = infer_client_ip(parts, &state.trusted_proxies);
tracing::debug!(ip = ?ip, "Inferred client IP address");
Ok(state.activity_tracker.clone().bind(ip))
}
}

#[async_trait]
impl FromRequestParts<AppState> for RequesterFingerprint {
type Rejection = Infallible;

async fn from_request_parts(
parts: &mut axum::http::request::Parts,
state: &AppState,
) -> Result<Self, Self::Rejection> {
// TODO: we may infer the IP twice, for the activity tracker and the limiter
let ip = infer_client_ip(parts, &state.trusted_proxies);

if let Some(ip) = ip {
Ok(RequesterFingerprint::new(ip))
} else {
// If we can't infer the IP address, we'll just use an empty fingerprint and
// warn about it
tracing::warn!("Could not infer client IP address for an operation which rate-limits based on IP addresses");
Ok(RequesterFingerprint::EMPTY)
}
}
}

#[async_trait]
impl FromRequestParts<AppState> for BoxRepository {
type Rejection = ErrorWrapper<mas_storage_pg::DatabaseError>;
Expand Down
9 changes: 7 additions & 2 deletions crates/cli/src/commands/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use clap::Parser;
use figment::Figment;
use itertools::Itertools;
use mas_config::{AppConfig, ClientsConfig, ConfigurationSection, UpstreamOAuth2Config};
use mas_handlers::{ActivityTracker, CookieManager, HttpClientFactory, MetadataCache};
use mas_handlers::{ActivityTracker, CookieManager, HttpClientFactory, Limiter, MetadataCache};
use mas_listener::{server::Server, shutdown::ShutdownStream};
use mas_matrix_synapse::SynapseConnection;
use mas_router::UrlBuilder;
Expand Down Expand Up @@ -200,6 +200,10 @@ impl Options {
// Listen for SIGHUP
register_sighup(&templates, &activity_tracker)?;

let limiter = Limiter::default();

limiter.start();

let graphql_schema = mas_handlers::graphql_schema(
&pool,
&policy_factory,
Expand All @@ -213,7 +217,6 @@ impl Options {
pool,
templates,
key_store,
metadata_cache,
cookie_manager,
encrypter,
url_builder,
Expand All @@ -222,9 +225,11 @@ impl Options {
graphql_schema,
http_client_factory,
password_manager,
metadata_cache,
site_config,
activity_tracker,
trusted_proxies,
limiter,
conn_acquisition_histogram: None,
};
s.init_metrics()?;
Expand Down
2 changes: 2 additions & 0 deletions crates/handlers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ zeroize = "1.8.1"
base64ct = "1.6.0"
camino.workspace = true
chrono.workspace = true
governor.workspace = true
indexmap = "2.2.6"
psl = "2.1.55"
time = "0.3.36"
url.workspace = true
mime = "0.3.17"
minijinja.workspace = true
nonzero_ext.workspace = true
rand.workspace = true
rand_chacha = "0.3.1"
headers.workspace = true
Expand Down
Loading
Loading