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 <agl@google.com>
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 271355f..9d07584 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -6,6 +6,7 @@
import (
"bytes"
+ "crypto"
"crypto/ecdsa"
"crypto/rsa"
"crypto/subtle"
@@ -14,6 +15,7 @@
"errors"
"fmt"
"io"
+ "math/big"
"net"
"strconv"
)
@@ -418,18 +420,32 @@
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 4c3d35a..f3fd08f 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -431,8 +431,28 @@
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 @@
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 acb9654..991196f 100644
--- a/ssl/test/runner/prf.go
+++ b/ssl/test/runner/prf.go
@@ -11,6 +11,7 @@
"crypto/sha1"
"crypto/sha256"
"crypto/sha512"
+ "errors"
"hash"
)
@@ -284,20 +285,39 @@
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
}
