From 5dc232bfa4ef896a205b3c4f5dd24a7381e8e6e5 Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Sun, 30 Aug 2026 15:30:52 +0000 Subject: [PATCH] fix(dns): report TCP fallback as inspection metadata --- docs/advanced-features.md | 2 +- docs/cli-reference.md | 2 +- docs/configuration.md | 4 +- internal/dnsinspect/dnsinspect.go | 54 ++++++++++++---- internal/dnsinspect/dnsinspect_test.go | 89 +++++++++++++++++++++++--- internal/resolver/system.go | 12 ++-- internal/resolver/system_test.go | 59 +++++++++++++++++ internal/resolver/udp_test.go | 47 ++++++++++++++ internal/resolver/wire.go | 5 +- main.go | 1 + skills/fetch/references/diagnostics.md | 2 +- 11 files changed, 246 insertions(+), 31 deletions(-) diff --git a/docs/advanced-features.md b/docs/advanced-features.md index a6f21e48..08d7f5e0 100644 --- a/docs/advanced-features.md +++ b/docs/advanced-features.md @@ -82,7 +82,7 @@ fetch --inspect-dns example.com fetch --inspect-dns --dns-server https://1.1.1.1/dns-query example.com ``` -Without `--dns-server`, inspection queries the nameservers listed in the system resolver configuration (`/etc/resolv.conf`) directly, including on macOS. It reports every record type (A, AAAA, CNAME, TXT, MX, NS, SOA, SRV, CAA, SVCB, and HTTPS) with per-record TTLs, but does not apply macOS scoped, per-interface, VPN, or `/etc/resolver` routing. On platforms without a usable resolver file (notably Windows), or when the name is resolved only through OS mechanisms (the hosts file, NSS modules, or mDNS), it uses the platform resolver for A and AAAA records without per-record TTLs. Platform-resolver records show their source and `TTL unavailable` individually. If direct DNS returns no address records, platform-resolver addresses are added while any records already returned by direct DNS remain visible. The `Lookup` section identifies this mixed resolver path and reports the platform fallback. With an explicit resolver, inspection queries the same record types concurrently. The default output uses `Lookup` and `Records` sections with the inspected name, resolver path, transport, transport security, source, status, result counts, query counts, and duration. Each record shows its normalized, fully qualified owner name before its value. Inspection output is written to stdout; invocation warnings and setup/configuration errors are written to stderr. If a query fails, successful records are retained, a `Failures` section reports the incomplete record types on stdout, and the command exits with status 1. `Transport security` describes encryption and certificate verification between fetch and the resolver; it does not indicate DNSSEC validation, which fetch does not perform. +Without `--dns-server`, inspection queries the nameservers listed in the system resolver configuration (`/etc/resolv.conf`) directly, including on macOS. It reports every record type (A, AAAA, CNAME, TXT, MX, NS, SOA, SRV, CAA, SVCB, and HTTPS) with per-record TTLs, but does not apply macOS scoped, per-interface, VPN, or `/etc/resolver` routing. On platforms without a usable resolver file (notably Windows), or when the name is resolved only through OS mechanisms (the hosts file, NSS modules, or mDNS), it uses the platform resolver for A and AAAA records without per-record TTLs. Platform-resolver records show their source and `TTL unavailable` individually. If direct DNS returns no address records, platform-resolver addresses are added while any records already returned by direct DNS remain visible. The `Lookup` section identifies this mixed resolver path and reports the platform fallback. With an explicit resolver, inspection queries the same record types concurrently. The default output uses `Lookup` and `Records` sections with the inspected name, resolver path, transport, transport security, source, status, result counts, query counts, and duration. Each record shows its normalized, fully qualified owner name before its value. Inspection output is written to stdout; invocation warnings and setup/configuration errors are written to stderr. If a query fails, successful records are retained, a `Failures` section reports the incomplete record types on stdout, and the command exits with status 1. `Transport security` describes encryption and certificate verification between fetch and the resolver; it does not indicate DNSSEC validation, which fetch does not perform. If a UDP response is truncated, fetch retries the query over TCP and reports the normal protocol fallback as `Transport: UDP → TCP fallback`, not as a warning. Use `-vv` to see which record-type queries used the fallback. ### Configuration File diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 233e5d66..321f3dc1 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -531,7 +531,7 @@ platform bootstrap and negotiate the standard `doq` ALPN. ### `--inspect-dns` -Inspect DNS resolution for the URL hostname only (no HTTP request is made). Without `--dns-server`, it queries the nameservers listed in the system resolver configuration (`/etc/resolv.conf`) directly, including on macOS. It reports every record type (A, AAAA, CNAME, TXT, MX, NS, SOA, SRV, CAA, SVCB, and HTTPS) with per-record TTLs, but does not apply macOS scoped, per-interface, VPN, or `/etc/resolver` routing. On platforms without a usable resolver file (notably Windows), or when the name is resolved only through OS mechanisms (the hosts file, NSS modules, or mDNS), it uses the platform resolver for A and AAAA records without per-record TTLs. Platform-resolver records show their source and `TTL unavailable` individually. If direct DNS returns no address records, platform-resolver addresses are added while any records already returned by direct DNS remain visible. The `Lookup` section identifies this mixed resolver path and reports the platform fallback. With an explicit resolver it queries the same record types concurrently. The default output uses `Lookup` and `Records` sections and includes the inspected name, resolver path, transport, transport security, source, status, result counts, query counts, and duration. Each record shows its normalized, fully qualified owner name before its value. Inspection output is written to stdout; invocation warnings and setup/configuration errors are written to stderr. If one query fails, successful records remain visible, a `Failures` section identifies the incomplete record types on stdout, and the command exits with status 1. `Transport security` describes encryption and certificate verification between fetch and the resolver; it does not indicate DNSSEC validation, which fetch does not perform. +Inspect DNS resolution for the URL hostname only (no HTTP request is made). Without `--dns-server`, it queries the nameservers listed in the system resolver configuration (`/etc/resolv.conf`) directly, including on macOS. It reports every record type (A, AAAA, CNAME, TXT, MX, NS, SOA, SRV, CAA, SVCB, and HTTPS) with per-record TTLs, but does not apply macOS scoped, per-interface, VPN, or `/etc/resolver` routing. On platforms without a usable resolver file (notably Windows), or when the name is resolved only through OS mechanisms (the hosts file, NSS modules, or mDNS), it uses the platform resolver for A and AAAA records without per-record TTLs. Platform-resolver records show their source and `TTL unavailable` individually. If direct DNS returns no address records, platform-resolver addresses are added while any records already returned by direct DNS remain visible. The `Lookup` section identifies this mixed resolver path and reports the platform fallback. With an explicit resolver it queries the same record types concurrently. The default output uses `Lookup` and `Records` sections and includes the inspected name, resolver path, transport, transport security, source, status, result counts, query counts, and duration. Each record shows its normalized, fully qualified owner name before its value. Inspection output is written to stdout; invocation warnings and setup/configuration errors are written to stderr. If one query fails, successful records remain visible, a `Failures` section identifies the incomplete record types on stdout, and the command exits with status 1. `Transport security` describes encryption and certificate verification between fetch and the resolver; it does not indicate DNSSEC validation, which fetch does not perform. If a UDP response is truncated, fetch retries the query over TCP and reports the normal protocol fallback as `Transport: UDP → TCP fallback`, not as a warning. Use `-vv` to see which record-type queries used the fallback. ```sh fetch --inspect-dns example.com diff --git a/docs/configuration.md b/docs/configuration.md index be6f888a..9a7145e9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -279,7 +279,9 @@ family preference. TCP and DoT use pipelined operation-scoped connections. DoQ uses a verified QUIC connection per resolver operation and one bidirectional stream per DNS query. Explicit DNS inspection queries all supported record types concurrently and reports partial results with a nonzero status when a -query fails. +query fails. A truncated UDP response is retried over TCP and reported as +`Transport: UDP → TCP fallback`, not as a warning; use `-vv` to identify the +record-type queries that used the fallback, including failed retries. ```ini # Use Google DNS diff --git a/internal/dnsinspect/dnsinspect.go b/internal/dnsinspect/dnsinspect.go index f60842e0..4fd33185 100644 --- a/internal/dnsinspect/dnsinspect.go +++ b/internal/dnsinspect/dnsinspect.go @@ -61,6 +61,7 @@ type Config struct { Timeout time.Duration URL *url.URL Silent bool + Verbosity core.Verbosity // ResolvConfPath overrides the resolver configuration file consulted when // no --dns-server is set. An empty value uses the platform default @@ -127,7 +128,7 @@ type result struct { duration time.Duration tcpFallback bool platformFallback bool - silent bool + verbosity core.Verbosity } type queryStatus uint8 @@ -228,7 +229,7 @@ func InspectWithError(ctx context.Context, output, errorOutput *core.Printer, cf writeDNSError(errorOutput, err) return 1 } - renderWithWarning(output, errorOutput, res) + render(output, res) partial := len(res.failures) > 0 if flushInspectionOutput(output, errorOutput) != 0 { return 1 @@ -252,7 +253,7 @@ func lookup(ctx context.Context, cfg *Config, host string, start time.Time) (*re security: resolverTransportSecurity(cfg, server), source: inspectionSource(server), records: make(map[string][]record), - silent: cfg.Silent, + verbosity: cfg.Verbosity, } // A missing --dns-server prefers the resolv.conf nameservers, which expose @@ -590,7 +591,7 @@ func platformResult(orig *result, records []record, start time.Time) *result { queryNoData: orig.queryNoData, tcpFallback: orig.tcpFallback, platformFallback: true, - silent: orig.silent, + verbosity: orig.verbosity, duration: time.Since(start), } for typ, values := range orig.records { @@ -1667,13 +1668,38 @@ func conciseDiagnostic(text string) string { } func render(p *core.Printer, res *result) { - renderWithWarning(p, p, res) + renderInspection(p, res) } -func renderWithWarning(p, warningOutput *core.Printer, res *result) { - renderInspection(p, res) - if res.tcpFallback { - core.WriteWarningMsgIf(warningOutput, "UDP response was truncated; used TCP fallback", res.silent) +func inspectionTransportSummary(res *result) string { + if res.tcpFallback && res.transport == "UDP" { + return "UDP → TCP fallback" + } + return res.transport +} + +func renderFallbackQueries(p *core.Printer, queries []queryResult) { + fallbacks := make([]queryResult, 0) + for _, query := range queries { + if query.tcpFallback { + fallbacks = append(fallbacks, query) + } + } + if len(fallbacks) == 0 { + return + } + + writeInspectionBlankLine(p) + renderInspectionSection(p, "Queries") + for _, query := range fallbacks { + status := "no data" + switch query.status { + case queryStatusData: + status = countPhrase(len(query.records), "record", "records") + case queryStatusFailed: + status = "failed" + } + writeInspectionField(p, query.typ.label, status+" · UDP → TCP fallback") } } @@ -1689,8 +1715,8 @@ func renderInspection(p *core.Printer, res *result) { if res.resolver != "" { writeInspectionField(p, "Resolver", res.resolver) } - if res.transport != "" { - writeInspectionField(p, "Transport", res.transport) + if transport := inspectionTransportSummary(res); transport != "" { + writeInspectionField(p, "Transport", transport) } if res.security != "" { writeInspectionField(p, "Transport security", displaySecurity(res.security)) @@ -1711,12 +1737,18 @@ func renderInspection(p *core.Printer, res *result) { if res.duration > 0 { writeInspectionField(p, "Timing", formatDuration(res.duration)) } + if res.tcpFallback && res.transport != "UDP" { + writeInspectionField(p, "TCP fallback", "used for truncated UDP response") + } if len(res.failures) > 0 { writeInspectionBlankLine(p) renderInspectionSection(p, "Failures") renderFailures(p, res.failures) } + if res.verbosity >= core.VExtraVerbose { + renderFallbackQueries(p, res.queries) + } writeInspectionBlankLine(p) renderInspectionSection(p, "Records") if recordCount(res) == 0 { diff --git a/internal/dnsinspect/dnsinspect_test.go b/internal/dnsinspect/dnsinspect_test.go index 15044f35..7237a48c 100644 --- a/internal/dnsinspect/dnsinspect_test.go +++ b/internal/dnsinspect/dnsinspect_test.go @@ -896,16 +896,89 @@ func TestResolverTargetUsesPlatformResolver(t *testing.T) { } } -func TestRenderSeparatesWarnings(t *testing.T) { - output := core.TestPrinter(false) - errors := core.TestPrinter(false) - renderWithWarning(output, errors, &result{tcpFallback: true}) +func TestRenderTCPFallbackAsTransportMetadata(t *testing.T) { + res := &result{ + host: "example.com", + transport: "UDP", + records: make(map[string][]record), + } + aggregate(res, []queryResult{{ + typ: inspectTypes[0], + records: []record{{typ: dnsmessage.TypeA, address: net.ParseIP("192.0.2.1")}}, + tcpFallback: true, + }}, time.Now()) + + p := core.TestPrinter(false) + render(p, res) + out := string(p.Bytes()) + if !strings.Contains(out, "Transport: UDP → TCP fallback") { + t.Fatalf("fallback transport metadata missing:\n%s", out) + } + if strings.Contains(out, "warning:") || strings.Contains(out, "truncated") { + t.Fatalf("fallback was rendered as a warning:\n%s", out) + } +} - if strings.Contains(string(output.Bytes()), "UDP response was truncated") { - t.Fatalf("output contains TCP fallback warning: %q", output.Bytes()) +func TestAggregateFailedTCPFallbackRemainsFailure(t *testing.T) { + res := &result{transport: "UDP", records: make(map[string][]record)} + if err := aggregate(res, []queryResult{{ + typ: inspectTypes[3], + err: errors.New("DNS TCP fallback: connection refused"), + tcpFallback: true, + }}, time.Now()); err == nil { + t.Fatal("aggregate() error = nil, want failed TCP retry error") + } + if len(res.failures) != 1 || res.queries[0].status != queryStatusFailed { + t.Fatalf("failed fallback result = %#v, want one failed query", res) + } + + p := core.TestPrinter(false) + render(p, res) + out := string(p.Bytes()) + if !strings.Contains(out, "Status: incomplete — 1 of 1 queries failed") || !strings.Contains(out, "DNS TCP fallback: connection refused") { + t.Fatalf("failed fallback was not rendered as a failure:\n%s", out) + } +} + +func TestRenderTCPFallbackDetailsAtExtraVerbose(t *testing.T) { + p := core.TestPrinter(false) + render(p, &result{ + host: "example.com", + transport: "UDP", + verbosity: core.VExtraVerbose, + queries: []queryResult{ + {typ: inspectTypes[0], status: queryStatusData, records: []record{{typ: dnsmessage.TypeA}}, tcpFallback: true}, + {typ: inspectTypes[3], status: queryStatusNoData, tcpFallback: true}, + }, + records: map[string][]record{}, + }) + + out := string(p.Bytes()) + for _, want := range []string{ + "Queries\n", + "A: 1 record · UDP → TCP fallback", + "TXT: no data · UDP → TCP fallback", + } { + if !strings.Contains(out, want) { + t.Fatalf("verbose fallback details missing %q:\n%s", want, out) + } + } +} + +func TestRenderWithoutTCPFallbackKeepsTransportUnchanged(t *testing.T) { + p := core.TestPrinter(false) + render(p, &result{ + host: "example.com", + transport: "UDP", + records: map[string][]record{}, + }) + + out := string(p.Bytes()) + if !strings.Contains(out, "Transport: UDP\n") { + t.Fatalf("transport changed without fallback:\n%s", out) } - if !strings.Contains(string(errors.Bytes()), "UDP response was truncated") { - t.Fatalf("error output missing TCP fallback warning: %q", errors.Bytes()) + if strings.Contains(out, "TCP fallback") { + t.Fatalf("output mentions fallback when none was used:\n%s", out) } } diff --git a/internal/resolver/system.go b/internal/resolver/system.go index 33129e07..1acd256f 100644 --- a/internal/resolver/system.go +++ b/internal/resolver/system.go @@ -187,20 +187,21 @@ func QuerySystemType(ctx context.Context, policy SystemResolverPolicy, host stri queryCtx, cancel := context.WithTimeout(ctx, timeout) message, fallback, err := lookupUDPMessage(queryCtx, policy.Nameservers[index], host, typ, attempts) cancel() + totalFallback = totalFallback || fallback if err != nil { lastErr = err continue } name, err := ParseName(host) if err != nil { - return nil, fallback, err + return nil, totalFallback, err } if message.Header.RCode != 0 { - return nil, fallback, fmt.Errorf("DNS response: %s", RCodeName(message.Header.RCode)) + return nil, totalFallback, fmt.Errorf("DNS response: %s", RCodeName(message.Header.RCode)) } authorized, err := AuthorizeAnswers(message, Question{Name: name, Type: typ, Class: 1}) if err != nil { - return nil, fallback, err + return nil, totalFallback, err } out := make([]Record, 0, len(authorized)) hasRequestedType := false @@ -209,10 +210,7 @@ func QuerySystemType(ctx context.Context, policy SystemResolverPolicy, host stri hasRequestedType = hasRequestedType || record.Type == typ } if !hasRequestedType { - return nil, fallback, errDNSNoData - } - if fallback { - totalFallback = true + return nil, totalFallback, errDNSNoData } return out, totalFallback, nil } diff --git a/internal/resolver/system_test.go b/internal/resolver/system_test.go index d4afb480..e2b641c3 100644 --- a/internal/resolver/system_test.go +++ b/internal/resolver/system_test.go @@ -77,6 +77,65 @@ func TestQuerySystemTypeRetriesAcrossNameservers(t *testing.T) { } } +func TestQuerySystemTypePropagatesFailedTCPFallback(t *testing.T) { + server, tcp := newUDPAndTCPTestServer(t) + defer server.close() + defer tcp.Close() + + done := make(chan error, 1) + go func() { + query, client, err := server.readQuery() + if err != nil { + done <- err + return + } + message, err := DecodeMessage(query) + if err != nil { + done <- err + return + } + truncated := responsePacket(query, message.Header.ID, message.Questions[0], nil) + _, err = server.udp.WriteToUDP(withTruncatedFlag(truncated), client) + if err != nil { + done <- err + return + } + connection, err := tcp.AcceptTCP() + if err != nil { + done <- err + return + } + _ = connection.Close() + done <- nil + }() + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _, fallback, err := QuerySystemType(ctx, SystemResolverPolicy{ + Nameservers: []string{server.addr()}, + Attempts: 1, + Timeout: time.Second, + }, "example.com", dnsTypeA) + if err == nil || !strings.Contains(err.Error(), "DNS TCP fallback") { + t.Fatalf("error = %v, want failed TCP fallback", err) + } + if !fallback { + t.Fatal("fallback = false, want attempted TCP fallback") + } + if err := <-done; err != nil { + t.Fatal(err) + } +} + +func withTruncatedFlag(packet []byte) []byte { + packet = append([]byte(nil), packet...) + flags := uint16(packet[2])<<8 | uint16(packet[3]) + flags |= 0x0200 + packet[2] = byte(flags >> 8) + packet[3] = byte(flags) + return packet +} + func TestParseResolvConfSkipsMalformedNameserversAndReadsPolicy(t *testing.T) { policy := ParseResolvConf(strings.TrimSpace(` # comments and malformed entries are ignored diff --git a/internal/resolver/udp_test.go b/internal/resolver/udp_test.go index 1abc90a0..8bb69853 100644 --- a/internal/resolver/udp_test.go +++ b/internal/resolver/udp_test.go @@ -321,6 +321,53 @@ func TestLookupWireTypeFallsBackToTCPWhenUDPIsTruncated(t *testing.T) { } } +func TestLookupUDPMessageReportsFailedTCPFallback(t *testing.T) { + server, tcp := newUDPAndTCPTestServer(t) + defer server.close() + defer tcp.Close() + + done := make(chan error, 1) + go func() { + query, client, err := server.readQuery() + if err != nil { + done <- err + return + } + message, err := DecodeMessage(query) + if err != nil { + done <- err + return + } + truncated := responsePacket(query, message.Header.ID, message.Questions[0], nil) + binary.BigEndian.PutUint16(truncated[2:4], binary.BigEndian.Uint16(truncated[2:4])|0x0200) + if _, err := server.udp.WriteToUDP(truncated, client); err != nil { + done <- err + return + } + + connection, err := tcp.AcceptTCP() + if err != nil { + done <- err + return + } + _ = connection.Close() + done <- nil + }() + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _, fallback, err := LookupUDPMessage(ctx, server.addr(), "example.com", dnsTypeA) + if err == nil || !strings.Contains(err.Error(), "DNS TCP fallback") { + t.Fatalf("error = %v, want failed TCP fallback", err) + } + if !fallback { + t.Fatal("fallback = false, want attempted TCP fallback") + } + if err := <-done; err != nil { + t.Fatal(err) + } +} + func TestLookupWireTypePreservesMatchingSERVFAIL(t *testing.T) { server := newUDPTestServer(t) defer server.close() diff --git a/internal/resolver/wire.go b/internal/resolver/wire.go index c3238369..473853bc 100644 --- a/internal/resolver/wire.go +++ b/internal/resolver/wire.go @@ -675,7 +675,10 @@ func lookupUDPMessage(ctx context.Context, serverAddr, host string, typ uint16, } message, err = transactTCP(ctx, tcpAddress, raw, id, question, transactionDeadline) if err != nil { - return nil, false, fmt.Errorf("DNS TCP fallback: %w", err) + // The fallback was attempted even though it did not produce a usable + // response. Preserve that fact for diagnostic callers while returning + // the retry error so it remains a real query failure. + return nil, true, fmt.Errorf("DNS TCP fallback: %w", err) } return message, true, nil } diff --git a/main.go b/main.go index 5b88c836..d033d511 100644 --- a/main.go +++ b/main.go @@ -831,6 +831,7 @@ func inspectDNS(ctx context.Context, app *cli.App, handle *core.Handle) int { Timeout: getValue(app.Cfg.Timeout), URL: app.URL, Silent: getValue(app.Cfg.Silent), + Verbosity: getVerbosity(app), }) } diff --git a/skills/fetch/references/diagnostics.md b/skills/fetch/references/diagnostics.md index 677ca929..3f177864 100644 --- a/skills/fetch/references/diagnostics.md +++ b/skills/fetch/references/diagnostics.md @@ -26,7 +26,7 @@ shows its normalized, fully qualified owner name before its value. Successful records remain visible when one query fails; the `Lookup` section reports an incomplete status and the `Failures` section identifies the failed types. The command exits nonzero. Inspection output, including the `Failures` section, goes -to stdout. Invocation warnings and setup/configuration errors go to stderr. `Transport security` describes the resolver connection only; it is not DNSSEC validation, which fetch does not perform. +to stdout. Invocation warnings and setup/configuration errors go to stderr. `Transport security` describes the resolver connection only; it is not DNSSEC validation, which fetch does not perform. A truncated UDP response is retried over TCP and is reported as transport metadata (`Transport: UDP → TCP fallback`), not as a warning. Use `-vv` to identify the record-type queries that used this fallback. ## TLS