feat: make sure LTE and v0.4 emit the same test keys (#1392)

This diff switches LTE to use the classic "engine", removes restrictions
regarding comparing test keys, and ensures with the existing test suite
that LTE and v0.4 produce the same top-level test keys under the same
conditions.

While there, let's also bump LTE's version number, lest we forget.

See ooni/probe#2634

internal/experiment/webconnectivitylte/analysisclassic.go

@@ -41,10 +41,16 @@ func (tk *TestKeys) analysisClassic(logger model.Logger) {
 	// 4. determine the DNS consistency
 	tk.DNSConsistency = analysisClassicDNSConsistency(woa)
 
-	// 5. compute the HTTPDiff values
+	// 5. set DNSExperimentFailure
+	if !woa.DNSExperimentFailure.IsNone() && woa.DNSExperimentFailure.Unwrap() != "" {
+		value := woa.DNSExperimentFailure.Unwrap()
+		tk.DNSExperimentFailure = &value
+	}
+
+	// 6. compute the HTTPDiff values
 	tk.setHTTPDiffValues(woa)
 
-	// 6. compute blocking & accessible
+	// 7. compute blocking & accessible
 	analysisClassicComputeBlockingAccessible(woa, tk)
 }
 
@@ -68,7 +74,8 @@ func analysisClassicDNSConsistency(woa *minipipeline.WebAnalysis) optional.Value
 func (tk *TestKeys) setHTTPDiffValues(woa *minipipeline.WebAnalysis) {
 	const bodyProportionFactor = 0.7
 	if !woa.HTTPFinalResponseDiffBodyProportionFactor.IsNone() {
-		value := woa.HTTPFinalResponseDiffBodyProportionFactor.Unwrap() > bodyProportionFactor
+		tk.BodyProportion = woa.HTTPFinalResponseDiffBodyProportionFactor.Unwrap()
+		value := tk.BodyProportion > bodyProportionFactor
 		tk.BodyLengthMatch = &value
 	}
 
@@ -89,6 +96,9 @@ func (tk *TestKeys) setHTTPDiffValues(woa *minipipeline.WebAnalysis) {
 }
 
 type analysisClassicTestKeysProxy interface {
+	// httpDiff returns true if there's an http-diff.
+	httpDiff() bool
+
 	// setBlockingString sets blocking to a string.
 	setBlockingString(value string)
 
@@ -98,8 +108,8 @@ type analysisClassicTestKeysProxy interface {
 	// setBlockingFalse sets Blocking to false.
 	setBlockingFalse()
 
-	// httpDiff returns true if there's an http-diff.
-	httpDiff() bool
+	// setHTTPExperimentFailure sets the HTTPExperimentFailure field.
+	setHTTPExperimentFailure(value optional.Value[string])
 }
 
 var _ analysisClassicTestKeysProxy = &TestKeys{}
@@ -148,6 +158,11 @@ func (tk *TestKeys) setBlockingString(value string) {
 	tk.Accessible = false
 }
 
+// setHTTPExperimentFailure implements analysisClassicTestKeysProxy.
+func (tk *TestKeys) setHTTPExperimentFailure(value optional.Value[string]) {
+	tk.HTTPExperimentFailure = value
+}
+
 func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk analysisClassicTestKeysProxy) {
 	// minipipeline.NewLinearWebAnalysis produces a woa.Linear sorted
 	//
@@ -217,6 +232,7 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 			// the control server is unreachable or blocked.
 			if entry.ControlHTTPFailure.IsNone() {
 				tk.setBlockingNil()
+				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
@@ -226,11 +242,13 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// should also set Accessible to false. However, v0.4
 				// does this and we should play along for the A/B testing.
 				tk.setBlockingFalse()
+				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
 			// 2.3. Handle the case where just the probe failed.
 			tk.setBlockingString("http-failure")
+			tk.setHTTPExperimentFailure(entry.Failure)
 			return
 		}
 
@@ -248,11 +266,13 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// the control accessing the website and telling us.
 				if entry.ControlHTTPFailure.IsNone() {
 					tk.setBlockingNil()
+					tk.setHTTPExperimentFailure(entry.Failure)
 					return
 				}
 
 				// 3.1.2. Otherwise, if the control worked, that's blocking.
 				tk.setBlockingString("http-failure")
+				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
@@ -262,11 +282,13 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// 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.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
 			// 3.3. Handle the case where just the probe failed.
 			tk.setBlockingString("http-failure")
+			tk.setHTTPExperimentFailure(entry.Failure)
 			return
 		}
 
@@ -281,11 +303,13 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// accessing the website should lead to.
 				if entry.ControlHTTPFailure.IsNone() {
 					tk.setBlockingNil()
+					tk.setHTTPExperimentFailure(entry.Failure)
 					return
 				}
 
 				// 4.1.2. Otherwise, if the control worked, that's blocking.
 				tk.setBlockingString("http-failure")
+				tk.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
@@ -295,11 +319,13 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// 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.setHTTPExperimentFailure(entry.Failure)
 				return
 			}
 
 			// 4.3. Handle the case where just the probe failed.
 			tk.setBlockingString("tcp_ip")
+			tk.setHTTPExperimentFailure(entry.Failure)
 			return
 		}
 
