From b735b3d600d8ea98995f5c87dc0095e401bc70c9 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 24 Aug 2023 23:47:17 +0200 Subject: [PATCH] feat(webconnectivityqa): add test for android_dns_cache_no_data (#1210) Part of https://github.com/ooni/probe/issues/2029. The general idea is to modify v0.4 in a subsequent PR to make it WAI for this test case. Currently v0.4 does not correctly support this case, which is why we're doing this work. (See also https://github.com/ooni/probe/issues/2499). --- go.mod | 2 +- go.sum | 4 +-- .../experiment/webconnectivity/qa_test.go | 3 ++ .../webconnectivityqa/dnsblocking.go | 25 +++++++++++++ .../webconnectivityqa/dnsblocking_test.go | 28 +++++++++++++++ .../experiment/webconnectivityqa/testcase.go | 10 ++++++ internal/netemx/android.go | 23 ++++++++++++ internal/netemx/qaenv.go | 35 ++++++++++++++----- internal/netxlite/getaddrinfo.go | 4 +-- internal/netxlite/getaddrinfo_bsd.go | 6 ++-- internal/netxlite/getaddrinfo_linux.go | 10 +++--- internal/netxlite/getaddrinfo_test.go | 6 ++-- internal/netxlite/getaddrinfo_windows.go | 2 +- 13 files changed, 132 insertions(+), 26 deletions(-) create mode 100644 internal/experiment/webconnectivityqa/dnsblocking.go create mode 100644 internal/experiment/webconnectivityqa/dnsblocking_test.go create mode 100644 internal/netemx/android.go diff --git a/go.mod b/go.mod index bcc2c12c2b..8ea805b24e 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/mitchellh/go-wordwrap v1.0.1 github.com/montanaflynn/stats v0.7.1 github.com/ooni/go-libtor v1.1.8 - github.com/ooni/netem v0.0.0-20230824170255-db1220e00a4b + github.com/ooni/netem v0.0.0-20230824211724-219d252971fc github.com/ooni/oocrypto v0.5.3 github.com/ooni/oohttp v0.6.3 github.com/ooni/probe-assets v0.18.0 diff --git a/go.sum b/go.sum index a2096af9fa..4c2abe744a 100644 --- a/go.sum +++ b/go.sum @@ -483,8 +483,8 @@ github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAl github.com/onsi/gomega v1.27.7 h1:fVih9JD6ogIiHUN6ePK7HJidyEDpWGVB5mzM7cWNXoU= github.com/ooni/go-libtor v1.1.8 h1:Wo3V3DVTxl5vZdxtQakqYP+DAHx7pPtAFSl1bnAa08w= github.com/ooni/go-libtor v1.1.8/go.mod h1:q1YyLwRD9GeMyeerVvwc0vJ2YgwDLTp2bdVcrh/JXyI= -github.com/ooni/netem v0.0.0-20230824170255-db1220e00a4b h1:bO3uWwIFe1ViVGQG5tIPfI6uVWNQozN0lxzS+VyG9F4= -github.com/ooni/netem v0.0.0-20230824170255-db1220e00a4b/go.mod h1:3LJOzTIu2O4ADDJN2ILG4ViJOqyH/u9fKY8QT2Rma8Y= +github.com/ooni/netem v0.0.0-20230824211724-219d252971fc h1:zHLh+al/LOYOha6lyIqW2TOhGvwtTupJ157Yh0AVS0o= +github.com/ooni/netem v0.0.0-20230824211724-219d252971fc/go.mod h1:3LJOzTIu2O4ADDJN2ILG4ViJOqyH/u9fKY8QT2Rma8Y= github.com/ooni/oocrypto v0.5.3 h1:CAb0Ze6q/EWD1PRGl9KqpzMfkut4O3XMaiKYsyxrWOs= github.com/ooni/oocrypto v0.5.3/go.mod h1:HjEQ5pQBl6btcWgAsKKq1tFo8CfBrZu63C/vPAUGIDk= github.com/ooni/oohttp v0.6.3 h1:MHydpeAPU/LSDSI/hIFJwZm4afBhd2Yo+rNxxFdeMCY= diff --git a/internal/experiment/webconnectivity/qa_test.go b/internal/experiment/webconnectivity/qa_test.go index b85fdd3dc6..df19358317 100644 --- a/internal/experiment/webconnectivity/qa_test.go +++ b/internal/experiment/webconnectivity/qa_test.go @@ -9,6 +9,9 @@ import ( func TestQA(t *testing.T) { for _, tc := range webconnectivityqa.AllTestCases() { t.Run(tc.Name, func(t *testing.T) { + if (tc.Flags & webconnectivityqa.TestCaseFlagNoV04) != 0 { + t.Skip("this nettest cannot run on Web Connectivity v0.4") + } measurer := NewExperimentMeasurer(Config{}) if err := webconnectivityqa.RunTestCase(measurer, tc); err != nil { t.Fatal(err) diff --git a/internal/experiment/webconnectivityqa/dnsblocking.go b/internal/experiment/webconnectivityqa/dnsblocking.go new file mode 100644 index 0000000000..47c236cdf5 --- /dev/null +++ b/internal/experiment/webconnectivityqa/dnsblocking.go @@ -0,0 +1,25 @@ +package webconnectivityqa + +import ( + "github.com/ooni/probe-cli/v3/internal/netemx" +) + +// dnsBlockingAndroidDNSCacheNoData is the case where we're on Android and the getaddrinfo +// resolver returns the android_dns_cache_no_data error. +func dnsBlockingAndroidDNSCacheNoData() *TestCase { + return &TestCase{ + Name: "measuring https://www.example.com/ with getaddrinfo errors and android_dns_cache_no_data", + Flags: TestCaseFlagNoV04, + Input: "https://www.example.com/", + Configure: func(env *netemx.QAEnv) { + // make sure the env knows we want to emulate our getaddrinfo wrapper behavior + env.EmulateAndroidGetaddrinfo(true) + + // remove the record so that the DNS query returns NXDOMAIN, which is then + // converted into android_dns_cache_no_data by the emulation layer + env.ISPResolverConfig().RemoveRecord("www.example.com") + }, + ExpectErr: false, + ExpectTestKeys: &testKeys{Accessible: false, Blocking: "dns"}, + } +} diff --git a/internal/experiment/webconnectivityqa/dnsblocking_test.go b/internal/experiment/webconnectivityqa/dnsblocking_test.go new file mode 100644 index 0000000000..b3294d03bc --- /dev/null +++ b/internal/experiment/webconnectivityqa/dnsblocking_test.go @@ -0,0 +1,28 @@ +package webconnectivityqa + +import ( + "context" + "errors" + "testing" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/netemx" + "github.com/ooni/probe-cli/v3/internal/netxlite" +) + +func TestDNSBlockingAndroidDNSCacheNoData(t *testing.T) { + env := netemx.MustNewScenario(netemx.InternetScenario) + tc := dnsBlockingAndroidDNSCacheNoData() + tc.Configure(env) + + env.Do(func() { + reso := netxlite.NewStdlibResolver(log.Log) + addrs, err := reso.LookupHost(context.Background(), "www.example.com") + if !errors.Is(err, netxlite.ErrAndroidDNSCacheNoData) { + t.Fatal("unexpected error", err) + } + if len(addrs) != 0 { + t.Fatal("expected to see no addresses") + } + }) +} diff --git a/internal/experiment/webconnectivityqa/testcase.go b/internal/experiment/webconnectivityqa/testcase.go index e02ac977f0..0d4b4fe4ba 100644 --- a/internal/experiment/webconnectivityqa/testcase.go +++ b/internal/experiment/webconnectivityqa/testcase.go @@ -2,11 +2,19 @@ package webconnectivityqa import "github.com/ooni/probe-cli/v3/internal/netemx" +const ( + // TestCaseFlagNoV04 means that this test case should not be run by v0.4 + TestCaseFlagNoV04 = 1 << iota +) + // TestCase is a test case we could run with this package. type TestCase struct { // Name is the test case name Name string + // Flags contains binary flags describing this test case. + Flags int64 + // Input is the input URL Input string @@ -23,6 +31,8 @@ type TestCase struct { // AllTestCases returns all the defined test cases. func AllTestCases() []*TestCase { return []*TestCase{ + dnsBlockingAndroidDNSCacheNoData(), + tlsBlockingConnectionReset(), sucessWithHTTP(), diff --git a/internal/netemx/android.go b/internal/netemx/android.go new file mode 100644 index 0000000000..4f519488fa --- /dev/null +++ b/internal/netemx/android.go @@ -0,0 +1,23 @@ +package netemx + +import ( + "context" + + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/netxlite" +) + +// androidStack wraps [netem.UnderlyingNetwork] to simulate what our getaddrinfo +// wrapper does on Android when it sees the EAI_NODATA return value. +type androidStack struct { + netem.UnderlyingNetwork +} + +// GetaddrinfoLookupANY implements [netem.UnderlyingNetwork] +func (as *androidStack) GetaddrinfoLookupANY(ctx context.Context, domain string) ([]string, string, error) { + addrs, cname, err := as.UnderlyingNetwork.GetaddrinfoLookupANY(ctx, domain) + if err != nil { + err = netxlite.NewErrGetaddrinfo(0, netxlite.ErrAndroidDNSCacheNoData) + } + return addrs, cname, err +} diff --git a/internal/netemx/qaenv.go b/internal/netemx/qaenv.go index 7d32a945a9..33f8177796 100644 --- a/internal/netemx/qaenv.go +++ b/internal/netemx/qaenv.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "sync" + "sync/atomic" "time" "github.com/ooni/netem" @@ -162,6 +163,10 @@ type QAEnv struct { // closables contains all entities where we have to take care of closing. closables []io.Closer + // emulateAndroidGetaddrinfo controls whether to emulate the behavior of our wrapper for + // the android implementation of getaddrinfo returning android_dns_cache_no_data + emulateAndroidGetaddrinfo *atomic.Bool + // ispResolverConfig is the DNS config used by the ISP resolver. ispResolverConfig *netem.DNSConfig @@ -199,14 +204,15 @@ func MustNewQAEnv(options ...QAEnvOption) *QAEnv { // create an empty QAEnv env := &QAEnv{ - clientNICWrapper: config.clientNICWrapper, - clientStack: nil, - closables: []io.Closer{}, - ispResolverConfig: netem.NewDNSConfig(), - dpi: netem.NewDPIEngine(config.logger), - once: sync.Once{}, - otherResolversConfig: netem.NewDNSConfig(), - topology: runtimex.Try1(netem.NewStarTopology(config.logger)), + clientNICWrapper: config.clientNICWrapper, + clientStack: nil, + closables: []io.Closer{}, + emulateAndroidGetaddrinfo: &atomic.Bool{}, + ispResolverConfig: netem.NewDNSConfig(), + dpi: netem.NewDPIEngine(config.logger), + once: sync.Once{}, + otherResolversConfig: netem.NewDNSConfig(), + topology: runtimex.Try1(netem.NewStarTopology(config.logger)), } // create all the required internals @@ -406,10 +412,21 @@ func (env *QAEnv) DPIEngine() *netem.DPIEngine { return env.dpi } +// EmulateAndroidGetaddrinfo configures [QAEnv] such that the Do method wraps +// the underlying client stack to return android_dns_cache_no_data on any error +// that occurs. This method can be safely called by multiple goroutines. +func (env *QAEnv) EmulateAndroidGetaddrinfo(value bool) { + env.emulateAndroidGetaddrinfo.Store(value) +} + // Do executes the given function such that [netxlite] code uses the // underlying clientStack rather than ordinary networking code. func (env *QAEnv) Do(function func()) { - WithCustomTProxy(env.clientStack, function) + var stack netem.UnderlyingNetwork = env.clientStack + if env.emulateAndroidGetaddrinfo.Load() { + stack = &androidStack{stack} + } + WithCustomTProxy(stack, function) } // Close closes all the resources used by [QAEnv]. diff --git a/internal/netxlite/getaddrinfo.go b/internal/netxlite/getaddrinfo.go index a69b5425bb..2ef23800e8 100644 --- a/internal/netxlite/getaddrinfo.go +++ b/internal/netxlite/getaddrinfo.go @@ -28,8 +28,8 @@ type ErrGetaddrinfo struct { Code int64 } -// newErrGetaddrinfo creates a new instance of the ErrGetaddrinfo type. -func newErrGetaddrinfo(code int64, err error) *ErrGetaddrinfo { +// NewErrGetaddrinfo creates a new instance of the ErrGetaddrinfo type. +func NewErrGetaddrinfo(code int64, err error) *ErrGetaddrinfo { return &ErrGetaddrinfo{ Underlying: err, Code: code, diff --git a/internal/netxlite/getaddrinfo_bsd.go b/internal/netxlite/getaddrinfo_bsd.go index 8f6a07bd38..faa5e014db 100644 --- a/internal/netxlite/getaddrinfo_bsd.go +++ b/internal/netxlite/getaddrinfo_bsd.go @@ -47,12 +47,12 @@ func (state *getaddrinfoState) toError(code int64, err error, goos string) error // comes up again. golang.org/issue/6232. err = syscall.EMFILE } - return newErrGetaddrinfo(code, err) + return NewErrGetaddrinfo(code, err) case C.EAI_NONAME: err = ErrOODNSNoSuchHost // so it becomes FailureDNSNXDOMAIN - return newErrGetaddrinfo(code, err) + return NewErrGetaddrinfo(code, err) default: err = ErrOODNSMisbehaving // so it becomes FailureDNSServerMisbehaving - return newErrGetaddrinfo(code, err) + return NewErrGetaddrinfo(code, err) } } diff --git a/internal/netxlite/getaddrinfo_linux.go b/internal/netxlite/getaddrinfo_linux.go index f625c83f1d..333e5c23d3 100644 --- a/internal/netxlite/getaddrinfo_linux.go +++ b/internal/netxlite/getaddrinfo_linux.go @@ -79,13 +79,13 @@ func (state *getaddrinfoState) toError(code int64, err error, goos string) error // comes up again. golang.org/issue/6232. err = syscall.EMFILE } - return newErrGetaddrinfo(code, err) + return NewErrGetaddrinfo(code, err) case C.EAI_NONAME: - return newErrGetaddrinfo(code, ErrOODNSNoSuchHost) + return NewErrGetaddrinfo(code, ErrOODNSNoSuchHost) case C.EAI_NODATA: return state.toErrorNODATA(err, goos) default: - return newErrGetaddrinfo(code, ErrOODNSMisbehaving) + return NewErrGetaddrinfo(code, ErrOODNSMisbehaving) } } @@ -154,8 +154,8 @@ func (state *getaddrinfoState) toError(code int64, err error, goos string) error func (state *getaddrinfoState) toErrorNODATA(err error, goos string) error { switch goos { case "android": - return newErrGetaddrinfo(C.EAI_NODATA, ErrAndroidDNSCacheNoData) + return NewErrGetaddrinfo(C.EAI_NODATA, ErrAndroidDNSCacheNoData) default: - return newErrGetaddrinfo(C.EAI_NODATA, ErrOODNSNoAnswer) + return NewErrGetaddrinfo(C.EAI_NODATA, ErrOODNSNoAnswer) } } diff --git a/internal/netxlite/getaddrinfo_test.go b/internal/netxlite/getaddrinfo_test.go index 7b8c2ae09a..da54311a12 100644 --- a/internal/netxlite/getaddrinfo_test.go +++ b/internal/netxlite/getaddrinfo_test.go @@ -28,7 +28,7 @@ func TestErrorToGetaddrinfoRetval(t *testing.T) { }{{ name: "with valid getaddrinfo error", args: args{ - newErrGetaddrinfo(144, nil), + NewErrGetaddrinfo(144, nil), }, want: 144, }, { @@ -49,7 +49,7 @@ func TestErrorToGetaddrinfoRetval(t *testing.T) { } } -func Test_newErrGetaddrinfo(t *testing.T) { +func TestNewErrGetaddrinfo(t *testing.T) { type args struct { code int64 err error @@ -66,7 +66,7 @@ func Test_newErrGetaddrinfo(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := newErrGetaddrinfo(tt.args.code, tt.args.err) + err := NewErrGetaddrinfo(tt.args.code, tt.args.err) if err == nil { t.Fatal("expected non-nil error") } diff --git a/internal/netxlite/getaddrinfo_windows.go b/internal/netxlite/getaddrinfo_windows.go index 2e0a1dc73a..66876fefe5 100644 --- a/internal/netxlite/getaddrinfo_windows.go +++ b/internal/netxlite/getaddrinfo_windows.go @@ -24,5 +24,5 @@ func (state *getaddrinfoState) toError(code int64, err error, goos string) error // is no other error, just cast code to a syscall err. err = syscall.Errno(code) } - return newErrGetaddrinfo(int64(code), err) + return NewErrGetaddrinfo(int64(code), err) }