Skip to content

Commit

Permalink
feat(netxlite): support extracting the CNAME (#875)
Browse files Browse the repository at this point in the history 
* feat(netxlite): support extracting the CNAME

Closes ooni/probe#2225

* fix(netxlite): attempt to increase coverage and improve tests

1. dnsovergetaddrinfo: specify the behavior of a DNSResponse returned
by this file to make it line with normal responses and write unit tests
to make sure we adhere to expectations;

2. dnsoverudp: make sure we wait to deferred responses also w/o a
custom context and post on a private channel and test that;

3. utls: recognize that we can actually write a test for NetConn and
what needs to change when we'll use go1.19 by default will just be
a cast that at that point can be removed.
  • Loading branch information
@bassosimone
bassosimone authored Aug 23, 2022
1 parent da1c13e commit cc24f28
Show file tree
Hide file tree
Showing 10 changed files with 390 additions and 39 deletions.
5 changes: 5 additions & 0 deletions internal/model/mocks/dnsresponse.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type DNSResponse struct {
MockDecodeHTTPS func() (*model.HTTPSSvc, error)
MockDecodeLookupHost func() ([]string, error)
MockDecodeNS func() ([]*net.NS, error)
MockDecodeCNAME func() (string, error)
}

var _ model.DNSResponse = &DNSResponse{}
Expand Down Expand Up @@ -45,3 +46,7 @@ func (r *DNSResponse) DecodeLookupHost() ([]string, error) {
func (r *DNSResponse) DecodeNS() ([]*net.NS, error) {
return r.MockDecodeNS()
}

func (r *DNSResponse) DecodeCNAME() (string, error) {
return r.MockDecodeCNAME()
}
16 changes: 16 additions & 0 deletions internal/model/mocks/dnsresponse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,20 @@ func TestDNSResponse(t *testing.T) {
t.Fatal("unexpected out")
}
})

t.Run("DecodeCNAME", func(t *testing.T) {
expected := errors.New("mocked error")
r := &DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", expected
},
}
out, err := r.DecodeCNAME()
if !errors.Is(err, expected) {
t.Fatal("unexpected err", err)
}
if out != "" {
t.Fatal("unexpected out")
}
})
}
3 changes: 3 additions & 0 deletions internal/model/netx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type DNSResponse interface {

// DecodeNS returns all the NS entries in this response.
DecodeNS() ([]*net.NS, error)

// DecodeCNAME returns the first CNAME entry in this response.
DecodeCNAME() (string, error)
}

// The DNSDecoder decodes DNS responses.
Expand Down
14 changes: 14 additions & 0 deletions internal/netxlite/dnsdecoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,19 @@ func (r *dnsResponse) DecodeNS() ([]*net.NS, error) {
return out, nil
}

// DecodeCNAME implements model.DNSResponse.DecodeCNAME.
func (r *dnsResponse) DecodeCNAME() (string, error) {
if err := r.rcodeToError(); err != nil {
return "", err
}
for _, answer := range r.msg.Answer {
switch avalue := answer.(type) {
case *dns.CNAME:
return avalue.Target, nil
}
}
return "", ErrOODNSNoAnswer
}

var _ model.DNSDecoder = &DNSDecoderMiekg{}
var _ model.DNSResponse = &dnsResponse{}
116 changes: 106 additions & 10 deletions internal/netxlite/dnsdecoder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
queryID = 17
unrelatedID = 14
)
reply := dnsGenLookupHostReplySuccess(dnsGenQuery(dns.TypeA, queryID))
reply := dnsGenLookupHostReplySuccess(dnsGenQuery(dns.TypeA, queryID), nil)
resp, err := d.DecodeResponse(reply, &mocks.DNSQuery{
MockID: func() uint16 {
return unrelatedID
Expand All @@ -62,7 +62,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil)
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand All @@ -81,7 +81,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil)
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
})
})

t.Run("dnsResponse.LookupHost", func(t *testing.T) {
t.Run("dnsResponse.DecodeLookupHost", func(t *testing.T) {
t.Run("with failure", func(t *testing.T) {
// Ensure that we're not trying to decode if rcode != 0
d := &DNSDecoderMiekg{}
Expand Down Expand Up @@ -352,7 +352,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil)
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand All @@ -375,7 +375,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, "1.1.1.1", "8.8.8.8")
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil, "1.1.1.1", "8.8.8.8")
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand Down Expand Up @@ -407,7 +407,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeAAAA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, "::1", "fe80::1")
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil, "::1", "fe80::1")
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand Down Expand Up @@ -439,7 +439,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeAAAA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, "1.1.1.1", "8.8.8.8")
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil, "1.1.1.1", "8.8.8.8")
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand All @@ -465,7 +465,7 @@ func TestDNSDecoderMiekg(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, "::1", "fe80::1")
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, nil, "::1", "fe80::1")
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
Expand All @@ -487,6 +487,82 @@ func TestDNSDecoderMiekg(t *testing.T) {
}
})
})

