Усилить проверку источника и ограничение запросов

.env.example

@@ -29,6 +29,9 @@ CRANK_MASTER_KEY=change-me-master-key
 CRANK_SESSION_SECRET=change-me-session-secret
 CRANK_PASSWORD_PEPPER=change-me-password-pepper
 CRANK_SESSION_TTL_HOURS=24
+# Trust X-Real-IP / X-Forwarded-For for client rate limiting. Enable only when
+# admin-api runs behind the bundled nginx (or another trusted reverse proxy).
+CRANK_TRUST_FORWARDED_HEADERS=true
 CRANK_BOOTSTRAP_ADMIN_EMAIL=owner@crank.local
 CRANK_BOOTSTRAP_ADMIN_PASSWORD=change-me-admin-password
 CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME=Crank Owner

Cargo.lock

@@ -618,6 +618,7 @@ dependencies = [
  "crank-schema",
  "crank-test-support",
  "futures-util",
+ "reqwest",
  "serde",
  "serde_json",
  "sha2 0.10.9",

apps/admin-api/src/main.rs

@@ -83,9 +83,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         } else {
             RequestRateLimiter::new(api_rate_limit)
         },
+        trust_forwarded_headers: env_flag("CRANK_TRUST_FORWARDED_HEADERS"),
     };
     let app = build_app(state);
     let listener = TcpListener::bind(socket_addr).await?;
+    let make_service = app.into_make_service_with_connect_info::<SocketAddr>();
 
     info!(
         runtime_max_concurrent_unary = runtime_limits.max_concurrent_unary,
@@ -101,7 +103,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     );
     info!("admin-api listening on {}", socket_addr);
 
-    axum::serve(listener, app).await?;
+    axum::serve(listener, make_service).await?;
 
     Ok(())
 }

apps/admin-api/src/rate_limit.rs

@@ -1,19 +1,30 @@
+use std::net::{IpAddr, SocketAddr};
+
 use axum::{
-    extract::{Request, State},
-    http::header::{COOKIE, HeaderMap},
+    extract::{ConnectInfo, Request, State},
+    http::HeaderMap,
     middleware::Next,
     response::Response,
 };
 use crank_runtime::RateLimitRejection;
 
