From b9195402b41a4ec8e6141c31ff7b6c6561a29b3d Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Fri, 5 Aug 2016 10:51:43 -0400
Subject: [PATCH] Align SSL_SESSION_up_ref with OpenSSL.

Only X509_up_ref left (it's still waiting on a few external callers).

BUG=89

Change-Id: Ia2aec2bb0a944356cb1ce29f3b58a26bdb8a9977
Reviewed-on: https://boringssl-review.googlesource.com/9141
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
---
 include/openssl/ssl.h  |  6 +++---
 ssl/handshake_client.c |  3 ++-
 ssl/handshake_server.c |  3 ++-
 ssl/ssl_lib.c          | 11 ++++++-----
 ssl/ssl_session.c      | 15 ++++++++-------
 ssl/test/bssl_shim.cc  |  4 ++--
 6 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index fcceb89a..02a0766b 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1533,9 +1533,9 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION)
  * used outside the library. */
 OPENSSL_EXPORT SSL_SESSION *SSL_SESSION_new(void);
 
-/* SSL_SESSION_up_ref, if |session| is not NULL, increments the reference count
- * of |session|. It then returns |session|. */
-OPENSSL_EXPORT SSL_SESSION *SSL_SESSION_up_ref(SSL_SESSION *session);
+/* SSL_SESSION_up_ref increments the reference count of |session| and returns
+ * one. */
+OPENSSL_EXPORT int SSL_SESSION_up_ref(SSL_SESSION *session);
 
 /* SSL_SESSION_free decrements the reference count of |session|. If it reaches
  * zero, all data referenced by |session| and |session| itself are released. */
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 850b2bc6..0b50505c 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -508,7 +508,8 @@ int ssl3_connect(SSL *ssl) {
 
         SSL_SESSION_free(ssl->s3->established_session);
         if (ssl->session != NULL) {
-          ssl->s3->established_session = SSL_SESSION_up_ref(ssl->session);
+          SSL_SESSION_up_ref(ssl->session);
+          ssl->s3->established_session = 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
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index c47ba67b..a6d26d15 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -487,7 +487,8 @@ int ssl3_accept(SSL *ssl) {
 
         SSL_SESSION_free(ssl->s3->established_session);
         if (ssl->session != NULL) {
-          ssl->s3->established_session = SSL_SESSION_up_ref(ssl->session);
+          SSL_SESSION_up_ref(ssl->session);
+          ssl->s3->established_session = ssl->session;
         } else {
           ssl->s3->established_session = ssl->s3->new_session;
           ssl->s3->established_session->not_resumable = 0;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 64fca640..6a2dae86 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2088,11 +2088,12 @@ void ssl_update_cache(SSL *ssl, int mode) {
     if (use_internal_cache) {
       SSL_CTX_add_session(ctx, ssl->s3->established_session);
     }
-    if (ctx->new_session_cb != NULL &&
-        !ctx->new_session_cb(ssl, SSL_SESSION_up_ref(
-            ssl->s3->established_session))) {
-      /* |new_session_cb|'s return value signals whether it took ownership. */
-      SSL_SESSION_free(ssl->s3->established_session);
+    if (ctx->new_session_cb != NULL) {
+      SSL_SESSION_up_ref(ssl->s3->established_session);
+      if (!ctx->new_session_cb(ssl, ssl->s3->established_session)) {
+        /* |new_session_cb|'s return value signals whether it took ownership. */
+        SSL_SESSION_free(ssl->s3->established_session);
+      }
     }
   }
 
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index 283639de..c3a48999 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -266,11 +266,9 @@ err:
   return 0;
 }
 
-SSL_SESSION *SSL_SESSION_up_ref(SSL_SESSION *session) {
-  if (session != NULL) {
-    CRYPTO_refcount_inc(&session->references);
-  }
-  return session;
+int SSL_SESSION_up_ref(SSL_SESSION *session) {
+  CRYPTO_refcount_inc(&session->references);
+  return 1;
 }
 
 void SSL_SESSION_free(SSL_SESSION *session) {
@@ -384,8 +382,11 @@ SSL_SESSION *SSL_get_session(const SSL *ssl) {
 }
 
 SSL_SESSION *SSL_get1_session(SSL *ssl) {
-  /* variant of SSL_get_session: caller really gets something */
-  return SSL_SESSION_up_ref(SSL_get_session(ssl));
+  SSL_SESSION *ret = SSL_get_session(ssl);
+  if (ret != NULL) {
+    SSL_SESSION_up_ref(ret);
+  }
+  return ret;
 }
 
 int SSL_SESSION_get_ex_new_index(long argl, void *argp,
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index b2360de5..61bdd7bf 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1421,8 +1421,8 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx,
     } else if (config->async) {
       // The internal session cache is disabled, so install the session
       // manually.
-      GetTestState(ssl.get())->pending_session.reset(
-          SSL_SESSION_up_ref(session));
+      SSL_SESSION_up_ref(session);
+      GetTestState(ssl.get())->pending_session.reset(session);
     }
   }