diff --git a/internal/experiment/webconnectivitylte/analysisclassic.go b/internal/experiment/webconnectivitylte/analysisclassic.go index 4f96e4be21..22e2048bf9 100644 --- a/internal/experiment/webconnectivitylte/analysisclassic.go +++ b/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 . -func analysisClassicSetHTTPExperimentFailureDNS(tk analysisClassicTestKeysProxy, entry *minipipeline.WebObservation) { - if entry.TagDepth.UnwrapOr(0) <= 0 { - return - } - tk.setHTTPExperimentFailure(entry.Failure) -} diff --git a/internal/experiment/webconnectivitylte/analysisext.go b/internal/experiment/webconnectivitylte/analysisext.go index 972d75af45..f23d359cd1 100644 --- a/internal/experiment/webconnectivitylte/analysisext.go +++ b/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 { diff --git a/internal/experiment/webconnectivityqa/badssl.go b/internal/experiment/webconnectivityqa/badssl.go index 17e6efc5fc..8f6a321216 100644 --- a/internal/experiment/webconnectivityqa/badssl.go +++ b/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, }, } } diff --git a/internal/experiment/webconnectivityqa/dnsblocking.go b/internal/experiment/webconnectivityqa/dnsblocking.go index 207c248a92..3145ee22ac 100644 --- a/internal/experiment/webconnectivityqa/dnsblocking.go +++ b/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,7 +45,7 @@ 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 @@ -52,13 +53,14 @@ func dnsBlockingNXDOMAIN() *TestCase { }, 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", }, } } diff --git a/internal/experiment/webconnectivityqa/ghost.go b/internal/experiment/webconnectivityqa/ghost.go new file mode 100644 index 0000000000..ed3143229f --- /dev/null +++ b/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 censor DNS censoring 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 censor DNS censoring 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", + }, + } +} diff --git a/internal/experiment/webconnectivityqa/testcase.go b/internal/experiment/webconnectivityqa/testcase.go index 0b0b0b7b02..70151a869f 100644 --- a/internal/experiment/webconnectivityqa/testcase.go +++ b/internal/experiment/webconnectivityqa/testcase.go @@ -52,6 +52,9 @@ func AllTestCases() []*TestCase { dnsHijackingToProxyWithHTTPURL(), dnsHijackingToProxyWithHTTPSURL(), + ghostDNSBlockingWithHTTP(), + ghostDNSBlockingWithHTTPS(), + httpBlockingConnectionReset(), httpDiffWithConsistentDNS(), diff --git a/internal/experiment/webconnectivityqa/websitedown.go b/internal/experiment/webconnectivityqa/websitedown.go index aee748d52e..e5982db9d0 100644 --- a/internal/experiment/webconnectivityqa/websitedown.go +++ b/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, }, } }