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

@@ -130,6 +130,10 @@ Things which do not work:
 * 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

@@ -508,7 +508,10 @@ int ssl3_connect(SSL_HANDSHAKE *hs) {
             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 @@ int ssl3_connect(SSL_HANDSHAKE *hs) {
         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

@@ -218,6 +218,7 @@ void ssl_update_cache(SSL_HANDSHAKE *hs, int mode) {
   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

@@ -2372,6 +2372,13 @@ static bool DoExchange(bssl::UniquePtr<SSL_SESSION> *out_session, SSL *ssl,
     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));