Reject empty ALPN protocols.

https://tools.ietf.org/html/rfc7301#section-3.1 specifies that a
ProtocolName may not be empty. This change enforces this in ClientHello
and ServerHello messages.

Thanks to Doug Hogan for reporting this.

Change-Id: Iab879c83145007799b94d2725201ede1a39e4596
Reviewed-on: https://boringssl-review.googlesource.com/5390
Reviewed-by: Adam Langley <agl@google.com>

ssl/test/runner/common.go

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 2e30208..bad3ebe 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -544,6 +544,10 @@
 	// of ALPN works regardless of their relative order.
 	SwapNPNAndALPN bool
 
+	// ALPNProtocol, if not nil, sets the ALPN protocol that a server will
+	// return.
+	ALPNProtocol *string
+
 	// AllowSessionVersionMismatch causes the server to resume sessions
 	// regardless of the version associated with the session.
 	AllowSessionVersionMismatch bool

ssl/test/runner/handshake_client.go

diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 951b956..bc10fe7 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -45,7 +45,7 @@
 
 	nextProtosLength := 0
 	for _, proto := range c.config.NextProtos {
-		if l := len(proto); l == 0 || l > 255 {
+		if l := len(proto); l > 255 {
 			return errors.New("tls: invalid NextProtos value")
 		} else {
 			nextProtosLength += 1 + l

ssl/test/runner/handshake_messages.go

diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index ce214fd..4fee9d4 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -119,7 +119,7 @@
 	if len(m.alpnProtocols) > 0 {
 		extensionsLength += 2
 		for _, s := range m.alpnProtocols {
-			if l := len(s); l == 0 || l > 255 {
+			if l := len(s); l > 255 {
 				panic("invalid ALPN protocol")
 			}
 			extensionsLength++
@@ -625,6 +625,7 @@
 	ticketSupported         bool
 	secureRenegotiation     []byte
 	alpnProtocol            string
+	alpnProtocolEmpty       bool
 	duplicateExtension      bool
 	channelIDRequested      bool
 	extendedMasterSecret    bool
@@ -653,6 +654,7 @@
 		bytes.Equal(m.secureRenegotiation, m1.secureRenegotiation) &&
 		(m.secureRenegotiation == nil) == (m1.secureRenegotiation == nil) &&
 		m.alpnProtocol == m1.alpnProtocol &&
+		m.alpnProtocolEmpty == m1.alpnProtocolEmpty &&
 		m.duplicateExtension == m1.duplicateExtension &&
 		m.channelIDRequested == m1.channelIDRequested &&
 		m.extendedMasterSecret == m1.extendedMasterSecret &&
@@ -695,7 +697,7 @@
 	if m.channelIDRequested {
 		numExtensions++
 	}
-	if alpnLen := len(m.alpnProtocol); alpnLen > 0 {
+	if alpnLen := len(m.alpnProtocol); alpnLen > 0 || m.alpnProtocolEmpty {
 		if alpnLen >= 256 {
 			panic("invalid ALPN protocol")
 		}
@@ -784,7 +786,7 @@
 		copy(z, m.secureRenegotiation)
 		z = z[len(m.secureRenegotiation):]
 	}
-	if alpnLen := len(m.alpnProtocol); alpnLen > 0 {
+	if alpnLen := len(m.alpnProtocol); alpnLen > 0 || m.alpnProtocolEmpty {
 		z[0] = byte(extensionALPN >> 8)
 		z[1] = byte(extensionALPN & 0xff)
 		l := 2 + 1 + alpnLen
@@ -869,6 +871,7 @@
 	m.ocspStapling = false
 	m.ticketSupported = false
 	m.alpnProtocol = ""
+	m.alpnProtocolEmpty = false
 	m.extendedMasterSecret = false
 
 	if len(data) == 0 {
@@ -940,6 +943,7 @@
 			}
 			d = d[1:]
 			m.alpnProtocol = string(d)
+			m.alpnProtocolEmpty = len(d) == 0
 		case extensionChannelID:
 			if length > 0 {
 				return false

ssl/test/runner/handshake_server.go

diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 220e30ca..3902ed3 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -285,7 +285,12 @@
 	}
 
 	if len(hs.clientHello.alpnProtocols) > 0 {
-		if selectedProto, fallback := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos); !fallback {
+		if proto := c.config.Bugs.ALPNProtocol; proto != nil {
+			hs.hello.alpnProtocol = *proto
+			hs.hello.alpnProtocolEmpty = len(*proto) == 0
+			c.clientProtocol = *proto
+			c.usedALPN = true
+		} else if selectedProto, fallback := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos); !fallback {
 			hs.hello.alpnProtocol = selectedProto
 			c.clientProtocol = selectedProto
 			c.usedALPN = true

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 2b56462..9dee3b3 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2829,6 +2829,38 @@
 		expectedNextProtoType: alpn,
 		resumeSession:         true,
 	})
+	var emptyString string
+	testCases = append(testCases, testCase{
+		testType: clientTest,
+		name:     "ALPNClient-EmptyProtocolName",
+		config: Config{
+			NextProtos: []string{""},
+			Bugs: ProtocolBugs{
+				// A server returning an empty ALPN protocol
+				// should be rejected.
+				ALPNProtocol: &emptyString,
+			},
+		},
+		flags: []string{
+			"-advertise-alpn", "\x03foo",
+		},
+		shouldFail: true,
+		expectedError: ":PARSE_TLSEXT:",
+	})
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "ALPNServer-EmptyProtocolName",
+		config: Config{
+			// A ClientHello containing an empty ALPN protocol
+			// should be rejected.
+			NextProtos: []string{"foo", "", "baz"},
+		},
+		flags: []string{
+			"-select-alpn", "foo",
+		},
+		shouldFail: true,
+		expectedError: ":PARSE_TLSEXT:",
+	})
 	// Resume with a corrupt ticket.
 	testCases = append(testCases, testCase{
 		testType: serverTest,