diff --git a/pkg/server/router/tcp/router.go b/pkg/server/router/tcp/router.go index ad116cd0a5..2d9c003fa9 100644 --- a/pkg/server/router/tcp/router.go +++ b/pkg/server/router/tcp/router.go @@ -18,7 +18,15 @@ import ( "github.com/traefik/traefik/v2/pkg/tcp" ) -const defaultBufSize = 4096 +const ( + defaultBufSize = 4096 + // Per RFC 8446 Section 5.1, the maximum TLS record payload length is 2^14 (16384) bytes. + // A ClientHello is always a plaintext record, so any value exceeding this limit is invalid + // and likely indicates an attack attempting to force oversized per-connection buffer allocations. + // However, in practice the go server handshake can read up to 16384 + 2048 bytes, + // so we need to allow for some extra bytes to avoid rejecting valid handshakes. + maxTLSRecordLen = 16384 + 2048 +) // Router is a TCP router. type Router struct { @@ -395,6 +403,14 @@ func clientHelloInfo(br *bufio.Reader) (*clientHello, error) { 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 + } + if recordHeaderLen+recLen > defaultBufSize { br = bufio.NewReaderSize(br, recordHeaderLen+recLen) } diff --git a/pkg/server/router/tcp/router_test.go b/pkg/server/router/tcp/router_test.go index 9783bd591b..918c21da35 100644 --- a/pkg/server/router/tcp/router_test.go +++ b/pkg/server/router/tcp/router_test.go @@ -1,6 +1,7 @@ package tcp import ( + "bufio" "bytes" "crypto/tls" "errors" @@ -630,6 +631,7 @@ func Test_Routing(t *testing.T) { _ = serverHTTPS.Serve(httpsForwarder) }() + // The HTTPS forwarder will be added as tcp.TLSHandler (to handle TLS). router.SetHTTPSForwarder(httpsForwarder) stoppedTCP := make(chan struct{}) @@ -1063,3 +1065,115 @@ func checkHTTPSTLS10(addr string, timeout time.Duration) error { func checkHTTPSTLS12(addr string, timeout time.Duration) error { return checkHTTPS(addr, timeout, tls.VersionTLS12) } + +// Test_clientHelloInfo_oversizedRecordLength verifies that clientHelloInfo +// does not block or allocate excessive memory when a client sends a TLS +// record header with a maliciously large record length (up to 0xFFFF). +// +// Without the fix, clientHelloInfo allocates a ~65KB bufio.Reader and blocks +// on Peek(65540), waiting for bytes that never arrive (until readTimeout). +// With the fix, records exceeding the TLS maximum plaintext size (16384) +// are rejected immediately. +func Test_clientHelloInfo_oversizedRecordLength(t *testing.T) { + testCases := []struct { + desc string + recLen uint16 + }{ + { + desc: "max uint16 record length (0xFFFF)", + recLen: 0xFFFF, + }, + { + desc: "just above TLS maximum (18433)", + recLen: 18433, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + serverConn, clientConn := net.Pipe() + defer serverConn.Close() + defer clientConn.Close() + + type result struct { + hello *clientHello + err error + } + resultCh := make(chan result, 1) + + go func() { + br := bufio.NewReader(serverConn) + hello, err := clientHelloInfo(br) + resultCh <- result{hello, err} + }() + + // Send a TLS record header with an oversized record length. + // Only the 5-byte header is sent; the client then stalls. + hdr := []byte{ + 0x16, // Content Type: Handshake + 0x03, 0x03, // Version: TLS 1.2 + byte(test.recLen >> 8), // Length high byte + byte(test.recLen & 0xFF), // Length low byte + } + _, err := clientConn.Write(hdr) + require.NoError(t, err) + + // Without the fix, clientHelloInfo blocks on Peek(recLen+5) + // since only 5 bytes are available. The test would time out. + // 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) + case <-time.After(5 * time.Second): + t.Fatal("clientHelloInfo blocked on oversized TLS record length — recLen is not capped") + } + }) + } +} + +// Test_clientHelloInfo_validRecordLength verifies that clientHelloInfo +// still works correctly with legitimate TLS record sizes. +func Test_clientHelloInfo_validRecordLength(t *testing.T) { + serverConn, clientConn := net.Pipe() + defer serverConn.Close() + defer clientConn.Close() + + type result struct { + hello *clientHello + err error + } + resultCh := make(chan result, 1) + + go func() { + br := bufio.NewReader(serverConn) + hello, err := clientHelloInfo(br) + resultCh <- result{hello, err} + }() + + // Build a TLS record header with a small (valid) record length. + recLen := 100 + hdr := []byte{ + 0x16, // Content Type: Handshake + 0x03, 0x03, // Version: TLS 1.2 + byte(recLen >> 8), // Length high byte + byte(recLen & 0xFF), // Length low byte + } + payload := make([]byte, recLen) + + _, err := clientConn.Write(append(hdr, payload...)) + require.NoError(t, err) + clientConn.Close() + + select { + case r := <-resultCh: + require.NoError(t, r.err) + require.NotNil(t, r.hello) + assert.True(t, r.hello.isTLS) + case <-time.After(5 * time.Second): + t.Fatal("clientHelloInfo blocked on valid TLS record") + } +}