-use crate::{auth::SESSION_COOKIE_NAME, error::ApiError, state::AppState};
+use crate::{error::ApiError, state::AppState};
 
 pub async fn apply_api_rate_limit(
     State(state): State<AppState>,
     request: Request,
     next: Next,
 ) -> Result<Response, ApiError> {
-    let key = rate_limit_key(request.headers(), request.uri().path());
+    let peer_ip = request
+        .extensions()
+        .get::<ConnectInfo<SocketAddr>>()
+        .map(|ConnectInfo(address)| address.ip());
+    let key = rate_limit_key(
+        request.headers(),
+        request.uri().path(),
+        peer_ip,
+        state.trust_forwarded_headers,
+    );
     if let Err(rejection) = state.api_rate_limiter.check(&key).await {
         return Err(ApiError::rate_limited_with_context(
             "request rate limit exceeded",
@@ -30,56 +41,64 @@ fn rejection_context(rejection: RateLimitRejection) -> serde_json::Value {
     })
 }
 
-fn rate_limit_key(headers: &HeaderMap, path: &str) -> String {
-    if let Some(session_id) = session_id_from_headers(headers) {
-        return format!("session:{session_id}");
+fn rate_limit_key(
+    headers: &HeaderMap,
+    path: &str,
+    peer_ip: Option<IpAddr>,
+    trust_forwarded_headers: bool,
+) -> String {
+    if trust_forwarded_headers && let Some(client_ip) = forwarded_client_ip(headers) {
+        return format!("ip:{client_ip}");
     }
 
-    if let Some(forwarded_for) = header_value(headers, "x-forwarded-for") {
-        let ip = forwarded_for
-            .split(',')
-            .next()
-            .map(str::trim)
-            .filter(|value| !value.is_empty())
-            .unwrap_or("unknown");
-        return format!("ip:{ip}");
-    }
-
-    if let Some(real_ip) = header_value(headers, "x-real-ip") {
-        return format!("ip:{real_ip}");
+    if let Some(peer_ip) = peer_ip {
+        return format!("ip:{peer_ip}");
     }
 
     format!("anonymous:{path}")
 }
 
-fn session_id_from_headers(headers: &HeaderMap) -> Option<String> {
-    let cookies = headers.get(COOKIE)?.to_str().ok()?;
-    for part in cookies.split(';') {
-        let (name, value) = part.trim().split_once('=')?;
-        if name != SESSION_COOKIE_NAME {
-            continue;
-        }
-        let (session_id, _) = value.split_once('.')?;
-        if !session_id.is_empty() {
-            return Some(session_id.to_owned());
-        }
+/// Resolves the client IP from proxy headers, assuming a single trusted proxy.
+///
+/// `X-Real-IP` is preferred because a trusted proxy (e.g. nginx) sets it to the
+/// real peer address. For `X-Forwarded-For` the proxy *appends* the observed
+/// peer, so the last entry is the trustworthy hop; taking the first entry (as
+/// naive implementations do) would let a client spoof its address by sending a
+/// pre-populated header.
+fn forwarded_client_ip(headers: &HeaderMap) -> Option<IpAddr> {
+    if let Some(real_ip) = header_value(headers, "x-real-ip").and_then(parse_ip) {
+        return Some(real_ip);
     }
 
-    None
+    header_value(headers, "x-forwarded-for")?
+        .split(',')
+        .map(str::trim)
+        .rfind(|value| !value.is_empty())
+        .and_then(parse_ip)
 }
 
 fn header_value<'a>(headers: &'a HeaderMap, name: &'static str) -> Option<&'a str> {
     headers.get(name)?.to_str().ok().map(str::trim)
 }
 
+fn parse_ip(value: &str) -> Option<IpAddr> {
+    value.parse().ok()
+}
+
 #[cfg(test)]
 mod tests {
+    use std::net::{IpAddr, Ipv4Addr};
+
     use axum::http::{HeaderMap, HeaderValue, header::COOKIE};
 
     use super::rate_limit_key;
 
+    fn peer() -> Option<IpAddr> {
+        Some(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 7)))
+    }
+
     #[test]
-    fn keys_by_session_cookie_first() {
+    fn unverified_session_cookie_cannot_change_client_key() {
         let mut headers = HeaderMap::new();
         headers.insert(
             COOKIE,
@@ -88,19 +107,86 @@ mod tests {
         headers.insert("x-forwarded-for", HeaderValue::from_static("10.0.0.5"));
 
         assert_eq!(
-            rate_limit_key(&headers, "/api/auth/login"),
-            "session:sess_123"
+            rate_limit_key(&headers, "/api/auth/login", peer(), true),
+            "ip:10.0.0.5"
         );
     }
 
     #[test]
-    fn falls_back_to_forwarded_ip() {
+    fn ignores_forwarded_headers_when_untrusted() {
+        let mut headers = HeaderMap::new();
+        headers.insert("x-forwarded-for", HeaderValue::from_static("10.0.0.5"));
+        headers.insert("x-real-ip", HeaderValue::from_static("10.0.0.9"));
+
+        assert_eq!(
+            rate_limit_key(&headers, "/api/auth/login", peer(), false),
+            "ip:203.0.113.7"
+        );
+    }
+
+    #[test]
+    fn prefers_real_ip_when_trusted() {
         let mut headers = HeaderMap::new();
         headers.insert(
             "x-forwarded-for",
-            HeaderValue::from_static("10.0.0.5, 10.0.0.6"),
+            HeaderValue::from_static("1.2.3.4, 10.0.0.6"),
+        );
+        headers.insert("x-real-ip", HeaderValue::from_static("10.0.0.9"));
+
+        assert_eq!(
+            rate_limit_key(&headers, "/api/auth/login", peer(), true),
+            "ip:10.0.0.9"
+        );
+    }
+
+    #[test]
+    fn uses_last_forwarded_hop_when_trusted() {
+        // A client can prepend spoofed entries; the trusted proxy appends the
+        // real peer, so the last entry is authoritative.
+        let mut headers = HeaderMap::new();
+        headers.insert(
+            "x-forwarded-for",
+            HeaderValue::from_static("1.2.3.4, 10.0.0.6"),
         );
 
-        assert_eq!(rate_limit_key(&headers, "/api/auth/login"), "ip:10.0.0.5");
+        assert_eq!(
+            rate_limit_key(&headers, "/api/auth/login", peer(), true),
+            "ip:10.0.0.6"
+        );
+    }
+
+    #[test]
+    fn falls_back_to_peer_ip_without_headers() {
+        let headers = HeaderMap::new();
+
+        assert_eq!(
+            rate_limit_key(&headers, "/api/auth/login", peer(), true),
+            "ip:203.0.113.7"
+        );
+    }
+
+    #[test]
+    fn ignores_invalid_forwarded_ip_values() {
+        let mut headers = HeaderMap::new();
+        headers.insert("x-real-ip", HeaderValue::from_static("not-an-ip"));
+        headers.insert(
+            "x-forwarded-for",
+            HeaderValue::from_static("198.51.100.8, also-not-an-ip"),
+        );
+
+        assert_eq!(
+            rate_limit_key(&headers, "/api/auth/login", peer(), true),
+            "ip:203.0.113.7"
+        );
+    }
+
+    #[test]
+    fn falls_back_to_path_without_peer() {
+        let headers = HeaderMap::new();
+
+        assert_eq!(
+            rate_limit_key(&headers, "/api/auth/login", None, false),
+            "anonymous:/api/auth/login"
+        );
     }
 }

apps/admin-api/src/state.rs

@@ -5,4 +5,10 @@ use crank_runtime::RequestRateLimiter;
 pub struct AppState {
     pub service: AdminService,
     pub api_rate_limiter: RequestRateLimiter,
+    /// Whether to trust `X-Real-IP` / `X-Forwarded-For` for client identification.
+    ///
+    /// Only enable when the service sits behind a trusted reverse proxy that
+    /// overwrites these headers (e.g. the bundled nginx). When disabled the
+    /// real TCP peer address is used, which a client cannot spoof.
+    pub trust_forwarded_headers: bool,
 }

apps/admin-api/tests/integration/auth_rate_limit.rs

@@ -109,10 +109,14 @@ async fn rejects_rapid_login_requests_with_429() {
         api_rate_limiter: crank_runtime::RequestRateLimiter::new(
             crank_runtime::RequestRateLimitConfig::new(1, 1).unwrap(),
         ),
+        trust_forwarded_headers: false,
     });
     let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel();
     let handle = tokio::spawn(async move {
-        axum::serve(listener, app)
+        axum::serve(
+            listener,
+            app.into_make_service_with_connect_info::<std::net::SocketAddr>(),
+        )
         .with_graceful_shutdown(async move {
             let _ = shutdown_rx.await;
         })

apps/admin-api/tests/integration/common.rs

@@ -104,6 +104,7 @@ pub(super) fn build_test_app(
         api_rate_limiter: crank_runtime::RequestRateLimiter::new(
             crank_runtime::RequestRateLimitConfig::new(10_000, 10_000).unwrap(),
         ),
+        trust_forwarded_headers: false,
     })
 }
 

crates/crank-community-mcp/Cargo.toml

@@ -15,6 +15,7 @@ crank-registry = { path = "../crank-registry" }
 crank-runtime = { path = "../crank-runtime" }
 crank-schema = { path = "../crank-schema" }
 futures-util = "0.3"
+reqwest.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 sha2.workspace = true

crates/crank-community-mcp/src/transport.rs

@@ -12,6 +12,7 @@ use axum::{
     },
 };
 use futures_util::stream;
+use reqwest::Url;
 use serde_json::Value;
 
 use crate::jsonrpc::{
@@ -42,21 +43,44 @@ impl AllowedOrigins {
     }
 
     pub(super) fn is_allowed(&self, origin: &str) -> bool {
-        if origin.starts_with("http://localhost")
-            || origin.starts_with("http://127.0.0.1")
-            || origin.starts_with("https://localhost")
-            || origin.starts_with("https://127.0.0.1")
-        {
+        let Some(origin) = parse_origin(origin, true) else {
+            return false;
+        };
+
+        if is_loopback_origin(&origin) {
             return true;
         }
 
         match &self.public_origin {
-            Some(public_origin) => public_origin == origin,
+            Some(public_origin) => public_origin == &origin.origin().ascii_serialization(),
             None => false,
         }
     }
 }
 
+fn is_loopback_origin(origin: &Url) -> bool {
+    matches!(origin.host_str(), Some("localhost" | "127.0.0.1" | "[::1]"))
+}
+
+fn parse_origin(value: &str, origin_only: bool) -> Option<Url> {
+    let origin = Url::parse(value).ok()?;
+    if !matches!(origin.scheme(), "http" | "https")
+        || origin.host_str().is_none()
+        || !origin.username().is_empty()
+        || origin.password().is_some()
+    {
+        return None;
+    }
+
+    if origin_only
+        && (origin.path() != "/" || origin.query().is_some() || origin.fragment().is_some())
+    {
+        return None;
+    }
+
+    Some(origin)
+}
+
 pub(super) fn validate_origin(
     allowed_origins: &AllowedOrigins,
     headers: &HeaderMap,
@@ -298,14 +322,58 @@ pub(super) fn is_valid_request_id(value: &str) -> bool {
 }
 
 pub(super) fn extract_origin(url: &str) -> Option<String> {
-    let mut parts = url.split('/');
-    let scheme = parts.next()?;
-    let empty = parts.next()?;
-    let authority = parts.next()?;
+    Some(parse_origin(url, false)?.origin().ascii_serialization())
+}
 
-    if empty.is_empty() {
-        return Some(format!("{scheme}//{authority}"));
+#[cfg(test)]
+mod tests {
+    use super::AllowedOrigins;
+
+    #[test]
+    fn allows_loopback_origins_with_and_without_port() {
+        let origins = AllowedOrigins::new(None);
+
+        assert!(origins.is_allowed("http://localhost"));
+        assert!(origins.is_allowed("http://localhost:3000"));
+        assert!(origins.is_allowed("https://127.0.0.1:8443"));
+        assert!(origins.is_allowed("http://[::1]:3000"));
     }
 
-    None
+    #[test]
+    fn rejects_hosts_that_only_prefix_match_loopback() {
+        let origins = AllowedOrigins::new(None);
+
+        assert!(!origins.is_allowed("http://localhost.attacker.com"));
+        assert!(!origins.is_allowed("http://127.0.0.1.attacker.com"));
+        assert!(!origins.is_allowed("http://localhost@attacker.com"));
+        assert!(!origins.is_allowed("http://attacker.com/http://localhost"));
+        assert!(!origins.is_allowed("http://[::1].attacker.com"));
+        assert!(!origins.is_allowed("http://attacker@[::1]"));
+    }
+
+    #[test]
+    fn matches_configured_public_origin_exactly() {
+        let origins = AllowedOrigins::new(Some("https://crank.example.com".to_owned()));
+
+        assert!(origins.is_allowed("https://crank.example.com"));
+        assert!(!origins.is_allowed("https://crank.example.com.attacker.com"));
+        assert!(!origins.is_allowed("https://evil.example.com"));
+    }
+
+    #[test]
+    fn rejects_non_http_schemes() {
+        let origins = AllowedOrigins::new(None);
+
+        assert!(!origins.is_allowed("file://localhost"));
+        assert!(!origins.is_allowed("javascript://localhost"));
+    }
+
+    #[test]
+    fn rejects_values_that_are_not_origins() {
+        let origins = AllowedOrigins::new(None);
+
+        assert!(!origins.is_allowed("http://localhost/path"));
+        assert!(!origins.is_allowed("http://localhost?query=value"));
+        assert!(!origins.is_allowed("http://localhost#fragment"));
+    }
 }

deploy/community/.env.images.example

@@ -26,6 +26,10 @@ CRANK_MASTER_KEY=change-me-master-key
 CRANK_SESSION_SECRET=change-me-session-secret
 CRANK_PASSWORD_PEPPER=change-me-password-pepper
 CRANK_SESSION_TTL_HOURS=24
+# Trust X-Real-IP / X-Forwarded-For for client rate limiting. Keep enabled only
+# when admin-api runs behind the bundled nginx (or another trusted reverse
+# proxy). Set to false if you expose admin-api directly to untrusted clients.
+CRANK_TRUST_FORWARDED_HEADERS=true
 CRANK_BOOTSTRAP_ADMIN_EMAIL=owner@crank.local
 CRANK_BOOTSTRAP_ADMIN_PASSWORD=change-me-admin-password
 CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME=Crank Owner

deploy/community/docker-compose.images.yml

@@ -54,6 +54,7 @@ services:
       CRANK_SESSION_SECRET: ${CRANK_SESSION_SECRET}
       CRANK_PASSWORD_PEPPER: ${CRANK_PASSWORD_PEPPER}
       CRANK_SESSION_TTL_HOURS: ${CRANK_SESSION_TTL_HOURS:-24}
+      CRANK_TRUST_FORWARDED_HEADERS: ${CRANK_TRUST_FORWARDED_HEADERS:-true}
       CRANK_BOOTSTRAP_ADMIN_EMAIL: ${CRANK_BOOTSTRAP_ADMIN_EMAIL}
       CRANK_BOOTSTRAP_ADMIN_PASSWORD: ${CRANK_BOOTSTRAP_ADMIN_PASSWORD}
       CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME: ${CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME:-Crank Owner}

deploy/community/docker-compose.yml

@@ -38,6 +38,7 @@ services:
       CRANK_SESSION_SECRET: ${CRANK_SESSION_SECRET}
       CRANK_PASSWORD_PEPPER: ${CRANK_PASSWORD_PEPPER}
       CRANK_SESSION_TTL_HOURS: ${CRANK_SESSION_TTL_HOURS:-24}
+      CRANK_TRUST_FORWARDED_HEADERS: ${CRANK_TRUST_FORWARDED_HEADERS:-true}
       CRANK_BOOTSTRAP_ADMIN_EMAIL: ${CRANK_BOOTSTRAP_ADMIN_EMAIL}
       CRANK_BOOTSTRAP_ADMIN_PASSWORD: ${CRANK_BOOTSTRAP_ADMIN_PASSWORD}
       CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME: ${CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME:-Crank Owner}

docker-compose.yml

@@ -36,6 +36,7 @@ services:
       CRANK_SESSION_SECRET: ${CRANK_SESSION_SECRET}
       CRANK_PASSWORD_PEPPER: ${CRANK_PASSWORD_PEPPER}
       CRANK_SESSION_TTL_HOURS: ${CRANK_SESSION_TTL_HOURS:-24}
+      CRANK_TRUST_FORWARDED_HEADERS: ${CRANK_TRUST_FORWARDED_HEADERS:-true}
       CRANK_BOOTSTRAP_ADMIN_EMAIL: ${CRANK_BOOTSTRAP_ADMIN_EMAIL}
       CRANK_BOOTSTRAP_ADMIN_PASSWORD: ${CRANK_BOOTSTRAP_ADMIN_PASSWORD}
       CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME: ${CRANK_BOOTSTRAP_ADMIN_DISPLAY_NAME:-Crank Owner}