From 7944a9f008d8b6020c1b652a375f2e7b63644e09 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 12 Jul 2016 22:27:01 -0400 Subject: [PATCH] Account for key size when selecting RSA-PSS. RSASSA-PSS with SHA-512 is slightly too large for 1024-bit RSA. One should not be using 1024-bit RSA, but it's common enough for tests (including our own in runner before they were regenerated), that we should probably do the size check and avoid unnecessary turbulence to everyone else's test setups. Change-Id: If0c7e401d7d05404755cba4cbff76de3bc65c138 Reviewed-on: https://boringssl-review.googlesource.com/8746 Reviewed-by: Steven Valdez Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/ssl_rsa.c | 18 ++++++++++++++-- ssl/test/runner/rsa_1024_cert.pem | 15 +++++++++++++ ssl/test/runner/rsa_1024_key.pem | 15 +++++++++++++ ssl/test/runner/runner.go | 36 ++++++++++++++++++++++++++++++- util/run_android_tests.go | 2 ++ 5 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 ssl/test/runner/rsa_1024_cert.pem create mode 100644 ssl/test/runner/rsa_1024_key.pem diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index 929de04e..90834e5f 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -726,8 +726,22 @@ int ssl_private_key_supports_signature_algorithm(SSL *ssl, } if (is_rsa_pss(&md, signature_algorithm)) { - return ssl_private_key_type(ssl) == EVP_PKEY_RSA && - ssl3_protocol_version(ssl) >= TLS1_3_VERSION; + if (ssl3_protocol_version(ssl) < TLS1_3_VERSION || + ssl_private_key_type(ssl) != EVP_PKEY_RSA) { + return 0; + } + + /* Ensure the RSA key is large enough for the hash. RSASSA-PSS requires that + * emLen be at least hLen + sLen + 2. Both hLen and sLen are the size of the + * hash in TLS. Reasonable RSA key sizes are large enough for the largest + * defined RSASSA-PSS algorithm, but 1024-bit RSA is slightly too large for + * SHA-512. 1024-bit RSA is sometimes used for test credentials, so check + * the size to fall back to another algorithm. */ + if (ssl_private_key_max_signature_len(ssl) < 2 * EVP_MD_size(md) + 2) { + return 0; + } + + return 1; } return 0; diff --git a/ssl/test/runner/rsa_1024_cert.pem b/ssl/test/runner/rsa_1024_cert.pem new file mode 100644 index 00000000..4de4f49a --- /dev/null +++ b/ssl/test/runner/rsa_1024_cert.pem @@ -0,0 +1,15 @@ +-----BEGIN CERTIFICATE----- +MIICWDCCAcGgAwIBAgIJAPuwTC6rEJsMMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV +BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX +aWRnaXRzIFB0eSBMdGQwHhcNMTQwNDIzMjA1MDQwWhcNMTcwNDIyMjA1MDQwWjBF +MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50 +ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB +gQDYK8imMuRi/03z0K1Zi0WnvfFHvwlYeyK9Na6XJYaUoIDAtB92kWdGMdAQhLci +HnAjkXLI6W15OoV3gA/ElRZ1xUpxTMhjP6PyY5wqT5r6y8FxbiiFKKAnHmUcrgfV +W28tQ+0rkLGMryRtrukXOgXBv7gcrmU7G1jC2a7WqmeI8QIDAQABo1AwTjAdBgNV +HQ4EFgQUi3XVrMsIvg4fZbf6Vr5sp3Xaha8wHwYDVR0jBBgwFoAUi3XVrMsIvg4f +Zbf6Vr5sp3Xaha8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQUFAAOBgQA76Hht +ldY9avcTGSwbwoiuIqv0jTL1fHFnzy3RHMLDh+Lpvolc5DSrSJHCP5WuK0eeJXhr +T5oQpHL9z/cCDLAKCKRa4uV0fhEdOWBqyR9p8y5jJtye72t6CuFUV5iqcpF4BH4f +j2VNHwsSrJwkD4QUGlUtH7vwnQmyCFxZMmWAJg== +-----END CERTIFICATE----- diff --git a/ssl/test/runner/rsa_1024_key.pem b/ssl/test/runner/rsa_1024_key.pem new file mode 100644 index 00000000..e9107bfe --- /dev/null +++ b/ssl/test/runner/rsa_1024_key.pem @@ -0,0 +1,15 @@ +-----BEGIN RSA PRIVATE KEY----- +MIICXgIBAAKBgQDYK8imMuRi/03z0K1Zi0WnvfFHvwlYeyK9Na6XJYaUoIDAtB92 +kWdGMdAQhLciHnAjkXLI6W15OoV3gA/ElRZ1xUpxTMhjP6PyY5wqT5r6y8FxbiiF +KKAnHmUcrgfVW28tQ+0rkLGMryRtrukXOgXBv7gcrmU7G1jC2a7WqmeI8QIDAQAB +AoGBAIBy09Fd4DOq/Ijp8HeKuCMKTHqTW1xGHshLQ6jwVV2vWZIn9aIgmDsvkjCe +i6ssZvnbjVcwzSoByhjN8ZCf/i15HECWDFFh6gt0P5z0MnChwzZmvatV/FXCT0j+ +WmGNB/gkehKjGXLLcjTb6dRYVJSCZhVuOLLcbWIV10gggJQBAkEA8S8sGe4ezyyZ +m4e9r95g6s43kPqtj5rewTsUxt+2n4eVodD+ZUlCULWVNAFLkYRTBCASlSrm9Xhj +QpmWAHJUkQJBAOVzQdFUaewLtdOJoPCtpYoY1zd22eae8TQEmpGOR11L6kbxLQsk +aMly/DOnOaa82tqAGTdqDEZgSNmCeKKknmECQAvpnY8GUOVAubGR6c+W90iBuQLj +LtFp/9ihd2w/PoDwrHZaoUYVcT4VSfJQog/k7kjE4MYXYWL8eEKg3WTWQNECQQDk +104Wi91Umd1PzF0ijd2jXOERJU1wEKe6XLkYYNHWQAe5l4J4MWj9OdxFXAxIuuR/ +tfDwbqkta4xcux67//khAkEAvvRXLHTaa6VFzTaiiO8SaFsHV3lQyXOtMrBpB5jd +moZWgjHvB2W9Ckn7sDqsPB+U2tyX0joDdQEyuiMECDY8oQ== +-----END RSA PRIVATE KEY----- diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 3531909f..45dc7cba 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -61,6 +61,7 @@ type testCert int const ( testCertRSA testCert = iota + testCertRSA1024 testCertECDSAP256 testCertECDSAP384 testCertECDSAP521 @@ -68,6 +69,7 @@ const ( const ( rsaCertificateFile = "cert.pem" + rsa1024CertificateFile = "rsa_1024_cert.pem" ecdsaP256CertificateFile = "ecdsa_p256_cert.pem" ecdsaP384CertificateFile = "ecdsa_p384_cert.pem" ecdsaP521CertificateFile = "ecdsa_p521_cert.pem" @@ -75,13 +77,20 @@ const ( const ( rsaKeyFile = "key.pem" + rsa1024KeyFile = "rsa_1024_key.pem" ecdsaP256KeyFile = "ecdsa_p256_key.pem" ecdsaP384KeyFile = "ecdsa_p384_key.pem" ecdsaP521KeyFile = "ecdsa_p521_key.pem" channelIDKeyFile = "channel_id_key.pem" ) -var rsaCertificate, ecdsaP256Certificate, ecdsaP384Certificate, ecdsaP521Certificate Certificate +var ( + rsaCertificate Certificate + rsa1024Certificate Certificate + ecdsaP256Certificate Certificate + ecdsaP384Certificate Certificate + ecdsaP521Certificate Certificate +) var testCerts = []struct { id testCert @@ -94,6 +103,12 @@ var testCerts = []struct { keyFile: rsaKeyFile, cert: &rsaCertificate, }, + { + id: testCertRSA1024, + certFile: rsa1024CertificateFile, + keyFile: rsa1024KeyFile, + cert: &rsa1024Certificate, + }, { id: testCertECDSAP256, certFile: ecdsaP256CertificateFile, @@ -5173,6 +5188,25 @@ func addSignatureAlgorithmTests() { }, expectedPeerSignatureAlgorithm: signatureECDSAWithP256AndSHA256, }) + + // RSASSA-PSS with SHA-512 is too large for 1024-bit RSA. Test that the + // server does not attempt to sign in that case. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "RSA-PSS-Large", + config: Config{ + MaxVersion: VersionTLS13, + VerifySignatureAlgorithms: []signatureAlgorithm{ + signatureRSAPSSWithSHA512, + }, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, rsa1024CertificateFile), + "-key-file", path.Join(*resourceDir, rsa1024KeyFile), + }, + shouldFail: true, + expectedError: ":NO_COMMON_SIGNATURE_ALGORITHMS:", + }) } // timeouts is the retransmit schedule for BoringSSL. It doubles and diff --git a/util/run_android_tests.go b/util/run_android_tests.go index c9f9020e..84155d5f 100644 --- a/util/run_android_tests.go +++ b/util/run_android_tests.go @@ -251,6 +251,8 @@ func main() { "ssl/test/runner/ecdsa_p521_cert.pem", "ssl/test/runner/ecdsa_p521_key.pem", "ssl/test/runner/key.pem", + "ssl/test/runner/rsa_1024_cert.pem", + "ssl/test/runner/rsa_1024_key.pem", ) fmt.Printf("Building runner...\n")