feat(webconnectivitylte): handle ghost DNS censorship

internal/experiment/webconnectivitylte/analysisclassic.go

@@ -74,7 +74,10 @@ func analysisClassicDNSConsistency(woa *minipipeline.WebAnalysis) optional.Value
 		return optional.Some("consistent")
 
 	case woa.DNSLookupSuccessWithInvalidAddressesClassic.Len() > 0 || // unexpected addrs; or
-		woa.DNSLookupUnexpectedFailure.Len() > 0: // unexpected failures
+		woa.DNSLookupUnexpectedFailure.Len() > 0 || // unexpected failures; or
+		(woa.DNSLookupSuccess.Len() > 0 && // successful lookups; and
+			!woa.ControlExpectations.IsNone() && // we have control info; and
+			woa.ControlExpectations.Unwrap().DNSAddresses.Len() <= 0): // control resolved nothing
 		return optional.Some("inconsistent")
 
 	default:
@@ -106,6 +109,9 @@ type analysisClassicTestKeysProxy interface {
 
 	// setHTTPExperimentFailure sets the HTTPExperimentFailure field.
 	setHTTPExperimentFailure(value optional.Value[string])
+
+	// setWebsiteDown sets the test keys for a down website.
+	setWebsiteDown()
 }
 
 var _ analysisClassicTestKeysProxy = &TestKeys{}
@@ -167,6 +173,17 @@ func (tk *TestKeys) setHTTPExperimentFailure(value optional.Value[string]) {
 	tk.HTTPExperimentFailure = value
 }
 
