crypto/tls: enforce ALPN overlap when negotiated on both sides

During the TLS handshake if the server doesn't support any of the application protocols requested by the client, send the no_application_protocol alert and abort the handshake on the server side. This enforces the requirements of RFC 7301. Change-Id: Iced2bb5c6efc607497de1c40ee3de9c2b393fa5d Reviewed-on: https://go-review.googlesource.com/c/go/+/289209 Trust: Roland Shoemaker <roland@golang.org> Trust: Katie Hockman <katie@golang.org> Run-TryBot: Roland Shoemaker <roland@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
golang · May 6, 2021 · 90d6bbb · 90d6bbb
1 parent 402f177
commit 90d6bbb
Show file tree

Hide file tree

Showing 10 changed files with 303 additions and 180 deletions.
diff --git a/doc/go1.17.html b/doc/go1.17.html
@@ -216,6 +216,13 @@ <h3 id="crypto/tls"><a href="/pkg/crypto/tls">crypto/tls</a></h3>
   has no effect.
 </p>
 
+<p><!-- CL 289209 -->
+  When <a href="/pkg/crypto/tls#Config">Config.NextProtos</a> is set, servers now
+  enforce that there is an overlap between the configured protocols and the protocols
+  advertised by the client, if any. If there is no overlap the connection is closed
+  with the <code>no_application_protocol</code> alert, as required by RFC 7301.
+</p>
+
 <h3 id="runtime/cgo"><a href="/pkg/runtime/cgo">Cgo</a></h3>
 
 <p>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
@@ -618,7 +618,11 @@ type Config struct {
 	RootCAs *x509.CertPool
 
 	// NextProtos is a list of supported application level protocols, in
-	// order of preference.
+	// order of preference. If both peers support ALPN, the selected
+	// protocol will be one from this list, and the connection will fail
+	// if there is no mutually supported protocol. If NextProtos is empty
+	// or the peer doesn't support ALPN, the connection will succeed and
+	// ConnectionState.NegotiatedProtocol will be empty."
 	NextProtos []string
 
 	// ServerName is used to verify the hostname on the returned

diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
@@ -1224,6 +1224,56 @@ func TestHandshakeClientALPNMatch(t *testing.T) {
 	runClientTestTLS13(t, test)
 }
 
+func TestServerSelectingUnconfiguredApplicationProtocol(t *testing.T) {
+	// This checks that the server can't select an application protocol that the
+	// client didn't offer.
+
+	c, s := localPipe(t)
+	errChan := make(chan error, 1)
+
+	go func() {
+		client := Client(c, &Config{
+			ServerName:   "foo",
+			CipherSuites: []uint16{TLS_RSA_WITH_AES_128_GCM_SHA256},
+			NextProtos:   []string{"http", "something-else"},
+		})
+		errChan <- client.Handshake()
+	}()
+
+	var header [5]byte
+	if _, err := io.ReadFull(s, header[:]); err != nil {
+		t.Fatal(err)
+	}
+	recordLen := int(header[3])<<8 | int(header[4])
+
+	record := make([]byte, recordLen)
+	if _, err := io.ReadFull(s, record); err != nil {
+		t.Fatal(err)
+	}
+
+	serverHello := &serverHelloMsg{
+		vers:         VersionTLS12,
+		random:       make([]byte, 32),
+		cipherSuite:  TLS_RSA_WITH_AES_128_GCM_SHA256,
+		alpnProtocol: "how-about-this",
+	}
+	serverHelloBytes := serverHello.marshal()
+
+	s.Write([]byte{
+		byte(recordTypeHandshake),
+		byte(VersionTLS12 >> 8),
+		byte(VersionTLS12 & 0xff),
+		byte(len(serverHelloBytes) >> 8),
+		byte(len(serverHelloBytes)),
+	})
+	s.Write(serverHelloBytes)
+	s.Close()
+
+	if err := <-errChan; !strings.Contains(err.Error(), "server selected unadvertised ALPN protocol") {
+		t.Fatalf("Expected error about unconfigured cipher suite but got %q", err)
+	}
+}
+
 // sctsBase64 contains data from `openssl s_client -serverinfo 18 -connect ritter.vg:443`
 const sctsBase64 = "ABIBaQFnAHUApLkJkLQYWBSHuxOizGdwCjw1mAT5G9+443fNDsgN3BAAAAFHl5nuFgAABAMARjBEAiAcS4JdlW5nW9sElUv2zvQyPoZ6ejKrGGB03gjaBZFMLwIgc1Qbbn+hsH0RvObzhS+XZhr3iuQQJY8S9G85D9KeGPAAdgBo9pj4H2SCvjqM7rkoHUz8cVFdZ5PURNEKZ6y7T0/7xAAAAUeX4bVwAAAEAwBHMEUCIDIhFDgG2HIuADBkGuLobU5a4dlCHoJLliWJ1SYT05z6AiEAjxIoZFFPRNWMGGIjskOTMwXzQ1Wh2e7NxXE1kd1J0QsAdgDuS723dc5guuFCaR+r4Z5mow9+X7By2IMAxHuJeqj9ywAAAUhcZIqHAAAEAwBHMEUCICmJ1rBT09LpkbzxtUC+Hi7nXLR0J+2PmwLp+sJMuqK+AiEAr0NkUnEVKVhAkccIFpYDqHOlZaBsuEhWWrYpg2RtKp0="
 

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
@@ -217,11 +217,14 @@ func (hs *serverHandshakeState) processClientHello() error {
 		c.serverName = hs.clientHello.serverName
 	}
 
-	if len(hs.clientHello.alpnProtocols) > 0 {
-		if selectedProto := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos); selectedProto != "" {
-			hs.hello.alpnProtocol = selectedProto
-			c.clientProtocol = selectedProto
-		}
+	if len(c.config.NextProtos) > 0 && len(hs.clientHello.alpnProtocols) > 0 {
+		selectedProto := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos)
+		if selectedProto == "" {
+			c.sendAlert(alertNoApplicationProtocol)
+			return fmt.Errorf("tls: client requested unsupported application protocols (%s)", hs.clientHello.alpnProtocols)
+		}
+		hs.hello.alpnProtocol = selectedProto
+		c.clientProtocol = selectedProto
 	}
 
 	hs.cert, err = c.config.getCertificate(clientHelloInfo(hs.ctx, c, hs.clientHello))

diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
@@ -920,13 +920,27 @@ func TestHandshakeServerALPNNoMatch(t *testing.T) {
 		name: "ALPN-NoMatch",
 		// Note that this needs OpenSSL 1.0.2 because that is the first
 		// version that supports the -alpn flag.
+		command:                       []string{"openssl", "s_client", "-alpn", "proto2,proto1", "-cipher", "ECDHE-RSA-CHACHA20-POLY1305", "-ciphersuites", "TLS_CHACHA20_POLY1305_SHA256"},
+		config:                        config,
+		expectHandshakeErrorIncluding: "client requested unsupported application protocol",
+	}
+	runServerTestTLS12(t, test)
+	runServerTestTLS13(t, test)
+}
+
+func TestHandshakeServerALPNNotConfigured(t *testing.T) {
+	config := testConfig.Clone()
+	config.NextProtos = nil
+
+	test := &serverTest{
+		name: "ALPN-NotConfigured",
+		// Note that this needs OpenSSL 1.0.2 because that is the first
+		// version that supports the -alpn flag.
 		command: []string{"openssl", "s_client", "-alpn", "proto2,proto1", "-cipher", "ECDHE-RSA-CHACHA20-POLY1305", "-ciphersuites", "TLS_CHACHA20_POLY1305_SHA256"},
 		config:  config,
 		validate: func(state ConnectionState) error {
-			// Rather than reject the connection, Go doesn't select
-			// a protocol when there is no overlap.
 			if state.NegotiatedProtocol != "" {
-				return fmt.Errorf("Got protocol %q, wanted ''", state.NegotiatedProtocol)
+				return fmt.Errorf("Got protocol %q, wanted nothing", state.NegotiatedProtocol)
 			}
 			return nil
 		},

diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
@@ -11,6 +11,7 @@ import (
 	"crypto/hmac"
 	"crypto/rsa"
 	"errors"
+	"fmt"
 	"hash"
 	"io"
 	"sync/atomic"
@@ -567,11 +568,14 @@ func (hs *serverHandshakeStateTLS13) sendServerParameters() error {
 
 	encryptedExtensions := new(encryptedExtensionsMsg)
 
-	if len(hs.clientHello.alpnProtocols) > 0 {
-		if selectedProto := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos); selectedProto != "" {
-			encryptedExtensions.alpnProtocol = selectedProto
-			c.clientProtocol = selectedProto
+	if len(c.config.NextProtos) > 0 && len(hs.clientHello.alpnProtocols) > 0 {
+		selectedProto := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos)
+		if selectedProto == "" {
+			c.sendAlert(alertNoApplicationProtocol)
+			return fmt.Errorf("tls: client requested unsupported application protocols (%s)", hs.clientHello.alpnProtocols)
 		}
+		encryptedExtensions.alpnProtocol = selectedProto
+		c.clientProtocol = selectedProto
 	}
 
 	hs.transcript.Write(encryptedExtensions.marshal())

diff --git a/src/crypto/tls/testdata/Server-TLSv12-ALPN-NoMatch b/src/crypto/tls/testdata/Server-TLSv12-ALPN-NoMatch
@@ -1,7 +1,7 @@
 >>> Flow 1 (client to server)
-00000000  16 03 01 00 9d 01 00 00  99 03 03 7f fc 15 86 d1  |................|
-00000010  83 09 78 47 8d cd 7b 88  b3 86 52 27 bc da bc 8d  |..xG..{...R'....|
-00000020  0e 5d 35 44 21 17 7b d9  67 b9 fb 00 00 04 cc a8  |.]5D!.{.g.......|
+00000000  16 03 01 00 9d 01 00 00  99 03 03 24 15 a8 f2 f5  |...........$....|
+00000010  53 02 78 f0 4c f7 82 3c  68 7d a0 b1 9a 0f 29 32  |S.x.L..<h}....)2|
+00000020  9c 38 cc e7 92 95 63 f2  30 53 46 00 00 04 cc a8  |.8....c.0SF.....|
 00000030  00 ff 01 00 00 6c 00 0b  00 04 03 00 01 02 00 0a  |.....l..........|
 00000040  00 0c 00 0a 00 1d 00 17  00 1e 00 19 00 18 00 23  |...............#|
 00000050  00 00 00 10 00 10 00 0e  06 70 72 6f 74 6f 32 06  |.........proto2.|
@@ -11,81 +11,4 @@
 00000090  03 03 02 03 03 01 02 01  03 02 02 02 04 02 05 02  |................|
 000000a0  06 02                                             |..|
 >>> Flow 2 (server to client)
-00000000  16 03 03 00 3b 02 00 00  37 03 03 00 00 00 00 00  |....;...7.......|
-00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
-00000020  00 00 00 44 4f 57 4e 47  52 44 01 00 cc a8 00 00  |...DOWNGRD......|
-00000030  0f 00 23 00 00 ff 01 00  01 00 00 0b 00 02 01 00  |..#.............|
-00000040  16 03 03 02 59 0b 00 02  55 00 02 52 00 02 4f 30  |....Y...U..R..O0|
-00000050  82 02 4b 30 82 01 b4 a0  03 02 01 02 02 09 00 e8  |..K0............|
-00000060  f0 9d 3f e2 5b ea a6 30  0d 06 09 2a 86 48 86 f7  |..?.[..0...*.H..|
-00000070  0d 01 01 0b 05 00 30 1f  31 0b 30 09 06 03 55 04  |......0.1.0...U.|
-00000080  0a 13 02 47 6f 31 10 30  0e 06 03 55 04 03 13 07  |...Go1.0...U....|
-00000090  47 6f 20 52 6f 6f 74 30  1e 17 0d 31 36 30 31 30  |Go Root0...16010|
-000000a0  31 30 30 30 30 30 30 5a  17 0d 32 35 30 31 30 31  |1000000Z..250101|
-000000b0  30 30 30 30 30 30 5a 30  1a 31 0b 30 09 06 03 55  |000000Z0.1.0...U|
-000000c0  04 0a 13 02 47 6f 31 0b  30 09 06 03 55 04 03 13  |....Go1.0...U...|
-000000d0  02 47 6f 30 81 9f 30 0d  06 09 2a 86 48 86 f7 0d  |.Go0..0...*.H...|
-000000e0  01 01 01 05 00 03 81 8d  00 30 81 89 02 81 81 00  |.........0......|
-000000f0  db 46 7d 93 2e 12 27 06  48 bc 06 28 21 ab 7e c4  |.F}...'.H..(!.~.|
-00000100  b6 a2 5d fe 1e 52 45 88  7a 36 47 a5 08 0d 92 42  |..]..RE.z6G....B|
-00000110  5b c2 81 c0 be 97 79 98  40 fb 4f 6d 14 fd 2b 13  |[.....y.@.Om..+.|
-00000120  8b c2 a5 2e 67 d8 d4 09  9e d6 22 38 b7 4a 0b 74  |....g....."8.J.t|
-00000130  73 2b c2 34 f1 d1 93 e5  96 d9 74 7b f3 58 9f 6c  |s+.4......t{.X.l|
-00000140  61 3c c0 b0 41 d4 d9 2b  2b 24 23 77 5b 1c 3b bd  |a<..A..++$#w[.;.|
-00000150  75 5d ce 20 54 cf a1 63  87 1d 1e 24 c4 f3 1d 1a  |u]. T..c...$....|
-00000160  50 8b aa b6 14 43 ed 97  a7 75 62 f4 14 c8 52 d7  |P....C...ub...R.|
-00000170  02 03 01 00 01 a3 81 93  30 81 90 30 0e 06 03 55  |........0..0...U|
-00000180  1d 0f 01 01 ff 04 04 03  02 05 a0 30 1d 06 03 55  |...........0...U|
-00000190  1d 25 04 16 30 14 06 08  2b 06 01 05 05 07 03 01  |.%..0...+.......|
-000001a0  06 08 2b 06 01 05 05 07  03 02 30 0c 06 03 55 1d  |..+.......0...U.|
-000001b0  13 01 01 ff 04 02 30 00  30 19 06 03 55 1d 0e 04  |......0.0...U...|
-000001c0  12 04 10 9f 91 16 1f 43  43 3e 49 a6 de 6d b6 80  |.......CC>I..m..|
-000001d0  d7 9f 60 30 1b 06 03 55  1d 23 04 14 30 12 80 10  |..`0...U.#..0...|
-000001e0  48 13 49 4d 13 7e 16 31  bb a3 01 d5 ac ab 6e 7b  |H.IM.~.1......n{|
-000001f0  30 19 06 03 55 1d 11 04  12 30 10 82 0e 65 78 61  |0...U....0...exa|
-00000200  6d 70 6c 65 2e 67 6f 6c  61 6e 67 30 0d 06 09 2a  |mple.golang0...*|
-00000210  86 48 86 f7 0d 01 01 0b  05 00 03 81 81 00 9d 30  |.H.............0|
-00000220  cc 40 2b 5b 50 a0 61 cb  ba e5 53 58 e1 ed 83 28  |.@+[P.a...SX...(|
-00000230  a9 58 1a a9 38 a4 95 a1  ac 31 5a 1a 84 66 3d 43  |.X..8....1Z..f=C|
-00000240  d3 2d d9 0b f2 97 df d3  20 64 38 92 24 3a 00 bc  |.-...... d8.$:..|
-00000250  cf 9c 7d b7 40 20 01 5f  aa d3 16 61 09 a2 76 fd  |..}.@ ._...a..v.|
-00000260  13 c3 cc e1 0c 5c ee b1  87 82 f1 6c 04 ed 73 bb  |.....\.....l..s.|
-00000270  b3 43 77 8d 0c 1c f1 0f  a1 d8 40 83 61 c9 4c 72  |.Cw.......@.a.Lr|
-00000280  2b 9d ae db 46 06 06 4d  f4 c1 b3 3e c0 d1 bd 42  |+...F..M...>...B|
-00000290  d4 db fe 3d 13 60 84 5c  21 d3 3b e9 fa e7 16 03  |...=.`.\!.;.....|
-000002a0  03 00 ac 0c 00 00 a8 03  00 1d 20 2f e5 7d a3 47  |.......... /.}.G|
-000002b0  cd 62 43 15 28 da ac 5f  bb 29 07 30 ff f6 84 af  |.bC.(.._.).0....|
-000002c0  c4 cf c2 ed 90 99 5f 58  cb 3b 74 08 04 00 80 b8  |......_X.;t.....|
-000002d0  a8 88 ac 85 ea 59 ac f1  41 e8 2d a2 76 3c 3b 4f  |.....Y..A.-.v<;O|
-000002e0  58 90 b7 03 74 4b 7a a7  5a 65 ea 08 9c cf e9 4d  |X...tKz.Ze.....M|
-000002f0  b4 8a ef f3 e1 d8 0a 83  0f 50 29 0b 59 77 90 e9  |.........P).Yw..|
-00000300  f3 e8 ca 6c b5 da e5 2b  95 47 e7 ed ff d6 3b 30  |...l...+.G....;0|
-00000310  45 61 2c af 5c 8c 4c df  bd c4 dc 28 dd d2 31 fa  |Ea,.\.L....(..1.|
-00000320  be 65 2b a4 cd 7c 41 29  4c 99 07 97 5c 2a 3c a7  |.e+..|A)L...\*<.|
-00000330  4d 9c ed 72 eb a1 a4 9e  db eb a0 cf c7 c2 b1 3b  |M..r...........;|
-00000340  5a d9 f8 f8 8e d5 07 81  f6 65 aa 0d 4f 4d 11 16  |Z........e..OM..|
-00000350  03 03 00 04 0e 00 00 00                           |........|
->>> Flow 3 (client to server)
-00000000  16 03 03 00 25 10 00 00  21 20 5f d2 13 b1 79 f6  |....%...! _...y.|
-00000010  f3 2a 21 f5 89 a3 22 29  73 30 14 60 1d 1e 77 8a  |.*!...")s0.`..w.|
-00000020  f4 1a 92 3f b0 04 06 98  1a 1e 14 03 03 00 01 01  |...?............|
-00000030  16 03 03 00 20 63 10 89  c0 c0 56 37 40 8c e8 5e  |.... c....V7@..^|
-00000040  7f f0 f0 e3 a0 8e d5 20  33 5f dd c3 16 e8 eb 6c  |....... 3_.....l|
-00000050  c3 a8 75 6d dc                                    |..um.|
->>> Flow 4 (server to client)
-00000000  16 03 03 00 8b 04 00 00  87 00 00 00 00 00 81 50  |...............P|
-00000010  46 ad c1 db a8 38 86 7b  2b bb fd d0 c3 42 3e 00  |F....8.{+....B>.|
-00000020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 94  |................|
-00000030  6f e0 18 83 51 ed 14 ef  68 ca 42 c5 4c e3 f1 12  |o...Q...h.B.L...|
-00000040  a1 17 a6 ee 99 af e8 06  65 d0 6d c1 4f ce 92 7c  |........e.m.O..||
-00000050  40 df 41 c1 90 e3 e0 d8  a1 95 da 38 25 26 ea b5  |@.A........8%&..|
-00000060  ca a9 42 5f 8c 55 d4 d2  73 a6 a2 b6 22 49 38 16  |..B_.U..s..."I8.|
-00000070  ec 70 52 f9 c0 12 18 9e  9b 4d e3 6d 49 b7 3b c0  |.pR......M.mI.;.|
-00000080  e9 53 9d 06 96 fc a9 06  8c 2a 7a c5 7d 48 47 ef  |.S.......*z.}HG.|
-00000090  14 03 03 00 01 01 16 03  03 00 20 19 27 38 37 bf  |.......... .'87.|
-000000a0  07 4e 2f 77 b9 73 4b dd  c8 f8 4c c5 f1 35 86 2b  |.N/w.sK...L..5.+|
-000000b0  97 7e 0f 89 4b bf db 81  76 8a 41 17 03 03 00 1d  |.~..K...v.A.....|
-000000c0  6d b8 c3 eb b1 5a f3 06  97 04 61 fc 82 74 5d a0  |m....Z....a..t].|
-000000d0  73 57 75 6e 66 53 3e 12  5e 0d 60 31 52 15 03 03  |sWunfS>.^.`1R...|
-000000e0  00 12 e4 93 fb 7b cb ee  d6 70 ac af 5f 8b 82 9b  |.....{...p.._...|
-000000f0  e5 0b 68 9c                                       |..h.|
+00000000  15 03 03 00 02 02 78                              |......x|