Fill in a fake session ID for TLS 1.3.
Historically, OpenSSL filled in a fake session ID for ticket-only
client sessions. Conscrypt relies on this to implement some weird Java
API where every session has an ID and may be queried out of the client
session cache and, e.g., revoked that way.
(Note that a correct client session cache is not keyed by session ID and
indeed this allows one server to knock out another server's sessions by
matching session IDs. But existing APIs are existing APIs.)
For consistency between TLS 1.2 and TLS 1.3, as well as matching
OpenSSL's TLS 1.3 implementation, do the same in TLS 1.3. Note this
smooths over our cross-version resumption tests by allowing for
something odd: it is now syntactically possible to resume a TLS 1.3
session at TLS 1.2. It doesn't matter either way, but now a different
codepath rejects certain cases.
Change-Id: I9caf4f0c3b2e2e24ae25752826d47bce77e65616
Reviewed-on: https://boringssl-review.googlesource.com/31525
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 907fcb6..ae6670f 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -166,6 +166,7 @@
#include <openssl/md5.h>
#include <openssl/mem.h>
#include <openssl/rand.h>
+#include <openssl/sha.h>
#include "../crypto/internal.h"
#include "internal.h"
@@ -607,7 +608,7 @@
}
}
- if (!ssl->s3->initial_handshake_complete && ssl->session != NULL &&
+ if (!ssl->s3->initial_handshake_complete && ssl->session != nullptr &&
ssl->session->session_id_length != 0 &&
CBS_mem_equal(&session_id, ssl->session->session_id,
ssl->session->session_id_length)) {
@@ -1606,14 +1607,11 @@
}
session->ticket_lifetime_hint = ticket_lifetime_hint;
- // Generate a session ID for this session based on the session ticket. We use
- // the session ID mechanism for detecting ticket resumption. This also fits in
- // with assumptions elsewhere in OpenSSL.
- if (!EVP_Digest(CBS_data(&ticket), CBS_len(&ticket),
- session->session_id, &session->session_id_length,
- EVP_sha256(), NULL)) {
- return ssl_hs_error;
- }
+ // Generate a session ID for this session. Some callers expect all sessions to
+ // have a session ID. Additionally, it acts as the session ID to signal
+ // resumption.
+ SHA256(CBS_data(&ticket), CBS_len(&ticket), session->session_id);
+ session->session_id_length = SHA256_DIGEST_LENGTH;
if (renewed_session) {
session->not_resumable = false;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 74c4e9e..2f78032 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -4281,6 +4281,21 @@
server_ctx_.get()));
}
+// Test that ticket-based sessions on the client get fake session IDs.
+TEST_P(SSLVersionTest, FakeIDsForTickets) {
+ SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH);
+ SSL_CTX_set_session_cache_mode(server_ctx_.get(), SSL_SESS_CACHE_BOTH);
+
+ bssl::UniquePtr<SSL_SESSION> session =
+ CreateClientSession(client_ctx_.get(), server_ctx_.get());
+ ASSERT_TRUE(session);
+
+ EXPECT_TRUE(SSL_SESSION_has_ticket(session.get()));
+ unsigned session_id_length;
+ SSL_SESSION_get_id(session.get(), &session_id_length);
+ EXPECT_NE(session_id_length, 0u);
+}
+
// These tests test multi-threaded behavior. They are intended to run with
// ThreadSanitizer.
#if !defined(OPENSSL_NO_THREADS)
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 6bbaecf..4bcf603 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -7625,17 +7625,6 @@
},
})
} else {
- error := ":OLD_SESSION_VERSION_NOT_RETURNED:"
- // Clients offering TLS 1.3 will send a fake session ID
- // unrelated to the session being offer. This session ID is
- // invalid for the server to echo, so the handshake fails at
- // a different point. It's not syntactically possible for a
- // server to convince our client that it's accepted a TLS
- // 1.3 session at an older version.
- if resumeVers.version < VersionTLS13 && sessionVers.version >= VersionTLS13 {
- error = ":SERVER_ECHOED_INVALID_SESSION_ID:"
- }
-
testCases = append(testCases, testCase{
protocol: protocol,
name: "Resume-Client-Mismatch" + suffix,
@@ -7654,7 +7643,7 @@
},
expectedResumeVersion: resumeVers.version,
shouldFail: true,
- expectedError: error,
+ expectedError: ":OLD_SESSION_VERSION_NOT_RETURNED:",
flags: []string{
"-on-initial-tls13-variant", strconv.Itoa(sessionVers.tls13Variant),
"-on-resume-tls13-variant", strconv.Itoa(resumeVers.tls13Variant),
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index c1befbb..7de70b0 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -24,6 +24,7 @@
#include <openssl/digest.h>
#include <openssl/err.h>
#include <openssl/mem.h>
+#include <openssl/sha.h>
#include <openssl/stack.h>
#include "../crypto/internal.h"
@@ -910,6 +911,11 @@
}
}
+ // Generate a session ID for this session. Some callers expect all sessions to
+ // have a session ID.
+ SHA256(CBS_data(&ticket), CBS_len(&ticket), session->session_id);
+ session->session_id_length = SHA256_DIGEST_LENGTH;
+
session->ticket_age_add_valid = true;
session->not_resumable = false;