+// setWebsiteDown implements analysisClassicTestKeysProxy.
+func (tk *TestKeys) setWebsiteDown() {
+	if !tk.DNSConsistency.IsNone() && tk.DNSConsistency.Unwrap() == "inconsistent" {
+		tk.Blocking = "dns"
+		tk.Accessible = false
+	} else {
+		tk.Blocking = false
+		tk.Accessible = false
+	}
+}
+
 func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk analysisClassicTestKeysProxy) {
 	// minipipeline.NewLinearWebAnalysis produces a woa.Linear sorted
 	//
@@ -242,10 +259,7 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 
 			// 2.2. Handle the case where both the probe and the control failed.
 			if entry.ControlHTTPFailure.Unwrap() != "" {
-				// TODO(bassosimone): returning this result is wrong and we
-				// should also set Accessible to false. However, v0.4
-				// does this and we should play along for the A/B testing.
-				tk.setBlockingFalse()
+				tk.setWebsiteDown()
 				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
@@ -282,10 +296,7 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 
 			// 3.2. Handle the case where both probe and control failed.
 			if entry.ControlTLSHandshakeFailure.Unwrap() != "" {
-				// TODO(bassosimone): returning this result is wrong and we
-				// should set Accessible and Blocking to false. However, v0.4
-				// does this and we should play along for the A/B testing.
-				tk.setBlockingNil()
+				tk.setWebsiteDown()
 				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
@@ -319,10 +330,7 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 
 			// 4.2. Handle the case where both probe and control failed.
 			if entry.ControlTCPConnectFailure.Unwrap() != "" {
-				// TODO(bassosimone): returning this result is wrong and we
-				// should set Accessible and Blocking to false. However, v0.4
-				// does this and we should play along for the A/B testing.
-				tk.setBlockingFalse()
+				tk.setWebsiteDown()
 				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
@@ -344,40 +352,27 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// accessing the website should lead to.
 				if entry.ControlHTTPFailure.IsNone() {
 					tk.setBlockingFalse()
-					analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
+					tk.setHTTPExperimentFailure(entry.Failure)
 					return
 				}
 
 				// 5.1.2. Otherwise, if the control worked, that's blocking.
 				tk.setBlockingString("dns")
-				analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
+				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
 			// 5.2. Handle the case where both probe and control failed.
 			if entry.ControlDNSLookupFailure.Unwrap() != "" {
-				// TODO(bassosimone): returning this result is wrong and we
-				// should set Accessible and Blocking to false. However, v0.4
-				// does this and we should play along for the A/B testing.
-				tk.setBlockingFalse()
-				analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
+				tk.setWebsiteDown()
+				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
 			// 5.3. Handle the case where just the probe failed.
 			tk.setBlockingString("dns")
-			analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
+			tk.setHTTPExperimentFailure(entry.Failure)
 			return
 		}
 	}
 }
-
-// analysisClassicSetHTTPExperimentFailureDNS sets the HTTPExperimentFailure if and
-// only if the entry's TagDepth is >= 1. We have a v0.4 bug where we do not properly
-// set this value for the first HTTP request <facepalm>.
-func analysisClassicSetHTTPExperimentFailureDNS(tk analysisClassicTestKeysProxy, entry *minipipeline.WebObservation) {
-	if entry.TagDepth.UnwrapOr(0) <= 0 {
-		return
-	}
-	tk.setHTTPExperimentFailure(entry.Failure)
-}

internal/experiment/webconnectivitylte/analysisext.go

@@ -262,11 +262,14 @@ func analysisExtExpectedFailures(tk *TestKeys, analysis *minipipeline.WebAnalysi
 	//
 	//	tk.NullNullFlags |= AnalysisFlagNullNullSuccessfulHTTPS
 	//
-	// is set by analysisExtHTTPFinalResponse
+	// is set by analysisExtHTTPFinalResponse when we detect that we do not
+	// have a control response but nonetheless observe successful HTTPS.
 
-	// if the control did not resolve any address but the probe could, this is
+	// If the control did not resolve any address but the probe could, this is
 	// quite likely censorship injecting addrs for otherwise "down" or nonexisting
-	// domains, which lives on as a ghost haunting people
+	// domains, which lives on as a ghost haunting people.
+	//
+	// See https://github.com/ooni/probe/issues/2308.
 	if !analysis.ControlExpectations.IsNone() {
 		expect := analysis.ControlExpectations.Unwrap()
 		if expect.DNSAddresses.Len() <= 0 && analysis.DNSLookupSuccess.Len() > 0 {

internal/experiment/webconnectivityqa/badssl.go

@@ -10,7 +10,7 @@ import (
 func badSSLWithExpiredCertificate() *TestCase {
 	return &TestCase{
 		Name:  "badSSLWithExpiredCertificate",
-		Flags: 0,
+		Flags: TestCaseFlagNoV04,
 		Input: "https://expired.badssl.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// nothing
@@ -21,8 +21,8 @@ func badSSLWithExpiredCertificate() *TestCase {
 			HTTPExperimentFailure: "ssl_invalid_certificate",
 			XStatus:               16, // StatusAnomalyControlFailure
 			XNullNullFlags:        4,  // analysisFlagNullNullTLSMisconfigured
-			Accessible:            nil,
-			Blocking:              nil,
+			Accessible:            false,
+			Blocking:              false,
 		},
 	}
 }
@@ -32,7 +32,7 @@ func badSSLWithExpiredCertificate() *TestCase {
 func badSSLWithWrongServerName() *TestCase {
 	return &TestCase{
 		Name:  "badSSLWithWrongServerName",
-		Flags: 0,
+		Flags: TestCaseFlagNoV04,
 		Input: "https://wrong.host.badssl.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// nothing
@@ -43,8 +43,8 @@ func badSSLWithWrongServerName() *TestCase {
 			HTTPExperimentFailure: "ssl_invalid_hostname",
 			XStatus:               16, // StatusAnomalyControlFailure
 			XNullNullFlags:        4,  // analysisFlagNullNullTLSMisconfigured
-			Accessible:            nil,
-			Blocking:              nil,
+			Accessible:            false,
+			Blocking:              false,
 		},
 	}
 }
@@ -53,7 +53,7 @@ func badSSLWithWrongServerName() *TestCase {
 func badSSLWithUnknownAuthorityWithConsistentDNS() *TestCase {
 	return &TestCase{
 		Name:  "badSSLWithUnknownAuthorityWithConsistentDNS",
-		Flags: 0,
+		Flags: TestCaseFlagNoV04,
 		Input: "https://untrusted-root.badssl.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// nothing
@@ -64,8 +64,8 @@ func badSSLWithUnknownAuthorityWithConsistentDNS() *TestCase {
 			HTTPExperimentFailure: "ssl_unknown_authority",
 			XStatus:               16, // StatusAnomalyControlFailure
 			XNullNullFlags:        4,  // analysisFlagNullNullTLSMisconfigured
-			Accessible:            nil,
-			Blocking:              nil,
+			Accessible:            false,
+			Blocking:              false,
 		},
 	}
 }

internal/experiment/webconnectivityqa/dnsblocking.go

@@ -9,7 +9,7 @@ import (
 func dnsBlockingAndroidDNSCacheNoData() *TestCase {
 	return &TestCase{
 		Name:  "dnsBlockingAndroidDNSCacheNoData",
-		Flags: 0,
+		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
@@ -21,13 +21,14 @@ func dnsBlockingAndroidDNSCacheNoData() *TestCase {
 		},
 		ExpectErr: false,
 		ExpectTestKeys: &testKeys{
-			DNSExperimentFailure: "android_dns_cache_no_data",
-			DNSConsistency:       "inconsistent",
-			XStatus:              2080, // StatusExperimentDNS | StatusAnomalyDNS
-			XDNSFlags:            2,    // AnalysisDNSFlagUnexpectedFailure
-			XBlockingFlags:       33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
-			Accessible:           false,
-			Blocking:             "dns",
+			DNSExperimentFailure:  "android_dns_cache_no_data",
+			HTTPExperimentFailure: "android_dns_cache_no_data",
+			DNSConsistency:        "inconsistent",
+			XStatus:               2080, // StatusExperimentDNS | StatusAnomalyDNS
+			XDNSFlags:             2,    // AnalysisDNSFlagUnexpectedFailure
+			XBlockingFlags:        33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
+			Accessible:            false,
+			Blocking:              "dns",
 		},
 	}
 }
@@ -44,21 +45,22 @@ func dnsBlockingNXDOMAIN() *TestCase {
 	*/
 	return &TestCase{
 		Name:  "dnsBlockingNXDOMAIN",
-		Flags: 0,
+		Flags: TestCaseFlagNoV04,
 		Input: "https://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// remove the record so that the DNS query returns NXDOMAIN
 			env.ISPResolverConfig().RemoveRecord("www.example.com")
 		},
 		ExpectErr: false,
 		ExpectTestKeys: &testKeys{
-			DNSExperimentFailure: "dns_nxdomain_error",
-			DNSConsistency:       "inconsistent",
-			XStatus:              2080, // StatusExperimentDNS | StatusAnomalyDNS
-			XDNSFlags:            2,    // AnalysisDNSFlagUnexpectedFailure
-			XBlockingFlags:       33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
-			Accessible:           false,
-			Blocking:             "dns",
+			DNSExperimentFailure:  "dns_nxdomain_error",
+			HTTPExperimentFailure: "dns_nxdomain_error",
+			DNSConsistency:        "inconsistent",
+			XStatus:               2080, // StatusExperimentDNS | StatusAnomalyDNS
+			XDNSFlags:             2,    // AnalysisDNSFlagUnexpectedFailure
+			XBlockingFlags:        33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
+			Accessible:            false,
+			Blocking:              "dns",
 		},
 	}
 }

internal/experiment/webconnectivityqa/ghost.go

@@ -0,0 +1,81 @@
+package webconnectivityqa
+
+import (
+	"github.com/apex/log"
+	"github.com/ooni/netem"
+	"github.com/ooni/probe-cli/v3/internal/netemx"
+)
+
+// ghostDNSBlockingWithHTTP is the case where the domain does not exist anymore but
+// there's still ghost censorship because of the system resolver configuration, which
+// says that we should censor the domain by returning a specific IP address.
+//
+// See https://github.com/ooni/probe/issues/2308.
+func ghostDNSBlockingWithHTTP() *TestCase {
+	return &TestCase{
+		Name:  "ghostDNSBlockingWithHTTP",
+		Flags: TestCaseFlagNoV04,
+		Input: "http://itsat.info/",
+		Configure: func(env *netemx.QAEnv) {
+			// remove the record so that the DNS query returns NXDOMAIN
+			env.ISPResolverConfig().RemoveRecord("itsat.info")
+			env.OtherResolversConfig().RemoveRecord("itsat.info")
+
+			// however introduce a rule causing DNS to respond to the query
+			env.DPIEngine().AddRule(&netem.DPISpoofDNSResponse{
+				Addresses: []string{
+					netemx.AddressPublicBlockpage,
+				},
+				Logger: log.Log,
+				Domain: "itsat.info",
+			})
+		},
+		ExpectErr: false,
+		ExpectTestKeys: &testKeys{
+			DNSExperimentFailure: nil,
+			DNSConsistency:       "inconsistent",
+			XBlockingFlags:       16, // AnalysisBlockingFlagHTTPDiff
+			XNullNullFlags:       16, // AnalysisFlagNullNullUnexpectedDNSLookupSuccess
+			XStatus:              16, // StatusAnomalyControlFailure
+			Accessible:           false,
+			Blocking:             "dns",
+		},
+	}
+}
+
+// ghostDNSBlockingWithHTTPS is the case where the domain does not exist anymore but
+// there's still ghost censorship because of the system resolver configuration, which
+// says that we should censor the domain by returning a specific IP address.
+//
+// See https://github.com/ooni/probe/issues/2308.
+func ghostDNSBlockingWithHTTPS() *TestCase {
+	return &TestCase{
+		Name:  "ghostDNSBlockingWithHTTPS",
+		Flags: 0,
+		Input: "https://itsat.info/",
+		Configure: func(env *netemx.QAEnv) {
+			// remove the record so that the DNS query returns NXDOMAIN
+			env.ISPResolverConfig().RemoveRecord("itsat.info")
+			env.OtherResolversConfig().RemoveRecord("itsat.info")
+
+			// however introduce a rule causing DNS to respond to the query
+			env.DPIEngine().AddRule(&netem.DPISpoofDNSResponse{
+				Addresses: []string{
+					netemx.AddressPublicBlockpage,
+				},
+				Logger: log.Log,
+				Domain: "itsat.info",
+			})
+		},
+		ExpectErr: false,
+		ExpectTestKeys: &testKeys{
+			DNSExperimentFailure:  nil,
+			DNSConsistency:        "inconsistent",
+			HTTPExperimentFailure: "connection_refused",
+			XNullNullFlags:        16,   // AnalysisFlagNullNullUnexpectedDNSLookupSuccess
+			XStatus:               4256, // StatusExperimentConnect | StatusAnomalyDNS | StatusAnomalyConnect
+			Accessible:            false,
+			Blocking:              "dns",
+		},
+	}
+}

internal/experiment/webconnectivityqa/testcase.go

@@ -52,6 +52,9 @@ func AllTestCases() []*TestCase {
 		dnsHijackingToProxyWithHTTPURL(),
 		dnsHijackingToProxyWithHTTPSURL(),
 
+		ghostDNSBlockingWithHTTP(),
+		ghostDNSBlockingWithHTTPS(),
+
 		httpBlockingConnectionReset(),
 
 		httpDiffWithConsistentDNS(),

internal/experiment/webconnectivityqa/websitedown.go

@@ -21,18 +21,19 @@ func websiteDownNXDOMAIN() *TestCase {
 	*/
 	return &TestCase{
 		Name:      "websiteDownNXDOMAIN",
-		Flags:     0,                         // see above
+		Flags:     TestCaseFlagNoV04,
 		Input:     "http://www.example.xyz/", // domain not defined in the simulation
 		Configure: nil,
 		ExpectErr: false,
 		ExpectTestKeys: &testKeys{
-			DNSExperimentFailure: "dns_nxdomain_error",
-			DNSConsistency:       "consistent",
-			XStatus:              2052, // StatusExperimentDNS | StatusSuccessNXDOMAIN
-			XBlockingFlags:       0,
-			XNullNullFlags:       1, // analysisFlagNullNullNoAddrs
-			Accessible:           true,
-			Blocking:             false,
+			DNSExperimentFailure:  "dns_nxdomain_error",
+			HTTPExperimentFailure: "dns_nxdomain_error",
+			DNSConsistency:        "consistent",
+			XStatus:               2052, // StatusExperimentDNS | StatusSuccessNXDOMAIN
+			XBlockingFlags:        0,
+			XNullNullFlags:        1, // analysisFlagNullNullNoAddrs
+			Accessible:            false,
+			Blocking:              false,
 		},
 	}
 }