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/handshake_server.cc

diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 02603a1..eaf3a5e 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -892,14 +892,17 @@
     hs->can_release_private_key = true;
   } else {
     hs->ticket_expected = tickets_supported;
-    ssl_set_session(ssl, NULL);
-    if (!ssl_get_new_session(hs, 1 /* server */)) {
+    ssl_set_session(ssl, nullptr);
+    if (!ssl_get_new_session(hs)) {
       return ssl_hs_error;
     }
 
-    // Clear the session ID if we want the session to be single-use.
-    if (!(ssl->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) {
-      hs->new_session->session_id_length = 0;
+    // Assign a session ID if not using session tickets.
+    if (!hs->ticket_expected &&
+        (ssl->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) {
+      hs->new_session->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;
+      RAND_bytes(hs->new_session->session_id,
+                 hs->new_session->session_id_length);
     }
   }