crypto/tls: rewrite some messages with golang.org/x/crypto/cryptobyte

As a first round, rewrite those handshake message types which can be
reused in TLS 1.3 with golang.org/x/crypto/cryptobyte. All other types
changed significantly in TLS 1.3 and will require separate
implementations. They will be ported to cryptobyte in a later CL.

The only semantic changes should be enforcing the random length on the
marshaling side, enforcing a couple more "must not be empty" on the
unmarshaling side, and checking the rest of the SNI list even if we only
take the first.

Change-Id: Idd2ced60c558fafcf02ee489195b6f3b4735fe22
Reviewed-on: https://go-review.googlesource.com/c/144115
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

conn.go

@@ -899,7 +899,7 @@ func (c *Conn) readHandshake() (interface{}, error) {
 		m = new(certificateMsg)
 	case typeCertificateRequest:
 		m = &certificateRequestMsg{
-			hasSignatureAndHash: c.vers >= VersionTLS12,
+			hasSignatureAlgorithm: c.vers >= VersionTLS12,
 		}
 	case typeCertificateStatus:
 		m = new(certificateStatusMsg)
@@ -911,7 +911,7 @@ func (c *Conn) readHandshake() (interface{}, error) {
 		m = new(clientKeyExchangeMsg)
 	case typeCertificateVerify:
 		m = &certificateVerifyMsg{
-			hasSignatureAndHash: c.vers >= VersionTLS12,
+			hasSignatureAlgorithm: c.vers >= VersionTLS12,
 		}
 	case typeNextProtocol:
 		m = new(nextProtoMsg)

handshake_client.go

@@ -471,7 +471,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 
 	if chainToSend != nil && len(chainToSend.Certificate) > 0 {
 		certVerify := &certificateVerifyMsg{
-			hasSignatureAndHash: c.vers >= VersionTLS12,
+			hasSignatureAlgorithm: c.vers >= VersionTLS12,
 		}
 
 		key, ok := chainToSend.PrivateKey.(crypto.Signer)
@@ -486,7 +486,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 			return err
 		}
 		// SignatureAndHashAlgorithm was introduced in TLS 1.2.
-		if certVerify.hasSignatureAndHash {
+		if certVerify.hasSignatureAlgorithm {
 			certVerify.signatureAlgorithm = signatureAlgorithm
 		}
 		digest, err := hs.finishedHash.hashForClientCertificate(sigType, hashFunc, hs.masterSecret)
@@ -739,7 +739,7 @@ func (hs *clientHandshakeState) getCertificate(certReq *certificateRequestMsg) (
 	if c.config.GetClientCertificate != nil {
 		var signatureSchemes []SignatureScheme
 
-		if !certReq.hasSignatureAndHash {
+		if !certReq.hasSignatureAlgorithm {
 			// Prior to TLS 1.2, the signature schemes were not
 			// included in the certificate request message. In this
 			// case we use a plausible list based on the acceptable

handshake_messages_test.go

@@ -20,7 +20,9 @@ var tests = []interface{}{
 
 	&certificateMsg{},
 	&certificateRequestMsg{},
-	&certificateVerifyMsg{},
+	&certificateVerifyMsg{
+		hasSignatureAlgorithm: true,
+	},
 	&certificateStatusMsg{},
 	&clientKeyExchangeMsg{},
 	&nextProtoMsg{},
@@ -149,6 +151,10 @@ func (*clientHelloMsg) Generate(rand *rand.Rand, size int) reflect.Value {
 	if rand.Intn(10) > 5 {
 		m.scts = true
 	}
+	if rand.Intn(10) > 5 {
+		m.secureRenegotiationSupported = true
+		m.secureRenegotiation = randomBytes(rand.Intn(50)+1, rand)
+	}
 
 	return reflect.ValueOf(m)
 }
@@ -180,6 +186,11 @@ func (*serverHelloMsg) Generate(rand *rand.Rand, size int) reflect.Value {
 		m.scts = append(m.scts, randomBytes(rand.Intn(500)+1, rand))
 	}
 
+	if rand.Intn(10) > 5 {
+		m.secureRenegotiationSupported = true
+		m.secureRenegotiation = randomBytes(rand.Intn(50)+1, rand)
+	}
+
 	return reflect.ValueOf(m)
 }
 
@@ -204,6 +215,8 @@ func (*certificateRequestMsg) Generate(rand *rand.Rand, size int) reflect.Value
 
 func (*certificateVerifyMsg) Generate(rand *rand.Rand, size int) reflect.Value {
 	m := &certificateVerifyMsg{}
+	m.hasSignatureAlgorithm = true
+	m.signatureAlgorithm = SignatureScheme(rand.Intn(30000))
 	m.signature = randomBytes(rand.Intn(15)+1, rand)
 	return reflect.ValueOf(m)
 }

handshake_server.go

@@ -418,7 +418,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 			byte(certTypeECDSASign),
 		}
 		if c.vers >= VersionTLS12 {
-			certReq.hasSignatureAndHash = true
+			certReq.hasSignatureAlgorithm = true
 			certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms
 		}
 

handshake_server_test.go

@@ -101,13 +101,17 @@ var badProtocolVersions = []uint16{0x0000, 0x0005, 0x0100, 0x0105, 0x0200, 0x020
 
 func TestRejectBadProtocolVersion(t *testing.T) {
 	for _, v := range badProtocolVersions {
-		testClientHelloFailure(t, testConfig, &clientHelloMsg{vers: v}, "unsupported, maximum protocol version")
+		testClientHelloFailure(t, testConfig, &clientHelloMsg{
+			vers:   v,
+			random: make([]byte, 32),
+		}, "unsupported, maximum protocol version")
 	}
 }
 
 func TestNoSuiteOverlap(t *testing.T) {
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{0xff00},
 		compressionMethods: []uint8{compressionNone},
 	}
@@ -117,6 +121,7 @@ func TestNoSuiteOverlap(t *testing.T) {
 func TestNoCompressionOverlap(t *testing.T) {
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		compressionMethods: []uint8{0xff},
 	}
@@ -126,6 +131,7 @@ func TestNoCompressionOverlap(t *testing.T) {
 func TestNoRC4ByDefault(t *testing.T) {
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		compressionMethods: []uint8{compressionNone},
 	}
@@ -137,7 +143,11 @@ func TestNoRC4ByDefault(t *testing.T) {
 }
 
 func TestRejectSNIWithTrailingDot(t *testing.T) {
-	testClientHelloFailure(t, testConfig, &clientHelloMsg{vers: VersionTLS12, serverName: "foo.com."}, "unexpected message")
+	testClientHelloFailure(t, testConfig, &clientHelloMsg{
+		vers:       VersionTLS12,
+		random:     make([]byte, 32),
+		serverName: "foo.com.",
+	}, "unexpected message")
 }
 
 func TestDontSelectECDSAWithRSAKey(t *testing.T) {
@@ -145,6 +155,7 @@ func TestDontSelectECDSAWithRSAKey(t *testing.T) {
 	// won't be selected if the server's private key doesn't support it.
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA},
 		compressionMethods: []uint8{compressionNone},
 		supportedCurves:    []CurveID{CurveP256},
@@ -170,6 +181,7 @@ func TestDontSelectRSAWithECDSAKey(t *testing.T) {
 	// won't be selected if the server's private key doesn't support it.
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA},
 		compressionMethods: []uint8{compressionNone},
 		supportedCurves:    []CurveID{CurveP256},
@@ -242,11 +254,9 @@ func TestRenegotiationExtension(t *testing.T) {
 func TestTLS12OnlyCipherSuites(t *testing.T) {
 	// Test that a Server doesn't select a TLS 1.2-only cipher suite when
 	// the client negotiates TLS 1.1.
-	var zeros [32]byte
-
 	clientHello := &clientHelloMsg{
 		vers:   VersionTLS11,
-		random: zeros[:],
+		random: make([]byte, 32),
 		cipherSuites: []uint16{
 			// The Server, by default, will use the client's
 			// preference order. So the GCM cipher suite
@@ -878,6 +888,7 @@ func TestHandshakeServerSNIGetCertificateError(t *testing.T) {
 
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		compressionMethods: []uint8{compressionNone},
 		serverName:         "test",
@@ -898,6 +909,7 @@ func TestHandshakeServerEmptyCertificates(t *testing.T) {
 
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		compressionMethods: []uint8{compressionNone},
 	}
@@ -909,6 +921,7 @@ func TestHandshakeServerEmptyCertificates(t *testing.T) {
 
 	clientHello = &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		compressionMethods: []uint8{compressionNone},
 	}
@@ -1212,6 +1225,7 @@ func TestSNIGivenOnFailure(t *testing.T) {
 
 	clientHello := &clientHelloMsg{
 		vers:               VersionTLS10,
+		random:             make([]byte, 32),
 		cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
 		compressionMethods: []uint8{compressionNone},
 		serverName:         expectedServerName,