From c09312974c595d4ee51b9b8b5b8488d7640388c3 Mon Sep 17 00:00:00 2001 From: Odin Hultgren van der Horst Date: Fri, 5 Jan 2024 13:25:17 +0100 Subject: [PATCH 01/13] Added WithTCPRetry option that fixses truncated answers --- resolver.go | 26 ++++++++++++++++++++++++++ resolver_test.go | 13 +++++++++++++ 2 files changed, 39 insertions(+) diff --git a/resolver.go b/resolver.go index ba43e61..7afa703 100644 --- a/resolver.go +++ b/resolver.go @@ -67,6 +67,15 @@ func WithTimeout(timeout time.Duration) Option { } } +// WithTCPRetry specifies that requests should be retried with TCP if responses +// are truncated. +func WithTCPRetry() Option { + return func(r *Resolver) { + r.tcpRetry = true + } +} + + // Resolver implements a primitive, non-recursive, caching DNS resolver. type Resolver struct { dialer ContextDialer @@ -74,6 +83,7 @@ type Resolver struct { cache *cache capacity int expire bool + tcpRetry bool } // NewResolver returns an initialized Resolver with options. @@ -332,6 +342,22 @@ func (r *Resolver) exchangeIP(ctx context.Context, host, ip, qname, qtype string rmsg, dur, err = client.ExchangeWithConnContext(ctx, &qmsg, dconn) conn.Close() } + if r.tcpRetry && rmsg != nil && rmsg.MsgHdr.Truncated { + // Since we are doing another query, we need to recheck the deadline + if dl, ok := ctx.Deadline(); ok { + if start.After(dl.Add(-TypicalResponseTime)) { // bail if we can't finish in time (start is too close to deadline) + return nil, ErrTimeout + } + timeout = dl.Sub(start) + } + // Retry with TCP + conn, err := dialer.DialContext(ctx, "tcp", addr) + if err == nil { + dconn := &dns.Conn{Conn: conn} + rmsg, dur, err = client.ExchangeWithConnContext(ctx, &qmsg, dconn) + conn.Close() + } + } select { case <-ctx.Done(): // Finished too late diff --git a/resolver_test.go b/resolver_test.go index dd9ee09..f89bed8 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -179,6 +179,19 @@ func TestGoogleTXT(t *testing.T) { st.Expect(t, count(rrs, func(rr RR) bool { return rr.Type == "TXT" }) >= 1, true) } +func TestGoogleTXTTCPRetry(t *testing.T) { + r := NewResolver() + rrs, err := r.ResolveErr("google.com", "TXT") + st.Expect(t, err, nil) + st.Expect(t, len(rrs) >= 4, true) + + r2 := NewResolver(WithTCPRetry()) + rrs2, err := r2.ResolveErr("google.com", "TXT") + st.Expect(t, err, nil) + st.Expect(t, len(rrs2) > len(rrs), true) +} + + func TestAppleA(t *testing.T) { r := NewResolver() rrs, err := r.ResolveErr("apple.com", "A") From 1582a7ff53fff8d7ec27322d2cb90048cc104388 Mon Sep 17 00:00:00 2001 From: Odin Hultgren Van Der Horst Date: Mon, 29 Jan 2024 21:49:53 +0100 Subject: [PATCH 02/13] Update resolver.go Co-authored-by: Cameron Walters (cee-dub) --- resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolver.go b/resolver.go index 7afa703..45ec3a9 100644 --- a/resolver.go +++ b/resolver.go @@ -68,7 +68,7 @@ func WithTimeout(timeout time.Duration) Option { } // WithTCPRetry specifies that requests should be retried with TCP if responses -// are truncated. +// are truncated. The retry must still complete within the timeout or context deadline. func WithTCPRetry() Option { return func(r *Resolver) { r.tcpRetry = true From d18731dadce210227f7465dbcea33eca71ffc080 Mon Sep 17 00:00:00 2001 From: Odin Hultgren van der Horst Date: Mon, 29 Jan 2024 21:51:38 +0100 Subject: [PATCH 03/13] Fixed so timeout is used correctly on tcp retry --- resolver.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/resolver.go b/resolver.go index 45ec3a9..d5d7251 100644 --- a/resolver.go +++ b/resolver.go @@ -348,7 +348,7 @@ func (r *Resolver) exchangeIP(ctx context.Context, host, ip, qname, qtype string if start.After(dl.Add(-TypicalResponseTime)) { // bail if we can't finish in time (start is too close to deadline) return nil, ErrTimeout } - timeout = dl.Sub(start) + client.Timeout = dl.Sub(start) } // Retry with TCP conn, err := dialer.DialContext(ctx, "tcp", addr) @@ -361,10 +361,10 @@ func (r *Resolver) exchangeIP(ctx context.Context, host, ip, qname, qtype string select { case <-ctx.Done(): // Finished too late - logCancellation(host, &qmsg, rmsg, depth, dur, timeout) + logCancellation(host, &qmsg, rmsg, depth, dur, client.Timeout) return nil, ctx.Err() default: - logExchange(host, &qmsg, rmsg, depth, dur, timeout, err) // Log hostname instead of IP + logExchange(host, &qmsg, rmsg, depth, dur, client.Timeout, err) // Log hostname instead of IP } if err != nil { return nil, err From 4410a257fd5d10a0785d9cfb4d6e95d2af14318f Mon Sep 17 00:00:00 2001 From: Cameron Walters Date: Mon, 12 Feb 2024 07:21:54 -0800 Subject: [PATCH 04/13] cmd/dnsr: add -t to expose WithTCPRetry option --- cmd/dnsr/main.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/dnsr/main.go b/cmd/dnsr/main.go index f52442f..81cf676 100644 --- a/cmd/dnsr/main.go +++ b/cmd/dnsr/main.go @@ -14,16 +14,13 @@ import ( var ( verbose bool - resolver = dnsr.New(10000) + tcpRetry bool + resolver = dnsr.NewResolver(dnsr.WithCache(1000)) ) func init() { - flag.BoolVar( - &verbose, - "v", - false, - "print verbose info to the console", - ) + flag.BoolVar(&verbose, "v", false, "print verbose info to the console") + flag.BoolVar(&tcpRetry, "t", false, "enable TCP retry") } func logV(fmt string, args ...interface{}) { @@ -47,6 +44,9 @@ func main() { } else if _, isType := dns.StringToType[args[len(args)-1]]; len(args) > 1 && isType { qtype, args = args[len(args)-1], args[:len(args)-1] } + if tcpRetry { + resolver = dnsr.NewResolver(dnsr.WithTCPRetry()) + } if verbose { dnsr.DebugLogger = os.Stderr } From b10d9644f5c2b5197425c1de7ae69aa2d1fdbc54 Mon Sep 17 00:00:00 2001 From: Odin Hultgren van der Horst Date: Fri, 5 Jan 2024 13:25:17 +0100 Subject: [PATCH 05/13] Added WithTCPRetry option that fixses truncated answers --- resolver.go | 26 ++++++++++++++++++++++++++ resolver_test.go | 13 +++++++++++++ 2 files changed, 39 insertions(+) diff --git a/resolver.go b/resolver.go index ba43e61..7afa703 100644 --- a/resolver.go +++ b/resolver.go @@ -67,6 +67,15 @@ func WithTimeout(timeout time.Duration) Option { } } +// WithTCPRetry specifies that requests should be retried with TCP if responses +// are truncated. +func WithTCPRetry() Option { + return func(r *Resolver) { + r.tcpRetry = true + } +} + + // Resolver implements a primitive, non-recursive, caching DNS resolver. type Resolver struct { dialer ContextDialer @@ -74,6 +83,7 @@ type Resolver struct { cache *cache capacity int expire bool + tcpRetry bool } // NewResolver returns an initialized Resolver with options. @@ -332,6 +342,22 @@ func (r *Resolver) exchangeIP(ctx context.Context, host, ip, qname, qtype string rmsg, dur, err = client.ExchangeWithConnContext(ctx, &qmsg, dconn) conn.Close() } + if r.tcpRetry && rmsg != nil && rmsg.MsgHdr.Truncated { + // Since we are doing another query, we need to recheck the deadline + if dl, ok := ctx.Deadline(); ok { + if start.After(dl.Add(-TypicalResponseTime)) { // bail if we can't finish in time (start is too close to deadline) + return nil, ErrTimeout + } + timeout = dl.Sub(start) + } + // Retry with TCP + conn, err := dialer.DialContext(ctx, "tcp", addr) + if err == nil { + dconn := &dns.Conn{Conn: conn} + rmsg, dur, err = client.ExchangeWithConnContext(ctx, &qmsg, dconn) + conn.Close() + } + } select { case <-ctx.Done(): // Finished too late diff --git a/resolver_test.go b/resolver_test.go index dd9ee09..f89bed8 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -179,6 +179,19 @@ func TestGoogleTXT(t *testing.T) { st.Expect(t, count(rrs, func(rr RR) bool { return rr.Type == "TXT" }) >= 1, true) } +func TestGoogleTXTTCPRetry(t *testing.T) { + r := NewResolver() + rrs, err := r.ResolveErr("google.com", "TXT") + st.Expect(t, err, nil) + st.Expect(t, len(rrs) >= 4, true) + + r2 := NewResolver(WithTCPRetry()) + rrs2, err := r2.ResolveErr("google.com", "TXT") + st.Expect(t, err, nil) + st.Expect(t, len(rrs2) > len(rrs), true) +} + + func TestAppleA(t *testing.T) { r := NewResolver() rrs, err := r.ResolveErr("apple.com", "A") From 79dfda7d45371dd22d2804b33e544f5bca8a02be Mon Sep 17 00:00:00 2001 From: Odin Hultgren Van Der Horst Date: Mon, 29 Jan 2024 21:49:53 +0100 Subject: [PATCH 06/13] Update resolver.go Co-authored-by: Cameron Walters (cee-dub) --- resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolver.go b/resolver.go index 7afa703..45ec3a9 100644 --- a/resolver.go +++ b/resolver.go @@ -68,7 +68,7 @@ func WithTimeout(timeout time.Duration) Option { } // WithTCPRetry specifies that requests should be retried with TCP if responses -// are truncated. +// are truncated. The retry must still complete within the timeout or context deadline. func WithTCPRetry() Option { return func(r *Resolver) { r.tcpRetry = true From 136e35ae9b22e5fb100bb32e56ab25de549ecd0f Mon Sep 17 00:00:00 2001 From: Odin Hultgren van der Horst Date: Mon, 29 Jan 2024 21:51:38 +0100 Subject: [PATCH 07/13] Fixed so timeout is used correctly on tcp retry --- resolver.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/resolver.go b/resolver.go index 45ec3a9..d5d7251 100644 --- a/resolver.go +++ b/resolver.go @@ -348,7 +348,7 @@ func (r *Resolver) exchangeIP(ctx context.Context, host, ip, qname, qtype string if start.After(dl.Add(-TypicalResponseTime)) { // bail if we can't finish in time (start is too close to deadline) return nil, ErrTimeout } - timeout = dl.Sub(start) + client.Timeout = dl.Sub(start) } // Retry with TCP conn, err := dialer.DialContext(ctx, "tcp", addr) @@ -361,10 +361,10 @@ func (r *Resolver) exchangeIP(ctx context.Context, host, ip, qname, qtype string select { case <-ctx.Done(): // Finished too late - logCancellation(host, &qmsg, rmsg, depth, dur, timeout) + logCancellation(host, &qmsg, rmsg, depth, dur, client.Timeout) return nil, ctx.Err() default: - logExchange(host, &qmsg, rmsg, depth, dur, timeout, err) // Log hostname instead of IP + logExchange(host, &qmsg, rmsg, depth, dur, client.Timeout, err) // Log hostname instead of IP } if err != nil { return nil, err From f0721b20350428b978a4e27054a48d21850c3256 Mon Sep 17 00:00:00 2001 From: Cameron Walters Date: Mon, 12 Feb 2024 07:21:54 -0800 Subject: [PATCH 08/13] cmd/dnsr: add -t to expose WithTCPRetry option --- cmd/dnsr/main.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/dnsr/main.go b/cmd/dnsr/main.go index f52442f..81cf676 100644 --- a/cmd/dnsr/main.go +++ b/cmd/dnsr/main.go @@ -14,16 +14,13 @@ import ( var ( verbose bool - resolver = dnsr.New(10000) + tcpRetry bool + resolver = dnsr.NewResolver(dnsr.WithCache(1000)) ) func init() { - flag.BoolVar( - &verbose, - "v", - false, - "print verbose info to the console", - ) + flag.BoolVar(&verbose, "v", false, "print verbose info to the console") + flag.BoolVar(&tcpRetry, "t", false, "enable TCP retry") } func logV(fmt string, args ...interface{}) { @@ -47,6 +44,9 @@ func main() { } else if _, isType := dns.StringToType[args[len(args)-1]]; len(args) > 1 && isType { qtype, args = args[len(args)-1], args[:len(args)-1] } + if tcpRetry { + resolver = dnsr.NewResolver(dnsr.WithTCPRetry()) + } if verbose { dnsr.DebugLogger = os.Stderr } From a6c33c19dc031824d95c0cbf706f678f9e5201e6 Mon Sep 17 00:00:00 2001 From: Odin Hultgren van der Horst Date: Fri, 5 Jan 2024 11:58:21 +0100 Subject: [PATCH 09/13] Wrote test to check TXT results with default golang LookupTXT --- resolver_test.go | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/resolver_test.go b/resolver_test.go index f89bed8..e4ad83f 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -12,6 +12,29 @@ import ( "github.com/nbio/st" ) +func CheckTXT(t *testing.T, domain string) { + r := NewResolver() + rrs, err := r.ResolveErr(domain, "TXT") + st.Expect(t, err, nil) + + rrs2, err := net.LookupTXT(domain) + st.Expect(t, err, nil) + for _, rr := range rrs2 { + exsists := false + for _, rr2 := range rrs { + if rr2.Type == "TXT" && rr == rr2.Value { + exsists = true + } + } + if !exsists { + t.Errorf("TXT record %q not found", rr) + } + } + if count(rrs, func(rr RR) bool { return rr.Type == "TXT" }) != len(rrs2) { + t.Errorf("TXT record count mismatch: %d != %d", count(rrs, func(rr RR) bool { return rr.Type == "TXT" }), len(rrs2)) + } +} + func TestMain(m *testing.M) { flag.Parse() timeout := os.Getenv("DNSR_TIMEOUT") @@ -171,12 +194,11 @@ func TestGoogleMulti(t *testing.T) { } func TestGoogleTXT(t *testing.T) { - r := NewResolver() - rrs, err := r.ResolveErr("google.com", "TXT") - st.Expect(t, err, nil) - st.Expect(t, len(rrs) >= 4, true) - // Google will have at least an SPF record, but might transiently have verification records too. - st.Expect(t, count(rrs, func(rr RR) bool { return rr.Type == "TXT" }) >= 1, true) + CheckTXT(t, "google.com") +} + +func TestCloudflareTXT(t *testing.T) { + CheckTXT(t, "cloudflare.com") } func TestGoogleTXTTCPRetry(t *testing.T) { From cc908dfeebf29b24e8eec84ce83c047863b46672 Mon Sep 17 00:00:00 2001 From: Odin Hultgren Van Der Horst Date: Fri, 5 Jan 2024 12:55:36 +0100 Subject: [PATCH 10/13] Update resolver_test.go Co-authored-by: Mark McDonnell --- resolver_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/resolver_test.go b/resolver_test.go index e4ad83f..cbf6901 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -30,8 +30,9 @@ func CheckTXT(t *testing.T, domain string) { t.Errorf("TXT record %q not found", rr) } } - if count(rrs, func(rr RR) bool { return rr.Type == "TXT" }) != len(rrs2) { - t.Errorf("TXT record count mismatch: %d != %d", count(rrs, func(rr RR) bool { return rr.Type == "TXT" }), len(rrs2)) + c := count(rrs, func(rr RR) bool { return rr.Type == "TXT" }) + if c != len(rrs2) { + t.Errorf("TXT record count mismatch: %d != %d", c, len(rrs2)) } } From 325223723cd7529b787ffce4d712c04249f99ff9 Mon Sep 17 00:00:00 2001 From: Odin Hultgren Van Der Horst Date: Fri, 5 Jan 2024 12:55:41 +0100 Subject: [PATCH 11/13] Update resolver_test.go Co-authored-by: Mark McDonnell --- resolver_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/resolver_test.go b/resolver_test.go index cbf6901..9f5611e 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -20,13 +20,13 @@ func CheckTXT(t *testing.T, domain string) { rrs2, err := net.LookupTXT(domain) st.Expect(t, err, nil) for _, rr := range rrs2 { - exsists := false + exists := false for _, rr2 := range rrs { if rr2.Type == "TXT" && rr == rr2.Value { - exsists = true + exists = true } } - if !exsists { + if !exists { t.Errorf("TXT record %q not found", rr) } } From 679c47d21904708d988523fe05e112793ca2e2dd Mon Sep 17 00:00:00 2001 From: Odin Hultgren van der Horst Date: Fri, 5 Jan 2024 13:25:17 +0100 Subject: [PATCH 12/13] Fixed truncated answer --- resolver.go | 1 - 1 file changed, 1 deletion(-) diff --git a/resolver.go b/resolver.go index d5d7251..1f87431 100644 --- a/resolver.go +++ b/resolver.go @@ -75,7 +75,6 @@ func WithTCPRetry() Option { } } - // Resolver implements a primitive, non-recursive, caching DNS resolver. type Resolver struct { dialer ContextDialer From 45be6553e4b33d0b4d2ac22702939c75e3bf8b2e Mon Sep 17 00:00:00 2001 From: Cameron Walters Date: Mon, 12 Feb 2024 09:30:17 -0800 Subject: [PATCH 13/13] fix(resolver_test): use new WithTCPRetry option --- resolver_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/resolver_test.go b/resolver_test.go index 9f5611e..e604d5a 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -13,7 +13,7 @@ import ( ) func CheckTXT(t *testing.T, domain string) { - r := NewResolver() + r := NewResolver(WithTCPRetry()) rrs, err := r.ResolveErr(domain, "TXT") st.Expect(t, err, nil) @@ -214,7 +214,6 @@ func TestGoogleTXTTCPRetry(t *testing.T) { st.Expect(t, len(rrs2) > len(rrs), true) } - func TestAppleA(t *testing.T) { r := NewResolver() rrs, err := r.ResolveErr("apple.com", "A")