diff --git a/internal/model/mocks/dnsdecoder.go b/internal/model/mocks/dnsdecoder.go index fcff1c4..6db9137 100644 --- a/internal/model/mocks/dnsdecoder.go +++ b/internal/model/mocks/dnsdecoder.go @@ -1,20 +1,30 @@ package mocks -import "github.com/ooni/probe-cli/v3/internal/model" +import ( + "github.com/miekg/dns" + "github.com/ooni/probe-cli/v3/internal/model" +) // DNSDecoder allows mocking dnsx.DNSDecoder. type DNSDecoder struct { - MockDecodeLookupHost func(qtype uint16, reply []byte) ([]string, error) + MockDecodeLookupHost func(qtype uint16, reply []byte, queryID uint16) ([]string, error) - MockDecodeHTTPS func(reply []byte) (*model.HTTPSSvc, error) + MockDecodeHTTPS func(reply []byte, queryID uint16) (*model.HTTPSSvc, error) + + MockDecodeReply func(reply []byte) (*dns.Msg, error) } // DecodeLookupHost calls MockDecodeLookupHost. -func (e *DNSDecoder) DecodeLookupHost(qtype uint16, reply []byte) ([]string, error) { - return e.MockDecodeLookupHost(qtype, reply) +func (e *DNSDecoder) DecodeLookupHost(qtype uint16, reply []byte, queryID uint16) ([]string, error) { + return e.MockDecodeLookupHost(qtype, reply, queryID) } // DecodeHTTPS calls MockDecodeHTTPS. -func (e *DNSDecoder) DecodeHTTPS(reply []byte) (*model.HTTPSSvc, error) { - return e.MockDecodeHTTPS(reply) +func (e *DNSDecoder) DecodeHTTPS(reply []byte, queryID uint16) (*model.HTTPSSvc, error) { + return e.MockDecodeHTTPS(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 3ca6d21..e51b1cc 100644 --- a/internal/model/mocks/dnsdecoder_test.go +++ b/internal/model/mocks/dnsdecoder_test.go @@ -12,11 +12,11 @@ func TestDNSDecoder(t *testing.T) { t.Run("DecodeLookupHost", func(t *testing.T) { expected := errors.New("mocked error") e := &DNSDecoder{ - MockDecodeLookupHost: func(qtype uint16, reply []byte) ([]string, error) { + MockDecodeLookupHost: func(qtype uint16, reply []byte, queryID uint16) ([]string, error) { return nil, expected }, } - out, err := e.DecodeLookupHost(dns.TypeA, make([]byte, 17)) + out, err := e.DecodeLookupHost(dns.TypeA, make([]byte, 17), dns.Id()) if !errors.Is(err, expected) { t.Fatal("unexpected err", err) } @@ -28,11 +28,27 @@ func TestDNSDecoder(t *testing.T) { t.Run("DecodeHTTPS", func(t *testing.T) { expected := errors.New("mocked error") e := &DNSDecoder{ - MockDecodeHTTPS: func(reply []byte) (*model.HTTPSSvc, error) { + MockDecodeHTTPS: func(reply []byte, queryID uint16) (*model.HTTPSSvc, error) { return nil, expected }, } - out, err := e.DecodeHTTPS(make([]byte, 17)) + out, err := e.DecodeHTTPS(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{ + MockDecodeReply: func(reply []byte) (*dns.Msg, error) { + return nil, expected + }, + } + out, err := e.DecodeReply(make([]byte, 17)) if !errors.Is(err, expected) { t.Fatal("unexpected err", err) } diff --git a/internal/model/mocks/dnsencoder.go b/internal/model/mocks/dnsencoder.go index 0723f2f..d59ea19 100644 --- a/internal/model/mocks/dnsencoder.go +++ b/internal/model/mocks/dnsencoder.go @@ -2,10 +2,10 @@ package mocks // DNSEncoder allows mocking dnsx.DNSEncoder. type DNSEncoder struct { - MockEncode func(domain string, qtype uint16, padding bool) ([]byte, error) + MockEncode func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) } // Encode calls MockEncode. -func (e *DNSEncoder) Encode(domain string, qtype uint16, padding bool) ([]byte, error) { +func (e *DNSEncoder) Encode(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { return e.MockEncode(domain, qtype, padding) } diff --git a/internal/model/mocks/dnsencoder_test.go b/internal/model/mocks/dnsencoder_test.go index 7380cfd..0918f77 100644 --- a/internal/model/mocks/dnsencoder_test.go +++ b/internal/model/mocks/dnsencoder_test.go @@ -11,16 +11,19 @@ func TestDNSEncoder(t *testing.T) { t.Run("Encode", func(t *testing.T) { expected := errors.New("mocked error") e := &DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return nil, expected + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, expected }, } - out, err := e.Encode("dns.google", dns.TypeA, true) + out, queryID, err := e.Encode("dns.google", dns.TypeA, true) if !errors.Is(err, expected) { t.Fatal("unexpected err", err) } if out != nil { t.Fatal("unexpected out") } + if queryID != 0 { + t.Fatal("unexpected queryID") + } }) } diff --git a/internal/model/mocks/http_test.go b/internal/model/mocks/http_test.go index 7351207..0cabb67 100644 --- a/internal/model/mocks/http_test.go +++ b/internal/model/mocks/http_test.go @@ -9,6 +9,18 @@ import ( ) func TestHTTPTransport(t *testing.T) { + t.Run("Network", func(t *testing.T) { + expected := "quic" + txp := &HTTPTransport{ + MockNetwork: func() string { + return expected + }, + } + if txp.Network() != expected { + t.Fatal("unexpected network value") + } + }) + t.Run("RoundTrip", func(t *testing.T) { expected := errors.New("mocked error") txp := &HTTPTransport{ diff --git a/internal/model/netx.go b/internal/model/netx.go index c99e0da..16b8df3 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -9,6 +9,7 @@ import ( "time" "github.com/lucas-clemente/quic-go" + "github.com/miekg/dns" ) // @@ -25,6 +26,8 @@ type DNSDecoder interface { // // - data contains the reply bytes read from a DNSTransport // + // - queryID is the original query ID + // // Returns: // // - on success, a list of IP addrs inside the reply and a nil error @@ -33,9 +36,9 @@ type DNSDecoder interface { // // Note that this function will return an error if there is no // IP address inside of the reply. - DecodeLookupHost(qtype uint16, data []byte) ([]string, error) + DecodeLookupHost(qtype uint16, data []byte, queryID uint16) ([]string, error) - // DecodeHTTPS decodes an HTTPS reply. + // DecodeHTTPS is like DecodeLookupHost but decodes an HTTPS reply. // // The argument is the reply as read by the DNSTransport. // @@ -46,7 +49,22 @@ type DNSDecoder interface { // This function will return an error if the HTTPS reply does not // contain at least a valid ALPN entry. It will not return // an error, though, when there are no IPv4/IPv6 hints in the reply. - DecodeHTTPS(data []byte) (*HTTPSSvc, error) + DecodeHTTPS(data []byte, queryID uint16) (*HTTPSSvc, error) + + // DecodeReply decodes a DNS reply message. + // + // Arguments: + // + // - data is the raw reply + // + // If you use this function, remember that: + // + // 1. the Rcode MAY be nonzero; + // + // 2. the replyID MAY NOT match the original query ID. + // + // That is, this is a very basic parsing method. + DecodeReply(data []byte) (*dns.Msg, error) } // The DNSEncoder encodes DNS queries to bytes @@ -61,9 +79,10 @@ type DNSEncoder interface { // // - padding is whether to add padding to the query. // - // On success, this function returns a valid byte array and - // a nil error. On failure, we have an error and the byte array is nil. - Encode(domain string, qtype uint16, padding bool) ([]byte, error) + // On success, this function returns a valid byte array, the queryID, and + // a nil error. On failure, we have a non-nil error, a nil arrary and a zero + // query ID. + Encode(domain string, qtype uint16, padding bool) ([]byte, uint16, error) } // DNSTransport represents an abstract DNS transport. diff --git a/internal/netxlite/classify.go b/internal/netxlite/classify.go index 4314728..6bb3d21 100644 --- a/internal/netxlite/classify.go +++ b/internal/netxlite/classify.go @@ -287,6 +287,9 @@ func classifyResolverError(err error) string { if errors.Is(err, ErrOODNSServfail) { return FailureDNSServfailError } + if errors.Is(err, ErrDNSReplyWithWrongQueryID) { + return FailureDNSReplyWithWrongQueryID + } return classifyGenericError(err) } diff --git a/internal/netxlite/classify_test.go b/internal/netxlite/classify_test.go index 69f1d16..01049de 100644 --- a/internal/netxlite/classify_test.go +++ b/internal/netxlite/classify_test.go @@ -275,6 +275,12 @@ func TestClassifyResolverError(t *testing.T) { } }) + t.Run("for dns reply with wrong queryID", func(t *testing.T) { + if classifyResolverError(ErrDNSReplyWithWrongQueryID) != FailureDNSReplyWithWrongQueryID { + t.Fatal("unexpected result") + } + }) + t.Run("for another kind of error", func(t *testing.T) { if classifyResolverError(io.EOF) != FailureEOFError { t.Fatal("unexpected result") diff --git a/internal/netxlite/dnsdecoder.go b/internal/netxlite/dnsdecoder.go index ee0b564..22ef9fb 100644 --- a/internal/netxlite/dnsdecoder.go +++ b/internal/netxlite/dnsdecoder.go @@ -1,6 +1,8 @@ package netxlite import ( + "errors" + "github.com/miekg/dns" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -8,11 +10,26 @@ import ( // DNSDecoderMiekg uses github.com/miekg/dns to implement the Decoder. type DNSDecoderMiekg struct{} -func (d *DNSDecoderMiekg) parseReply(data []byte) (*dns.Msg, error) { +// ErrDNSReplyWithWrongQueryID indicates we have got a DNS reply with the wrong queryID. +var ErrDNSReplyWithWrongQueryID = errors.New(FailureDNSReplyWithWrongQueryID) + +// DecodeReply implements model.DNSDecoder.DecodeReply +func (d *DNSDecoderMiekg) DecodeReply(data []byte) (*dns.Msg, error) { reply := new(dns.Msg) if err := reply.Unpack(data); err != nil { return nil, err } + return reply, nil +} + +func (d *DNSDecoderMiekg) parseReply(data []byte, queryID uint16) (*dns.Msg, error) { + reply, err := d.DecodeReply(data) + if err != nil { + return nil, err + } + if reply.Id != queryID { + return nil, ErrDNSReplyWithWrongQueryID + } // TODO(bassosimone): map more errors to net.DNSError names // TODO(bassosimone): add support for lame referral. switch reply.Rcode { @@ -29,8 +46,8 @@ func (d *DNSDecoderMiekg) parseReply(data []byte) (*dns.Msg, error) { } } -func (d *DNSDecoderMiekg) DecodeHTTPS(data []byte) (*model.HTTPSSvc, error) { - reply, err := d.parseReply(data) +func (d *DNSDecoderMiekg) DecodeHTTPS(data []byte, queryID uint16) (*model.HTTPSSvc, error) { + reply, err := d.parseReply(data, queryID) if err != nil { return nil, err } @@ -64,8 +81,8 @@ func (d *DNSDecoderMiekg) DecodeHTTPS(data []byte) (*model.HTTPSSvc, error) { return out, nil } -func (d *DNSDecoderMiekg) DecodeLookupHost(qtype uint16, data []byte) ([]string, error) { - reply, err := d.parseReply(data) +func (d *DNSDecoderMiekg) DecodeLookupHost(qtype uint16, data []byte, queryID uint16) ([]string, error) { + reply, err := d.parseReply(data, queryID) if err != nil { return nil, err } diff --git a/internal/netxlite/dnsdecoder_test.go b/internal/netxlite/dnsdecoder_test.go index 9915502..896701d 100644 --- a/internal/netxlite/dnsdecoder_test.go +++ b/internal/netxlite/dnsdecoder_test.go @@ -8,15 +8,32 @@ import ( "github.com/google/go-cmp/cmp" "github.com/miekg/dns" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) func TestDNSDecoder(t *testing.T) { t.Run("LookupHost", func(t *testing.T) { t.Run("UnpackError", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost(dns.TypeA, nil) - if err == nil { - t.Fatal("expected an error here") + data, err := d.DecodeLookupHost(dns.TypeA, nil, 0) + if err == nil || err.Error() != "dns: overflow unpacking uint16" { + t.Fatal("unexpected error", err) + } + if data != nil { + t.Fatal("expected nil data here") + } + }) + + t.Run("wrong query ID", func(t *testing.T) { + d := &DNSDecoderMiekg{} + const ( + queryID = 17 + unrelatedID = 14 + ) + reply := dnsGenLookupHostReplySuccess(dnsGenQuery(dns.TypeA, queryID)) + data, err := d.DecodeLookupHost(dns.TypeA, reply, unrelatedID) + if !errors.Is(err, ErrDNSReplyWithWrongQueryID) { + t.Fatal("unexpected error", err) } if data != nil { t.Fatal("expected nil data here") @@ -25,8 +42,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("NXDOMAIN", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeA, dnsGenReplyWithError(t, dns.TypeA, dns.RcodeNameError)) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeA, dnsGenReplyWithError( + dnsGenQuery(dns.TypeA, queryID), dns.RcodeNameError), queryID) if err == nil || !strings.HasSuffix(err.Error(), "no such host") { t.Fatal("not the error we expected", err) } @@ -37,8 +55,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("Refused", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeA, dnsGenReplyWithError(t, dns.TypeA, dns.RcodeRefused)) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeA, dnsGenReplyWithError( + dnsGenQuery(dns.TypeA, queryID), dns.RcodeRefused), queryID) if !errors.Is(err, ErrOODNSRefused) { t.Fatal("not the error we expected", err) } @@ -49,8 +68,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("Servfail", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeA, dnsGenReplyWithError(t, dns.TypeA, dns.RcodeServerFailure)) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeA, dnsGenReplyWithError( + dnsGenQuery(dns.TypeA, queryID), dns.RcodeServerFailure), queryID) if !errors.Is(err, ErrOODNSServfail) { t.Fatal("not the error we expected", err) } @@ -61,7 +81,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("no address", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost(dns.TypeA, dnsGenLookupHostReplySuccess(t, dns.TypeA)) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeA, dnsGenLookupHostReplySuccess( + dnsGenQuery(dns.TypeA, queryID)), queryID) if !errors.Is(err, ErrOODNSNoAnswer) { t.Fatal("not the error we expected", err) } @@ -72,8 +94,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("decode A", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeA, dnsGenLookupHostReplySuccess(t, dns.TypeA, "1.1.1.1", "8.8.8.8")) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeA, dnsGenLookupHostReplySuccess( + dnsGenQuery(dns.TypeA, queryID), "1.1.1.1", "8.8.8.8"), queryID) if err != nil { t.Fatal(err) } @@ -90,8 +113,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("decode AAAA", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeAAAA, dnsGenLookupHostReplySuccess(t, dns.TypeAAAA, "::1", "fe80::1")) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeAAAA, dnsGenLookupHostReplySuccess( + dnsGenQuery(dns.TypeAAAA, queryID), "::1", "fe80::1"), queryID) if err != nil { t.Fatal(err) } @@ -108,8 +132,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("unexpected A reply", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeA, dnsGenLookupHostReplySuccess(t, dns.TypeAAAA, "::1", "fe80::1")) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeA, dnsGenLookupHostReplySuccess( + dnsGenQuery(dns.TypeAAAA, queryID), "::1", "fe80::1"), queryID) if !errors.Is(err, ErrOODNSNoAnswer) { t.Fatal("not the error we expected", err) } @@ -120,8 +145,9 @@ func TestDNSDecoder(t *testing.T) { t.Run("unexpected AAAA reply", func(t *testing.T) { d := &DNSDecoderMiekg{} - data, err := d.DecodeLookupHost( - dns.TypeAAAA, dnsGenLookupHostReplySuccess(t, dns.TypeA, "1.1.1.1", "8.8.4.4.")) + queryID := dns.Id() + data, err := d.DecodeLookupHost(dns.TypeAAAA, dnsGenLookupHostReplySuccess( + dnsGenQuery(dns.TypeA, queryID), "1.1.1.1", "8.8.4.4"), queryID) if !errors.Is(err, ErrOODNSNoAnswer) { t.Fatal("not the error we expected", err) } @@ -139,7 +165,7 @@ func TestDNSDecoder(t *testing.T) { if err != nil { t.Fatal(err) } - reply, err := d.parseReply(data) + reply, err := d.parseReply(data, 0) if !errors.Is(err, ErrOODNSMisbehaving) { // catch all error t.Fatal("not the error we expected", err) } @@ -151,7 +177,7 @@ func TestDNSDecoder(t *testing.T) { t.Run("DecodeHTTPS", func(t *testing.T) { t.Run("with nil data", func(t *testing.T) { d := &DNSDecoderMiekg{} - reply, err := d.DecodeHTTPS(nil) + reply, err := d.DecodeHTTPS(nil, 0) if err == nil || err.Error() != "dns: overflow unpacking uint16" { t.Fatal("not the error we expected", err) } @@ -160,10 +186,28 @@ func TestDNSDecoder(t *testing.T) { } }) - t.Run("with empty answer", func(t *testing.T) { - data := dnsGenHTTPSReplySuccess(t, nil, nil, nil) + t.Run("wrong query ID", func(t *testing.T) { d := &DNSDecoderMiekg{} - reply, err := d.DecodeHTTPS(data) + const ( + queryID = 17 + unrelatedID = 14 + ) + reply := dnsGenHTTPSReplySuccess(dnsGenQuery(dns.TypeA, queryID), nil, nil, nil) + data, err := d.DecodeLookupHost(dns.TypeA, 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 := dnsGenHTTPSReplySuccess( + dnsGenQuery(dns.TypeHTTPS, queryID), nil, nil, nil) + d := &DNSDecoderMiekg{} + reply, err := d.DecodeHTTPS(data, queryID) if !errors.Is(err, ErrOODNSNoAnswer) { t.Fatal("unexpected err", err) } @@ -173,12 +217,14 @@ func TestDNSDecoder(t *testing.T) { }) t.Run("with full answer", func(t *testing.T) { + queryID := dns.Id() alpn := []string{"h3"} v4 := []string{"1.1.1.1"} v6 := []string{"::1"} - data := dnsGenHTTPSReplySuccess(t, alpn, v4, v6) + data := dnsGenHTTPSReplySuccess( + dnsGenQuery(dns.TypeHTTPS, queryID), alpn, v4, v6) d := &DNSDecoderMiekg{} - reply, err := d.DecodeHTTPS(data) + reply, err := d.DecodeHTTPS(data, queryID) if err != nil { t.Fatal(err) } @@ -195,64 +241,73 @@ func TestDNSDecoder(t *testing.T) { }) } -// dnsGenReplyWithError generates a DNS reply for the given -// query type (e.g., dns.TypeA) using code as the Rcode. -func dnsGenReplyWithError(t *testing.T, qtype uint16, code int) []byte { +// dnsGenQuery generates a query suitable to be used with testing. +func dnsGenQuery(qtype uint16, queryID uint16) []byte { question := dns.Question{ Name: dns.Fqdn("x.org"), Qtype: qtype, Qclass: dns.ClassINET, } query := new(dns.Msg) - query.Id = dns.Id() + query.Id = queryID query.RecursionDesired = true query.Question = make([]dns.Question, 1) query.Question[0] = question + data, err := query.Pack() + runtimex.PanicOnError(err, "query.Pack failed") + return data +} + +// dnsGenReplyWithError generates a DNS reply for the given +// query type (e.g., dns.TypeA) using code as the Rcode. +func dnsGenReplyWithError(rawQuery []byte, code int) []byte { + query := new(dns.Msg) + err := query.Unpack(rawQuery) + runtimex.PanicOnError(err, "query.Unpack failed") reply := new(dns.Msg) reply.Compress = true reply.MsgHdr.RecursionAvailable = true reply.SetRcode(query, code) data, err := reply.Pack() - if err != nil { - t.Fatal(err) - } + runtimex.PanicOnError(err, "reply.Pack failed") return data } // dnsGenLookupHostReplySuccess generates a successful DNS reply for the given // qtype (e.g., dns.TypeA) containing the given ips... in the answer. -func dnsGenLookupHostReplySuccess(t *testing.T, qtype uint16, ips ...string) []byte { - question := dns.Question{ - Name: dns.Fqdn("x.org"), - Qtype: qtype, - Qclass: dns.ClassINET, - } +func dnsGenLookupHostReplySuccess(rawQuery []byte, ips ...string) []byte { query := new(dns.Msg) - query.Id = dns.Id() - query.RecursionDesired = true - query.Question = make([]dns.Question, 1) - query.Question[0] = question + err := query.Unpack(rawQuery) + runtimex.PanicOnError(err, "query.Unpack failed") + runtimex.PanicIfFalse(len(query.Question) == 1, "more than one question") + question := query.Question[0] reply := new(dns.Msg) reply.Compress = true reply.MsgHdr.RecursionAvailable = true reply.SetReply(query) for _, ip := range ips { - switch qtype { + switch question.Qtype { case dns.TypeA: + if isIPv6(ip) { + continue + } reply.Answer = append(reply.Answer, &dns.A{ Hdr: dns.RR_Header{ Name: dns.Fqdn("x.org"), - Rrtype: qtype, + Rrtype: question.Qtype, Class: dns.ClassINET, Ttl: 0, }, A: net.ParseIP(ip), }) case dns.TypeAAAA: + if !isIPv6(ip) { + continue + } reply.Answer = append(reply.Answer, &dns.AAAA{ Hdr: dns.RR_Header{ Name: dns.Fqdn("x.org"), - Rrtype: qtype, + Rrtype: question.Qtype, Class: dns.ClassINET, Ttl: 0, }, @@ -261,25 +316,16 @@ func dnsGenLookupHostReplySuccess(t *testing.T, qtype uint16, ips ...string) []b } } data, err := reply.Pack() - if err != nil { - t.Fatal(err) - } + runtimex.PanicOnError(err, "reply.Pack failed") return data } // dnsGenHTTPSReplySuccess generates a successful HTTPS response containing // the given (possibly nil) alpns, ipv4s, and ipv6s. -func dnsGenHTTPSReplySuccess(t *testing.T, alpns, ipv4s, ipv6s []string) []byte { - question := dns.Question{ - Name: dns.Fqdn("x.org"), - Qtype: dns.TypeHTTPS, - Qclass: dns.ClassINET, - } +func dnsGenHTTPSReplySuccess(rawQuery []byte, alpns, ipv4s, ipv6s []string) []byte { query := new(dns.Msg) - query.Id = dns.Id() - query.RecursionDesired = true - query.Question = make([]dns.Question, 1) - query.Question[0] = question + err := query.Unpack(rawQuery) + runtimex.PanicOnError(err, "query.Unpack failed") reply := new(dns.Msg) reply.Compress = true reply.MsgHdr.RecursionAvailable = true @@ -315,8 +361,6 @@ func dnsGenHTTPSReplySuccess(t *testing.T, alpns, ipv4s, ipv6s []string) []byte answer.Value = append(answer.Value, &dns.SVCBIPv6Hint{Hint: addrs}) } data, err := reply.Pack() - if err != nil { - t.Fatal(err) - } + runtimex.PanicOnError(err, "reply.Pack failed") return data } diff --git a/internal/netxlite/dnsencoder.go b/internal/netxlite/dnsencoder.go index dc5cdf4..f5394be 100644 --- a/internal/netxlite/dnsencoder.go +++ b/internal/netxlite/dnsencoder.go @@ -19,7 +19,7 @@ const ( dnsDNSSECEnabled = true ) -func (e *DNSEncoderMiekg) Encode(domain string, qtype uint16, padding bool) ([]byte, error) { +func (e *DNSEncoderMiekg) Encode(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { question := dns.Question{ Name: dns.Fqdn(domain), Qtype: qtype, @@ -43,7 +43,8 @@ func (e *DNSEncoderMiekg) Encode(domain string, qtype uint16, padding bool) ([]b opt.Padding = make([]byte, remainder) query.IsEdns0().Option = append(query.IsEdns0().Option, opt) } - return query.Pack() + data, err := query.Pack() + return data, query.Id, err } var _ model.DNSEncoder = &DNSEncoderMiekg{} diff --git a/internal/netxlite/dnsencoder_test.go b/internal/netxlite/dnsencoder_test.go index fabdd89..6f85809 100644 --- a/internal/netxlite/dnsencoder_test.go +++ b/internal/netxlite/dnsencoder_test.go @@ -10,7 +10,7 @@ import ( func TestDNSEncoder(t *testing.T) { t.Run("encode A", func(t *testing.T) { e := &DNSEncoderMiekg{} - data, err := e.Encode("x.org", dns.TypeA, false) + data, _, err := e.Encode("x.org", dns.TypeA, false) if err != nil { t.Fatal(err) } @@ -19,7 +19,7 @@ func TestDNSEncoder(t *testing.T) { t.Run("encode AAAA", func(t *testing.T) { e := &DNSEncoderMiekg{} - data, err := e.Encode("x.org", dns.TypeAAAA, false) + data, _, err := e.Encode("x.org", dns.TypeAAAA, false) if err != nil { t.Fatal(err) } @@ -31,7 +31,7 @@ func TestDNSEncoder(t *testing.T) { // array of values we obtain the right query size. getquerylen := func(domainlen int, padding bool) int { e := &DNSEncoderMiekg{} - data, err := e.Encode( + data, _, err := e.Encode( // This is not a valid name because it ends up being way // longer than 255 octets. However, the library is allowing // us to generate such name and we are not going to send diff --git a/internal/netxlite/errno.go b/internal/netxlite/errno.go index 456d86b..476578b 100644 --- a/internal/netxlite/errno.go +++ b/internal/netxlite/errno.go @@ -1,5 +1,5 @@ // Code generated by go generate; DO NOT EDIT. -// Generated: 2022-05-13 19:09:07.343096 +0200 CEST m=+0.512294417 +// Generated: 2022-05-14 18:04:43.744122 +0200 CEST m=+0.315992417 package netxlite @@ -25,6 +25,7 @@ const ( FailureDNSNoAnswer = "dns_no_answer" FailureDNSNonRecoverableFailure = "dns_non_recoverable_failure" FailureDNSRefusedError = "dns_refused_error" + FailureDNSReplyWithWrongQueryID = "dns_reply_with_wrong_query_id" FailureDNSServerMisbehaving = "dns_server_misbehaving" FailureDNSServfailError = "dns_servfail_error" FailureDNSTemporaryFailure = "dns_temporary_failure" @@ -75,6 +76,7 @@ var failuresMap = map[string]string{ "dns_non_recoverable_failure": "dns_non_recoverable_failure", "dns_nxdomain_error": "dns_nxdomain_error", "dns_refused_error": "dns_refused_error", + "dns_reply_with_wrong_query_id": "dns_reply_with_wrong_query_id", "dns_server_misbehaving": "dns_server_misbehaving", "dns_servfail_error": "dns_servfail_error", "dns_temporary_failure": "dns_temporary_failure", diff --git a/internal/netxlite/internal/generrno/main.go b/internal/netxlite/internal/generrno/main.go index 8de2143..4c75357 100644 --- a/internal/netxlite/internal/generrno/main.go +++ b/internal/netxlite/internal/generrno/main.go @@ -159,6 +159,7 @@ var Specs = []*ErrorSpec{ NewLibraryError("DNS_server_misbehaving"), NewLibraryError("DNS_no_answer"), NewLibraryError("DNS_servfail_error"), + NewLibraryError("DNS_reply_with_wrong_query_ID"), NewLibraryError("EOF_error"), NewLibraryError("generic_timeout_error"), NewLibraryError("QUIC_incompatible_version"), diff --git a/internal/netxlite/parallelresolver.go b/internal/netxlite/parallelresolver.go index 1da5397..bf0e2cc 100644 --- a/internal/netxlite/parallelresolver.go +++ b/internal/netxlite/parallelresolver.go @@ -85,7 +85,7 @@ func (r *ParallelResolver) LookupHost(ctx context.Context, hostname string) ([]s // LookupHTTPS implements Resolver.LookupHTTPS. func (r *ParallelResolver) LookupHTTPS( ctx context.Context, hostname string) (*model.HTTPSSvc, error) { - querydata, err := r.Encoder.Encode( + querydata, queryID, err := r.Encoder.Encode( hostname, dns.TypeHTTPS, r.Txp.RequiresPadding()) if err != nil { return nil, err @@ -94,7 +94,7 @@ func (r *ParallelResolver) LookupHTTPS( if err != nil { return nil, err } - return r.Decoder.DecodeHTTPS(replydata) + return r.Decoder.DecodeHTTPS(replydata, queryID) } // parallelResolverResult is the internal representation of a @@ -107,7 +107,7 @@ type parallelResolverResult struct { // lookupHost issues a lookup host query for the specified qtype (e.g., dns.A). func (r *ParallelResolver) lookupHost(ctx context.Context, hostname string, qtype uint16, out chan<- *parallelResolverResult) { - querydata, err := r.Encoder.Encode(hostname, qtype, r.Txp.RequiresPadding()) + querydata, queryID, err := r.Encoder.Encode(hostname, qtype, r.Txp.RequiresPadding()) if err != nil { out <- ¶llelResolverResult{ addrs: []string{}, @@ -123,7 +123,7 @@ func (r *ParallelResolver) lookupHost(ctx context.Context, hostname string, } return } - addrs, err := r.Decoder.DecodeLookupHost(qtype, replydata) + addrs, err := r.Decoder.DecodeLookupHost(qtype, replydata, queryID) out <- ¶llelResolverResult{ addrs: addrs, err: err, diff --git a/internal/netxlite/parallelresolver_test.go b/internal/netxlite/parallelresolver_test.go index 772340a..349a500 100644 --- a/internal/netxlite/parallelresolver_test.go +++ b/internal/netxlite/parallelresolver_test.go @@ -34,8 +34,8 @@ func TestParallelResolver(t *testing.T) { txp := NewDNSOverTLS((&tls.Dialer{}).DialContext, "8.8.8.8:853") r := ParallelResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return nil, mocked + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, mocked }, }, Txp: txp, @@ -72,7 +72,7 @@ func TestParallelResolver(t *testing.T) { t.Run("empty reply", func(t *testing.T) { txp := &mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { - return dnsGenLookupHostReplySuccess(t, dns.TypeA), nil + return dnsGenLookupHostReplySuccess(query), nil }, MockRequiresPadding: func() bool { return true @@ -91,7 +91,7 @@ func TestParallelResolver(t *testing.T) { t.Run("with A reply", func(t *testing.T) { txp := &mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { - return dnsGenLookupHostReplySuccess(t, dns.TypeA, "8.8.8.8"), nil + return dnsGenLookupHostReplySuccess(query, "8.8.8.8"), nil }, MockRequiresPadding: func() bool { return true @@ -103,14 +103,14 @@ func TestParallelResolver(t *testing.T) { t.Fatal(err) } if len(addrs) != 1 || addrs[0] != "8.8.8.8" { - t.Fatal("not the result we expected") + t.Fatal("not the result we expected", addrs) } }) t.Run("with AAAA reply", func(t *testing.T) { txp := &mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { - return dnsGenLookupHostReplySuccess(t, dns.TypeAAAA, "::1"), nil + return dnsGenLookupHostReplySuccess(query, "::1"), nil }, MockRequiresPadding: func() bool { return true @@ -122,7 +122,7 @@ func TestParallelResolver(t *testing.T) { t.Fatal(err) } if len(addrs) != 1 || addrs[0] != "::1" { - t.Fatal("not the result we expected") + t.Fatal("not the result we expected", addrs) } }) @@ -182,8 +182,8 @@ func TestParallelResolver(t *testing.T) { expected := errors.New("mocked error") r := &ParallelResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return nil, expected + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, expected }, }, Decoder: nil, @@ -208,8 +208,8 @@ func TestParallelResolver(t *testing.T) { expected := errors.New("mocked error") r := &ParallelResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return make([]byte, 64), nil + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil }, }, Decoder: nil, @@ -237,12 +237,12 @@ func TestParallelResolver(t *testing.T) { expected := errors.New("mocked error") r := &ParallelResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return make([]byte, 64), nil + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil }, }, Decoder: &mocks.DNSDecoder{ - MockDecodeHTTPS: func(reply []byte) (*model.HTTPSSvc, error) { + MockDecodeHTTPS: func(reply []byte, queryID uint16) (*model.HTTPSSvc, error) { return nil, expected }, }, diff --git a/internal/netxlite/serialresolver.go b/internal/netxlite/serialresolver.go index 5b87e9b..66ab153 100644 --- a/internal/netxlite/serialresolver.go +++ b/internal/netxlite/serialresolver.go @@ -84,7 +84,7 @@ func (r *SerialResolver) LookupHost(ctx context.Context, hostname string) ([]str // LookupHTTPS implements Resolver.LookupHTTPS. func (r *SerialResolver) LookupHTTPS( ctx context.Context, hostname string) (*model.HTTPSSvc, error) { - querydata, err := r.Encoder.Encode( + querydata, queryID, err := r.Encoder.Encode( hostname, dns.TypeHTTPS, r.Txp.RequiresPadding()) if err != nil { return nil, err @@ -93,7 +93,7 @@ func (r *SerialResolver) LookupHTTPS( if err != nil { return nil, err } - return r.Decoder.DecodeHTTPS(replydata) + return r.Decoder.DecodeHTTPS(replydata, queryID) } func (r *SerialResolver) lookupHostWithRetry( @@ -126,7 +126,7 @@ func (r *SerialResolver) lookupHostWithRetry( // qtype (dns.A or dns.AAAA) without retrying on failure. func (r *SerialResolver) lookupHostWithoutRetry( ctx context.Context, hostname string, qtype uint16) ([]string, error) { - querydata, err := r.Encoder.Encode(hostname, qtype, r.Txp.RequiresPadding()) + querydata, queryID, err := r.Encoder.Encode(hostname, qtype, r.Txp.RequiresPadding()) if err != nil { return nil, err } @@ -134,5 +134,5 @@ func (r *SerialResolver) lookupHostWithoutRetry( if err != nil { return nil, err } - return r.Decoder.DecodeLookupHost(qtype, replydata) + return r.Decoder.DecodeLookupHost(qtype, replydata, queryID) } diff --git a/internal/netxlite/serialresolver_test.go b/internal/netxlite/serialresolver_test.go index 7975298..8e3b620 100644 --- a/internal/netxlite/serialresolver_test.go +++ b/internal/netxlite/serialresolver_test.go @@ -7,7 +7,6 @@ import ( "net" "testing" - "github.com/miekg/dns" "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" @@ -51,8 +50,8 @@ func TestSerialResolver(t *testing.T) { txp := NewDNSOverTLS((&tls.Dialer{}).DialContext, "8.8.8.8:853") r := SerialResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return nil, mocked + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, mocked }, }, Txp: txp, @@ -89,7 +88,7 @@ func TestSerialResolver(t *testing.T) { t.Run("empty reply", func(t *testing.T) { txp := &mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { - return dnsGenLookupHostReplySuccess(t, dns.TypeA), nil + return dnsGenLookupHostReplySuccess(query), nil }, MockRequiresPadding: func() bool { return true @@ -108,7 +107,7 @@ func TestSerialResolver(t *testing.T) { t.Run("with A reply", func(t *testing.T) { txp := &mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { - return dnsGenLookupHostReplySuccess(t, dns.TypeA, "8.8.8.8"), nil + return dnsGenLookupHostReplySuccess(query, "8.8.8.8"), nil }, MockRequiresPadding: func() bool { return true @@ -127,7 +126,7 @@ func TestSerialResolver(t *testing.T) { t.Run("with AAAA reply", func(t *testing.T) { txp := &mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query []byte) (reply []byte, err error) { - return dnsGenLookupHostReplySuccess(t, dns.TypeAAAA, "::1"), nil + return dnsGenLookupHostReplySuccess(query, "::1"), nil }, MockRequiresPadding: func() bool { return true @@ -189,8 +188,8 @@ func TestSerialResolver(t *testing.T) { expected := errors.New("mocked error") r := &SerialResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return nil, expected + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return nil, 0, expected }, }, Decoder: nil, @@ -215,8 +214,8 @@ func TestSerialResolver(t *testing.T) { expected := errors.New("mocked error") r := &SerialResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return make([]byte, 64), nil + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil }, }, Decoder: nil, @@ -244,12 +243,12 @@ func TestSerialResolver(t *testing.T) { expected := errors.New("mocked error") r := &SerialResolver{ Encoder: &mocks.DNSEncoder{ - MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, error) { - return make([]byte, 64), nil + MockEncode: func(domain string, qtype uint16, padding bool) ([]byte, uint16, error) { + return make([]byte, 64), 0, nil }, }, Decoder: &mocks.DNSDecoder{ - MockDecodeHTTPS: func(reply []byte) (*model.HTTPSSvc, error) { + MockDecodeHTTPS: func(reply []byte, queryID uint16) (*model.HTTPSSvc, error) { return nil, expected }, },