Test client auth under TLS 1.2 hash mismatch and SSL 3.

Maintain a handshake buffer in prf.go to implement TLS 1.2 client auth. Also
use it for SSL 3. This isn't strictly necessary as we know the hash functions,
but Go's hash.Hash interface lacks a Copy method.

Also fix the server-side tests which failed to test every TLS version.

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

5 files changed, 96 insertions, 74 deletions

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 0dcb084..78e484f 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -138,24 +138,24 @@ const (
 // signatureAndHash mirrors the TLS 1.2, SignatureAndHashAlgorithm struct. See
 // RFC 5246, section A.4.1.
 type signatureAndHash struct {
-	hash, signature uint8
+	signature, hash uint8
 }
 
 // supportedSKXSignatureAlgorithms contains the signature and hash algorithms
 // that the code advertises as supported in a TLS 1.2 ClientHello.
 var supportedSKXSignatureAlgorithms = []signatureAndHash{
-	{hashSHA256, signatureRSA},
-	{hashSHA256, signatureECDSA},
-	{hashSHA1, signatureRSA},
-	{hashSHA1, signatureECDSA},
+	{signatureRSA, hashSHA256},
+	{signatureECDSA, hashSHA256},
+	{signatureRSA, hashSHA1},
+	{signatureECDSA, hashSHA1},
 }
 
 // supportedClientCertSignatureAlgorithms contains the signature and hash
 // algorithms that the code advertises as supported in a TLS 1.2
 // CertificateRequest.
 var supportedClientCertSignatureAlgorithms = []signatureAndHash{
-	{hashSHA256, signatureRSA},
-	{hashSHA256, signatureECDSA},
+	{signatureRSA, hashSHA256},
+	{signatureECDSA, hashSHA256},
 }
 
 // ConnectionState records basic TLS details about the connection.
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index c683913..708d282 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -474,6 +474,8 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 		c.writeRecord(recordTypeHandshake, ckx.marshal())
 	}
 
+	hs.masterSecret = masterFromPreMasterSecret(c.vers, hs.suite, preMasterSecret, hs.hello.random, hs.serverHello.random)
+
 	if chainToSend != nil {
 		var signed []byte
 		certVerify := &certificateVerifyMsg{
@@ -487,7 +489,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 				break
 			}
 			var digest []byte
-			digest, _, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash)
+			digest, _, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
 			if err != nil {
 				break
 			}
@@ -503,7 +505,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 			}
 			var digest []byte
 			var hashFunc crypto.Hash
-			digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash)
+			digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
 			if err != nil {
 				break
 			}
@@ -521,7 +523,8 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 		c.writeRecord(recordTypeHandshake, certVerify.marshal())
 	}
 
-	hs.masterSecret = masterFromPreMasterSecret(c.vers, hs.suite, preMasterSecret, hs.hello.random, hs.serverHello.random)
+	hs.finishedHash.discardHandshakeBuffer()
+
 	return nil
 }
 
@@ -576,6 +579,7 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) {
 		// Restore masterSecret and peerCerts from previous state
 		hs.masterSecret = hs.session.masterSecret
 		c.peerCertificates = hs.session.serverCertificates
+		hs.finishedHash.discardHandshakeBuffer()
 		return true, nil
 	}
 	return false, nil
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 72fa502..855f992 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -342,6 +342,7 @@ func (hs *serverHandshakeState) doResumeHandshake() error {
 	hs.hello.ticketSupported = c.config.Bugs.RenewTicketOnResume
 
 	hs.finishedHash = newFinishedHash(c.vers, hs.suite)
+	hs.finishedHash.discardHandshakeBuffer()
 	hs.writeClientHash(hs.clientHello.marshal())
 	hs.writeServerHash(hs.hello.marshal())
 
@@ -478,6 +479,13 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 	}
 	hs.writeClientHash(ckx.marshal())
 
