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>
This commit is contained in:
David Benjamin 2017-11-21 07:48:20 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent 67623735e0
commit 855d5046c7
7 changed files with 12 additions and 184 deletions

View File

@ -151,7 +151,7 @@ extern "C" {
// 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)

View File

@ -1125,16 +1125,7 @@ enum ssl_private_key_result_t {
// 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 @@ struct ssl_private_key_method_st {
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_set_tmp_ecdh(SSL *ssl, const EC_KEY *ec_key);
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

View File

@ -193,33 +193,6 @@ static int setup_ctx(SSL *ssl, EVP_MD_CTX *ctx, EVP_PKEY *pkey, uint16_t sigalg,
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 @@ enum ssl_private_key_result_t ssl_private_key_sign(
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 @@ bool ssl_private_key_supports_signature_algorithm(SSL_HANDSHAKE *hs,
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 @@ int SSL_CTX_set_signing_algorithm_prefs(SSL_CTX *ctx, const uint16_t *prefs,
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 @@ int SSL_CTX_set_verify_algorithm_prefs(SSL_CTX *ctx, const uint16_t *prefs,
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;
}

View File

@ -433,10 +433,7 @@ static ssl_private_key_result_t AsyncPrivateKeyComplete(
}
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 @@ static bool GetCertificate(SSL *ssl, bssl::UniquePtr<X509> *out_x509,
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());

View File

@ -8306,8 +8306,8 @@ func addSignatureAlgorithmTests() {
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 @@ func addSignatureAlgorithmTests() {
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 @@ func addSignatureAlgorithmTests() {
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 @@ func addSignatureAlgorithmTests() {
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,
})

View File

@ -132,7 +132,6 @@ const Flag<bool> kBoolFlags[] = {
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 },

View File

@ -26,7 +26,6 @@ struct TestConfig {
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;