Skip to content

Commit

Permalink
crypto/tls: allow 256KiB certificate messages
Browse files Browse the repository at this point in the history
During handshake, lift the message length limit, but only for
certificate messages.

Fixes #50773

Change-Id: Ida9d83f4219c4386ca71ed3ef72b22259665a187
Reviewed-on: https://go-review.googlesource.com/c/go/+/585402
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
  • Loading branch information
rolandshoemaker authored and gopherbot committed May 23, 2024
1 parent 5fc5555 commit 726fd5b
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 8 deletions.
13 changes: 7 additions & 6 deletions src/crypto/tls/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,13 @@ func VersionName(version uint16) string {
}

const (
maxPlaintext = 16384 // maximum plaintext payload length
maxCiphertext = 16384 + 2048 // maximum ciphertext payload length
maxCiphertextTLS13 = 16384 + 256 // maximum ciphertext length in TLS 1.3
recordHeaderLen = 5 // record header length
maxHandshake = 65536 // maximum handshake we support (protocol max is 16 MB)
maxUselessRecords = 16 // maximum number of consecutive non-advancing records
maxPlaintext = 16384 // maximum plaintext payload length
maxCiphertext = 16384 + 2048 // maximum ciphertext payload length
maxCiphertextTLS13 = 16384 + 256 // maximum ciphertext length in TLS 1.3
recordHeaderLen = 5 // record header length
maxHandshake = 65536 // maximum handshake we support (protocol max is 16 MB)
maxHandshakeCertificateMsg = 262144 // maximum certificate message size (256 KiB)
maxUselessRecords = 16 // maximum number of consecutive non-advancing records
)

// TLS record types.
Expand Down
16 changes: 14 additions & 2 deletions src/crypto/tls/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -1089,10 +1089,22 @@ func (c *Conn) readHandshake(transcript transcriptHash) (any, error) {
return nil, err
}
data := c.hand.Bytes()

maxHandshakeSize := maxHandshake
// hasVers indicates we're past the first message, forcing someone trying to
// make us just allocate a large buffer to at least do the initial part of
// the handshake first.
if c.haveVers && data[0] == typeCertificate {
// Since certificate messages are likely to be the only messages that
// can be larger than maxHandshake, we use a special limit for just
// those messages.
maxHandshakeSize = maxHandshakeCertificateMsg
}

n := int(data[1])<<16 | int(data[2])<<8 | int(data[3])
if n > maxHandshake {
if n > maxHandshakeSize {
c.sendAlertLocked(alertInternalError)
return nil, c.in.setErrorLocked(fmt.Errorf("tls: handshake message of length %d bytes exceeds maximum of %d bytes", n, maxHandshake))
return nil, c.in.setErrorLocked(fmt.Errorf("tls: handshake message of length %d bytes exceeds maximum of %d bytes", n, maxHandshakeSize))
}
if err := c.readHandshakeBytes(4 + n); err != nil {
return nil, err
Expand Down
56 changes: 56 additions & 0 deletions src/crypto/tls/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"encoding/json"
"encoding/pem"
"errors"
Expand Down Expand Up @@ -2002,3 +2003,58 @@ func TestX509KeyPairPopulateCertificate(t *testing.T) {
}
})
}

func TestEarlyLargeCertMsg(t *testing.T) {
client, server := localPipe(t)

go func() {
if _, err := client.Write([]byte{byte(recordTypeHandshake), 3, 4, 0, 4, typeCertificate, 1, 255, 255}); err != nil {
t.Log(err)
}
}()

expectedErr := "tls: handshake message of length 131071 bytes exceeds maximum of 65536 bytes"
servConn := Server(server, testConfig)
err := servConn.Handshake()
if err == nil {
t.Fatal("unexpected success")
}
if err.Error() != expectedErr {
t.Fatalf("unexpected error: got %q, want %q", err, expectedErr)
}
}

func TestLargeCertMsg(t *testing.T) {
k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatal(err)
}
tmpl := &x509.Certificate{
SerialNumber: big.NewInt(1),
Subject: pkix.Name{CommonName: "test"},
ExtraExtensions: []pkix.Extension{
{
Id: asn1.ObjectIdentifier{1, 2, 3},
// Ballast to inflate the certificate beyond the
// regular handshake record size.
Value: make([]byte, 65536),
},
},
}
cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, k.Public(), k)
if err != nil {
t.Fatal(err)
}

clientConfig, serverConfig := testConfig.Clone(), testConfig.Clone()
clientConfig.InsecureSkipVerify = true
serverConfig.Certificates = []Certificate{
{
Certificate: [][]byte{cert},
PrivateKey: k,
},
}
if _, _, err := testHandshake(t, clientConfig, serverConfig); err != nil {
t.Fatalf("unexpected failure :%s", err)
}
}

0 comments on commit 726fd5b

Please sign in to comment.