From c09945980a80a3b92362bd2e99a883051d2dd4d7 Mon Sep 17 00:00:00 2001 From: woodsaj Date: Thu, 17 Nov 2016 20:14:32 +0800 Subject: [PATCH] crypto/tls: reject CT extension with no SCTs included When the CT extension is enabled but no SCTs are present, the existing code calls "continue" which causes resizing the data byte slice to be skipped. In fact, such extensions should be rejected. Fixes #17958 Change-Id: Iad12da10d1ea72d04ae2e1012c28bb2636f06bcd Reviewed-on: https://go-review.googlesource.com/33265 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/crypto/tls/handshake_messages.go | 5 +-- src/crypto/tls/handshake_messages_test.go | 45 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/crypto/tls/handshake_messages.go b/src/crypto/tls/handshake_messages.go index ab8e60ae11..2ea4ddba36 100644 --- a/src/crypto/tls/handshake_messages.go +++ b/src/crypto/tls/handshake_messages.go @@ -802,12 +802,9 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool { } l := int(d[0])<<8 | int(d[1]) d = d[2:] - if len(d) != l { + if len(d) != l || l == 0 { return false } - if l == 0 { - continue - } m.scts = make([][]byte, 0, 3) for len(d) != 0 { diff --git a/src/crypto/tls/handshake_messages_test.go b/src/crypto/tls/handshake_messages_test.go index 95d825bd17..cb3634c538 100644 --- a/src/crypto/tls/handshake_messages_test.go +++ b/src/crypto/tls/handshake_messages_test.go @@ -5,6 +5,7 @@ package tls import ( + "bytes" "math/rand" "reflect" "testing" @@ -260,3 +261,47 @@ func (*sessionState) Generate(rand *rand.Rand, size int) reflect.Value { } return reflect.ValueOf(s) } + +func TestRejectEmptySCTList(t *testing.T) { + // https://tools.ietf.org/html/rfc6962#section-3.3.1 specifies that + // empty SCT lists are invalid. + + var random [32]byte + sct := []byte{0x42, 0x42, 0x42, 0x42} + serverHello := serverHelloMsg{ + vers: VersionTLS12, + random: random[:], + scts: [][]byte{sct}, + } + serverHelloBytes := serverHello.marshal() + + var serverHelloCopy serverHelloMsg + if !serverHelloCopy.unmarshal(serverHelloBytes) { + t.Fatal("Failed to unmarshal initial message") + } + + // Change serverHelloBytes so that the SCT list is empty + i := bytes.Index(serverHelloBytes, sct) + if i < 0 { + t.Fatal("Cannot find SCT in ServerHello") + } + + var serverHelloEmptySCT []byte + serverHelloEmptySCT = append(serverHelloEmptySCT, serverHelloBytes[:i-6]...) + // Append the extension length and SCT list length for an empty list. + serverHelloEmptySCT = append(serverHelloEmptySCT, []byte{0, 2, 0, 0}...) + serverHelloEmptySCT = append(serverHelloEmptySCT, serverHelloBytes[i+4:]...) + + // Update the handshake message length. + serverHelloEmptySCT[1] = byte((len(serverHelloEmptySCT) - 4) >> 16) + serverHelloEmptySCT[2] = byte((len(serverHelloEmptySCT) - 4) >> 8) + serverHelloEmptySCT[3] = byte(len(serverHelloEmptySCT) - 4) + + // Update the extensions length + serverHelloEmptySCT[42] = byte((len(serverHelloEmptySCT) - 44) >> 8) + serverHelloEmptySCT[43] = byte((len(serverHelloEmptySCT) - 44)) + + if serverHelloCopy.unmarshal(serverHelloEmptySCT) { + t.Fatal("Unmarshaled ServerHello with empty SCT list") + } +} -- 2.50.0