Skip to content

Commit

Permalink
[backport] fix(registry): mark torsf as disabled by default
Browse files Browse the repository at this point in the history
This diff backports #1359 to the release/3.19 branch.

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 ooni/probe#2553

While there, use slightly better naming for an echcheck function.
  • Loading branch information
bassosimone committed Oct 11, 2023
1 parent d13a7ca commit 7bcf415
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
4 changes: 2 additions & 2 deletions internal/experiment/echcheck/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand All @@ -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()
Expand Down
7 changes: 5 additions & 2 deletions internal/registry/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/registry/torsf.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func init() {
)
},
config: &torsf.Config{},
enabledByDefault: true,
enabledByDefault: false,
inputPolicy: model.InputNone,
}
}

0 comments on commit 7bcf415

Please sign in to comment.