feat(webconnectivitylte): classic supports XNullNullFlags (#1454)

This diff extends webconnectivitylte's classic analysis engine and
minipipeline to generate `XNullNullFlags`.

With this change implemented, we have successfully replaced the "orig"
engine with "classic".

This work is part of ooni/probe#2640.

While this diff is large, most of the changes are in the generated files
for the minipipeline test suite.

internal/cmd/minipipeline/testdata/analysis.json

@@ -1,7 +1,15 @@
 {
-  "ControlFinalResponseExpectations": {
-    "Failure": ""
+  "ControlExpectations": {
+    "DNSAddresses": [
+      "130.192.16.171"
+    ],
+    "FinalResponseFailure": ""
   },
+  "DNSLookupSuccess": [
+    1,
+    2,
+    3
+  ],
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithValidAddress": [
     1,
@@ -20,12 +28,14 @@
   "DNSExperimentFailure": null,
   "DNSLookupExpectedFailure": [],
   "DNSLookupExpectedSuccess": [],
+  "TCPConnectExpectedFailure": [],
   "TCPConnectUnexpectedFailure": [],
   "TCPConnectUnexpectedFailureDuringWebFetch": [],
   "TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
   "TCPConnectUnexplainedFailure": [],
   "TCPConnectUnexplainedFailureDuringWebFetch": [],
   "TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
+  "TLSHandshakeExpectedFailure": [],
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],

internal/cmd/minipipeline/testdata/analysis_classic.json

@@ -1,7 +1,13 @@
 {
-  "ControlFinalResponseExpectations": {
-    "Failure": ""
+  "ControlExpectations": {
+    "DNSAddresses": [
+      "130.192.16.171"
+    ],
+    "FinalResponseFailure": ""
   },
+  "DNSLookupSuccess": [
+    2
+  ],
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithValidAddress": [
     1,
@@ -17,12 +23,14 @@
   "DNSExperimentFailure": null,
   "DNSLookupExpectedFailure": [],
   "DNSLookupExpectedSuccess": [],
+  "TCPConnectExpectedFailure": [],
   "TCPConnectUnexpectedFailure": [],
   "TCPConnectUnexpectedFailureDuringWebFetch": [],
   "TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
   "TCPConnectUnexplainedFailure": [],
   "TCPConnectUnexplainedFailureDuringWebFetch": [],
   "TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
+  "TLSHandshakeExpectedFailure": [],
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],

internal/cmd/minipipeline/testdata/observations.json

@@ -382,7 +382,10 @@
       "ControlHTTPResponseTitle": "Nexa Center for Internet \u0026 Society | Il centro Nexa è un centro di ricerca del Dipartimento di Automatica e Informatica del Politecnico di Torino"
     }
   },
-  "ControlFinalResponseExpectations": {
-    "Failure": ""
+  "ControlExpectations": {
+    "DNSAddresses": [
+      "130.192.16.171"
+    ],
+    "FinalResponseFailure": ""
   }
 }

internal/cmd/minipipeline/testdata/observations_classic.json

@@ -141,7 +141,10 @@
       "ControlHTTPResponseTitle": "Nexa Center for Internet \u0026 Society | Il centro Nexa è un centro di ricerca del Dipartimento di Automatica e Informatica del Politecnico di Torino"
     }
   },
-  "ControlFinalResponseExpectations": {
-    "Failure": ""
+  "ControlExpectations": {
+    "DNSAddresses": [
+      "130.192.16.171"
+    ],
+    "FinalResponseFailure": ""
   }
 }

internal/experiment/webconnectivitylte/analysiscore.go

