Test that signature_algorithm preferences are enforced.
Both on the client and the server.
Change-Id: I9892c6dbbb29938154aba4f53b10e8b5231f9c47
Reviewed-on: https://boringssl-review.googlesource.com/4071
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index a9c5083..5cc8e42 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -675,6 +675,10 @@
// IgnorePeerCipherPreferences, if true, causes the peer's cipher
// preferences to be ignored.
IgnorePeerCipherPreferences bool
+
+ // IgnorePeerSignatureAlgorithmPreferences, if true, causes the peer's
+ // signature algorithm preferences to be ignored.
+ IgnorePeerSignatureAlgorithmPreferences bool
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 0669750..8aca9cc 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -6,7 +6,6 @@
import (
"bytes"
- "crypto"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rsa"
@@ -580,33 +579,39 @@
hasSignatureAndHash: c.vers >= VersionTLS12,
}
+ // Determine the hash to sign.
+ var signatureType uint8
+ switch c.config.Certificates[0].PrivateKey.(type) {
+ case *ecdsa.PrivateKey:
+ signatureType = signatureECDSA
+ case *rsa.PrivateKey:
+ signatureType = signatureRSA
+ default:
+ c.sendAlert(alertInternalError)
+ return errors.New("unknown private key type")
+ }
+ if c.config.Bugs.IgnorePeerSignatureAlgorithmPreferences {
+ certReq.signatureAndHashes = c.config.signatureAndHashesForClient()
+ }
+ certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, c.config.signatureAndHashesForClient(), signatureType)
+ if err != nil {
+ c.sendAlert(alertInternalError)
+ return err
+ }
+ digest, hashFunc, err := hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
+ if err != nil {
+ c.sendAlert(alertInternalError)
+ return err
+ }
+
switch key := c.config.Certificates[0].PrivateKey.(type) {
case *ecdsa.PrivateKey:
- certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, signatureECDSA)
- if err != nil {
- break
- }
- var digest []byte
- digest, _, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
- 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})
}
case *rsa.PrivateKey:
- 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, hs.masterSecret)
- if err != nil {
- break
- }
signed, err = rsa.SignPKCS1v15(c.config.rand(), key, hashFunc, digest)
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 00ee44b..1ab4c5f 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -193,6 +193,9 @@
if c.clientVersion < VersionTLS12 && len(hs.clientHello.signatureAndHashes) > 0 {
return false, fmt.Errorf("tls: client included signature_algorithms before TLS 1.2")
}
+ if config.Bugs.IgnorePeerSignatureAlgorithmPreferences {
+ hs.clientHello.signatureAndHashes = config.signatureAndHashesForServer()
+ }
c.vers, ok = config.mutualVersion(hs.clientHello.vers)
if !ok {
diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go
index efb032e..5b88f0f 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -56,7 +56,7 @@
var tls12HashId uint8
if ka.version >= VersionTLS12 {
- if tls12HashId, err = pickTLS12HashForSignature(signatureRSA, clientHello.signatureAndHashes); err != nil {
+ if tls12HashId, err = pickTLS12HashForSignature(signatureRSA, clientHello.signatureAndHashes, config.signatureAndHashesForServer()); err != nil {
return nil, err
}
}
@@ -212,20 +212,19 @@
// pickTLS12HashForSignature returns a TLS 1.2 hash identifier for signing a
// ServerKeyExchange given the signature type being used and the client's
// advertized list of supported signature and hash combinations.
-func pickTLS12HashForSignature(sigType uint8, clientSignatureAndHashes []signatureAndHash) (uint8, error) {
- if len(clientSignatureAndHashes) == 0 {
+func pickTLS12HashForSignature(sigType uint8, clientList, serverList []signatureAndHash) (uint8, error) {
+ if len(clientList) == 0 {
// If the client didn't specify any signature_algorithms
// extension then we can assume that it supports SHA1. See
// http://tools.ietf.org/html/rfc5246#section-7.4.1.4.1
return hashSHA1, nil
}
- for _, sigAndHash := range clientSignatureAndHashes {
+ for _, sigAndHash := range clientList {
if sigAndHash.signature != sigType {
continue
}
- switch sigAndHash.hash {
- case hashSHA1, hashSHA256:
+ if isSupportedSignatureAndHash(sigAndHash, serverList) {
return sigAndHash.hash, nil
}
}
@@ -279,7 +278,7 @@
var tls12HashId uint8
var err error
if ka.version >= VersionTLS12 {
- if tls12HashId, err = pickTLS12HashForSignature(ka.sigType, clientHello.signatureAndHashes); err != nil {
+ if tls12HashId, err = pickTLS12HashForSignature(ka.sigType, clientHello.signatureAndHashes, config.signatureAndHashesForServer()); err != nil {
return nil, err
}
}
diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go
index 75a8933..d445e76 100644
--- a/ssl/test/runner/prf.go
+++ b/ssl/test/runner/prf.go
@@ -323,14 +323,14 @@
// 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) {
+func (h finishedHash) selectClientCertSignatureAlgorithm(serverList, clientList []signatureAndHash, sigType uint8) (signatureAndHash, error) {
if h.version < VersionTLS12 {
// Nothing to negotiate before TLS 1.2.
return signatureAndHash{signature: sigType}, nil
}
for _, v := range serverList {
- if v.signature == sigType && v.hash == hashSHA256 {
+ if v.signature == sigType && isSupportedSignatureAndHash(v, clientList) {
return v, nil
}
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d72ac43..a295cca 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2922,6 +2922,45 @@
},
},
})
+
+ // Test that hash preferences are enforced. BoringSSL defaults to
+ // rejecting MD5 signatures.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SigningHash-ClientAuth-Enforced",
+ config: Config{
+ Certificates: []Certificate{rsaCertificate},
+ SignatureAndHashes: []signatureAndHash{
+ {signatureRSA, hashMD5},
+ // Advertise SHA-1 so the handshake will
+ // proceed, but the shim's preferences will be
+ // ignored in CertificateVerify generation, so
+ // MD5 will be chosen.
+ {signatureRSA, hashSHA1},
+ },
+ Bugs: ProtocolBugs{
+ IgnorePeerSignatureAlgorithmPreferences: true,
+ },
+ },
+ flags: []string{"-require-any-client-certificate"},
+ shouldFail: true,
+ expectedError: ":WRONG_SIGNATURE_TYPE:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "SigningHash-ServerKeyExchange-Enforced",
+ config: Config{
+ CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+ SignatureAndHashes: []signatureAndHash{
+ {signatureRSA, hashMD5},
+ },
+ Bugs: ProtocolBugs{
+ IgnorePeerSignatureAlgorithmPreferences: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_SIGNATURE_TYPE:",
+ })
}
// timeouts is the retransmit schedule for BoringSSL. It doubles and
