kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Add bssl::UpRef.

Browse Source 
bssl::UniquePtr and FOO_up_ref do not play well together. Add a helper
to simplify this. This allows us to write things like:

   foo->cert = UpRef(bar->cert);

instead of:

   if (bar->cert) {
     X509_up_ref(bar->cert.get());
   }
   foo->cert.reset(bar->cert.get());

This also plays well with PushToStack. To append something to a stack
while taking a reference, it's just:

   PushToStack(certs, UpRef(cert))

Change-Id: I99ae8de22b837588a2d8ffb58f86edc1d03ed46a
Reviewed-on: https://boringssl-review.googlesource.com/29584
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2018-06-29 17:46:42 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 2e74fdaa4a
commit 2908dd141f
19 changed files with 57 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
crypto/pool/pool_test.cc
View File

@ -29,8 +29,7 @@ TEST(PoolTest, Unpooled) {
Bytes(CRYPTO_BUFFER_data(buf.get()), CRYPTO_BUFFER_len(buf.get())));
// Test that reference-counting works properly.
CRYPTO_BUFFER_up_ref(buf.get());
bssl::UniquePtr<CRYPTO_BUFFER> buf2(buf.get());
bssl::UniquePtr<CRYPTO_BUFFER> buf2 = bssl::UpRef(buf);
}
TEST(PoolTest, Empty) {

6
crypto/x509/x509_test.cc
View File

@ -530,10 +530,9 @@ static bssl::UniquePtr<STACK_OF(X509)> CertsToStack(
return nullptr;
}
for (auto cert : certs) {
if (!sk_X509_push(stack.get(), cert)) {
if (!bssl::PushToStack(stack.get(), bssl::UpRef(cert))) {
return nullptr;
}
X509_up_ref(cert);
}
return stack;
@ -548,10 +547,9 @@ static bssl::UniquePtr<STACK_OF(X509_CRL)> CRLsToStack(
return nullptr;
}
for (auto crl : crls) {
if (!sk_X509_CRL_push(stack.get(), crl)) {
if (!bssl::PushToStack(stack.get(), bssl::UpRef(crl))) {
return nullptr;
}
X509_CRL_up_ref(crl);
}
return stack;

4
fuzz/ssl_ctx_api.cc
View File

@ -212,10 +212,8 @@ struct GlobalState {
cert_.reset(d2i_X509(NULL, &bufp, sizeof(kCertificateDER)));
assert(cert_.get() != nullptr);
X509_up_ref(cert_.get());
certs_.reset(sk_X509_new_null());
sk_X509_push(certs_.get(), cert_.get());
PushToStack(certs_.get(), UpRef(cert_));
}
bssl::UniquePtr<EVP_PKEY> pkey_;

12
include/openssl/base.h
View File

@ -448,6 +448,18 @@ class StackAllocated {
template <typename T>
using UniquePtr = std::unique_ptr<T, internal::Deleter<T>>;
#define BORINGSSL_MAKE_UP_REF(type, up_ref_func) \
static inline UniquePtr<type> UpRef(type *v) { \
if (v != nullptr) { \
up_ref_func(v); \
} \
return UniquePtr<type>(v); \
} \
\
static inline UniquePtr<type> UpRef(const UniquePtr<type> &ptr) { \
return UpRef(ptr.get()); \
}
} // namespace bssl
} // extern C++

1
include/openssl/bio.h
View File

@ -874,6 +874,7 @@ extern "C++" {
namespace bssl {
BORINGSSL_MAKE_DELETER(BIO, BIO_free)
BORINGSSL_MAKE_UP_REF(BIO, BIO_up_ref)
} // namespace bssl

1
include/openssl/evp.h
View File

@ -840,6 +840,7 @@ extern "C++" {
namespace bssl {
BORINGSSL_MAKE_DELETER(EVP_PKEY, EVP_PKEY_free)
BORINGSSL_MAKE_UP_REF(EVP_PKEY, EVP_PKEY_up_ref)
BORINGSSL_MAKE_DELETER(EVP_PKEY_CTX, EVP_PKEY_CTX_free)
} // namespace bssl

1
include/openssl/pool.h
View File

@ -91,6 +91,7 @@ namespace bssl {
BORINGSSL_MAKE_DELETER(CRYPTO_BUFFER_POOL, CRYPTO_BUFFER_POOL_free)
BORINGSSL_MAKE_DELETER(CRYPTO_BUFFER, CRYPTO_BUFFER_free)
BORINGSSL_MAKE_UP_REF(CRYPTO_BUFFER, CRYPTO_BUFFER_up_ref)
} // namespace bssl

2
include/openssl/ssl.h
View File

@ -4444,7 +4444,9 @@ namespace bssl {
BORINGSSL_MAKE_DELETER(SSL, SSL_free)
BORINGSSL_MAKE_DELETER(SSL_CTX, SSL_CTX_free)
BORINGSSL_MAKE_UP_REF(SSL_CTX, SSL_CTX_up_ref)
BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free)
BORINGSSL_MAKE_UP_REF(SSL_SESSION, SSL_SESSION_up_ref)
// Certificate compression.
//

2
include/openssl/x509.h
View File

@ -1132,8 +1132,10 @@ namespace bssl {
BORINGSSL_MAKE_DELETER(NETSCAPE_SPKI, NETSCAPE_SPKI_free)
BORINGSSL_MAKE_DELETER(RSA_PSS_PARAMS, RSA_PSS_PARAMS_free)
BORINGSSL_MAKE_DELETER(X509, X509_free)
BORINGSSL_MAKE_UP_REF(X509, X509_up_ref)
BORINGSSL_MAKE_DELETER(X509_ALGOR, X509_ALGOR_free)
BORINGSSL_MAKE_DELETER(X509_CRL, X509_CRL_free)
BORINGSSL_MAKE_UP_REF(X509_CRL, X509_CRL_up_ref)
BORINGSSL_MAKE_DELETER(X509_CRL_METHOD, X509_CRL_METHOD_free)
BORINGSSL_MAKE_DELETER(X509_EXTENSION, X509_EXTENSION_free)
BORINGSSL_MAKE_DELETER(X509_INFO, X509_INFO_free)

6
ssl/handshake_client.cc
View File

@ -465,8 +465,7 @@ static enum ssl_hs_wait_t do_enter_early_data(SSL_HANDSHAKE *hs) {
// Stash the early data session, so connection properties may be queried out
// of it.
hs->in_early_data = true;
SSL_SESSION_up_ref(ssl->session);
hs->early_session.reset(ssl->session);
hs->early_session = UpRef(ssl->session);
hs->can_early_write = true;
hs->state = state_read_server_hello;
@ -1622,8 +1621,7 @@ static enum ssl_hs_wait_t do_finish_client_handshake(SSL_HANDSHAKE *hs) {
ssl->method->on_handshake_complete(ssl);
if (ssl->session != NULL) {
SSL_SESSION_up_ref(ssl->session);
ssl->s3->established_session.reset(ssl->session);
ssl->s3->established_session = UpRef(ssl->session);
} else {
// We make a copy of the session in order to maintain the immutability
// of the new established_session due to False Start. The caller may

3
ssl/handshake_server.cc
View File

@ -1474,8 +1474,7 @@ static enum ssl_hs_wait_t do_finish_server_handshake(SSL_HANDSHAKE *hs) {
}
if (ssl->session != NULL) {
SSL_SESSION_up_ref(ssl->session);
ssl->s3->established_session.reset(ssl->session);
ssl->s3->established_session = UpRef(ssl->session);
} else {
ssl->s3->established_session = std::move(hs->new_session);
ssl->s3->established_session->not_resumable = false;

7
ssl/ssl_asn1.cc
View File

@ -703,10 +703,9 @@ UniquePtr<SSL_SESSION> SSL_SESSION_parse(CBS *cbs,
return nullptr;
}
CRYPTO_BUFFER *buffer = CRYPTO_BUFFER_new_from_CBS(&cert, pool);
if (buffer == NULL ||
!sk_CRYPTO_BUFFER_push(ret->certs, buffer)) {
CRYPTO_BUFFER_free(buffer);
UniquePtr<CRYPTO_BUFFER> buffer(CRYPTO_BUFFER_new_from_CBS(&cert, pool));
if (buffer == nullptr ||
!PushToStack(ret->certs, std::move(buffer))) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return nullptr;
}

26
ssl/ssl_cert.cc
View File

@ -162,11 +162,7 @@ UniquePtr<CERT> ssl_cert_dup(CERT *cert) {
}
}
if (cert->privatekey) {
EVP_PKEY_up_ref(cert->privatekey.get());
ret->privatekey.reset(cert->privatekey.get());
}
ret->privatekey = UpRef(cert->privatekey);
ret->key_method = cert->key_method;
if (!ret->sigalgs.CopyFrom(cert->sigalgs)) {
@ -178,16 +174,8 @@ UniquePtr<CERT> ssl_cert_dup(CERT *cert) {
ret->x509_method->cert_dup(ret.get(), cert);
if (cert->signed_cert_timestamp_list) {
CRYPTO_BUFFER_up_ref(cert->signed_cert_timestamp_list.get());
ret->signed_cert_timestamp_list.reset(
cert->signed_cert_timestamp_list.get());
}
if (cert->ocsp_response) {
CRYPTO_BUFFER_up_ref(cert->ocsp_response.get());
ret->ocsp_response.reset(cert->ocsp_response.get());
}
ret->signed_cert_timestamp_list = UpRef(cert->signed_cert_timestamp_list);
ret->ocsp_response = UpRef(cert->ocsp_response);
ret->sid_ctx_length = cert->sid_ctx_length;
OPENSSL_memcpy(ret->sid_ctx, cert->sid_ctx, sizeof(ret->sid_ctx));
@ -289,16 +277,12 @@ static int cert_set_chain_and_key(
}
for (size_t i = 0; i < num_certs; i++) {
if (!sk_CRYPTO_BUFFER_push(certs_sk.get(), certs[i])) {
if (!PushToStack(certs_sk.get(), UpRef(certs[i]))) {
return 0;
}
CRYPTO_BUFFER_up_ref(certs[i]);
}
if (privkey != nullptr) {
EVP_PKEY_up_ref(privkey);
}
cert->privatekey.reset(privkey);
cert->privatekey = UpRef(privkey);
cert->key_method = privkey_method;
cert->chain = std::move(certs_sk);

9
ssl/ssl_lib.cc
View File

@ -295,10 +295,10 @@ void ssl_update_cache(SSL_HANDSHAKE *hs, int mode) {
SSL_CTX_add_session(ctx, ssl->s3->established_session.get());
}
if (ctx->new_session_cb != NULL) {
SSL_SESSION_up_ref(ssl->s3->established_session.get());
if (!ctx->new_session_cb(ssl, ssl->s3->established_session.get())) {
UniquePtr<SSL_SESSION> ref = UpRef(ssl->s3->established_session);
if (ctx->new_session_cb(ssl, ref.get())) {
// |new_session_cb|'s return value signals whether it took ownership.
SSL_SESSION_free(ssl->s3->established_session.get());
ref.release();
}
}
}
@ -2836,8 +2836,7 @@ int SSL_clear(SSL *ssl) {
// depends on this behavior, so emulate it.
UniquePtr<SSL_SESSION> session;
if (!ssl->server && ssl->s3->established_session != NULL) {
session.reset(ssl->s3->established_session.get());
SSL_SESSION_up_ref(session.get());
session = UpRef(ssl->s3->established_session);
}
// The ssl->d1->mtu is simultaneously configuration (preserved across

4
ssl/ssl_privkey.cc
View File

@ -89,9 +89,7 @@ static int ssl_set_pkey(CERT *cert, EVP_PKEY *pkey) {
return 0;
}
EVP_PKEY_up_ref(pkey);
cert->privatekey.reset(pkey);
cert->privatekey = UpRef(pkey);
return 1;
}

24
ssl/ssl_session.cc
View File

@ -209,17 +209,15 @@ UniquePtr<SSL_SESSION> SSL_SESSION_dup(SSL_SESSION *session, int dup_flags) {
}
}
if (session->certs != NULL) {
new_session->certs = sk_CRYPTO_BUFFER_new_null();
auto buf_up_ref = [](CRYPTO_BUFFER *buf) {
CRYPTO_BUFFER_up_ref(buf);
return buf;
};
new_session->certs = sk_CRYPTO_BUFFER_deep_copy(session->certs, buf_up_ref,
CRYPTO_BUFFER_free);
if (new_session->certs == NULL) {
return nullptr;
}
for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(session->certs); i++) {
CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(session->certs, i);
if (!sk_CRYPTO_BUFFER_push(new_session->certs, buffer)) {
return nullptr;
}
CRYPTO_BUFFER_up_ref(buffer);
}
}
if (!session->x509_method->session_dup(new_session.get(), session)) {
@ -677,11 +675,8 @@ static enum ssl_hs_wait_t ssl_lookup_session(
OPENSSL_memcpy(data.session_id, session_id, session_id_len);
MutexReadLock lock(&ssl->session_ctx->lock);
session.reset(lh_SSL_SESSION_retrieve(ssl->session_ctx->sessions, &data));
if (session) {
// |lh_SSL_SESSION_retrieve| returns a non-owning pointer.
SSL_SESSION_up_ref(session.get());
}
// |lh_SSL_SESSION_retrieve| returns a non-owning pointer.
session = UpRef(lh_SSL_SESSION_retrieve(ssl->session_ctx->sessions, &data));
// TODO(davidben): This should probably move it to the front of the list.
}
@ -1118,8 +1113,7 @@ void *SSL_SESSION_get_ex_data(const SSL_SESSION *session, int idx) {
int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *session) {
// Although |session| is inserted into two structures (a doubly-linked list
// and the hash table), |ctx| only takes one reference.
SSL_SESSION_up_ref(session);
UniquePtr<SSL_SESSION> owned_session(session);
UniquePtr<SSL_SESSION> owned_session = UpRef(session);
SSL_SESSION *old_session;
MutexWriteLock lock(&ctx->lock);

7
ssl/ssl_test.cc
View File

@ -1322,9 +1322,7 @@ TEST(SSLTest, ClientCAList) {
bssl::UniquePtr<STACK_OF(X509_NAME)> stack(sk_X509_NAME_new_null());
ASSERT_TRUE(stack);
ASSERT_TRUE(sk_X509_NAME_push(stack.get(), name_dup.get()));
name_dup.release();
ASSERT_TRUE(PushToStack(stack.get(), std::move(name_dup)));
// |SSL_set_client_CA_list| takes ownership.
SSL_set_client_CA_list(ssl.get(), stack.release());
@ -3235,8 +3233,7 @@ TEST(SSLTest, ClientCABuffers) {
bssl::UniquePtr<STACK_OF(CRYPTO_BUFFER)> ca_names(
sk_CRYPTO_BUFFER_new_null());
ASSERT_TRUE(ca_names);
ASSERT_TRUE(sk_CRYPTO_BUFFER_push(ca_names.get(), ca_name.get()));
ca_name.release();
ASSERT_TRUE(PushToStack(ca_names.get(), std::move(ca_name)));
SSL_CTX_set0_client_CAs(server_ctx.get(), ca_names.release());
// Configure client and server to accept all certificates.

12
ssl/ssl_x509.cc
View File

@ -209,13 +209,10 @@ static int ssl_cert_set_chain(CERT *cert, STACK_OF(X509) *chain) {
return 0;
}
CRYPTO_BUFFER *leaf = sk_CRYPTO_BUFFER_value(cert->chain.get(), 0);
if (!sk_CRYPTO_BUFFER_push(new_chain.get(), leaf)) {
return 0;
}
// |leaf| might be NULL if it's a “leafless” chain.
if (leaf != nullptr) {
CRYPTO_BUFFER_up_ref(leaf);
CRYPTO_BUFFER *leaf = sk_CRYPTO_BUFFER_value(cert->chain.get(), 0);
if (!PushToStack(new_chain.get(), UpRef(leaf))) {
return 0;
}
}
@ -552,12 +549,11 @@ STACK_OF(X509) *SSL_get_peer_cert_chain(const SSL *ssl) {
for (size_t i = 1; i < sk_X509_num(session->x509_chain); i++) {
X509 *cert = sk_X509_value(session->x509_chain, i);
if (!sk_X509_push(session->x509_chain_without_leaf, cert)) {
if (!PushToStack(session->x509_chain_without_leaf, UpRef(cert))) {
sk_X509_pop_free(session->x509_chain_without_leaf, X509_free);
session->x509_chain_without_leaf = NULL;
return NULL;
}
X509_up_ref(cert);
}
}

8
ssl/test/bssl_shim.cc
View File

@ -203,10 +203,9 @@ static bool LoadCertificate(bssl::UniquePtr<X509> *out_x509,
break;
}
if (!sk_X509_push(out_chain->get(), cert.get())) {
if (!bssl::PushToStack(out_chain->get(), std::move(cert))) {
return false;
}
cert.release(); // sk_X509_push takes ownership.
}
uint32_t err = ERR_peek_last_error();
@ -316,10 +315,9 @@ static bssl::UniquePtr<STACK_OF(X509_NAME)> DecodeHexX509Names(
return nullptr;
}
if (!sk_X509_NAME_push(ret.get(), name.get())) {
if (!bssl::PushToStack(ret.get(), std::move(name))) {
return nullptr;
}
name.release();
}
return ret;
@ -1640,7 +1638,7 @@ static bool CheckAuthProperties(SSL *ssl, bool is_resume,
if (!sk_X509_insert(expect_chain.get(), expect_leaf.get(), 0)) {
return false;
}
X509_up_ref(expect_leaf.get()); // sk_X509_push takes ownership.
X509_up_ref(expect_leaf.get()); // sk_X509_insert takes ownership.
}
bssl::UniquePtr<X509> leaf(SSL_get_peer_certificate(ssl));