From a1ee20fed8c398aba30f83b285ae5ea682c28daa Mon Sep 17 00:00:00 2001 From: freetoshi-nakamoto Date: Wed, 9 Sep 2026 15:43:21 +0300 Subject: [PATCH] fix(echcheck): do not attempt a TLS handshake when the TCP connect failed When the dial fails, conn is nil and tls.Client(nil, ...).HandshakeContext panics inside crypto/tls. This is what users saw with min-ng.test.defo.ie once its port 15443 stopped answering. Return the test keys after recording the failed TCP connect instead; that is the measurement. Adds a localhost regression test that dials a closed port with and without GREASE. It panics without the fix. Closes #1728 --- internal/experiment/echcheck/handshake.go | 3 ++ .../experiment/echcheck/handshake_test.go | 38 +++++++++++++++++++ internal/experiment/echcheck/measure.go | 2 +- internal/experiment/echcheck/measure_test.go | 2 +- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go index 7e1e17767..47c656070 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -69,6 +69,9 @@ func handshake(ctx context.Context, isGrease bool, startTime time.Time, address ol1.Stop(err) tk.TCPConnects = append(tk.TCPConnects, trace.TCPConnects()...) tk.NetworkEvents = append(tk.NetworkEvents, trace.NetworkEvents()...) + if err != nil { + return tk + } ol2 := logx.NewOperationLogger(logger, "echcheck: DialTLS%s", d) start := time.Now() diff --git a/internal/experiment/echcheck/handshake_test.go b/internal/experiment/echcheck/handshake_test.go index f71859f0a..ee0474936 100644 --- a/internal/experiment/echcheck/handshake_test.go +++ b/internal/experiment/echcheck/handshake_test.go @@ -6,6 +6,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "net" "net/http" "net/http/httptest" "net/url" @@ -158,3 +159,40 @@ func TestHandshake(t *testing.T) { }) } } + +func TestHandshakeTCPConnectFailure(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + address := l.Addr().String() + l.Close() + + parsed := &url.URL{Scheme: "https", Host: address} + for _, sendGrease := range []bool{false, true} { + t.Run(fmt.Sprintf("grease=%v", sendGrease), func(t *testing.T) { + ecl := []byte{} + if sendGrease { + grease, err := generateGreaseyECHConfigList(rand.Reader, parsed.Hostname()) + if err != nil { + t.Fatal(err) + } + ecl = grease + } + ch, err := startHandshake(context.Background(), ecl, sendGrease, time.Now(), address, parsed, model.DiscardLogger, nil) + if err != nil { + t.Fatal(err) + } + result := <-ch + if len(result.TCPConnects) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnects)) + } + if result.TCPConnects[0].Status.Failure == nil { + t.Fatal("expected the TCPConnect to carry a failure") + } + if len(result.TLSHandshakes) != 0 { + t.Fatal("expected no TLS handshake without a connection, got: ", len(result.TLSHandshakes)) + } + }) + } +} diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index e1d2bd7d5..d7819b2fb 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -18,7 +18,7 @@ import ( const ( testName = "echcheck" - testVersion = "0.3.1" + testVersion = "0.3.2" defaultURL = "https://cloudflare-ech.com/cdn-cgi/trace" ) diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index 1c6930017..a4ac50a27 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -16,7 +16,7 @@ func TestNewExperimentMeasurer(t *testing.T) { if measurer.ExperimentName() != "echcheck" { t.Fatal("unexpected name") } - if measurer.ExperimentVersion() != "0.3.1" { + if measurer.ExperimentVersion() != "0.3.2" { t.Fatal("unexpected version") } }