commit 96123c01b8c872945004ea9a2c7a1e7c7bf13823
author David Benjamin <davidben@google.com> Tue Aug 29 11:47:04 2017 -0400
committer Przemyslaw Szczepaniak <pszczepaniak@google.com> Fri Sep 01 08:39:34 2017 +0000
tree 87596878721d10925afaeb18c4c86be9456bb734
parent 0a452296161a161f4eec75b7094688fd7699aedd

Mark renego-established sessions not resumable.

We do not call the new_session callback on renego, but a consumer using
SSL_get_session may still attempt to resume such a session. Leave the
not_resumable flag unset. Also document this renegotiation restriction.

This is a cherry-pick of https://boringssl-review.googlesource.com/19664
from BoringSSL, in preparation for cherry-picking
https://boringssl-review.googlesource.com/19665.

Exempt-From-Owner-Approval: flooey is on vacation 
Merged-In: I615af59fe8af4c56e6cf83a364c0fd69be70c415
Merged-In: I41df37881b9c2228b9a7b3569f1532f3b0dcbaea
Bug: 64827202
Change-Id: Ic81acb033d8166a6bd00edcbfa06157af12f98aa

src/PORTING.md

diff --git a/src/PORTING.md b/src/PORTING.md
index b9d6752..888ad42 100644
--- a/src/PORTING.md
+++ b/src/PORTING.md
@@ -128,6 +128,10 @@
 * If a HelloRequest is received while `SSL_write` has unsent application data,
   the renegotiation is rejected.
 
+* Renegotiation does not participate in session resumption. The client will
+  not offer a session on renegotiation or resume any session established by a
+  renegotiation handshake.
+
 ### Lowercase hexadecimal
 
 BoringSSL's `BN_bn2hex` function uses lowercase hexadecimal digits instead of

src/ssl/handshake_client.cc

diff --git a/src/ssl/handshake_client.cc b/src/ssl/handshake_client.cc
index 9efbf0a..f58bcff 100644
--- a/src/ssl/handshake_client.cc
+++ b/src/ssl/handshake_client.cc
@@ -503,7 +503,10 @@
             ret = -1;
             goto end;
           }
-          ssl->s3->established_session->not_resumable = 0;
+          /* Renegotiations do not participate in session resumption. */
+          if (!ssl->s3->initial_handshake_complete) {
+            ssl->s3->established_session->not_resumable = 0;
+          }
 
           SSL_SESSION_free(hs->new_session);
           hs->new_session = NULL;
@@ -513,12 +516,8 @@
         break;
 
       case SSL_ST_OK: {
-        const int is_initial_handshake = !ssl->s3->initial_handshake_complete;
         ssl->s3->initial_handshake_complete = 1;
-        if (is_initial_handshake) {
-          /* Renegotiations do not participate in session resumption. */
-          ssl_update_cache(hs, SSL_SESS_CACHE_CLIENT);
-        }
+        ssl_update_cache(hs, SSL_SESS_CACHE_CLIENT);
 
         ret = 1;
         ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_DONE, 1);

src/ssl/ssl_lib.cc

diff --git a/src/ssl/ssl_lib.cc b/src/ssl/ssl_lib.cc
index 7441925..b8d4c27 100644
--- a/src/ssl/ssl_lib.cc
+++ b/src/ssl/ssl_lib.cc
@@ -1811,6 +1811,7 @@
   SSL_CTX *ctx = ssl->session_ctx;
   /* Never cache sessions with empty session IDs. */
   if (ssl->s3->established_session->session_id_length == 0 ||
+      ssl->s3->established_session->not_resumable ||
       (ctx->session_cache_mode & mode) != mode) {
     return;
   }

src/ssl/test/bssl_shim.cc

diff --git a/src/ssl/test/bssl_shim.cc b/src/ssl/test/bssl_shim.cc
index cd846be..04f0b7c 100644
--- a/src/ssl/test/bssl_shim.cc
+++ b/src/ssl/test/bssl_shim.cc
@@ -2242,6 +2242,13 @@
     return false;
   }
 
+  if (SSL_total_renegotiations(ssl) > 0 &&
+      !SSL_get_session(ssl)->not_resumable) {
+    fprintf(stderr,
+            "Renegotiations should never produce resumable sessions.\n");
+    return false;
+  }
+
   if (SSL_total_renegotiations(ssl) != config->expect_total_renegotiations) {
     fprintf(stderr, "Expected %d renegotiations, got %d\n",
             config->expect_total_renegotiations, SSL_total_renegotiations(ssl));