Check for resumption identifiers in SSL_SESSION_is_resumable.
This aligns with OpenSSL. In particular, we clear not_resumable as soon
as the SSL_SESSION is complete, but it may not have an ID or ticket.
(Due to APIs like SSL_get_session, SSL_SESSION needs to act both as a
resumption handle and a bundle of connection properties.)
Along the way, use the modified function in a few internal checks which,
with the ssl_update_cache change, removes the last dependency within the
library on the placeholder SHA256 IDs.
Change-Id: Ic225109ff31ec63ec08625e9f61a20cf0d9dd648
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47447
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 8f23d95..1a14c11 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -402,9 +402,7 @@
if (ssl->session != nullptr) {
if (ssl->session->is_server ||
!ssl_supports_version(hs, ssl->session->ssl_version) ||
- (ssl->session->session_id_length == 0 &&
- ssl->session->ticket.empty()) ||
- ssl->session->not_resumable ||
+ !SSL_SESSION_is_resumable(ssl->session.get()) ||
!ssl_session_is_time_valid(ssl, ssl->session.get()) ||
(ssl->quic_method != nullptr) != ssl->session->is_quic ||
ssl->s3->initial_handshake_complete) {
@@ -1666,8 +1664,8 @@
}
session->ticket_lifetime_hint = ticket_lifetime_hint;
- // Generate a session ID for this session. Some callers expect all sessions to
- // have a session ID.
+ // Historically, OpenSSL filled in fake session IDs for ticket-based sessions.
+ // TODO(davidben): Are external callers relying on this? Try removing this.
SHA256(CBS_data(&ticket), CBS_len(&ticket), session->session_id);
session->session_id_length = SHA256_DIGEST_LENGTH;
