crypto/tls: reject duplicate extensions

Does what it says on the tin.

Fixes #51088

Change-Id: I12c0fa6bba1c1ce96c1ad31ba387c77a93f801c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/384894
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/crypto/tls/handshake_messages.go b/src/crypto/tls/handshake_messages.go
index 17cf85910fafd8603949d68a6360b972e3aa5802..7ab0f100b8bcee1762032f8158ed0c9d1a5c6191 100644 (file)
--- a/src/crypto/tls/handshake_messages.go
+++ b/src/crypto/tls/handshake_messages.go
@@ -384,6 +384,7 @@ func (m *clientHelloMsg) unmarshal(data []byte) bool {
                return false
        }
 
+       seenExts := make(map[uint16]bool)
        for !extensions.Empty() {
                var extension uint16
                var extData cryptobyte.String
@@ -392,6 +393,11 @@ func (m *clientHelloMsg) unmarshal(data []byte) bool {
                        return false
                }
 
+               if seenExts[extension] {
+                       return false
+               }
+               seenExts[extension] = true
+
                switch extension {
                case extensionServerName:
                        // RFC 6066, Section 3
@@ -750,6 +756,7 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool {
                return false
        }
 
+       seenExts := make(map[uint16]bool)
        for !extensions.Empty() {
                var extension uint16
                var extData cryptobyte.String
@@ -758,6 +765,11 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool {
                        return false
                }
 
+               if seenExts[extension] {
+                       return false
+               }
+               seenExts[extension] = true
+
                switch extension {
                case extensionStatusRequest:
                        m.ocspStapling = true

diff --git a/src/crypto/tls/handshake_messages_test.go b/src/crypto/tls/handshake_messages_test.go
index cc427bf72a01726b54b1748f696174a83fd504a8..49452da8b4ff1dba97b7a240b47c44d207085d0a 100644 (file)
--- a/src/crypto/tls/handshake_messages_test.go
+++ b/src/crypto/tls/handshake_messages_test.go
@@ -6,6 +6,7 @@ package tls
 
 import (
        "bytes"
+       "encoding/hex"
        "math/rand"
        "reflect"
        "strings"
@@ -463,3 +464,23 @@ func TestRejectEmptySCT(t *testing.T) {
                t.Fatal("Unmarshaled ServerHello with zero-length SCT")
        }
 }
+
+func TestRejectDuplicateExtensions(t *testing.T) {
+       clientHelloBytes, err := hex.DecodeString("010000440303000000000000000000000000000000000000000000000000000000000000000000000000001c0000000a000800000568656c6c6f0000000a000800000568656c6c6f")
+       if err != nil {
+               t.Fatalf("failed to decode test ClientHello: %s", err)
+       }
+       var clientHelloCopy clientHelloMsg
+       if clientHelloCopy.unmarshal(clientHelloBytes) {
+               t.Error("Unmarshaled ClientHello with duplicate extensions")
+       }
+
+       serverHelloBytes, err := hex.DecodeString("02000030030300000000000000000000000000000000000000000000000000000000000000000000000000080005000000050000")
+       if err != nil {
+               t.Fatalf("failed to decode test ServerHello: %s", err)
+       }
+       var serverHelloCopy serverHelloMsg
+       if serverHelloCopy.unmarshal(serverHelloBytes) {
+               t.Fatal("Unmarshaled ServerHello with duplicate extensions")
+       }
+}