+	preMasterSecret, err := keyAgreement.processClientKeyExchange(config, hs.cert, ckx, c.vers)
+	if err != nil {
+		c.sendAlert(alertHandshakeFailure)
+		return err
+	}
+	hs.masterSecret = masterFromPreMasterSecret(c.vers, hs.suite, preMasterSecret, hs.clientHello.random, hs.hello.random)
+
 	// If we received a client cert in response to our certificate request message,
 	// the client will send us a certificateVerifyMsg immediately after the
 	// clientKeyExchangeMsg.  This message is a digest of all preceding
@@ -526,7 +534,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 				break
 			}
 			var digest []byte
-			digest, _, err = hs.finishedHash.hashForClientCertificate(signatureAndHash)
+			digest, _, err = hs.finishedHash.hashForClientCertificate(signatureAndHash, hs.masterSecret)
 			if err != nil {
 				break
 			}
@@ -541,7 +549,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 			}
 			var digest []byte
 			var hashFunc crypto.Hash
-			digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(signatureAndHash)
+			digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(signatureAndHash, hs.masterSecret)
 			if err != nil {
 				break
 			}
@@ -555,12 +563,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 		hs.writeClientHash(certVerify.marshal())
 	}
 
-	preMasterSecret, err := keyAgreement.processClientKeyExchange(config, hs.cert, ckx, c.vers)
-	if err != nil {
-		c.sendAlert(alertHandshakeFailure)
-		return err
-	}
-	hs.masterSecret = masterFromPreMasterSecret(c.vers, hs.suite, preMasterSecret, hs.clientHello.random, hs.hello.random)
+	hs.finishedHash.discardHandshakeBuffer()
 
 	return nil
 }
diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go
index 55a3614..6d0db97 100644
--- a/ssl/test/runner/prf.go
+++ b/ssl/test/runner/prf.go
@@ -182,9 +182,9 @@ func newFinishedHash(version uint16, cipherSuite *cipherSuite) finishedHash {
 			newHash = sha512.New384
 		}
 
-		return finishedHash{newHash(), newHash(), nil, nil, version, prf12(newHash)}
+		return finishedHash{newHash(), newHash(), nil, nil, []byte{}, version, prf12(newHash)}
 	}
-	return finishedHash{sha1.New(), sha1.New(), md5.New(), md5.New(), version, prf10}
+	return finishedHash{sha1.New(), sha1.New(), md5.New(), md5.New(), []byte{}, version, prf10}
 }
 
 // A finishedHash calculates the hash of a set of handshake messages suitable
@@ -197,11 +197,15 @@ type finishedHash struct {
 	clientMD5 hash.Hash
 	serverMD5 hash.Hash
 
+	// In TLS 1.2 (and SSL 3 for implementation convenience), a
+	// full buffer is required.
+	buffer []byte
+
 	version uint16
 	prf     func(result, secret, label, seed []byte)
 }
 
