From 3c8b204ef5c6783b15bed02f7b7418e78aaa4178 Mon Sep 17 00:00:00 2001 From: devStorm <59678453+developStorm@users.noreply.github.com> Date: Tue, 19 Nov 2024 00:45:51 -0800 Subject: [PATCH 1/2] feat: lazy query A/AAAA of NSes when iterating on authorities --- src/zdns/lookup.go | 78 +++++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/src/zdns/lookup.go b/src/zdns/lookup.go index b34bc244..905dc6a9 100644 --- a/src/zdns/lookup.go +++ b/src/zdns/lookup.go @@ -897,30 +897,35 @@ func (r *Resolver) iterateOnAuthorities(ctx context.Context, qWithMeta *Question if len(result.Authorities) == 0 { return nil, trace, StatusNoAuth, nil } - var newLayer string - newTrace := trace - nameServers := make([]NameServer, 0, len(result.Authorities)) - for i, elem := range result.Authorities { - // Skip DS and RRSIG records as they are not nameservers but may present in the section - // https://datatracker.ietf.org/doc/html/rfc4035#section-3.1.4 + + // Shuffle authorities to try them in random order + authorities := make([]interface{}, len(result.Authorities)) + copy(authorities, result.Authorities) + rand.Shuffle(len(authorities), func(i, j int) { + authorities[i], authorities[j] = authorities[j], authorities[i] + }) + + for _, elem := range authorities { + // Skip DS and RRSIG records switch elem.(type) { case DSAnswer, RRSIGAnswer: continue } if util.HasCtxExpired(&ctx) { - return &SingleQueryResult{}, newTrace, StatusTimeout, nil + return &SingleQueryResult{}, trace, StatusTimeout, nil } - var ns *NameServer - var nsStatus Status - var nextLayer string + r.verboseLog(depth+1, "Trying Authority: ", elem) - ns, nsStatus, nextLayer, newTrace = r.extractAuthority(ctx, elem, layer, depth, result, newTrace) + + // Extract authority details + ns, nsStatus, nextLayer, newTrace := r.extractAuthority(ctx, elem, layer, depth, result, trace) + trace = newTrace r.verboseLog(depth+1, "Output from extract authorities: ", ns.String()) if nsStatus == StatusIterTimeout { - r.verboseLog(depth+2, "--> Hit iterative timeout: ") - return &SingleQueryResult{}, newTrace, StatusIterTimeout, nil + r.verboseLog(depth+2, "--> Hit iterative timeout") + return &SingleQueryResult{}, trace, StatusIterTimeout, nil } if nsStatus != StatusNoError { @@ -931,33 +936,29 @@ func (r *Resolver) iterateOnAuthorities(ctx context.Context, qWithMeta *Question } else { r.verboseLog(depth+2, "--> Auth find failed for name ", qWithMeta.Q.Name, " with status: ", newStatus) } - if i+1 == len(result.Authorities) { - r.verboseLog(depth+2, "--> No more authorities to try for name ", qWithMeta.Q.Name, ", terminating: ", nsStatus) - } - } else { - // We have a valid nameserver - newLayer = nextLayer - nameServers = append(nameServers, *ns) + continue } - } - if len(nameServers) == 0 { - r.verboseLog(depth+1, fmt.Sprintf("--> Auth found no valid nameservers for name: %s Terminating", qWithMeta.Q.Name)) - return &SingleQueryResult{}, newTrace, StatusServFail, errors.New("no valid nameservers found") - } + // Try iterative lookup immediately with this nameserver + iterateResult, newTrace, status, err := r.iterativeLookup(ctx, qWithMeta, []NameServer{*ns}, depth+1, nextLayer, trace) + trace = newTrace - iterateResult, newTrace, status, err := r.iterativeLookup(ctx, qWithMeta, nameServers, depth+1, newLayer, newTrace) - if status == StatusNoNeededGlue { - r.verboseLog((depth + 2), "--> Auth resolution of ", nameServers, " was unsuccessful. No glue to follow", status) - return iterateResult, newTrace, status, err - } else if isStatusAnswer(status) { - r.verboseLog((depth + 1), "--> Auth Resolution of ", nameServers, " success: ", status) - return iterateResult, newTrace, status, err - } else { - // We don't allow the continue fall through in order to report the last auth failure code, not STATUS_ERROR - r.verboseLog((depth + 2), "--> Iterative resolution of ", qWithMeta.Q.Name, " at ", nameServers, " Failed. Last auth. Terminating: ", status) - return iterateResult, newTrace, status, err + if status == StatusNoNeededGlue { + r.verboseLog(depth+2, "--> Auth resolution of ", ns, " was unsuccessful. No glue to follow") + continue + } + + if isStatusAnswer(status) { + r.verboseLog(depth+1, "--> Auth Resolution of ", ns, " success: ", status) + return iterateResult, trace, status, err + } + + r.verboseLog(depth+2, "--> Iterative resolution of ", qWithMeta.Q.Name, " at ", ns, " Failed: ", status) } + + // If we get here, all authorities failed + r.verboseLog(depth+2, "--> No more authorities to try for name ", qWithMeta.Q.Name, ", terminating") + return &SingleQueryResult{}, trace, StatusServFail, errors.New("no valid nameservers found or all lookups failed") } func (r *Resolver) extractAuthority(ctx context.Context, authority interface{}, layer string, depth int, result *SingleQueryResult, trace Trace) (*NameServer, Status, string, Trace) { @@ -995,7 +996,12 @@ func (r *Resolver) extractAuthority(ctx context.Context, authority interface{}, q.Q.Type = dns.TypeA } q.RetriesRemaining = &r.retriesRemaining + + // A/AAAA records for NSes are not on the chain of trust, so we don't need to validate DNSSEC + // Doing this to save us some time (this can propogate A LOT of queries in certain cases) + r.shouldValidateDNSSEC = false res, trace, status, _ = r.iterativeLookup(ctx, &q, r.rootNameServers, depth+1, ".", trace) + r.shouldValidateDNSSEC = true } if status == StatusIterTimeout || status == StatusNoNeededGlue { return nil, status, "", trace From 989055717374f7f39a108b4cbad318596ed4acb9 Mon Sep 17 00:00:00 2001 From: devStorm <59678453+developStorm@users.noreply.github.com> Date: Tue, 19 Nov 2024 16:01:41 -0800 Subject: [PATCH 2/2] fix: should restore the previous dnssec setting --- src/zdns/lookup.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/zdns/lookup.go b/src/zdns/lookup.go index 905dc6a9..d64ccd56 100644 --- a/src/zdns/lookup.go +++ b/src/zdns/lookup.go @@ -893,6 +893,8 @@ func constructSingleQueryResultFromDNSMsg(res *SingleQueryResult, r *dns.Msg) (* return res, r, StatusNoError, nil } +// iterateOnAuthorities takes the authorities from the referrals of a nameserver, shuffles them, and iteratively tries to do a lookup against them. +// If one succeeds, we return without trying the others. If one fails, we iterate to the next. func (r *Resolver) iterateOnAuthorities(ctx context.Context, qWithMeta *QuestionWithMetadata, depth int, result *SingleQueryResult, layer string, trace Trace) (*SingleQueryResult, Trace, Status, error) { if len(result.Authorities) == 0 { return nil, trace, StatusNoAuth, nil @@ -999,9 +1001,10 @@ func (r *Resolver) extractAuthority(ctx context.Context, authority interface{}, // A/AAAA records for NSes are not on the chain of trust, so we don't need to validate DNSSEC // Doing this to save us some time (this can propogate A LOT of queries in certain cases) + prevSecValue := r.shouldValidateDNSSEC r.shouldValidateDNSSEC = false res, trace, status, _ = r.iterativeLookup(ctx, &q, r.rootNameServers, depth+1, ".", trace) - r.shouldValidateDNSSEC = true + r.shouldValidateDNSSEC = prevSecValue } if status == StatusIterTimeout || status == StatusNoNeededGlue { return nil, status, "", trace