From 21c77f67313dbf2ff39bb7704e212265d73a3d61 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Tue, 1 Sep 2026 21:17:45 -0400 Subject: [PATCH 1/2] Count sign-ups by network, not only by address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #41 — and corrects that ticket's premise while doing it. The existing limit is per ADDRESS, not per account: 50 an hour keyed on the client IP. What it misses is that more addresses is the cheapest thing an abuser buys. Fifty accounts from one address is refused. Fifty each from two hundred addresses inside one /24 is ten thousand accounts, and one request at a time it looks exactly like a busy afternoon. So the address limit stays, and a much larger ceiling now sits behind it keyed on the network. ⚠️ **/48 for IPv6, not /64.** A /64 is one machine and a customer is often handed many, so keying on it is barely better than keying on the address. A /48 is a site — the unit somebody actually rents. ⚠️ **An IPv4-mapped address is IPv4.** Treating `::ffff:203.0.113.7` as IPv6 would drop every mapped address into one enormous shared bucket, which is the same as no limit and worse: everybody in it would be punished for each other. 🔴 **Unparseable input keys on ITSELF, never on a constant.** Failing open into a shared bucket would let one attacker exhaust everybody's budget, and returning empty would make the check removable by sending nonsense. Two tests hold that, including one asserting two different garbage values do not collide. ⚠️ The allowance is deliberately eight times the per-address one. A university, an office, or a carrier behind CGNAT is thousands of real people on one network, and refusing them is a far worse outcome than admitting a determined abuser. This is a ceiling on industrial-scale registration, not a throttle on ordinary use. ⚠️ Both limits go through `consumeAll`, so a refusal by either does not spend the other — otherwise one machine could burn the network budget and lock out its neighbours. Nine tests, proven able to fail: folding IPv6 into one bucket turns eight red. 479 tests in 41 suites pass. --- Sources/FlamenetRelay/RoutesAccount.swift | 66 ++++++++++- .../SignupNetworkLimitTests.swift | 104 ++++++++++++++++++ 2 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift diff --git a/Sources/FlamenetRelay/RoutesAccount.swift b/Sources/FlamenetRelay/RoutesAccount.swift index 5b451d9..bc08415 100644 --- a/Sources/FlamenetRelay/RoutesAccount.swift +++ b/Sources/FlamenetRelay/RoutesAccount.swift @@ -17,6 +17,21 @@ struct AccountRoutes: Sendable { /// and threading the config through would make every future limit look like /// it belongs here too. let signupMaxPerHour: Int + /// New accounts per NETWORK per hour — a /24 for IPv4, a /48 for IPv6. + /// + /// 🔴 **The per-address limit above counts a rented subnet as separate + /// strangers.** Fifty accounts from one address is refused; fifty each from + /// two hundred addresses in one /24 is ten thousand accounts and looks, one + /// request at a time, exactly like a busy afternoon. The cheapest thing an + /// abuser buys is more addresses, so a limit keyed on a single address is + /// the one they can always step around. + /// + /// ⚠️ **Deliberately generous.** A university, an office, or a carrier + /// behind CGNAT is thousands of real people sharing one network, and + /// refusing them is a far worse outcome than admitting a determined abuser. + /// This is a ceiling on industrial-scale registration, not a throttle on + /// ordinary use. + var signupMaxPerNetworkPerHour: Int = signupMaxPerNetworkDefault /// Five failures per IP+username per fifteen minutes, matching the plugin. /// @@ -68,6 +83,9 @@ struct AccountRoutes: Sendable { /// speed bump against lazy bulk registration. A speed bump whose false /// positives are "cannot use the product at all" should be generous. static let signupMaxPerHourDefault = 50 + /// Eight times the per-address allowance: enough that a shared network of + /// real people never notices, small enough that a subnet cannot be farmed. + static let signupMaxPerNetworkDefault = 400 // MARK: - POST /account/signup @@ -75,8 +93,15 @@ struct AccountRoutes: Sendable { // Charged BEFORE any validation, deliberately. Charging only on success // leaves an unlimited username-availability oracle — and charging only // after the format checks lets a script probe shapes for free. - guard await limits.consume("signup:\(clientIP)", - limit: signupMaxPerHour, now: now) else { + // ⚠️ Both together through `consumeAll`, so a refusal by either does not + // spend the other — the note on `RateLimiting` says why, and it matters + // more here: burning the network budget on requests the address limit + // already refused would let one machine lock out its neighbours. + guard await limits.consumeAll([ + (key: "signup:\(clientIP)", amount: 1, limit: signupMaxPerHour), + (key: "signup:net:\(Self.network(of: clientIP))", amount: 1, + limit: signupMaxPerNetworkPerHour), + ], now: now) else { throw WireError.rateLimited( "Too many accounts created from here recently. Try again later.") } @@ -424,6 +449,43 @@ struct AccountRoutes: Sendable { token: token, expires_in: issuer.lifetime, audience: issuer.audience)) } + /// The network an address belongs to: a /24 for IPv4, a /48 for IPv6. + /// + /// ⚠️ **/48 rather than /64 for IPv6.** A /64 is one machine and a customer + /// is often handed many, so keying on it is barely better than keying on + /// the address. A /48 is a site — the unit somebody actually rents, and + /// therefore the unit worth counting. + /// + /// ⚠️ An address this cannot parse returns itself, so an odd or spoofed + /// value degrades to the per-address limit rather than to no limit at all. + /// Failing open here would make the whole check removable by sending + /// nonsense. + static func network(of ip: String) -> String { + let address = ip.trimmingCharacters(in: .whitespaces).lowercased() + // IPv4-mapped IPv6 (`::ffff:203.0.113.7`) is an IPv4 address wearing a + // hat; treating it as IPv6 would put every mapped address in one + // enormous shared bucket. + let bare = address.hasPrefix("::ffff:") ? String(address.dropFirst(7)) : address + + if bare.contains(".") { + let octets = bare.split(separator: ".") + guard octets.count == 4, octets.allSatisfy({ UInt8($0) != nil }) else { return ip } + return octets.prefix(3).joined(separator: ".") + ".0/24" + } + if bare.contains(":") { + // The first three groups are the /48. `::` cannot hide any of them + // unless the address starts with it, and then there is nothing to + // key on anyway. + guard !bare.hasPrefix("::") else { return ip } + let groups = bare.split(separator: ":", omittingEmptySubsequences: false) + guard groups.count >= 3, groups.prefix(3).allSatisfy({ !$0.isEmpty }) else { + return ip + } + return groups.prefix(3).joined(separator: ":") + "::/48" + } + return ip + } + // MARK: - Credentials /// A credential is `.`. The selector is a lookup key held diff --git a/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift b/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift new file mode 100644 index 0000000..a44818c --- /dev/null +++ b/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift @@ -0,0 +1,104 @@ +import Foundation +import Testing + +@testable import FlamenetRelay + +/// Counting sign-ups by network, not only by address. +/// +/// 🔴 **The per-address limit counts a rented subnet as separate strangers.** +/// Fifty accounts from one address is refused; fifty each from two hundred +/// addresses in one /24 is ten thousand accounts, and one request at a time it +/// looks exactly like a busy afternoon. More addresses is the cheapest thing an +/// abuser buys, so an address-keyed limit is the one they always step around. +/// +/// ⚠️ **The expensive failure is the other one.** A university, an office, or a +/// carrier behind CGNAT is thousands of real people on one network. Half of +/// this suite is about not folding addresses together that should stay apart. +@Suite("sign-up network limits") +struct SignupNetworkLimitTests { + + // MARK: - What counts as one network + + @Test("IPv4 addresses in one /24 share a network") + func ipv4SameNetwork() { + let key = AccountRoutes.network(of: "203.0.113.7") + #expect(key == "203.0.113.0/24") + for other in ["203.0.113.1", "203.0.113.254", "203.0.113.0"] { + #expect(AccountRoutes.network(of: other) == key) + } + } + + /// 🔴 A neighbouring /24 is somebody else. Folding these together would + /// make one abuser able to lock out an unrelated network. + @Test("a neighbouring /24 is a different network") + func ipv4NeighbourIsDifferent() { + #expect(AccountRoutes.network(of: "203.0.113.7") + != AccountRoutes.network(of: "203.0.114.7")) + #expect(AccountRoutes.network(of: "203.0.113.7") + != AccountRoutes.network(of: "198.51.100.7")) + } + + @Test("IPv6 addresses in one /48 share a network") + func ipv6SameNetwork() { + let key = AccountRoutes.network(of: "2001:db8:1234:5678::1") + #expect(key == "2001:db8:1234::/48") + // A different /64 inside the same /48 is the same site. + #expect(AccountRoutes.network(of: "2001:db8:1234:9999::abcd") == key) + } + + /// ⚠️ /48 not /64, and this is the case that says why: a customer handed + /// many /64s would otherwise get a fresh budget for each one. + @Test("a different /48 is a different network") + func ipv6NeighbourIsDifferent() { + #expect(AccountRoutes.network(of: "2001:db8:1234:5678::1") + != AccountRoutes.network(of: "2001:db8:9999:5678::1")) + } + + /// An IPv4-mapped address is an IPv4 address wearing a hat. Treating it as + /// IPv6 would drop every mapped address into one enormous shared bucket — + /// which is the same as having no limit, and worse, would punish everybody + /// in it for each other. + @Test("an IPv4-mapped IPv6 address is treated as IPv4") + func mappedAddressIsIPv4() { + #expect(AccountRoutes.network(of: "::ffff:203.0.113.7") == "203.0.113.0/24") + #expect(AccountRoutes.network(of: "::FFFF:203.0.113.9") == "203.0.113.0/24") + } + + /// 🔴 **Fails closed, not open.** Something unparseable degrades to the + /// per-address limit. If it collapsed to a constant instead, one attacker + /// could exhaust everybody's shared budget; if it returned empty, the check + /// would be removable by sending nonsense. + @Test("an address that cannot be parsed keys on itself") + func malformedKeysOnItself() { + for odd in ["", "not-an-address", "203.0.113", "203.0.113.999", + "999.999.999.999", "::", "::1"] { + #expect(AccountRoutes.network(of: odd) == odd, + "\(odd) should key on itself rather than a shared bucket") + } + } + + /// Two unparseable values must not land in the same bucket either. + @Test("two unparseable addresses do not share a bucket") + func malformedAreNotFoldedTogether() { + #expect(AccountRoutes.network(of: "garbage-one") + != AccountRoutes.network(of: "garbage-two")) + } + + /// Whitespace and case are not a way to get a fresh budget. + @Test("padding and case do not create a new network") + func normalisedBeforeKeying() { + #expect(AccountRoutes.network(of: " 203.0.113.7 ") == "203.0.113.0/24") + #expect(AccountRoutes.network(of: "2001:DB8:1234:5678::1") == "2001:db8:1234::/48") + } + + // MARK: - The ceiling itself + + /// ⚠️ Generous by design. A shared network of real people must never meet + /// this, so it sits well above the per-address allowance rather than beside + /// it. + @Test("the network allowance is far larger than the address allowance") + func networkAllowanceIsGenerous() { + #expect(AccountRoutes.signupMaxPerNetworkDefault + > AccountRoutes.signupMaxPerHourDefault * 4) + } +} -- 2.54.0 From e7e1a787551ebde31da2446e02bf14442305d63e Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Wed, 2 Sep 2026 15:34:32 -0400 Subject: [PATCH 2/2] Give network counting a switch, with zero meaning off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FN_SIGNUP_MAX_PER_NETWORK, defaulting to the generous 400/hr the ceiling was already written for. On by default: a limit keyed on a single address is the one an abuser always steps around, and turning that off by default would leave the hole this closes. 🔴 Zero means OFF, and the bucket is left out of the list rather than passed as a limit of zero. A zero limit handed to the limiter refuses every sign-up on the relay — an operator setting this to 0 meaning "stop counting networks" would take the door off its hinges instead. Pinned by a test that signs two accounts up from one /24 with the limit at zero. Also tested: a low ceiling really does refuse the second account from the same /24 (the case per-address counting cannot see), and an unset variable leaves the default in place rather than silently disabling it. No effect on the Android build in the field: v8 signs up well under 400 per network per hour, and the default is unchanged from what this MR already had. 482 pass. --- Resources/Views/configuration.leaf | 13 ++- Sources/FlamenetRelay/Config.swift | 11 +++ Sources/FlamenetRelay/Entrypoint.swift | 3 +- Sources/FlamenetRelay/RoutesAccount.swift | 15 ++-- .../SignupNetworkLimitTests.swift | 85 +++++++++++++++++++ docs/CONFIGURATION.md | 1 + 6 files changed, 121 insertions(+), 7 deletions(-) diff --git a/Resources/Views/configuration.leaf b/Resources/Views/configuration.leaf index 06d1c1a..8f85927 100644 --- a/Resources/Views/configuration.leaf +++ b/Resources/Views/configuration.leaf @@ -3,7 +3,7 @@ + source-sha256: 00f2d337316548ad -->

