whatsapp: implement remaining checks

As mentioned in the commit, we see a 400 Bad Request error from
WhatsApp when using the User-Agent we use for measurements along
with the standard Golang's ClientHello fingerprint.

This looks like MITM detection like https://mitm.watch to me.

A fix for this issue could be to find out a combination of User-Agent
and ClientHello that does not trigger 400 and keep the test as it
should according to the spec.

Yet, if there is MITM detection, it may change. This will likely
cause future false positives, and we already have a bunch of such
false positives for the IM tests.

Also, it currently seems safe to assume that, if we can perform
a TLS handshake with a certificate pool we trust, then we are
talking with WhatsApp. Therefore, the status code and the returned
web page matter much less than they did when we wrote the initial
implementation of the WhatsApp experiment.

What's more, because the HTTP request only redirects us, we should
probably also simplify that check, to avoid asserting anything on
the returned web page _if_ we're correctly redirected.

How to properly do this will be researched in the next sprint
as part of #740.

Further investigating this issue should also be fun.

This work is part of #55.

experiment/whatsapp/whatsapp.go

@@ -6,6 +6,13 @@
 //
 // This implementation does not currently perform the CIDR check, which is
 // know to be broken. We shall fix this issue at the spec level first.
+//
+// This implemention currently triggers what looks like MITM blocking by
+// WhatsApp, where the combination of User-Agent header and TLS ClientHello
+// we use causes a `400 Bad Request` error. We have experimentally seen we
+// can avoid such error by using `miniooni/0.1.0-dev` as User-Agent. We may
+// want to find out a better implementation in the future. But doing that
+// is tricky as it may cause subsequent false positives down the line.
 package whatsapp
 
 import (
@@ -73,13 +80,13 @@ func NewTestKeys() *TestKeys {
 
 // Update updates the TestKeys using the given MultiOutput result.
 func (tk *TestKeys) Update(v urlgetter.MultiOutput) {
-	// update the easy to update entries first
+	// Update the easy to update entries first
 	tk.NetworkEvents = append(tk.NetworkEvents, v.TestKeys.NetworkEvents...)
 	tk.Queries = append(tk.Queries, v.TestKeys.Queries...)
 	tk.Requests = append(tk.Requests, v.TestKeys.Requests...)
 	tk.TCPConnect = append(tk.TCPConnect, v.TestKeys.TCPConnect...)
 	tk.TLSHandshakes = append(tk.TLSHandshakes, v.TestKeys.TLSHandshakes...)
-	// set the status of WhatsApp endpoints
+	// Set the status of WhatsApp endpoints
 	if endpointPattern.MatchString(v.Input.Target) {
 		if v.TestKeys.Failure != nil {
 			endpoint := strings.ReplaceAll(v.Input.Target, "tcpconnect://", "")
@@ -89,23 +96,29 @@ func (tk *TestKeys) Update(v urlgetter.MultiOutput) {
 		tk.WhatsappEndpointsStatus = "ok"
 		return
 	}
-	// set the status of the registration service
+	// Set the status of the registration service
 	if v.Input.Target == RegistrationServiceURL {
-		// TODO(bassosimone): here we should check the HTTP status code
 		tk.RegistrationServerFailure = v.TestKeys.Failure
 		if v.TestKeys.Failure == nil {
 			tk.RegistrationServerStatus = "ok"
 		}
 		return
 	}
-	// track result of accessing the web interface
-	// TODO(bassosimone): here we should check the HTTP status code
-	// as well as the webpage contains "WhatsApp Web".
+	// Track result of accessing the web interface.
+	//
+	// We treat HTTPS differently. A comment above describing what looks
+	// like MITM detection should be enough to understand this code.
 	switch v.Input.Target {
 	case WebHTTPSURL:
 		tk.WhatsappHTTPSFailure = v.TestKeys.Failure
 	case WebHTTPURL:
-		tk.WhatsappHTTPFailure = v.TestKeys.Failure
+		failure := v.TestKeys.Failure
+		title := `<title>WhatsApp Web</title>`
+		if failure == nil && strings.Contains(v.TestKeys.HTTPResponseBody, title) == false {
+			failureString := "whatsapp_missing_title_error"
+			failure = &failureString
+		}
+		tk.WhatsappHTTPFailure = failure
 	}
 }
 
@@ -124,19 +137,28 @@ func (tk *TestKeys) ComputeWebStatus() {
 	tk.WhatsappWebFailure = tk.WhatsappHTTPFailure
 }
 
-type measurer struct {
-	config Config
+// Measurer performs the measurement
+type Measurer struct {
+	// Config contains the experiment settings. If empty we
+	// will be using default settings.
+	Config Config
+
+	// Getter is an optional getter to be used for testing.
+	Getter urlgetter.MultiGetter
 }
 
-func (m measurer) ExperimentName() string {
+// ExperimentName implements ExperimentMeasurer.ExperimentName
+func (m Measurer) ExperimentName() string {
 	return testName
 }
 
-func (m measurer) ExperimentVersion() string {
+// ExperimentVersion implements ExperimentMeasurer.ExperimentVersion
+func (m Measurer) ExperimentVersion() string {
 	return testVersion
 }
 
-func (m measurer) Run(
+// Run implements ExperimentMeasurer.Run
+func (m Measurer) Run(
 	ctx context.Context, sess model.ExperimentSession,
 	measurement *model.Measurement, callbacks model.ExperimentCallbacks,
 ) error {
@@ -156,14 +178,26 @@ func (m measurer) Run(
 	rnd.Shuffle(len(inputs), func(i, j int) {
 		inputs[i], inputs[j] = inputs[j], inputs[i]
 	})
-	if m.config.AllEndpoints == false {
+	if m.Config.AllEndpoints == false {
 		inputs = inputs[0:1]
 	}
-	inputs = append(inputs, urlgetter.MultiInput{Target: RegistrationServiceURL})
-	inputs = append(inputs, urlgetter.MultiInput{Target: WebHTTPSURL})
-	inputs = append(inputs, urlgetter.MultiInput{Target: WebHTTPURL})
+	inputs = append(inputs, urlgetter.MultiInput{
+		Config: urlgetter.Config{FailOnHTTPError: true},
+		Target: RegistrationServiceURL,
+	})
+	inputs = append(inputs, urlgetter.MultiInput{
+		// See the above comment regarding what seems MITM detection to
+		// understand why we're not forcing FailOnHTTPError here.
+		Target: WebHTTPSURL,
+	})
+	inputs = append(inputs, urlgetter.MultiInput{
+		// We may eventually start seeing HTTP 400 errors here. See the
+		// above comment on what seems MITM detection.
+		Config: urlgetter.Config{FailOnHTTPError: true},
+		Target: WebHTTPURL,
+	})
 	// measure in parallel
-	multi := urlgetter.Multi{Begin: time.Now(), Session: sess}
+	multi := urlgetter.Multi{Begin: time.Now(), Getter: m.Getter, Session: sess}
 	testkeys := NewTestKeys()
 	testkeys.Agent = "redirect"
 	measurement.TestKeys = testkeys
@@ -176,5 +210,5 @@ func (m measurer) Run(
 
 // NewExperimentMeasurer creates a new ExperimentMeasurer.
 func NewExperimentMeasurer(config Config) model.ExperimentMeasurer {
-	return measurer{config: config}
+	return Measurer{Config: config}
 }

experiment/whatsapp/whatsapp_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-engine/atomicx"
 	"github.com/ooni/probe-engine/experiment/handler"
 	"github.com/ooni/probe-engine/experiment/urlgetter"
 	"github.com/ooni/probe-engine/experiment/whatsapp"
@@ -231,8 +232,10 @@ func TestTestKeysOnlyEndpointsFailure(t *testing.T) {
 		TestKeys: urlgetter.TestKeys{},
 	})
 	tk.Update(urlgetter.MultiOutput{
-		Input:    urlgetter.MultiInput{Target: whatsapp.WebHTTPURL},
-		TestKeys: urlgetter.TestKeys{},
+		Input: urlgetter.MultiInput{Target: whatsapp.WebHTTPURL},
+		TestKeys: urlgetter.TestKeys{
+			HTTPResponseBody: "<title>WhatsApp Web</title>",
+		},
 	})
 	tk.ComputeWebStatus()
 	if tk.RegistrationServerFailure != nil {
@@ -274,8 +277,10 @@ func TestTestKeysOnlyRegistrationServerFailure(t *testing.T) {
 		TestKeys: urlgetter.TestKeys{},
 	})
 	tk.Update(urlgetter.MultiOutput{
-		Input:    urlgetter.MultiInput{Target: whatsapp.WebHTTPURL},
-		TestKeys: urlgetter.TestKeys{},
+		Input: urlgetter.MultiInput{Target: whatsapp.WebHTTPURL},
+		TestKeys: urlgetter.TestKeys{
+			HTTPResponseBody: "<title>WhatsApp Web</title>",
+		},
 	})
 	tk.ComputeWebStatus()
 	if *tk.RegistrationServerFailure != failure {
@@ -343,3 +348,52 @@ func TestTestKeysOnlyWebFailure(t *testing.T) {
 		t.Fatal("invalid WhatsappWebStatus")
 	}
 }
+
+func TestWeConfigureWebChecksToFailOnHTTPError(t *testing.T) {
+	called := atomicx.NewInt64()
+	failOnErrorWebHTTPS := atomicx.NewInt64()
+	failOnErrorWebHTTP := atomicx.NewInt64()
+	failOnErrorRegistrationService := atomicx.NewInt64()
+	measurer := whatsapp.Measurer{
+		Config: whatsapp.Config{},
+		Getter: func(ctx context.Context, g urlgetter.Getter) (urlgetter.TestKeys, error) {
+			called.Add(1)
+			switch g.Target {
+			case whatsapp.WebHTTPSURL:
+				if g.Config.FailOnHTTPError {
+					failOnErrorWebHTTPS.Add(1)
+				}
+			case whatsapp.WebHTTPURL:
+				if g.Config.FailOnHTTPError {
+					failOnErrorWebHTTP.Add(1)
+				}
+			case whatsapp.RegistrationServiceURL:
+				if g.Config.FailOnHTTPError {
+					failOnErrorRegistrationService.Add(1)
+				}
+			}
+			return urlgetter.DefaultMultiGetter(ctx, g)
+		},
+	}
+	ctx := context.Background()
+	sess := &mockable.ExperimentSession{
+		MockableLogger: log.Log,
+	}
+	measurement := new(model.Measurement)
+	callbacks := handler.NewPrinterCallbacks(log.Log)
+	if err := measurer.Run(ctx, sess, measurement, callbacks); err != nil {
+		t.Fatal(err)
+	}
+	if called.Load() < 1 {
+		t.Fatal("not called")
+	}
+	if failOnErrorWebHTTPS.Load() != 0 {
+		t.Fatal("configured fail on error for Web HTTPS")
+	}
+	if failOnErrorWebHTTP.Load() != 1 {
+		t.Fatal("not configured fail on error for Web HTTP")
+	}
+	if failOnErrorRegistrationService.Load() != 1 {
+		t.Fatal("not configured fail on error for registragtion service")
+	}
+}