diff --git a/internal/cmd/apitool/main_test.go b/internal/cmd/apitool/main_test.go index 4d9a29960b..b9718be4c4 100644 --- a/internal/cmd/apitool/main_test.go +++ b/internal/cmd/apitool/main_test.go @@ -11,6 +11,7 @@ func init() { } func TestCheck(t *testing.T) { + t.Skip("see https://github.com/ooni/probe/issues/2098") *mode = "check" main() } diff --git a/internal/cmd/jafar/uncensored/uncensored.go b/internal/cmd/jafar/uncensored/uncensored.go index 6dab98e5f8..21bfb4fdc1 100644 --- a/internal/cmd/jafar/uncensored/uncensored.go +++ b/internal/cmd/jafar/uncensored/uncensored.go @@ -67,6 +67,11 @@ func (c *Client) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSv return nil, errors.New("not implemented") } +// LookupNS implements model.Resolver.LookupNS. +func (c *Client) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("not implemented") +} + // Network implements Resolver.Network func (c *Client) Network() string { return c.dnsClient.Network() diff --git a/internal/cmd/oohelper/internal/fake_test.go b/internal/cmd/oohelper/internal/fake_test.go index 12e536ce26..cbcbda2e37 100644 --- a/internal/cmd/oohelper/internal/fake_test.go +++ b/internal/cmd/oohelper/internal/fake_test.go @@ -56,6 +56,10 @@ func (c FakeResolver) LookupHTTPS(ctx context.Context, domain string) (*model.HT return nil, errors.New("not implemented") } +func (c FakeResolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("not implemented") +} + var _ model.Resolver = FakeResolver{} type FakeTransport struct { diff --git a/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go b/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go index 8df8aa41cb..e43bba05f6 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go @@ -56,6 +56,10 @@ func (c FakeResolver) LookupHTTPS(ctx context.Context, domain string) (*model.HT return nil, errors.New("not implemented") } +func (c FakeResolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("not implemented") +} + var _ model.Resolver = FakeResolver{} type FakeTransport struct { diff --git a/internal/engine/internal/sessionresolver/sessionresolver.go b/internal/engine/internal/sessionresolver/sessionresolver.go index ab5c17ad95..254a7a0ba5 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver.go +++ b/internal/engine/internal/sessionresolver/sessionresolver.go @@ -29,6 +29,7 @@ import ( "errors" "fmt" "math/rand" + "net" "net/url" "sync" "time" @@ -110,9 +111,16 @@ func (r *Resolver) Stats() string { return fmt.Sprintf("sessionresolver: %s", string(data)) } +var errNotImplemented = errors.New("not implemented") + // LookupHTTPS implements Resolver.LookupHTTPS. func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { - return nil, errors.New("not implemented") + return nil, errNotImplemented +} + +// LookupNS implements Resolver.LookupNS. +func (r *Resolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errNotImplemented } // ErrLookupHost indicates that LookupHost failed. diff --git a/internal/engine/internal/sessionresolver/sessionresolver_test.go b/internal/engine/internal/sessionresolver/sessionresolver_test.go index f62dd2f817..51f75bc02b 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver_test.go +++ b/internal/engine/internal/sessionresolver/sessionresolver_test.go @@ -343,3 +343,27 @@ func TestShouldSkipWithProxyWorks(t *testing.T) { } } } + +func TestUnimplementedFunctions(t *testing.T) { + t.Run("LookupHTTPS", func(t *testing.T) { + r := &Resolver{} + https, err := r.LookupHTTPS(context.Background(), "dns.google") + if !errors.Is(err, errNotImplemented) { + t.Fatal("unexpected error", err) + } + if https != nil { + t.Fatal("expected nil result") + } + }) + + t.Run("LookupNS", func(t *testing.T) { + r := &Resolver{} + ns, err := r.LookupNS(context.Background(), "dns.google") + if !errors.Is(err, errNotImplemented) { + t.Fatal("unexpected error", err) + } + if len(ns) > 0 { + t.Fatal("expected empty result") + } + }) +} diff --git a/internal/engine/netx/resolver/cache.go b/internal/engine/netx/resolver/cache.go index 34f5e021e5..8c4ece7aa1 100644 --- a/internal/engine/netx/resolver/cache.go +++ b/internal/engine/netx/resolver/cache.go @@ -25,7 +25,7 @@ func (r *CacheResolver) LookupHost( if err != nil { return nil, err } - if r.ReadOnly == false { + if !r.ReadOnly { r.Set(hostname, entry) } return entry, nil diff --git a/internal/engine/netx/resolver/cache_test.go b/internal/engine/netx/resolver/cache_test.go index d1d549f129..f768c0f5a7 100644 --- a/internal/engine/netx/resolver/cache_test.go +++ b/internal/engine/netx/resolver/cache_test.go @@ -6,14 +6,11 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" - "github.com/ooni/probe-cli/v3/internal/model" ) func TestCacheFailure(t *testing.T) { expected := errors.New("mocked error") - var r model.Resolver = resolver.FakeResolver{ - Err: expected, - } + r := resolver.NewFakeResolverWithExplicitError(expected) cache := &resolver.CacheResolver{Resolver: r} addrs, err := cache.LookupHost(context.Background(), "www.google.com") if !errors.Is(err, expected) { @@ -28,9 +25,8 @@ func TestCacheFailure(t *testing.T) { } func TestCacheHitSuccess(t *testing.T) { - var r model.Resolver = resolver.FakeResolver{ - Err: errors.New("mocked error"), - } + expected := errors.New("mocked error") + r := resolver.NewFakeResolverWithExplicitError(expected) cache := &resolver.CacheResolver{Resolver: r} cache.Set("dns.google.com", []string{"8.8.8.8"}) addrs, err := cache.LookupHost(context.Background(), "dns.google.com") @@ -43,9 +39,7 @@ func TestCacheHitSuccess(t *testing.T) { } func TestCacheMissSuccess(t *testing.T) { - var r model.Resolver = resolver.FakeResolver{ - Result: []string{"8.8.8.8"}, - } + r := resolver.NewFakeResolverWithResult([]string{"8.8.8.8"}) cache := &resolver.CacheResolver{Resolver: r} addrs, err := cache.LookupHost(context.Background(), "dns.google.com") if err != nil { @@ -60,9 +54,7 @@ func TestCacheMissSuccess(t *testing.T) { } func TestCacheReadonlySuccess(t *testing.T) { - var r model.Resolver = resolver.FakeResolver{ - Result: []string{"8.8.8.8"}, - } + r := resolver.NewFakeResolverWithResult([]string{"8.8.8.8"}) cache := &resolver.CacheResolver{Resolver: r, ReadOnly: true} addrs, err := cache.LookupHost(context.Background(), "dns.google.com") if err != nil { diff --git a/internal/engine/netx/resolver/fake_test.go b/internal/engine/netx/resolver/fake_test.go index 154ddf8d8a..ebdbf13dc1 100644 --- a/internal/engine/netx/resolver/fake_test.go +++ b/internal/engine/netx/resolver/fake_test.go @@ -7,8 +7,10 @@ import ( "net" "time" - "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/model/mocks" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) type FakeDialer struct { @@ -108,48 +110,53 @@ func (fe FakeEncoder) Encode(domain string, qtype uint16, padding bool) ([]byte, return fe.Data, fe.Err } -type FakeResolver struct { - NumFailures *atomicx.Int64 - Err error - Result []string -} - -func NewFakeResolverThatFails() FakeResolver { - return FakeResolver{NumFailures: &atomicx.Int64{}, Err: errNotFound} -} - -func NewFakeResolverWithResult(r []string) FakeResolver { - return FakeResolver{NumFailures: &atomicx.Int64{}, Result: r} -} - -var errNotFound = &net.DNSError{ - Err: "no such host", -} - -func (c FakeResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - time.Sleep(10 * time.Microsecond) - if c.Err != nil { - if c.NumFailures != nil { - c.NumFailures.Add(1) - } - return nil, c.Err +func NewFakeResolverThatFails() model.Resolver { + return NewFakeResolverWithExplicitError(netxlite.ErrOODNSNoSuchHost) +} + +func NewFakeResolverWithExplicitError(err error) model.Resolver { + runtimex.PanicIfNil(err, "passed nil error") + return &mocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return nil, err + }, + MockNetwork: func() string { + return "fake" + }, + MockAddress: func() string { + return "" + }, + MockCloseIdleConnections: func() { + // nothing + }, + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") + }, + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("not implemented") + }, } - return c.Result, nil } -func (c FakeResolver) Network() string { - return "fake" -} - -func (c FakeResolver) Address() string { - return "" -} - -func (c FakeResolver) CloseIdleConnections() {} - -func (c FakeResolver) LookupHTTPS( - ctx context.Context, domain string) (*model.HTTPSSvc, error) { - return nil, errors.New("not implemented") +func NewFakeResolverWithResult(r []string) model.Resolver { + return &mocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return r, nil + }, + MockNetwork: func() string { + return "fake" + }, + MockAddress: func() string { + return "" + }, + MockCloseIdleConnections: func() { + // nothing + }, + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") + }, + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("not implemented") + }, + } } - -var _ model.Resolver = FakeResolver{} diff --git a/internal/engine/netx/resolver/saver_test.go b/internal/engine/netx/resolver/saver_test.go index ecd1764852..d9ad7c9e80 100644 --- a/internal/engine/netx/resolver/saver_test.go +++ b/internal/engine/netx/resolver/saver_test.go @@ -16,10 +16,8 @@ func TestSaverResolverFailure(t *testing.T) { expected := errors.New("no such host") saver := &trace.Saver{} reso := resolver.SaverResolver{ - Resolver: resolver.FakeResolver{ - Err: expected, - }, - Saver: saver, + Resolver: resolver.NewFakeResolverWithExplicitError(expected), + Saver: saver, } addrs, err := reso.LookupHost(context.Background(), "www.google.com") if !errors.Is(err, expected) { @@ -65,10 +63,8 @@ func TestSaverResolverSuccess(t *testing.T) { expected := []string{"8.8.8.8", "8.8.4.4"} saver := &trace.Saver{} reso := resolver.SaverResolver{ - Resolver: resolver.FakeResolver{ - Result: expected, - }, - Saver: saver, + Resolver: resolver.NewFakeResolverWithResult(expected), + Saver: saver, } addrs, err := reso.LookupHost(context.Background(), "www.google.com") if err != nil { diff --git a/internal/engine/probeservices/checkreportid_test.go b/internal/engine/probeservices/checkreportid_test.go index d43e3d2b5e..662423bbcd 100644 --- a/internal/engine/probeservices/checkreportid_test.go +++ b/internal/engine/probeservices/checkreportid_test.go @@ -14,6 +14,7 @@ import ( ) func TestCheckReportIDWorkingAsIntended(t *testing.T) { + t.Skip("see https://github.com/ooni/probe/issues/2098") client := probeservices.Client{ APIClientTemplate: httpx.APIClientTemplate{ BaseURL: "https://ams-pg.ooni.org/", diff --git a/internal/model/mocks/dnsdecoder.go b/internal/model/mocks/dnsdecoder.go index 6db913702d..66d3f0efa2 100644 --- a/internal/model/mocks/dnsdecoder.go +++ b/internal/model/mocks/dnsdecoder.go @@ -1,6 +1,8 @@ package mocks import ( + "net" + "github.com/miekg/dns" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -8,10 +10,9 @@ import ( // DNSDecoder allows mocking dnsx.DNSDecoder. type DNSDecoder struct { MockDecodeLookupHost func(qtype uint16, reply []byte, queryID uint16) ([]string, error) - - MockDecodeHTTPS func(reply []byte, queryID uint16) (*model.HTTPSSvc, error) - - MockDecodeReply func(reply []byte) (*dns.Msg, error) + MockDecodeHTTPS func(reply []byte, queryID uint16) (*model.HTTPSSvc, error) + MockDecodeNS func(reply []byte, queryID uint16) ([]*net.NS, error) + MockDecodeReply func(reply []byte) (*dns.Msg, error) } // DecodeLookupHost calls MockDecodeLookupHost. @@ -24,6 +25,11 @@ func (e *DNSDecoder) DecodeHTTPS(reply []byte, queryID uint16) (*model.HTTPSSvc, return e.MockDecodeHTTPS(reply, queryID) } +// DecodeNS calls MockDecodeNS. +func (e *DNSDecoder) DecodeNS(reply []byte, queryID uint16) ([]*net.NS, error) { + return e.MockDecodeNS(reply, queryID) +} + // DecodeReply calls MockDecodeReply. func (e *DNSDecoder) DecodeReply(reply []byte) (*dns.Msg, error) { return e.MockDecodeReply(reply) diff --git a/internal/model/mocks/dnsdecoder_test.go b/internal/model/mocks/dnsdecoder_test.go index e51b1ccffb..1d52f97681 100644 --- a/internal/model/mocks/dnsdecoder_test.go +++ b/internal/model/mocks/dnsdecoder_test.go @@ -2,6 +2,7 @@ package mocks import ( "errors" + "net" "testing" "github.com/miekg/dns" @@ -41,6 +42,22 @@ func TestDNSDecoder(t *testing.T) { } }) + t.Run("DecodeNS", func(t *testing.T) { + expected := errors.New("mocked error") + e := &DNSDecoder{ + MockDecodeNS: func(reply []byte, queryID uint16) ([]*net.NS, error) { + return nil, expected + }, + } + out, err := e.DecodeNS(make([]byte, 17), dns.Id()) + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if out != nil { + t.Fatal("unexpected out") + } + }) + t.Run("DecodeReply", func(t *testing.T) { expected := errors.New("mocked error") e := &DNSDecoder{ diff --git a/internal/model/mocks/resolver.go b/internal/model/mocks/resolver.go index 909e197f6e..d9879fdd90 100644 --- a/internal/model/mocks/resolver.go +++ b/internal/model/mocks/resolver.go @@ -2,6 +2,7 @@ package mocks import ( "context" + "net" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -13,6 +14,7 @@ type Resolver struct { MockAddress func() string MockCloseIdleConnections func() MockLookupHTTPS func(ctx context.Context, domain string) (*model.HTTPSSvc, error) + MockLookupNS func(ctx context.Context, domain string) ([]*net.NS, error) } // LookupHost calls MockLookupHost. @@ -39,3 +41,8 @@ func (r *Resolver) CloseIdleConnections() { func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { return r.MockLookupHTTPS(ctx, domain) } + +// LookupNS calls MockLookupNS. +func (r *Resolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { + return r.MockLookupNS(ctx, domain) +} diff --git a/internal/model/mocks/resolver_test.go b/internal/model/mocks/resolver_test.go index 4b6b4b8046..199f924245 100644 --- a/internal/model/mocks/resolver_test.go +++ b/internal/model/mocks/resolver_test.go @@ -3,6 +3,7 @@ package mocks import ( "context" "errors" + "net" "testing" "github.com/ooni/probe-cli/v3/internal/model" @@ -77,4 +78,21 @@ func TestResolver(t *testing.T) { t.Fatal("expected nil addr") } }) + + t.Run("LookupNS", func(t *testing.T) { + expected := errors.New("mocked error") + r := &Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, expected + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "dns.google") + if !errors.Is(err, expected) { + t.Fatal("unexpected error", err) + } + if ns != nil { + t.Fatal("expected nil addr") + } + }) } diff --git a/internal/model/netx.go b/internal/model/netx.go index 16b8df397b..93c690719c 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -51,6 +51,9 @@ type DNSDecoder interface { // an error, though, when there are no IPv4/IPv6 hints in the reply. DecodeHTTPS(data []byte, queryID uint16) (*HTTPSSvc, error) + // DecodeNS is like DecodeHTTPS but for NS queries. + DecodeNS(data []byte, queryID uint16) ([]*net.NS, error) + // DecodeReply decodes a DNS reply message. // // Arguments: @@ -194,6 +197,9 @@ type Resolver interface { // LookupHTTPS issues an HTTPS query for a domain. LookupHTTPS( ctx context.Context, domain string) (*HTTPSSvc, error) + + // LookupNS issues a NS query for a domain. + LookupNS(ctx context.Context, domain string) ([]*net.NS, error) } // TLSDialer is a Dialer dialing TLS connections. diff --git a/internal/netxlite/bogon.go b/internal/netxlite/bogon.go index cbac02768e..7d954371bd 100644 --- a/internal/netxlite/bogon.go +++ b/internal/netxlite/bogon.go @@ -6,8 +6,6 @@ package netxlite // This file helps us to decide if an IPAddr is a bogon. // -// TODO(bassosimone): code in engine/netx should use this file. - import ( "net" diff --git a/internal/netxlite/dnsdecoder.go b/internal/netxlite/dnsdecoder.go index a5571013db..b37fd9f557 100644 --- a/internal/netxlite/dnsdecoder.go +++ b/internal/netxlite/dnsdecoder.go @@ -6,6 +6,7 @@ package netxlite import ( "errors" + "net" "github.com/miekg/dns" "github.com/ooni/probe-cli/v3/internal/model" @@ -111,4 +112,22 @@ func (d *DNSDecoderMiekg) DecodeLookupHost(qtype uint16, data []byte, queryID ui return addrs, nil } +func (d *DNSDecoderMiekg) DecodeNS(data []byte, queryID uint16) ([]*net.NS, error) { + reply, err := d.parseReply(data, queryID) + if err != nil { + return nil, err + } + out := []*net.NS{} + for _, answer := range reply.Answer { + switch avalue := answer.(type) { + case *dns.NS: + out = append(out, &net.NS{Host: avalue.Ns}) + } + } + if len(out) < 1 { + return nil, ErrOODNSNoAnswer + } + return out, nil +} + var _ model.DNSDecoder = &DNSDecoderMiekg{} diff --git a/internal/netxlite/dnsdecoder_test.go b/internal/netxlite/dnsdecoder_test.go index 896701d7cb..ed716f54e4 100644 --- a/internal/netxlite/dnsdecoder_test.go +++ b/internal/netxlite/dnsdecoder_test.go @@ -192,8 +192,8 @@ func TestDNSDecoder(t *testing.T) { queryID = 17 unrelatedID = 14 ) - reply := dnsGenHTTPSReplySuccess(dnsGenQuery(dns.TypeA, queryID), nil, nil, nil) - data, err := d.DecodeLookupHost(dns.TypeA, reply, unrelatedID) + reply := dnsGenHTTPSReplySuccess(dnsGenQuery(dns.TypeHTTPS, queryID), nil, nil, nil) + data, err := d.DecodeHTTPS(reply, unrelatedID) if !errors.Is(err, ErrDNSReplyWithWrongQueryID) { t.Fatal("unexpected error", err) } @@ -239,6 +239,64 @@ func TestDNSDecoder(t *testing.T) { } }) }) + + t.Run("DecodeNS", func(t *testing.T) { + t.Run("with nil data", func(t *testing.T) { + d := &DNSDecoderMiekg{} + reply, err := d.DecodeNS(nil, 0) + if err == nil || err.Error() != "dns: overflow unpacking uint16" { + t.Fatal("not the error we expected", err) + } + if reply != nil { + t.Fatal("expected nil reply") + } + }) + + t.Run("wrong query ID", func(t *testing.T) { + d := &DNSDecoderMiekg{} + const ( + queryID = 17 + unrelatedID = 14 + ) + reply := dnsGenNSReplySuccess(dnsGenQuery(dns.TypeNS, queryID)) + data, err := d.DecodeNS(reply, unrelatedID) + if !errors.Is(err, ErrDNSReplyWithWrongQueryID) { + t.Fatal("unexpected error", err) + } + if data != nil { + t.Fatal("expected nil data here") + } + }) + + t.Run("with empty answer", func(t *testing.T) { + queryID := dns.Id() + data := dnsGenNSReplySuccess(dnsGenQuery(dns.TypeNS, queryID)) + d := &DNSDecoderMiekg{} + reply, err := d.DecodeNS(data, queryID) + if !errors.Is(err, ErrOODNSNoAnswer) { + t.Fatal("unexpected err", err) + } + if reply != nil { + t.Fatal("expected nil reply") + } + }) + + t.Run("with full answer", func(t *testing.T) { + queryID := dns.Id() + data := dnsGenNSReplySuccess(dnsGenQuery(dns.TypeNS, queryID), "ns1.zdns.google.") + d := &DNSDecoderMiekg{} + reply, err := d.DecodeNS(data, queryID) + if err != nil { + t.Fatal(err) + } + if len(reply) != 1 { + t.Fatal("unexpected reply length") + } + if reply[0].Host != "ns1.zdns.google." { + t.Fatal("unexpected reply host") + } + }) + }) } // dnsGenQuery generates a query suitable to be used with testing. @@ -281,6 +339,10 @@ func dnsGenLookupHostReplySuccess(rawQuery []byte, ips ...string) []byte { runtimex.PanicOnError(err, "query.Unpack failed") runtimex.PanicIfFalse(len(query.Question) == 1, "more than one question") question := query.Question[0] + runtimex.PanicIfFalse( + question.Qtype == dns.TypeA || question.Qtype == dns.TypeAAAA, + "invalid query type (expected A or AAAA)", + ) reply := new(dns.Msg) reply.Compress = true reply.MsgHdr.RecursionAvailable = true @@ -326,6 +388,9 @@ func dnsGenHTTPSReplySuccess(rawQuery []byte, alpns, ipv4s, ipv6s []string) []by query := new(dns.Msg) err := query.Unpack(rawQuery) runtimex.PanicOnError(err, "query.Unpack failed") + runtimex.PanicIfFalse(len(query.Question) == 1, "expected just a single question") + question := query.Question[0] + runtimex.PanicIfFalse(question.Qtype == dns.TypeHTTPS, "expected HTTPS query") reply := new(dns.Msg) reply.Compress = true reply.MsgHdr.RecursionAvailable = true @@ -364,3 +429,31 @@ func dnsGenHTTPSReplySuccess(rawQuery []byte, alpns, ipv4s, ipv6s []string) []by runtimex.PanicOnError(err, "reply.Pack failed") return data } + +// dnsGenNSReplySuccess generates a successful NS reply using the given names. +func dnsGenNSReplySuccess(rawQuery []byte, names ...string) []byte { + query := new(dns.Msg) + err := query.Unpack(rawQuery) + runtimex.PanicOnError(err, "query.Unpack failed") + runtimex.PanicIfFalse(len(query.Question) == 1, "more than one question") + question := query.Question[0] + runtimex.PanicIfFalse(question.Qtype == dns.TypeNS, "expected NS query") + reply := new(dns.Msg) + reply.Compress = true + reply.MsgHdr.RecursionAvailable = true + reply.SetReply(query) + for _, name := range names { + reply.Answer = append(reply.Answer, &dns.NS{ + Hdr: dns.RR_Header{ + Name: dns.Fqdn("x.org"), + Rrtype: question.Qtype, + Class: dns.ClassINET, + Ttl: 0, + }, + Ns: name, + }) + } + data, err := reply.Pack() + runtimex.PanicOnError(err, "reply.Pack failed") + return data +} diff --git a/internal/netxlite/parallelresolver.go b/internal/netxlite/parallelresolver.go index 4b80eb8994..a5dfed7f6a 100644 --- a/internal/netxlite/parallelresolver.go +++ b/internal/netxlite/parallelresolver.go @@ -6,6 +6,7 @@ package netxlite import ( "context" + "net" "github.com/miekg/dns" "github.com/ooni/probe-cli/v3/internal/atomicx" @@ -129,3 +130,18 @@ func (r *ParallelResolver) lookupHost(ctx context.Context, hostname string, err: err, } } + +// LookupNS implements Resolver.LookupNS. +func (r *ParallelResolver) LookupNS( + ctx context.Context, hostname string) ([]*net.NS, error) { + querydata, queryID, err := r.Encoder.Encode( + hostname, dns.TypeNS, r.Txp.RequiresPadding()) + if err != nil { + return nil, err + } + replydata, err := r.Txp.RoundTrip(ctx, querydata) + if err != nil { + return nil, err + } + return r.Decoder.DecodeNS(replydata, queryID) +} diff --git a/internal/netxlite/parallelresolver_test.go b/internal/netxlite/parallelresolver_test.go index 349a500723..e334daea92 100644 --- a/internal/netxlite/parallelresolver_test.go +++ b/internal/netxlite/parallelresolver_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "errors" + "net" "testing" "github.com/miekg/dns" @@ -266,4 +267,94 @@ func TestParallelResolver(t *testing.T) { } }) }) + + t.Run("LookupNS", func(t *testing.T) { + t.Run("for encoding error", func(t *testing.T) { + expected := errors.New("mocked error") + r := &ParallelResolver{ + Encoder: &mocks.DNSEncoder{ + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, expected + }, + }, + Decoder: nil, + NumTimeouts: &atomicx.Int64{}, + Txp: &mocks.DNSTransport{ + MockRequiresPadding: func() bool { + return false + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "example.com") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if ns != nil { + t.Fatal("unexpected result") + } + }) + + t.Run("for round-trip error", func(t *testing.T) { + expected := errors.New("mocked error") + r := &ParallelResolver{ + Encoder: &mocks.DNSEncoder{ + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil + }, + }, + Decoder: nil, + NumTimeouts: &atomicx.Int64{}, + Txp: &mocks.DNSTransport{ + MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { + return nil, expected + }, + MockRequiresPadding: func() bool { + return false + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "example.com") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if ns != nil { + t.Fatal("unexpected result") + } + }) + + t.Run("for decode error", func(t *testing.T) { + expected := errors.New("mocked error") + r := &ParallelResolver{ + Encoder: &mocks.DNSEncoder{ + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil + }, + }, + Decoder: &mocks.DNSDecoder{ + MockDecodeNS: func(reply []byte, queryID uint16) ([]*net.NS, error) { + return nil, expected + }, + }, + NumTimeouts: &atomicx.Int64{}, + Txp: &mocks.DNSTransport{ + MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { + return make([]byte, 128), nil + }, + MockRequiresPadding: func() bool { + return false + }, + }, + } + ctx := context.Background() + https, err := r.LookupNS(ctx, "example.com") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if https != nil { + t.Fatal("unexpected result") + } + }) + }) } diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index 9a201b5ad2..dbb287ab0d 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -136,6 +136,16 @@ func (r *resolverSystem) LookupHTTPS( return nil, ErrNoDNSTransport } +func (r *resolverSystem) LookupNS( + ctx context.Context, domain string) ([]*net.NS, error) { + // TODO(bassosimone): figure out in which context it makes sense + // to issue this query. How is this implemented under the hood by + // the stdlib? Is it using /etc/resolve.conf on Unix? Until we + // known all these details, let's pretend this functionality does + // not exist in the stdlib and focus on custom resolvers. + return nil, ErrNoDNSTransport +} + // resolverLogger is a resolver that emits events type resolverLogger struct { Resolver model.Resolver @@ -188,6 +198,21 @@ func (r *resolverLogger) CloseIdleConnections() { r.Resolver.CloseIdleConnections() } +func (r *resolverLogger) LookupNS( + ctx context.Context, domain string) ([]*net.NS, error) { + prefix := fmt.Sprintf("resolve[NS] %s with %s (%s)", domain, r.Network(), r.Address()) + r.Logger.Debugf("%s...", prefix) + start := time.Now() + ns, err := r.Resolver.LookupNS(ctx, domain) + elapsed := time.Since(start) + if err != nil { + r.Logger.Debugf("%s... %s in %s", prefix, err, elapsed) + return nil, err + } + r.Logger.Debugf("%s... %+v in %s", prefix, ns, elapsed) + return ns, nil +} + // resolverIDNA supports resolving Internationalized Domain Names. // // See RFC3492 for more information. @@ -226,6 +251,15 @@ func (r *resolverIDNA) CloseIdleConnections() { r.Resolver.CloseIdleConnections() } +func (r *resolverIDNA) LookupNS( + ctx context.Context, domain string) ([]*net.NS, error) { + host, err := idna.ToASCII(domain) + if err != nil { + return nil, err + } + return r.Resolver.LookupNS(ctx, host) +} + // resolverShortCircuitIPAddr recognizes when the input hostname is an // IP address and returns it immediately to the caller. type resolverShortCircuitIPAddr struct { @@ -266,6 +300,18 @@ func (r *resolverShortCircuitIPAddr) CloseIdleConnections() { r.Resolver.CloseIdleConnections() } +// ErrDNSIPAddress indicates that you passed an IP address to a DNS +// function that only works with domain names. +var ErrDNSIPAddress = errors.New("ooresolver: expected domain, found IP address") + +func (r *resolverShortCircuitIPAddr) LookupNS( + ctx context.Context, hostname string) ([]*net.NS, error) { + if net.ParseIP(hostname) != nil { + return nil, ErrDNSIPAddress + } + return r.Resolver.LookupNS(ctx, hostname) +} + // IsIPv6 returns true if the given candidate is a valid IP address // representation and such representation is IPv6. func IsIPv6(candidate string) (bool, error) { @@ -313,6 +359,11 @@ func (r *nullResolver) LookupHTTPS( return nil, ErrNoResolver } +func (r *nullResolver) LookupNS( + ctx context.Context, domain string) ([]*net.NS, error) { + return nil, ErrNoResolver +} + // resolverErrWrapper is a Resolver that knows about wrapping errors. type resolverErrWrapper struct { Resolver model.Resolver @@ -348,3 +399,12 @@ func (r *resolverErrWrapper) Address() string { func (r *resolverErrWrapper) CloseIdleConnections() { r.Resolver.CloseIdleConnections() } + +func (r *resolverErrWrapper) LookupNS( + ctx context.Context, domain string) ([]*net.NS, error) { + out, err := r.Resolver.LookupNS(ctx, domain) + if err != nil { + return nil, newErrWrapper(classifyResolverError, ResolveOperation, err) + } + return out, nil +} diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go index 6405aae60e..8a1dc8cd23 100644 --- a/internal/netxlite/resolver_test.go +++ b/internal/netxlite/resolver_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "io" + "net" "strings" "sync" "testing" @@ -166,6 +167,17 @@ func TestResolverSystem(t *testing.T) { t.Fatal("expected nil result") } }) + + t.Run("LookupNS", func(t *testing.T) { + r := &resolverSystem{} + ns, err := r.LookupNS(context.Background(), "x.org") + if !errors.Is(err, ErrNoDNSTransport) { + t.Fatal("not the error we expected") + } + if ns != nil { + t.Fatal("expected nil result") + } + }) } func TestResolverLogger(t *testing.T) { @@ -312,6 +324,94 @@ func TestResolverLogger(t *testing.T) { }) }) + t.Run("CloseIdleConnections", func(t *testing.T) { + var called bool + child := &mocks.Resolver{ + MockCloseIdleConnections: func() { + called = true + }, + } + reso := &resolverLogger{ + Resolver: child, + Logger: model.DiscardLogger, + } + reso.CloseIdleConnections() + if !called { + t.Fatal("not called") + } + }) + + t.Run("LookupNS", func(t *testing.T) { + t.Run("with success", func(t *testing.T) { + var count int + lo := &mocks.Logger{ + MockDebugf: func(format string, v ...interface{}) { + count++ + }, + } + expected := []*net.NS{{ + Host: "ns1.zdns.google.", + }} + r := &resolverLogger{ + Logger: lo, + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return expected, nil + }, + MockNetwork: func() string { + return "system" + }, + MockAddress: func() string { + return "" + }, + }, + } + ns, err := r.LookupNS(context.Background(), "dns.google") + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expected, ns); diff != "" { + t.Fatal(diff) + } + if count != 2 { + t.Fatal("unexpected count") + } + }) + + t.Run("with failure", func(t *testing.T) { + var count int + lo := &mocks.Logger{ + MockDebugf: func(format string, v ...interface{}) { + count++ + }, + } + expected := errors.New("mocked error") + r := &resolverLogger{ + Logger: lo, + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, expected + }, + MockNetwork: func() string { + return "system" + }, + MockAddress: func() string { + return "" + }, + }, + } + ns, err := r.LookupNS(context.Background(), "dns.google") + if !errors.Is(err, expected) { + t.Fatal("not the error we expected", err) + } + if ns != nil { + t.Fatal("expected nil addr here") + } + if count != 2 { + t.Fatal("unexpected count") + } + }) + }) } func TestResolverIDNA(t *testing.T) { @@ -424,6 +524,63 @@ func TestResolverIDNA(t *testing.T) { t.Fatal("invalid address") } }) + + t.Run("CloseIdleConnections", func(t *testing.T) { + var called bool + child := &mocks.Resolver{ + MockCloseIdleConnections: func() { + called = true + }, + } + reso := &resolverIDNA{child} + reso.CloseIdleConnections() + if !called { + t.Fatal("not called") + } + }) + + t.Run("LookupNS", func(t *testing.T) { + t.Run("with valid IDNA in input", func(t *testing.T) { + expected := []*net.NS{{ + Host: "ns1.zdns.google.", + }} + r := &resolverIDNA{ + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + if domain != "xn--d1acpjx3f.xn--p1ai" { + return nil, errors.New("passed invalid domain") + } + return expected, nil + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "яндекс.рф") + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expected, ns); diff != "" { + t.Fatal(diff) + } + }) + + t.Run("with invalid punycode", func(t *testing.T) { + r := &resolverIDNA{Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("should not happen") + }, + }} + // See https://www.farsightsecurity.com/blog/txt-record/punycode-20180711/ + ctx := context.Background() + ns, err := r.LookupNS(ctx, "xn--0000h") + if err == nil || !strings.HasPrefix(err.Error(), "idna: invalid label") { + t.Fatal("not the error we expected") + } + if ns != nil { + t.Fatal("expected no response here") + } + }) + }) } func TestResolverShortCircuitIPAddr(t *testing.T) { @@ -520,6 +677,100 @@ func TestResolverShortCircuitIPAddr(t *testing.T) { } }) }) + + t.Run("LookupNS", func(t *testing.T) { + t.Run("with IPv4 addr", func(t *testing.T) { + r := &resolverShortCircuitIPAddr{ + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("mocked error") + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "8.8.8.8") + if !errors.Is(err, ErrDNSIPAddress) { + t.Fatal("unexpected error", err) + } + if len(ns) > 0 { + t.Fatal("invalid result") + } + }) + + t.Run("with IPv6 addr", func(t *testing.T) { + r := &resolverShortCircuitIPAddr{ + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("mocked error") + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "::1") + if !errors.Is(err, ErrDNSIPAddress) { + t.Fatal("unexpected error", err) + } + if len(ns) > 0 { + t.Fatal("invalid result") + } + }) + + t.Run("with domain", func(t *testing.T) { + r := &resolverShortCircuitIPAddr{ + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, errors.New("mocked error") + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "dns.google") + if err == nil || err.Error() != "mocked error" { + t.Fatal("not the error we expected", err) + } + if len(ns) > 0 { + t.Fatal("invalid result") + } + }) + }) + + t.Run("Network", func(t *testing.T) { + child := &mocks.Resolver{ + MockNetwork: func() string { + return "x" + }, + } + reso := &resolverShortCircuitIPAddr{child} + if reso.Network() != "x" { + t.Fatal("invalid result") + } + }) + + t.Run("Address", func(t *testing.T) { + child := &mocks.Resolver{ + MockAddress: func() string { + return "x" + }, + } + reso := &resolverShortCircuitIPAddr{child} + if reso.Address() != "x" { + t.Fatal("invalid result") + } + }) + + t.Run("CloseIdleConnections", func(t *testing.T) { + var called bool + child := &mocks.Resolver{ + MockCloseIdleConnections: func() { + called = true + }, + } + reso := &resolverShortCircuitIPAddr{child} + reso.CloseIdleConnections() + if !called { + t.Fatal("not called") + } + }) } func TestIsIPv6(t *testing.T) { @@ -592,6 +843,18 @@ func TestNullResolver(t *testing.T) { } r.CloseIdleConnections() // for coverage }) + + t.Run("LookupNS", func(t *testing.T) { + r := &nullResolver{} + ctx := context.Background() + ns, err := r.LookupNS(ctx, "dns.google") + if !errors.Is(err, ErrNoResolver) { + t.Fatal("unexpected error", err) + } + if len(ns) > 0 { + t.Fatal("unexpected result") + } + }) } func TestResolverErrWrapper(t *testing.T) { @@ -719,4 +982,46 @@ func TestResolverErrWrapper(t *testing.T) { } }) }) + + t.Run("LookupNS", func(t *testing.T) { + t.Run("on success", func(t *testing.T) { + expected := []*net.NS{{ + Host: "antani.local.", + }} + reso := &resolverErrWrapper{ + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return expected, nil + }, + }, + } + ctx := context.Background() + ns, err := reso.LookupNS(ctx, "antani.local") + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expected, ns); diff != "" { + t.Fatal(diff) + } + }) + + t.Run("on failure", func(t *testing.T) { + expected := io.EOF + reso := &resolverErrWrapper{ + Resolver: &mocks.Resolver{ + MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) { + return nil, expected + }, + }, + } + ctx := context.Background() + ns, err := reso.LookupNS(ctx, "") + if err == nil || err.Error() != FailureEOFError { + t.Fatal("unexpected err", err) + } + if len(ns) > 0 { + t.Fatal("unexpected addrs") + } + }) + }) } diff --git a/internal/netxlite/serialresolver.go b/internal/netxlite/serialresolver.go index be1f4bb3f2..b0805abd78 100644 --- a/internal/netxlite/serialresolver.go +++ b/internal/netxlite/serialresolver.go @@ -23,8 +23,8 @@ import ( // Deprecated: please use ParallelResolver in new code. We cannot // remove this code as long as we use tracing for measuring. // -// QUIRK: unlike the ParallelResolver, this resolver retries each -// query three times for soft errors. +// QUIRK: unlike the ParallelResolver, this resolver's LookupHost retries +// each query three times for soft errors. type SerialResolver struct { // Encoder is the MANDATORY encoder to use. Encoder model.DNSEncoder @@ -142,3 +142,18 @@ func (r *SerialResolver) lookupHostWithoutRetry( } return r.Decoder.DecodeLookupHost(qtype, replydata, queryID) } + +// LookupNS implements Resolver.LookupNS. +func (r *SerialResolver) LookupNS( + ctx context.Context, hostname string) ([]*net.NS, error) { + querydata, queryID, err := r.Encoder.Encode( + hostname, dns.TypeNS, r.Txp.RequiresPadding()) + if err != nil { + return nil, err + } + replydata, err := r.Txp.RoundTrip(ctx, querydata) + if err != nil { + return nil, err + } + return r.Decoder.DecodeNS(replydata, queryID) +} diff --git a/internal/netxlite/serialresolver_test.go b/internal/netxlite/serialresolver_test.go index 8e3b6201bf..af5df96b94 100644 --- a/internal/netxlite/serialresolver_test.go +++ b/internal/netxlite/serialresolver_test.go @@ -272,4 +272,94 @@ func TestSerialResolver(t *testing.T) { } }) }) + + t.Run("LookupNS", func(t *testing.T) { + t.Run("for encoding error", func(t *testing.T) { + expected := errors.New("mocked error") + r := &SerialResolver{ + Encoder: &mocks.DNSEncoder{ + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, expected + }, + }, + Decoder: nil, + NumTimeouts: &atomicx.Int64{}, + Txp: &mocks.DNSTransport{ + MockRequiresPadding: func() bool { + return false + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "example.com") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if ns != nil { + t.Fatal("unexpected result") + } + }) + + t.Run("for round-trip error", func(t *testing.T) { + expected := errors.New("mocked error") + r := &SerialResolver{ + Encoder: &mocks.DNSEncoder{ + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil + }, + }, + Decoder: nil, + NumTimeouts: &atomicx.Int64{}, + Txp: &mocks.DNSTransport{ + MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { + return nil, expected + }, + MockRequiresPadding: func() bool { + return false + }, + }, + } + ctx := context.Background() + ns, err := r.LookupNS(ctx, "example.com") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if ns != nil { + t.Fatal("unexpected result") + } + }) + + t.Run("for decode error", func(t *testing.T) { + expected := errors.New("mocked error") + r := &SerialResolver{ + Encoder: &mocks.DNSEncoder{ + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil + }, + }, + Decoder: &mocks.DNSDecoder{ + MockDecodeNS: func(reply []byte, queryID uint16) ([]*net.NS, error) { + return nil, expected + }, + }, + NumTimeouts: &atomicx.Int64{}, + Txp: &mocks.DNSTransport{ + MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { + return make([]byte, 128), nil + }, + MockRequiresPadding: func() bool { + return false + }, + }, + } + ctx := context.Background() + https, err := r.LookupNS(ctx, "example.com") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if https != nil { + t.Fatal("unexpected result") + } + }) + }) } diff --git a/internal/ooapi/integration_test.go b/internal/ooapi/integration_test.go index 0fca2f7470..d539d81c61 100644 --- a/internal/ooapi/integration_test.go +++ b/internal/ooapi/integration_test.go @@ -46,6 +46,7 @@ func TestWithRealServerDoCheckIn(t *testing.T) { } func TestWithRealServerDoCheckReportID(t *testing.T) { + t.Skip("see https://github.com/ooni/probe/issues/2098") if testing.Short() { t.Skip("skip test in short mode") }