Configuring a relay

@@ -310,6 +310,17 @@ rotated, so treat this as a speed bump against lazy bulk registration rather than a defence. +FN_SIGNUP_MAX_PER_NETWORK +400/hr +New accounts per network — a /24 for IPv4, a /48 for IPv6. The +per-address limit counts a rented subnet as separate strangers, and more +addresses is the cheapest thing an abuser buys. Deliberately generous: a +campus or a carrier behind CGNAT is thousands of real people on one +network, and refusing them is the worse failure. 0 +switches network counting off, leaving only the per-address +limit. + + FN_SEND_RATE 600/hr Envelopes, per sender or per recipient when sealed. diff --git a/Sources/FlamenetRelay/Config.swift b/Sources/FlamenetRelay/Config.swift index 980bb60..6fb97f1 100644 --- a/Sources/FlamenetRelay/Config.swift +++ b/Sources/FlamenetRelay/Config.swift @@ -108,6 +108,15 @@ struct Config: Sendable { /// New accounts per IP per hour. See RoutesAccount.signupMaxPerHour. let signupMaxPerHour: Int + + /// New accounts per network per hour — a /24 for IPv4, a /48 for IPv6. + /// + /// On by default and deliberately generous: the per-address limit counts a + /// rented subnet as separate strangers, and more addresses is the cheapest + /// thing an abuser buys. **`0` switches it off**, leaving only the + /// per-address limit — for an operator who would rather admit an abuser + /// than risk refusing a campus. + let signupMaxPerNetworkPerHour: Int let attachmentBytesPerHour: Int /// The rolling 30-day per-account upload budget (C3). The hourly limits /// above stop bursts; this stops a steady drip from quietly owning the @@ -231,6 +240,8 @@ struct Config: Sendable { attachmentMaxBytes: int("FN_ATTACHMENT_MAX_BYTES", 10 * 1024 * 1024), attachmentRatePerHour: int("FN_ATTACHMENT_RATE", 60), signupMaxPerHour: int("FN_SIGNUP_MAX_PER_HOUR", AccountRoutes.signupMaxPerHourDefault), + signupMaxPerNetworkPerHour: + int("FN_SIGNUP_MAX_PER_NETWORK", AccountRoutes.signupMaxPerNetworkDefault), attachmentBytesPerHour: int("FN_ATTACHMENT_BYTES_RATE", 200 * 1024 * 1024), attachmentBytesPerMonth: int("FN_ATTACHMENT_BYTES_PER_MONTH", 500 * 1024 * 1024), attachmentRetentionDays: int("FN_ATTACHMENT_RETENTION_DAYS", 30), diff --git a/Sources/FlamenetRelay/Entrypoint.swift b/Sources/FlamenetRelay/Entrypoint.swift index df5e0a5..a077467 100644 --- a/Sources/FlamenetRelay/Entrypoint.swift +++ b/Sources/FlamenetRelay/Entrypoint.swift @@ -327,7 +327,8 @@ func configure(_ app: Application) async throws { audience: config.audience, lifetime: config.tokenLifetime), limits: makeLimiter("signin", AccountRoutes.signinLockout), - signupMaxPerHour: config.signupMaxPerHour) + signupMaxPerHour: config.signupMaxPerHour, + signupMaxPerNetworkPerHour: config.signupMaxPerNetworkPerHour) try register(account: accounts, config: config, on: app) // The shipped iOS client's surface, mounted where it already looks. diff --git a/Sources/FlamenetRelay/RoutesAccount.swift b/Sources/FlamenetRelay/RoutesAccount.swift index bc08415..a51bdd1 100644 --- a/Sources/FlamenetRelay/RoutesAccount.swift +++ b/Sources/FlamenetRelay/RoutesAccount.swift @@ -97,11 +97,16 @@ struct AccountRoutes: Sendable { // spend the other — the note on `RateLimiting` says why, and it matters // more here: burning the network budget on requests the address limit // already refused would let one machine lock out its neighbours. - guard await limits.consumeAll([ - (key: "signup:\(clientIP)", amount: 1, limit: signupMaxPerHour), - (key: "signup:net:\(Self.network(of: clientIP))", amount: 1, - limit: signupMaxPerNetworkPerHour), - ], now: now) else { + // + // ⚠️ `0` switches the network bucket off, and it is left OUT of the list + // rather than passed as a limit of zero — a zero limit would refuse + // every sign-up on the relay, which is the exact opposite of off. + var buckets = [(key: "signup:\(clientIP)", amount: 1, limit: signupMaxPerHour)] + if signupMaxPerNetworkPerHour > 0 { + buckets.append((key: "signup:net:\(Self.network(of: clientIP))", amount: 1, + limit: signupMaxPerNetworkPerHour)) + } + guard await limits.consumeAll(buckets, now: now) else { throw WireError.rateLimited( "Too many accounts created from here recently. Try again later.") } diff --git a/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift b/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift index a44818c..9ddc8ef 100644 --- a/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift +++ b/Tests/FlamenetRelayTests/SignupNetworkLimitTests.swift @@ -1,5 +1,8 @@ +import Crypto +import Fluent import Foundation import Testing +import Vapor @testable import FlamenetRelay @@ -101,4 +104,86 @@ struct SignupNetworkLimitTests { #expect(AccountRoutes.signupMaxPerNetworkDefault > AccountRoutes.signupMaxPerHourDefault * 4) } + + // MARK: - Helpers + + private let now = 1_700_000_000 + + private func routes(_ h: RelayHarness) -> AccountRoutes { + AccountRoutes( + issuer: Issuer(signingKeyRaw: Data(repeating: 1, count: 32), + audience: "test.relay", lifetime: 300), + limits: RateLimiter(), + signupMaxPerHour: 1000, + signupMaxPerNetworkPerHour: AccountRoutes.signupMaxPerNetworkDefault) + } + + private func post(_ h: RelayHarness, _ body: String) -> Request { + var headers = HTTPHeaders() + headers.contentType = .json + var buf = ByteBufferAllocator().buffer(capacity: body.utf8.count) + buf.writeString(body) + return Request(application: h.app, method: .POST, url: URI(path: "/account/signup"), + headers: headers, collectedBody: buf, + on: h.app.eventLoopGroup.next()) + } + + // MARK: - The switch + + /// 🔴 The trap this exists to catch: `0` must mean **off**, not "refuse + /// everybody". + /// + /// A limit of zero handed to the limiter refuses every sign-up on the + /// relay — an operator who set `FN_SIGNUP_MAX_PER_NETWORK=0` meaning + /// "stop counting networks" would take the whole door off its hinges + /// instead. So the bucket is left out of the list rather than passed as + /// zero, and this pins that. + @Test("zero switches network counting off and still lets people sign up") + func zeroMeansOffNotClosed() async throws { + try await withRelay { h in + var a = self.routes(h) + a.signupMaxPerNetworkPerHour = 0 + // Two accounts from the same /24. With a zero limit read as a + // limit, neither would exist. + for i in 0..<2 { + _ = try await a.signup( + self.post(h, #"{"username":"net\#(i)","email":"net\#(i)@example.com","password":"hunter2hunter2"}"#), + clientIP: "203.0.113.\(i)", now: self.now) + } + #expect(try await User.query(on: h.app.db).count() == 2) + } + } + + /// And with it on, the ceiling is real. + @Test("a low ceiling refuses the account past it") + func theCeilingHolds() async throws { + try await withRelay { h in + var a = self.routes(h) + a.signupMaxPerNetworkPerHour = 1 + _ = try await a.signup( + self.post(h, #"{"username":"first","email":"first@example.com","password":"hunter2hunter2"}"#), + clientIP: "203.0.113.1", now: self.now) + // A different address, the same /24 — this is the case the + // per-address limit cannot see. + await #expect(throws: WireError.self) { + _ = try await a.signup( + self.post(h, #"{"username":"second","email":"second@example.com","password":"hunter2hunter2"}"#), + clientIP: "203.0.113.2", now: self.now) + } + } + } + + /// Unset means the generous default, never off by accident. + @Test("an unset variable leaves the ceiling in place") + func unsetKeepsTheDefault() throws { + let env = [ + "FN_ISSUER_PUBLIC_KEY": Curve25519.Signing.PrivateKey() + .publicKey.rawRepresentation.base64EncodedString(), + "FN_RELAY_AUDIENCE": "test.relay", + ] + #expect(try Config.fromEnvironment(env).signupMaxPerNetworkPerHour + == AccountRoutes.signupMaxPerNetworkDefault) + var on = env; on["FN_SIGNUP_MAX_PER_NETWORK"] = "0" + #expect(try Config.fromEnvironment(on).signupMaxPerNetworkPerHour == 0) + } } diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 4a0f7a7..95c3733 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -124,6 +124,7 @@ Defaults are sane; raise them only with a reason. | `FN_BUNDLE_RATE` | `120`/hr | Prekey-bundle fetches per requester. Each pops a one-time prekey per device, so an unmetered version is a pool drain. | | `FN_BUNDLE_RATE_TARGET` | `6`/hr | Per requester-target pair. Deliberately **not** metered per target alone — that would let one abuser make a popular account unstartable for everyone. | | `FN_SIGNUP_MAX_PER_HOUR` | `50`/hr | New accounts per IP. ⚠️ An IP is not a person — carrier CGNAT, offices and campuses all share one. It was `5`, which refused the sixth user behind any shared connection. Per-IP is trivially rotated, so treat this as a speed bump against lazy bulk registration rather than a defence. | +| `FN_SIGNUP_MAX_PER_NETWORK` | `400`/hr | New accounts per network — a /24 for IPv4, a /48 for IPv6. The per-address limit counts a rented subnet as separate strangers, and more addresses is the cheapest thing an abuser buys. Deliberately generous: a campus or a carrier behind CGNAT is thousands of real people on one network, and refusing them is the worse failure. **`0` switches network counting off**, leaving only the per-address limit. | | `FN_SEND_RATE` | `600`/hr | Envelopes, per sender or per recipient when sealed. | | `FN_ATTACHMENT_RATE` | `60`/hr | | | `FN_ATTACHMENT_BYTES_RATE` | `200 MiB`/hr | | -- 2.54.0