diff options
author | David Benjamin <davidben@google.com> | 2016-07-09 02:28:04 +0300 |
---|---|---|
committer | David Benjamin <davidben@google.com> | 2016-07-12 21:24:26 +0300 |
commit | a95e9f3010ab6dcc668d95e90b5b1b69399a53bc (patch) | |
tree | 132eaf7b47ac4e7589ed91dbd87a8b47f27c0e05 | |
parent | 51dd7d637905c8fd6100ab9e5c1ac65ff5ece670 (diff) |
Test that signature verification checks the key type.
{sha256,ecdsa} should not be silently accepted for an RSA key.
Change-Id: I0c0eea5071f7a59f2707ca0ea023a16cc4126d6a
Reviewed-on: https://boringssl-review.googlesource.com/8697
Reviewed-by: David Benjamin <davidben@google.com>
-rw-r--r-- | ssl/test/runner/common.go | 4 | ||||
-rw-r--r-- | ssl/test/runner/handshake_client.go | 3 | ||||
-rw-r--r-- | ssl/test/runner/key_agreement.go | 3 | ||||
-rw-r--r-- | ssl/test/runner/runner.go | 39 |
4 files changed, 49 insertions, 0 deletions
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 99c4e54b..023de3a9 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -892,6 +892,10 @@ type ProtocolBugs struct { // OuterRecordType, if non-zero, is the outer record type to use instead // of application data. OuterRecordType recordType + + // SendSignatureAlgorithm, if non-zero, causes all signatures to be sent + // with the given signature algorithm rather than the one negotiated. + SendSignatureAlgorithm signatureAlgorithm } func (c *Config) serverInit() { diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 1de92453..7857b1d1 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -783,6 +783,9 @@ func (hs *clientHandshakeState) doFullHandshake() error { msg[0] ^= 0x80 } certVerify.signature, err = signMessage(c.vers, privKey, c.config, certVerify.signatureAlgorithm, msg) + if err == nil && c.config.Bugs.SendSignatureAlgorithm != 0 { + certVerify.signatureAlgorithm = c.config.Bugs.SendSignatureAlgorithm + } } else { // SSL 3.0's client certificate construction is // incompatible with signatureAlgorithm. diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go index 5f8e7ea6..0c9dc358 100644 --- a/ssl/test/runner/key_agreement.go +++ b/ssl/test/runner/key_agreement.go @@ -414,6 +414,9 @@ func (ka *signedKeyAgreement) signParameters(config *Config, cert *Certificate, if err != nil { return nil, err } + if config.Bugs.SendSignatureAlgorithm != 0 { + sigAlg = config.Bugs.SendSignatureAlgorithm + } skx := new(serverKeyExchangeMsg) if config.Bugs.UnauthenticatedECDH { diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 7c0e38bd..03a0c584 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -4794,6 +4794,45 @@ func addSignatureAlgorithmTests() { expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA384, }) + // Test that signature verification takes the key type into account. + // + // TODO(davidben): Test this in TLS 1.3. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Verify-ClientAuth-SignatureType", + config: Config{ + MaxVersion: VersionTLS12, + Certificates: []Certificate{rsaCertificate}, + SignatureAlgorithms: []signatureAlgorithm{ + signatureRSAPKCS1WithSHA256, + }, + Bugs: ProtocolBugs{ + SendSignatureAlgorithm: signatureECDSAWithP256AndSHA256, + }, + }, + flags: []string{ + "-require-any-client-certificate", + }, + shouldFail: true, + expectedError: ":WRONG_SIGNATURE_TYPE:", + }) + + testCases = append(testCases, testCase{ + name: "Verify-ServerKeyExchange-SignatureType", + config: Config{ + MaxVersion: VersionTLS12, + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + SignatureAlgorithms: []signatureAlgorithm{ + signatureRSAPKCS1WithSHA256, + }, + Bugs: ProtocolBugs{ + SendSignatureAlgorithm: signatureECDSAWithP256AndSHA256, + }, + }, + shouldFail: true, + expectedError: ":WRONG_SIGNATURE_TYPE:", + }) + // Test that, if the list is missing, the peer falls back to SHA-1 in // TLS 1.2, but not TLS 1.3. testCases = append(testCases, testCase{ |