diff --git a/server/crates/kvitt-server/src/api/rate_limit.rs b/server/crates/kvitt-server/src/api/rate_limit.rs index 4888345..7330f17 100644 --- a/server/crates/kvitt-server/src/api/rate_limit.rs +++ b/server/crates/kvitt-server/src/api/rate_limit.rs @@ -1,6 +1,8 @@ //! Rate limiting (SPEC §8.6, AC-2.12, §8.5), as a tower layer that `router.rs` attaches to every route. //! -//! * Unauthenticated endpoints (E-1 to E-6 and unknown paths): at most 60 requests per minute per client IP. +//! * Unauthenticated endpoints (E-1 to E-5 and unknown paths): at most 60 requests per minute per client IP. +//! * APK downloads (E-6, `/apk/files/{file_name}`, D-119): at most 10 requests per minute per client IP, in a bucket +//! of their own that does not count against the 60 (security review L-3: each answer can be a 10 MB file). //! * Authenticated endpoints: at most 600 requests per minute per device, keyed by the hash of the bearer token //! (the token identifies the device; the limiter never touches the database). A request to an authenticated //! endpoint without a well-formed bearer token counts against its IP like an unauthenticated one. @@ -43,6 +45,8 @@ use crate::{AppState, lock}; /// Unauthenticated requests per window per client IP (SPEC §8.6). pub const UNAUTHENTICATED_LIMIT: u32 = 60; +/// APK downloads (E-6) per window per client IP (SPEC §8.6, D-119). +pub const APK_DOWNLOAD_LIMIT: u32 = 10; /// Authenticated requests per window per device (SPEC §8.6). pub const AUTHENTICATED_LIMIT: u32 = 600; /// The request window (SPEC §8.6: per minute). @@ -66,6 +70,8 @@ const PUBLIC_ROUTES: [&str; 6] = [ "/apk/files/{file_name}", ]; const ACCEPT_ROUTE: &str = "/invites/accept"; +/// Route template of E-6, limited by [`APK_DOWNLOAD_LIMIT`] instead of [`UNAUTHENTICATED_LIMIT`]. +const APK_ROUTE: &str = "/apk/files/{file_name}"; /// The layer for `router.rs`. One limiter per router: build the router once and reuse it (as `serve` does). pub fn layer(state: &AppState) -> RateLimitLayer { @@ -132,6 +138,7 @@ struct TokenState { #[derive(Debug, Default)] struct Buckets { ip: HashMap, + apk: HashMap, tokens: HashMap, accept_failures: HashMap, last_prune: Option>, @@ -144,6 +151,7 @@ impl Buckets { } self.last_prune = Some(now); self.ip.retain(|_, w| !w.expired(now, REQUEST_WINDOW)); + self.apk.retain(|_, w| !w.expired(now, REQUEST_WINDOW)); // Accepted tokens too: re-accepting one costs a single request counted against its IP and given back. self.tokens .retain(|_, t| t.window.is_some_and(|w| !w.expired(now, REQUEST_WINDOW))); @@ -172,7 +180,12 @@ struct Admitted { /// How a request is counted. enum Key { Ip(IpAddr), - Token { ip: IpAddr, hash: String }, + /// E-6 from this client IP. + Apk(IpAddr), + Token { + ip: IpAddr, + hash: String, + }, } /// The shared limiter state. @@ -242,6 +255,9 @@ impl Limiter { } fn key(request: &Request, route: &str, ip: IpAddr) -> Key { + if route == APK_ROUTE { + return Key::Apk(ip); + } if PUBLIC_ROUTES.contains(&route) || route.is_empty() { return Key::Ip(ip); } @@ -273,6 +289,14 @@ impl Limiter { buckets.hit_ip(*ip, now)?; Ok(Admitted::default()) } + Key::Apk(ip) => { + let mut slot = buckets.apk.get(ip).copied(); + let result = hit(&mut slot, now, REQUEST_WINDOW, APK_DOWNLOAD_LIMIT); + if let Some(w) = slot { + buckets.apk.insert(*ip, w); + } + result.map(|()| Admitted::default()) + } Key::Token { ip, hash } => { let accepted = buckets.tokens.get(hash).is_some_and(|t| t.accepted); let ip_window = if accepted { @@ -308,6 +332,7 @@ impl Limiter { } } } + Key::Apk(_) => {} Key::Token { ip, hash } => { if status.is_success() { buckets.tokens.entry(hash.clone()).or_default().accepted = true; @@ -526,6 +551,30 @@ mod tests { assert_eq!(ip_count(&limiter), 0); } + #[test] + fn apk_downloads_have_their_own_bucket_and_are_pruned() { + let now = parse_ts("2026-10-17T12:00:00.000Z").unwrap(); + let limiter = Limiter::new(Arc::new(FixedClock::new(now)), String::new()); + let ip = IpAddr::V4(Ipv4Addr::LOCALHOST); + for _ in 0..APK_DOWNLOAD_LIMIT { + assert!(limiter.admit(&Key::Apk(ip), false, now).is_ok()); + } + assert_eq!(limiter.admit(&Key::Apk(ip), false, now), Err(60)); + assert_eq!( + ip_count(&limiter), + 0, + "downloads do not count against the 60" + ); + assert!(limiter.admit(&Key::Ip(ip), false, now).is_ok()); + let later = now + Duration::minutes(11); + assert!( + limiter + .admit(&Key::Apk(IpAddr::V4(Ipv4Addr::BROADCAST)), false, later) + .is_ok() + ); + assert_eq!(lock(&limiter.buckets).apk.len(), 1, "only the new entry"); + } + #[test] fn pruning_drops_expired_windows() { let now = parse_ts("2026-10-17T12:00:00.000Z").unwrap(); diff --git a/server/crates/kvitt-server/tests/core_api/rate_limit.rs b/server/crates/kvitt-server/tests/core_api/rate_limit.rs index 591ec8d..1150dea 100644 --- a/server/crates/kvitt-server/tests/core_api/rate_limit.rs +++ b/server/crates/kvitt-server/tests/core_api/rate_limit.rs @@ -200,3 +200,67 @@ async fn ten_failed_invite_accepts_lock_the_ip_for_the_rest_of_the_hour() { let reply = accept(&good, "198.51.100.1:1").await; assert_eq!(reply.status, 200, "{}", reply.text()); } + +#[tokio::test] +async fn apk_downloads_10_per_minute_per_ip_in_their_own_bucket() { + let app = TestApp::empty().await; + let dir = app.data_dir().join("apk"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("kvitt-412.apk"), b"apk").unwrap(); + std::fs::write( + dir.join("latest.json"), + example("E-5-apk-latest/response.json").to_string(), + ) + .unwrap(); + let file = "/apk/files/kvitt-412.apk"; + for i in 0..10 { + let reply = app.send(from("192.0.2.1:1", None, file, None)).await; + assert_eq!(reply.status, 200, "download {i}"); + } + app.clock + .set(parse_ts(NOW).unwrap() + Duration::seconds(20)); + let limited = app.send(from("192.0.2.1:1", None, file, None)).await; + assert_error(&limited, 429, "rate_limited"); + assert_eq!(limited.headers["retry-after"], "40"); + // Unknown files count too: the limit is per route, not per answer. + let limited = app + .send(from("192.0.2.1:1", None, "/apk/files/kvitt-1.apk", None)) + .await; + assert_error(&limited, 429, "rate_limited"); + // Another client is not affected; neither are the other unauthenticated endpoints of the same client. + let other = app.send(from("192.0.2.2:1", None, file, None)).await; + assert_eq!(other.status, 200); + for path in ["/apk/latest", "/i/7K3MQX9D2HPA", "/health"] { + let reply = app.send(from("192.0.2.1:1", None, path, None)).await; + assert_eq!(reply.status, 200, "{path}"); + } + // Behind the trusted proxy the forwarded client is the key, as for the 60 per minute. + for _ in 0..10 { + app.send(from("127.0.0.9:1", Some("203.0.113.5"), file, None)) + .await; + } + let limited = app + .send(from("127.0.0.9:1", Some("203.0.113.5"), file, None)) + .await; + assert_error(&limited, 429, "rate_limited"); + // The window ends a minute after the first download. + app.clock + .set(parse_ts(NOW).unwrap() + Duration::seconds(60)); + let again = app.send(from("192.0.2.1:1", None, file, None)).await; + assert_eq!(again.status, 200); +} + +#[tokio::test] +async fn apk_downloads_do_not_use_up_the_unauthenticated_limit() { + let app = TestApp::empty().await; + for _ in 0..10 { + app.send(from("192.0.2.1:1", None, "/apk/files/kvitt-1.apk", None)) + .await; + } + for i in 0..60 { + let reply = app.send(from("192.0.2.1:1", None, "/health", None)).await; + assert_eq!(reply.status, 200, "request {i}"); + } + let limited = app.send(from("192.0.2.1:1", None, "/health", None)).await; + assert_error(&limited, 429, "rate_limited"); +}