-func (h finishedHash) Write(msg []byte) (n int, err error) {
+func (h *finishedHash) Write(msg []byte) (n int, err error) {
 	h.client.Write(msg)
 	h.server.Write(msg)
 
@@ -209,14 +213,19 @@ func (h finishedHash) Write(msg []byte) (n int, err error) {
 		h.clientMD5.Write(msg)
 		h.serverMD5.Write(msg)
 	}
+
+	if h.buffer != nil {
+		h.buffer = append(h.buffer, msg...)
+	}
+
 	return len(msg), nil
 }
 
 // finishedSum30 calculates the contents of the verify_data member of a SSLv3
 // Finished message given the MD5 and SHA1 hashes of a set of handshake
 // messages.
-func finishedSum30(md5, sha1 hash.Hash, masterSecret []byte, magic [4]byte) []byte {
-	md5.Write(magic[:])
+func finishedSum30(md5, sha1 hash.Hash, masterSecret []byte, magic []byte) []byte {
+	md5.Write(magic)
 	md5.Write(masterSecret)
 	md5.Write(ssl30Pad1[:])
 	md5Digest := md5.Sum(nil)
@@ -227,7 +236,7 @@ func finishedSum30(md5, sha1 hash.Hash, masterSecret []byte, magic [4]byte) []by
 	md5.Write(md5Digest)
 	md5Digest = md5.Sum(nil)
 
-	sha1.Write(magic[:])
+	sha1.Write(magic)
 	sha1.Write(masterSecret)
 	sha1.Write(ssl30Pad1[:40])
 	sha1Digest := sha1.Sum(nil)
@@ -251,7 +260,7 @@ var ssl3ServerFinishedMagic = [4]byte{0x53, 0x52, 0x56, 0x52}
 // Finished message.
 func (h finishedHash) clientSum(masterSecret []byte) []byte {
 	if h.version == VersionSSL30 {
-		return finishedSum30(h.clientMD5, h.client, masterSecret, ssl3ClientFinishedMagic)
+		return finishedSum30(h.clientMD5, h.client, masterSecret, ssl3ClientFinishedMagic[:])
 	}
 
 	out := make([]byte, finishedVerifyLength)
@@ -271,7 +280,7 @@ func (h finishedHash) clientSum(masterSecret []byte) []byte {
 // Finished message.
 func (h finishedHash) serverSum(masterSecret []byte) []byte {
 	if h.version == VersionSSL30 {
-		return finishedSum30(h.serverMD5, h.server, masterSecret, ssl3ServerFinishedMagic)
+		return finishedSum30(h.serverMD5, h.server, masterSecret, ssl3ServerFinishedMagic[:])
 	}
 
 	out := make([]byte, finishedVerifyLength)
@@ -292,7 +301,7 @@ func (h finishedHash) serverSum(masterSecret []byte) []byte {
 func (h finishedHash) selectClientCertSignatureAlgorithm(serverList []signatureAndHash, sigType uint8) (signatureAndHash, error) {
 	if h.version < VersionTLS12 {
 		// Nothing to negotiate before TLS 1.2.
-		return signatureAndHash{sigType, 0}, nil
+		return signatureAndHash{signature: sigType}, nil
 	}
 
 	for _, v := range serverList {
@@ -305,13 +314,24 @@ func (h finishedHash) selectClientCertSignatureAlgorithm(serverList []signatureA
 
 // hashForClientCertificate returns a digest, hash function, and TLS 1.2 hash
 // id suitable for signing by a TLS client certificate.
-func (h finishedHash) hashForClientCertificate(signatureAndHash signatureAndHash) ([]byte, crypto.Hash, error) {
+func (h finishedHash) hashForClientCertificate(signatureAndHash signatureAndHash, masterSecret []byte) ([]byte, crypto.Hash, error) {
+	if h.version == VersionSSL30 {
+		if signatureAndHash.signature != signatureRSA {
+			return nil, 0, errors.New("tls: unsupported signature type for client certificate")
+		}
+
+		md5Hash := md5.New()
+		md5Hash.Write(h.buffer)
+		sha1Hash := sha1.New()
+		sha1Hash.Write(h.buffer)
+		return finishedSum30(md5Hash, sha1Hash, masterSecret, nil), crypto.MD5SHA1, nil
+	}
 	if h.version >= VersionTLS12 {
 		if signatureAndHash.hash != hashSHA256 {
 			return nil, 0, errors.New("tls: unsupported hash function for client certificate")
 		}
-		digest := h.server.Sum(nil)
-		return digest, crypto.SHA256, nil
+		digest := sha256.Sum256(h.buffer)
+		return digest[:], crypto.SHA256, nil
 	}
 	if signatureAndHash.signature == signatureECDSA {
 		digest := h.server.Sum(nil)
@@ -337,3 +357,9 @@ func (h finishedHash) hashForChannelID(resumeHash []byte) []byte {
 	hash.Write(h.server.Sum(nil))
 	return hash.Sum(nil)
 }
+
+// discardHandshakeBuffer is called when there is no more need to
+// buffer the entirety of the handshake messages.
+func (h *finishedHash) discardHandshakeBuffer() {
+	h.buffer = nil
+}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 9645f70..79bf99c 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -965,29 +965,14 @@ func addClientAuthTests() {
 	certPool.AddCert(cert)
 
 	for _, ver := range tlsVersions {
-		if ver.version == VersionSSL30 {
-			// TODO(davidben): The Go implementation does not
-			// correctly compute CertificateVerify hashes for SSLv3.
-			continue
-		}
-
-		var cipherSuites []uint16
-		if ver.version >= VersionTLS12 {
-			// Pick a SHA-256 cipher suite. The Go implementation
-			// does not correctly handle client auth with a SHA-384
-			// cipher suite.
-			cipherSuites = []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}
-		}
-
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			name:     ver.name + "-Client-ClientAuth-RSA",
 			config: Config{
-				MinVersion:   ver.version,
-				MaxVersion:   ver.version,
-				CipherSuites: cipherSuites,
-				ClientAuth:   RequireAnyClientCert,
-				ClientCAs:    certPool,
+				MinVersion: ver.version,
+				MaxVersion: ver.version,
+				ClientAuth: RequireAnyClientCert,
+				ClientCAs:  certPool,
 			},
 			flags: []string{
 				"-cert-file", rsaCertificateFile,
@@ -995,36 +980,41 @@ func addClientAuthTests() {
 			},
 		})
 		testCases = append(testCases, testCase{
-			testType: clientTest,
-			name:     ver.name + "-Client-ClientAuth-ECDSA",
-			config: Config{
-				MinVersion:   ver.version,
-				MaxVersion:   ver.version,
-				CipherSuites: cipherSuites,
-				ClientAuth:   RequireAnyClientCert,
-				ClientCAs:    certPool,
-			},
-			flags: []string{
-				"-cert-file", ecdsaCertificateFile,
-				"-key-file", ecdsaKeyFile,
-			},
-		})
-		testCases = append(testCases, testCase{
 			testType: serverTest,
 			name:     ver.name + "-Server-ClientAuth-RSA",
 			config: Config{
+				MinVersion:   ver.version,
+				MaxVersion:   ver.version,
 				Certificates: []Certificate{rsaCertificate},
 			},
 			flags: []string{"-require-any-client-certificate"},
 		})
-		testCases = append(testCases, testCase{
-			testType: serverTest,
-			name:     ver.name + "-Server-ClientAuth-ECDSA",
-			config: Config{
-				Certificates: []Certificate{ecdsaCertificate},
-			},
-			flags: []string{"-require-any-client-certificate"},
-		})
+		if ver.version != VersionSSL30 {
+			testCases = append(testCases, testCase{
+				testType: serverTest,
+				name:     ver.name + "-Server-ClientAuth-ECDSA",
+				config: Config{
+					MinVersion:   ver.version,
+					MaxVersion:   ver.version,
+					Certificates: []Certificate{ecdsaCertificate},
+				},
+				flags: []string{"-require-any-client-certificate"},
+			})
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				name:     ver.name + "-Client-ClientAuth-ECDSA",
+				config: Config{
+					MinVersion: ver.version,
+					MaxVersion: ver.version,
+					ClientAuth: RequireAnyClientCert,
+					ClientCAs:  certPool,
+				},
+				flags: []string{
+					"-cert-file", ecdsaCertificateFile,
+					"-key-file", ecdsaKeyFile,
+				},
+			})
+		}
 	}
 }
 
@@ -1092,8 +1082,7 @@ func addStateMachineCoverageTests(async, splitHandshake bool, protocol protocol)
 		testType: clientTest,
 		name:     "ClientAuth-Client" + suffix,
 		config: Config{
-			CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA},
-			ClientAuth:   RequireAnyClientCert,
+			ClientAuth: RequireAnyClientCert,
 			Bugs: ProtocolBugs{
 				MaxHandshakeRecordLength: maxHandshakeRecordLength,
 			},