From ca3d545d7f34d67c8fb05265a0f1cf7f88076776 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 14 Jul 2016 12:51:01 -0400 Subject: [PATCH] Add SSL_set_signing_algorithm_prefs. This gives us a sigalg-based API for configuring signing algorithms. Change-Id: Ib746a56ebd1061eadd2620cdb140d5171b59bc02 Reviewed-on: https://boringssl-review.googlesource.com/8784 Reviewed-by: Adam Langley --- include/openssl/ssl.h | 44 ++++++++++++--- include/openssl/tls1.h | 17 ------ ssl/ssl_rsa.c | 13 +++++ ssl/test/bssl_shim.cc | 8 +++ ssl/test/runner/runner.go | 113 ++++++++++++++++++++++++++++++++++---- ssl/test/test_config.cc | 18 ++++++ ssl/test/test_config.h | 2 + 7 files changed, 181 insertions(+), 34 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index d0bc1038..7b4b3497 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -869,13 +869,31 @@ OPENSSL_EXPORT int SSL_CTX_set_ocsp_response(SSL_CTX *ctx, const uint8_t *response, size_t response_len); -/* SSL_set_private_key_digest_prefs copies |num_digests| NIDs from |digest_nids| - * into |ssl|. These digests will be used, in decreasing order of preference, - * when signing with |ssl|'s private key. It returns one on success and zero on - * error. */ -OPENSSL_EXPORT int SSL_set_private_key_digest_prefs(SSL *ssl, - const int *digest_nids, - size_t num_digests); +/* SSL_SIGN_* are signature algorithm values as defined in TLS 1.3. */ +#define SSL_SIGN_RSA_PKCS1_SHA1 0x0201 +#define SSL_SIGN_RSA_PKCS1_SHA256 0x0401 +#define SSL_SIGN_RSA_PKCS1_SHA384 0x0501 +#define SSL_SIGN_RSA_PKCS1_SHA512 0x0601 +#define SSL_SIGN_ECDSA_SHA1 0x0203 +#define SSL_SIGN_ECDSA_SECP256R1_SHA256 0x0403 +#define SSL_SIGN_ECDSA_SECP384R1_SHA384 0x0503 +#define SSL_SIGN_ECDSA_SECP521R1_SHA512 0x0603 +#define SSL_SIGN_RSA_PSS_SHA256 0x0700 +#define SSL_SIGN_RSA_PSS_SHA384 0x0701 +#define SSL_SIGN_RSA_PSS_SHA512 0x0702 + +/* SSL_SIGN_RSA_PKCS1_MD5_SHA1 is an internal signature algorithm used to + * specify raw RSASSA-PKCS1-v1_5 with an MD5/SHA-1 concatenation, as used in TLS + * before TLS 1.2. */ +#define SSL_SIGN_RSA_PKCS1_MD5_SHA1 0xff01 + +/* SSL_set_signing_algorithm_prefs configures |ssl| to use |prefs| as the + * preference list when signing with |ssl|'s private key. It returns one on + * success and zero on error. |prefs| should not include the internal-only value + * |SSL_SIGN_RSA_PKCS1_MD5_SHA1|. */ +OPENSSL_EXPORT int SSL_set_signing_algorithm_prefs(SSL *ssl, + const uint16_t *prefs, + size_t prefs_len); /* Certificate and private key convenience functions. */ @@ -3482,6 +3500,18 @@ OPENSSL_EXPORT int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *out, OPENSSL_EXPORT uint32_t SSL_SESSION_get_key_exchange_info( const SSL_SESSION *session); +/* SSL_set_private_key_digest_prefs copies |num_digests| NIDs from |digest_nids| + * into |ssl|. These digests will be used, in decreasing order of preference, + * when signing with |ssl|'s private key. It returns one on success and zero on + * error. + * + * Use |SSL_set_signing_algorithm_prefs| instead. + * + * TODO(davidben): Remove this API when callers have been updated. */ +OPENSSL_EXPORT int SSL_set_private_key_digest_prefs(SSL *ssl, + const int *digest_nids, + size_t num_digests); + /* Private structures. * diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index 84ff12fb..ae1ab7cc 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h @@ -245,23 +245,6 @@ extern "C" { #define TLSEXT_hash_sha384 5 #define TLSEXT_hash_sha512 6 -#define SSL_SIGN_RSA_PKCS1_SHA1 0x0201 -#define SSL_SIGN_RSA_PKCS1_SHA256 0x0401 -#define SSL_SIGN_RSA_PKCS1_SHA384 0x0501 -#define SSL_SIGN_RSA_PKCS1_SHA512 0x0601 -#define SSL_SIGN_ECDSA_SHA1 0x0203 -#define SSL_SIGN_ECDSA_SECP256R1_SHA256 0x0403 -#define SSL_SIGN_ECDSA_SECP384R1_SHA384 0x0503 -#define SSL_SIGN_ECDSA_SECP521R1_SHA512 0x0603 -#define SSL_SIGN_RSA_PSS_SHA256 0x0700 -#define SSL_SIGN_RSA_PSS_SHA384 0x0701 -#define SSL_SIGN_RSA_PSS_SHA512 0x0702 - -/* Reserved SignatureScheme value to indicate RSA with MD5-SHA1. This will never - * be negotiated in TLS 1.2 and up, but is used to unify signing interfaces in - * older TLS versions. */ -#define SSL_SIGN_RSA_PKCS1_MD5_SHA1 0xff01 - #define TLSEXT_MAXLEN_host_name 255 /* PSK ciphersuites from 4279 */ diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index 3e0894e4..fec5d467 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -335,6 +335,19 @@ void SSL_CTX_set_private_key_method(SSL_CTX *ctx, ctx->cert->key_method = key_method; } +int SSL_set_signing_algorithm_prefs(SSL *ssl, const uint16_t *prefs, + size_t prefs_len) { + ssl->cert->sigalgs_len = 0; + ssl->cert->sigalgs = BUF_memdup(prefs, prefs_len * sizeof(prefs[0])); + if (ssl->cert->sigalgs == NULL) { + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + return 0; + } + ssl->cert->sigalgs_len = prefs_len; + + return 1; +} + OPENSSL_COMPILE_ASSERT(sizeof(int) >= 2 * sizeof(uint16_t), digest_list_conversion_cannot_overflow); diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 02af8f26..3cfbeb39 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -315,6 +315,14 @@ static bool GetCertificate(SSL *ssl, ScopedX509 *out_x509, } } + if (!config->signing_prefs.empty()) { + std::vector u16s(config->signing_prefs.begin(), + config->signing_prefs.end()); + if (!SSL_set_signing_algorithm_prefs(ssl, u16s.data(), u16s.size())) { + return false; + } + } + if (!config->key_file.empty()) { *out_pkey = LoadPrivateKey(config->key_file.c_str()); if (!*out_pkey) { diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 5bbf57d3..5a3256ca 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -259,8 +259,6 @@ type testCase struct { messageLen int // messageCount is the number of test messages that will be sent. messageCount int - // digestPrefs is the list of digest preferences from the client. - digestPrefs string // certFile is the path to the certificate to use for the server. certFile string // keyFile is the path to the private key to use for the server. @@ -744,11 +742,6 @@ func runTest(test *testCase, shimPath string, mallocNumToFail int64) error { } } - if test.digestPrefs != "" { - flags = append(flags, "-digest-prefs") - flags = append(flags, test.digestPrefs) - } - if test.protocol == dtls { flags = append(flags, "-dtls") } @@ -4736,6 +4729,13 @@ func addSignatureAlgorithmTests() { TLS_DHE_RSA_WITH_AES_128_CBC_SHA, } + var allAlgorithms []signatureAlgorithm + for _, alg := range testSignatureAlgorithms { + if alg.id != 0 { + allAlgorithms = append(allAlgorithms, alg.id) + } + } + // Make sure each signature algorithm works. Include some fake values in // the list and ensure they're ignored. for _, alg := range testSignatureAlgorithms { @@ -4899,6 +4899,41 @@ func addSignatureAlgorithmTests() { expectedError: ":BAD_SIGNATURE:", }) } + + if ver.version >= VersionTLS12 && !shouldFail { + testCases = append(testCases, testCase{ + name: "ClientAuth-Sign-Negotiate" + suffix, + config: Config{ + MaxVersion: ver.version, + ClientAuth: RequireAnyClientCert, + VerifySignatureAlgorithms: allAlgorithms, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), + "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), + "-enable-all-curves", + "-signing-prefs", strconv.Itoa(int(alg.id)), + }, + expectedPeerSignatureAlgorithm: alg.id, + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ServerAuth-Sign-Negotiate" + suffix, + config: Config{ + MaxVersion: ver.version, + CipherSuites: signingCiphers, + VerifySignatureAlgorithms: allAlgorithms, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), + "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), + "-enable-all-curves", + "-signing-prefs", strconv.Itoa(int(alg.id)), + }, + expectedPeerSignatureAlgorithm: alg.id, + }) + } } } @@ -5094,6 +5129,24 @@ func addSignatureAlgorithmTests() { // the server digests. // // TODO(davidben): Add TLS 1.3 versions of these. + testCases = append(testCases, testCase{ + name: "NoCommonAlgorithms-Digests", + config: Config{ + MaxVersion: VersionTLS12, + ClientAuth: RequireAnyClientCert, + VerifySignatureAlgorithms: []signatureAlgorithm{ + signatureRSAPKCS1WithSHA512, + signatureRSAPKCS1WithSHA1, + }, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, rsaCertificateFile), + "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-digest-prefs", "SHA256", + }, + shouldFail: true, + expectedError: ":NO_COMMON_SIGNATURE_ALGORITHMS:", + }) testCases = append(testCases, testCase{ name: "NoCommonAlgorithms", config: Config{ @@ -5107,8 +5160,26 @@ func addSignatureAlgorithmTests() { flags: []string{ "-cert-file", path.Join(*resourceDir, rsaCertificateFile), "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA256)), + }, + shouldFail: true, + expectedError: ":NO_COMMON_SIGNATURE_ALGORITHMS:", + }) + testCases = append(testCases, testCase{ + name: "NoCommonAlgorithms-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + ClientAuth: RequireAnyClientCert, + VerifySignatureAlgorithms: []signatureAlgorithm{ + signatureRSAPSSWithSHA512, + signatureRSAPSSWithSHA384, + }, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, rsaCertificateFile), + "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-signing-prefs", strconv.Itoa(int(signatureRSAPSSWithSHA256)), }, - digestPrefs: "SHA256", shouldFail: true, expectedError: ":NO_COMMON_SIGNATURE_ALGORITHMS:", }) @@ -5125,8 +5196,8 @@ func addSignatureAlgorithmTests() { flags: []string{ "-cert-file", path.Join(*resourceDir, rsaCertificateFile), "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-digest-prefs", "SHA256,SHA1", }, - digestPrefs: "SHA256,SHA1", expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA256, }) testCases = append(testCases, testCase{ @@ -5141,8 +5212,8 @@ func addSignatureAlgorithmTests() { flags: []string{ "-cert-file", path.Join(*resourceDir, rsaCertificateFile), "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-digest-prefs", "SHA512,SHA256,SHA1", }, - digestPrefs: "SHA512,SHA256,SHA1", expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA1, }) testCases = append(testCases, testCase{ @@ -5164,6 +5235,28 @@ func addSignatureAlgorithmTests() { expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA256, }) + // Test that the signing preference list may include extra algorithms + // without negotiation problems. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "FilterExtraAlgorithms", + config: Config{ + MaxVersion: VersionTLS12, + VerifySignatureAlgorithms: []signatureAlgorithm{ + signatureRSAPKCS1WithSHA256, + }, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, rsaCertificateFile), + "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-signing-prefs", strconv.Itoa(int(fakeSigAlg1)), + "-signing-prefs", strconv.Itoa(int(signatureECDSAWithP256AndSHA256)), + "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA256)), + "-signing-prefs", strconv.Itoa(int(fakeSigAlg2)), + }, + expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA256, + }) + // In TLS 1.2 and below, ECDSA uses the curve list rather than the // signature algorithms. testCases = append(testCases, testCase{ diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 1c3302ed..24a4646a 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -153,6 +153,10 @@ const Flag kIntFlags[] = { { "-initial-timeout-duration-ms", &TestConfig::initial_timeout_duration_ms }, }; +const Flag> kIntVectorFlags[] = { + { "-signing-prefs", &TestConfig::signing_prefs }, +}; + } // namespace bool ParseConfig(int argc, char **argv, TestConfig *out_config) { @@ -208,6 +212,20 @@ bool ParseConfig(int argc, char **argv, TestConfig *out_config) { continue; } + std::vector *int_vector_field = + FindField(out_config, kIntVectorFlags, argv[i]); + if (int_vector_field) { + i++; + if (i >= argc) { + fprintf(stderr, "Missing parameter\n"); + return false; + } + + // Each instance of the flag adds to the list. + int_vector_field->push_back(atoi(argv[i])); + continue; + } + fprintf(stderr, "Unknown argument: %s\n", argv[i]); return false; } diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index b3c858d7..4ee717ee 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -16,6 +16,7 @@ #define HEADER_TEST_CONFIG #include +#include struct TestConfig { @@ -25,6 +26,7 @@ struct TestConfig { bool resume = false; bool fallback_scsv = false; std::string digest_prefs; + std::vector signing_prefs; std::string key_file; std::string cert_file; std::string expected_server_name;