c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(dnsovergetaddrinfo): collect the CNAME (#876)

Browse Source 
* feat(dnsovergetaddrinfo): collect the CNAME

This diff modifies how dnsovergetaddrinfo.go works such that the
returned DNSResponse includes the CNAME.

Closes https://github.com/ooni/probe/issues/2226.

While there, recognize that we can remove getaddrinfoLookupHost and
always call getaddrinfoLookupANY everywhere. (This simplification is
why we did https://github.com/ooni/probe-cli/pull/874.)

* fix: extra debugging because of failing CI

Everything is OK locally (on macOS). However, maybe things are a bit
different on GNU/Linux perhaps?

Here's the error:

```
--- FAIL: TestPass (0.11s)
    resolver_test.go:113: unexpected rcode
FAIL
coverage: 95.7% of statements
FAIL	github.com/ooni/probe-cli/v3/internal/cmd/jafar/resolver	0.242s
```

I'm a bit confused because jafar's resolver is _unrelated_. But actually this
error never occurred again after a committed the debugging diff.
This commit is contained in:
Simone Basso 2022-08-23 13:53:08 +02:00 committed by GitHub
parent cc24f28b9d
commit 080abf90d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 49 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/cmd/jafar/resolver/resolver_test.go
View File

@ -110,7 +110,7 @@ func checkrequest(
func checksuccess(t *testing.T, reply *dns.Msg) {
if reply.Rcode != dns.RcodeSuccess {
t.Fatal("unexpected rcode")
t.Fatal("unexpected rcode", reply.Rcode)
}
if len(reply.Answer) < 1 {
t.Fatal("too few answers")

47
internal/netxlite/dnsovergetaddrinfo.go
View File

@ -17,8 +17,11 @@ import (
// dnsOverGetaddrinfoTransport is a DNSTransport using getaddrinfo.
type dnsOverGetaddrinfoTransport struct {
testableTimeout time.Duration
testableLookupHost func(ctx context.Context, domain string) ([]string, error)
// (OPTIONAL) allows to run tests with a short timeout
testableTimeout time.Duration
// (OPTIONAL) allows to mock the underlying getaddrinfo call
testableLookupANY func(ctx context.Context, domain string) ([]string, string, error)
}
var _ model.DNSTransport = &dnsOverGetaddrinfoTransport{}
@ -28,13 +31,13 @@ func (txp *dnsOverGetaddrinfoTransport) RoundTrip(
if query.Type() != dns.TypeANY {
return nil, ErrNoDNSTransport
}
addrs, err := txp.lookup(ctx, query.Domain())
addrs, cname, err := txp.lookup(ctx, query.Domain())
if err != nil {
return nil, err
}
resp := &dnsOverGetaddrinfoResponse{
addrs: addrs,
cname: "", // TODO: implement this functionality
cname: cname,
query: query,
}
return resp, nil
@ -46,30 +49,42 @@ type dnsOverGetaddrinfoResponse struct {
query model.DNSQuery
}
// Used to move addrs and cname out of the worker goroutine
type dnsOverGetaddrinfoAddrsAndCNAME struct {
// List of resolved addresses (it's a bug if this is empty)
addrs []string
// Resolved CNAME or empty string
cname string
}
func (txp *dnsOverGetaddrinfoTransport) lookup(
ctx context.Context, hostname string) ([]string, error) {
ctx context.Context, hostname string) ([]string, string, error) {
// This code forces adding a shorter timeout to the domain name
// resolutions when using the system resolver. We have seen cases
// in which such a timeout becomes too large. One such case is
// described in https://github.com/ooni/probe/issues/1726.
addrsch, errch := make(chan []string, 1), make(chan error, 1)
outch, errch := make(chan *dnsOverGetaddrinfoAddrsAndCNAME, 1), make(chan error, 1)
ctx, cancel := context.WithTimeout(ctx, txp.timeout())
defer cancel()
go func() {
addrs, err := txp.lookupfn()(ctx, hostname)
addrs, cname, err := txp.lookupfn()(ctx, hostname)
if err != nil {
errch <- err
return
}
addrsch <- addrs
outch <- &dnsOverGetaddrinfoAddrsAndCNAME{
addrs: addrs,
cname: cname,
}
}()
select {
case <-ctx.Done():
return nil, ctx.Err()
case addrs := <-addrsch:
return addrs, nil
return nil, "", ctx.Err()
case p := <-outch:
return p.addrs, p.cname, nil
case err := <-errch:
return nil, err
return nil, "", err
}
}
@ -80,11 +95,11 @@ func (txp *dnsOverGetaddrinfoTransport) timeout() time.Duration {
return 15 * time.Second
}
func (txp *dnsOverGetaddrinfoTransport) lookupfn() func(ctx context.Context, domain string) ([]string, error) {
if txp.testableLookupHost != nil {
return txp.testableLookupHost
func (txp *dnsOverGetaddrinfoTransport) lookupfn() func(ctx context.Context, domain string) ([]string, string, error) {
if txp.testableLookupANY != nil {
return txp.testableLookupANY
}
return getaddrinfoLookupHost
return getaddrinfoLookupANY
}
func (txp *dnsOverGetaddrinfoTransport) RequiresPadding() bool {

20
internal/netxlite/dnsovergetaddrinfo_test.go
View File

@ -57,8 +57,8 @@ func TestDNSOverGetaddrinfo(t *testing.T) {
t.Run("RoundTrip", func(t *testing.T) {
t.Run("with invalid query type", func(t *testing.T) {
txp := &dnsOverGetaddrinfoTransport{
testableLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return []string{"8.8.8.8"}, nil
testableLookupANY: func(ctx context.Context, domain string) ([]string, string, error) {
return []string{"8.8.8.8"}, "dns.google", nil
},
}
encoder := &DNSEncoderMiekg{}
@ -75,8 +75,8 @@ func TestDNSOverGetaddrinfo(t *testing.T) {
t.Run("with success", func(t *testing.T) {
txp := &dnsOverGetaddrinfoTransport{
testableLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return []string{"8.8.8.8"}, nil
testableLookupANY: func(ctx context.Context, domain string) ([]string, string, error) {
return []string{"8.8.8.8"}, "dns.google", nil
},
}
encoder := &DNSEncoderMiekg{}
@ -124,10 +124,10 @@ func TestDNSOverGetaddrinfo(t *testing.T) {
done := make(chan interface{})
txp := &dnsOverGetaddrinfoTransport{
testableTimeout: 1 * time.Microsecond,
testableLookupHost: func(ctx context.Context, domain string) ([]string, error) {
testableLookupANY: func(ctx context.Context, domain string) ([]string, string, error) {
defer wg.Done()
<-done
return []string{"8.8.8.8"}, nil
return []string{"8.8.8.8"}, "dns.google", nil
},
}
encoder := &DNSEncoderMiekg{}
@ -150,10 +150,10 @@ func TestDNSOverGetaddrinfo(t *testing.T) {
done := make(chan interface{})
txp := &dnsOverGetaddrinfoTransport{
testableTimeout: 1 * time.Microsecond,
testableLookupHost: func(ctx context.Context, domain string) ([]string, error) {
testableLookupANY: func(ctx context.Context, domain string) ([]string, string, error) {
defer wg.Done()
<-done
return nil, errors.New("no such host")
return nil, "", errors.New("no such host")
},
}
encoder := &DNSEncoderMiekg{}
@ -172,8 +172,8 @@ func TestDNSOverGetaddrinfo(t *testing.T) {
t.Run("with NXDOMAIN", func(t *testing.T) {
txp := &dnsOverGetaddrinfoTransport{
testableLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return nil, ErrOODNSNoSuchHost
testableLookupANY: func(ctx context.Context, domain string) ([]string, string, error) {
return nil, "", ErrOODNSNoSuchHost
},
}
encoder := &DNSEncoderMiekg{}

17
internal/netxlite/getaddrinfo.go
View File

@ -1,21 +1,6 @@
package netxlite
import (
"context"
"errors"
)
// getaddrinfoLookupHost performs a DNS lookup and returns the
// results. If we were compiled with CGO_ENABLED=0, then this
// function calls net.DefaultResolver.LookupHost. Otherwise,
// we call getaddrinfo. In such a case, if getaddrinfo returns a nonzero
// return value, we'll return as error an instance of the
// ErrGetaddrinfo error. This error will contain the specific
// code returned by getaddrinfo in its .Code field.
func getaddrinfoLookupHost(ctx context.Context, domain string) ([]string, error) {
addrs, _, err := getaddrinfoLookupANY(ctx, domain)
return addrs, err
}
import "errors"
// ErrGetaddrinfo represents a getaddrinfo failure.
type ErrGetaddrinfo struct {

5
internal/netxlite/getaddrinfo_cgo.go
View File

@ -48,7 +48,10 @@ func getaddrinfoResolverNetwork() string {
// the error that occurred. On error, the list of addresses is empty. The
// CNAME may be empty on success, if there's no CNAME, but may also be
// non-empty on failure, if the lookup result included a CNAME answer but
// did not include any A or AAAA answers.
// did not include any A or AAAA answers. If getaddrinfo returns a nonzero
// return value, we'll return as error an instance of the
// ErrGetaddrinfo error. This error will contain the specific
// code returned by getaddrinfo in its .Code field.
func getaddrinfoLookupANY(ctx context.Context, domain string) ([]string, string, error) {
return getaddrinfoStateSingleton.LookupANY(ctx, domain)
}

4
internal/netxlite/getaddrinfo_test.go
View File

@ -7,8 +7,8 @@ import (
"testing"
)
func TestGetaddrinfoLookupHost(t *testing.T) {
addrs, err := getaddrinfoLookupHost(context.Background(), "127.0.0.1")
func TestGetaddrinfoLookupANY(t *testing.T) {
addrs, _, err := getaddrinfoLookupANY(context.Background(), "127.0.0.1")
if err != nil {
t.Fatal(err)
}