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 <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

5 files changed, 83 insertions, 3 deletions

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
@@ -95,6 +104,12 @@ var testCerts = []struct {
 		cert:     &rsaCertificate,
 	},
 	{
+		id:       testCertRSA1024,
+		certFile: rsa1024CertificateFile,
+		keyFile:  rsa1024KeyFile,
+		cert:     &rsa1024Certificate,
+	},
+	{
 		id:       testCertECDSAP256,
 		certFile: ecdsaP256CertificateFile,
 		keyFile:  ecdsaP256KeyFile,
@@ -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")