From 33451c7d02597b0ae90941800e084fdfabe0002d Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Wed, 11 Oct 2023 13:15:04 +0200
Subject: [PATCH] fix(registry): mark torsf as disabled by default (#1359)

There may be upcoming changes in torsf which may cause it to fail
consistently as it occurred during the 3.18 cycle.

Therefore, be defensive and make it disabled by default.

Part of https://github.com/ooni/probe/issues/2553

While there, use slightly better naming for an echcheck function.
---
 internal/experiment/echcheck/handshake.go | 4 ++--
 internal/registry/factory_test.go         | 7 +++++--
 internal/registry/torsf.go                | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go
index 3ef15e67fd..4597d89fe9 100644
--- a/internal/experiment/echcheck/handshake.go
+++ b/internal/experiment/echcheck/handshake.go
@@ -37,7 +37,7 @@ func handshakeWithEch(ctx context.Context, conn net.Conn, zeroTime time.Time,
 	return handshakeWithExtension(ctx, conn, zeroTime, address, sni, []utls.TLSExtension{&utlsEchExtension}, logger)
 }
 
-func handshakeMaybePrintECH(doprint bool) string {
+func handshakeMaybePrintWithECH(doprint bool) string {
 	if doprint {
 		return "WithECH"
 	}
@@ -51,7 +51,7 @@ func handshakeWithExtension(ctx context.Context, conn net.Conn, zeroTime time.Ti
 	handshakerConstructor := newHandshakerWithExtensions(extensions)
 	tracedHandshaker := handshakerConstructor(log.Log, &utls.HelloFirefox_Auto)
 
-	ol := logx.NewOperationLogger(logger, "echcheck: TLSHandshake%s", handshakeMaybePrintECH(len(extensions) > 0))
+	ol := logx.NewOperationLogger(logger, "echcheck: TLSHandshake%s", handshakeMaybePrintWithECH(len(extensions) > 0))
 	start := time.Now()
 	maybeTLSConn, err := tracedHandshaker.Handshake(ctx, conn, tlsConfig)
 	finish := time.Now()
diff --git a/internal/registry/factory_test.go b/internal/registry/factory_test.go
index 0d7b14997e..b49ed21d56 100644
--- a/internal/registry/factory_test.go
+++ b/internal/registry/factory_test.go
@@ -481,8 +481,11 @@ func TestNewFactory(t *testing.T) {
 			inputPolicy:      model.InputNone,
 		},
 		"torsf": {
-			enabledByDefault: true,
-			inputPolicy:      model.InputNone,
+			// We suspect there will be changes in torsf SNI soon. We are not prepared to
+			// serve these changes using the check-in API. Hence, disable torsf by default
+			// and require enabling it using the check-in API feature flags.
+			//enabledByDefault: false,
+			inputPolicy: model.InputNone,
 		},
 		"urlgetter": {
 			enabledByDefault: true,
diff --git a/internal/registry/torsf.go b/internal/registry/torsf.go
index ad2d6051c3..7e087f59c9 100644
--- a/internal/registry/torsf.go
+++ b/internal/registry/torsf.go
@@ -17,7 +17,7 @@ func init() {
 			)
 		},
 		config:           &torsf.Config{},
-		enabledByDefault: true,
+		enabledByDefault: false,
 		inputPolicy:      model.InputNone,
 	}
 }