@@ -314,11 +340,13 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// accessing the website should lead to.
 				if entry.ControlHTTPFailure.IsNone() {
 					tk.setBlockingFalse()
+					analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
 					return
 				}
 
 				// 5.1.2. Otherwise, if the control worked, that's blocking.
 				tk.setBlockingString("dns")
+				analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
 				return
 			}
 
@@ -328,12 +356,24 @@ func analysisClassicComputeBlockingAccessible(woa *minipipeline.WebAnalysis, tk
 				// 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)
 				return
 			}
 
 			// 5.3. Handle the case where just the probe failed.
 			tk.setBlockingString("dns")
+			analysisClassicSetHTTPExperimentFailureDNS(tk, entry)
 			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/analysiscore.go

@@ -38,7 +38,7 @@ const (
 
 // AnalysisEngineFn is the function that runs the analysis engine for
 // processing and scoring measurements collected by LTE.
-var AnalysisEngineFn func(tk *TestKeys, logger model.Logger) = AnalysisEngineOrig
+var AnalysisEngineFn func(tk *TestKeys, logger model.Logger) = AnalysisEngineClassic
 
 // analysisToplevel is the toplevel function that analyses the results
 // of the experiment once all network tasks have completed.

internal/experiment/webconnectivitylte/analysishttpcore.go

@@ -7,6 +7,7 @@ package webconnectivitylte
 import (
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
+	"github.com/ooni/probe-cli/v3/internal/optional"
 )
 
 // analysisHTTPToplevel is the toplevel analysis function for HTTP results.
@@ -31,7 +32,9 @@ func (tk *TestKeys) analysisHTTPToplevel(logger model.Logger) {
 		return
 	}
 	finalRequest := tk.Requests[0]
-	tk.HTTPExperimentFailure = finalRequest.Failure
+	if finalRequest.Failure != nil {
+		tk.HTTPExperimentFailure = optional.Some(*finalRequest.Failure)
+	}
 
 	// don't perform any futher analysis without TH data
 	if tk.Control == nil || tk.ControlRequest == nil {

internal/experiment/webconnectivitylte/measurer.go

@@ -37,7 +37,7 @@ func (m *Measurer) ExperimentName() string {
 
 // ExperimentVersion implements model.ExperimentMeasurer.
 func (m *Measurer) ExperimentVersion() string {
-	return "0.5.26"
+	return "0.5.27"
 }
 
 // Run implements model.ExperimentMeasurer.

internal/experiment/webconnectivitylte/testkeys.go

@@ -86,7 +86,7 @@ type TestKeys struct {
 
 	// HTTPExperimentFailure indicates whether there was a failure in
 	// the final HTTP request that we recorded.
-	HTTPExperimentFailure *string `json:"http_experiment_failure"`
+	HTTPExperimentFailure optional.Value[string] `json:"http_experiment_failure"`
 
 	// BlockingFlags explains why we think that the website is blocked.
 	BlockingFlags int64 `json:"x_blocking_flags"`
@@ -95,6 +95,9 @@ type TestKeys struct {
 	// blocking = null, accessible = null measurements did
 	NullNullFlags int64 `json:"x_null_null_flags"`
 
+	// BodyProportion is the value used to compute BodyLength.
+	BodyProportion float64 `json:"body_proportion"`
+
 	// BodyLength match tells us whether the body length matches.
 	BodyLengthMatch *bool `json:"body_length_match"`
 
@@ -356,9 +359,10 @@ func NewTestKeys() *TestKeys {
 		DNSFlags:              0,
 		DNSExperimentFailure:  nil,
 		DNSConsistency:        optional.None[string](),
-		HTTPExperimentFailure: nil,
+		HTTPExperimentFailure: optional.None[string](),
 		BlockingFlags:         0,
 		NullNullFlags:         0,
+		BodyProportion:        0,
 		BodyLengthMatch:       nil,
 		HeadersMatch:          nil,
 		StatusCodeMatch:       nil,

internal/experiment/webconnectivityqa/badssl.go

@@ -10,7 +10,7 @@ import (
 func badSSLWithExpiredCertificate() *TestCase {
 	return &TestCase{
 		Name:  "badSSLWithExpiredCertificate",
-		Flags: TestCaseFlagNoLTE, // LTE flags it correctly but let's focus on v0.4 for now
+		Flags: 0,
 		Input: "https://expired.badssl.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// nothing
@@ -32,7 +32,7 @@ func badSSLWithExpiredCertificate() *TestCase {
 func badSSLWithWrongServerName() *TestCase {
 	return &TestCase{
 		Name:  "badSSLWithWrongServerName",
-		Flags: TestCaseFlagNoLTE, // LTE flags it correctly but let's focus on v0.4 for now
+		Flags: 0,
 		Input: "https://wrong.host.badssl.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// nothing
@@ -53,7 +53,7 @@ func badSSLWithWrongServerName() *TestCase {
 func badSSLWithUnknownAuthorityWithConsistentDNS() *TestCase {
 	return &TestCase{
 		Name:  "badSSLWithUnknownAuthorityWithConsistentDNS",
-		Flags: TestCaseFlagNoLTE, // LTE flags it correctly but let's focus on v0.4 for now
+		Flags: 0,
 		Input: "https://untrusted-root.badssl.com/",
 		Configure: func(env *netemx.QAEnv) {
 			// nothing
@@ -79,7 +79,7 @@ func badSSLWithUnknownAuthorityWithInconsistentDNS() *TestCase {
 		Configure: func(env *netemx.QAEnv) {
 
 			// add DPI rule to force all the cleartext DNS queries to
-			// point the client to used the ISPProxyAddress
+			// point the client to use the ISPProxyAddress
 			env.DPIEngine().AddRule(&netem.DPISpoofDNSResponse{
 				Addresses: []string{netemx.AddressBadSSLCom},
 				Logger:    env.Logger(),

internal/experiment/webconnectivityqa/control.go

@@ -11,7 +11,7 @@ import (
 func controlFailureWithSuccessfulHTTPWebsite() *TestCase {
 	return &TestCase{
 		Name:  "controlFailureWithSuccessfulHTTPWebsite",
-		Flags: TestCaseFlagNoLTE, // BUG: has "consistent" DNS but still blocking=null and accessible=null
+		Flags: 0,
 		Input: "http://www.example.org/",
 		Configure: func(env *netemx.QAEnv) {
 
@@ -56,7 +56,7 @@ func controlFailureWithSuccessfulHTTPWebsite() *TestCase {
 func controlFailureWithSuccessfulHTTPSWebsite() *TestCase {
 	return &TestCase{
 		Name:  "controlFailureWithSuccessfulHTTPSWebsite",
-		Flags: TestCaseFlagNoLTE, // because it (correctly!) sets the DNS as consistent thanks to TLS
+		Flags: 0,
 		Input: "https://www.example.org/",
 		Configure: func(env *netemx.QAEnv) {
 

internal/experiment/webconnectivityqa/dnsblocking.go

@@ -47,8 +47,7 @@ func dnsBlockingNXDOMAIN() *TestCase {
 		Flags: 0,
 		Input: "https://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
-			// remove the record so that the DNS query returns NXDOMAIN, which is then
-			// converted into android_dns_cache_no_data by the emulation layer
+			// remove the record so that the DNS query returns NXDOMAIN
 			env.ISPResolverConfig().RemoveRecord("www.example.com")
 		},
 		ExpectErr: false,
@@ -68,7 +67,7 @@ func dnsBlockingNXDOMAIN() *TestCase {
 func dnsBlockingBOGON() *TestCase {
 	return &TestCase{
 		Name:  "dnsBlockingBOGON",
-		Flags: TestCaseFlagNoLTE, // We're not ready yet
+		Flags: 0,
 		Input: "https://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 			env.ISPResolverConfig().RemoveRecord("www.example.com")

internal/experiment/webconnectivityqa/dnshijacking.go

@@ -12,7 +12,7 @@ func dnsHijackingToProxyWithHTTPURL() *TestCase {
 	// transparent TLS proxy really makes our analysis a bit more complex
 	return &TestCase{
 		Name:  "dnsHijackingToProxyWithHTTPURL",
-		Flags: TestCaseFlagNoLTE, // BUG: LTE thinks the DNS is consistent
+		Flags: 0,
 		Input: "http://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 
@@ -49,7 +49,7 @@ func dnsHijackingToProxyWithHTTPSURL() *TestCase {
 	// transparent TLS proxy really makes our analysis a bit more complex
 	return &TestCase{
 		Name:  "dnsHijackingToProxyWithHTTPSURL",
-		Flags: TestCaseFlagNoLTE, // BUG: LTE thinks the DNS is consistent
+		Flags: 0,
 		Input: "https://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 

internal/experiment/webconnectivityqa/httpdiff.go

@@ -11,7 +11,7 @@ import (
 func httpDiffWithConsistentDNS() *TestCase {
 	return &TestCase{
 		Name:  "httpDiffWithConsistentDNS",
-		Flags: TestCaseFlagNoLTE, // BUG: LTE does not set whether the headers match
+		Flags: 0,
 		Input: "http://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 
@@ -49,12 +49,12 @@ func httpDiffWithConsistentDNS() *TestCase {
 func httpDiffWithInconsistentDNS() *TestCase {
 	return &TestCase{
 		Name:  "httpDiffWithInconsistentDNS",
-		Flags: TestCaseFlagNoLTE, // BUG: LTE does not detect any HTTP diff here
+		Flags: 0,
 		Input: "http://www.example.com/",
 		Configure: func(env *netemx.QAEnv) {
 
 			// add DPI rule to force all the cleartext DNS queries to
-			// point the client to used the ISPProxyAddress
+			// point the client to use the ISPProxyAddress
 			env.DPIEngine().AddRule(&netem.DPISpoofDNSResponse{
 				Addresses: []string{netemx.ISPProxyAddress},
 				Logger:    env.Logger(),