refactor(minipipeline): adjust how we compute TCP and TLS anomalies (#…

…1410) Part of ooni/probe#2634
ooni · Nov 30, 2023 · 0c683d7 · 0c683d7
1 parent 6a605a6
commit 0c683d7
Show file tree

Hide file tree

Showing 69 changed files with 237 additions and 236 deletions.
diff --git a/internal/cmd/minipipeline/testdata/analysis.json b/internal/cmd/minipipeline/testdata/analysis.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": 1,
@@ -17,8 +19,6 @@
   "HTTPFinalResponsesWithTLS": {
     "4": true
   },
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/internal/cmd/minipipeline/testdata/analysis_classic.json b/internal/cmd/minipipeline/testdata/analysis_classic.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": 1,
@@ -17,8 +19,6 @@
   "HTTPFinalResponsesWithTLS": {
     "4": true
   },
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/internal/minipipeline/analysis.go b/internal/minipipeline/analysis.go
@@ -14,11 +14,12 @@ func AnalyzeWebObservations(container *WebObservationsContainer) *WebAnalysis {
 	analysis.ComputeDNSLookupSuccessWithInvalidAddressesClassic(container)
 	analysis.ComputeDNSLookupUnexpectedFailure(container)
 
+	analysis.ComputeTCPConnectUnexpectedFailure(container)
+	analysis.ComputeTLSHandshakeUnexpectedFailure(container)
+
 	analysis.ComputeDNSExperimentFailure(container)
 	analysis.ComputeDNSPossiblyNonexistingDomains(container)
 
-	analysis.ComputeTCPTransactionsWithUnexpectedTCPConnectFailures(container)
-	analysis.ComputeTCPTransactionsWithUnexpectedTLSHandshakeFailures(container)
 	analysis.ComputeTCPTransactionsWithUnexpectedHTTPFailures(container)
 	analysis.ComputeTCPTransactionsWithUnexplainedUnexpectedFailures(container)
 
@@ -47,6 +48,12 @@ type WebAnalysis struct {
 	// DNSLookupUnexpectedFailure contains DNS transactions with unexpected failures.
 	DNSLookupUnexpectedFailure Set[int64]
 
+	// TCPConnectUnexpectedFailure contains TCP endpoint transactions with unexpected failures.
+	TCPConnectUnexpectedFailure Set[int64]
+
+	// TLSHandshakeUnexpectedFailure contains TLS endpoint transactions with unexpected failures.
+	TLSHandshakeUnexpectedFailure Set[int64]
+
 	// DNSExperimentFailure is the first failure experienced by a getaddrinfo-like resolver.
 	DNSExperimentFailure optional.Value[string]
 
@@ -86,14 +93,6 @@ type WebAnalysis struct {
 	// itself with whether there's control data, because TLS suffices.
 	HTTPFinalResponsesWithTLS optional.Value[map[int64]bool]
 
-	// TCPTransactionsWithUnexpectedTCPConnectFailures contains the TCP transaction IDs that
-	// contain TCP connect failures while the control measurement succeeded.
-	TCPTransactionsWithUnexpectedTCPConnectFailures optional.Value[map[int64]bool]
-
-	// TCPTransactionsWithUnexpectedTLSHandshakeFailures contains the TCP transaction IDs that
-	// contain TLS handshake failures while the control measurement succeeded.
-	TCPTransactionsWithUnexpectedTLSHandshakeFailures optional.Value[map[int64]bool]
-
 	// TCPSTransactionsWithUnexpectedHTTPFailures contains the TCP transaction IDs that
 	// contain HTTP failures while the control measurement succeeded.
 	TCPTransactionsWithUnexpectedHTTPFailures optional.Value[map[int64]bool]
@@ -262,6 +261,81 @@ func (wa *WebAnalysis) ComputeDNSLookupUnexpectedFailure(c *WebObservationsConta
 	}
 }
 
+// ComputeTCPConnectUnexpectedFailure computes the TCPConnectUnexpectedFailure field.
+func (wa *WebAnalysis) ComputeTCPConnectUnexpectedFailure(c *WebObservationsContainer) {
+	for _, obs := range c.KnownTCPEndpoints {
+		// dials once we started following redirects should not be considered
+		if obs.TagDepth.IsNone() || obs.TagDepth.Unwrap() != 0 {
+			continue
+		}
+
+		// handle the case where there is no measurement
+		if obs.TCPConnectFailure.IsNone() {
+			continue
+		}
+
+		// handle the case where there is no control information
+		if obs.ControlTCPConnectFailure.IsNone() {
+			continue
+		}
+
+		// handle the case where both the probe and the control fail
+		if obs.TCPConnectFailure.Unwrap() != "" && obs.ControlTCPConnectFailure.Unwrap() != "" {
+			continue
+		}
+
+		// handle the case where only the control fails
+		if obs.ControlTCPConnectFailure.Unwrap() != "" {
+			continue
+		}
+
+		// handle the case where only the probe fails
+		if obs.TCPConnectFailure.Unwrap() != "" {
+			if utilsTCPConnectFailureSeemsMisconfiguredIPv6(obs) {
+				continue
+			}
+			wa.TCPConnectUnexpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
+			continue
+		}
+	}
+}
+
+// ComputeTLSHandshakeUnexpectedFailure computes the TLSHandshakeUnexpectedFailure field.
+func (wa *WebAnalysis) ComputeTLSHandshakeUnexpectedFailure(c *WebObservationsContainer) {
+	for _, obs := range c.KnownTCPEndpoints {
+		// handshakes once we started following redirects should not be considered
+		if obs.TagDepth.IsNone() || obs.TagDepth.Unwrap() != 0 {
+			continue
+		}
+
+		// handle the case where there is no measurement
+		if obs.TLSHandshakeFailure.IsNone() {
+			continue
+		}
+
+		// handle the case where there is no control information
+		if obs.ControlTLSHandshakeFailure.IsNone() {
+			continue
+		}
+
+		// handle the case where both the probe and the control fail
+		if obs.TLSHandshakeFailure.Unwrap() != "" && obs.ControlTCPConnectFailure.Unwrap() != "" {
+			continue
+		}
+
+		// handle the case where only the control fails
+		if obs.ControlTLSHandshakeFailure.Unwrap() != "" {
+			continue
+		}
+
+		// handle the case where only the probe fails
+		if obs.TLSHandshakeFailure.Unwrap() != "" {
+			wa.TLSHandshakeUnexpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
+			continue
+		}
+	}
+}
+
 // ComputeDNSExperimentFailure computes the DNSExperimentFailure field.
 func (wa *WebAnalysis) ComputeDNSExperimentFailure(c *WebObservationsContainer) {
 
@@ -366,79 +440,6 @@ func (wa *WebAnalysis) ComputeDNSPossiblyNonexistingDomains(c *WebObservationsCo
 	wa.DNSPossiblyNonexistingDomains = optional.Some(state)
 }
 
-// ComputeTCPTransactionsWithUnexpectedTCPConnectFailures computes the TCPTransactionsWithUnexpectedTCPConnectFailures field.
-func (wa *WebAnalysis) ComputeTCPTransactionsWithUnexpectedTCPConnectFailures(c *WebObservationsContainer) {
-	var state map[int64]bool
-
-	for _, obs := range c.KnownTCPEndpoints {
-		// we cannot do anything unless we have both records
-		if obs.TCPConnectFailure.IsNone() || obs.ControlTCPConnectFailure.IsNone() {
-			continue
-		}
-
-		// flip state from None to empty once we have seen the first
-		// suitable set of measurement/control pairs
-		if state == nil {
-			state = make(map[int64]bool)
-		}
-
-		// skip cases with no failures
-		if obs.TCPConnectFailure.Unwrap() == "" {
-			continue
-		}
-
-		// skip cases where also the control failed
-		if obs.ControlTCPConnectFailure.Unwrap() != "" {
-			continue
-		}
-
-		// skip cases where the root cause could be a misconfigured IPv6 stack
-		if utilsTCPConnectFailureSeemsMisconfiguredIPv6(obs) {
-			continue
-		}
-
-		// update state
-		state[obs.EndpointTransactionID.Unwrap()] = true
-	}
-
-	// note that optional.Some constructs None if state is nil
-	wa.TCPTransactionsWithUnexpectedTCPConnectFailures = optional.Some(state)
-}
-
-// ComputeTCPTransactionsWithUnexpectedTLSHandshakeFailures computes the TCPTransactionsWithUnexpectedTLSHandshakeFailures field.
-func (wa *WebAnalysis) ComputeTCPTransactionsWithUnexpectedTLSHandshakeFailures(c *WebObservationsContainer) {
-	var state map[int64]bool
-
-	for _, obs := range c.KnownTCPEndpoints {
-		// we cannot do anything unless we have both records
-		if obs.TLSHandshakeFailure.IsNone() || obs.ControlTLSHandshakeFailure.IsNone() {
-			continue
-		}
-
-		// flip state from None to empty once we have seen the first
-		// suitable set of measurement/control pairs
-		if state == nil {
-			state = make(map[int64]bool)
-		}
-
-		// skip cases with no failures
-		if obs.TLSHandshakeFailure.Unwrap() == "" {
-			continue
-		}
-
-		// skip cases where also the control failed
-		if obs.ControlTLSHandshakeFailure.Unwrap() != "" {
-			continue
-		}
-
-		// update state
-		state[obs.EndpointTransactionID.Unwrap()] = true
-	}
-
-	// note that optional.Some constructs None if state is nil
-	wa.TCPTransactionsWithUnexpectedTLSHandshakeFailures = optional.Some(state)
-}
-
 // ComputeTCPTransactionsWithUnexpectedHTTPFailures computes the TCPTransactionsWithUnexpectedHTTPFailures field.
 func (wa *WebAnalysis) ComputeTCPTransactionsWithUnexpectedHTTPFailures(c *WebObservationsContainer) {
 	var state map[int64]bool

diff --git a/...inipipeline/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis.json b/...inipipeline/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": null,
@@ -10,8 +12,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...ine/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis_classic.json b/...ine/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis_classic.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -10,8 +12,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...tdata/webconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis.json b/...tdata/webconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": null,
@@ -10,8 +12,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...bconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis_classic.json b/...bconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis_classic.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -10,8 +12,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...ata/webconnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis.json b/...ata/webconnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis.json
@@ -8,6 +8,8 @@
     2
   ],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": 1,
@@ -23,8 +25,6 @@
   "HTTPFinalResponsesWithTLS": {
     "4": true
   },
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/...onnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis_classic.json b/...onnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis_classic.json
@@ -6,6 +6,8 @@
     2
   ],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -14,8 +16,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/...l/minipipeline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis.json b/...l/minipipeline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": null,
@@ -10,8 +12,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...peline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis_classic.json b/...peline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis_classic.json
@@ -2,6 +2,8 @@
   "DNSLookupSuccessWithInvalidAddresses": [],
   "DNSLookupSuccessWithInvalidAddressesClassic": [],
   "DNSLookupUnexpectedFailure": [],
+  "TCPConnectUnexpectedFailure": [],
+  "TLSHandshakeUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -10,8 +12,6 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedTCPConnectFailures": {},
-  "TCPTransactionsWithUnexpectedTLSHandshakeFailures": {},
   "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }