From 7a3ffcc3d9cb67fe00562d249c703dd3ee2390fb Mon Sep 17 00:00:00 2001 From: Julien Salleyron Date: Mon, 23 Feb 2026 14:06:05 +0100 Subject: [PATCH] Fix TLS handshake error handling --- integration/fixtures/simple_ddos.toml | 34 +++++++++++++++++ integration/simple_test.go | 54 +++++++++++++++++++++++++++ pkg/server/router/tcp/router.go | 29 +++++--------- pkg/server/router/tcp/router_test.go | 4 +- 4 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 integration/fixtures/simple_ddos.toml diff --git a/integration/fixtures/simple_ddos.toml b/integration/fixtures/simple_ddos.toml new file mode 100644 index 000000000..c079d3506 --- /dev/null +++ b/integration/fixtures/simple_ddos.toml @@ -0,0 +1,34 @@ +[global] +checkNewVersion = false +sendAnonymousUsage = false + +[api] +insecure = true +[log] +level = "DEBUG" + +[entryPoints] + +[entryPoints.web] +address = ":8000" +[entryPoints.web.transport.respondingTimeouts] +readTimeout="200ms" + + +[entryPoints.tcp] +address = ":8001" +[entryPoints.tcp.transport.respondingTimeouts] +readTimeout="200ms" + + +[providers.file] +filename = "{{ .SelfFilename }}" + + +[tcp.routers.withtls] +rule="HostSNI(`*`)" +service="noop" +[tcp.routers.withtls.tls] + +[[tcp.services.noop.loadBalancer.servers]] +address="127.0.0.1:8080" diff --git a/integration/simple_test.go b/integration/simple_test.go index c2f1392c4..e8938c894 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -1562,3 +1562,57 @@ func (s *SimpleSuite) TestEncodedCharactersDifferentEntryPoints() { require.NoError(s.T(), err) } } + +func (s *SimpleSuite) TestDDOS() { + s.createComposeProject("base") + + s.composeUp() + defer s.composeDown() + + file := s.adaptFile("fixtures/simple_ddos.toml", struct{}{}) + + _, output := s.cmdTraefik(withConfigFile(file)) + + defer func() { + if s.T().Failed() { + s.T().Log("---- Traefik Logs ----") + s.T().Log(output) + } + }() + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("HostSNI(`*`)")) + require.NoError(s.T(), err) + + // Try with an http router. + conn, err := net.Dial("tcp", "127.0.0.1:8000") + require.NoError(s.T(), err) + + waitForWritePartial(s.T(), conn) + + // Try with a tcp router only. + conn, err = net.Dial("tcp", "127.0.0.1:8001") + require.NoError(s.T(), err) + + waitForWritePartial(s.T(), conn) +} + +func waitForWritePartial(t *testing.T, conn net.Conn) { + t.Helper() + + end := make(chan struct{}) + go func() { + if _, err := conn.Write([]byte{0x16, 0x03, 0x03, 0x00, 0x10}); err != nil { + require.NoError(t, err) + } + + _, err := conn.Read(make([]byte, 1)) + require.ErrorIs(t, err, io.EOF) + + close(end) + }() + + select { + case <-end: + case <-time.After(500 * time.Millisecond): + t.Fatalf("timeout waiting for connection timeout") + } +} diff --git a/pkg/server/router/tcp/router.go b/pkg/server/router/tcp/router.go index 2d9c003fa..8b98c853d 100644 --- a/pkg/server/router/tcp/router.go +++ b/pkg/server/router/tcp/router.go @@ -6,6 +6,7 @@ import ( "context" "crypto/tls" "errors" + "fmt" "io" "net" "net/http" @@ -129,6 +130,11 @@ func (r *Router) ServeTCP(conn tcp.WriteCloser) { br := bufio.NewReader(conn) hello, err := clientHelloInfo(br) if err != nil { + var opErr *net.OpError + if !errors.Is(err, io.EOF) && (!errors.As(err, &opErr) || !opErr.Timeout()) { + log.WithoutContext().Debugf("Error while reading client hello: %s", err) + } + conn.Close() return } @@ -367,11 +373,7 @@ type clientHello struct { func clientHelloInfo(br *bufio.Reader) (*clientHello, error) { hdr, err := br.Peek(1) if err != nil { - var opErr *net.OpError - if !errors.Is(err, io.EOF) && (!errors.As(err, &opErr) || !opErr.Timeout()) { - log.WithoutContext().Debugf("Error while peeking first byte: %s", err) - } - return nil, err + return nil, fmt.Errorf("peeking first byte: %w", err) } // No valid TLS record has a type of 0x80, however SSLv2 handshakes start with an uint16 length @@ -395,20 +397,13 @@ func clientHelloInfo(br *bufio.Reader) (*clientHello, error) { const recordHeaderLen = 5 hdr, err = br.Peek(recordHeaderLen) if err != nil { - log.WithoutContext().Errorf("Error while peeking client hello headers: %s", err) - return &clientHello{ - peeked: getPeeked(br), - }, nil + return nil, fmt.Errorf("peeking client hello headers: %w", err) } recLen := int(hdr[3])<<8 | int(hdr[4]) // ignoring version in hdr[1:3] if recLen > maxTLSRecordLen { - log.WithoutContext().Debugf("Error while peeking client hello bytes, oversized record: %d", recLen) - return &clientHello{ - isTLS: true, - peeked: getPeeked(br), - }, nil + return nil, fmt.Errorf("peeking client hello bytes, oversized record: %d", recLen) } if recordHeaderLen+recLen > defaultBufSize { @@ -417,11 +412,7 @@ func clientHelloInfo(br *bufio.Reader) (*clientHello, error) { helloBytes, err := br.Peek(recordHeaderLen + recLen) if err != nil { - log.WithoutContext().Errorf("Error while peeking client hello bytes: %s", err) - return &clientHello{ - isTLS: true, - peeked: getPeeked(br), - }, nil + return nil, fmt.Errorf("peeking client hello bytes: %w", err) } sni := "" diff --git a/pkg/server/router/tcp/router_test.go b/pkg/server/router/tcp/router_test.go index 918c21da3..837923e57 100644 --- a/pkg/server/router/tcp/router_test.go +++ b/pkg/server/router/tcp/router_test.go @@ -1125,9 +1125,7 @@ func Test_clientHelloInfo_oversizedRecordLength(t *testing.T) { // With the fix, it returns immediately. select { case r := <-resultCh: - require.NoError(t, r.err) - require.NotNil(t, r.hello) - assert.True(t, r.hello.isTLS) + require.Error(t, r.err) case <-time.After(5 * time.Second): t.Fatal("clientHelloInfo blocked on oversized TLS record length — recLen is not capped") }