From f20c8d8ef3254c84efceb8f412f725784d51e1f6 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 20 Sep 2023 17:27:28 +0200 Subject: [PATCH] Infer client IP address from the peer address and the X-Forwarded-Proxy header --- Cargo.lock | 5 ++ crates/axum-utils/src/error_wrapper.rs | 35 +++++++++ crates/axum-utils/src/lib.rs | 2 + crates/cli/Cargo.toml | 3 + crates/{handlers => cli}/src/app_state.rs | 90 +++++++++++++++-------- crates/cli/src/commands/server.rs | 14 ++-- crates/cli/src/main.rs | 1 + crates/cli/src/server.rs | 3 +- crates/config/Cargo.toml | 1 + crates/config/src/sections/http.rs | 18 +++++ crates/handlers/src/lib.rs | 6 +- crates/handlers/src/test_utils.rs | 5 +- docs/config.schema.json | 56 +++++++++++++- 13 files changed, 195 insertions(+), 44 deletions(-) create mode 100644 crates/axum-utils/src/error_wrapper.rs rename crates/{handlers => cli}/src/app_state.rs (79%) diff --git a/Cargo.lock b/Cargo.lock index 64f1c8bd..19eff9df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2405,6 +2405,7 @@ version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf466541e9d546596ee94f9f69590f89473455f88372423e0008fc1a7daf100e" dependencies = [ + "schemars", "serde", ] @@ -2685,14 +2686,17 @@ dependencies = [ "dotenvy", "httpdate", "hyper", + "ipnetwork", "itertools 0.11.0", "listenfd", "mas-config", "mas-data-model", "mas-email", + "mas-graphql", "mas-handlers", "mas-http", "mas-iana", + "mas-keystore", "mas-listener", "mas-matrix", "mas-matrix-synapse", @@ -2744,6 +2748,7 @@ dependencies = [ "chrono", "figment", "indoc", + "ipnetwork", "mas-iana", "mas-jose", "mas-keystore", diff --git a/crates/axum-utils/src/error_wrapper.rs b/crates/axum-utils/src/error_wrapper.rs new file mode 100644 index 00000000..379325e5 --- /dev/null +++ b/crates/axum-utils/src/error_wrapper.rs @@ -0,0 +1,35 @@ +// Copyright 2023 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 axum::response::{IntoResponse, Response}; +use http::StatusCode; + +/// A simple wrapper around an error that implements [`IntoResponse`]. +pub struct ErrorWrapper(pub T); + +impl From for ErrorWrapper { + fn from(input: T) -> Self { + Self(input) + } +} + +impl IntoResponse for ErrorWrapper +where + T: std::error::Error, +{ + fn into_response(self) -> Response { + // TODO: make this a bit more user friendly + (StatusCode::INTERNAL_SERVER_ERROR, self.0.to_string()).into_response() + } +} diff --git a/crates/axum-utils/src/lib.rs b/crates/axum-utils/src/lib.rs index 9174ee12..9336eb08 100644 --- a/crates/axum-utils/src/lib.rs +++ b/crates/axum-utils/src/lib.rs @@ -25,6 +25,7 @@ pub mod client_authorization; pub mod cookies; pub mod csrf; +pub mod error_wrapper; pub mod fancy_error; pub mod http_client_factory; pub mod jwt; @@ -35,6 +36,7 @@ pub mod user_authorization; pub use axum; pub use self::{ + error_wrapper::ErrorWrapper, fancy_error::FancyError, session::{SessionInfo, SessionInfoExt}, }; diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 33e851c9..ed55b6a6 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -15,6 +15,7 @@ clap = { version = "4.4.4", features = ["derive"] } dotenvy = "0.15.7" httpdate = "1.0.3" hyper = { version = "0.14.27", features = ["full"] } +ipnetwork = "0.20.0" itertools = "0.11.0" listenfd = "1.0.1" rand.workspace = true @@ -49,9 +50,11 @@ sentry-tower = { version = "0.31.7", features = ["http"] } mas-config = { path = "../config" } mas-data-model = { path = "../data-model" } mas-email = { path = "../email" } +mas-graphql = { path = "../graphql" } mas-handlers = { path = "../handlers", default-features = false } mas-http = { path = "../http", default-features = false, features = ["axum", "client"] } mas-iana = { path = "../iana" } +mas-keystore = { path = "../keystore" } mas-listener = { path = "../listener" } mas-matrix = { path = "../matrix" } mas-matrix-synapse = { path = "../matrix-synapse" } diff --git a/crates/handlers/src/app_state.rs b/crates/cli/src/app_state.rs similarity index 79% rename from crates/handlers/src/app_state.rs rename to crates/cli/src/app_state.rs index 2df24db8..9b4497e8 100644 --- a/crates/handlers/src/app_state.rs +++ b/crates/cli/src/app_state.rs @@ -1,4 +1,4 @@ -// Copyright 2022 The Matrix.org Foundation C.I.C. +// Copyright 2022-2023 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. @@ -12,15 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{convert::Infallible, sync::Arc, time::Instant}; +use std::{convert::Infallible, net::IpAddr, sync::Arc, time::Instant}; use axum::{ async_trait, extract::{FromRef, FromRequestParts}, - response::{IntoResponse, Response}, }; -use hyper::StatusCode; -use mas_axum_utils::{cookies::CookieManager, http_client_factory::HttpClientFactory}; +use ipnetwork::IpNetwork; +use mas_handlers::{ + passwords::PasswordManager, ActivityTracker, BoundActivityTracker, CookieManager, ErrorWrapper, + HttpClientFactory, MatrixHomeserver, MetadataCache, SiteConfig, +}; use mas_keystore::{Encrypter, Keystore}; use mas_policy::{Policy, PolicyFactory}; use mas_router::UrlBuilder; @@ -34,11 +36,6 @@ use opentelemetry::{ use rand::SeedableRng; use sqlx::PgPool; -use crate::{ - passwords::PasswordManager, site_config::SiteConfig, upstream_oauth2::cache::MetadataCache, - ActivityTracker, BoundActivityTracker, MatrixHomeserver, -}; - #[derive(Clone)] pub struct AppState { pub pool: PgPool, @@ -55,6 +52,7 @@ pub struct AppState { pub metadata_cache: MetadataCache, pub site_config: SiteConfig, pub activity_tracker: ActivityTracker, + pub trusted_proxies: Vec, pub conn_acquisition_histogram: Option>, } @@ -238,25 +236,6 @@ impl FromRequestParts for BoxRng { } } -/// A simple wrapper around an error that implements [`IntoResponse`]. -pub struct ErrorWrapper(T); - -impl From for ErrorWrapper { - fn from(input: T) -> Self { - Self(input) - } -} - -impl IntoResponse for ErrorWrapper -where - T: std::error::Error, -{ - fn into_response(self) -> Response { - // TODO: make this a bit more user friendly - (StatusCode::INTERNAL_SERVER_ERROR, self.0.to_string()).into_response() - } -} - #[async_trait] impl FromRequestParts for Policy { type Rejection = ErrorWrapper; @@ -282,16 +261,63 @@ impl FromRequestParts for ActivityTracker { } } +fn infer_client_ip( + parts: &axum::http::request::Parts, + trusted_proxies: &[IpNetwork], +) -> Option { + let connection_info = parts.extensions.get::(); + + let peer = if let Some(info) = connection_info { + // We can always trust the proxy protocol to give us the correct IP address + if let Some(proxy) = info.get_proxy_ref() { + if let Some(source) = proxy.source() { + return Some(source.ip()); + } + } + + info.get_peer_addr().map(|addr| addr.ip()) + } else { + None + }; + + // Get the list of IPs from the X-Forwarded-For header + let peers_from_header = parts + .headers + .get("x-forwarded-for") + .and_then(|value| value.to_str().ok()) + .map(|value| value.split(',').filter_map(|v| v.parse().ok())) + .into_iter() + .flatten(); + + // This constructs a list of IP addresses that might be the client's IP address. + // Each intermediate proxy is supposed to add the client's IP address to front + // of the list. We are effectively adding the IP we got from the socket to the + // front of the list. + let peer_list: Vec = peer.into_iter().chain(peers_from_header).collect(); + + // We'll fallback to the first IP in the list if all the IPs we got are trusted + let fallback = peer_list.first().copied(); + + // Now we go through the list, and the IP of the client is the first IP that is + // not in the list of trusted proxies, starting from the back. + let client_ip = peer_list + .iter() + .rfind(|ip| !trusted_proxies.iter().any(|network| network.contains(**ip))) + .copied(); + + client_ip.or(fallback) +} + #[async_trait] impl FromRequestParts for BoundActivityTracker { type Rejection = Infallible; async fn from_request_parts( - _parts: &mut axum::http::request::Parts, + parts: &mut axum::http::request::Parts, state: &AppState, ) -> Result { - // TODO: grab the IP address from the request - let ip = None; + let ip = infer_client_ip(parts, &state.trusted_proxies); + tracing::debug!(ip = ?ip, "Inferred client IP address"); Ok(state.activity_tracker.clone().bind(ip)) } } diff --git a/crates/cli/src/commands/server.rs b/crates/cli/src/commands/server.rs index 55e3cc24..b8f105f4 100644 --- a/crates/cli/src/commands/server.rs +++ b/crates/cli/src/commands/server.rs @@ -19,8 +19,7 @@ use clap::Parser; use itertools::Itertools; use mas_config::AppConfig; use mas_handlers::{ - ActivityTracker, AppState, CookieManager, HttpClientFactory, MatrixHomeserver, MetadataCache, - SiteConfig, + ActivityTracker, CookieManager, HttpClientFactory, MatrixHomeserver, MetadataCache, SiteConfig, }; use mas_listener::{server::Server, shutdown::ShutdownStream}; use mas_matrix_synapse::SynapseConnection; @@ -33,9 +32,12 @@ use rand::{ use tokio::signal::unix::SignalKind; use tracing::{info, info_span, warn, Instrument}; -use crate::util::{ - database_pool_from_config, mailer_from_config, password_manager_from_config, - policy_factory_from_config, register_sighup, templates_from_config, +use crate::{ + app_state::AppState, + util::{ + database_pool_from_config, mailer_from_config, password_manager_from_config, + policy_factory_from_config, register_sighup, templates_from_config, + }, }; #[derive(Parser, Debug, Default)] @@ -144,6 +146,7 @@ impl Options { // Initialize the activity tracker // Activity is flushed every minute let activity_tracker = ActivityTracker::new(pool.clone(), Duration::from_secs(60)); + let trusted_proxies = config.http.trusted_proxies.clone(); // Explicitly the config to properly zeroize secret keys drop(config); @@ -169,6 +172,7 @@ impl Options { password_manager, site_config, activity_tracker, + trusted_proxies, conn_acquisition_histogram: None, }; s.init_metrics()?; diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 926a1a98..c65e6df3 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -29,6 +29,7 @@ use tracing_subscriber::{ use crate::sentry_transport::HyperTransportFactory; +mod app_state; mod commands; mod sentry_transport; mod server; diff --git a/crates/cli/src/server.rs b/crates/cli/src/server.rs index 8ab620eb..2e96c455 100644 --- a/crates/cli/src/server.rs +++ b/crates/cli/src/server.rs @@ -31,7 +31,6 @@ use hyper::{ }; use listenfd::ListenFd; use mas_config::{HttpBindConfig, HttpResource, HttpTlsConfig, UnixOrTcp}; -use mas_handlers::AppState; use mas_listener::{unix_or_tcp::UnixOrTcpListener, ConnectionInfo}; use mas_router::Route; use mas_templates::Templates; @@ -52,6 +51,8 @@ use tower_http::{services::ServeDir, set_header::SetResponseHeaderLayer}; use tracing::{warn, Span}; use tracing_opentelemetry::OpenTelemetrySpanExt; +use crate::app_state::AppState; + const MAS_LISTENER_NAME: Key = Key::from_static_str("mas.listener.name"); #[inline] diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index e69dd418..3fe7faa9 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -18,6 +18,7 @@ anyhow.workspace = true camino = { version = "1.1.6", features = ["serde1"] } chrono.workspace = true figment = { version = "0.10.10", features = ["env", "yaml", "test"] } +ipnetwork = { version = "0.20.0", features = ["serde", "schemars"] } schemars = { version = "0.8.15", features = ["url", "chrono"] } ulid.workspace = true url.workspace = true diff --git a/crates/config/src/sections/http.rs b/crates/config/src/sections/http.rs index ea3c66b6..978f3404 100644 --- a/crates/config/src/sections/http.rs +++ b/crates/config/src/sections/http.rs @@ -19,6 +19,7 @@ use std::{borrow::Cow, io::Cursor, ops::Deref}; use anyhow::bail; use async_trait::async_trait; use camino::Utf8PathBuf; +use ipnetwork::IpNetwork; use mas_keystore::PrivateKey; use rand::Rng; use schemars::JsonSchema; @@ -60,6 +61,17 @@ fn http_listener_assets_path_default() -> Utf8PathBuf { "./share/assets/".into() } +fn default_trusted_proxies() -> Vec { + vec![ + IpNetwork::new([192, 128, 0, 0].into(), 16).unwrap(), + IpNetwork::new([172, 16, 0, 0].into(), 12).unwrap(), + IpNetwork::new([10, 0, 0, 0].into(), 10).unwrap(), + IpNetwork::new(std::net::Ipv4Addr::LOCALHOST.into(), 8).unwrap(), + IpNetwork::new([0xfd00, 0, 0, 0, 0, 0, 0, 0].into(), 8).unwrap(), + IpNetwork::new(std::net::Ipv6Addr::LOCALHOST.into(), 128).unwrap(), + ] +} + /// Kind of socket #[derive(Debug, Serialize, Deserialize, JsonSchema, Clone, Copy)] #[serde(rename_all = "lowercase")] @@ -319,6 +331,11 @@ pub struct HttpConfig { #[serde(default)] pub listeners: Vec, + /// List of trusted reverse proxies that can set the `X-Forwarded-For` + /// header + #[serde(default = "default_trusted_proxies")] + pub trusted_proxies: Vec, + /// Public URL base from where the authentication service is reachable pub public_base: Url, @@ -359,6 +376,7 @@ impl Default for HttpConfig { }], }, ], + trusted_proxies: default_trusted_proxies(), issuer: Some(default_public_base()), public_base: default_public_base(), } diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index dfed6067..172afdea 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -59,7 +59,6 @@ use sqlx::PgPool; use tower::util::AndThenLayer; use tower_http::cors::{Any, CorsLayer}; -mod app_state; mod compat; mod graphql; mod health; @@ -89,11 +88,12 @@ macro_rules! impl_from_error_for_route { }; } -pub use mas_axum_utils::{cookies::CookieManager, http_client_factory::HttpClientFactory}; +pub use mas_axum_utils::{ + cookies::CookieManager, http_client_factory::HttpClientFactory, ErrorWrapper, +}; pub use self::{ activity_tracker::{ActivityTracker, Bound as BoundActivityTracker}, - app_state::AppState, compat::MatrixHomeserver, graphql::schema as graphql_schema, site_config::SiteConfig, diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index 78aee642..af088c4c 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -30,7 +30,9 @@ use hyper::{ header::{CONTENT_TYPE, COOKIE, SET_COOKIE}, Request, Response, StatusCode, }; -use mas_axum_utils::{cookies::CookieManager, http_client_factory::HttpClientFactory}; +use mas_axum_utils::{ + cookies::CookieManager, http_client_factory::HttpClientFactory, ErrorWrapper, +}; use mas_keystore::{Encrypter, JsonWebKey, JsonWebKeySet, Keystore, PrivateKey}; use mas_matrix::{HomeserverConnection, MockHomeserverConnection}; use mas_policy::{InstantiateError, Policy, PolicyFactory}; @@ -46,7 +48,6 @@ use tower::{Layer, Service, ServiceExt}; use url::Url; use crate::{ - app_state::ErrorWrapper, passwords::{Hasher, PasswordManager}, site_config::SiteConfig, upstream_oauth2::cache::MetadataCache, diff --git a/docs/config.schema.json b/docs/config.schema.json index 9ad3b7f5..30245b37 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -109,7 +109,15 @@ ] } ], - "public_base": "http://[::]:8080/" + "public_base": "http://[::]:8080/", + "trusted_proxies": [ + "192.128.0.0/16", + "172.16.0.0/12", + "10.0.0.0/10", + "127.0.0.1/8", + "fd00::/8", + "::1/128" + ] }, "allOf": [ { @@ -838,6 +846,21 @@ "description": "Public URL base from where the authentication service is reachable", "type": "string", "format": "uri" + }, + "trusted_proxies": { + "description": "List of trusted reverse proxies that can set the `X-Forwarded-For` header", + "default": [ + "192.128.0.0/16", + "172.16.0.0/12", + "10.0.0.0/10", + "127.0.0.1/8", + "fd00::/8", + "::1/128" + ], + "type": "array", + "items": { + "$ref": "#/definitions/IpNetwork" + } } } }, @@ -889,6 +912,37 @@ } } }, + "IpNetwork": { + "oneOf": [ + { + "title": "v4", + "allOf": [ + { + "$ref": "#/definitions/Ipv4Network" + } + ] + }, + { + "title": "v6", + "allOf": [ + { + "$ref": "#/definitions/Ipv6Network" + } + ] + } + ], + "x-rust-type": "ipnetwork::IpNetwork" + }, + "Ipv4Network": { + "type": "string", + "pattern": "^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\/(3[0-2]|[0-2]?[0-9])$", + "x-rust-type": "ipnetwork::Ipv4Network" + }, + "Ipv6Network": { + "type": "string", + "pattern": "^(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\")[/](12[0-8]|1[0-1][0-9]|[0-9]?[0-9])$", + "x-rust-type": "ipnetwork::Ipv6Network" + }, "JsonWebKeyEcEllipticCurve": { "description": "JSON Web Key EC Elliptic Curve", "anyOf": [