From 841c42a4d83e7661492c2eaeffd419993fbfce6e Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 28 Jul 2026 22:49:53 +0200 Subject: [PATCH] feat: resolve /dnsaddr before filtering A /dnsaddr carries no transport component, so filter-addrs can neither match it nor exclude it: a ws-speaking peer was dropped by ?filter-addrs=ws, and a quic-only peer survived ?filter-addrs=!quic-v1. Neither is detectable by the client. someguy now resolves before boxo filters, which fixes both directions. See ipfs/specs#542. A filtered request has its /dnsaddr replaced, since keeping it would let a record survive a filter meant to exclude it. The exception is a positive filter naming dnsaddr, the one filter a /dnsaddr matches; the client is asking for indirections, so the /dnsaddr is kept alongside what it resolves to. An unfiltered request gets the resolved addresses added and keeps the /dnsaddr, so it can dial now and re-resolve later. SOMEGUY_DNSADDR_RESOLUTION names what an unfiltered response does with the /dnsaddr: append (default), replace, filtered, or never. Hostnames in provider records are attacker-chosen, and resolution runs inline while the response streams, so it is bounded on four axes: DNS lookups per request, resolved addresses per record, recursion depth, and a per-lookup timeout, which together also cap the delay resolution can add to one response. The per-record limit is threaded through the recursion rather than applied to the finished result: a TXT record that lists itself expands as fan^depth, and re-entering a cached name costs no lookup, so a limit checked at the end still let one request build millions of addresses. - resolve detached from the request context behind a singleflight, so a disconnect cannot cache its own cancellation as a 15 minute failure - shed load once the request is gone rather than spend the rest of its budget on queries nothing will read - keep the original whenever an expansion is not whole, so a failed or throttled lookup does not drop the indirection the rest lives behind - drop a /dnsaddr naming a different peer: nothing it yields can belong to the record carrying it - normalize only the hostname in cache keys, over ASCII: lowering a whole multiaddr folds case-sensitive components together, and Unicode case mapping folds U+0130 and U+212A onto ASCII, which let one record cache a failure under another name's key - skip lookups madns can never satisfy, such as a /p2p-circuit suffix - order addresses by how directly they dial: IP, DNS, dnsaddr, relay last - require go 1.26 --- AGENTS.md | 30 + CHANGELOG.md | 5 + README.md | 1 + dnsaddr.go | 568 ++++++++++++++++++ dnsaddr_test.go | 1035 +++++++++++++++++++++++++++++++++ docs/dnsaddr-resolution.md | 149 +++++ docs/environment-variables.md | 18 + docs/metrics.md | 9 + go.mod | 6 +- main.go | 11 + server.go | 41 +- server_routers.go | 160 ++++- server_test.go | 14 +- 13 files changed, 2021 insertions(+), 26 deletions(-) create mode 100644 dnsaddr.go create mode 100644 dnsaddr_test.go create mode 100644 docs/dnsaddr-resolution.md diff --git a/AGENTS.md b/AGENTS.md index 01de66d..4faabab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,35 @@ someguy is a server implementing the [Delegated Routing V1 HTTP API](https://spe It proxies requests to the Amino DHT and other delegated routing endpoints. It is a caching proxy, not a libp2p node. +## someguy is a proxy: pass through what you do not understand + +someguy sits between clients and other routing systems. Anything it does not +recognize belongs to somebody else, so it forwards it untouched rather than +dropping it or normalizing it. someguy is not the only thing that will read +these records. + +**Unknown record schemas.** A record whose `Schema` is not `peer` (or the +deprecated `bitswap`) arrives as a `types.UnknownRecord` holding its raw JSON, +and must reach the client with its fields intact. Every place that switches on +schema has to fall through: `sanitizeRouter`, `dnsAddrRouter`, and boxo's own +filter all do. Adding a `default` arm that touches the record, or a case that +rewrites one, breaks forward compatibility for a schema shipped after this +build. + +**Unknown multiaddr components.** An address may carry protocols someguy has no +handling for. It must survive with its components in order and unmodified. +`addrSortRank` buckets anything it does not know rather than dropping it, and +`filterPrivateMultiaddr` removes only private addresses. Nothing may rewrite an +address it did not parse for a reason. + +Note the limit of this: a protocol name absent from the multiaddr registry +fails to parse in go-multiaddr, so boxo rejects the record before someguy sees +it. someguy can only pass through what it can parse, and widening that is a +go-multiaddr concern, not a someguy one. + +`TestUnknownSchemaPassesThrough` and `TestUnknownMultiaddrProtocolPassesThrough` +hold both properties. + ## Build and test ```bash @@ -25,5 +54,6 @@ Run `gofmt` and `go vet ./...` before committing. - [environment-variables.md](docs/environment-variables.md): all config flags and environment variables - [peer-address-caching.md](docs/peer-address-caching.md): how `/providers` and `/peers` cache and refresh peer addresses +- [dnsaddr-resolution.md](docs/dnsaddr-resolution.md): why `/dnsaddr` is resolved before `filter-addrs`, and how it is bounded - [metrics.md](docs/metrics.md): Prometheus metrics - [tracing.md](docs/tracing.md): OpenTelemetry tracing diff --git a/CHANGELOG.md b/CHANGELOG.md index a1c63f6..23ba391 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,8 +15,13 @@ The following emojis are used to highlight certain changes: ### Added +- someguy now replaces a `/dnsaddr` provider address with the addresses it names, before it applies `filter-addrs`. A `/dnsaddr` carries no transport component, so a filter could neither match nor exclude it: a provider reachable only through a `/dnsaddr` was dropped from a filtered response, and a provider the client asked to exclude survived one. someguy resolves on every request by default. A request that sends `filter-addrs` gets the `/dnsaddr` replaced, since keeping it would let a record survive a filter meant to exclude it (unless a positive filter entry names `dnsaddr`, the one filter that can match it; then the `/dnsaddr` is kept). A request without a filter gets the resolved addresses added and keeps the `/dnsaddr`, so it can dial straight away and still re-resolve later. Set `SOMEGUY_DNSADDR_RESOLUTION=replace` to drop the `/dnsaddr` from unfiltered responses too, `filtered` to skip the lookup for unfiltered requests, or `never` to disable it (the default is `append`). Lookups are cached and bounded per request, watched by two new metrics: `someguy_routers_dnsaddr_resolutions` and `someguy_routers_dnsaddr_resolution_duration_seconds`. See [`docs/dnsaddr-resolution.md`](https://github.com/ipfs/someguy/blob/main/docs/dnsaddr-resolution.md). [#174](https://github.com/ipfs/someguy/pull/174) +- Addresses within a record are now ordered by how directly a client can dial them: IP first, then DNS names, then `/dnsaddr`, with `/p2p-circuit` relays last. [#174](https://github.com/ipfs/someguy/pull/174) + ### Changed +- Go 1.26 is now the minimum. [#174](https://github.com/ipfs/someguy/pull/174) + ### Removed ### Fixed diff --git a/README.md b/README.md index 46fd2fe..6ff3f55 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,7 @@ See [environment-variables.md](docs/environment-variables.md) for URL formats an - [environment-variables.md](docs/environment-variables.md): all config flags and environment variables - [peer-address-caching.md](docs/peer-address-caching.md): how `/providers` and `/peers` cache and refresh peer addresses +- [dnsaddr-resolution.md](docs/dnsaddr-resolution.md): why `/dnsaddr` is resolved before `filter-addrs`, and how it is bounded - [metrics.md](docs/metrics.md): Prometheus metrics - [tracing.md](docs/tracing.md): OpenTelemetry tracing diff --git a/dnsaddr.go b/dnsaddr.go new file mode 100644 index 0000000..51c6883 --- /dev/null +++ b/dnsaddr.go @@ -0,0 +1,568 @@ +package main + +import ( + "context" + "errors" + "fmt" + "net/http" + "slices" + "strings" + "time" + + lru "github.com/hashicorp/golang-lru/v2" + "github.com/ipfs/boxo/routing/http/types" + "github.com/libp2p/go-libp2p/core/peer" + ma "github.com/multiformats/go-multiaddr" + madns "github.com/multiformats/go-multiaddr-dns" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "golang.org/x/sync/singleflight" +) + +const ( + // DNSAddrRecursionLimit bounds how many times a /dnsaddr may point at + // another /dnsaddr. It matches go-libp2p's dial path, which stops at the + // same depth. + DNSAddrRecursionLimit = 4 + + // DNSAddrLookupTimeout bounds one TXT lookup. Resolution runs while the + // response is streaming, so this is the delay a single unreachable + // nameserver adds to the first byte a client sees. A healthy lookup takes + // tens of milliseconds. Together with MaxDNSAddrLookupsPerRequest it also + // bounds the total time one request can spend blocked on lookups. + DNSAddrLookupTimeout = time.Second + + // DNSAddrCacheTTL is how long a resolved set is reused. madns discards the + // DNS TTL, so this is a fixed value rather than the record's own. Observed + // dnsaddr TXT TTLs are 300-600s, and this matches the max-age someguy puts + // on a response with results. + DNSAddrCacheTTL = 5 * time.Minute + + // DNSAddrFailureCacheTTL is how long a failed lookup is remembered. It is + // longer than the success TTL so a hostname that does not resolve is not + // retried on every request. A lookup that merely timed out uses + // DNSAddrCacheTTL instead: a slow nameserver is worth retrying sooner than + // a name that actively fails to resolve. + DNSAddrFailureCacheTTL = 15 * time.Minute + + // DNSAddrCacheSize caps distinct hostnames remembered. Honest traffic uses + // a handful; the size exists so records naming many hostnames cannot grow + // the cache without bound. + DNSAddrCacheSize = 4096 + + // MaxDNSAddrLookupsPerRequest caps how many DNS lookups one request may + // trigger. Provider records are published by anyone, so without this a + // single request could name thousands of hostnames and turn someguy into a + // relay for DNS floods. Cached names are free and do not count against it. + // Past the cap the address passes through unresolved. + MaxDNSAddrLookupsPerRequest = 16 + + // MaxDNSAddrResolvedPerRecord caps how many addresses resolution may add to + // one record. It is threaded through the recursion rather than applied to + // the finished result: a TXT record that lists itself expands as + // fan^DNSAddrRecursionLimit, and re-entering an already-cached name costs + // no lookup, so a limit checked only at the end would still let one cheap + // request build millions of addresses first. go-libp2p bounds the same + // expansion the same way, in ResolveDNSAddr's outputLimit. + MaxDNSAddrResolvedPerRecord = 100 +) + +var ( + dnsAddrResolutions = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "dnsaddr_resolutions", + Subsystem: "routers", + Namespace: name, + Help: "Outcomes of /dnsaddr resolution; resolved/empty/failed count DNS queries, shared by concurrent requests", + }, []string{"result"}) + + dnsAddrResolutionDuration = promauto.NewHistogram(prometheus.HistogramOpts{ + Name: "dnsaddr_resolution_duration_seconds", + Subsystem: "routers", + Namespace: name, + Help: "Duration of /dnsaddr DNS queries, one sample per query", + Buckets: []float64{0.005, 0.01, 0.05, 0.1, 0.25, 0.5, 1, 2}, + }) +) + +const ( + dnsAddrResultHit = "cache-hit" + dnsAddrResultResolved = "resolved" + dnsAddrResultEmpty = "empty" + dnsAddrResultFailed = "failed" + dnsAddrResultThrottled = "throttled" +) + +// dnsAddrBudget is the number of DNS lookups one /routing/v1 request may still +// spend on resolution. It bounds the DNS traffic one request can cause, and, +// because each lookup blocks the streaming response for at most +// DNSAddrLookupTimeout, the delay resolution can add to the response. +type dnsAddrBudget struct { + lookups int +} + +func newDNSAddrBudget() *dnsAddrBudget { + return &dnsAddrBudget{lookups: MaxDNSAddrLookupsPerRequest} +} + +// spend reports whether another DNS query is allowed, consuming one if so. +func (b *dnsAddrBudget) spend() bool { + if b.lookups <= 0 { + dnsAddrResolutions.WithLabelValues(dnsAddrResultThrottled).Inc() + return false + } + b.lookups-- + return true +} + +type dnsAddrCacheEntry struct { + addrs []ma.Multiaddr + // ok is false for a lookup that failed, so a cached failure keeps reporting + // the expansion as incomplete instead of looking like an empty success. + ok bool + expires time.Time +} + +// dnsAddrResolver turns /dnsaddr multiaddrs into the addresses they name. +// +// A /dnsaddr carries no transport component, so an address filter such as +// filter-addrs=ws can neither match it nor exclude it, and a provider reachable +// only through a /dnsaddr is dropped from a filtered response while a provider +// the client asked to exclude survives one. Resolving before the filter runs +// fixes both directions. See docs/dnsaddr-resolution.md. +type dnsAddrResolver struct { + resolver *madns.Resolver + + // flight dedupes concurrent lookups of one name: requests that miss the + // cache while a lookup for the same name is in flight wait for its result + // instead of querying DNS again. + flight singleflight.Group + + cache *lru.Cache[string, dnsAddrCacheEntry] +} + +func newDNSAddrResolver(resolver *madns.Resolver) (*dnsAddrResolver, error) { + cache, err := lru.New[string, dnsAddrCacheEntry](DNSAddrCacheSize) + if err != nil { + return nil, err + } + if resolver == nil { + resolver = madns.DefaultResolver + } + return &dnsAddrResolver{resolver: resolver, cache: cache}, nil +} + +// asciiLower lowercases ASCII letters and leaves every other byte alone. +// +// strings.ToLower applies Unicode simple case mapping, which maps a few +// non-ASCII runes onto ASCII: U+0130 to 'i' and U+212A to 'k'. Using it on a +// hostname would let "lİbp2p.io" share a cache entry with "libp2p.io", so one +// record could cache a failure under another's name. +func asciiLower(s string) string { + var b []byte + for i := 0; i < len(s); i++ { + if c := s[i]; c >= 'A' && c <= 'Z' { + if b == nil { + b = []byte(s) + } + b[i] = c + ('a' - 'A') + } + } + if b == nil { + return s + } + return string(b) +} + +// dnsAddrCacheKey normalizes addr into a cache and singleflight key. +// +// Only the hostname is normalized: DNS names are case-insensitive and a +// trailing dot names the same zone, so those variants must share one entry. +// Everything after the host is kept verbatim, because components such as a +// certhash or a peer ID are case-sensitive and folding them would serve one +// address the resolution of a different one. +func dnsAddrCacheKey(addr ma.Multiaddr) string { + first, rest := ma.SplitFirst(addr) + if first == nil || first.Protocol().Code != ma.P_DNSADDR { + return addr.String() + } + key := "/dnsaddr/" + strings.TrimSuffix(asciiLower(first.Value()), ".") + if len(rest) > 0 { + key += rest.String() + } + return key +} + +// resolvableDNSAddr reports whether addr is a bare /dnsaddr/, the only +// shape a TXT lookup can satisfy. +// +// madns matches published entries against whatever follows the host in the +// queried address, and real entries end in /p2p/. An address carrying +// anything else, a /p2p-circuit suffix for instance, can only ever resolve to +// nothing, so recognizing it here saves a lookup and avoids caching an empty +// result for it. +func resolvableDNSAddr(addr ma.Multiaddr) bool { + protos := addr.Protocols() + return len(protos) == 1 && protos[0].Code == ma.P_DNSADDR +} + +// lookup resolves one /dnsaddr, reading through the cache. It spends budget +// only when the cache cannot answer, so a cached name is free. ok is false +// whenever the answer is not the full truth: the request spent its lookup +// budget, is gone, or DNS failed. Callers use that to decide whether to keep +// the original /dnsaddr. +func (d *dnsAddrResolver) lookup(ctx context.Context, addr ma.Multiaddr, budget *dnsAddrBudget) (addrs []ma.Multiaddr, ok bool) { + key := dnsAddrCacheKey(addr) + + if entry, found := d.cache.Get(key); found && time.Now().Before(entry.expires) { + dnsAddrResolutions.WithLabelValues(dnsAddrResultHit).Inc() + return entry.addrs, entry.ok + } + + // Check before spending: once the response this is for is gone, starting + // more queries only adds detached DNS traffic nothing will read. + if ctx.Err() != nil { + return nil, false + } + if !budget.spend() { + return nil, false + } + + // The query runs detached from the request context, deduped with other + // requests asking for the same name while it is in flight. A client that + // disconnects mid-lookup can therefore neither cache its cancellation as a + // resolution failure nor waste the answer. + ch := d.flight.DoChan(key, func() (any, error) { + return d.resolve(context.WithoutCancel(ctx), addr, key), nil + }) + + select { + case res := <-ch: + entry, _ := res.Val.(dnsAddrCacheEntry) + return entry.addrs, entry.ok + case <-ctx.Done(): + // The response this lookup was for is gone. The query keeps running so + // the answer still lands in the cache, but this expansion is not whole. + return nil, false + } +} + +// resolve queries DNS and caches the outcome. ctx must not carry the request's +// cancellation: a canceled request caching its own cancellation as a failure +// would suppress resolution for every client for DNSAddrFailureCacheTTL. +func (d *dnsAddrResolver) resolve(ctx context.Context, addr ma.Multiaddr, key string) dnsAddrCacheEntry { + ctx, cancel := context.WithTimeout(ctx, DNSAddrLookupTimeout) + defer cancel() + + start := time.Now() + resolved, err := d.resolver.Resolve(ctx, addr) + dnsAddrResolutionDuration.Observe(time.Since(start).Seconds()) + + entry := dnsAddrCacheEntry{addrs: resolved, ok: true} + ttl := DNSAddrCacheTTL + switch { + case err != nil: + logger.Debugw("dnsaddr resolution failed", "addr", key, "err", err) + dnsAddrResolutions.WithLabelValues(dnsAddrResultFailed).Inc() + entry = dnsAddrCacheEntry{ok: false} + // With cancellation severed above, DeadlineExceeded can only be our own + // lookup timeout: a slow nameserver, retried sooner than a name that + // actively fails to resolve. + if !errors.Is(err, context.DeadlineExceeded) { + ttl = DNSAddrFailureCacheTTL + } + case len(resolved) == 0: + dnsAddrResolutions.WithLabelValues(dnsAddrResultEmpty).Inc() + ttl = DNSAddrFailureCacheTTL + default: + dnsAddrResolutions.WithLabelValues(dnsAddrResultResolved).Inc() + } + + entry.expires = time.Now().Add(ttl) + d.cache.Add(key, entry) + return entry +} + +// resolveAddrs expands every /dnsaddr in addrs into what it resolves to. +// +// keepOriginal decides whether the /dnsaddr survives alongside its resolved +// addresses. It must not when the request carries an address filter that +// cannot match a /dnsaddr: a surviving one would keep a record alive that the +// client asked to exclude. (DNSAddrResolution.action decides, including the +// exception for a filter that names dnsaddr itself.) Without a filter there is +// nothing to skew, and keeping it lets the client re-resolve later while still +// having addresses it can dial now. +// +// An address whose expansion is not whole keeps its original, so a DNS outage, +// a throttled lookup, or a truncated expansion all degrade to the previous +// behavior instead of dropping the indirection the missing addresses live +// behind. +func (d *dnsAddrResolver) resolveAddrs(ctx context.Context, pid peer.ID, addrs []types.Multiaddr, budget *dnsAddrBudget, keepOriginal bool) []types.Multiaddr { + if !containsDNSAddr(addrs) { + return addrs + } + + out := make([]types.Multiaddr, 0, len(addrs)) + seen := make(map[string]struct{}, len(addrs)) + room := MaxDNSAddrResolvedPerRecord + + for _, addr := range addrs { + if addr.Multiaddr == nil || !isDNSAddr(addr.Multiaddr) { + appendUnique(&out, seen, addr.Multiaddr) + continue + } + if id, _ := splitPeerID(addr.Multiaddr); id != "" && id != pid { + // Names a different peer: nothing it yields can belong to this + // record, so it is dropped and never looked up. + continue + } + + resolved, complete := d.expand(ctx, pid, addr.Multiaddr, budget, &room, DNSAddrRecursionLimit) + for _, r := range resolved { + appendUnique(&out, seen, r) + } + if keepOriginal || !complete || len(resolved) == 0 { + appendUnique(&out, seen, addr.Multiaddr) + } + } + + return out +} + +// expand resolves one /dnsaddr, following nested /dnsaddr results up to depth. +// +// room is the number of addresses this record may still gain, shared across the +// whole recursion so a self-referential TXT record cannot fan out. complete is +// false when any part of the expansion was skipped: over the recursion limit, +// out of room, out of request budget, or a failed lookup. +func (d *dnsAddrResolver) expand(ctx context.Context, pid peer.ID, addr ma.Multiaddr, budget *dnsAddrBudget, room *int, depth int) (out []ma.Multiaddr, complete bool) { + if depth <= 0 || *room <= 0 { + return nil, false + } + // Re-entering a cached name costs no lookup, so without this a request that + // is already over could keep expanding for free. + if ctx.Err() != nil { + return nil, false + } + + // Strip a trailing /p2p before looking up: DNS never sees it, so keeping it + // would give one hostname a cache entry per variant. resolveAddrs already + // dropped addrs naming a different peer. + _, bare := splitPeerID(addr) + if !resolvableDNSAddr(bare) { + return nil, false + } + + results, ok := d.lookup(ctx, bare, budget) + complete = ok + for _, r := range results { + if *room <= 0 { + return out, false + } + // A dnsaddr TXT record can list addresses for several peers, and the + // lookup above queries the bare hostname, so discard anything that + // names a different peer here. + id, rest := splitPeerID(r) + if id != "" && id != pid { + continue + } + if len(rest) == 0 { + continue + } + if isDNSAddr(rest) { + nested, nestedComplete := d.expand(ctx, pid, rest, budget, room, depth-1) + complete = complete && nestedComplete + out = append(out, nested...) + continue + } + out = append(out, rest) + *room-- + } + return out, complete +} + +// splitPeerID returns the /p2p component of addr, if any, and addr without it. +// The peer ID is dropped because the record already carries it in its own ID +// field, and every other address someguy returns omits it. +func splitPeerID(addr ma.Multiaddr) (peer.ID, ma.Multiaddr) { + rest, last := ma.SplitLast(addr) + if last == nil || last.Protocol().Code != ma.P_P2P { + return "", addr + } + id, err := peer.Decode(last.Value()) + if err != nil { + return "", rest + } + return id, rest +} + +func isDNSAddr(addr ma.Multiaddr) bool { + for _, p := range addr.Protocols() { + if p.Code == ma.P_DNSADDR { + return true + } + } + return false +} + +func containsDNSAddr(addrs []types.Multiaddr) bool { + for _, a := range addrs { + if a.Multiaddr != nil && isDNSAddr(a.Multiaddr) { + return true + } + } + return false +} + +// appendUnique appends addr to out unless it is empty or already present, and +// reports whether it appended. +func appendUnique(out *[]types.Multiaddr, seen map[string]struct{}, addr ma.Multiaddr) bool { + if len(addr) == 0 { + return false + } + k := addr.String() + if _, dup := seen[k]; dup { + return false + } + seen[k] = struct{}{} + *out = append(*out, types.Multiaddr{Multiaddr: addr}) + return true +} + +// DNSAddrResolution controls when someguy resolves a /dnsaddr and what an +// unfiltered response does with the original: each mode is named after that. +// A request that sends filter-addrs always gets the /dnsaddr replaced in the +// resolving modes, because a filter cannot match one; see addrFilter.action +// for the one filter value that is the exception. +type DNSAddrResolution string + +const ( + // DNSAddrResolutionAppend resolves on every request, so every client sees + // the same addresses and none of them has to resolve a /dnsaddr itself. + // An unfiltered response gains the resolved addresses and keeps the + // /dnsaddr, so the client can dial now and re-resolve later. + DNSAddrResolutionAppend DNSAddrResolution = "append" + // DNSAddrResolutionReplace resolves like append, but an unfiltered + // response also has the /dnsaddr replaced: smaller responses, at the cost + // of the indirection the client could have re-resolved later. + DNSAddrResolutionReplace DNSAddrResolution = "replace" + // DNSAddrResolutionFiltered resolves only when the request carries + // filter-addrs. An unfiltered response keeps the /dnsaddr, so that client + // keeps the indirection and can re-resolve when it dials. + DNSAddrResolutionFiltered DNSAddrResolution = "filtered" + // DNSAddrResolutionNever disables resolution. + DNSAddrResolutionNever DNSAddrResolution = "never" +) + +func ParseDNSAddrResolution(s string) (DNSAddrResolution, error) { + switch DNSAddrResolution(s) { + case DNSAddrResolutionAppend: + return DNSAddrResolutionAppend, nil + case DNSAddrResolutionReplace: + return DNSAddrResolutionReplace, nil + case DNSAddrResolutionFiltered: + return DNSAddrResolutionFiltered, nil + case DNSAddrResolutionNever: + return DNSAddrResolutionNever, nil + default: + return "", fmt.Errorf("invalid dnsaddr resolution mode %q, must be one of [append, replace, filtered, never]", s) + } +} + +type addrFilterCtxKey struct{} + +// addrFilter is what a request's filter-addrs means for a /dnsaddr. +type addrFilter struct { + // matchesDNSAddr is true when a positive entry names dnsaddr itself. That + // is the one filter a bare /dnsaddr does match, so replacing it would drop + // the records the client asked for. + matchesDNSAddr bool +} + +// parseAddrFilter reduces a filter-addrs value to what matters for a /dnsaddr. +// It mirrors how boxo parses the same value (lowercase, split on commas, no +// trimming, entries prefixed with "!" negate), so the two sides cannot +// disagree about whether the filter names dnsaddr. +func parseAddrFilter(param string) addrFilter { + return addrFilter{ + matchesDNSAddr: slices.Contains(strings.Split(strings.ToLower(param), ","), "dnsaddr"), + } +} + +// action is what a request carrying this filter does with a /dnsaddr. +// +// The default is replace: a /dnsaddr matches no transport, so it would +// otherwise survive a filter meant to exclude it. The exception is a positive +// filter naming dnsaddr itself, which does match one, sent by a client asking +// for the indirections; keep the /dnsaddr then, alongside whatever it +// resolves to. When dnsaddr is the only positive entry the lookups are +// wasted, since boxo's filter drops every resolved address, but that filter +// shape is rare and the waste is bounded by the request's budget. +func (f addrFilter) action() dnsAddrAction { + if f.matchesDNSAddr { + return dnsAddrAppend + } + return dnsAddrReplace +} + +// withAddrFilter records the request's filter-addrs so the routers can see it. +// +// The /routing/v1 handler parses filter-addrs and applies it to whatever the +// router returns, without telling the router anything, so someguy reads the +// query itself and passes the value down the request context. That context is +// the one the handler hands to the router, so the value arrives intact. +// +// Only the providers and peers endpoints actually filter; recording the query +// elsewhere (closest peers ignores it) would make the routers replace a +// /dnsaddr on a response nothing filters. +func withAddrFilter(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + filtered := strings.HasPrefix(r.URL.Path, "/routing/v1/providers/") || + strings.HasPrefix(r.URL.Path, "/routing/v1/peers/") + if f := r.URL.Query().Get("filter-addrs"); f != "" && filtered { + r = r.WithContext(context.WithValue(r.Context(), addrFilterCtxKey{}, parseAddrFilter(f))) + } + next.ServeHTTP(w, r) + }) +} + +// dnsAddrAction is what to do with a /dnsaddr on one request. +type dnsAddrAction int + +const ( + // dnsAddrSkip leaves the /dnsaddr alone and does no DNS lookup. + dnsAddrSkip dnsAddrAction = iota + // dnsAddrReplace swaps the /dnsaddr for what it resolves to. + dnsAddrReplace + // dnsAddrAppend adds the resolved addresses and keeps the /dnsaddr. + dnsAddrAppend +) + +// action decides what this request does with a /dnsaddr. +// +// A request that filters usually replaces, because a /dnsaddr matches no +// transport and would otherwise survive a filter meant to exclude it; see +// addrFilter.action for the one filter that can match it. What an unfiltered +// request does is the mode's namesake choice; the zero value behaves like +// append, the configured default. +func (m DNSAddrResolution) action(ctx context.Context) dnsAddrAction { + f, filtered := ctx.Value(addrFilterCtxKey{}).(addrFilter) + switch m { + case DNSAddrResolutionNever: + return dnsAddrSkip + case DNSAddrResolutionFiltered: + if filtered { + return f.action() + } + return dnsAddrSkip + case DNSAddrResolutionReplace: + if filtered { + return f.action() + } + return dnsAddrReplace + default: + if filtered { + return f.action() + } + return dnsAddrAppend + } +} diff --git a/dnsaddr_test.go b/dnsaddr_test.go new file mode 100644 index 0000000..b12f470 --- /dev/null +++ b/dnsaddr_test.go @@ -0,0 +1,1035 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/ipfs/boxo/routing/http/server" + "github.com/ipfs/boxo/routing/http/types" + "github.com/ipfs/boxo/routing/http/types/iter" + "github.com/ipfs/go-cid" + "github.com/libp2p/go-libp2p/core/peer" + ma "github.com/multiformats/go-multiaddr" + madns "github.com/multiformats/go-multiaddr-dns" + "github.com/stretchr/testify/require" +) + +// stubDNS answers TXT lookups from a fixed table so these tests never touch the +// network. It counts lookups so the cache and the per-request budget can be +// asserted on. A non-zero delay makes every lookup take that long, so tests can +// hold one in flight. +type stubDNS struct { + txt map[string][]string + errs map[string]error + delay time.Duration + + // A canceled request can leave several detached lookups running at once, + // so counting has to be safe to call concurrently. + mu sync.Mutex + counts map[string]int +} + +func (s *stubDNS) count(name string) int { + s.mu.Lock() + defer s.mu.Unlock() + return s.counts[name] +} + +// distinct is how many different names were looked up. +func (s *stubDNS) distinct() int { + s.mu.Lock() + defer s.mu.Unlock() + return len(s.counts) +} + +func (s *stubDNS) LookupIPAddr(context.Context, string) ([]net.IPAddr, error) { return nil, nil } + +func (s *stubDNS) LookupTXT(ctx context.Context, name string) ([]string, error) { + s.mu.Lock() + s.counts[name]++ + s.mu.Unlock() + + if s.delay > 0 { + select { + case <-time.After(s.delay): + case <-ctx.Done(): + return nil, ctx.Err() + } + } + if err, ok := s.errs[name]; ok { + return nil, err + } + return s.txt[name], nil +} + +func newStubResolver(t *testing.T, s *stubDNS) *dnsAddrResolver { + t.Helper() + mr, err := madns.NewResolver(madns.WithDefaultResolver(s)) + require.NoError(t, err) + r, err := newDNSAddrResolver(mr) + require.NoError(t, err) + return r +} + +const ( + testPeerA = "12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn" + testPeerB = "12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz" + testCID = "bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4" +) + +func mustAddrs(t *testing.T, ss ...string) []types.Multiaddr { + t.Helper() + var out []types.Multiaddr + for _, s := range ss { + m, err := ma.NewMultiaddr(s) + require.NoError(t, err) + out = append(out, types.Multiaddr{Multiaddr: m}) + } + return out +} + +func addrStrings(addrs []types.Multiaddr) []string { + out := make([]string, 0, len(addrs)) + for _, a := range addrs { + out = append(out, a.String()) + } + return out +} + +// The zero value has to resolve, because that is the configured default and +// because a mode that silently did nothing would be the wrong way to fail. +func TestDNSAddrResolutionDefault(t *testing.T) { + t.Parallel() + + mode, err := ParseDNSAddrResolution("append") + require.NoError(t, err) + require.Equal(t, DNSAddrResolutionAppend, mode) + mode, err = ParseDNSAddrResolution("replace") + require.NoError(t, err) + require.Equal(t, DNSAddrResolutionReplace, mode) + + // With no filter-addrs on the request: the mode's namesake choice. + noFilter := t.Context() + require.Equal(t, dnsAddrAppend, DNSAddrResolutionAppend.action(noFilter), + "without a filter there is nothing to skew, so keep the dnsaddr and add the resolved addrs") + require.Equal(t, dnsAddrReplace, DNSAddrResolutionReplace.action(noFilter), + "the operator chose smaller responses over the client's ability to re-resolve") + require.Equal(t, dnsAddrSkip, DNSAddrResolutionFiltered.action(noFilter)) + require.Equal(t, dnsAddrSkip, DNSAddrResolutionNever.action(noFilter)) + + // With filter-addrs on the request. + filtered := context.WithValue(t.Context(), addrFilterCtxKey{}, parseAddrFilter("ws")) + require.Equal(t, dnsAddrReplace, DNSAddrResolutionAppend.action(filtered), + "a surviving dnsaddr would keep a record alive that the filter meant to drop") + require.Equal(t, dnsAddrReplace, DNSAddrResolutionReplace.action(filtered)) + require.Equal(t, dnsAddrReplace, DNSAddrResolutionFiltered.action(filtered)) + require.Equal(t, dnsAddrSkip, DNSAddrResolutionNever.action(filtered)) + + // filter-addrs=dnsaddr is the one filter a bare /dnsaddr does match, so + // replace would empty the very response the client asked for. + dnsaddrOnly := context.WithValue(t.Context(), addrFilterCtxKey{}, parseAddrFilter("dnsaddr")) + require.Equal(t, dnsAddrAppend, DNSAddrResolutionAppend.action(dnsaddrOnly), + "keep the dnsaddr; boxo's filter drops the appended resolved addrs") + require.Equal(t, dnsAddrAppend, DNSAddrResolutionReplace.action(dnsaddrOnly), + "the dnsaddr filter exception holds in replace mode too") + require.Equal(t, dnsAddrAppend, DNSAddrResolutionFiltered.action(dnsaddrOnly)) + + mixed := context.WithValue(t.Context(), addrFilterCtxKey{}, parseAddrFilter("DNSAddr,ws")) + require.Equal(t, dnsAddrAppend, DNSAddrResolutionAppend.action(mixed), + "the filter can match both the dnsaddr and what it resolves to, and boxo lowercases entries") + require.Equal(t, dnsAddrAppend, DNSAddrResolutionReplace.action(mixed)) + require.Equal(t, dnsAddrAppend, DNSAddrResolutionFiltered.action(mixed)) + + negated := context.WithValue(t.Context(), addrFilterCtxKey{}, parseAddrFilter("!dnsaddr")) + require.Equal(t, dnsAddrReplace, DNSAddrResolutionAppend.action(negated), + "excluding dnsaddr is exactly what replace does") + require.Equal(t, dnsAddrReplace, DNSAddrResolutionReplace.action(negated)) + + _, err = ParseDNSAddrResolution("nonsense") + require.Error(t, err) +} + +// requestHasAddrFilter reports whether withAddrFilter recorded filter-addrs +// on the request context. +func requestHasAddrFilter(ctx context.Context) bool { + _, ok := ctx.Value(addrFilterCtxKey{}).(addrFilter) + return ok +} + +// withAddrFilter must record filter-addrs only for the endpoints whose +// responses boxo actually filters: replace mode on any other endpoint would +// cost the client the /dnsaddr indirection with nothing gained. +func TestAddrFilterScope(t *testing.T) { + t.Parallel() + + var saw bool + h := withAddrFilter(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + saw = requestHasAddrFilter(r.Context()) + })) + + for path, want := range map[string]bool{ + "/routing/v1/providers/x": true, + "/routing/v1/peers/x": true, + "/routing/v1/dht/closest/peers/x": false, + "/routing/v1/ipns/x": false, + } { + h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, path+"?filter-addrs=ws", nil)) + require.Equal(t, want, saw, path) + } + + h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/routing/v1/providers/x", nil)) + require.False(t, saw, "no filter-addrs param, nothing to record") +} + +func TestDNSAddrResolver(t *testing.T) { + t.Parallel() + + pidA, err := peer.Decode(testPeerA) + require.NoError(t, err) + + t.Run("replaces dnsaddr with the addresses it names", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/example.com"), budget, false) + // The /p2p component is dropped: the record already carries the ID. + require.Equal(t, []string{"/dns4/example.com/tcp/3000/ws"}, addrStrings(got)) + }) + + t.Run("drops addresses that name a different peer", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.shared.example": { + "dnsaddr=/dns4/shared.example/tcp/1/ws/p2p/" + testPeerA, + "dnsaddr=/dns4/shared.example/tcp/2/ws/p2p/" + testPeerB, + }, + }} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/shared.example"), budget, false) + require.Equal(t, []string{"/dns4/shared.example/tcp/1/ws"}, addrStrings(got)) + }) + + t.Run("keeps the dnsaddr when resolution fails", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, errs: map[string]error{ + "_dnsaddr.broken.example": fmt.Errorf("SERVFAIL"), + }} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/broken.example"), budget, false) + require.Equal(t, []string{"/dnsaddr/broken.example"}, addrStrings(got), + "a DNS failure must not drop the provider") + }) + + t.Run("follows nested dnsaddr but stops at the recursion limit", func(t *testing.T) { + t.Parallel() + txt := map[string][]string{} + // A chain longer than the limit, ending in a real address that must not be reached. + for i := 0; i < DNSAddrRecursionLimit+2; i++ { + txt[fmt.Sprintf("_dnsaddr.hop%d.example", i)] = []string{ + fmt.Sprintf("dnsaddr=/dnsaddr/hop%d.example/p2p/%s", i+1, testPeerA), + } + } + txt[fmt.Sprintf("_dnsaddr.hop%d.example", DNSAddrRecursionLimit+2)] = []string{ + "dnsaddr=/dns4/deep.example/tcp/1/ws/p2p/" + testPeerA, + } + s := &stubDNS{counts: map[string]int{}, txt: txt} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/hop0.example"), budget, false) + require.Equal(t, []string{"/dnsaddr/hop0.example"}, addrStrings(got), + "an over-deep chain resolves to nothing, so the original is kept") + require.LessOrEqual(t, s.distinct(), DNSAddrRecursionLimit, "recursion must stop at the limit") + }) + + t.Run("caps distinct lookups per request", func(t *testing.T) { + t.Parallel() + txt := map[string][]string{} + var addrs []string + for i := 0; i < MaxDNSAddrLookupsPerRequest+5; i++ { + host := fmt.Sprintf("h%d.example", i) + txt["_dnsaddr."+host] = []string{"dnsaddr=/dns4/" + host + "/tcp/1/ws/p2p/" + testPeerA} + addrs = append(addrs, "/dnsaddr/"+host) + } + s := &stubDNS{counts: map[string]int{}, txt: txt} + budget := newDNSAddrBudget() + newStubResolver(t, s).resolveAddrs(t.Context(), pidA, mustAddrs(t, addrs...), budget, false) + require.Equal(t, MaxDNSAddrLookupsPerRequest, s.distinct(), + "a single request must not be able to trigger unbounded DNS lookups") + }) + + t.Run("caches so a repeated hostname is looked up once", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.cached.example": {"dnsaddr=/dns4/cached.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + for i := 0; i < 5; i++ { + budget := newDNSAddrBudget() + r.resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/cached.example"), budget, false) + } + require.Equal(t, 1, s.count("_dnsaddr.cached.example")) + }) + + t.Run("cache hits do not consume the lookup budget", func(t *testing.T) { + t.Parallel() + txt := map[string][]string{} + var addrs []string + n := MaxDNSAddrLookupsPerRequest + 5 + for i := 0; i < n; i++ { + host := fmt.Sprintf("warm%d.example", i) + txt["_dnsaddr."+host] = []string{"dnsaddr=/dns4/" + host + "/tcp/1/ws/p2p/" + testPeerA} + addrs = append(addrs, "/dnsaddr/"+host) + } + s := &stubDNS{counts: map[string]int{}, txt: txt} + r := newStubResolver(t, s) + for _, a := range addrs { + budget := newDNSAddrBudget() + r.resolveAddrs(t.Context(), pidA, mustAddrs(t, a), budget, false) + } + + budget := newDNSAddrBudget() + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, addrs...), budget, false) + require.Len(t, got, n, "every cached name resolves, even past the lookup cap") + for _, a := range got { + require.False(t, isDNSAddr(a.Multiaddr), "nothing should be left unresolved: %s", a) + } + require.Equal(t, MaxDNSAddrLookupsPerRequest, budget.lookups, "cache hits are free") + }) + + t.Run("shares the lookup budget across all records of one response", func(t *testing.T) { + t.Parallel() + txt := map[string][]string{} + var recs []*types.PeerRecord + for i := 0; i < MaxDNSAddrLookupsPerRequest+5; i++ { + host := fmt.Sprintf("rec%d.example", i) + txt["_dnsaddr."+host] = []string{"dnsaddr=/dns4/" + host + "/tcp/1/ws/p2p/" + testPeerA} + recs = append(recs, &types.PeerRecord{ + Schema: types.SchemaPeer, ID: &pidA, + Addrs: mustAddrs(t, "/dnsaddr/"+host), + }) + } + s := &stubDNS{counts: map[string]int{}, txt: txt} + r := withDNSAddrResolution(dnsStubRouter{recs: recs}, newStubResolver(t, s), DNSAddrResolutionAppend) + it, err := r.FindProviders(t.Context(), cid.MustParse(testCID), 0) + require.NoError(t, err) + for it.Next() { + } + require.Equal(t, MaxDNSAddrLookupsPerRequest, s.distinct(), + "the cap is per request, not per record") + + // FindPeers goes through resolvePeerRecords and must share one budget + // across the response the same way. + s2 := &stubDNS{counts: map[string]int{}, txt: txt} + r2 := withDNSAddrResolution(dnsStubRouter{recs: recs}, newStubResolver(t, s2), DNSAddrResolutionAppend) + it2, err := r2.FindPeers(t.Context(), pidA, 0) + require.NoError(t, err) + for it2.Next() { + } + require.Equal(t, MaxDNSAddrLookupsPerRequest, s2.distinct()) + }) + + t.Run("a request that is already gone starts no lookup", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.healthy.example": {"dnsaddr=/dns4/healthy.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + + canceled, cancel := context.WithCancel(t.Context()) + cancel() + budget := newDNSAddrBudget() + got := r.resolveAddrs(canceled, pidA, mustAddrs(t, "/dnsaddr/healthy.example"), budget, false) + + require.Equal(t, []string{"/dnsaddr/healthy.example"}, addrStrings(got), + "nothing resolved, so the indirection is kept") + require.Zero(t, s.distinct(), "a dead request must not start new DNS queries") + require.Equal(t, MaxDNSAddrLookupsPerRequest, budget.lookups, "and must not spend budget on them") + }) + + t.Run("a lookup already in flight still fills the cache", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, delay: 50 * time.Millisecond, txt: map[string][]string{ + "_dnsaddr.healthy.example": {"dnsaddr=/dns4/healthy.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan []types.Multiaddr, 1) + go func() { + budget := newDNSAddrBudget() + done <- r.resolveAddrs(ctx, pidA, mustAddrs(t, "/dnsaddr/healthy.example"), budget, false) + }() + + // Cancel only once the query is actually running, so this exercises the + // in-flight case rather than the shed-early one above. + require.Eventually(t, func() bool { return s.count("_dnsaddr.healthy.example") > 0 }, + 5*time.Second, time.Millisecond) + cancel() + + select { + case got := <-done: + require.Equal(t, []string{"/dnsaddr/healthy.example"}, addrStrings(got), + "the caller stops waiting and keeps the indirection") + case <-time.After(5 * time.Second): + t.Fatal("resolveAddrs did not return after its request was canceled") + } + + // The detached query carries on, so the real answer lands in the cache + // rather than the cancellation being remembered as a failure. + require.Eventually(t, func() bool { + entry, ok := r.cache.Get("/dnsaddr/healthy.example") + return ok && entry.ok && len(entry.addrs) > 0 + }, 5*time.Second, time.Millisecond) + + budget := newDNSAddrBudget() + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/healthy.example"), budget, false) + require.Equal(t, []string{"/dns4/healthy.example/tcp/1/ws"}, addrStrings(got)) + require.Equal(t, 1, s.count("_dnsaddr.healthy.example"), "the cached answer is reused") + }) + + t.Run("a lookup timeout is retried sooner than a hard failure", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, errs: map[string]error{ + "_dnsaddr.slow.example": context.DeadlineExceeded, + "_dnsaddr.dead.example": fmt.Errorf("NXDOMAIN"), + }} + r := newStubResolver(t, s) + budget := newDNSAddrBudget() + r.resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/slow.example", "/dnsaddr/dead.example"), budget, false) + + expiry := func(key string) time.Time { + entry, ok := r.cache.Get(key) + require.True(t, ok, key) + return entry.expires + } + require.GreaterOrEqual(t, + expiry("/dnsaddr/dead.example").Sub(expiry("/dnsaddr/slow.example")), + DNSAddrFailureCacheTTL-DNSAddrCacheTTL, + "a slow nameserver is worth retrying sooner than a dead name") + }) + + t.Run("hostname variants share one cache entry", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.varied.example": {"dnsaddr=/dns4/varied.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + // Lowercase first, so the one real DNS query matches the stub table. + for _, variant := range []string{ + "/dnsaddr/varied.example", + "/dnsaddr/VARIED.example", + "/dnsaddr/varied.example/p2p/" + testPeerA, + } { + budget := newDNSAddrBudget() + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, variant), budget, false) + require.Equal(t, []string{"/dns4/varied.example/tcp/1/ws"}, addrStrings(got), variant) + } + require.Equal(t, map[string]int{"_dnsaddr.varied.example": 1}, s.counts, + "case and /p2p-suffix variants must not each query DNS") + }) + + t.Run("an expired cache entry is looked up again", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.expired.example": {"dnsaddr=/dns4/expired.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + r.cache.Add("/dnsaddr/expired.example", dnsAddrCacheEntry{expires: time.Now().Add(-time.Second)}) + + budget := newDNSAddrBudget() + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/expired.example"), budget, false) + require.Equal(t, []string{"/dns4/expired.example/tcp/1/ws"}, addrStrings(got)) + require.Equal(t, 1, s.count("_dnsaddr.expired.example")) + }) + + t.Run("caps how many addresses one record can gain", func(t *testing.T) { + t.Parallel() + txt := map[string][]string{} + fanout := func(host string, n int) { + var entries []string + for i := 0; i < n; i++ { + entries = append(entries, fmt.Sprintf("dnsaddr=/dns4/%s/tcp/%d/ws/p2p/%s", host, i+1, testPeerA)) + } + txt["_dnsaddr."+host] = entries + } + fanout("wide1.example", 60) + fanout("wide2.example", 60) + fanout("wide3.example", 60) + s := &stubDNS{counts: map[string]int{}, txt: txt} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, + mustAddrs(t, "/dnsaddr/wide1.example", "/dnsaddr/wide2.example", "/dnsaddr/wide3.example"), budget, false) + // All 60 from wide1; 40 from wide2 plus its original, kept because its + // expansion was truncated; wide3 passed through unresolved. + require.Len(t, got, MaxDNSAddrResolvedPerRecord+2) + strs := addrStrings(got) + require.Contains(t, strs, "/dnsaddr/wide2.example", + "a truncated expansion keeps the indirection alongside what fit") + require.Contains(t, strs, "/dnsaddr/wide3.example") + require.NotContains(t, s.counts, "_dnsaddr.wide3.example", + "no lookups for a record already at its cap") + }) + + t.Run("drops a dnsaddr whose /p2p names a different peer", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, + mustAddrs(t, "/dnsaddr/other.example/p2p/"+testPeerB), budget, false) + require.Empty(t, got, "an addr naming another peer cannot yield usable addresses") + require.Empty(t, s.counts, "and is not worth a lookup") + }) + + t.Run("resolves legacy bitswap records", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + //lint:ignore SA1019 // ignore staticcheck + rec := &types.BitswapRecord{Schema: types.SchemaBitswap, ID: &pidA, Addrs: mustAddrs(t, "/dnsaddr/example.com")} + r := withDNSAddrResolution(bitswapStubRouter{rec: rec}, newStubResolver(t, s), DNSAddrResolutionAppend) + it, err := r.FindProviders(t.Context(), cid.MustParse(testCID), 0) + require.NoError(t, err) + var got []string + for it.Next() { + //lint:ignore SA1019 // ignore staticcheck + rec, ok := it.Val().Val.(*types.BitswapRecord) + require.True(t, ok) + got = append(got, addrStrings(rec.Addrs)...) + } + require.ElementsMatch(t, []string{"/dnsaddr/example.com", "/dns4/example.com/tcp/3000/ws"}, got, + "the legacy schema branch must resolve like SchemaPeer") + }) + + t.Run("concurrent requests share one lookup", func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + s := &stubDNS{counts: map[string]int{}, delay: DNSAddrLookupTimeout / 2, txt: map[string][]string{ + "_dnsaddr.busy.example": {"dnsaddr=/dns4/busy.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + addrs := mustAddrs(t, "/dnsaddr/busy.example") + results := make(chan []string, 5) + for i := 0; i < 5; i++ { + go func() { + budget := newDNSAddrBudget() + results <- addrStrings(r.resolveAddrs(t.Context(), pidA, addrs, budget, false)) + }() + } + for i := 0; i < 5; i++ { + require.Equal(t, []string{"/dns4/busy.example/tcp/1/ws"}, <-results) + } + require.Equal(t, 1, s.count("_dnsaddr.busy.example"), + "requests that miss while a lookup is in flight must wait for it, not repeat it") + }) + }) +} + +// dnsStubRouter returns fixed records, copied per call so several requests can +// run against one stub. +type dnsStubRouter struct { + router + recs []*types.PeerRecord + // raw is returned verbatim ahead of recs, for schemas someguy has no type + // for. + raw []types.Record +} + +func (r dnsStubRouter) FindProviders(context.Context, cid.Cid, int) (iter.ResultIter[types.Record], error) { + out := make([]types.Record, 0, len(r.recs)+len(r.raw)) + out = append(out, r.raw...) + for _, rec := range r.recs { + cp := *rec + out = append(out, &cp) + } + return iter.ToResultIter(iter.FromSlice(out)), nil +} + +func (r dnsStubRouter) FindPeers(context.Context, peer.ID, int) (iter.ResultIter[*types.PeerRecord], error) { + out := make([]*types.PeerRecord, 0, len(r.recs)) + for _, rec := range r.recs { + cp := *rec + out = append(out, &cp) + } + return iter.ToResultIter(iter.FromSlice(out)), nil +} + +func (r dnsStubRouter) GetClosestPeers(context.Context, cid.Cid) (iter.ResultIter[*types.PeerRecord], error) { + out := make([]*types.PeerRecord, 0, len(r.recs)) + for _, rec := range r.recs { + cp := *rec + out = append(out, &cp) + } + return iter.ToResultIter(iter.FromSlice(out)), nil +} + +// bitswapStubRouter returns one legacy bitswap record. +type bitswapStubRouter struct { + router + //lint:ignore SA1019 // ignore staticcheck + rec *types.BitswapRecord +} + +func (r bitswapStubRouter) FindProviders(context.Context, cid.Cid, int) (iter.ResultIter[types.Record], error) { + cp := *r.rec + return iter.ToResultIter(iter.FromSlice([]types.Record{&cp})), nil +} + +// The whole point of resolving is that boxo's filter, which runs after someguy +// hands back a record, can then match on a real transport. This drives the real +// HTTP handler to prove the ordering works end to end. +func TestDNSAddrResolutionThroughHandler(t *testing.T) { + t.Parallel() + + pid, err := peer.Decode(testPeerA) + require.NoError(t, err) + provPath := "/routing/v1/providers/" + testCID + + newServer := func(t *testing.T, mode DNSAddrResolution) *httptest.Server { + t.Helper() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + Addrs: mustAddrs(t, "/dnsaddr/example.com"), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), mode) + h := server.Handler(&composableRouter{providers: r}) + srv := httptest.NewServer(withAddrFilter(h)) + t.Cleanup(srv.Close) + return srv + } + + get := func(t *testing.T, srv *httptest.Server, path string) []string { + t.Helper() + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL+path, nil) + require.NoError(t, err) + req.Header.Set("Accept", "application/json") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + var body struct { + Providers []struct { + Addrs []string + } + Peers []struct { + Addrs []string + } + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) + var out []string + for _, p := range body.Providers { + out = append(out, p.Addrs...) + } + for _, p := range body.Peers { + out = append(out, p.Addrs...) + } + return out + } + + t.Run("filtered mode resolves so the ws filter matches", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionFiltered) + require.Equal(t, []string{"/dns4/example.com/tcp/3000/ws"}, get(t, srv, provPath+"?filter-addrs=ws"), + "a provider reachable over ws must survive filter-addrs=ws") + }) + + t.Run("filtered mode leaves the dnsaddr alone when no filter is sent", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionFiltered) + require.Equal(t, []string{"/dnsaddr/example.com"}, get(t, srv, provPath), + "an unfiltered request keeps the indirection and costs no DNS lookup") + }) + + t.Run("unfiltered request keeps the dnsaddr and gains the resolved addrs", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionAppend) + require.ElementsMatch(t, + []string{"/dnsaddr/example.com", "/dns4/example.com/tcp/3000/ws"}, + get(t, srv, provPath), + "the client can dial now and still re-resolve later") + }) + + t.Run("replace mode drops the dnsaddr even without a filter", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionReplace) + require.Equal(t, []string{"/dns4/example.com/tcp/3000/ws"}, get(t, srv, provPath), + "the operator chose smaller responses over the client's ability to re-resolve") + }) + + t.Run("filtered request drops the dnsaddr it cannot match", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionAppend) + require.Equal(t, []string{"/dns4/example.com/tcp/3000/ws"}, get(t, srv, provPath+"?filter-addrs=ws"), + "keeping the dnsaddr here would let it survive filters it cannot match") + }) + + t.Run("negative filter excludes a peer that really speaks the transport", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionFiltered) + require.Empty(t, get(t, srv, provPath+"?filter-addrs=!ws"), + "resolution has to fix negative filters too, not just positive ones") + }) + + t.Run("a filter naming dnsaddr keeps the indirection", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + // The second addr names a different peer and must be discarded + // without costing a lookup. + Addrs: mustAddrs(t, "/dnsaddr/example.com", "/dnsaddr/other.example/p2p/"+testPeerB), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{providers: r}))) + t.Cleanup(srv.Close) + require.Equal(t, []string{"/dnsaddr/example.com"}, get(t, srv, provPath+"?filter-addrs=dnsaddr"), + "this is the one filter a /dnsaddr matches, so replacing would empty the response") + require.Equal(t, 1, s.distinct(), + "the kept dnsaddr resolves like any other; only the foreign-peer one costs no lookup") + }) + + t.Run("a filter mixing dnsaddr with a transport gets both", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionAppend) + require.ElementsMatch(t, + []string{"/dnsaddr/example.com", "/dns4/example.com/tcp/3000/ws"}, + get(t, srv, provPath+"?filter-addrs=dnsaddr,ws"), + "the filter can match the indirection and what it resolves to") + }) + + t.Run("excluding dnsaddr replaces it with what it names", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionAppend) + require.Equal(t, []string{"/dns4/example.com/tcp/3000/ws"}, get(t, srv, provPath+"?filter-addrs=!dnsaddr")) + }) + + t.Run("never mode leaves the dnsaddr alone even when filtering", func(t *testing.T) { + t.Parallel() + srv := newServer(t, DNSAddrResolutionNever) + require.Empty(t, get(t, srv, provPath+"?filter-addrs=ws"), "unresolved dnsaddr cannot match ws") + }) + + t.Run("peers endpoint resolves and filters too", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + Addrs: mustAddrs(t, "/dnsaddr/example.com"), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{peers: r}))) + t.Cleanup(srv.Close) + require.Equal(t, []string{"/dns4/example.com/tcp/3000/ws"}, + get(t, srv, "/routing/v1/peers/"+peer.ToCid(pid).String()+"?filter-addrs=ws"), + "boxo filters /routing/v1/peers as well, so FindPeers must resolve like FindProviders") + }) + + t.Run("orders addresses ip, dns, dnsaddr, then relay", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + relay := "/ip4/9.9.9.9/tcp/4001/p2p/" + testPeerB + "/p2p-circuit" + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + Addrs: mustAddrs(t, relay, "/dnsaddr/example.com", "/ip4/1.2.3.4/tcp/4001"), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{providers: r}))) + t.Cleanup(srv.Close) + require.Equal(t, []string{ + "/ip4/1.2.3.4/tcp/4001", + "/dns4/example.com/tcp/3000/ws", + "/dnsaddr/example.com", + relay, + }, get(t, srv, provPath), "a client dialing in order tries the most direct addresses first") + }) + + t.Run("sorts records without any dnsaddr too", func(t *testing.T) { + t.Parallel() + relay := "/ip4/9.9.9.9/tcp/4001/p2p/" + testPeerB + "/p2p-circuit" + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + Addrs: mustAddrs(t, relay, "/dns4/example.com/tcp/443/tls/ws", "/ip4/1.2.3.4/tcp/4001"), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, &stubDNS{counts: map[string]int{}}), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{providers: r}))) + t.Cleanup(srv.Close) + require.Equal(t, []string{ + "/ip4/1.2.3.4/tcp/4001", + "/dns4/example.com/tcp/443/tls/ws", + relay, + }, get(t, srv, provPath), "delegated-router records with plain addrs come out ordered as well") + }) + + t.Run("closest peers endpoint resolves but never replaces", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + Addrs: mustAddrs(t, "/dnsaddr/example.com"), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{dht: r}))) + t.Cleanup(srv.Close) + want := []string{"/dns4/example.com/tcp/3000/ws", "/dnsaddr/example.com"} + require.Equal(t, want, get(t, srv, "/routing/v1/dht/closest/peers/"+testCID)) + require.Equal(t, want, get(t, srv, "/routing/v1/dht/closest/peers/"+testCID+"?filter-addrs=ws"), + "boxo never filters this endpoint, so filter-addrs must not cost the client the dnsaddr") + }) +} + +// The bounds have to hold against a record built to defeat them, not just +// against well-formed DNS. +func TestDNSAddrResolverBounds(t *testing.T) { + t.Parallel() + + pidA, err := peer.Decode(testPeerA) + require.NoError(t, err) + + // A TXT record that lists itself expands as fan^depth unless the output + // limit is threaded through the recursion. Re-entering a cached name costs + // no lookup, so a limit applied only to the finished result would still let + // one request build millions of addresses before anything checked. + t.Run("a self-referential record cannot fan out", func(t *testing.T) { + t.Parallel() + const fan = 40 + var entries []string + for i := 0; i < fan; i++ { + entries = append(entries, + "dnsaddr=/dnsaddr/evil.example/p2p/"+testPeerA, + fmt.Sprintf("dnsaddr=/ip4/10.0.0.%d/tcp/1/p2p/%s", i, testPeerA)) + } + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{"_dnsaddr.evil.example": entries}} + r := newStubResolver(t, s) + + budget := newDNSAddrBudget() + start := time.Now() + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/evil.example"), budget, false) + elapsed := time.Since(start) + + // +1 because the unresolvable remainder keeps the original /dnsaddr. + require.LessOrEqual(t, len(got), MaxDNSAddrResolvedPerRecord+1) + require.Less(t, elapsed, 2*time.Second, "bounded output means bounded work") + require.Equal(t, 1, s.count("_dnsaddr.evil.example"), "one cached name, one query") + require.Contains(t, addrStrings(got), "/dnsaddr/evil.example", + "a truncated expansion keeps the indirection the rest lives behind") + }) + + t.Run("the per-record cap is shared across a record's addresses", func(t *testing.T) { + t.Parallel() + txt := map[string][]string{} + var addrs []string + for i := 0; i < 4; i++ { + host := fmt.Sprintf("many%d.example", i) + var entries []string + for j := 0; j < 60; j++ { + entries = append(entries, fmt.Sprintf("dnsaddr=/ip4/10.%d.0.%d/tcp/1/p2p/%s", i, j, testPeerA)) + } + txt["_dnsaddr."+host] = entries + addrs = append(addrs, "/dnsaddr/"+host) + } + s := &stubDNS{counts: map[string]int{}, txt: txt} + budget := newDNSAddrBudget() + got := newStubResolver(t, s).resolveAddrs(t.Context(), pidA, mustAddrs(t, addrs...), budget, false) + + resolved := 0 + for _, a := range got { + if !isDNSAddr(a.Multiaddr) { + resolved++ + } + } + require.LessOrEqual(t, resolved, MaxDNSAddrResolvedPerRecord, + "240 available addresses must not all land in one record") + }) + + // Only a bare /dnsaddr/ can match a published TXT entry, so anything + // else must not cost a lookup or a 15 minute empty cache entry. + t.Run("an unsatisfiable shape costs no lookup", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}} + r := newStubResolver(t, s) + relay := "/dnsaddr/relay.example/p2p/" + testPeerB + "/p2p-circuit/p2p/" + testPeerA + + budget := newDNSAddrBudget() + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, relay), budget, false) + + require.Equal(t, []string{relay}, addrStrings(got), "kept as it was") + require.Zero(t, s.distinct(), "madns could never satisfy this, so do not ask") + require.Equal(t, MaxDNSAddrLookupsPerRequest, budget.lookups) + }) + + t.Run("a request with no budget left resolves nothing more", func(t *testing.T) { + t.Parallel() + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.late.example": {"dnsaddr=/dns4/late.example/tcp/1/ws/p2p/" + testPeerA}, + }} + r := newStubResolver(t, s) + + budget := newDNSAddrBudget() + budget.lookups = 0 + got := r.resolveAddrs(t.Context(), pidA, mustAddrs(t, "/dnsaddr/late.example"), budget, false) + + require.Equal(t, []string{"/dnsaddr/late.example"}, addrStrings(got)) + require.Zero(t, s.distinct(), "with the budget spent nothing more is looked up") + }) +} + +// Normalizing an attacker-controlled name into a shared key namespace is how a +// cache becomes a poisoning primitive, so only the hostname is normalized and +// only over ASCII. +func TestDNSAddrCacheKey(t *testing.T) { + t.Parallel() + + key := func(s string) string { + m, err := ma.NewMultiaddr(s) + require.NoError(t, err) + return dnsAddrCacheKey(m) + } + + t.Run("hostname case and trailing dot share one entry", func(t *testing.T) { + t.Parallel() + want := key("/dnsaddr/bootstrap.libp2p.io") + require.Equal(t, want, key("/dnsaddr/Bootstrap.LibP2P.io")) + require.Equal(t, want, key("/dnsaddr/bootstrap.libp2p.io.")) + }) + + t.Run("a unicode lookalike does not fold onto an ascii name", func(t *testing.T) { + t.Parallel() + // strings.ToLower maps U+0130 to 'i' and U+212A to 'k', which would let + // these share the victim's cache entry and its 15 minute failure TTL. + require.NotEqual(t, key("/dnsaddr/bootstrap.libp2p.io"), key("/dnsaddr/bootstrap.lİbp2p.io")) + require.NotEqual(t, key("/dnsaddr/kbootstrap.io"), key("/dnsaddr/Kbootstrap.io")) + }) + + t.Run("case-sensitive components after the host are preserved", func(t *testing.T) { + t.Parallel() + a := "/dnsaddr/wt.example/p2p/" + testPeerA + b := "/dnsaddr/wt.example/p2p/" + testPeerB + require.NotEqual(t, key(a), key(b), "distinct peer IDs must not share a key") + require.Contains(t, key(a), testPeerA, "the peer ID keeps its case") + }) +} + +// The spec expects unknown schemas to survive a proxy: someguy is not the only +// thing that may understand a record. Every layer that switches on schema +// (sanitizeRouter, dnsAddrRouter, boxo's filter) has to fall through, and a +// switch is easy to grow a case that swallows the rest. +func TestUnknownSchemaPassesThrough(t *testing.T) { + t.Parallel() + + const raw = `{"Schema":"some-future-thing","Addrs":["/dnsaddr/example.com"],"Extra":{"k":"v"}}` + var unknown types.UnknownRecord + require.NoError(t, unknown.UnmarshalJSON([]byte(raw))) + require.Equal(t, "some-future-thing", unknown.GetSchema()) + + s := &stubDNS{counts: map[string]int{}, txt: map[string][]string{ + "_dnsaddr.example.com": {"dnsaddr=/dns4/example.com/tcp/3000/ws/p2p/" + testPeerA}, + }} + stub := dnsStubRouter{raw: []types.Record{&unknown}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{providers: r}))) + t.Cleanup(srv.Close) + + for _, query := range []string{"", "?filter-addrs=ws"} { + url := fmt.Sprintf("%s/routing/v1/providers/%s%s", + srv.URL, "bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4", query) + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, url, nil) + require.NoError(t, err) + req.Header.Set("Accept", "application/json") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + require.NoError(t, err) + + var got struct { + Providers []map[string]any + } + require.NoError(t, json.Unmarshal(body, &got)) + require.Len(t, got.Providers, 1, "query %q dropped the record", query) + require.Equal(t, "some-future-thing", got.Providers[0]["Schema"]) + require.Equal(t, map[string]any{"k": "v"}, got.Providers[0]["Extra"], + "fields someguy does not understand must survive") + require.Equal(t, []any{"/dnsaddr/example.com"}, got.Providers[0]["Addrs"], + "someguy must not reach into a schema it does not know, even to resolve") + } + require.Zero(t, s.distinct(), "an unknown schema costs no DNS lookup") +} + +// A multiaddr may carry protocols someguy has no handling for. It must survive +// intact: not dropped, not reordered ahead of things a client should try first, +// and never rewritten. Nothing here is special-cased anywhere in someguy, which +// is the point. +func TestUnknownMultiaddrProtocolPassesThrough(t *testing.T) { + t.Parallel() + + const ( + onion = "/onion3/vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd:1234" + memory = "/memory/42" + ip = "/ip4/1.2.3.4/tcp/4001" + ) + + pid, err := peer.Decode(testPeerA) + require.NoError(t, err) + + s := &stubDNS{counts: map[string]int{}} + stub := dnsStubRouter{recs: []*types.PeerRecord{{ + Schema: types.SchemaPeer, ID: &pid, + Addrs: mustAddrs(t, onion, memory, ip), + }}} + r := withDNSAddrResolution(stub, newStubResolver(t, s), DNSAddrResolutionAppend) + srv := httptest.NewServer(withAddrFilter(server.Handler(&composableRouter{providers: r}))) + t.Cleanup(srv.Close) + + get := func(query string) []string { + t.Helper() + url := fmt.Sprintf("%s/routing/v1/providers/%s%s", + srv.URL, "bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4", query) + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, url, nil) + require.NoError(t, err) + req.Header.Set("Accept", "application/json") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + var body struct { + Providers []struct{ Addrs []string } + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) + require.Len(t, body.Providers, 1) + return body.Providers[0].Addrs + } + + t.Run("survives unchanged and sorts behind directly dialable addresses", func(t *testing.T) { + require.Equal(t, []string{ip, memory, onion}, get(""), + "unknown protocols are kept verbatim, ranked after ones someguy can place") + }) + + t.Run("a filter naming the unknown protocol matches it", func(t *testing.T) { + require.Equal(t, []string{onion}, get("?filter-addrs=onion3"), + "filtering is by protocol component, so it works without someguy knowing the protocol") + }) + + t.Run("a filter not naming it excludes it", func(t *testing.T) { + require.Equal(t, []string{ip}, get("?filter-addrs=tcp")) + }) + + require.Zero(t, s.distinct(), "no address here is a /dnsaddr, so nothing is looked up") +} diff --git a/docs/dnsaddr-resolution.md b/docs/dnsaddr-resolution.md new file mode 100644 index 0000000..bf232da --- /dev/null +++ b/docs/dnsaddr-resolution.md @@ -0,0 +1,149 @@ +# DNSADDR Resolution + +A provider record can carry a `/dnsaddr` address, which names a hostname whose +DNS TXT record holds the real addresses: + +```console +$ dig +short TXT _dnsaddr.bitswap.example.net +"dnsaddr=/dns4/bitswap.example.net/tcp/3000/ws/p2p/Qma8ddFEQWEU8ijWvdxXm3nxU7oHsRtCykAaVz8WUYhiKn" +``` + +## Why it has to be resolved before filtering + +`filter-addrs` matches on the protocol components present in an address. A +`/dnsaddr` has exactly one component, `dnsaddr`, and no transport, so the filter +can neither match it nor exclude it. That breaks in both directions: + +| request | unresolved `/dnsaddr` | correct answer | +| --- | --- | --- | +| `?filter-addrs=ws` on a peer whose TXT record is `ws` | dropped | kept | +| `?filter-addrs=!quic-v1` on a peer whose TXT record is `quic-v1` | kept | dropped | + +A client cannot detect either case. It asked for peers it can dial and got a +peer it cannot, or it asked to exclude a transport and got it anyway. + +When the request filters, resolving must **replace** the `/dnsaddr` rather than +sit alongside it. Keeping both leaves the second row broken, because the +surviving `/dnsaddr` does not match the excluded transport and so keeps the +record alive. + +## When someguy resolves + +Controlled by +[`SOMEGUY_DNSADDR_RESOLUTION`](environment-variables.md#someguy_dnsaddr_resolution), +which defaults to `append`. Each mode is named after what a request without a +filter does with the `/dnsaddr`: + +| mode | request sends `filter-addrs` | request sends no filter | +| --- | --- | --- | +| `append` (default) | replaced by the resolved addresses | resolved addresses added, `/dnsaddr` kept | +| `replace` | replaced by the resolved addresses | replaced by the resolved addresses | +| `filtered` | replaced by the resolved addresses | left alone, no lookup | +| `never` | left alone, no lookup | left alone, no lookup | + +A filtered request is replaced in every resolving mode, because only a +filtered request can be skewed by a surviving `/dnsaddr`: a filter cannot +match one, so keeping it would hand back a record the client asked to exclude. +Nothing filters an unfiltered request, so the default returns both: the client +can dial the resolved addresses now and re-resolve the `/dnsaddr` later, when +the operator has changed it. + +That matters because `/dnsaddr` is an indirection on purpose. An operator uses +it to change their address set without republishing provider records, and a +resolved address is a snapshot from when someguy answered. Replacing it is +worth it for a filtered request, which has already said it only wants +addresses it can use. Set `replace` if you would rather every response carried +only resolved addresses: responses shrink and no client has to speak DNS, but +a client that holds a record longer than someguy's DNS cache TTL cannot +re-resolve the hostname from the response alone. Set `filtered` if you would +rather someguy did no DNS work for requests that did not ask for filtering. + +One filter value is the exception: `dnsaddr` names a protocol component, so a +positive `?filter-addrs=dnsaddr` does match a `/dnsaddr`, and the client +sending it is asking for the indirections themselves. Then someguy keeps the +`/dnsaddr` alongside whatever it resolves. A `/dnsaddr` naming a different +peer is still dropped, a check that costs no lookup. A negative `!dnsaddr` +replaces as usual, which is exactly the exclusion it asks for. + +## Bounds + +Provider records are published by anyone, so the hostnames someguy is asked to +look up are attacker-influenced. Resolution is bounded on five axes: + +- **Per request.** One request triggers at most `MaxDNSAddrLookupsPerRequest` + DNS lookups; names answered from the cache are free. Past the cap the address + is passed through unresolved, which is the behavior from before this existed. + Without this, one cheap request could name thousands of hostnames and make + someguy a relay for a DNS flood. +- **Per record.** Resolution adds at most `MaxDNSAddrResolvedPerRecord` + addresses to one record, the same bound go-libp2p puts on its dial path, so a + record fanning out through nested `/dnsaddr` cannot balloon the response. A + `/dnsaddr` whose expansion was cut short by any of these bounds keeps its + original alongside whatever fit, so the client retains the indirection. +- **Per lookup.** Each TXT lookup has its own short timeout. Resolution happens + while the response streams, so this timeout is the delay one unreachable + nameserver adds to the first byte the client sees, and together with the + per-request lookup cap it bounds the total delay resolution can add to one + response. The lookup runs detached from the request that triggered it and is + shared with every request waiting on the same name: a client that disconnects + mid-lookup cannot cache its cancellation as a failure, and a popular name is + queried once, not once per waiting request. +- **Recursion and breadth.** A `/dnsaddr` may resolve to another `/dnsaddr`. + someguy follows at most `DNSAddrRecursionLimit` hops, matching go-libp2p's + dial path, and threads a per-record output limit through the recursion. Depth + alone is not enough: a TXT record that lists itself expands as + `fan^depth`, and re-entering an already-cached name costs no lookup, so a + limit applied to the finished result would still let one request build + millions of addresses first. +- **Caching.** Resolved and failed lookups are both cached, failures for + longer, so a repeated or dead hostname is not looked up again on every + request. A lookup that merely timed out is retried sooner than one that + failed outright. madns discards the DNS TTL, so these are fixed values rather + than the record's own. Hostname case and a `/p2p` suffix do not split the + cache: one name has one entry. + +Three more rules apply. Addresses whose `/p2p` component names a different peer +are discarded, because a TXT record can list addresses for several peers and +someguy is resolving on behalf of one. The private-address filter runs again +over the result, because `manet` classifies `/dnsaddr/anything` as public while +the addresses behind it may be private. And only a bare `/dnsaddr/` is +looked up at all: madns matches published entries against whatever follows the +host, real entries end in `/p2p/`, so any other shape can only resolve to +nothing and is passed through without spending a lookup. + +Cache keys normalize the hostname and nothing else, over ASCII only. Lowering a +whole multiaddr would fold case-sensitive components onto one key, and Unicode +case mapping folds a few non-ASCII runes onto ASCII, which would let one record +cache a failure under another name's key. + +If any part of an expansion is missing, someguy keeps the original `/dnsaddr`: +a failed lookup, a request out of lookup budget or already gone, a truncated +expansion. The client keeps the indirection the missing addresses live behind, +and a DNS outage degrades to the old behavior instead of dropping providers. + +## Address order + +Within a record, addresses are sorted by how directly a client can dial them: +IP addresses first, then DNS names, then `/dnsaddr` indirections, then +everything else, with `/p2p-circuit` relay addresses last. A client that dials +in listed order tries the cheapest route first and falls back to a relay only +as a last resort. + +This order applies to every record someguy serves while resolution is enabled. +With `SOMEGUY_DNSADDR_RESOLUTION=never`, only records from the DHT keep it +(they pass the same sort while being sanitized); records from delegated +routers are returned as received. + +## What this does not fix + +Resolution makes the filter correct. It does not make an address usable. A TXT +record advertising `/ws` rather than `/tls/ws` still cannot be dialed from an +HTTPS page, so a browser client can correctly receive a provider it still cannot +reach. That is for the provider operator to fix. + +## Related + +- [ipfs/specs#542](https://github.com/ipfs/specs/issues/542): the IPIP proposing + this behavior for all Delegated Routing implementations +- [environment-variables.md](environment-variables.md#someguy_dnsaddr_resolution) +- [metrics.md](metrics.md) diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 4d94882..8bb4dbc 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -9,6 +9,7 @@ The environment variables below override `someguy`'s built-in defaults. - [`SOMEGUY_CACHED_ADDR_BOOK_RECENT_TTL`](#someguy_cached_addr_book_recent_ttl) - [`SOMEGUY_CACHED_ADDR_BOOK_ACTIVE_PROBING`](#someguy_cached_addr_book_active_probing) - [`SOMEGUY_CACHED_ADDR_BOOK_MAX_CONCURRENT_FIND_PEERS`](#someguy_cached_addr_book_max_concurrent_find_peers) + - [`SOMEGUY_DNSADDR_RESOLUTION`](#someguy_dnsaddr_resolution) - [`SOMEGUY_ROUTING_TIMEOUT`](#someguy_routing_timeout) - [`SOMEGUY_RECORDS_LIMIT`](#someguy_records_limit) - [`SOMEGUY_STREAMING_RECORDS_LIMIT`](#someguy_streaming_records_limit) @@ -75,6 +76,23 @@ Raise this only if `someguy_cached_router_find_peer_lookups_rejected` keeps incr Default: `512` +### `SOMEGUY_DNSADDR_RESOLUTION` + +Controls when Someguy replaces a `/dnsaddr` address with the addresses it names. + +A `/dnsaddr` carries no transport component, so [`filter-addrs`](https://specs.ipfs.tech/routing/http-routing-v1/) can neither match it nor exclude it. A provider reachable only through a `/dnsaddr` is dropped from a filtered response, and a provider the client asked to exclude survives one. Resolving before the filter runs fixes both. + +- `append` (default): resolve on every request. A request that sends `filter-addrs` gets the `/dnsaddr` replaced, because a filter cannot match one and it would otherwise survive a filter meant to exclude it. A request without a filter gets the resolved addresses added and keeps the `/dnsaddr`, so it can dial now and re-resolve later. +- `replace`: like `append`, but a request without a filter also gets the `/dnsaddr` replaced. Responses are smaller and no client has to resolve a `/dnsaddr` itself, at the cost of the indirection: a client cannot re-resolve the hostname later from the response alone. +- `filtered`: resolve only when the request sends `filter-addrs`, and replace the `/dnsaddr` when it does. Requests without a filter are left alone and cost no DNS lookup. +- `never`: never resolve. + +In every resolving mode, a filter whose positive entries name `dnsaddr` itself keeps the `/dnsaddr`: that is the one filter which can match it, and the client sending it is asking for the indirections. See [dnsaddr-resolution.md](dnsaddr-resolution.md) for the details. + +Resolved sets are cached, and one request cannot trigger an unbounded number of DNS lookups. See [dnsaddr-resolution.md](dnsaddr-resolution.md). + +Default: `append` + ### `SOMEGUY_ROUTING_TIMEOUT` Maximum time one `/routing/v1` request spends in the routers. diff --git a/docs/metrics.md b/docs/metrics.md index 3e75147..01b7b35 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -36,3 +36,12 @@ tracked here. See [peer-address-caching.md](peer-address-caching.md). - `someguy_cached_router_find_peer_lookups_rejected`: counter of lookups skipped because the cap was reached. A sustained increase means providers are being dropped that Someguy would otherwise have resolved, so either raise the cap or look at where the traffic is coming from. - `someguy_cached_router_find_peer_lookup_duration_seconds_[bucket|sum|count]`: histogram of how long each background lookup runs. Multiply by the dispatch rate to get expected concurrency, which is how the cap is sized. + +### DNSADDR resolution + +Someguy replaces `/dnsaddr` provider addresses with the addresses they name +before applying `filter-addrs`. See +[dnsaddr-resolution.md](dnsaddr-resolution.md). + +- `someguy_routers_dnsaddr_resolutions{result}`: counter of resolution outcomes, labeled `cache-hit`, `resolved`, `empty`, `failed`, or `throttled` (out of per-request lookups). `cache-hit` and `throttled` count once per `/dnsaddr` seen; `resolved`, `empty`, and `failed` count once per DNS query, and concurrent requests waiting on the same name share one query. A rising `throttled` means requests are hitting the per-request lookup cap. A low `cache-hit` share means the hostnames being asked for keep changing, which is what an abusive client looks like. +- `someguy_routers_dnsaddr_resolution_duration_seconds_[bucket|sum|count]`: histogram of DNS query durations, one sample per query. Queries run detached from the request, but a record whose name is being queried waits for the answer, so the tail here is added latency for responses that hit uncached names. diff --git a/go.mod b/go.mod index ccbdd15..0a88e5a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/ipfs/someguy -go 1.25.7 +go 1.26.0 require ( contrib.go.opencensus.io/exporter/prometheus v0.4.2 @@ -16,6 +16,7 @@ require ( github.com/libp2p/go-libp2p-kad-dht v0.42.1 github.com/libp2p/go-libp2p-record v0.3.1 github.com/multiformats/go-multiaddr v0.16.1 + github.com/multiformats/go-multiaddr-dns v0.6.0 github.com/multiformats/go-multibase v0.3.0 github.com/multiformats/go-multihash v0.2.3 github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 @@ -29,6 +30,7 @@ require ( go.opentelemetry.io/otel v1.44.0 go.opentelemetry.io/otel/sdk v1.44.0 go.opentelemetry.io/otel/trace v1.44.0 + golang.org/x/sync v0.22.0 golang.org/x/sys v0.47.0 ) @@ -87,7 +89,6 @@ require ( github.com/mr-tron/base58 v1.3.0 // indirect github.com/multiformats/go-base32 v0.1.0 // indirect github.com/multiformats/go-base36 v0.2.0 // indirect - github.com/multiformats/go-multiaddr-dns v0.6.0 // indirect github.com/multiformats/go-multiaddr-fmt v0.1.0 // indirect github.com/multiformats/go-multicodec v0.10.0 // indirect github.com/multiformats/go-multistream v0.6.1 // indirect @@ -148,7 +149,6 @@ require ( golang.org/x/exp v0.0.0-20260718201538-764159d718ef // indirect golang.org/x/mod v0.38.0 // indirect golang.org/x/net v0.57.0 // indirect - golang.org/x/sync v0.22.0 // indirect golang.org/x/telemetry v0.0.0-20260708182218-49f421fb7959 // indirect golang.org/x/text v0.40.0 // indirect golang.org/x/time v0.12.0 // indirect diff --git a/main.go b/main.go index f382916..3fd5fb8 100644 --- a/main.go +++ b/main.go @@ -68,6 +68,12 @@ func main() { EnvVars: []string{"SOMEGUY_CACHED_ADDR_BOOK_MAX_CONCURRENT_FIND_PEERS"}, Usage: "maximum background FindPeer lookups running at once for provider records that arrive without addresses", }, + &cli.StringFlag{ + Name: "dnsaddr-resolution", + Value: string(DNSAddrResolutionAppend), + EnvVars: []string{"SOMEGUY_DNSADDR_RESOLUTION"}, + Usage: "what an unfiltered response does with a /dnsaddr it resolved: 'append' (add the resolved addresses, keep the /dnsaddr), 'replace' (drop the /dnsaddr), 'filtered' (resolve only when the request sends filter-addrs), or 'never'; a request that sends filter-addrs gets it replaced in every resolving mode, unless the filter itself names dnsaddr", + }, &cli.DurationFlag{ Name: "routing-timeout", DefaultText: DefaultRoutingTimeout.String(), @@ -214,6 +220,10 @@ func main() { if streamingRecordsLimit < 0 { return fmt.Errorf("streaming-records-limit must be non-negative, got %d (0 means unbounded)", streamingRecordsLimit) } + dnsAddrResolution, err := ParseDNSAddrResolution(ctx.String("dnsaddr-resolution")) + if err != nil { + return err + } cfg := &config{ listenAddress: ctx.String("listen-address"), dhtType: ctx.String("dht"), @@ -222,6 +232,7 @@ func main() { cachedAddrBookRecentTTL: ctx.Duration("cached-addr-book-recent-ttl"), cachedAddrBookMaxFindPeers: ctx.Int("cached-addr-book-max-concurrent-find-peers"), routingTimeout: ctx.Duration("routing-timeout"), + dnsAddrResolution: dnsAddrResolution, recordsLimit: recordsLimit, streamingRecordsLimit: streamingRecordsLimit, diff --git a/server.go b/server.go index c3ab9f4..a59530d 100644 --- a/server.go +++ b/server.go @@ -116,6 +116,7 @@ type config struct { cachedAddrBookRecentTTL time.Duration cachedAddrBookMaxFindPeers int routingTimeout time.Duration + dnsAddrResolution DNSAddrResolution recordsLimit int streamingRecordsLimit int @@ -240,9 +241,18 @@ func start(ctx context.Context, cfg *config) error { } // Combine HTTP routers with DHT and additional routers - crRouters := combineRouters(h, dhtRouting, cachedAddrBook, providerHTTPRouters, blockProviderRouters) - prRouters := combineRouters(h, dhtRouting, cachedAddrBook, peerHTTPRouters, nil) - ipnsRouters := combineRouters(h, dhtRouting, cachedAddrBook, ipnsHTTPRouters, nil) + var dnsAddr *dnsAddrResolver + if cfg.dnsAddrResolution != DNSAddrResolutionNever { + dnsAddr, err = newDNSAddrResolver(nil) + if err != nil { + return err + } + fmt.Printf("Resolving /dnsaddr provider addresses: %s\n", cfg.dnsAddrResolution) + } + + crRouters := combineRouters(h, dhtRouting, cachedAddrBook, providerHTTPRouters, blockProviderRouters, dnsAddr, cfg.dnsAddrResolution) + prRouters := combineRouters(h, dhtRouting, cachedAddrBook, peerHTTPRouters, nil, dnsAddr, cfg.dnsAddrResolution) + ipnsRouters := combineRouters(h, dhtRouting, cachedAddrBook, ipnsHTTPRouters, nil, dnsAddr, cfg.dnsAddrResolution) // Create DHT router for GetClosestPeers endpoint var dhtRouters router @@ -252,6 +262,11 @@ func start(ctx context.Context, cfg *config) error { } else if dhtRouting != nil { dhtRouters = sanitizeRouter{libp2pRouter{host: h, routing: dhtRouting}} } + if dhtRouters != nil { + // Peerstore addresses can carry /dnsaddr too (learned via identify), + // so the closest-peers endpoint resolves like the other endpoints. + dhtRouters = withDNSAddrResolution(dhtRouters, dnsAddr, cfg.dnsAddrResolution) + } _, port, err := net.SplitHostPort(cfg.listenAddress) if err != nil { @@ -288,6 +303,10 @@ func start(ctx context.Context, cfg *config) error { dht: dhtRouters, }, handlerOpts...) + // Record filter-addrs so the routers can see it. Must wrap the + // /routing/v1 handler, whose request context is the one the routers get. + handler = withAddrFilter(handler) + // Add CORS. handler = cors.New(cors.Options{ AllowedOrigins: []string{"*"}, @@ -401,7 +420,7 @@ func newHost(cfg *config) (host.Host, error) { // combineRouters combines delegated HTTP routers with DHT and additional routers. // It no longer creates HTTP clients (that's done in createDelegatedHTTPRouters). -func combineRouters(h host.Host, dht routing.Routing, cachedAddrBook *cachedAddrBook, delegatedRouters, additionalRouters []router) router { +func combineRouters(h host.Host, dht routing.Routing, cachedAddrBook *cachedAddrBook, delegatedRouters, additionalRouters []router, dnsAddr *dnsAddrResolver, dnsAddrMode DNSAddrResolution) router { var dhtRouter router if cachedAddrBook != nil { @@ -415,7 +434,7 @@ func combineRouters(h host.Host, dht routing.Routing, cachedAddrBook *cachedAddr if dhtRouter == nil { return composableRouter{} } - return dhtRouter + return withDNSAddrResolution(dhtRouter, dnsAddr, dnsAddrMode) } var routers []router @@ -425,9 +444,17 @@ func combineRouters(h host.Host, dht routing.Routing, cachedAddrBook *cachedAddr } routers = append(routers, additionalRouters...) - return parallelRouter{ - routers: routers, + // Resolution wraps the composed router rather than sitting beside + // sanitizeRouter, because /dnsaddr records reach someguy from the delegated + // HTTP routers, which sanitizeRouter does not cover. + return withDNSAddrResolution(parallelRouter{routers: routers}, dnsAddr, dnsAddrMode) +} + +func withDNSAddrResolution(r router, resolver *dnsAddrResolver, mode DNSAddrResolution) router { + if resolver == nil || mode == DNSAddrResolutionNever { + return r } + return dnsAddrRouter{router: r, resolver: resolver, mode: mode} } func withTracingAndDebug(next http.Handler, authToken string) http.Handler { diff --git a/server_routers.go b/server_routers.go index 0e31ba2..f921a37 100644 --- a/server_routers.go +++ b/server_routers.go @@ -21,6 +21,7 @@ import ( "github.com/libp2p/go-libp2p/core/host" "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/core/routing" + ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" ) @@ -645,21 +646,154 @@ func filterPrivateMultiaddr(a []types.Multiaddr) []types.Multiaddr { b = append(b, addr) } - // Sort for a stable response across requests, and place relay - // (/p2p-circuit) addresses last so a client tries direct, dialable addresses - // first and only falls back to a relay. Addresses can arrive in - // nondeterministic order (e.g. the peerstore stores them in a map), and this - // runs on every record from every router, so all sources are covered. - slices.SortFunc(b, func(x, y types.Multiaddr) int { - xRelay, yRelay := isRelayAddr(x.Multiaddr), isRelayAddr(y.Multiaddr) - if xRelay != yRelay { - if xRelay { - return 1 + slices.SortFunc(b, compareAddrs) + + return b +} + +// compareAddrs sorts for a stable response across requests, ordered by how +// directly a client can dial each address: IP addresses first, then DNS names, +// then /dnsaddr indirections that need a TXT lookup, then anything else, with +// relay (/p2p-circuit) addresses as the last resort. Addresses can arrive in +// nondeterministic order (e.g. the peerstore stores them in a map). +func compareAddrs(x, y types.Multiaddr) int { + if d := addrSortRank(x.Multiaddr) - addrSortRank(y.Multiaddr); d != 0 { + return d + } + return bytes.Compare(x.Multiaddr.Bytes(), y.Multiaddr.Bytes()) +} + +// sortedAddrs returns addrs in compareAddrs order without dropping any. It is +// for records that pass through otherwise untouched, so every record in a +// response comes out ordered the same way. +func sortedAddrs(addrs []types.Multiaddr) []types.Multiaddr { + out := slices.Clone(addrs) + slices.SortFunc(out, compareAddrs) + return out +} + +// addrSortRank buckets an address by how directly a client can dial it: IP +// addresses, then DNS names that cost one lookup, then /dnsaddr indirections +// that need a TXT lookup, then everything else, then /p2p-circuit relays. A +// relay reached through any transport counts as a relay. +func addrSortRank(addr ma.Multiaddr) int { + if isRelayAddr(addr) { + return 4 + } + protos := addr.Protocols() + if len(protos) == 0 { + return 3 + } + switch protos[0].Code { + case ma.P_IP4, ma.P_IP6: + return 0 + case ma.P_DNS, ma.P_DNS4, ma.P_DNS6: + return 1 + case ma.P_DNSADDR: + return 2 + default: + return 3 + } +} + +// dnsAddrRouter replaces /dnsaddr addresses with the addresses they name, +// before the /routing/v1 handler applies filter-addrs. +// +// It wraps each whole composed router rather than sitting next to +// sanitizeRouter, because sanitizeRouter only covers the DHT branch while +// /dnsaddr records reach someguy from delegated HTTP routers. +type dnsAddrRouter struct { + router + resolver *dnsAddrResolver + mode DNSAddrResolution +} + +var _ server.ContentRouter = dnsAddrRouter{} + +//lint:ignore SA1019 // ignore staticcheck +func (r dnsAddrRouter) ProvideBitswap(ctx context.Context, req *server.BitswapWriteProvideRequest) (time.Duration, error) { + return 0, routing.ErrNotSupported +} + +// resolveRecordAddrs resolves a record's addresses and re-runs the private +// address filter over the result. Resolution has to happen before that filter, +// not after: manet treats /dnsaddr/anything as public, so a name that resolves +// to an RFC1918 address would otherwise be served to clients. +// +// Records that need no resolution are still sorted, so the whole response is +// ordered the same way, delegated-router records included. +func (r dnsAddrRouter) resolveRecordAddrs(ctx context.Context, pid *peer.ID, addrs []types.Multiaddr, budget *dnsAddrBudget) []types.Multiaddr { + if pid == nil || !containsDNSAddr(addrs) { + return sortedAddrs(addrs) + } + action := r.mode.action(ctx) + if action == dnsAddrSkip { + return sortedAddrs(addrs) + } + return filterPrivateMultiaddr(r.resolver.resolveAddrs(ctx, *pid, addrs, budget, action == dnsAddrAppend)) +} + +func (r dnsAddrRouter) FindProviders(ctx context.Context, key cid.Cid, limit int) (iter.ResultIter[types.Record], error) { + it, err := r.router.FindProviders(ctx, key, limit) + if err != nil { + return nil, err + } + + budget := newDNSAddrBudget() + return iter.Map(it, func(v iter.Result[types.Record]) iter.Result[types.Record] { + if v.Err != nil || v.Val == nil { + return v + } + + switch v.Val.GetSchema() { + case types.SchemaPeer: + result, ok := v.Val.(*types.PeerRecord) + if !ok { + return v } - return -1 + result.Addrs = r.resolveRecordAddrs(ctx, result.ID, result.Addrs, budget) + v.Val = result + + //lint:ignore SA1019 // ignore staticcheck + case types.SchemaBitswap: + //lint:ignore SA1019 // ignore staticcheck + result, ok := v.Val.(*types.BitswapRecord) + if !ok { + return v + } + result.Addrs = r.resolveRecordAddrs(ctx, result.ID, result.Addrs, budget) + v.Val = result + } + + return v + }), nil +} + +// resolvePeerRecords resolves every record of one response, sharing one +// budget across it. +func (r dnsAddrRouter) resolvePeerRecords(ctx context.Context, it iter.ResultIter[*types.PeerRecord]) iter.ResultIter[*types.PeerRecord] { + budget := newDNSAddrBudget() + return iter.Map(it, func(v iter.Result[*types.PeerRecord]) iter.Result[*types.PeerRecord] { + if v.Err != nil || v.Val == nil { + return v } - return bytes.Compare(x.Multiaddr.Bytes(), y.Multiaddr.Bytes()) + v.Val.Addrs = r.resolveRecordAddrs(ctx, v.Val.ID, v.Val.Addrs, budget) + return v }) +} - return b +func (r dnsAddrRouter) FindPeers(ctx context.Context, pid peer.ID, limit int) (iter.ResultIter[*types.PeerRecord], error) { + it, err := r.router.FindPeers(ctx, pid, limit) + if err != nil { + return nil, err + } + return r.resolvePeerRecords(ctx, it), nil +} + +func (r dnsAddrRouter) GetClosestPeers(ctx context.Context, key cid.Cid) (iter.ResultIter[*types.PeerRecord], error) { + it, err := r.router.GetClosestPeers(ctx, key) + if err != nil { + return nil, err + } + return r.resolvePeerRecords(ctx, it), nil } diff --git a/server_test.go b/server_test.go index 3c857bd..208b9e4 100644 --- a/server_test.go +++ b/server_test.go @@ -21,16 +21,24 @@ func TestCombineRouters(t *testing.T) { mockRouter := composableRouter{} // Check that combineRouters with DHT only returns sanitizeRouter - v := combineRouters(nil, &bundledDHT{}, nil, nil, nil) + v := combineRouters(nil, &bundledDHT{}, nil, nil, nil, nil, DNSAddrResolutionNever) require.IsType(t, sanitizeRouter{}, v) // Check that combineRouters with delegated routers only returns parallelRouter - v = combineRouters(nil, nil, nil, []router{mockRouter}, nil) + v = combineRouters(nil, nil, nil, []router{mockRouter}, nil, nil, DNSAddrResolutionNever) require.IsType(t, parallelRouter{}, v) // Check that combineRouters with both DHT and delegated routers returns parallelRouter - v = combineRouters(nil, &bundledDHT{}, nil, []router{mockRouter}, nil) + v = combineRouters(nil, &bundledDHT{}, nil, []router{mockRouter}, nil, nil, DNSAddrResolutionNever) require.IsType(t, parallelRouter{}, v) + + // Check that a resolver wraps both branches in dnsAddrRouter + resolver, err := newDNSAddrResolver(nil) + require.NoError(t, err) + v = combineRouters(nil, &bundledDHT{}, nil, nil, nil, resolver, DNSAddrResolutionAppend) + require.IsType(t, dnsAddrRouter{}, v) + v = combineRouters(nil, nil, nil, []router{mockRouter}, nil, resolver, DNSAddrResolutionAppend) + require.IsType(t, dnsAddrRouter{}, v) } // A record that resolved early has to reach the client while later records