|
| 1 | +From 792613641e28dda603369bc34bc32263f24de8ac Mon Sep 17 00:00:00 2001 |
| 2 | +From: David Benjamin < [email protected]> |
| 3 | +Date: Mon, 12 Feb 2024 18:09:37 -0500 |
| 4 | +Subject: [PATCH] Remove some indirection in SSL_certs_clear |
| 5 | + |
| 6 | +If we move SSL_certs_clear to ssl_cert.cc, ssl_cert_clear_certs does not |
| 7 | +need to be in the header. Moreover, its only other caller, ~CERT(), does |
| 8 | +not need to call it. Now that everything outside of SSL_X509_METHOD is |
| 9 | +managed with scopers, the destructor does it automatically. And |
| 10 | +cert_free on SSL_X509_METHOD already automatically calls cert_clear, so |
| 11 | +it's a no-op to do it again. |
| 12 | + |
| 13 | +Change-Id: Ief9c704cc45440288783564ac4db4a27fbec1bfc |
| 14 | +Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66370 |
| 15 | +Commit-Queue: David Benjamin < [email protected]> |
| 16 | +Reviewed-by: Bob Beck < [email protected]> |
| 17 | +--- |
| 18 | + ssl/internal.h | 1 - |
| 19 | + ssl/ssl_cert.cc | 45 +++++++++++++++++++++------------------------ |
| 20 | + ssl/ssl_lib.cc | 7 ------- |
| 21 | + 3 files changed, 21 insertions(+), 32 deletions(-) |
| 22 | + |
| 23 | +diff --git a/ssl/internal.h b/ssl/internal.h |
| 24 | +index 80d0675d8..38617640c 100644 |
| 25 | +--- a/ssl/internal.h |
| 26 | ++++ b/ssl/internal.h |
| 27 | +@@ -3341,7 +3341,6 @@ struct SSL_CONFIG { |
| 28 | + static const size_t kMaxEarlyDataAccepted = 14336; |
| 29 | + |
| 30 | + UniquePtr<CERT> ssl_cert_dup(CERT *cert); |
| 31 | +-void ssl_cert_clear_certs(CERT *cert); |
| 32 | + bool ssl_set_cert(CERT *cert, UniquePtr<CRYPTO_BUFFER> buffer); |
| 33 | + bool ssl_is_key_type_supported(int key_type); |
| 34 | + // ssl_compare_public_and_private_key returns true if |pubkey| is the public |
| 35 | +diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc |
| 36 | +index 163c3f4cd..6a4b63c54 100644 |
| 37 | +--- a/ssl/ssl_cert.cc |
| 38 | ++++ b/ssl/ssl_cert.cc |
| 39 | +@@ -143,10 +143,7 @@ CERT::CERT(const SSL_X509_METHOD *x509_method_arg) |
| 40 | + } |
| 41 | + } |
| 42 | + |
| 43 | +-CERT::~CERT() { |
| 44 | +- ssl_cert_clear_certs(this); |
| 45 | +- x509_method->cert_free(this); |
| 46 | +-} |
| 47 | ++CERT::~CERT() { x509_method->cert_free(this); } |
| 48 | + |
| 49 | + static CRYPTO_BUFFER *buffer_up_ref(const CRYPTO_BUFFER *buffer) { |
| 50 | + CRYPTO_BUFFER_up_ref(const_cast<CRYPTO_BUFFER *>(buffer)); |
| 51 | +@@ -218,26 +215,6 @@ UniquePtr<CERT> ssl_cert_dup(CERT *cert) { |
| 52 | + return ret; |
| 53 | + } |
| 54 | + |
| 55 | +-// Free up and clear all certificates and chains |
| 56 | +-void ssl_cert_clear_certs(CERT *cert) { |
| 57 | +- if (cert == nullptr) { |
| 58 | +- return; |
| 59 | +- } |
| 60 | +- |
| 61 | +- cert->x509_method->cert_clear(cert); |
| 62 | +- |
| 63 | +- cert->cert_private_key_idx = -1; |
| 64 | +- for (auto &cert_private_key : cert->cert_private_keys) { |
| 65 | +- cert_private_key.chain.reset(); |
| 66 | +- cert_private_key.privatekey.reset(); |
| 67 | +- } |
| 68 | +- cert->key_method = nullptr; |
| 69 | +- |
| 70 | +- cert->dc.reset(); |
| 71 | +- cert->dc_privatekey.reset(); |
| 72 | +- cert->dc_key_method = nullptr; |
| 73 | +-} |
| 74 | +- |
| 75 | + static void ssl_cert_set_cert_cb(CERT *cert, int (*cb)(SSL *ssl, void *arg), |
| 76 | + void *arg) { |
| 77 | + cert->cert_cb = cb; |
| 78 | +@@ -984,6 +961,26 @@ int SSL_CTX_set_chain_and_key(SSL_CTX *ctx, CRYPTO_BUFFER *const *certs, |
| 79 | + privkey_method); |
| 80 | + } |
| 81 | + |
| 82 | ++void SSL_certs_clear(SSL *ssl) { |
| 83 | ++ if (!ssl->config) { |
| 84 | ++ return; |
| 85 | ++ } |
| 86 | ++ |
| 87 | ++ CERT *cert = ssl->config->cert.get(); |
| 88 | ++ cert->x509_method->cert_clear(cert); |
| 89 | ++ |
| 90 | ++ cert->cert_private_key_idx = -1; |
| 91 | ++ for (auto &cert_private_key : cert->cert_private_keys) { |
| 92 | ++ cert_private_key.chain.reset(); |
| 93 | ++ cert_private_key.privatekey.reset(); |
| 94 | ++ } |
| 95 | ++ cert->key_method = nullptr; |
| 96 | ++ |
| 97 | ++ cert->dc.reset(); |
| 98 | ++ cert->dc_privatekey.reset(); |
| 99 | ++ cert->dc_key_method = nullptr; |
| 100 | ++} |
| 101 | ++ |
| 102 | + const STACK_OF(CRYPTO_BUFFER) *SSL_CTX_get0_chain(const SSL_CTX *ctx) { |
| 103 | + if (!ssl_cert_check_cert_private_keys_usage(ctx->cert.get())) { |
| 104 | + return nullptr; |
| 105 | +diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc |
| 106 | +index c74aa04b1..092dbef31 100644 |
| 107 | +--- a/ssl/ssl_lib.cc |
| 108 | ++++ b/ssl/ssl_lib.cc |
| 109 | +@@ -1614,13 +1614,6 @@ const uint8_t *SSL_get0_session_id_context(const SSL *ssl, size_t *out_len) { |
| 110 | + return ssl->config->cert->sid_ctx; |
| 111 | + } |
| 112 | + |
| 113 | +-void SSL_certs_clear(SSL *ssl) { |
| 114 | +- if (!ssl->config) { |
| 115 | +- return; |
| 116 | +- } |
| 117 | +- ssl_cert_clear_certs(ssl->config->cert.get()); |
| 118 | +-} |
| 119 | +- |
| 120 | + int SSL_get_fd(const SSL *ssl) { return SSL_get_rfd(ssl); } |
| 121 | + |
| 122 | + int SSL_get_rfd(const SSL *ssl) { |
0 commit comments