Security audit 2026-10-15: relay fixes
ci / checks (pull_request) Successful in 12s
ci / swift (pull_request) Successful in 9m10s
ci / image (pull_request) Successful in 1m49s
ci / deploy (pull_request) Skipped

- Block list: a blocked account that is unlisted and not on the caller's
  buddy list comes back as "User #<id>" with an empty slug. Rows and shape
  are unchanged, so unblock still works in both apps.
- Vouching: a suspended account can no longer be issued a vouch (403
  account_suspended).
- Federation: outbound deposit, get and fetch run the outbound address
  guard, with allowlisted peers exempt as in PeerDirectory.
- Email: control characters in an address are refused as bad_email, and
  the mailer refuses such a recipient and strips them from headers.
- Key log: inclusion, consistency, device and subject reads are metered at
  3600 per address per hour each, like the head.

Not changed: email_taken stays (both apps map it), strict account tokens
stay off until the account service stamps iss, aud and jti, and the
credential and current-password hardening needs an app update first.

swift test: 1015 of 1015 pass, 6 new.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-10-15 09:21:34 -04:00
co-authored by Claude Opus 5.5
parent ec9c542cf5
commit 842f11e952
13 changed files with 304 additions and 12 deletions
+2 -1
View File
@@ -250,7 +250,8 @@ func configure(_ app: Application) async throws {
signingKeyRaw: seed,
client: app.client,
// The same rule that admits a peer decides who we will dial.
admits: { policy == "open" || peers.contains($0) })
admits: { policy == "open" || peers.contains($0) },
pinned: policy == "allowlist" ? Set(peers) : [])
}(),
replay: replayCache,
attachmentStore: attachmentStore,
+26 -6
View File
@@ -253,6 +253,13 @@ struct FederationSender: Sendable {
let client: any Client
/// Peers this relay is willing to dial, by the same rule it admits them.
let admits: @Sendable (String) -> Bool
/// Resolves a peer name and refuses one pointing somewhere private, as
/// `PeerDirectory` does before it fetches a descriptor (relay#55). Under
/// `open` federation `to_relay` is whatever a client typed.
var addressGuard: OutboundAddressGuard = OutboundAddressGuard()
/// Allowlisted peers, dialled wherever they resolve, for the reason
/// `PeerDirectory.pinned` gives.
var pinned: Set<String> = []
enum SendError: Error, CustomStringConvertible {
case notPermitted(String)
@@ -289,13 +296,28 @@ struct FederationSender: Sendable {
return headers
}
/// Admitted, a dialable name, and resolving only to public addresses.
/// A name that resolves nowhere is unreachable, as a failed connect would
/// be; one that resolves privately is refused by policy.
private func screen(_ peer: String) async throws {
guard admits(peer), PeerDirectory.isFetchableHost(peer) else {
throw SendError.notPermitted(peer)
}
guard !pinned.contains(peer) else { return }
do {
try await addressGuard.check(peer)
} catch OutboundAddressGuard.Refusal.doesNotResolve {
throw SendError.unreachable(peer)
} catch {
throw SendError.notPermitted(peer)
}
}
/// Hand ciphertext to the relay that homes the recipient.
func deposit(
to peer: String, envelopes: [FederationDepositRequest.Item], now: Int
) async throws -> FederationDepositResponse {
guard admits(peer), PeerDirectory.isFetchableHost(peer) else {
throw SendError.notPermitted(peer)
}
try await screen(peer)
let path = "/e2e/federation/deposit"
// Encoded once, and the very bytes that were signed are the bytes sent.
let payload = try JSONEncoder().encode(FederationDepositRequest(envelopes: envelopes))
@@ -332,9 +354,7 @@ struct FederationSender: Sendable {
/// An authenticated GET at a peer, for any of the routes it exposes to us.
func get(from peer: String, path: String, now: Int) async throws -> ClientResponse {
guard admits(peer), PeerDirectory.isFetchableHost(peer) else {
throw SendError.notPermitted(peer)
}
try await screen(peer)
let headers = try signedHeaders(
method: "GET", path: path, destination: peer, body: Data(), now: now)
do {
+24 -3
View File
@@ -15,9 +15,12 @@ struct Mailer: Sendable {
case unexpected(code: Int, line: String, step: String)
case tlsRefused
case authRequiredButUnconfigured
case unsafeRecipient
var errorDescription: String? {
switch self {
case .unsafeRecipient:
return "The recipient address holds a control character or an angle bracket."
case .unexpected(let code, let line, let step):
return "SMTP \(step) got \(code): \(line)"
case .tlsRefused:
@@ -31,6 +34,11 @@ struct Mailer: Sendable {
/// Send one plain-text message.
func send(to recipient: String, subject: String, body: String) async throws {
// Sign-up refuses these already. Checked again here because the
// address is written raw into RCPT TO and the To header, where a CR or
// LF starts a command or a header of the sender's choosing.
guard !Self.containsControl(recipient), !recipient.contains("<"), !recipient.contains(">")
else { throw Failure.unsafeRecipient }
// The handler goes in during the bootstrap, NOT after connect.
let reader = LineHandler()
let channel = try await ClientBootstrap(group: group)
@@ -101,6 +109,19 @@ struct Mailer: Sendable {
return String(field[field.index(after: open)..<close])
}
/// Whether any character is a control character: CR, LF, NUL, tab, DEL.
static func containsControl(_ s: String) -> Bool {
s.unicodeScalars.contains { $0.properties.generalCategory == .control }
}
/// A header value with every control character removed, so no value can
/// end its own line.
static func headerValue(_ s: String) -> String {
String(
String.UnicodeScalarView(
s.unicodeScalars.filter { $0.properties.generalCategory != .control }))
}
/// One RFC 5322 message.
static func message(from: String, to: String, subject: String, body: String) -> String {
let stuffed =
@@ -111,9 +132,9 @@ struct Mailer: Sendable {
.joined(separator: "\r\n")
let date = ISO8601DateFormatter().string(from: Date())
return [
"From: \(from)",
"To: <\(to)>",
"Subject: \(subject)",
"From: \(headerValue(from))",
"To: <\(headerValue(to))>",
"Subject: \(headerValue(subject))",
"Date: \(date)",
"MIME-Version: 1.0",
"Content-Type: text/plain; charset=utf-8",
@@ -776,6 +776,9 @@ struct AccountRoutes: Sendable {
private func isPlausibleEmail(_ s: String) -> Bool {
let t = s.trimmingCharacters(in: .whitespacesAndNewlines)
guard t.count >= 6, t.count <= 254, !t.contains(" ") else { return false }
// CR or LF inside an address would end the SMTP command and the To
// header it is written into, so no control character gets this far.
guard !Mailer.containsControl(t) else { return false }
let parts = t.split(separator: "@")
guard parts.count == 2, !parts[0].isEmpty else { return false }
return parts[1].contains(".") && !parts[1].hasPrefix(".") && !parts[1].hasSuffix(".")
+20
View File
@@ -10,6 +10,22 @@ extension Routes {
/// checks a key, so this is far above any one person and below a loop.
static let logHeadPerHour = 3600
/// Proof, device and subject reads per address per hour, each route its
/// own bucket. Each one loads the whole log, so a loop is the cost here;
/// a client checking keys asks about as often as it reads the head.
static let logReadPerHour = 3600
/// Charge one log read to this address, or refuse with the head's 429.
private func meterLogRead(_ request: Request, _ route: String) async throws {
let now = unixNow()
let meter = "log:\(route):ip:\(clientIP(request))"
guard await limits.consume(meter, limit: Self.logReadPerHour, now: now) else {
throw WireError.rateLimited(
"Too many log requests from here recently.",
retryAfter: await limits.retryAfter([meter], now: now))
}
}
/// The signed head, from `keyLogHead`, which loads no leaves while the log
/// is unchanged.
func logHead(_ request: Request) async throws -> Response {
@@ -55,6 +71,7 @@ extension Routes {
/// Proof that the leaf at `index` is in the tree of size `size`.
func logInclusion(_ request: Request) async throws -> Response {
try await meterLogRead(request, "inclusion")
let leaves = try await KeyLog.leaves(on: request.db)
guard let index = request.query[Int.self, at: "index"] else {
throw WireError.badRequest("index is required.")
@@ -82,6 +99,7 @@ extension Routes {
/// Proof that the tree of size `from` is a prefix of the tree of size `to`.
func logConsistency(_ request: Request) async throws -> Response {
try await meterLogRead(request, "consistency")
let leaves = try await KeyLog.leaves(on: request.db)
guard let from = request.query[Int.self, at: "from"] else {
throw WireError.badRequest("from is required.")
@@ -105,6 +123,7 @@ extension Routes {
/// Where a device's entries are, so a client can find its own key without
/// scanning the log.
func logDevice(_ request: Request, deviceID: String) async throws -> Response {
try await meterLogRead(request, "device")
guard isValidDeviceID(deviceID) else {
throw WireError.malformedDeviceID
}
@@ -130,6 +149,7 @@ extension Routes {
/// §19, every entry published under one subject (relay#32).
func logSubjectEntries(_ request: Request) async throws -> Response {
try await meterLogRead(request, "subject")
// A code, not a 404: a relay that has enumeration switched off still
// serves the log head and the proofs, and a monitor should say "this
// relay does not answer that" rather than "there is no log here".
+22 -2
View File
@@ -55,18 +55,38 @@ extension MobileRoutes {
// MARK: - Blocks
/// Every row the caller wrote comes back, because the id is what unblocks.
/// The name and username come back only for an account the caller could
/// already resolve: a listed one, or one on the caller's own buddy list.
/// `toggleBlock` takes any id, so naming every blocked id would turn
/// "block 1..N, then list" into a directory of Unlisted accounts. An
/// account the caller may not see, and an id nobody holds, both answer
/// `User #<id>` with an empty slug (design B), which both apps render.
func blocks(_ request: Request) async throws -> Response {
let me = try await caller(request)
let rows = try await Block.query(on: request.db)
.filter(\.$userID == me).sort(\.$id, .ascending).all()
var out: [BlockListResponse.Row] = []
for row in rows {
guard let u = try await User.find(row.blockedID, on: request.db) else { continue }
out.append(.init(id: row.blockedID, name: u.displayName, slug: u.username))
if let u = try await User.find(row.blockedID, on: request.db),
try await Self.mayName(row.blockedID, to: me, on: request.db)
{
out.append(.init(id: row.blockedID, name: u.displayName, slug: u.username))
} else {
out.append(.init(id: row.blockedID, name: "User #\(row.blockedID)", slug: ""))
}
}
return try .json(BlockListResponse(blocked: out))
}
/// Whether `viewer` may be told who `userID` is: listed, or already on the
/// viewer's buddy list, which is only ever added to by a listed username.
static func mayName(_ userID: Int, to viewer: Int, on db: any Database) async throws -> Bool {
if try await Discoverability.isListed(userID, on: db) { return true }
return try await Buddy.query(on: db)
.filter(\.$userID == viewer).filter(\.$buddyID == userID).first() != nil
}
/// POST blocks, DELETE unblocks, the app toggles by method, as the plugin
/// did.
func toggleBlock(_ request: Request, target: Int, blocking: Bool) async throws -> Response {
+3
View File
@@ -55,6 +55,9 @@ extension AccountRoutes {
guard let basic = request.headers.basicAuthorization,
let userID = try await verifyAppCredential(basic.password, on: request.db)
else { throw WireError.credentialNotAccepted }
// A suspended account cannot bring anybody else in: the same refusal
// `mintCredential` and `MobileRoutes.caller` give.
try await User.refuseIfSuspended(userID, on: request.db)
return try await Self.issueVouch(request, userID: userID, now: now)
}
@@ -126,6 +126,30 @@ struct AccountRoutesTests {
}
}
/// A CR or LF inside the address would reach RCPT TO and the To header of
/// the verification mail. Refused with the code the apps already know.
@Test("an email address holding a control character is refused as bad_email")
func controlCharactersInEmailRefused() async throws {
try await withRelay { h in
for email in [
#"eve@example.com\r\nX-Bcc:evil.example"#,
#"eve@example.com\nX-Injected:1"#,
#"eve@exa\u0000mple.com"#,
#"eve@example.com\rRCPT"#,
] {
let err = await #expect(throws: WireError.self) {
_ = try await signup(h, signupBody(user: "eve", email: email))
}
#expect(code(err!) == "bad_email", "for \(email)")
}
let created = try await User.query(on: h.db).count()
#expect(created == 0)
// An ordinary address still signs up.
let ok = try await signup(h, signupBody(user: "eve", email: "eve@example.com"))
#expect(ok.status == .ok)
}
}
@Test("a duplicate username and a duplicate email are distinguished")
func duplicatesAreReported() async throws {
try await withRelay { h in
@@ -302,6 +302,60 @@ struct KeyLogRoutesTests {
}
}
/// Each read loads the whole log. Generous, so a client checking keys is
/// never refused, and a loop is.
@Test("the proof, device and subject reads are metered per address")
func logReadsAreMetered() async throws {
let limits = RateLimiter()
try await withRelay(limits: limits) { h in
try await appendEntries(h, 4)
let reads: [(String, String, () async throws -> Response)] = [
(
"inclusion", "/e2e/log/inclusion?index=1",
{
try await h.routes.logInclusion(get(h, "/e2e/log/inclusion?index=1"))
}
),
(
"consistency", "/e2e/log/consistency?from=1&to=4",
{
try await h.routes.logConsistency(
get(h, "/e2e/log/consistency?from=1&to=4"))
}
),
(
"device", "/e2e/log/device/aaaa-0001",
{
try await h.routes.logDevice(
get(h, "/e2e/log/device/aaaa-0001"), deviceID: "aaaa-0001")
}
),
(
"subject", "/e2e/log/subject?subject=user1@test",
{
try await h.routes.logSubjectEntries(
get(h, "/e2e/log/subject?subject=user1@test"))
}
),
]
for (route, path, read) in reads {
// Under the limit, the answer is unchanged.
#expect(try await read().status == .ok, "\(route)")
// Spend the rest of this address's hour, then one more.
_ = await limits.consume(
"log:\(route):ip:\(clientIP(get(h, path)))",
amount: Routes.logReadPerHour - 1,
limit: Routes.logReadPerHour, now: unixNow())
do {
_ = try await read()
Issue.record("a \(route) read past the hourly limit was served")
} catch let e as WireError {
#expect(e.httpStatus == .tooManyRequests, "\(route)")
}
}
}
}
@Test("a malformed device id is refused")
func malformedDeviceIDRefused() async throws {
try await withRelay { h in
@@ -98,6 +98,23 @@ struct MailerAPNsStoreTests {
#expect(msg.contains("charset=utf-8\r\n\r\nhi"))
}
/// A CR or LF in a header value would end the header and start one of
/// the caller's choosing.
@Test("a header value cannot carry a line break into the headers")
func headerInjectionStripped() {
let msg = Mailer.message(
from: "a@b.example", to: "c@d.example\r\nBcc: x@evil.example",
subject: "s\nX-Injected: 1", body: "hi")
let headers = msg.components(separatedBy: "\r\n\r\n")[0]
.components(separatedBy: "\r\n")
#expect(!headers.contains { $0.hasPrefix("Bcc:") })
#expect(!headers.contains { $0.hasPrefix("X-Injected:") })
#expect(headers.contains("To: <c@d.exampleBcc: x@evil.example>"))
#expect(Mailer.containsControl("a@b.example\r\n"))
#expect(Mailer.containsControl("a\u{0}b"))
#expect(!Mailer.containsControl("someone@example.com"))
}
@Test("an empty body still produces a well-formed message")
func emptyBody() {
let msg = Mailer.message(
@@ -144,6 +144,49 @@ struct MobileExtrasTests {
}
}
/// `toggleBlock` takes any id, so the list must not name an Unlisted
/// account the caller never resolved: "block 1..N, then list" would be a
/// directory. The row stays, with a placeholder, so it can be unblocked.
@Test("the block list does not name an unlisted account the caller cannot see")
func blockListHidesUnlisted() async throws {
try await withRelay { h in
let ada = try await account(h, id: 1, name: "ada")
_ = try await account(h, id: 2, name: "hidden")
_ = try await account(h, id: 3, name: "friend")
try await Discoverability.set(2, listed: false, on: h.db)
try await Discoverability.set(3, listed: false, on: h.db)
// Ada added Friend while Friend was listed.
let b = Buddy()
b.userID = 1
b.buddyID = 3
try await b.create(on: h.db)
let m = mobile(h)
for target in [2, 3, 99] {
_ = try await m.toggleBlock(
request(h, .POST, "/messenger/blocks/\(target)", credential: ada),
target: target, blocking: true)
}
let res = try await m.blocks(request(h, .GET, "/messenger/blocks", credential: ada))
struct List: Decodable {
struct Row: Decodable {
let id: Int
let name: String
let slug: String
}
let blocked: [Row]
}
let list = try JSONDecoder().decode(
List.self, from: Data(buffer: res.body.buffer ?? ByteBuffer()))
#expect(list.blocked.map(\.id) == [2, 3, 99])
#expect(!text(res).contains("hidden"))
#expect(list.blocked[0].name == "User #2" && list.blocked[0].slug.isEmpty)
#expect(list.blocked[1].slug == "friend")
// An unlisted account and an id nobody holds answer alike.
#expect(list.blocked[2].name == "User #99" && list.blocked[2].slug.isEmpty)
}
}
@Test("blocking yourself or a nonsense id is refused")
func blockValidations() async throws {
try await withRelay { h in
@@ -192,6 +192,51 @@ struct OutboundAddressGuardTests {
}
}
/// The outbound half: deposits and fetches at a peer a client named. The
/// directory screened its own fetch; the sender dialled whatever it was
/// given.
@Test("the federation sender refuses a private peer before dialling, except a pinned one")
func senderIsGuarded() async throws {
let loop = EmbeddedEventLoop()
let client = CountingClient(eventLoop: loop, body: "{}")
func sender(_ resolves: [String: [String]], pinned: Set<String> = []) -> FederationSender {
FederationSender(
audience: "me.example", signingKeyRaw: Data(repeating: 7, count: 32),
client: client, admits: { _ in true },
addressGuard: guarded(resolves), pinned: pinned)
}
let now = 1_700_000_000
for address in ["127.0.0.1", "169.254.169.254", "10.1.2.3"] {
let s = sender(["evil.example": [address]])
do {
_ = try await s.get(from: "evil.example", path: "/e2e/account/homes/x", now: now)
Issue.record("dialled a peer resolving to \(address)")
} catch FederationSender.SendError.notPermitted {
} catch { Issue.record("wrong refusal for \(address): \(error)") }
do {
_ = try await s.deposit(to: "evil.example", envelopes: [], now: now)
Issue.record("deposited at a peer resolving to \(address)")
} catch FederationSender.SendError.notPermitted {
} catch { Issue.record("wrong refusal for \(address): \(error)") }
}
#expect(client.fetches == 0, "the sender dialled a private address")
// A name that resolves nowhere reads as unreachable, as before.
do {
_ = try await sender([:]).get(from: "gone.example", path: "/x", now: now)
Issue.record("dialled a name that does not resolve")
} catch FederationSender.SendError.unreachable {}
#expect(client.fetches == 0)
// A public peer, and an allowlisted one on a private address, are dialled.
_ = try await sender(["peer.example": ["93.184.216.34"]])
.get(from: "peer.example", path: "/x", now: now)
_ = try await sender(["relay2.example": ["172.18.0.25"]], pinned: ["relay2.example"])
.get(from: "relay2.example", path: "/x", now: now)
#expect(client.fetches == 2)
}
@Test("a peer name pointing at the metadata endpoint is refused before any request")
func metadataRefused() async {
let (dir, client) = directory(
@@ -107,6 +107,27 @@ struct SuspensionTests {
}
}
@Test("a suspended account cannot mint an invite code")
func noVouchWhileSuspended() async throws {
try await withRelay { h in
let cred = try await account(h, id: 4, name: "dee")
try await suspend(h, 4, true)
do {
_ = try await accounts(h).issueVouch(
basic(h, .POST, "/account/vouch", credential: cred), now: 1_700_000_000)
Issue.record("a suspended account minted an invite code")
} catch {
#expect(isSuspendedRefusal(error), "\(error)")
}
#expect(try await VouchCode.query(on: h.db).count() == 0)
// Lifted, the same credential mints one.
try await suspend(h, 4, false)
let res = try await accounts(h).issueVouch(
basic(h, .POST, "/account/vouch", credential: cred), now: 1_700_000_000)
#expect(res.status == .ok)
}
}
@Test("a token that still verifies is refused on every request while suspended")
func bearerRefused() async throws {
try await withRelay { h in