commit 5ef40c60f67e9060b33f02f49b366f508c0d09fa
author David Benjamin <davidben@google.com> Wed Aug 23 21:28:29 2017 -0700
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Aug 24 16:10:54 2017 +0000
tree dabf6de173679141e88e9527e4c56862320a532b
parent 2c46c10631a934b1d3426802e1b5f8be76852a9c

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.

Change-Id: I5361f522700b02edf5272ba5089c0777e5dafb09
Reviewed-on: https://boringssl-review.googlesource.com/19664
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

PORTING.md

diff --git a/PORTING.md b/PORTING.md
index ca9f6a4..eca7194 100644
--- a/PORTING.md
+++ b/PORTING.md
@@ -130,6 +130,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

ssl/handshake_client.cc

diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index e3c4641..6afd00b 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -508,7 +508,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;
+          }
 
           hs->new_session.reset();
         }
@@ -517,12 +520,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);

ssl/ssl_lib.cc

diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 32ec272..9ecd7df 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -218,6 +218,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;
   }

ssl/test/bssl_shim.cc

diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 8f4126e..d7d2efa 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -2372,6 +2372,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));