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>
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 @@
// 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 @@
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 @@
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 @@
}
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 @@
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 @@
// 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 @@
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 @@
}
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 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
// 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) 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 @@
// 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 @@
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 @@
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 @@
},
})
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 @@
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,
},
