Federation resolves a peer name before fetching it (relay#55) #133

Merged
paulhitt merged 3 commits from fix/federation-resolves-before-it-fetches into main 2026-09-06 23:00:18 +00:00
paulhitt commented 2026-09-06 21:03:00 +00:00 (Migrated from gitlab.com)

isFetchableHost filters the SPELLING of a peer name — it rejects IPv4
literals, .local, .internal, names without a dot — and then
descriptor(for:) fetches https://<audience>/.well-known/flamenet/relay
on the inbound verification path. internal.attacker.example is an
entirely ordinary hostname that passes every one of those rules and can
resolve to 127.0.0.1, 169.254.169.254, or anything else the person
who owns it wants. Under open federation that name arrives in a header
from anyone.

OutboundAddressGuard resolves first and refuses unless EVERY answer is
publicly routable. Every address, not the first: a name answering with
one public and one private record is not a name with a harmless extra
record, it is the shape of the attack, because which record gets used is
the attacker's choice.

Applied in every mode, not only open, for the reason the issue gives —
open is a switch somebody flips later, in a hurry, on the strength of a
config comment about spam, and nothing at that moment would tell them
they had also opened an outbound fetch primitive.

Addresses are compared as BYTES. 127.0.0.1 is not the only spelling of
loopback, and ::ffff:127.0.0.1 and 64:ff9b::a9fe:a9fe are v6
addresses whose own prefixes match no v6 rule and which connect to
loopback and to the metadata endpoint respectively. Any guard that checks
v6 rules against v6 addresses and stops there is open.

Two things found while writing it.

inet_pton on Darwin accepts a leading zero and reads it as DECIMAL:
0177.0.0.1 parses to 177.0.0.1, a public address, while inet_aton,
curl and every browser read 0177 as octal — 127 — which is loopback.
Same eleven characters, two verdicts. It cannot bite here, because
IPAddress is only ever given resolver output, which is canonical, and
isFetchableHost rejects four-numeric-label names before resolution is
attempted. Both facts are now written down and tested, because that
filter is load-bearing for a reason it was not written for.

getaddrinfo BLOCKS, and this runs inside an actor on the inbound path.
The first version would have occupied a thread of Swift's cooperative
pool — one per core — until DNS answered or timed out, so a handful of
envelopes naming black-holed hosts would have stalled unrelated request
handling across the relay. Trading an SSRF for a denial of service is a
bad trade, and Federation.swift already worries about exactly this
shape for the fetch itself. Resolution now hops to a DispatchQueue.

A refusal is reported as unknownPeer, deliberately: telling the sender
"that resolved to a private address" would turn this into an internal
port scanner with a helpful error channel.

The residual TOCTOU is stated in the code rather than papered over.
The connection re-resolves, so a DNS server the attacker controls can
answer publicly at the check and privately at the connect. Closing that
needs the connection pinned to the address we checked, which Vapor's
any Client gives no way to express. What this removes is the entire
static case — every version of this that does not require running
authoritative DNS and winning a race.

The test relay#55 asked for is here, and the assertion that makes it mean
anything is the fetch COUNT, not the throw: checking only that the call
threw would pass on an implementation that fetched first and rejected the
answer afterwards, by which point the request has already gone out from
inside the network. Removing the guard reddens both.

Four existing federation tests needed a resolver injected — they use
peer.example, which deliberately does not exist, so the real resolver
refuses it. Their fake resolver answers publicly for names but passes
literals straight through, so addressCausesNoFetch is not disarmed.

664 tests, 0 failures.

Closes #55

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

`isFetchableHost` filters the SPELLING of a peer name — it rejects IPv4 literals, `.local`, `.internal`, names without a dot — and then `descriptor(for:)` fetches `https://<audience>/.well-known/flamenet/relay` on the inbound verification path. `internal.attacker.example` is an entirely ordinary hostname that passes every one of those rules and can resolve to `127.0.0.1`, `169.254.169.254`, or anything else the person who owns it wants. Under `open` federation that name arrives in a header from anyone. `OutboundAddressGuard` resolves first and refuses unless EVERY answer is publicly routable. Every address, not the first: a name answering with one public and one private record is not a name with a harmless extra record, it is the shape of the attack, because which record gets used is the attacker's choice. Applied in every mode, not only `open`, for the reason the issue gives — `open` is a switch somebody flips later, in a hurry, on the strength of a config comment about spam, and nothing at that moment would tell them they had also opened an outbound fetch primitive. Addresses are compared as BYTES. `127.0.0.1` is not the only spelling of loopback, and `::ffff:127.0.0.1` and `64:ff9b::a9fe:a9fe` are v6 addresses whose own prefixes match no v6 rule and which connect to loopback and to the metadata endpoint respectively. Any guard that checks v6 rules against v6 addresses and stops there is open. Two things found while writing it. `inet_pton` on Darwin accepts a leading zero and reads it as DECIMAL: `0177.0.0.1` parses to `177.0.0.1`, a public address, while `inet_aton`, curl and every browser read `0177` as octal — 127 — which is loopback. Same eleven characters, two verdicts. It cannot bite here, because `IPAddress` is only ever given resolver output, which is canonical, and `isFetchableHost` rejects four-numeric-label names before resolution is attempted. Both facts are now written down and tested, because that filter is load-bearing for a reason it was not written for. `getaddrinfo` BLOCKS, and this runs inside an actor on the inbound path. The first version would have occupied a thread of Swift's cooperative pool — one per core — until DNS answered or timed out, so a handful of envelopes naming black-holed hosts would have stalled unrelated request handling across the relay. Trading an SSRF for a denial of service is a bad trade, and `Federation.swift` already worries about exactly this shape for the fetch itself. Resolution now hops to a DispatchQueue. A refusal is reported as `unknownPeer`, deliberately: telling the sender "that resolved to a private address" would turn this into an internal port scanner with a helpful error channel. The residual TOCTOU is stated in the code rather than papered over. The connection re-resolves, so a DNS server the attacker controls can answer publicly at the check and privately at the connect. Closing that needs the connection pinned to the address we checked, which Vapor's `any Client` gives no way to express. What this removes is the entire static case — every version of this that does not require running authoritative DNS and winning a race. The test relay#55 asked for is here, and the assertion that makes it mean anything is the fetch COUNT, not the throw: checking only that the call threw would pass on an implementation that fetched first and rejected the answer afterwards, by which point the request has already gone out from inside the network. Removing the guard reddens both. Four existing federation tests needed a resolver injected — they use `peer.example`, which deliberately does not exist, so the real resolver refuses it. Their fake resolver answers publicly for names but passes literals straight through, so `addressCausesNoFetch` is not disarmed. 664 tests, 0 failures. Closes #55 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
paulhitt commented 2026-09-06 21:03:00 +00:00 (Migrated from gitlab.com)

assigned to @paulhitt

assigned to @paulhitt
paulhitt commented 2026-09-06 21:17:12 +00:00 (Migrated from gitlab.com)

added 1 commit

  • a5e30ed1 - Build on Linux too: SOCK_STREAM is a different type under Glibc

Compare with previous version

added 1 commit <ul><li>a5e30ed1 - Build on Linux too: SOCK_STREAM is a different type under Glibc</li></ul> [Compare with previous version](/paulhitt/flamenet-relay/-/merge_requests/70/diffs?diff_id=2007734505&start_sha=4bd1b137266464af11a90db03ac0dd81814ae440)
paulhitt commented 2026-09-06 21:53:00 +00:00 (Migrated from gitlab.com)

aborted the automatic merge because the source branch was updated. Learn more.

aborted the automatic merge because the source branch was updated. [Learn more](https://gitlab.com/help/ci/pipelines/merge_trains.md#merge-request-dropped-from-the-merge-train).
paulhitt commented 2026-09-06 21:53:00 +00:00 (Migrated from gitlab.com)

added 1 commit

  • c6266831 - Assert the invariant that holds on both platforms, not macOS's parse

Compare with previous version

added 1 commit <ul><li>c6266831 - Assert the invariant that holds on both platforms, not macOS&#39;s parse</li></ul> [Compare with previous version](/paulhitt/flamenet-relay/-/merge_requests/70/diffs?diff_id=2007751108&start_sha=a5e30ed18a80e62120dd8f0f4af40470cee5039d)
paulhitt commented 2026-09-06 23:00:19 +00:00 (Migrated from gitlab.com)

mentioned in commit 56f039e153

mentioned in commit 56f039e1539b9aa81d3d5eceb2bbfaa1dfb3f200
paulhitt (Migrated from gitlab.com) merged commit 56f039e153 into main 2026-09-06 23:00:19 +00:00
paulhitt commented 2026-09-09 18:58:52 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
Sign in to join this conversation.