From 3fa2668a68215c5c774172b6e990eb35fd62072f Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 2 Nov 2026 10:23:44 -0600 Subject: [PATCH 1/3] feat(api): make the per-IP invite limit configurable at runtime The global ceiling and proof-of-work difficulty can already be changed with an environment variable, but the per-IP allowance is a constant, so raising it for a launch and putting it back means building and swapping a binary twice. Add INVITES_PER_IP_PER_DAY, defaulting to the existing 4. 0 is refused at startup because it would turn invites off, while 0 on the global ceiling turns the ceiling off. An IP can hold more than the limit after it is lowered, so retry_after now counts down to below the limit rather than to the oldest entry. Claude-Session: https://claude.ai/code/session_01VjxvYQUzmUE7o4fLzHUmpo --- rust/api/README.md | 15 +++++ rust/api/src/main.rs | 12 ++++ rust/api/src/rate_limit.rs | 88 +++++++++++++++++++++++++++--- rust/api/src/routes.rs | 56 ++++++++++++++++--- rust/api/tests/invite_limit_env.rs | 86 +++++++++++++++++++++++++++++ 5 files changed, 243 insertions(+), 14 deletions(-) create mode 100644 rust/api/tests/invite_limit_env.rs diff --git a/rust/api/README.md b/rust/api/README.md index 2e6bc65b..7da7f478 100644 --- a/rust/api/README.md +++ b/rust/api/README.md @@ -80,6 +80,21 @@ id is in its URL), then tell each of them to reload that success page. Do not cl with `notary_period`: the server cannot tell which signatures failed to unblind, and a donor whose key worked would be able to mint a second one. +## Invite limits + +These are set as `Environment=` lines in `/etc/systemd/system/gkapi.service`. Changing one +needs `sudo systemctl daemon-reload && sudo systemctl restart gkapi`, and no new binary. The +restart drops any unexpired proof-of-work challenges, and the hourly count is rebuilt from +`invite_rate_limits.json`. + +| Variable | Default | Meaning | +|---|---|---| +| `GLOBAL_INVITES_PER_HOUR` | 200 | Ceiling across everyone over a sliding hour. 0 disables it. | +| `INVITES_PER_IP_PER_DAY` | 4 | Allowance for one IP over a sliding 24 hours, 1 to 1000. Phones behind a carrier NAT share one. | +| `INVITE_POW_DIFFICULTY` | 16 | Base proof-of-work bits. Adds 2 at 90 invites in the hour and 4 at 150. Those thresholds do not move with the ceiling. | + +To undo a temporary raise, delete or restore the line and do the same two commands. + ## Notary keys and the monthly schedule The notary directory (`--notary-dir`, `/home/gkapi/delegate-keys` on nova) holds one diff --git a/rust/api/src/main.rs b/rust/api/src/main.rs index ae61bf50..f54c182d 100644 --- a/rust/api/src/main.rs +++ b/rust/api/src/main.rs @@ -88,6 +88,9 @@ fn load_invite_config(matches: &clap::ArgMatches) -> Option { .unwrap_or("/var/lib/gkapi/tor_exit_list.txt"), )); let global_invites_per_hour = matches.get_one::("global-invites-per-hour").copied(); + let invites_per_ip_per_day = matches + .get_one::("invites-per-ip-per-day") + .map(|n| *n as usize); let pow_base_difficulty = matches .get_one::("invite-pow-difficulty") .copied() @@ -151,6 +154,7 @@ fn load_invite_config(matches: &clap::ArgMatches) -> Option { rate_limit_file, tor_exit_cache, global_invites_per_hour, + invites_per_ip_per_day, pow_base_difficulty, room_owner_vk, inviter_signing_key, @@ -309,6 +313,14 @@ async fn main() { .value_parser(value_parser!(usize)) .help("Emergency hourly ceiling across all issued invitations (0 disables)."), ) + .arg( + Arg::new("invites-per-ip-per-day") + .long("invites-per-ip-per-day") + .value_name("N") + .env("INVITES_PER_IP_PER_DAY") + .value_parser(value_parser!(u32).range(1..=1000)) + .help("Invitations one IP may receive in 24 hours (default 4)."), + ) .arg( Arg::new("invite-pow-difficulty") .long("invite-pow-difficulty") diff --git a/rust/api/src/rate_limit.rs b/rust/api/src/rate_limit.rs index 17f69e44..89e4e72a 100644 --- a/rust/api/src/rate_limit.rs +++ b/rust/api/src/rate_limit.rs @@ -13,7 +13,11 @@ use std::sync::Mutex; use std::time::{Duration as StdDuration, Instant}; use thiserror::Error; -/// Maximum number of invites allowed per IP within the time window. +/// Default number of invites allowed per IP within the time window. +/// +/// Runtime-configurable via `INVITES_PER_IP_PER_DAY`, so a launch can raise it +/// and put it back without a new binary. This default is what applies when +/// that is unset. /// /// Kept deliberately low as an anti-spam measure for the freenet.org/quickstart /// flow, which mints invites into the shared "Freenet Official" room. A few @@ -67,6 +71,7 @@ pub struct RateLimitData { pub struct RateLimiter { data_path: PathBuf, window_hours: i64, + max_per_window: usize, /// Mutex for thread-safe access to the file lock: Mutex<()>, } @@ -81,10 +86,21 @@ impl RateLimiter { Self { data_path, window_hours, + max_per_window: MAX_INVITES_PER_WINDOW, lock: Mutex::new(()), } } + /// Replace the default per-IP allowance. + pub fn with_max_per_window(mut self, max_per_window: usize) -> Self { + self.max_per_window = max_per_window; + self + } + + pub fn max_per_window(&self) -> usize { + self.max_per_window + } + /// Check if an IP is rate limited, and record the access if allowed /// /// Returns Ok(true) if the request is allowed, Ok(false) if rate limited @@ -117,7 +133,7 @@ impl RateLimiter { // Check if IP has reached the limit let timestamps = data.invites.entry(ip_str).or_default(); - if timestamps.len() >= MAX_INVITES_PER_WINDOW { + if timestamps.len() >= self.max_per_window { return Ok(false); // Rate limited } @@ -141,17 +157,22 @@ impl RateLimiter { if let Some(timestamps) = data.invites.get(&ip_str) { // Filter to only valid timestamps within window - let valid_timestamps: Vec<_> = timestamps + let mut valid_timestamps: Vec<_> = timestamps .iter() .filter_map(|ts| DateTime::parse_from_rfc3339(ts).ok()) .map(|t| -> DateTime { t.into() }) .filter(|t| now - *t < window) .collect(); - // If at limit, return time until oldest expires - if valid_timestamps.len() >= MAX_INVITES_PER_WINDOW { - if let Some(oldest) = valid_timestamps.iter().min() { - let expires_at = *oldest + window; + // If at limit, return the time until the count drops below it. + // That is not always the oldest entry: an IP can hold more than + // the limit after the limit is lowered, which is how a temporary + // raise is rolled back. + if valid_timestamps.len() >= self.max_per_window { + valid_timestamps.sort_unstable(); + let index = valid_timestamps.len() - self.max_per_window; + if let Some(frees_a_slot) = valid_timestamps.get(index) { + let expires_at = *frees_a_slot + window; let remaining = expires_at - now; return Ok(Some(remaining.num_seconds())); } @@ -558,6 +579,59 @@ mod tests { ); } + #[test] + fn configured_limit_replaces_the_default_allowance() { + let dir = tempdir().unwrap(); + let limiter = + RateLimiter::new(dir.path().join("rate_limits.json"), 24).with_max_per_window(6); + let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 8)); + + for i in 1..=6 { + assert!( + limiter.check_and_record(ip).unwrap(), + "invite {i} of 6 should be allowed" + ); + } + assert!( + !limiter.check_and_record(ip).unwrap(), + "7th invite in window must be rejected" + ); + } + + /// Lowering the limit after a raise leaves IPs holding more than the new + /// limit. They can ask again once enough entries have expired, not when + /// the oldest one does. + #[test] + fn retry_after_counts_down_to_below_the_limit_when_over_it() { + let dir = tempdir().unwrap(); + let path = dir.path().join("rate_limits.json"); + let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 9)); + let now = Utc::now(); + let hours_ago = [23, 20, 10, 5, 1, 0]; + let timestamps = hours_ago + .iter() + .map(|h| (now - Duration::hours(*h)).to_rfc3339()) + .collect(); + let data = RateLimitData { + invites: HashMap::from([(ip.to_string(), timestamps)]), + }; + fs::write(&path, serde_json::to_string(&data).unwrap()).unwrap(); + + let retry = RateLimiter::new(path, 24) + .with_max_per_window(4) + .get_retry_after(ip) + .unwrap() + .expect("an IP over the limit must have a retry time"); + + // Six held and a limit of 4: the third oldest (10h ago) has to expire, + // which is 14h away. The oldest alone would say 1h. + let hour = Duration::hours(1).num_seconds(); + assert!( + retry > 13 * hour && retry <= 14 * hour, + "expected about 14h, got {retry}s" + ); + } + #[test] fn test_rate_limiter_allows_up_to_max_requests() { let dir = tempdir().unwrap(); diff --git a/rust/api/src/routes.rs b/rust/api/src/routes.rs index c245e89c..be731f06 100644 --- a/rust/api/src/routes.rs +++ b/rust/api/src/routes.rs @@ -48,16 +48,21 @@ pub struct InviteState { } impl InviteState { + #[allow(clippy::too_many_arguments)] pub fn new( rate_limit_file: PathBuf, tor_exit_cache: Option, global_invites_per_hour: Option, + invites_per_ip_per_day: Option, pow_base_difficulty: u8, room_owner_vk: VerifyingKey, inviter_signing_key: SigningKey, room_name: String, ) -> Self { - let rate_limiter = Arc::new(RateLimiter::new(rate_limit_file, 24)); + let invites_per_ip = invites_per_ip_per_day.unwrap_or(MAX_INVITES_PER_WINDOW); + info!("Per-IP invite limit: {invites_per_ip} per 24 hours"); + let rate_limiter = + Arc::new(RateLimiter::new(rate_limit_file, 24).with_max_per_window(invites_per_ip)); let tor_exits = Arc::new(TorExitList::new(tor_exit_cache)); let recent_ages = match rate_limiter.recent_events(GLOBAL_WINDOW_MINUTES) { // Seed only traffic the new policy would have admitted. Otherwise @@ -566,6 +571,13 @@ fn get_client_ip(addr: SocketAddr, headers: &HeaderMap) -> IpAddr { } } +fn per_ip_limit_message(state: &InviteState) -> String { + format!( + "Rate limited. You can request up to {} invites per 24 hours.", + state.rate_limiter.max_per_window() + ) +} + fn invite_error( status: StatusCode, message: impl Into, @@ -636,9 +648,7 @@ async fn get_invite_challenge( Ok(Some(retry_after)) => { return Err(invite_error( StatusCode::TOO_MANY_REQUESTS, - format!( - "Rate limited. You can request up to {MAX_INVITES_PER_WINDOW} invites per 24 hours." - ), + per_ip_limit_message(&state), Some(retry_after), )); } @@ -717,9 +727,7 @@ async fn create_room_invite( ); return Err(invite_error( StatusCode::TOO_MANY_REQUESTS, - format!( - "Rate limited. You can request up to {MAX_INVITES_PER_WINDOW} invites per 24 hours." - ), + per_ip_limit_message(&state), retry_after, )); } @@ -1166,6 +1174,39 @@ mod invite_handler_tests { assert_eq!(state.global_bucket.current(), 3); } + /// The configured limit has to reach both handlers and what the refused + /// user is told, not only the limiter. + #[tokio::test] + async fn configured_per_ip_limit_is_enforced_and_reported() { + let dir = tempfile::tempdir().unwrap(); + let mut state = state_with(&dir, &["185.220.101.1"], 100); + let limit = MAX_INVITES_PER_WINDOW + 2; + state.rate_limiter = Arc::new( + RateLimiter::new(dir.path().join("configured.json"), 24).with_max_per_window(limit), + ); + + for i in 0..limit { + assert_eq!( + request_proxied(&state, "198.51.100.4").await, + StatusCode::OK, + "invite {i} of {limit} should be allowed" + ); + } + let (status, body) = get_invite_challenge( + State(state.clone()), + ConnectInfo(addr("127.0.0.1")), + proxied("198.51.100.4"), + ) + .await + .expect_err("the allowance is used up"); + assert_eq!(status, StatusCode::TOO_MANY_REQUESTS); + assert!( + body.error.contains(&format!("up to {limit} invites")), + "the refusal must state the configured limit, got: {}", + body.error + ); + } + #[tokio::test] async fn per_ip_rejection_refunds_global_capacity_and_proof() { let dir = tempfile::tempdir().unwrap(); @@ -1203,6 +1244,7 @@ mod invite_handler_tests { dir.path().join("rl.json"), Some(cache), Some(200), + None, 4, signing_key.verifying_key(), signing_key, diff --git a/rust/api/tests/invite_limit_env.rs b/rust/api/tests/invite_limit_env.rs new file mode 100644 index 00000000..083d73f2 --- /dev/null +++ b/rust/api/tests/invite_limit_env.rs @@ -0,0 +1,86 @@ +//! `INVITES_PER_IP_PER_DAY` has to reach the running service. The unit tests in +//! `rate_limit.rs` and `routes.rs` set the limit directly, so they stay green +//! if the clap argument or its plumbing through `main.rs` is broken. + +use ed25519_dalek::SigningKey; +use std::io::{BufRead, BufReader}; +use std::process::{Command, Stdio}; +use std::sync::mpsc; +use std::time::Duration; + +fn gkapi(dir: &std::path::Path) -> Command { + let notary = dir.join("notary"); + std::fs::create_dir_all(¬ary).unwrap(); + let signing_key = dir.join("room_signing_key.bin"); + std::fs::write(&signing_key, [7u8; 32]).unwrap(); + let owner = SigningKey::from_bytes(&[9; 32]).verifying_key(); + + let mut command = Command::new(env!("CARGO_BIN_EXE_ghostkey-api")); + command + .current_dir(dir) + .args(["--port", "0", "--notary-dir", notary.to_str().unwrap()]) + .env_remove("INVITES_PER_IP_PER_DAY") + .env("ROOM_SIGNING_KEY_FILE", signing_key) + .env( + "ROOM_OWNER_VK", + bs58::encode(owner.as_bytes()).into_string(), + ) + .env("RATE_LIMIT_FILE", dir.join("rate_limits.json")) + .env("TOR_EXIT_CACHE", dir.join("tor_exits.txt")) + .stdout(Stdio::null()) + .stderr(Stdio::piped()); + command +} + +/// Start gkapi and wait for the line reporting the limit it is enforcing. +fn reported_limit(mut command: Command) -> Option { + let mut child = command.spawn().expect("failed to spawn ghostkey-api"); + let stderr = child.stderr.take().unwrap(); + let (sender, receiver) = mpsc::channel(); + std::thread::spawn(move || { + let line = BufReader::new(stderr) + .lines() + .map_while(Result::ok) + .find(|line| line.contains("Per-IP invite limit")); + let _ = sender.send(line); + }); + let line = receiver + .recv_timeout(Duration::from_secs(30)) + .ok() + .flatten(); + child.kill().unwrap(); + child.wait().unwrap(); + line +} + +#[test] +fn env_var_sets_the_per_ip_limit() { + let dir = tempfile::tempdir().unwrap(); + let mut command = gkapi(dir.path()); + command.env("INVITES_PER_IP_PER_DAY", "7"); + + let line = reported_limit(command).expect("gkapi never reported its limit"); + assert!(line.contains("limit: 7 per 24 hours"), "got: {line}"); +} + +#[test] +fn per_ip_limit_defaults_to_four_when_unset() { + let dir = tempfile::tempdir().unwrap(); + + let line = reported_limit(gkapi(dir.path())).expect("gkapi never reported its limit"); + assert!(line.contains("limit: 4 per 24 hours"), "got: {line}"); +} + +/// The global ceiling uses 0 to mean "off". Here it would refuse everyone, so +/// it must stop the service at startup rather than take invites down quietly. +#[test] +fn zero_per_ip_limit_is_refused_at_startup() { + let dir = tempfile::tempdir().unwrap(); + let mut command = gkapi(dir.path()); + command.env("INVITES_PER_IP_PER_DAY", "0"); + + let output = command.output().expect("failed to run ghostkey-api"); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("invites-per-ip-per-day"), "got: {stderr}"); +} From 007606421bea7e05a6a400ce18b2bcde765799aa Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 2 Nov 2026 10:57:08 -0600 Subject: [PATCH 2/3] fix(api): address review of the configurable per-IP invite limit Cap the variable at 100, well under the global ceiling, and warn at startup when it differs from the default, so a forgotten raise shows up. with_max_per_window(0) now means 1 rather than refusing with no retry time. Write the rate-limit file to a temporary file and rename it. Restarts become routine with this setting, and a truncated file makes every invite return 500. Document the operating traps in the README: the build requirement, the verify step, that a bad value stops donations too, and that .env has no effect. Comments now say 4 is only the default. Tests tie the retry time to admission, cover the rejected and top values, clear the child's environment, and check create's refusal message. The startup test is renamed, since it reads the log and not a refused request. Claude-Session: https://claude.ai/code/session_01VjxvYQUzmUE7o4fLzHUmpo --- rust/api/README.md | 32 ++++++- rust/api/src/main.rs | 2 +- rust/api/src/rate_limit.rs | 134 ++++++++++++++++++++++++----- rust/api/src/routes.rs | 35 +++++++- rust/api/src/tor.rs | 5 +- rust/api/tests/invite_limit_env.rs | 111 ++++++++++++++++++------ 6 files changed, 261 insertions(+), 58 deletions(-) diff --git a/rust/api/README.md b/rust/api/README.md index 7da7f478..ae6ab788 100644 --- a/rust/api/README.md +++ b/rust/api/README.md @@ -85,15 +85,41 @@ donor whose key worked would be able to mint a second one. These are set as `Environment=` lines in `/etc/systemd/system/gkapi.service`. Changing one needs `sudo systemctl daemon-reload && sudo systemctl restart gkapi`, and no new binary. The restart drops any unexpired proof-of-work challenges, and the hourly count is rebuilt from -`invite_rate_limits.json`. +`invite_rate_limits.json`. They must go in the unit file: `.env` is read after the arguments +are parsed, so a value there has no effect. | Variable | Default | Meaning | |---|---|---| | `GLOBAL_INVITES_PER_HOUR` | 200 | Ceiling across everyone over a sliding hour. 0 disables it. | -| `INVITES_PER_IP_PER_DAY` | 4 | Allowance for one IP over a sliding 24 hours, 1 to 1000. Phones behind a carrier NAT share one. | +| `INVITES_PER_IP_PER_DAY` | 4 | Allowance for one IP over a sliding 24 hours, 1 to 100. Phones behind a carrier NAT share one. The global ceiling still applies to every invite. | | `INVITE_POW_DIFFICULTY` | 16 | Base proof-of-work bits. Adds 2 at 90 invites in the hour and 4 at 150. Those thresholds do not move with the ceiling. | -To undo a temporary raise, delete or restore the line and do the same two commands. +`INVITES_PER_IP_PER_DAY` needs a build containing freenet/web#227. An older binary ignores the +variable without complaint, and rolling back to one means the limit is 4 whatever the unit +says. + +**Verify after every restart:** + +```bash +systemctl is-active gkapi +sudo journalctl -u gkapi -n 50 | grep 'Per-IP invite limit' +``` + +The log line is the authority on the limit in force. Check that its timestamp is from this +restart: an older line means the new process has not started. No line means the binary is too +old, or the invite endpoint is not configured. + +**A bad or blank value stops the whole service, donations included.** Each variable is parsed +at startup, so gkapi exits on an invalid value. Under `Restart=always` it then loops every few +seconds without any alert, so don't rely on the restart command to report it. + +**Leave a dated comment beside any raise in the unit file**, saying why and when it should come +back down, and update the existing comment there that explains the ceiling. A changed +`INVITES_PER_IP_PER_DAY` is logged as a warning at each start, and nothing else will remind +anyone. + +To undo a temporary raise, delete or restore the line, do the same two commands and verify +again. ## Notary keys and the monthly schedule diff --git a/rust/api/src/main.rs b/rust/api/src/main.rs index f54c182d..75a2160e 100644 --- a/rust/api/src/main.rs +++ b/rust/api/src/main.rs @@ -318,7 +318,7 @@ async fn main() { .long("invites-per-ip-per-day") .value_name("N") .env("INVITES_PER_IP_PER_DAY") - .value_parser(value_parser!(u32).range(1..=1000)) + .value_parser(value_parser!(u32).range(1..=100)) .help("Invitations one IP may receive in 24 hours (default 4)."), ) .arg( diff --git a/rust/api/src/rate_limit.rs b/rust/api/src/rate_limit.rs index 89e4e72a..6e805f52 100644 --- a/rust/api/src/rate_limit.rs +++ b/rust/api/src/rate_limit.rs @@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use std::collections::{HashMap, VecDeque}; use std::fs; +use std::io::Write; use std::net::IpAddr; use std::path::PathBuf; use std::sync::Mutex; @@ -15,16 +16,16 @@ use thiserror::Error; /// Default number of invites allowed per IP within the time window. /// -/// Runtime-configurable via `INVITES_PER_IP_PER_DAY`, so a launch can raise it -/// and put it back without a new binary. This default is what applies when -/// that is unset. +/// This is only the default: `INVITES_PER_IP_PER_DAY` overrides it at startup, +/// so a launch can raise it and put it back without a new binary. The startup +/// log line `Per-IP invite limit: ...` is the authority on the deployed value. /// -/// Kept deliberately low as an anti-spam measure for the freenet.org/quickstart +/// The default is kept deliberately low as an anti-spam measure for the freenet.org/quickstart /// flow, which mints invites into the shared "Freenet Official" room. A few /// invites per day covers legitimate re-invites while still slowing bulk -/// account creation. Raising this without a matching anti-abuse story re-opens -/// the vector this limit exists to slow. See -/// `test_rate_limiter_enforces_four_per_window`. +/// account creation. Raising it, here or through the environment, without a +/// matching anti-abuse story re-opens the vector this limit exists to slow. See +/// `test_rate_limiter_enforces_four_per_window`, which pins the default. /// /// NOTE: this limit is per-IP and therefore cannot bound an actor who rotates /// IPs. The global emergency ceiling and proof of work provide that bound. @@ -91,9 +92,10 @@ impl RateLimiter { } } - /// Replace the default per-IP allowance. + /// Replace the default per-IP allowance. At least 1: a limit of 0 could + /// refuse every request with no time at which asking again would work. pub fn with_max_per_window(mut self, max_per_window: usize) -> Self { - self.max_per_window = max_per_window; + self.max_per_window = max_per_window.max(1); self } @@ -228,7 +230,17 @@ impl RateLimiter { fs::create_dir_all(parent)?; } let content = serde_json::to_string_pretty(data)?; - fs::write(&self.data_path, content)?; + + // Write to a temporary file and rename it into place, so a process + // stopped mid-write leaves the previous file whole. A truncated file + // fails to parse, which refuses every invite until it is repaired. + // `self.lock` is held by every caller, so one temporary path is enough. + let mut temp_path = self.data_path.clone().into_os_string(); + temp_path.push(".tmp"); + let mut temp_file = fs::File::create(&temp_path)?; + temp_file.write_all(content.as_bytes())?; + temp_file.sync_all()?; + fs::rename(&temp_path, &self.data_path)?; Ok(()) } } @@ -549,7 +561,9 @@ mod tests { assert!(limiter.check_and_record(ip).unwrap()); } - /// Pins the deployed anti-spam limit at exactly 4 invites / IP / window. + /// Pins the DEFAULT anti-spam limit at exactly 4 invites / IP / window. The + /// deployed value can differ: see `INVITES_PER_IP_PER_DAY` and the startup + /// log. /// /// The other tests are parameterized on `MAX_INVITES_PER_WINDOW`, so they /// pass at any value and would NOT catch an accidental bump of the @@ -559,7 +573,7 @@ mod tests { fn test_rate_limiter_enforces_four_per_window() { assert_eq!( MAX_INVITES_PER_WINDOW, 4, - "invite anti-spam limit must stay at 4/IP/window; raising it re-opens bulk account creation" + "default invite anti-spam limit must stay at 4/IP/window; raising it re-opens bulk account creation" ); let dir = tempdir().unwrap(); @@ -598,16 +612,9 @@ mod tests { ); } - /// Lowering the limit after a raise leaves IPs holding more than the new - /// limit. They can ask again once enough entries have expired, not when - /// the oldest one does. - #[test] - fn retry_after_counts_down_to_below_the_limit_when_over_it() { - let dir = tempdir().unwrap(); - let path = dir.path().join("rate_limits.json"); - let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 9)); + /// Store one IP's invites, given as hours before now. + fn write_invites(path: &std::path::Path, ip: IpAddr, hours_ago: &[i64]) { let now = Utc::now(); - let hours_ago = [23, 20, 10, 5, 1, 0]; let timestamps = hours_ago .iter() .map(|h| (now - Duration::hours(*h)).to_rfc3339()) @@ -615,7 +622,18 @@ mod tests { let data = RateLimitData { invites: HashMap::from([(ip.to_string(), timestamps)]), }; - fs::write(&path, serde_json::to_string(&data).unwrap()).unwrap(); + fs::write(path, serde_json::to_string(&data).unwrap()).unwrap(); + } + + /// Lowering the limit after a raise leaves IPs holding more than the new + /// limit. They can ask again once enough entries have expired, not when + /// the oldest one does. + #[test] + fn retry_after_counts_down_to_below_the_limit_when_over_it() { + let dir = tempdir().unwrap(); + let path = dir.path().join("rate_limits.json"); + let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 9)); + write_invites(&path, ip, &[23, 20, 10, 5, 1, 0]); let retry = RateLimiter::new(path, 24) .with_max_per_window(4) @@ -632,6 +650,78 @@ mod tests { ); } + /// The retry time above is only honest if admission agrees: the request + /// is refused until the third oldest entry has aged out, and not after. + #[test] + fn admission_reopens_when_the_retry_time_says_it_does() { + let dir = tempdir().unwrap(); + let path = dir.path().join("rate_limits.json"); + let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 10)); + let limiter = RateLimiter::new(path.clone(), 24).with_max_per_window(4); + + write_invites(&path, ip, &[23, 20, 10, 5, 1, 0]); + assert!(!limiter.check_and_record(ip).unwrap(), "six held"); + + write_invites(&path, ip, &[25, 25, 10, 5, 1, 0]); + assert!( + !limiter.check_and_record(ip).unwrap(), + "four still held after the two oldest aged out" + ); + + write_invites(&path, ip, &[25, 25, 25, 5, 1, 0]); + assert!( + limiter.check_and_record(ip).unwrap(), + "three held after the three oldest aged out" + ); + } + + #[test] + fn zero_limit_is_treated_as_one() { + let dir = tempdir().unwrap(); + let limiter = + RateLimiter::new(dir.path().join("rate_limits.json"), 24).with_max_per_window(0); + let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 11)); + + assert_eq!(limiter.max_per_window(), 1); + assert!(limiter.check_and_record(ip).unwrap()); + assert!(!limiter.check_and_record(ip).unwrap()); + assert!(limiter.get_retry_after(ip).unwrap().is_some()); + } + + #[test] + fn save_leaves_no_temporary_file_behind() { + let dir = tempdir().unwrap(); + let limiter = RateLimiter::new(dir.path().join("rate_limits.json"), 24); + + limiter + .check_and_record(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 12))) + .unwrap(); + + let names: Vec<_> = fs::read_dir(dir.path()) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .collect(); + assert_eq!(names, ["rate_limits.json"]); + } + + /// A write that fails part way must not touch the live file. The directory + /// in the temporary file's place makes the write fail; writing the live + /// file directly would succeed and this would not see an error. + #[test] + fn failed_save_leaves_the_existing_file_whole() { + let dir = tempdir().unwrap(); + let path = dir.path().join("rate_limits.json"); + let ip = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 13)); + let limiter = RateLimiter::new(path.clone(), 24); + assert!(limiter.check_and_record(ip).unwrap()); + let before = fs::read_to_string(&path).unwrap(); + + fs::create_dir(dir.path().join("rate_limits.json.tmp")).unwrap(); + + assert!(limiter.check_and_record(ip).is_err()); + assert_eq!(fs::read_to_string(&path).unwrap(), before); + } + #[test] fn test_rate_limiter_allows_up_to_max_requests() { let dir = tempdir().unwrap(); diff --git a/rust/api/src/routes.rs b/rust/api/src/routes.rs index be731f06..9f4fd948 100644 --- a/rust/api/src/routes.rs +++ b/rust/api/src/routes.rs @@ -59,10 +59,20 @@ impl InviteState { inviter_signing_key: SigningKey, room_name: String, ) -> Self { - let invites_per_ip = invites_per_ip_per_day.unwrap_or(MAX_INVITES_PER_WINDOW); + let rate_limiter = Arc::new( + RateLimiter::new(rate_limit_file, 24) + .with_max_per_window(invites_per_ip_per_day.unwrap_or(MAX_INVITES_PER_WINDOW)), + ); + let invites_per_ip = rate_limiter.max_per_window(); + if invites_per_ip != MAX_INVITES_PER_WINDOW { + warn!( + "INVITES_PER_IP_PER_DAY is {invites_per_ip}, not the default of \ + {MAX_INVITES_PER_WINDOW}. Remove it from the unit file if this is left over." + ); + } + // This wording is a contract: tests/invite_limit_env.rs and the verify + // step in rust/api/README.md both match it. Change them together. info!("Per-IP invite limit: {invites_per_ip} per 24 hours"); - let rate_limiter = - Arc::new(RateLimiter::new(rate_limit_file, 24).with_max_per_window(invites_per_ip)); let tor_exits = Arc::new(TorExitList::new(tor_exit_cache)); let recent_ages = match rate_limiter.recent_events(GLOBAL_WINDOW_MINUTES) { // Seed only traffic the new policy would have admitted. Otherwise @@ -1184,6 +1194,8 @@ mod invite_handler_tests { state.rate_limiter = Arc::new( RateLimiter::new(dir.path().join("configured.json"), 24).with_max_per_window(limit), ); + // Taken while the allowance remains, to reach create's own refusal. + let spare_proof = solve(state.pow.issue(state.global_bucket.current()).challenge); for i in 0..limit { assert_eq!( @@ -1205,6 +1217,23 @@ mod invite_handler_tests { "the refusal must state the configured limit, got: {}", body.error ); + + let refused = create_room_invite( + State(state.clone()), + ConnectInfo(addr("127.0.0.1")), + proxied("198.51.100.4"), + Json(spare_proof), + ) + .await; + let Err((status, body)) = refused else { + panic!("the allowance is used up, so create must refuse"); + }; + assert_eq!(status, StatusCode::TOO_MANY_REQUESTS); + assert!( + body.error.contains(&format!("up to {limit} invites")), + "create's refusal must state the configured limit, got: {}", + body.error + ); } #[tokio::test] diff --git a/rust/api/src/tor.rs b/rust/api/src/tor.rs index 6d26bcd8..253819fb 100644 --- a/rust/api/src/tor.rs +++ b/rust/api/src/tor.rs @@ -2,8 +2,9 @@ //! //! # Why this exists //! -//! gkapi's per-IP invite limit ([`crate::rate_limit::MAX_INVITES_PER_WINDOW`]) -//! is structurally defeated by Tor circuit rotation: every new exit node +//! gkapi's per-IP invite limit (by default +//! [`crate::rate_limit::MAX_INVITES_PER_WINDOW`]; the startup log gives the +//! deployed value) is structurally defeated by Tor circuit rotation: every new exit node //! presents a fresh IP, and therefore a fresh bucket. //! //! Measured from `invite_rate_limits.json` (1728 invites, diff --git a/rust/api/tests/invite_limit_env.rs b/rust/api/tests/invite_limit_env.rs index 083d73f2..f94b1e87 100644 --- a/rust/api/tests/invite_limit_env.rs +++ b/rust/api/tests/invite_limit_env.rs @@ -1,12 +1,16 @@ //! `INVITES_PER_IP_PER_DAY` has to reach the running service. The unit tests in //! `rate_limit.rs` and `routes.rs` set the limit directly, so they stay green //! if the clap argument or its plumbing through `main.rs` is broken. +//! +//! These read the startup log rather than getting a request refused. The log +//! reports the value held by the limiter, and the unit tests cover the limiter +//! from there on. use ed25519_dalek::SigningKey; use std::io::{BufRead, BufReader}; use std::process::{Command, Stdio}; use std::sync::mpsc; -use std::time::Duration; +use std::time::{Duration, Instant}; fn gkapi(dir: &std::path::Path) -> Command { let notary = dir.join("notary"); @@ -18,8 +22,10 @@ fn gkapi(dir: &std::path::Path) -> Command { let mut command = Command::new(env!("CARGO_BIN_EXE_ghostkey-api")); command .current_dir(dir) + // Nothing inherited: the machine running the tests may set the other + // invite variables, and this one, to anything. + .env_clear() .args(["--port", "0", "--notary-dir", notary.to_str().unwrap()]) - .env_remove("INVITES_PER_IP_PER_DAY") .env("ROOM_SIGNING_KEY_FILE", signing_key) .env( "ROOM_OWNER_VK", @@ -32,55 +38,106 @@ fn gkapi(dir: &std::path::Path) -> Command { command } -/// Start gkapi and wait for the line reporting the limit it is enforcing. -fn reported_limit(mut command: Command) -> Option { +/// Start gkapi and collect its log up to the line reporting the limit it is +/// enforcing. Empty if that line never arrives. +fn log_through_limit(mut command: Command) -> Vec { let mut child = command.spawn().expect("failed to spawn ghostkey-api"); let stderr = child.stderr.take().unwrap(); let (sender, receiver) = mpsc::channel(); std::thread::spawn(move || { - let line = BufReader::new(stderr) - .lines() - .map_while(Result::ok) - .find(|line| line.contains("Per-IP invite limit")); - let _ = sender.send(line); + let mut log = Vec::new(); + for line in BufReader::new(stderr).lines().map_while(Result::ok) { + log.push(line); + if log.last().unwrap().contains("Per-IP invite limit") { + let _ = sender.send(log); + return; + } + } }); - let line = receiver + let log = receiver .recv_timeout(Duration::from_secs(30)) - .ok() - .flatten(); + .unwrap_or_default(); child.kill().unwrap(); child.wait().unwrap(); - line + log +} + +fn assert_startup_reports(log: &[String], limit: u32) { + let line = log.last().expect("gkapi never reported its limit"); + assert!( + line.contains(&format!("limit: {limit} per 24 hours")), + "got: {line}" + ); +} + +fn warns_about_override(log: &[String]) -> bool { + log.iter().any(|line| line.contains("not the default of 4")) } #[test] -fn env_var_sets_the_per_ip_limit() { +fn startup_log_reports_the_limit_from_the_env_var() { let dir = tempfile::tempdir().unwrap(); let mut command = gkapi(dir.path()); command.env("INVITES_PER_IP_PER_DAY", "7"); - let line = reported_limit(command).expect("gkapi never reported its limit"); - assert!(line.contains("limit: 7 per 24 hours"), "got: {line}"); + let log = log_through_limit(command); + assert_startup_reports(&log, 7); + assert!(warns_about_override(&log), "got: {log:?}"); } #[test] -fn per_ip_limit_defaults_to_four_when_unset() { +fn highest_accepted_limit_starts() { let dir = tempfile::tempdir().unwrap(); + let mut command = gkapi(dir.path()); + command.env("INVITES_PER_IP_PER_DAY", "100"); - let line = reported_limit(gkapi(dir.path())).expect("gkapi never reported its limit"); - assert!(line.contains("limit: 4 per 24 hours"), "got: {line}"); + assert_startup_reports(&log_through_limit(command), 100); } -/// The global ceiling uses 0 to mean "off". Here it would refuse everyone, so -/// it must stop the service at startup rather than take invites down quietly. #[test] -fn zero_per_ip_limit_is_refused_at_startup() { +fn per_ip_limit_defaults_to_four_when_unset() { + let dir = tempfile::tempdir().unwrap(); + + let log = log_through_limit(gkapi(dir.path())); + assert_startup_reports(&log, 4); + assert!(!warns_about_override(&log), "got: {log:?}"); +} + +/// Run gkapi expecting it to refuse its configuration and exit. Bounded, so a +/// value that is wrongly accepted fails here instead of serving forever. +fn stderr_of_refused_start(value: &str) -> String { let dir = tempfile::tempdir().unwrap(); let mut command = gkapi(dir.path()); - command.env("INVITES_PER_IP_PER_DAY", "0"); + command.env("INVITES_PER_IP_PER_DAY", value); + let mut child = command.spawn().expect("failed to spawn ghostkey-api"); - let output = command.output().expect("failed to run ghostkey-api"); - assert!(!output.status.success()); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!(stderr.contains("invites-per-ip-per-day"), "got: {stderr}"); + let deadline = Instant::now() + Duration::from_secs(10); + let status = loop { + if let Some(status) = child.try_wait().unwrap() { + break status; + } + if Instant::now() > deadline { + child.kill().unwrap(); + child.wait().unwrap(); + panic!("gkapi started with INVITES_PER_IP_PER_DAY={value:?}"); + } + std::thread::sleep(Duration::from_millis(20)); + }; + assert!(!status.success(), "value {value:?}"); + let output = child.wait_with_output().unwrap(); + String::from_utf8_lossy(&output.stderr).into_owned() +} + +/// The global ceiling uses 0 to mean "off". Here it would refuse everyone, so +/// it must stop the service at startup rather than take invites down quietly. +/// A blank value is what a half-edited unit file produces. +#[test] +fn invalid_per_ip_limits_are_refused_at_startup() { + for value in ["0", "101", "abc", ""] { + let stderr = stderr_of_refused_start(value); + assert!( + stderr.contains("invites-per-ip-per-day"), + "value {value:?}, got: {stderr}" + ); + } } From b8406e03ac5626841306431463d537b2210c4bd1 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 2 Nov 2026 11:01:38 -0600 Subject: [PATCH 3/3] docs(api): verify the invite limit from the current run's journal Claude-Session: https://claude.ai/code/session_01VjxvYQUzmUE7o4fLzHUmpo --- rust/api/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/api/README.md b/rust/api/README.md index ae6ab788..8520132c 100644 --- a/rust/api/README.md +++ b/rust/api/README.md @@ -102,12 +102,12 @@ says. ```bash systemctl is-active gkapi -sudo journalctl -u gkapi -n 50 | grep 'Per-IP invite limit' +sudo journalctl -u gkapi _SYSTEMD_INVOCATION_ID=$(systemctl show -p InvocationID --value gkapi) \ + | grep 'Per-IP invite limit' ``` -The log line is the authority on the limit in force. Check that its timestamp is from this -restart: an older line means the new process has not started. No line means the binary is too -old, or the invite endpoint is not configured. +The log line is the authority on the limit in force. This command reads only the current run, +so no line means the binary is too old, or the invite endpoint is not configured. **A bad or blank value stops the whole service, donations included.** Each variable is parsed at startup, so gkapi exits on an invalid value. Under `Restart=always` it then loops every few