Unwind legacy SSL_PRIVATE_KEY_METHOD hooks.
After much procrastinating, we finally moved Chromium to the new stuff.
We can now delete this. This is a breaking change for
SSL_PRIVATE_KEY_METHOD consumers, but it should be trivial (remove some
unused fields in the struct). I've bumped BORINGSSL_API_VERSION to ease
any multi-sided changes that may be needed.
Change-Id: I9fe562590ad938bcb4fcf9af0fadeff1d48745fb
Reviewed-on: https://boringssl-review.googlesource.com/23224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/include/openssl/base.h b/include/openssl/base.h
index adb5047..9445556 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -151,7 +151,7 @@
// A consumer may use this symbol in the preprocessor to temporarily build
// against multiple revisions of BoringSSL at the same time. It is not
// recommended to do so for longer than is necessary.
-#define BORINGSSL_API_VERSION 4
+#define BORINGSSL_API_VERSION 5
#if defined(BORINGSSL_SHARED_LIBRARY)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index eba2fa3..53a8eb5 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1125,16 +1125,7 @@
// key hooks. This is used to off-load signing operations to a custom,
// potentially asynchronous, backend. Metadata about the key such as the type
// and size are parsed out of the certificate.
-//
-// TODO(davidben): This API has a number of legacy hooks. Remove the last
-// consumer of |sign_digest| and trim it.
struct ssl_private_key_method_st {
- // type is ignored and should be NULL.
- int (*type)(SSL *ssl);
-
- // max_signature_len is ignored and should be NULL.
- size_t (*max_signature_len)(SSL *ssl);
-
// sign signs the message |in| in using the specified signature algorithm. On
// success, it returns |ssl_private_key_success| and writes at most |max_out|
// bytes of signature data to |out| and sets |*out_len| to the number of bytes
@@ -1156,30 +1147,6 @@
uint16_t signature_algorithm,
const uint8_t *in, size_t in_len);
- // sign_digest signs |in_len| bytes of digest from |in|. |md| is the hash
- // function used to calculate |in|. On success, it returns
- // |ssl_private_key_success| and writes at most |max_out| bytes of signature
- // data to |out|. On failure, it returns |ssl_private_key_failure|. If the
- // operation has not completed, it returns |ssl_private_key_retry|. |sign|
- // should arrange for the high-level operation on |ssl| to be retried when the
- // operation is completed. This will result in a call to |complete|.
- //
- // If the key is an RSA key, implementations must use PKCS#1 padding. |in| is
- // the digest itself, so the DigestInfo prefix, if any, must be prepended by
- // |sign|. If |md| is |EVP_md5_sha1|, there is no prefix.
- //
- // It is an error to call |sign_digest| while another private key operation is
- // in progress on |ssl|.
- //
- // This function is deprecated. Implement |sign| instead.
- //
- // TODO(davidben): Remove this function.
- enum ssl_private_key_result_t (*sign_digest)(SSL *ssl, uint8_t *out,
- size_t *out_len, size_t max_out,
- const EVP_MD *md,
- const uint8_t *in,
- size_t in_len);
-
// decrypt decrypts |in_len| bytes of encrypted data from |in|. On success it
// returns |ssl_private_key_success|, writes at most |max_out| bytes of
// decrypted data to |out| and sets |*out_len| to the actual number of bytes
@@ -3978,18 +3945,6 @@
OPENSSL_EXPORT int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *out,
const char *dir);
-// 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);
-
// SSL_set_verify_result calls |abort| unless |result| is |X509_V_OK|.
//
// TODO(davidben): Remove this function once it has been removed from
diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc
index e9990af..134ad56 100644
--- a/ssl/ssl_privkey.cc
+++ b/ssl/ssl_privkey.cc
@@ -193,33 +193,6 @@
return 1;
}
-static int legacy_sign_digest_supported(const SSL_SIGNATURE_ALGORITHM *alg) {
- return (alg->pkey_type == EVP_PKEY_EC || alg->pkey_type == EVP_PKEY_RSA) &&
- !alg->is_rsa_pss;
-}
-
-static enum ssl_private_key_result_t legacy_sign(
- SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out, uint16_t sigalg,
- const uint8_t *in, size_t in_len) {
- // TODO(davidben): Remove support for |sign_digest|-only
- // |SSL_PRIVATE_KEY_METHOD|s.
- const SSL_SIGNATURE_ALGORITHM *alg = get_signature_algorithm(sigalg);
- if (alg == NULL || !legacy_sign_digest_supported(alg)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL_FOR_CUSTOM_KEY);
- return ssl_private_key_failure;
- }
-
- const EVP_MD *md = alg->digest_func();
- uint8_t hash[EVP_MAX_MD_SIZE];
- unsigned hash_len;
- if (!EVP_Digest(in, in_len, hash, &hash_len, md, NULL)) {
- return ssl_private_key_failure;
- }
-
- return ssl->cert->key_method->sign_digest(ssl, out, out_len, max_out, md,
- hash, hash_len);
-}
-
enum ssl_private_key_result_t ssl_private_key_sign(
SSL_HANDSHAKE *hs, uint8_t *out, size_t *out_len, size_t max_out,
uint16_t sigalg, Span<const uint8_t> in) {
@@ -229,9 +202,8 @@
if (hs->pending_private_key_op) {
ret = ssl->cert->key_method->complete(ssl, out, out_len, max_out);
} else {
- ret = (ssl->cert->key_method->sign != NULL ? ssl->cert->key_method->sign
- : legacy_sign)(
- ssl, out, out_len, max_out, sigalg, in.data(), in.size());
+ ret = ssl->cert->key_method->sign(ssl, out, out_len, max_out, sigalg,
+ in.data(), in.size());
}
hs->pending_private_key_op = ret == ssl_private_key_retry;
return ret;
@@ -308,14 +280,6 @@
return false;
}
- // Newer algorithms require message-based private keys.
- // TODO(davidben): Remove this check when sign_digest is gone.
- if (ssl->cert->key_method != NULL &&
- ssl->cert->key_method->sign == NULL &&
- !legacy_sign_digest_supported(alg)) {
- return false;
- }
-
return true;
}
@@ -511,7 +475,6 @@
prefs, num_prefs);
}
-
int SSL_set_signing_algorithm_prefs(SSL *ssl, const uint16_t *prefs,
size_t num_prefs) {
return set_algorithm_prefs(&ssl->cert->sigalgs, &ssl->cert->num_sigalgs,
@@ -523,52 +486,3 @@
return set_algorithm_prefs(&ctx->verify_sigalgs, &ctx->num_verify_sigalgs,
prefs, num_prefs);
}
-
-int SSL_set_private_key_digest_prefs(SSL *ssl, const int *digest_nids,
- size_t num_digests) {
- OPENSSL_free(ssl->cert->sigalgs);
-
- static_assert(sizeof(int) >= 2 * sizeof(uint16_t),
- "sigalgs allocation may overflow");
-
- ssl->cert->num_sigalgs = 0;
- ssl->cert->sigalgs =
- (uint16_t *)OPENSSL_malloc(sizeof(uint16_t) * 2 * num_digests);
- if (ssl->cert->sigalgs == NULL) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- return 0;
- }
-
- // Convert the digest list to a signature algorithms list.
- //
- // TODO(davidben): Replace this API with one that can express RSA-PSS, etc.
- for (size_t i = 0; i < num_digests; i++) {
- switch (digest_nids[i]) {
- case NID_sha1:
- ssl->cert->sigalgs[ssl->cert->num_sigalgs] = SSL_SIGN_RSA_PKCS1_SHA1;
- ssl->cert->sigalgs[ssl->cert->num_sigalgs + 1] = SSL_SIGN_ECDSA_SHA1;
- ssl->cert->num_sigalgs += 2;
- break;
- case NID_sha256:
- ssl->cert->sigalgs[ssl->cert->num_sigalgs] = SSL_SIGN_RSA_PKCS1_SHA256;
- ssl->cert->sigalgs[ssl->cert->num_sigalgs + 1] =
- SSL_SIGN_ECDSA_SECP256R1_SHA256;
- ssl->cert->num_sigalgs += 2;
- break;
- case NID_sha384:
- ssl->cert->sigalgs[ssl->cert->num_sigalgs] = SSL_SIGN_RSA_PKCS1_SHA384;
- ssl->cert->sigalgs[ssl->cert->num_sigalgs + 1] =
- SSL_SIGN_ECDSA_SECP384R1_SHA384;
- ssl->cert->num_sigalgs += 2;
- break;
- case NID_sha512:
- ssl->cert->sigalgs[ssl->cert->num_sigalgs] = SSL_SIGN_RSA_PKCS1_SHA512;
- ssl->cert->sigalgs[ssl->cert->num_sigalgs + 1] =
- SSL_SIGN_ECDSA_SECP521R1_SHA512;
- ssl->cert->num_sigalgs += 2;
- break;
- }
- }
-
- return 1;
-}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 7eca21d..8acabd7 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -433,10 +433,7 @@
}
static const SSL_PRIVATE_KEY_METHOD g_async_private_key_method = {
- nullptr /* type */,
- nullptr /* max_signature_len */,
AsyncPrivateKeySign,
- nullptr /* sign_digest */,
AsyncPrivateKeyDecrypt,
AsyncPrivateKeyComplete,
};
@@ -453,27 +450,6 @@
bssl::UniquePtr<EVP_PKEY> *out_pkey) {
const TestConfig *config = GetTestConfig(ssl);
- if (!config->digest_prefs.empty()) {
- bssl::UniquePtr<char> digest_prefs(
- OPENSSL_strdup(config->digest_prefs.c_str()));
- std::vector<int> digest_list;
-
- for (;;) {
- char *token =
- strtok(digest_list.empty() ? digest_prefs.get() : nullptr, ",");
- if (token == nullptr) {
- break;
- }
-
- digest_list.push_back(EVP_MD_type(EVP_get_digestbyname(token)));
- }
-
- if (!SSL_set_private_key_digest_prefs(ssl, digest_list.data(),
- digest_list.size())) {
- return false;
- }
- }
-
if (!config->signing_prefs.empty()) {
std::vector<uint16_t> u16s(config->signing_prefs.begin(),
config->signing_prefs.end());
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 7d02c15..8700af2 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -8306,8 +8306,8 @@
expectedError: ":NO_COMMON_SIGNATURE_ALGORITHMS:",
})
- // Test that hash preferences are enforced. BoringSSL does not implement
- // MD5 signatures.
+ // Test that signature preferences are enforced. BoringSSL does not
+ // implement MD5 signatures.
testCases = append(testCases, testCase{
testType: serverTest,
name: "ClientAuth-Enforced",
@@ -8376,26 +8376,8 @@
expectedError: ":WRONG_SIGNATURE_TYPE:",
})
- // Test that the agreed upon digest respects the client preferences and
- // the server digests.
- 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:",
- })
+ // Test that the negotiated signature algorithm respects the client and
+ // server preferences.
testCases = append(testCases, testCase{
name: "NoCommonAlgorithms",
config: Config{
@@ -8445,7 +8427,8 @@
flags: []string{
"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
"-key-file", path.Join(*resourceDir, rsaKeyFile),
- "-digest-prefs", "SHA256,SHA1",
+ "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA256)),
+ "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA1)),
},
expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA256,
})
@@ -8461,7 +8444,9 @@
flags: []string{
"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
"-key-file", path.Join(*resourceDir, rsaKeyFile),
- "-digest-prefs", "SHA512,SHA256,SHA1",
+ "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA512)),
+ "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA256)),
+ "-signing-prefs", strconv.Itoa(int(signatureRSAPKCS1WithSHA1)),
},
expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA1,
})
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 2d9f725..a5ce5a1 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -132,7 +132,6 @@
const Flag<std::string> kStringFlags[] = {
{ "-write-settings", &TestConfig::write_settings },
- { "-digest-prefs", &TestConfig::digest_prefs },
{ "-key-file", &TestConfig::key_file },
{ "-cert-file", &TestConfig::cert_file },
{ "-expect-server-name", &TestConfig::expected_server_name },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index b742f94..ea12d34 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -26,7 +26,6 @@
int resume_count = 0;
std::string write_settings;
bool fallback_scsv = false;
- std::string digest_prefs;
std::vector<int> signing_prefs;
std::vector<int> verify_prefs;
std::string key_file;
