fix(webconnectivity): handle android_dns_cache_no_data as dns anomaly

This diff changes the handling of android_dns_cache_no_data to mark it and
any other DNS inconsistenct as DNS anomaly.

The previous implementation was not handling any DNS inconsistency as an
anomaly, therefore I needed to adjust a bunch of summary_test.go tests.

Part of ooni/probe#2499 and
ooni/probe#2029.

I bumped the version number because this is a significant change
in the analysis algorithm implemented by the probe.

internal/experiment/webconnectivity/summary.go

@@ -100,6 +100,7 @@ func Summarize(tk *TestKeys) (out Summary) {
 	defer func() {
 		out.Blocking = DetermineBlocking(out)
 	}()
+
 	var (
 		accessible   = true
 		inaccessible = false
@@ -108,6 +109,7 @@ func Summarize(tk *TestKeys) (out Summary) {
 		httpFailure  = "http-failure"
 		tcpIP        = "tcp_ip"
 	)
+
 	// If the measurement was for an HTTPS website and the HTTP experiment
 	// succeeded, then either there is a compromised CA in our pool (which is
 	// certifi-go), or there is transparent proxying, or we are actually
@@ -119,11 +121,13 @@ func Summarize(tk *TestKeys) (out Summary) {
 		out.Status |= StatusSuccessSecure
 		return
 	}
+
 	// If we couldn't contact the control, we cannot do much more here.
 	if tk.ControlFailure != nil {
 		out.Status |= StatusAnomalyControlUnreachable
 		return
 	}
+
 	// If DNS failed with NXDOMAIN and the control DNS is consistent, then it
 	// means this website does not exist anymore. We need to include the weird
 	// cache failure on Android into this analysis because that failure means
@@ -142,15 +146,16 @@ func Summarize(tk *TestKeys) (out Summary) {
 		out.Status |= StatusSuccessNXDOMAIN | StatusExperimentDNS
 		return
 	}
-	// Otherwise, if DNS failed with NXDOMAIN, it's DNS based blocking.
-	// TODO(bassosimone): do we wanna include other errors here? Like timeout?
-	if tk.DNSExperimentFailure != nil &&
-		*tk.DNSExperimentFailure == netxlite.FailureDNSNXDOMAINError {
+
+	// Otherwise, if the DNS is inconsistent, flag a DNS failure. Note that this
+	// case also includes when the probe lookup fails and the TH one does not.
+	if tk.DNSConsistency != nil && *tk.DNSConsistency == DNSInconsistent {
 		out.Accessible = &inaccessible
 		out.BlockingReason = &dns
 		out.Status |= StatusAnomalyDNS | StatusExperimentDNS
 		return
 	}
+
 	// If we tried to connect more than once and never succedeed and we were
 	// able to measure DNS consistency, then we can conclude something.
 	if tk.TCPConnectAttempts > 0 && tk.TCPConnectSuccesses <= 0 && tk.DNSConsistency != nil {

internal/experiment/webconnectivity/summary_test.go

@@ -152,9 +152,7 @@ func TestSummarize(t *testing.T) {
 			BlockingReason: &dns,
 			Blocking:       &dns,
 			Accessible:     &falseValue,
-			Status: webconnectivity.StatusAnomalyConnect |
-				webconnectivity.StatusExperimentConnect |
-				webconnectivity.StatusAnomalyDNS,
+			Status:         webconnectivity.StatusAnomalyDNS | webconnectivity.StatusExperimentDNS,
 		},
 	}, {
 		name: "with TCP total failure and unexpected DNS consistency",
@@ -350,9 +348,7 @@ func TestSummarize(t *testing.T) {
 			BlockingReason: &dns,
 			Blocking:       &dns,
 			Accessible:     &falseValue,
-			Status: webconnectivity.StatusExperimentHTTP |
-				webconnectivity.StatusAnomalyTLSHandshake |
-				webconnectivity.StatusAnomalyDNS,
+			Status:         webconnectivity.StatusExperimentDNS | webconnectivity.StatusAnomalyDNS,
 		},
 	}, {
 		name: "with SSL unknown auth _and_ untrustworthy DNS _and_ a longer chain",
@@ -367,11 +363,10 @@ func TestSummarize(t *testing.T) {
 			},
 		},
 		wantOut: webconnectivity.Summary{
-			BlockingReason: &httpFailure,
-			Blocking:       &httpFailure,
+			BlockingReason: &dns,
+			Blocking:       &dns,
 			Accessible:     &falseValue,
-			Status: webconnectivity.StatusExperimentHTTP |
-				webconnectivity.StatusAnomalyTLSHandshake,
+			Status:         webconnectivity.StatusExperimentDNS | webconnectivity.StatusAnomalyDNS,
 		},
 	}, {
 		name: "with status code and body length matching",
@@ -442,8 +437,7 @@ func TestSummarize(t *testing.T) {
 			BlockingReason: &dns,
 			Blocking:       &dns,
 			Accessible:     &falseValue,
-			Status: webconnectivity.StatusAnomalyHTTPDiff |
-				webconnectivity.StatusAnomalyDNS,
+			Status:         webconnectivity.StatusExperimentDNS | webconnectivity.StatusAnomalyDNS,
 		},
 	}, {
 		name: "with suspect http-diff and consistent DNS",

internal/experiment/webconnectivity/webconnectivity.go

@@ -15,7 +15,7 @@ import (
 
 const (
 	testName    = "web_connectivity"
-	testVersion = "0.4.2"
+	testVersion = "0.4.3"
 )
 
 // Config contains the experiment config.

internal/experiment/webconnectivity/webconnectivity_test.go

@@ -21,7 +21,7 @@ func TestNewExperimentMeasurer(t *testing.T) {
 	if measurer.ExperimentName() != "web_connectivity" {
 		t.Fatal("unexpected name")
 	}
-	if measurer.ExperimentVersion() != "0.4.2" {
+	if measurer.ExperimentVersion() != "0.4.3" {
 		t.Fatal("unexpected version")
 	}
 }

internal/experiment/webconnectivityqa/dnsblocking.go

@@ -9,7 +9,7 @@ import (
 func dnsBlockingAndroidDNSCacheNoData() *TestCase {
 	return &TestCase{
 		Name:  "measuring https://www.example.com/ with getaddrinfo errors and android_dns_cache_no_data",
-		Flags: TestCaseFlagNoV04,
+		Flags: 0,
 		Input: "https://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// make sure the env knows we want to emulate our getaddrinfo wrapper behavior
@@ -19,7 +19,11 @@ func dnsBlockingAndroidDNSCacheNoData() *TestCase {
 			// 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"},
+		ExpectErr: false,
+		ExpectTestKeys: &testKeys{
+			DNSConsistency: "inconsistent",
+			Accessible:     false,
+			Blocking:       "dns",
+		},
 	}
 }

internal/experiment/webconnectivityqa/success.go

@@ -4,12 +4,14 @@ package webconnectivityqa
 func sucessWithHTTP() *TestCase {
 	return &TestCase{
 		Name:      "measuring http://www.example.com/ without censorship",
+		Flags:     0,
 		Input:     "http://www.example.com/",
 		Configure: nil,
 		ExpectErr: false,
 		ExpectTestKeys: &testKeys{
-			Accessible: true,
-			Blocking:   false,
+			DNSConsistency: "consistent",
+			Accessible:     true,
+			Blocking:       false,
 		},
 	}
 }
@@ -18,12 +20,14 @@ func sucessWithHTTP() *TestCase {
 func sucessWithHTTPS() *TestCase {
 	return &TestCase{
 		Name:      "measuring https://www.example.com/ without censorship",
+		Flags:     0,
 		Input:     "https://www.example.com/",
 		Configure: nil,
 		ExpectErr: false,
 		ExpectTestKeys: &testKeys{
-			Accessible: true,
-			Blocking:   false,
+			DNSConsistency: "consistent",
+			Accessible:     true,
+			Blocking:       false,
 		},
 	}
 }

internal/experiment/webconnectivityqa/testkeys.go

@@ -11,6 +11,9 @@ import (
 
 // testKeys is the test keys structure returned by this package.
 type testKeys struct {
+	// DNSConsistency indicates whether the DNS is consistent with the TH.
+	DNSConsistency any `json:"dns_consistency"`
+
 	// Accessible indicates whether the URL was accessible.
 	Accessible any `json:"accessible"`
 

internal/experiment/webconnectivityqa/tlsblocking.go

@@ -10,6 +10,7 @@ import (
 func tlsBlockingConnectionReset() *TestCase {
 	return &TestCase{
 		Name:  "measuring https://www.example.com/ with TLS blocking of the SNI",
+		Flags: 0,
 		Input: "https://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 			env.DPIEngine().AddRule(&netem.DPIResetTrafficForTLSSNI{
@@ -19,8 +20,9 @@ func tlsBlockingConnectionReset() *TestCase {
 		},
 		ExpectErr: false,
 		ExpectTestKeys: &testKeys{
-			Accessible: false,
-			Blocking:   "http-failure",
+			DNSConsistency: "consistent",
+			Accessible:     false,
+			Blocking:       "http-failure",
 		},
 	}
 }