Skip to content

Commit

Permalink
fix(registry): mark torsf as disabled by default (#1359)
Browse files Browse the repository at this point in the history
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 authored Oct 11, 2023
1 parent e6e00dc commit 33451c7
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 33451c7

Please sign in to comment.