commit 962b375bcb7de6422c2bc5c4ef6e527d8f74c74d
author David Benjamin <davidben@google.com> Mon May 10 15:17:18 2021 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu May 13 14:35:32 2021 +0000
tree a66198bbe77c19f452889c5a1f607118c76802a3
parent 8349dfc87e46d5914b0fefbb33241a95a9eef07d

Move session ID assignment out of ssl_get_new_session.

It's kind of weird that we assign a session ID, based on whether we
detect the handshake wants stateful resumption, and then erase it
afterwards.

Also remove the is_server parameter, which we can get from hs.

Change-Id: I94ac817c63abb08a457e0e0c29f5c2d2b60aa498
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47444
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

ssl/ssl_session.cc

diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index 91b2fff..41df63f 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -350,19 +350,19 @@
                                   session->cipher);
 }
 
-int ssl_get_new_session(SSL_HANDSHAKE *hs, int is_server) {
+bool ssl_get_new_session(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   if (ssl->mode & SSL_MODE_NO_SESSION_CREATION) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_SESSION_MAY_NOT_BE_CREATED);
-    return 0;
+    return false;
   }
 
   UniquePtr<SSL_SESSION> session = ssl_session_new(ssl->ctx->x509_method);
   if (session == NULL) {
-    return 0;
+    return false;
   }
 
-  session->is_server = is_server;
+  session->is_server = ssl->server;
   session->ssl_version = ssl->version;
   session->is_quic = ssl->quic_method != nullptr;
 
@@ -384,24 +384,9 @@
     session->auth_timeout = ssl->session_ctx->session_timeout;
   }
 
-  if (is_server) {
-    if (hs->ticket_expected || version >= TLS1_3_VERSION) {
-      // Don't set session IDs for sessions resumed with tickets. This will keep
-      // them out of the session cache.
-      session->session_id_length = 0;
-    } else {
-      session->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;
-      if (!RAND_bytes(session->session_id, session->session_id_length)) {
-        return 0;
-      }
-    }
-  } else {
-    session->session_id_length = 0;
-  }
-
   if (hs->config->cert->sid_ctx_length > sizeof(session->sid_ctx)) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-    return 0;
+    return false;
   }
   OPENSSL_memcpy(session->sid_ctx, hs->config->cert->sid_ctx,
                  hs->config->cert->sid_ctx_length);
@@ -413,7 +398,7 @@
 
   hs->new_session = std::move(session);
   ssl_set_session(ssl, NULL);
-  return 1;
+  return true;
 }
 
 int ssl_ctx_rotate_ticket_encryption_key(SSL_CTX *ctx) {