From de620d9c87293fd0ed3b0a348550833565a0b61a Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 18 Jul 2014 15:03:41 -0400 Subject: [PATCH] runner: Require the CertificateVerify signature and hash to match. This change can probably be ported over to upstream crypto/tls. The current Go TLS implementation ignores the signature and hash algorithm lists in CertificateVerify and CertificateRequest. Take these into account so that our tests assert OpenSSL fills them out correctly. Also fix a bug in the original code where 'err' within the switch block get shadowed. Change-Id: I5d9c0b31ebb4662ecc767ed885a20707f0e86216 Reviewed-on: https://boringssl-review.googlesource.com/1253 Reviewed-by: Adam Langley --- ssl/test/runner/handshake_client.go | 30 +++++++++++++++++------ ssl/test/runner/handshake_server.go | 37 +++++++++++++++++++++++++++-- ssl/test/runner/prf.go | 30 +++++++++++++++++++---- 3 files changed, 83 insertions(+), 14 deletions(-) diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 271355fcd..9d07584c3 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -6,6 +6,7 @@ package main import ( "bytes" + "crypto" "crypto/ecdsa" "crypto/rsa" "crypto/subtle" @@ -14,6 +15,7 @@ import ( "errors" "fmt" "io" + "math/big" "net" "strconv" ) @@ -418,18 +420,32 @@ func (hs *clientHandshakeState) doFullHandshake() error { switch key := c.config.Certificates[0].PrivateKey.(type) { case *ecdsa.PrivateKey: - digest, _, hashId := hs.finishedHash.hashForClientCertificate(signatureECDSA) - r, s, err := ecdsa.Sign(c.config.rand(), key, digest) + certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, signatureECDSA) + if err != nil { + break + } + var digest []byte + digest, _, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash) + if err != nil { + break + } + var r, s *big.Int + r, s, err = ecdsa.Sign(c.config.rand(), key, digest) if err == nil { signed, err = asn1.Marshal(ecdsaSignature{r, s}) } - certVerify.signatureAndHash.signature = signatureECDSA - certVerify.signatureAndHash.hash = hashId case *rsa.PrivateKey: - digest, hashFunc, hashId := hs.finishedHash.hashForClientCertificate(signatureRSA) + certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, signatureRSA) + if err != nil { + break + } + var digest []byte + var hashFunc crypto.Hash + digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash) + if err != nil { + break + } signed, err = rsa.SignPKCS1v15(c.config.rand(), key, hashFunc, digest) - certVerify.signatureAndHash.signature = signatureRSA - certVerify.signatureAndHash.hash = hashId default: err = errors.New("unknown private key type") } diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 4c3d35a25..f3fd08ff9 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -431,8 +431,28 @@ func (hs *serverHandshakeState) doFullHandshake() error { return unexpectedMessageError(certVerify, msg) } + // Determine the signature type. + var signatureAndHash signatureAndHash + if certVerify.hasSignatureAndHash { + signatureAndHash = certVerify.signatureAndHash + } else { + // Before TLS 1.2 the signature algorithm was implicit + // from the key type, and only one hash per signature + // algorithm was possible. Leave the hash as zero. + switch pub.(type) { + case *ecdsa.PublicKey: + signatureAndHash.signature = signatureECDSA + case *rsa.PublicKey: + signatureAndHash.signature = signatureRSA + } + } + switch key := pub.(type) { case *ecdsa.PublicKey: + if signatureAndHash.signature != signatureECDSA { + err = errors.New("tls: bad signature type for client's ECDSA certificate") + break + } ecdsaSig := new(ecdsaSignature) if _, err = asn1.Unmarshal(certVerify.signature, ecdsaSig); err != nil { break @@ -441,13 +461,26 @@ func (hs *serverHandshakeState) doFullHandshake() error { err = errors.New("ECDSA signature contained zero or negative values") break } - digest, _, _ := hs.finishedHash.hashForClientCertificate(signatureECDSA) + var digest []byte + digest, _, err = hs.finishedHash.hashForClientCertificate(signatureAndHash) + if err != nil { + break + } if !ecdsa.Verify(key, digest, ecdsaSig.R, ecdsaSig.S) { err = errors.New("ECDSA verification failure") break } case *rsa.PublicKey: - digest, hashFunc, _ := hs.finishedHash.hashForClientCertificate(signatureRSA) + if signatureAndHash.signature != signatureRSA { + err = errors.New("tls: bad signature type for client's RSA certificate") + break + } + var digest []byte + var hashFunc crypto.Hash + digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(signatureAndHash) + if err != nil { + break + } err = rsa.VerifyPKCS1v15(key, hashFunc, digest, certVerify.signature) } if err != nil { diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go index acb96549b..991196fd0 100644 --- a/ssl/test/runner/prf.go +++ b/ssl/test/runner/prf.go @@ -11,6 +11,7 @@ import ( "crypto/sha1" "crypto/sha256" "crypto/sha512" + "errors" "hash" ) @@ -284,20 +285,39 @@ func (h finishedHash) serverSum(masterSecret []byte) []byte { return out } +// selectClientCertSignatureAlgorithm returns a signatureAndHash to sign a +// client's CertificateVerify with, or an error if none can be found. +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 + } + + for _, v := range serverList { + if v.signature == sigType && v.hash == hashSHA256 { + return v, nil + } + } + return signatureAndHash{}, errors.New("tls: no supported signature algorithm found for signing client certificate") +} + // hashForClientCertificate returns a digest, hash function, and TLS 1.2 hash // id suitable for signing by a TLS client certificate. -func (h finishedHash) hashForClientCertificate(sigType uint8) ([]byte, crypto.Hash, uint8) { +func (h finishedHash) hashForClientCertificate(signatureAndHash signatureAndHash) ([]byte, crypto.Hash, error) { 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, hashSHA256 + return digest, crypto.SHA256, nil } - if sigType == signatureECDSA { + if signatureAndHash.signature == signatureECDSA { digest := h.server.Sum(nil) - return digest, crypto.SHA1, hashSHA1 + return digest, crypto.SHA1, nil } digest := make([]byte, 0, 36) digest = h.serverMD5.Sum(digest) digest = h.server.Sum(digest) - return digest, crypto.MD5SHA1, 0 /* not specified in TLS 1.2. */ + return digest, crypto.MD5SHA1, nil }