Convert a few more scopers.
Bug: 132
Change-Id: I75d6ce5a2256a4b464ca6a9378ac6b63a9bd47e2
Reviewed-on: https://boringssl-review.googlesource.com/18644
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 9e15e7e..fb1c4d8 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -817,14 +817,10 @@
}
/* Determine whether we are doing session resumption. */
+ UniquePtr<SSL_SESSION> session;
int tickets_supported = 0, renew_ticket = 0;
- /* TODO(davidben): Switch |ssl_get_prev_session| to take a |UniquePtr|
- * output and simplify this. */
- SSL_SESSION *session_raw = nullptr;
- auto session_ret = ssl_get_prev_session(ssl, &session_raw, &tickets_supported,
- &renew_ticket, &client_hello);
- UniquePtr<SSL_SESSION> session(session_raw);
- switch (session_ret) {
+ switch (ssl_get_prev_session(ssl, &session, &tickets_supported, &renew_ticket,
+ &client_hello)) {
case ssl_session_success:
break;
case ssl_session_error:
diff --git a/ssl/internal.h b/ssl/internal.h
index 3ac6716..fe73d1e 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2083,13 +2083,13 @@
};
/* ssl_get_prev_session looks up the previous session based on |client_hello|.
- * On success, it sets |*out_session| to the session or NULL if none was found.
- * If the session could not be looked up synchronously, it returns
+ * On success, it sets |*out_session| to the session or nullptr if none was
+ * found. If the session could not be looked up synchronously, it returns
* |ssl_session_retry| and should be called again. If a ticket could not be
* decrypted immediately it returns |ssl_session_ticket_retry| and should also
* be called again. Otherwise, it returns |ssl_session_error|. */
enum ssl_session_result_t ssl_get_prev_session(
- SSL *ssl, SSL_SESSION **out_session, int *out_tickets_supported,
+ SSL *ssl, UniquePtr<SSL_SESSION> *out_session, int *out_tickets_supported,
int *out_renew_ticket, const SSL_CLIENT_HELLO *client_hello);
/* The following flags determine which parts of the session are duplicated. */
@@ -2267,7 +2267,7 @@
* Retry later.
* |ssl_ticket_aead_error|: an error occured that is fatal to the connection. */
enum ssl_ticket_aead_result_t ssl_process_ticket(
- SSL *ssl, SSL_SESSION **out_session, int *out_renew_ticket,
+ SSL *ssl, UniquePtr<SSL_SESSION> *out_session, int *out_renew_ticket,
const uint8_t *ticket, size_t ticket_len, const uint8_t *session_id,
size_t session_id_len);
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index 02d6422..a680471 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -610,18 +610,17 @@
}
/* ssl_lookup_session looks up |session_id| in the session cache and sets
- * |*out_session| to an |SSL_SESSION| object if found. The caller takes
- * ownership of the result. */
+ * |*out_session| to an |SSL_SESSION| object if found. */
static enum ssl_session_result_t ssl_lookup_session(
- SSL *ssl, SSL_SESSION **out_session, const uint8_t *session_id,
+ SSL *ssl, UniquePtr<SSL_SESSION> *out_session, const uint8_t *session_id,
size_t session_id_len) {
- *out_session = NULL;
+ out_session->reset();
if (session_id_len == 0 || session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH) {
return ssl_session_success;
}
- SSL_SESSION *session = NULL;
+ UniquePtr<SSL_SESSION> session;
/* Try the internal cache, if it exists. */
if (!(ssl->session_ctx->session_cache_mode &
SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) {
@@ -631,26 +630,27 @@
OPENSSL_memcpy(data.session_id, session_id, session_id_len);
CRYPTO_MUTEX_lock_read(&ssl->session_ctx->lock);
- session = lh_SSL_SESSION_retrieve(ssl->session_ctx->sessions, &data);
- if (session != NULL) {
- SSL_SESSION_up_ref(session);
+ 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());
}
/* TODO(davidben): This should probably move it to the front of the list. */
CRYPTO_MUTEX_unlock_read(&ssl->session_ctx->lock);
}
/* Fall back to the external cache, if it exists. */
- if (session == NULL &&
- ssl->session_ctx->get_session_cb != NULL) {
+ if (!session && ssl->session_ctx->get_session_cb != NULL) {
int copy = 1;
- session = ssl->session_ctx->get_session_cb(ssl, (uint8_t *)session_id,
- session_id_len, ©);
+ session.reset(ssl->session_ctx->get_session_cb(ssl, (uint8_t *)session_id,
+ session_id_len, ©));
- if (session == NULL) {
+ if (!session) {
return ssl_session_success;
}
- if (session == SSL_magic_pending_session_ptr()) {
+ if (session.get() == SSL_magic_pending_session_ptr()) {
+ session.release(); // This pointer is not actually owned.
return ssl_session_retry;
}
@@ -659,34 +659,32 @@
* between threads, it must handle the reference count itself [i.e. copy ==
* 0], or things won't be thread-safe). */
if (copy) {
- SSL_SESSION_up_ref(session);
+ SSL_SESSION_up_ref(session.get());
}
/* Add the externally cached session to the internal cache if necessary. */
if (!(ssl->session_ctx->session_cache_mode &
SSL_SESS_CACHE_NO_INTERNAL_STORE)) {
- SSL_CTX_add_session(ssl->session_ctx, session);
+ SSL_CTX_add_session(ssl->session_ctx, session.get());
}
}
- if (session != NULL &&
- !ssl_session_is_time_valid(ssl, session)) {
+ if (session && !ssl_session_is_time_valid(ssl, session.get())) {
/* The session was from the cache, so remove it. */
- SSL_CTX_remove_session(ssl->session_ctx, session);
- SSL_SESSION_free(session);
- session = NULL;
+ SSL_CTX_remove_session(ssl->session_ctx, session.get());
+ session.reset();
}
- *out_session = session;
+ *out_session = std::move(session);
return ssl_session_success;
}
enum ssl_session_result_t ssl_get_prev_session(
- SSL *ssl, SSL_SESSION **out_session, int *out_tickets_supported,
+ SSL *ssl, UniquePtr<SSL_SESSION> *out_session, int *out_tickets_supported,
int *out_renew_ticket, const SSL_CLIENT_HELLO *client_hello) {
/* This is used only by servers. */
assert(ssl->server);
- SSL_SESSION *session = NULL;
+ UniquePtr<SSL_SESSION> session;
int renew_ticket = 0;
/* If tickets are disabled, always behave as if no tickets are present. */
@@ -704,7 +702,7 @@
case ssl_ticket_aead_success:
break;
case ssl_ticket_aead_ignore_ticket:
- assert(session == NULL);
+ assert(!session);
break;
case ssl_ticket_aead_error:
return ssl_session_error;
@@ -720,7 +718,7 @@
}
}
- *out_session = session;
+ *out_session = std::move(session);
*out_tickets_supported = tickets_supported;
*out_renew_ticket = renew_ticket;
return ssl_session_success;
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 9dbf24c..27a942a 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -3123,11 +3123,11 @@
}
enum ssl_ticket_aead_result_t ssl_process_ticket(
- SSL *ssl, SSL_SESSION **out_session, int *out_renew_ticket,
+ SSL *ssl, UniquePtr<SSL_SESSION> *out_session, int *out_renew_ticket,
const uint8_t *ticket, size_t ticket_len, const uint8_t *session_id,
size_t session_id_len) {
*out_renew_ticket = 0;
- *out_session = NULL;
+ out_session->reset();
if ((SSL_get_options(ssl) & SSL_OP_NO_TICKET) ||
session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH) {
@@ -3150,11 +3150,11 @@
}
/* Decode the session. */
- SSL_SESSION *session =
- SSL_SESSION_from_bytes(plaintext, plaintext_len, ssl->ctx);
+ UniquePtr<SSL_SESSION> session(
+ SSL_SESSION_from_bytes(plaintext, plaintext_len, ssl->ctx));
OPENSSL_free(plaintext);
- if (session == NULL) {
+ if (!session) {
ERR_clear_error(); /* Don't leave an error on the queue. */
return ssl_ticket_aead_ignore_ticket;
}
@@ -3164,7 +3164,7 @@
OPENSSL_memcpy(session->session_id, session_id, session_id_len);
session->session_id_length = session_id_len;
- *out_session = session;
+ *out_session = std::move(session);
return ssl_ticket_aead_success;
}
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 067c427..933affa 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -255,7 +255,7 @@
}
static enum ssl_ticket_aead_result_t select_session(
- SSL_HANDSHAKE *hs, uint8_t *out_alert, SSL_SESSION **out_session,
+ SSL_HANDSHAKE *hs, uint8_t *out_alert, UniquePtr<SSL_SESSION> *out_session,
int32_t *out_ticket_age_skew, const SSL_CLIENT_HELLO *client_hello) {
SSL *const ssl = hs->ssl;
*out_session = NULL;
@@ -288,7 +288,7 @@
/* TLS 1.3 session tickets are renewed separately as part of the
* NewSessionTicket. */
int unused_renew;
- SSL_SESSION *session = NULL;
+ UniquePtr<SSL_SESSION> session;
enum ssl_ticket_aead_result_t ret =
ssl_process_ticket(ssl, &session, &unused_renew, CBS_data(&ticket),
CBS_len(&ticket), NULL, 0);
@@ -302,10 +302,9 @@
return ret;
}
- if (!ssl_session_is_resumable(hs, session) ||
+ if (!ssl_session_is_resumable(hs, session.get()) ||
/* Historically, some TLS 1.3 tickets were missing ticket_age_add. */
!session->ticket_age_add_valid) {
- SSL_SESSION_free(session);
return ssl_ticket_aead_ignore_ticket;
}
@@ -323,7 +322,6 @@
/* To avoid overflowing |hs->ticket_age_skew|, we will not resume
* 68-year-old sessions. */
if (server_ticket_age > INT32_MAX) {
- SSL_SESSION_free(session);
return ssl_ticket_aead_ignore_ticket;
}
@@ -333,13 +331,12 @@
(int32_t)client_ticket_age - (int32_t)server_ticket_age;
/* Check the PSK binder. */
- if (!tls13_verify_psk_binder(hs, session, &binders)) {
- SSL_SESSION_free(session);
+ if (!tls13_verify_psk_binder(hs, session.get(), &binders)) {
*out_alert = SSL_AD_DECRYPT_ERROR;
return ssl_ticket_aead_error;
}
- *out_session = session;
+ *out_session = std::move(session);
return ssl_ticket_aead_success;
}
@@ -354,11 +351,11 @@
}
uint8_t alert = SSL_AD_DECODE_ERROR;
- SSL_SESSION *session = NULL;
+ UniquePtr<SSL_SESSION> session;
switch (select_session(hs, &alert, &session, &ssl->s3->ticket_age_skew,
&client_hello)) {
case ssl_ticket_aead_ignore_ticket:
- assert(session == NULL);
+ assert(!session);
if (!ssl_get_new_session(hs, 1 /* server */)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return ssl_hs_error;
@@ -368,7 +365,8 @@
case ssl_ticket_aead_success:
/* Carry over authentication information from the previous handshake into
* a fresh session. */
- hs->new_session = SSL_SESSION_dup(session, SSL_SESSION_DUP_AUTH_ONLY);
+ hs->new_session =
+ SSL_SESSION_dup(session.get(), SSL_SESSION_DUP_AUTH_ONLY);
if (/* Early data must be acceptable for this ticket. */
ssl->cert->enable_early_data &&
@@ -384,7 +382,6 @@
ssl->early_data_accepted = 1;
}
- SSL_SESSION_free(session);
if (hs->new_session == NULL) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return ssl_hs_error;