@@ -234,28 +234,25 @@ func (tk *TestKeys) analysisOrig(logger model.Logger) {
 }
 
 const (
-	// analysisFlagNullNullNoAddrs indicates neither the probe nor the TH were
-	// able to get any IP addresses from any resolver.
-	analysisFlagNullNullNoAddrs = 1 << iota
-
-	// analysisFlagNullNullAllConnectsFailed indicates that all the connect
+	// AnalysisFlagNullNullExpectedDNSLookupFailure indicates some of the DNS lookup
 	// attempts failed both in the probe and in the test helper.
-	analysisFlagNullNullAllConnectsFailed
+	AnalysisFlagNullNullExpectedDNSLookupFailure = 1 << iota
 
-	// analysisFlagNullNullTLSMisconfigured indicates that all the TLS handshake
+	// AnalysisFlagNullNullExpectedTCPConnectFailure indicates that some of the connect
 	// attempts failed both in the probe and in the test helper.
-	analysisFlagNullNullTLSMisconfigured
+	AnalysisFlagNullNullExpectedTCPConnectFailure
+
+	// AnalysisFlagNullNullExpectedTLSHandshakeFailure indicates that we have seen some TLS
+	// handshakes failing consistently for both the probe and the TH.
+	AnalysisFlagNullNullExpectedTLSHandshakeFailure
 
-	// analysisFlagNullNullSuccessfulHTTPS indicates that we had no TH data
+	// AnalysisFlagNullNullSuccessfulHTTPS indicates that we had no TH data
 	// but all the HTTP requests used always HTTPS and never failed.
-	analysisFlagNullNullSuccessfulHTTPS
-
-	// analysisFlagNullNullNXDOMAINWithCensorship indicates that we have
-	// seen no error with local DNS resolutions but, at the same time, the
-	// control failed with NXDOMAIN. When this happens, we probably have
-	// DNS interception locally, so all cleartext queries return the same
-	// bogus answers based on a rule applied on a now-expired domain.
-	analysisFlagNullNullNXDOMAINWithCensorship
+	AnalysisFlagNullNullSuccessfulHTTPS
+
+	// AnalysisFlagNullNullUnexpectedDNSLookupSuccess indicates the case
+	// where the TH resolved no addresses while the probe did.
+	AnalysisFlagNullNullUnexpectedDNSLookupSuccess
 )
 
 // analysisNullNullDetectTHDNSNXDOMAIN runs when .Blocking = nil and
@@ -303,7 +300,7 @@ func (tk *TestKeys) analysisNullNullDetectTHDNSNXDOMAIN(logger model.Logger) boo
 	failure := tk.Control.DNS.Failure
 	if failure != nil && *failure == model.THDNSNameError {
 		logger.Info("DNS censorship: local DNS success with remote NXDOMAIN")
-		tk.NullNullFlags |= analysisFlagNullNullNXDOMAINWithCensorship
+		tk.NullNullFlags |= AnalysisFlagNullNullUnexpectedDNSLookupSuccess
 		return true
 	}
 
@@ -358,7 +355,7 @@ func (tk *TestKeys) analysisNullNullDetectSuccessfulHTTPS(logger model.Logger) b
 	// only if we have at least one request
 	if len(tk.Requests) > 0 {
 		logger.Info("website likely accessible: seen successful chain of HTTPS transactions")
-		tk.NullNullFlags |= analysisFlagNullNullSuccessfulHTTPS
+		tk.NullNullFlags |= AnalysisFlagNullNullSuccessfulHTTPS
 		return true
 	}
 
@@ -406,7 +403,7 @@ func (tk *TestKeys) analysisNullNullDetectTLSMisconfigured(logger model.Logger)
 	// only if we have had some TLS handshakes for both probe and TH
 	if len(tk.TLSHandshakes) > 0 && len(tk.Control.TLSHandshake) > 0 {
 		logger.Info("website likely down: all TLS handshake attempts failed for both probe and TH")
-		tk.NullNullFlags |= analysisFlagNullNullTLSMisconfigured
+		tk.NullNullFlags |= AnalysisFlagNullNullExpectedTLSHandshakeFailure
 		return true
 	}
 
@@ -448,7 +445,7 @@ func (tk *TestKeys) analysisNullNullDetectAllConnectsFailed(logger model.Logger)
 	// only if we have had some addresses to connect
 	if len(tk.TCPConnect) > 0 && len(tk.Control.TCPConnect) > 0 {
 		logger.Info("website likely down: all TCP connect attempts failed for both probe and TH")
-		tk.NullNullFlags |= analysisFlagNullNullAllConnectsFailed
+		tk.NullNullFlags |= AnalysisFlagNullNullExpectedTCPConnectFailure
 		return true
 	}
 
