diff --git a/daemon/src/feedstore.rs b/daemon/src/feedstore.rs index bb1874e..d643d4f 100644 --- a/daemon/src/feedstore.rs +++ b/daemon/src/feedstore.rs @@ -1,19 +1,110 @@ -//! The feed directory: one `twtxt.txt` per username under `/feeds/`. -//! The path is the verified smolmail username, so usernames are validated +//! The feed directory: one `twtxt.txt` per feed under `/feeds/`. +//! The path is a slug derived from the verified smolmail address, validated //! before ever touching the filesystem. +//! +//! Two guards live here and must not be conflated. `valid_username` is the +//! *nick* rule, narrow on purpose because a nick lands in `# nick =` metadata +//! that third-party twtxt clients read. `valid_slug` is the *path* rule, wider +//! because a remote author's slug carries their host. use std::io; use std::path::{Path, PathBuf}; use smolboid_core::Feed; -/// 1-30 bytes of `[a-z0-9-]`: a valid feed path segment and nick. +/// 1-30 bytes of `[a-z0-9-]`: the nick rule. Deliberately unchanged and +/// deliberately not the path rule — see the module header. pub fn valid_username(name: &str) -> bool { !name.is_empty() && name.len() <= 30 && name.bytes().all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == b'-') } +/// Separators a local part may carry, per SPEC.md sec 3. +const SEPARATORS: &[u8] = b"._-"; + +/// The feed path slug: a bare local part for an author on our own server, +/// `local@host` or `local@host:port` for anyone else. +/// +/// This is the path guard, and it is independent of both `valid_username` and +/// fumi's address grammar so it cannot widen when either of those does. Before +/// it existed a path could not contain a dot, which is *why* traversal was +/// unreachable; now dots are legal and every rejection below is load-bearing. +pub fn valid_slug(name: &str) -> bool { + if name.is_empty() || name.len() > 200 || !name.is_ascii() { + return false; + } + // Lowercase is required rather than normalised. smolmail lowercases + // usernames and DNS is case-insensitive, so an uppercase slug is a caller + // bug — and folding it here would let two spellings race for one file on a + // case-insensitive filesystem. + if name.bytes().any(|c| c.is_ascii_uppercase()) { + return false; + } + let (local, rest) = match name.split_once('@') { + None => (name, None), + Some((local, rest)) => (local, Some(rest)), + }; + if !valid_local(local) { + return false; + } + let Some(rest) = rest else { + return true; + }; + // A second `@` lands in `rest` and fails `valid_host`. + let (host, port) = match rest.split_once(':') { + None => (rest, None), + Some((host, port)) => (host, Some(port)), + }; + valid_host(host) && port.is_none_or(valid_port) +} + +/// SPEC.md sec 3's username grammar, wider than `valid_username`: a legal +/// smolmail user such as `a.b` could not get a feed at all before this. +fn valid_local(local: &str) -> bool { + let bytes = local.as_bytes(); + if local.is_empty() || local.len() > 63 { + return false; + } + if !bytes + .iter() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || SEPARATORS.contains(c)) + { + return false; + } + if !bytes[0].is_ascii_alphanumeric() || !bytes[bytes.len() - 1].is_ascii_alphanumeric() { + return false; + } + // No two adjacent separators, which is also what rules out `..`. + !bytes + .windows(2) + .any(|w| SEPARATORS.contains(&w[0]) && SEPARATORS.contains(&w[1])) +} + +fn valid_host(host: &str) -> bool { + if host.is_empty() || host.len() > 120 { + return false; + } + host.split('.').all(|label| { + let bytes = label.as_bytes(); + !label.is_empty() + && label.len() <= 63 + && bytes + .iter() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || *c == b'-') + && bytes[0].is_ascii_alphanumeric() + && bytes[bytes.len() - 1].is_ascii_alphanumeric() + }) +} + +/// Never the default port: `Address::short()` omits 1961, so a slug carrying +/// it would be a second spelling of an address that already has one. +fn valid_port(port: &str) -> bool { + (1..=5).contains(&port.len()) + && port.bytes().all(|c| c.is_ascii_digit()) + && port.parse::().is_ok_and(|p| p != 1961) +} + #[derive(Clone)] pub struct FeedStore { root: PathBuf, @@ -26,11 +117,18 @@ impl FeedStore { Ok(FeedStore { root }) } - fn path(&self, username: &str) -> Result { - if !valid_username(username) { - return Err(format!("invalid username `{username}`")); + fn path(&self, slug: &str) -> Result { + if !valid_slug(slug) { + return Err(format!("invalid feed slug `{slug}`")); } - Ok(self.root.join(format!("{username}.txt"))) + let path = self.root.join(format!("{slug}.txt")); + // Defence in depth: the grammar above is far wider than the original + // `[a-z0-9-]`, so check the join actually stayed in the feed directory + // rather than trusting the character rules alone. + if path.parent() != Some(self.root.as_path()) { + return Err(format!("feed slug `{slug}` escapes the feed directory")); + } + Ok(path) } /// The feed file's mtime for `Last-Modified`; missing feeds are epoch. @@ -99,6 +197,87 @@ mod tests { assert!(!valid_username(&"a".repeat(31))); } + /// The path guard. Until `valid_slug` existed a dot could not appear in a + /// path, so traversal was structurally impossible; every rejection here is + /// now doing real work. + #[test] + fn slugs_gate_the_filesystem_path() { + for ok in [ + "tobias", + "a-b2", + "a.b", // legal smolmail user that had no feed before + "a_b", + "alice@other.org", + "alice@127.0.0.1:19641", + "alice@a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6", // an RNS destination hash + &"a".repeat(63), + &format!("a@{}.{}", "b".repeat(60), "c".repeat(59)), // 120-byte host + ] { + assert!(valid_slug(ok), "should accept {ok:?}"); + } + for bad in [ + "", + "Tobias", // uppercase is refused, never folded + "ALICE@x.org", + "alice@OTHER.org", + "a@..", + "a@.x", + "a@x.", + "a@b@c", + "@a", + "a@", + "a/b", + "a%40b", // the encoded form never reaches here + "a@b:1961", // short() omits the default port + "a@b:99999", + "a@b:0x10", + "a@b:", + "..", + "a..b", + "a._b", // two adjacent separators + ".a", + "a.", + "-a", + "a\u{0}b", + "a b", + "a@b c", + "héllo", + &"a".repeat(64), + &"a".repeat(201), + &format!("a@{}.{}", "b".repeat(60), "c".repeat(60)), // 121, over the cap + &format!("a@{}", "b".repeat(64)), // one label over 63 + ] { + assert!(!valid_slug(bad), "should reject {bad:?}"); + } + } + + /// The nick rule stays narrow even though the path rule widened; a nick + /// reaches `# nick =` metadata that other twtxt clients parse. + #[test] + fn the_nick_rule_did_not_widen_with_the_path_rule() { + for ok_as_path_only in ["a.b", "a_b", "alice@other.org", &"a".repeat(31)] { + assert!(!valid_username(ok_as_path_only), "nick must reject {ok_as_path_only:?}"); + } + assert!(valid_slug("a.b") && valid_slug("alice@other.org")); + } + + #[test] + fn remote_slugs_round_trip_through_the_store() { + let dir = std::env::temp_dir().join("smolboid-remote-slug-test"); + let _ = std::fs::remove_dir_all(&dir); + let feeds = FeedStore::new(&dir).unwrap(); + let slug = "alice@other.org"; + feeds + .create(slug, "alice", slug, "https://smol.place", &[7u8; 32]) + .unwrap(); + assert!(feeds.exists(slug).unwrap()); + assert_eq!( + feeds.load(slug).unwrap().meta("url"), + Some("https://smol.place/twtxt/alice@other.org.txt") + ); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn create_save_load_round_trip() { let dir = std::env::temp_dir().join("smolboid-feeds-test"); diff --git a/daemon/src/http.rs b/daemon/src/http.rs index 937d971..035490a 100644 --- a/daemon/src/http.rs +++ b/daemon/src/http.rs @@ -140,8 +140,19 @@ fn matches_cache(headers: &[(String, String)], etag: &str, modified: i64) -> boo } fn username_of(path: &str) -> Option { - let name = path.strip_prefix("/twtxt/")?.strip_suffix(".txt")?; - crate::feedstore::valid_username(name).then(|| name.to_owned()) + let name = path.strip_prefix("/twtxt/")?; + // Checked before any decoding: a slug is exactly one path segment, and + // decoding first would let `%2f` smuggle a separator past this. + if name.contains('/') { + return None; + } + let name = name.strip_suffix(".txt")?; + // Only `%40` is decoded, and only literally, because some twtxt clients + // percent-encode the `@` of a remote slug. No other escape is decoded, so + // nothing else can reach the filesystem — `valid_slug` rejects a stray + // `%` outright. + let name = name.replace("%40", "@"); + crate::feedstore::valid_slug(&name).then_some(name) } fn read_request(stream: &mut TcpStream) -> Option> { @@ -288,6 +299,38 @@ mod tests { (feeds, format!("{addr}")) } + /// A remote author's slug carries their host, so the route has to serve + /// the literal `@` and the `%40` some clients send for it — and nothing + /// else percent-encoded. + #[test] + fn remote_slugs_serve_at_both_spellings() { + let feeds = FeedStore::new(&dir("remote")).unwrap(); + let slug = "alice@other.org"; + feeds.create(slug, "alice", slug, "https://x", &[9; 32]).unwrap(); + let addr = format!("{}", start(feeds)).parse().unwrap(); + + for request in [ + "GET /twtxt/alice@other.org.txt HTTP/1.1\r\nHost: x\r\n\r\n", + "GET /twtxt/alice%40other.org.txt HTTP/1.1\r\nHost: x\r\n\r\n", + ] { + let response = get(&addr, request); + assert!(response.starts_with("HTTP/1.1 200 OK\r\n"), "{request}\n{response}"); + assert!(response.contains("# smol:author = alice@other.org\n"), "{response}"); + } + + // Only %40 is decoded. A separator must not arrive encoded. + for request in [ + "GET /twtxt/alice%2Fother.org.txt HTTP/1.1\r\n\r\n", + "GET /twtxt/%2e%2e/etc/passwd.txt HTTP/1.1\r\n\r\n", + "GET /twtxt/alice@other.org/../tobias.txt HTTP/1.1\r\n\r\n", + ] { + assert!( + get(&addr, request).starts_with("HTTP/1.1 404"), + "{request} should 404" + ); + } + } + #[test] fn serves_the_feed_with_the_planned_headers() { let (_, addr) = with_feed("headers"); @@ -439,7 +482,7 @@ mod tests { fn request_parsing_never_panics_and_gates_usernames() { let alphabet = [ "GET", "HEAD", " ", "/twtxt/", ".txt", "..", "%2F", "\u{0}", "\r\n", ":", "a", "-", - "\u{e9}", "\r\n\r\n", "smol:mail ", + "\u{e9}", "\r\n\r\n", "smol:mail ", "@", "%40", ".", "_", "1961", ]; let mut seed = 0x5eed_c0de_u64; for _ in 0..2000 { @@ -451,9 +494,11 @@ mod tests { if let Some((_, path, _)) = parse_request(bytes) { if let Some(name) = username_of(path) { // Whatever the parser accepts must be path-safe. - assert!(crate::feedstore::valid_username(&name), "{name:?}"); + assert!(crate::feedstore::valid_slug(&name), "{name:?}"); assert!(!name.contains('/')); assert!(!name.contains('\u{0}')); + assert!(!name.contains("..")); + assert!(!name.contains('%')); } } }