t.Run("dnsResponse.DecodeCNAME", func(t *testing.T) {
t.Run("with failure", func(t *testing.T) {
// Ensure that we're not trying to decode if rcode != 0
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenReplyWithError(rawQuery, dns.RcodeRefused)
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
},
}
resp, err := d.DecodeResponse(rawResponse, query)
if err != nil {
t.Fatal(err)
}
cname, err := resp.DecodeCNAME()
if !errors.Is(err, ErrOODNSRefused) {
t.Fatal("unexpected err", err)
}
if cname != "" {
t.Fatal("expected empty cname result")
}
})

t.Run("with empty answer", func(t *testing.T) {
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
var expectedCNAME *dnsCNAMEAnswer = nil // explicity not set
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, expectedCNAME, "8.8.8.8")
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
},
}
resp, err := d.DecodeResponse(rawResponse, query)
if err != nil {
t.Fatal(err)
}
cname, err := resp.DecodeCNAME()
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("unexpected err", err)
}
if cname != "" {
t.Fatal("expected empty cname result")
}
})

t.Run("with full answer", func(t *testing.T) {
expectedCNAME := &dnsCNAMEAnswer{
CNAME: "dns.google.",
}
d := &DNSDecoderMiekg{}
queryID := dns.Id()
rawQuery := dnsGenQuery(dns.TypeA, queryID)
rawResponse := dnsGenLookupHostReplySuccess(rawQuery, expectedCNAME, "8.8.8.8")
query := &mocks.DNSQuery{
MockID: func() uint16 {
return queryID
},
}
resp, err := d.DecodeResponse(rawResponse, query)
if err != nil {
t.Fatal(err)
}
cname, err := resp.DecodeCNAME()
if err != nil {
t.Fatal(err)
}
if cname != expectedCNAME.CNAME {
t.Fatal("unexpected cname", cname)
}
})
})
})
}

Expand Down Expand Up @@ -522,9 +598,18 @@ func dnsGenReplyWithError(rawQuery []byte, code int) []byte {
return data
}

// ImplementationNote: dnsCNAMEAnswer could have been a string but then
// dnsGenLookupHostReplySuccess invocations would have been confusing to read,
// because they would not have had a boundary between CNAME and addrs.

// dnsCNAMEAnswer is the DNS cname answer to include into a response.
type dnsCNAMEAnswer struct {
CNAME string
}

// dnsGenLookupHostReplySuccess generates a successful DNS reply containing the given ips...
// in the answers where each answer's type depends on the IP's type (A/AAAA).
func dnsGenLookupHostReplySuccess(rawQuery []byte, ips ...string) []byte {
func dnsGenLookupHostReplySuccess(rawQuery []byte, cname *dnsCNAMEAnswer, ips ...string) []byte {
query := new(dns.Msg)
err := query.Unpack(rawQuery)
runtimex.PanicOnError(err, "query.Unpack failed")
Expand Down Expand Up @@ -562,6 +647,17 @@ func dnsGenLookupHostReplySuccess(rawQuery []byte, ips ...string) []byte {
})
}
}
if cname != nil {
reply.Answer = append(reply.Answer, &dns.CNAME{
Hdr: dns.RR_Header{
Name: question.Name,
Rrtype: dns.TypeCNAME,
Class: dns.ClassINET,
Ttl: 0,
},
Target: cname.CNAME,
})
}
data, err := reply.Pack()
runtimex.PanicOnError(err, "reply.Pack failed")
return data
Expand Down
14 changes: 14 additions & 0 deletions internal/netxlite/dnsovergetaddrinfo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/miekg/dns"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

// dnsOverGetaddrinfoTransport is a DNSTransport using getaddrinfo.
Expand All @@ -33,13 +34,15 @@ func (txp *dnsOverGetaddrinfoTransport) RoundTrip(
}
resp := &dnsOverGetaddrinfoResponse{
addrs: addrs,
cname: "", // TODO: implement this functionality
query: query,
}
return resp, nil
}

type dnsOverGetaddrinfoResponse struct {
addrs []string
cname string
query model.DNSQuery
}

Expand Down Expand Up @@ -101,6 +104,7 @@ func (txp *dnsOverGetaddrinfoTransport) CloseIdleConnections() {
}

func (r *dnsOverGetaddrinfoResponse) Query() model.DNSQuery {
runtimex.PanicIfNil(r.query, "dnsOverGetaddrinfoResponse with nil query")
return r.query
}

Expand All @@ -117,9 +121,19 @@ func (r *dnsOverGetaddrinfoResponse) DecodeHTTPS() (*model.HTTPSSvc, error) {
}

func (r *dnsOverGetaddrinfoResponse) DecodeLookupHost() ([]string, error) {
if len(r.addrs) <= 0 {
return nil, ErrOODNSNoAnswer
}
return r.addrs, nil
}

func (r *dnsOverGetaddrinfoResponse) DecodeNS() ([]*net.NS, error) {
return nil, ErrNoDNSTransport
}

func (r *dnsOverGetaddrinfoResponse) DecodeCNAME() (string, error) {
if r.cname == "" {
return "", ErrOODNSNoAnswer
}
return r.cname, nil
}
Loading

0 comments on commit cc24f28

Please sign in to comment.