sniblocking: revamp classification mechanism (#397)

Part of #309

This is the result of a two hours brainstorming and pair
programming session with @fortuna, where we reviewed existing
codebase, and design, and we spelled out a classification
taxonomy more oriented towards communicating with a OONI user.

experiment/sniblocking/sniblocking.go

@@ -22,7 +22,7 @@ import (
 
 const (
 	testName    = "sni_blocking"
-	testVersion = "0.0.4"
+	testVersion = "0.0.5"
 )
 
 // Config contains the experiment config.
@@ -58,48 +58,40 @@ type TestKeys struct {
 }
 
 const (
-	classAccessibleInvalidHostname = "accessible_invalid_hostname"
-	classAccessibleValidHostname   = "accessible_valid_hostname"
-	classAnomalySSLError           = "anomaly_ssl_error"
-	classAnomalyTestHelperBlocked  = "anomaly_test_helper_blocked"
-	classAnomalyTimeout            = "anomaly_timeout"
-	classAnomalyUnexpectedFailure  = "anomaly_unexpected_failure"
-	classBlockedTCPIPError         = "blocked_tcpip_error"
+	classAnomalyTestHelperUnreachable   = "anomaly.test_helper_unreachable"
+	classAnomalyTimeout                 = "anomaly.timeout"
+	classAnomalyUnexpectedFailure       = "anomaly.unexpected_failure"
+	classInterferenceClosed             = "interference.closed"
+	classInterferenceInvalidCertificate = "interference.invalid_certificate"
+	classInterferenceReset              = "interference.reset"
+	classInterferenceUnknownAuthority   = "interference.unknown_authority"
+	classSuccessGotServerHello          = "success.got_server_hello"
 )
 
 func (tk *TestKeys) classify() string {
-	// This implementation of classify is loosely modeled after
-	// https://github.com/ooni/spec/pull/159#discussion_r373754706
 	if tk.Target.Failure == nil {
-		return classAccessibleValidHostname
+		return classSuccessGotServerHello
 	}
-	// TODO(bassosimone): we should write jafar tests to understand
-	// what error is returned in the case of MITM and make sure we
-	// can reliably detect and distinguish this case from other cases
-	// of TLS error. For now, the following is coded such that the
-	// MITM will result in classAnomalySSLErrror.
-	//
-	// See https://github.com/ooni/probe-engine/issues/393.
 	switch *tk.Target.Failure {
 	case modelx.FailureConnectionRefused:
-		return classAnomalyTestHelperBlocked
-	case modelx.FailureDNSNXDOMAINError:
-		return classAnomalyTestHelperBlocked
+		return classAnomalyTestHelperUnreachable
 	case modelx.FailureConnectionReset:
-		return classBlockedTCPIPError
+		return classInterferenceReset
+	case modelx.FailureDNSNXDOMAINError:
+		return classAnomalyTestHelperUnreachable
 	case modelx.FailureEOFError:
-		return classBlockedTCPIPError
-	case modelx.FailureSSLInvalidHostname:
-		return classAccessibleInvalidHostname
-	case modelx.FailureSSLUnknownAuthority:
-		return classAnomalySSLError
-	case modelx.FailureSSLInvalidCertificate:
-		return classAnomalySSLError
+		return classInterferenceClosed
 	case modelx.FailureGenericTimeoutError:
 		if tk.Control.Failure != nil {
-			return classAnomalyTestHelperBlocked
+			return classAnomalyTestHelperUnreachable
 		}
 		return classAnomalyTimeout
+	case modelx.FailureSSLInvalidCertificate:
+		return classInterferenceInvalidCertificate
+	case modelx.FailureSSLInvalidHostname:
+		return classSuccessGotServerHello
+	case modelx.FailureSSLUnknownAuthority:
+		return classInterferenceUnknownAuthority
 	}
 	return classAnomalyUnexpectedFailure
 }
@@ -229,7 +221,7 @@ func processall(
 		sentBytes += smk.BytesSent
 		receivedBytes += smk.BytesReceived
 		current++
-		sess.Logger().Infof(
+		sess.Logger().Debugf(
 			"sni_blocking: %s: %s [cached: %+v]", smk.SNI,
 			asString(smk.Failure), smk.Cached)
 		if current >= len(inputs) {

experiment/sniblocking/sniblocking_test.go

@@ -24,56 +24,56 @@ func TestUnitTestKeysClassify(t *testing.T) {
 	}
 	t.Run("with tk.Target.Failure == nil", func(t *testing.T) {
 		tk := new(TestKeys)
-		if tk.classify() != classAccessibleValidHostname {
+		if tk.classify() != classSuccessGotServerHello {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == connection_refused", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureConnectionRefused)
-		if tk.classify() != classAnomalyTestHelperBlocked {
+		if tk.classify() != classAnomalyTestHelperUnreachable {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == dns_nxdomain_error", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureDNSNXDOMAINError)
-		if tk.classify() != classAnomalyTestHelperBlocked {
+		if tk.classify() != classAnomalyTestHelperUnreachable {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == connection_reset", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureConnectionReset)
-		if tk.classify() != classBlockedTCPIPError {
+		if tk.classify() != classInterferenceReset {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == eof_error", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureEOFError)
-		if tk.classify() != classBlockedTCPIPError {
+		if tk.classify() != classInterferenceClosed {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == ssl_invalid_hostname", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureSSLInvalidHostname)
-		if tk.classify() != classAccessibleInvalidHostname {
+		if tk.classify() != classSuccessGotServerHello {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == ssl_unknown_authority", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureSSLUnknownAuthority)
-		if tk.classify() != classAnomalySSLError {
+		if tk.classify() != classInterferenceUnknownAuthority {
 			t.Fatal("unexpected result")
 		}
 	})
 	t.Run("with tk.Target.Failure == ssl_invalid_certificate", func(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureSSLInvalidCertificate)
-		if tk.classify() != classAnomalySSLError {
+		if tk.classify() != classInterferenceInvalidCertificate {
 			t.Fatal("unexpected result")
 		}
 	})
@@ -88,7 +88,7 @@ func TestUnitTestKeysClassify(t *testing.T) {
 		tk := new(TestKeys)
 		tk.Target.Failure = asStringPtr(modelx.FailureGenericTimeoutError)
 		tk.Control.Failure = asStringPtr(modelx.FailureGenericTimeoutError)
-		if tk.classify() != classAnomalyTestHelperBlocked {
+		if tk.classify() != classAnomalyTestHelperUnreachable {
 			t.Fatal("unexpected result")
 		}
 	})
@@ -106,7 +106,7 @@ func TestUnitNewExperimentMeasurer(t *testing.T) {
 	if measurer.ExperimentName() != "sni_blocking" {
 		t.Fatal("unexpected name")
 	}
-	if measurer.ExperimentVersion() != "0.0.4" {
+	if measurer.ExperimentVersion() != "0.0.5" {
 		t.Fatal("unexpected version")
 	}
 }