diff --git a/src/zdns/lookup.go b/src/zdns/lookup.go index 6b3a92a8..30c9c2bb 100644 --- a/src/zdns/lookup.go +++ b/src/zdns/lookup.go @@ -891,25 +891,15 @@ func (r *Resolver) cachedLookup(ctx context.Context, q Question, nameServer *Nam var result *SingleQueryResult var rawResp *dns.Msg var status Status - if r.dnsOverHTTPSEnabled { - r.verboseLog(depth, "****WIRE LOOKUP*** ", DoHProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) - result, rawResp, status, err = doDoHLookup(lookupCtx, connInfo.httpsClient, q, nameServer, requestIteration, r.ednsOptions, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit) - } else if r.dnsOverTLSEnabled { - r.verboseLog(depth, "****WIRE LOOKUP*** ", DoTProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) - result, rawResp, status, err = doDoTLookup(lookupCtx, connInfo, q, nameServer, r.rootCAs, r.verifyServerCert, requestIteration, r.ednsOptions, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit) - } else if connInfo.udpClient != nil { - r.verboseLog(depth, "****WIRE LOOKUP*** ", UDPProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) - result, rawResp, status, err = wireLookupUDP(lookupCtx, connInfo, q, nameServer, r.ednsOptions, requestIteration, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit) - if status == StatusTruncated && connInfo.tcpClient != nil { - // result truncated, try again with TCP - r.verboseLog(depth, "****WIRE LOOKUP*** ", TCPProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) - result, rawResp, status, err = wireLookupTCP(lookupCtx, connInfo, q, nameServer, r.ednsOptions, requestIteration, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit) + result, rawResp, status, err = r.performWireLookup(lookupCtx, connInfo, q, nameServer, requestIteration, depth, true) + if err == nil && shouldRetryWithoutEDNS(rawResp, r.ednsOptions, r.dnsSecEnabled) { + fallbackCtx, fallbackCancel := context.WithTimeout(ctx, r.networkTimeout) + defer fallbackCancel() + + if rateLimitErr := r.rateLimit.wait(fallbackCtx, *nameServer); rateLimitErr != nil { + return &SingleQueryResult{}, false, StatusError, trace, fmt.Errorf("rate limiter error for EDNS fallback against nameserver %s: %w", nameServer, rateLimitErr) } - } else if connInfo.tcpClient != nil { - r.verboseLog(depth, "****WIRE LOOKUP*** ", TCPProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) - result, rawResp, status, err = wireLookupTCP(lookupCtx, connInfo, q, nameServer, r.ednsOptions, requestIteration, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit) - } else { - return &SingleQueryResult{}, false, StatusError, trace, errors.New("no connection info for nameserver") + result, rawResp, status, err = r.performWireLookup(fallbackCtx, connInfo, q, nameServer, requestIteration, depth, false) } if err != nil { @@ -943,23 +933,44 @@ func (r *Resolver) cachedLookup(ctx context.Context, q Question, nameServer *Nam return result, isCached, status, trace, err } -func doDoTLookup(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, rootCAs *x509.CertPool, shouldVerifyServerCert, recursive bool, ednsOptions []dns.EDNS0, dnssec, checkingDisabled, authenticatedData bool) (*SingleQueryResult, *dns.Msg, Status, error) { +func (r *Resolver) performWireLookup(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, recursive bool, depth int, useEDNS bool) (*SingleQueryResult, *dns.Msg, Status, error) { + logPrefix := "****WIRE LOOKUP*** " + if !useEDNS { + logPrefix = "****WIRE LOOKUP WITHOUT EDNS*** " + } + + if r.dnsOverHTTPSEnabled { + r.verboseLog(depth, logPrefix, DoHProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) + return doDoHLookup(ctx, connInfo.httpsClient, q, nameServer, recursive, r.ednsOptions, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit, useEDNS) + } + if r.dnsOverTLSEnabled { + r.verboseLog(depth, logPrefix, DoTProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) + return doDoTLookup(ctx, connInfo, q, nameServer, r.rootCAs, r.verifyServerCert, recursive, r.ednsOptions, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit, useEDNS) + } + if connInfo.udpClient != nil { + r.verboseLog(depth, logPrefix, UDPProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) + result, rawResp, status, err := wireLookupUDP(ctx, connInfo, q, nameServer, r.ednsOptions, recursive, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit, useEDNS) + if status == StatusTruncated && connInfo.tcpClient != nil { + // result truncated, try again with TCP + r.verboseLog(depth, logPrefix, TCPProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) + return wireLookupTCP(ctx, connInfo, q, nameServer, r.ednsOptions, recursive, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit, useEDNS) + } + return result, rawResp, status, err + } + if connInfo.tcpClient != nil { + r.verboseLog(depth, logPrefix, TCPProtocol, " ", dns.TypeToString[q.Type], " ", q.Name, " ", nameServer) + return wireLookupTCP(ctx, connInfo, q, nameServer, r.ednsOptions, recursive, r.dnsSecEnabled, r.checkingDisabledBit, r.authenticatedDataBit, useEDNS) + } + return &SingleQueryResult{}, nil, StatusError, errors.New("no connection info for nameserver") +} + +func doDoTLookup(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, rootCAs *x509.CertPool, shouldVerifyServerCert, recursive bool, ednsOptions []dns.EDNS0, dnssec, checkingDisabled, authenticatedData, useEDNS bool) (*SingleQueryResult, *dns.Msg, Status, error) { if util.HasCtxExpired(ctx) { return nil, nil, StatusTimeout, errors.New("context expired") } - m := new(dns.Msg) - m.SetQuestion(dotName(q.Name), q.Type) - m.Question[0].Qclass = q.Class - m.RecursionDesired = recursive - m.CheckingDisabled = checkingDisabled - m.AuthenticatedData = authenticatedData + m := makeWireQuery(q, recursive, ednsOptions, dnssec, checkingDisabled, authenticatedData, useEDNS) m.Id = 12345 - m.SetEdns0(1232, dnssec) - if ednsOpt := m.IsEdns0(); ednsOpt != nil { - ednsOpt.Option = append(ednsOpt.Option, ednsOptions...) - } - // if tlsConn is nil or if this is a new nameserver, create a new connection var isConnNew bool if connInfo.tlsConn != nil { @@ -1038,18 +1049,8 @@ func doDoTLookup(ctx context.Context, connInfo *ConnectionInfo, q Question, name return constructSingleQueryResultFromDNSMsg(&res, responseMsg) } -func doDoHLookup(ctx context.Context, httpClient *http.Client, q Question, nameServer *NameServer, recursive bool, ednsOptions []dns.EDNS0, dnssec, checkingDisabled, authenticatedData bool) (*SingleQueryResult, *dns.Msg, Status, error) { - m := new(dns.Msg) - m.SetQuestion(dotName(q.Name), q.Type) - m.Question[0].Qclass = q.Class - m.RecursionDesired = recursive - m.CheckingDisabled = checkingDisabled - m.AuthenticatedData = authenticatedData - - m.SetEdns0(1232, dnssec) - if ednsOpt := m.IsEdns0(); ednsOpt != nil { - ednsOpt.Option = append(ednsOpt.Option, ednsOptions...) - } +func doDoHLookup(ctx context.Context, httpClient *http.Client, q Question, nameServer *NameServer, recursive bool, ednsOptions []dns.EDNS0, dnssec, checkingDisabled, authenticatedData, useEDNS bool) (*SingleQueryResult, *dns.Msg, Status, error) { + m := makeWireQuery(q, recursive, ednsOptions, dnssec, checkingDisabled, authenticatedData, useEDNS) bytes, err := m.Pack() if err != nil { return nil, nil, StatusError, errors.Wrap(err, "could not pack DNS message") @@ -1111,21 +1112,11 @@ func doDoHLookup(ctx context.Context, httpClient *http.Client, q Question, nameS } // wireLookupTCP performs a DNS lookup on-the-wire over TCP with the given parameters -func wireLookupTCP(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, ednsOptions []dns.EDNS0, recursive, dnssec, checkingDisabled, authenticatedData bool) (*SingleQueryResult, *dns.Msg, Status, error) { +func wireLookupTCP(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, ednsOptions []dns.EDNS0, recursive, dnssec, checkingDisabled, authenticatedData, useEDNS bool) (*SingleQueryResult, *dns.Msg, Status, error) { res := SingleQueryResult{Answers: []any{}, Authorities: []any{}, Additionals: []any{}} res.Resolver = nameServer.String() - m := new(dns.Msg) - m.SetQuestion(dotName(q.Name), q.Type) - m.Question[0].Qclass = q.Class - m.RecursionDesired = recursive - m.CheckingDisabled = checkingDisabled - m.AuthenticatedData = authenticatedData - - m.SetEdns0(1232, dnssec) - if ednsOpt := m.IsEdns0(); ednsOpt != nil { - ednsOpt.Option = append(ednsOpt.Option, ednsOptions...) - } + m := makeWireQuery(q, recursive, ednsOptions, dnssec, checkingDisabled, authenticatedData, useEDNS) var r *dns.Msg var err error @@ -1172,22 +1163,12 @@ func wireLookupTCP(ctx context.Context, connInfo *ConnectionInfo, q Question, na } // wireLookupUDP performs a DNS lookup on-the-wire over UDP with the given parameters -func wireLookupUDP(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, ednsOptions []dns.EDNS0, recursive, dnssec, checkingDisabled, authenticatedData bool) (*SingleQueryResult, *dns.Msg, Status, error) { +func wireLookupUDP(ctx context.Context, connInfo *ConnectionInfo, q Question, nameServer *NameServer, ednsOptions []dns.EDNS0, recursive, dnssec, checkingDisabled, authenticatedData, useEDNS bool) (*SingleQueryResult, *dns.Msg, Status, error) { res := SingleQueryResult{Answers: []any{}, Authorities: []any{}, Additionals: []any{}} res.Resolver = nameServer.String() res.Protocol = "udp" - m := new(dns.Msg) - m.SetQuestion(dotName(q.Name), q.Type) - m.Question[0].Qclass = q.Class - m.RecursionDesired = recursive - m.CheckingDisabled = checkingDisabled - m.AuthenticatedData = authenticatedData - - m.SetEdns0(1232, dnssec) - if ednsOpt := m.IsEdns0(); ednsOpt != nil { - ednsOpt.Option = append(ednsOpt.Option, ednsOptions...) - } + m := makeWireQuery(q, recursive, ednsOptions, dnssec, checkingDisabled, authenticatedData, useEDNS) var r *dns.Msg var err error @@ -1218,6 +1199,34 @@ func wireLookupUDP(ctx context.Context, connInfo *ConnectionInfo, q Question, na return constructSingleQueryResultFromDNSMsg(&res, r) } +func makeWireQuery(q Question, recursive bool, ednsOptions []dns.EDNS0, dnssec, checkingDisabled, authenticatedData, useEDNS bool) *dns.Msg { + m := new(dns.Msg) + m.SetQuestion(dotName(q.Name), q.Type) + m.Question[0].Qclass = q.Class + m.RecursionDesired = recursive + m.CheckingDisabled = checkingDisabled + m.AuthenticatedData = authenticatedData + + if useEDNS { + m.SetEdns0(1232, dnssec) + if ednsOpt := m.IsEdns0(); ednsOpt != nil { + ednsOpt.Option = append(ednsOpt.Option, ednsOptions...) + } + } + return m +} + +func shouldRetryWithoutEDNS(response *dns.Msg, ednsOptions []dns.EDNS0, dnssec bool) bool { + // RFC 6891 section 7 specifies FORMERR without an OPT record as the response + // from a server that does not implement EDNS. Queries that require an EDNS + // feature cannot be retried without changing their requested semantics. + return response != nil && + response.Rcode == dns.RcodeFormatError && + response.IsEdns0() == nil && + !dnssec && + len(ednsOptions) == 0 +} + // fills out all the fields in a SingleQueryResult from a dns.Msg directly. func constructSingleQueryResultFromDNSMsg(res *SingleQueryResult, r *dns.Msg) (*SingleQueryResult, *dns.Msg, Status, error) { if r.Rcode != dns.RcodeSuccess { diff --git a/src/zdns/lookup_test.go b/src/zdns/lookup_test.go index 072687b3..a693ed4d 100644 --- a/src/zdns/lookup_test.go +++ b/src/zdns/lookup_test.go @@ -2249,3 +2249,216 @@ func TestTCPConnNotPoisonedOnTimeout(t *testing.T) { /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // END TCP Connection Poison Tests /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// EDNS Fallback Tests +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +type countingNameServerRateLimiter struct { + waits atomic.Int32 +} + +func (l *countingNameServerRateLimiter) wait(context.Context, NameServer) error { + l.waits.Add(1) + return nil +} + +func startEDNSFallbackTestServer(t *testing.T, network string, formErrIncludesOPT bool, ednsDelay, plainDelay time.Duration) (NameServer, *atomic.Int32, *atomic.Int32) { + t.Helper() + + var ednsQueries atomic.Int32 + var plainQueries atomic.Int32 + handler := dns.HandlerFunc(func(w dns.ResponseWriter, req *dns.Msg) { + resp := new(dns.Msg) + if req.IsEdns0() != nil { + ednsQueries.Add(1) + time.Sleep(ednsDelay) + resp.SetRcodeFormatError(req) + if formErrIncludesOPT { + resp.SetEdns0(1232, false) + } + } else { + plainQueries.Add(1) + time.Sleep(plainDelay) + resp.SetReply(req) + resp.Authoritative = true + resp.Answer = []dns.RR{ + &dns.A{ + Hdr: dns.RR_Header{ + Name: req.Question[0].Name, + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: 60, + }, + A: net.ParseIP("192.0.2.1"), + }, + } + } + assert.NoError(t, w.WriteMsg(resp)) + }) + + server := &dns.Server{Net: network, Handler: handler} + var addr net.Addr + switch network { + case "udp": + packetConn, err := net.ListenPacket("udp", "127.0.0.1:0") + require.NoError(t, err) + server.PacketConn = packetConn + addr = packetConn.LocalAddr() + case "tcp": + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + server.Listener = listener + addr = listener.Addr() + default: + t.Fatalf("unsupported test network %q", network) + } + + go func() { + _ = server.ActivateAndServe() + }() + t.Cleanup(func() { + require.NoError(t, server.Shutdown()) + }) + + host, portString, err := net.SplitHostPort(addr.String()) + require.NoError(t, err) + port, err := strconv.Atoi(portString) + require.NoError(t, err) + return NameServer{IP: net.ParseIP(host), Port: uint16(port)}, &ednsQueries, &plainQueries +} + +func TestCachedLookupEDNSFallback(t *testing.T) { + tests := []struct { + name string + network string + formErrIncludesOPT bool + ednsOptions []dns.EDNS0 + dnssec bool + expectedStatus Status + expectedEDNSQueries int32 + expectedPlainQueries int32 + expectedRateLimitWaits int32 + }{ + { + name: "UDP retries without EDNS", + network: "udp", + expectedStatus: StatusNoError, + expectedEDNSQueries: 1, + expectedPlainQueries: 1, + expectedRateLimitWaits: 2, + }, + { + name: "TCP retries without EDNS", + network: "tcp", + expectedStatus: StatusNoError, + expectedEDNSQueries: 1, + expectedPlainQueries: 1, + expectedRateLimitWaits: 2, + }, + { + name: "FORMERR with OPT does not retry", + network: "udp", + formErrIncludesOPT: true, + expectedStatus: StatusFormErr, + expectedEDNSQueries: 1, + expectedPlainQueries: 0, + expectedRateLimitWaits: 1, + }, + { + name: "DNSSEC query does not retry", + network: "udp", + dnssec: true, + expectedStatus: StatusFormErr, + expectedEDNSQueries: 1, + expectedPlainQueries: 0, + expectedRateLimitWaits: 1, + }, + { + name: "explicit EDNS option does not retry", + network: "udp", + ednsOptions: []dns.EDNS0{ + &dns.EDNS0_NSID{Code: dns.EDNS0NSID}, + }, + expectedStatus: StatusFormErr, + expectedEDNSQueries: 1, + expectedPlainQueries: 0, + expectedRateLimitWaits: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nameServer, ednsQueries, plainQueries := startEDNSFallbackTestServer(t, tt.network, tt.formErrIncludesOPT, 0, 0) + rateLimiter := new(countingNameServerRateLimiter) + config := NewResolverConfig() + config.RateLimiter = rateLimiter + config.IPVersionMode = IPv4Only + config.ShouldRecycleSockets = false + config.NetworkTimeout = time.Second + config.LocalAddrsV4 = []net.IP{net.ParseIP("127.0.0.1")} + config.ExternalNameServersV4 = []NameServer{nameServer} + config.RootNameServersV4 = []NameServer{nameServer} + config.DNSSecEnabled = tt.dnssec + config.EdnsOptions = tt.ednsOptions + if tt.network == "udp" { + config.TransportMode = UDPOnly + } else { + config.TransportMode = TCPOnly + } + resolver, err := InitResolver(config) + require.NoError(t, err) + defer resolver.Close() + + q := Question{Name: "example.com", Type: dns.TypeA, Class: dns.ClassINET} + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + result, _, status, _, err := resolver.cachedLookup(ctx, q, &nameServer, q.Name, 0, true, true, false, nil) + + require.NoError(t, err) + require.Equal(t, tt.expectedStatus, status) + require.Equal(t, tt.expectedEDNSQueries, ednsQueries.Load()) + require.Equal(t, tt.expectedPlainQueries, plainQueries.Load()) + require.Equal(t, tt.expectedRateLimitWaits, rateLimiter.waits.Load()) + if tt.expectedStatus == StatusNoError { + require.Len(t, result.Answers, 1) + } + }) + } +} + +func TestExternalLookupEDNSFallbackGetsFreshNetworkTimeout(t *testing.T) { + const networkTimeout = 500 * time.Millisecond + const responseDelay = 300 * time.Millisecond + + nameServer, ednsQueries, plainQueries := startEDNSFallbackTestServer(t, "udp", false, responseDelay, responseDelay) + rateLimiter := new(countingNameServerRateLimiter) + config := NewResolverConfig() + config.RateLimiter = rateLimiter + config.TransportMode = UDPOnly + config.IPVersionMode = IPv4Only + config.ShouldRecycleSockets = false + config.NetworkTimeout = networkTimeout + config.Timeout = 2 * time.Second + config.LocalAddrsV4 = []net.IP{net.ParseIP("127.0.0.1")} + config.ExternalNameServersV4 = []NameServer{nameServer} + + resolver, err := InitResolver(config) + require.NoError(t, err) + defer resolver.Close() + + q := Question{Name: "example.com", Type: dns.TypeA, Class: dns.ClassINET} + result, _, status, err := resolver.ExternalLookup(context.Background(), &q, &nameServer) + + require.NoError(t, err) + require.Equal(t, StatusNoError, status) + require.Len(t, result.Answers, 1) + require.Equal(t, int32(1), ednsQueries.Load()) + require.Equal(t, int32(1), plainQueries.Load()) + require.Equal(t, int32(2), rateLimiter.waits.Load()) +} + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// END EDNS Fallback Tests +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/zdns/resolver.go b/src/zdns/resolver.go index cc54b72e..f2331f72 100644 --- a/src/zdns/resolver.go +++ b/src/zdns/resolver.go @@ -674,7 +674,9 @@ func (r *Resolver) ExternalLookup(ctx context.Context, q *Question, dstServer *N if r.isClosed { log.Fatal("resolver has been closed, cannot perform lookup") } - ctx, cancelFn := context.WithTimeout(ctx, r.networkTimeout) + // Bound the complete external lookup separately from each on-wire operation. + // cachedLookup applies networkTimeout to individual attempts, including EDNS fallback. + ctx, cancelFn := context.WithTimeout(ctx, r.timeout) defer cancelFn() // If dstServer is not provided, AND we're in HTTPS/TLS/TCP mode, AND we have a pre-existing external name server, use it if dstServer == nil && r.lastUsedExternalNameServer == nil {