1
Fork 0
mirror of https://github.com/thegeneralist01/archivr synced 2026-07-21 18:55:36 +02:00

security: fix two Codex review findings + add port option

Fix 1 — NixOS: store/ dir missing from ReadWritePaths
  archive_path points to .archivr/; captures write artifacts to the
  sibling store/ directory. Whitelisting only .archivr/ caused POST
  /captures to fail under ProtectSystem=strict.
  Fix: use builtins.dirOf a.path to whitelist the parent, which
  contains both .archivr/ and store/.

Fix 2 — Rust: X-Forwarded-For was unconditionally trusted
  An attacker could send a different IP on every login attempt,
  bypassing LOGIN_MAX_ATTEMPTS entirely.
  Fix: use ConnectInfo<SocketAddr> (via into_make_service_with_connect_info)
  as the primary rate-limit key; XFF is trusted only when the TCP peer
  is loopback (i.e. a local reverse proxy). Tests without a real socket
  fall back to XFF unchanged.

Port configuration:
  NixOS module: split 'bind' string into separate 'listenAddress'
  (default 127.0.0.1) and 'port' (default 8080) options. openFirewall
  now uses cfg.port directly instead of parsing it from the bind string.
  README NixOS example updated accordingly.
This commit is contained in:
TheGeneralist 2026-06-29 20:32:08 +02:00
parent 7ed7cb882f
commit 71e176cbd1
Signed by: thegeneralist01
SSH key fingerprint: SHA256:pp9qddbCNmVNoSjevdvQvM5z0DHN7LTa8qBMbcMq/R4
4 changed files with 70 additions and 27 deletions

View file

@ -21,7 +21,7 @@
use std::{
collections::{HashMap, VecDeque},
net::IpAddr,
net::{IpAddr, SocketAddr},
path::PathBuf,
sync::Arc,
time::{Duration, Instant},
@ -31,7 +31,7 @@ use parking_lot::Mutex;
use archivr_core::{archive, capture, database};
use axum::{
Json, Router,
extract::{Path, Query, Request, State},
extract::{ConnectInfo, Path, Query, Request, State},
http::StatusCode,
middleware::Next,
response::{IntoResponse, Response},
@ -165,12 +165,39 @@ async fn login_rate_limit(
}
fn extract_client_ip(req: &Request) -> IpAddr {
req.headers()
.get("x-forwarded-for")
.and_then(|v| v.to_str().ok())
.and_then(|s| s.split(',').next())
.and_then(|s| s.trim().parse().ok())
.unwrap_or(IpAddr::from([127, 0, 0, 1]))
// Attempt to read the real peer address injected by
// `into_make_service_with_connect_info` in main.rs.
let peer_ip: Option<IpAddr> = req
.extensions()
.get::<ConnectInfo<SocketAddr>>()
.map(|ci| ci.0.ip());
match peer_ip {
// Peer is a loopback address → the connection came from a local
// reverse proxy (nginx/caddy on the same host). Trust the first
// address in X-Forwarded-For as the real client IP.
Some(peer) if peer.is_loopback() => req
.headers()
.get("x-forwarded-for")
.and_then(|v| v.to_str().ok())
.and_then(|s| s.split(',').next())
.and_then(|s| s.trim().parse().ok())
.unwrap_or(peer),
// Peer is a real address → use it directly; ignoring X-Forwarded-For
// prevents header-spoofing attacks.
Some(peer) => peer,
// No ConnectInfo present (unit tests using .oneshot() without a real
// socket). Fall back to XFF for test compatibility.
None => req
.headers()
.get("x-forwarded-for")
.and_then(|v| v.to_str().ok())
.and_then(|s| s.split(',').next())
.and_then(|s| s.trim().parse().ok())
.unwrap_or(IpAddr::from([127, 0, 0, 1])),
}
}
/// Build the Axum router from a pre-constructed `AppState`.