@@ -498,6 +495,6 @@ func (tk *TestKeys) analysisNullNullDetectNoAddrs(logger model.Logger) bool {
 		return false
 	}
 	logger.Infof("website likely down: all DNS lookups failed for both probe and TH")
-	tk.NullNullFlags |= analysisFlagNullNullNoAddrs
+	tk.NullNullFlags |= AnalysisFlagNullNullExpectedDNSLookupFailure
 	return true
 }

internal/experiment/webconnectivitylte/analysisext.go

@@ -40,7 +40,9 @@ func analysisExtMain(tk *TestKeys, container *minipipeline.WebObservationsContai
 	// HTTP success analysis (i.e., only if we manage to get an HTTP response)
 	analysisExtHTTPFinalResponse(tk, analysis, &info)
 
-	// TODO(bassosimone): we need to also compute the null-null flags here
+	// handle the cases where the probe and the TH both failed, which we can confidently
+	// only evaluate for DNS, TCP, and TLS during the 0-th redirect.
+	analysisExtExpectedFailures(tk, analysis, &info)
 
 	// print the content of the analysis only if there's some content to print
 	if content := info.String(); content != "" {
@@ -120,6 +122,7 @@ func analysisExtHTTPFinalResponse(tk *TestKeys, analysis *minipipeline.WebAnalys
 	// this is automatic success.
 	if success := analysis.HTTPFinalResponseSuccessTLSWithoutControl; !success.IsNone() {
 		fmt.Fprintf(info, "- the final response (transaction: %d) uses TLS: automatic success\n", success.Unwrap())
+		tk.NullNullFlags |= AnalysisFlagNullNullSuccessfulHTTPS
 		tk.BlockingFlags |= AnalysisBlockingFlagSuccess
 		return
 	}
@@ -174,14 +177,14 @@ func analysisExtRedirectErrors(tk *TestKeys, analysis *minipipeline.WebAnalysis,
 	}
 
 	// we care about cases in which the TH succeeded
-	if analysis.ControlFinalResponseExpectations.IsNone() {
+	if analysis.ControlExpectations.IsNone() {
 		return
 	}
-	expect := analysis.ControlFinalResponseExpectations.Unwrap()
-	if expect.Failure.IsNone() {
+	expect := analysis.ControlExpectations.Unwrap()
+	if expect.FinalResponseFailure.IsNone() {
 		return
 	}
-	if expect.Failure.Unwrap() != "" {
+	if expect.FinalResponseFailure.Unwrap() != "" {
 		return
 	}
 
@@ -221,3 +224,57 @@ func analysisExtRedirectErrors(tk *TestKeys, analysis *minipipeline.WebAnalysis,
 		)
 	}
 }
+
+func analysisExtExpectedFailures(tk *TestKeys, analysis *minipipeline.WebAnalysis, info io.Writer) {
+	// Implementation note: in the "orig" engine this was called the "null-null" analysis
+	// because it aimed to address the cases in which failure and accessible were both set
+	// to null. We're keeping the original name so we can also keep the same flag we were
+	// using before. The flag is a "x_" kind of flag anyway.
+	//
+	// Also note that these cases ARE NOT MUTUALLY EXCLUSIVE meaning that these conditions
+	// could actually happen simultaneously in a bunch of cases.
+
+	if expected := analysis.DNSLookupExpectedFailure; expected.Len() > 0 {
+		tk.NullNullFlags |= AnalysisFlagNullNullExpectedDNSLookupFailure
+		fmt.Fprintf(
+			info, "- transactions with expected DNS lookup failures: %s\n",
+			expected.String(),
+		)
+	}
+
+	if expected := analysis.TCPConnectExpectedFailure; expected.Len() > 0 {
+		tk.NullNullFlags |= AnalysisFlagNullNullExpectedTCPConnectFailure
+		fmt.Fprintf(
+			info, "- transactions with expected TCP connect failures: %s\n",
+			expected.String(),
+		)
+	}
+
+	if expected := analysis.TLSHandshakeExpectedFailure; expected.Len() > 0 {
+		tk.NullNullFlags |= AnalysisFlagNullNullExpectedTLSHandshakeFailure
+		fmt.Fprintf(
+			info, "- transactions with expected TLS handshake failures: %s\n",
+			expected.String(),
+		)
+	}
+
+	// Note: the following flag
+	//
+	//	tk.NullNullFlags |= AnalysisFlagNullNullSuccessfulHTTPS
+	//
+	// is set by analysisExtHTTPFinalResponse
+
+	// 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
+	if !analysis.ControlExpectations.IsNone() {
+		expect := analysis.ControlExpectations.Unwrap()
+		if expect.DNSAddresses.Len() <= 0 && analysis.DNSLookupSuccess.Len() > 0 {
+			tk.NullNullFlags |= AnalysisFlagNullNullUnexpectedDNSLookupSuccess
+			fmt.Fprintf(
+				info, "- transactions that unexpectedly resolved IP addresses: %s\n",
+				analysis.DNSLookupSuccess.String(),
+			)
+		}
+	}
+}

internal/experiment/webconnectivityqa/badssl.go

@@ -94,6 +94,7 @@ func badSSLWithUnknownAuthorityWithInconsistentDNS() *TestCase {
 			XStatus:               9248, // StatusExperimentHTTP | StatusAnomalyTLSHandshake | StatusAnomalyDNS
 			XDNSFlags:             4,    // AnalysisDNSFlagUnexpectedAddrs
 			XBlockingFlags:        33,   // AnalysisBlockingFlagSuccess | AnalysisBlockingFlagDNSBlocking
+			XNullNullFlags:        4,    // AnalysisFlagNullNullExpectedTLSHandshakeFailure
 			Accessible:            false,
 			Blocking:              "dns",
 		},

internal/experiment/webconnectivityqa/testkeys.go

@@ -77,9 +77,6 @@ func compareTestKeys(expected, got *testKeys) error {
 		// ignore the fields that are specific to v0.4
 		options = append(options, cmpopts.IgnoreFields(testKeys{}, "XStatus"))
 
-		// TODO(bassosimone): ignore fields used by the v0.5 "orig" local analysis engine
-		options = append(options, cmpopts.IgnoreFields(testKeys{}, "XNullNullFlags"))
-
 	default:
 		return fmt.Errorf("unknown experiment version: %s", got.XExperimentVersion)
 	}

internal/minipipeline/analysis.go

@@ -96,7 +96,7 @@ func NewLinearWebAnalysis(input *WebObservationsContainer) (output []*WebObserva
 // but avoids calling [NewLinearyAnalysis] to generate a linear analysis.
 func AnalyzeWebObservationsWithoutLinearAnalysis(container *WebObservationsContainer) *WebAnalysis {
 	analysis := &WebAnalysis{
-		ControlFinalResponseExpectations: container.ControlFinalResponseExpectations,
+		ControlExpectations: container.ControlExpectations,
 	}
 
 	analysis.dnsComputeSuccessMetrics(container)
@@ -123,10 +123,14 @@ func AnalyzeWebObservationsWithLinearAnalysis(container *WebObservationsContaine
 //
 // The zero value of this struct is ready to use.
 type WebAnalysis struct {
-	// ControlFinalResponseExpectations summarizes the expectations we have
-	// for the control based on the final response. You should use this field
-	// to determine whether unexplained failures are expected or unexpected.
-	ControlFinalResponseExpectations optional.Value[*WebObservationsControlFinalResponseExpectation]
+	// ControlExpectations summarizes the expectations we have for the control. You should
+	// use this field to determine (a) whether unexplained failures are expected or unexpected
+	// and (b) whether we expect to see any resolved IP address and (c) possibly more flags.
+	ControlExpectations optional.Value[*WebObservationsControlExpectations]
+
+	// DNSLookupSuccess contains all DNS lookups that were successful and for which
+	// we potentially have control information (i.e., in the 0-th redirect).
+	DNSLookupSuccess Set[int64]
 
 	// DNSLookupSuccessWithInvalidAddresses contains DNS transactions with invalid IP addresses by
 	// taking into account control info, bogons, and TLS handshakes.
@@ -156,12 +160,17 @@ type WebAnalysis struct {
 	// before hitting redirects (i.e., when TagDepth==0).
 	DNSExperimentFailure optional.Value[string]
 
-	// DNSLookupExpectedFailure contains DNS transactions with expected failures.
+	// DNSLookupExpectedFailure contains DNS transactions with expected failures, i.e.,
+	// failures that are consistent for the probe and the TH.
 	DNSLookupExpectedFailure Set[int64]
 
 	// DNSLookupExpectedSuccess contains DNS transactions with expected successes.
 	DNSLookupExpectedSuccess Set[int64]
 
+	// TCPConnectExpectedFailure contains TCP connect transactions that failed
+	// consistently for the probe and the test helper.
+	TCPConnectExpectedFailure Set[int64]
+
 	// TCPConnectUnexpectedFailure contains TCP endpoint transactions with unexpected failures.
 	TCPConnectUnexpectedFailure Set[int64]
 
@@ -185,6 +194,10 @@ type WebAnalysis struct {
 	// while checking for connectivity, as opposed to fetching a webpage.
 	TCPConnectUnexplainedFailureDuringConnectivityCheck Set[int64]
 
+	// TLSHandshakeExpectedFailure contains TLS endpoint transactions that failed
+	// consistently for the probe and the test helper.
+	TLSHandshakeExpectedFailure Set[int64]
+
 	// TLSHandshakeUnexpectedFailure contains TLS endpoint transactions with unexpected failures.
 	TLSHandshakeUnexpectedFailure Set[int64]
 
@@ -264,6 +277,10 @@ func (wa *WebAnalysis) dnsComputeSuccessMetrics(c *WebObservationsContainer) {
 			continue
 		}
 
+		// track all the 0-th redirect lookups that succeded which helps to
+		// determine whether the probe UNEXPECTEDLY resolved any address.
+		wa.DNSLookupSuccess.Add(obs.DNSTransactionID.Unwrap())
+
 		// if there's a bogon, mark as invalid
 		if !obs.IPAddressBogon.IsNone() && obs.IPAddressBogon.Unwrap() {
 			wa.DNSLookupSuccessWithInvalidAddresses.Add(obs.DNSTransactionID.Unwrap())
@@ -462,8 +479,12 @@ func (wa *WebAnalysis) tcpComputeMetrics(c *WebObservationsContainer) {
 			continue
 		}
 
-		// handle the case where only the control fails
+		// handle the case where the control fails
 		if obs.ControlTCPConnectFailure.Unwrap() != "" {
+			// if also the probe failed mark this failure as expected
+			if obs.TCPConnectFailure.Unwrap() != "" {
+				wa.TCPConnectExpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
+			}
 			continue
 		}
 
@@ -517,8 +538,12 @@ func (wa *WebAnalysis) tlsComputeMetrics(c *WebObservationsContainer) {
 			continue
 		}
 
-		// handle the case where only the control fails
+		// handle the case where the control fails
 		if obs.ControlTLSHandshakeFailure.Unwrap() != "" {
+			// if also the probe failed mark this failure as expected
+			if obs.TLSHandshakeFailure.Unwrap() != "" {
+				wa.TLSHandshakeExpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
+			}
 			continue
 		}
 

internal/minipipeline/classic.go

@@ -53,7 +53,7 @@ func ClassicFilter(input *WebObservationsContainer) (output *WebObservationsCont
 	}
 
 	// ControlFinalResponseExpectations
-	output.ControlFinalResponseExpectations = input.ControlFinalResponseExpectations
+	output.ControlExpectations = input.ControlExpectations
 
 	return
 }