mirror of
https://github.com/traefik/traefik.git
synced 2026-02-18 18:20:28 -05:00
Cap TLS record length to RFC 8446 limit in ClientHello peeking
This commit is contained in:
parent
0beed101ec
commit
72e2454e42
2 changed files with 131 additions and 1 deletions
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue