feat(webconnectivity@v0.5): probe and TH can't connect => website down (#957)

This diff introduces a special rule to avoid emitting null, null when all the connects failed in both the probe and the TH.

While there, recognize that the subset of null, null we're hunting actually deals with websites that are down, so change the internal naming to reflect that and make the code easier to read/understand.

See https://github.com/ooni/probe/issues/2299

internal/experiment/webconnectivity/analysiscore.go

@@ -1,6 +1,11 @@
 package webconnectivity
 
-import "github.com/ooni/probe-cli/v3/internal/model"
+import (
+	"fmt"
+	"net"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
 
 //
 // Core analysis
@@ -83,6 +88,9 @@ const (
 //     +--------------------------------------+----------------+-------------+
 //
 // It's a very simple rule, that should preserve previous semantics.
+//
+// As an improvement over Web Connectivity v0.4, we also attempt to identify
+// special subcases of a null, null result to provide the user with more information.
 func (tk *TestKeys) analysisToplevel(logger model.Logger) {
 	// Since we run after all tasks have completed (or so we assume) we're
 	// not going to use any form of locking here.
@@ -135,11 +143,20 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) {
 		)
 
 	default:
-		if tk.analysisNullNullDetectNoAddrs(logger) {
+		if tk.analysisWebsiteDownDetectNoAddrs(logger) {
 			tk.Blocking = false
 			tk.Accessible = false
 			logger.Infof(
-				"NO_AVAILABLE_ADDRS: flags=%d, accessible=%+v, blocking=%+v",
+				"WEBSITE_DOWN_DNS: flags=%d, accessible=%+v, blocking=%+v",
+				tk.BlockingFlags, tk.Accessible, tk.Blocking,
+			)
+			return
+		}
+		if tk.analysisWebsiteDownDetectAllConnectsFailed(logger) {
+			tk.Blocking = false
+			tk.Accessible = false
+			logger.Infof(
+				"WEBSITE_DOWN_TCP: flags=%d, accessible=%+v, blocking=%+v",
 				tk.BlockingFlags, tk.Accessible, tk.Blocking,
 			)
 			return
@@ -154,12 +171,58 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) {
 }
 
 const (
-	// analysisFlagNullNullNoAddrs indicates neither the probe nor the TH were
+	// analysisFlagWebsiteDownNoAddrs indicates neither the probe nor the TH were
 	// able to get any IP addresses from any resolver.
-	analysisFlagNullNullNoAddrs = 1 << iota
+	analysisFlagWebsiteDownNoAddrs = 1 << iota
+
+	// analysisFlagWebsiteDownAllConnectsFailed indicates that all the connect
+	// attempts failed both in the probe and in the test helper.
+	analysisFlagWebsiteDownAllConnectsFailed
 )
 
-// analysisNullNullDetectNoAddrs attempts to see whether we
+// analysisWebsiteDownDetectAllConnectsFailed attempts to detect whether we are in
+// the .Blocking = nil, .Accessible = nil case because all the TCP connect
+// attempts by either the probe or the TH have failed.
+//
+// See https://explorer.ooni.org/measurement/20220911T105037Z_webconnectivity_IT_30722_n1_ruzuQ219SmIO9SrT?input=https://doh.centraleu.pi-dns.com/dns-query?dns=q80BAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB
+// for an example measurement with this behavior.
+//
+// See https://github.com/ooni/probe/issues/2299 for the reference issue.
+func (tk *TestKeys) analysisWebsiteDownDetectAllConnectsFailed(logger model.Logger) bool {
+	if tk.Control == nil {
+		// we need control data to say we're in this case
+		return false
+	}
+
+	for _, entry := range tk.TCPConnect {
+		if entry.Status.Failure == nil {
+			// we need all connect attempts to fail
+			return false
+		}
+		epnt := net.JoinHostPort(entry.IP, fmt.Sprintf("%d", entry.Port))
+		thEntry, found := tk.Control.TCPConnect[epnt]
+		if !found {
+			// we need exactly the same attempts to have failed
+			return false
+		}
+		if thEntry.Failure == nil {
+			// we need all TH attempts to fail
+			return false
+		}
+	}
+
+	// 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.WebsiteDownFlags |= analysisFlagWebsiteDownAllConnectsFailed
+		return true
+	}
+
+	// safety net in case we're passed empty lists/maps
+	return false
+}
+
+// analysisWebsiteDownDetectNoAddrs attempts to see whether we
 // ended up into the .Blocking = nil, .Accessible = nil case because
 // the domain is expired and all queries returned no addresses.
 //
@@ -173,7 +236,7 @@ const (
 //
 // See https://github.com/ooni/probe/issues/2029 for more information
 // on Android's getaddrinfo behavior.
-func (tk *TestKeys) analysisNullNullDetectNoAddrs(logger model.Logger) bool {
+func (tk *TestKeys) analysisWebsiteDownDetectNoAddrs(logger model.Logger) bool {
 	if tk.Control == nil {
 		// we need control data to say we're in this case
 		return false
@@ -200,7 +263,7 @@ func (tk *TestKeys) analysisNullNullDetectNoAddrs(logger model.Logger) bool {
 		// when the TH used addresses, we're not in the NoAddresses case
 		return false
 	}
-	logger.Infof("Neither the probe nor the TH resolved any addresses")
-	tk.NullNullFlags |= analysisFlagNullNullNoAddrs
+	logger.Infof("website likely down: all DNS lookups failed for both probe and TH")
+	tk.WebsiteDownFlags |= analysisFlagWebsiteDownNoAddrs
 	return true
 }

internal/experiment/webconnectivity/measurer.go

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

internal/experiment/webconnectivity/testkeys.go

@@ -72,7 +72,7 @@ type TestKeys struct {
 	// ControlFailure contains the failure of the control experiment.
 	ControlFailure *string `json:"control_failure"`
 
-	// DNSFlags contains DNS analysis flags.
+	// DNSFlags describes specific DNS anomalies we observed.
 	DNSFlags int64 `json:"x_dns_flags"`
 
 	// DNSExperimentFailure indicates whether there was a failure in any
@@ -87,13 +87,11 @@ type TestKeys struct {
 	// the final HTTP request that we recorded.
 	HTTPExperimentFailure *string `json:"http_experiment_failure"`
 
-	// BlockingFlags contains blocking flags.
+	// BlockingFlags explains why we think that the website is blocked.
 	BlockingFlags int64 `json:"x_blocking_flags"`
 
-	// NullNullFlags explains why we determined that a measurement is not
-	// failed by detecting specific conditions that would have otherwise
-	// caused .Accessible = nil and .Blocking = nil
-	NullNullFlags int64 `json:"x_null_null_flags"`
+	// WebsiteDownFlags explains why we determined that the website is down.
+	WebsiteDownFlags int64 `json:"x_website_down_flags"`
 
 	// BodyLength match tells us whether the body length matches.
 	BodyLengthMatch *bool `json:"body_length_match"`
@@ -339,7 +337,7 @@ func NewTestKeys() *TestKeys {
 		DNSConsistency:        "",
 		HTTPExperimentFailure: nil,
 		BlockingFlags:         0,
-		NullNullFlags:         0,
+		WebsiteDownFlags:      0,
 		BodyLengthMatch:       nil,
 		HeadersMatch:          nil,
 		StatusCodeMatch:       nil,