Measure session->timeout from ticket issuance.
The distinction for full handshakes is not meaningful (the timestamp is
currently the start of the handshake), but for renewed sessions, we
currently retain the timestamp of the original issuance.
Instead, when minting or receiving tickets, adjust session->time and
session->timeout so that session->time is the ticket issuance time.
This is still not our final TLS 1.3 behavior (which will need a both
renewable and non-renewable times to honor the server ticket lifetime),
but it gets us closer and unblocks handling ticket_age_add from TLS 1.3
draft 18 and sends the correct NewSessionTicket lifetime.
This fixes the ticket lifetime hint which we emit on the server to
mirror the true ticket lifetime. It also fixes the TLS 1.3 server code
to not set the ticket lifetime hint. There is no need to waste ticket
size with it, it is no longer a "hint" in TLS 1.3, and even in the TLS
1.3 code we didn't fill it in on the server.
Change-Id: I140541f1005a24e53e1b1eaa90996d6dada1c3a1
Reviewed-on: https://boringssl-review.googlesource.com/12105
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 3ddca6f..c83299a 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3682,7 +3682,11 @@
* non-fatal certificate errors. */
long verify_result;
+ /* timeout is the lifetime of the session in seconds, measured from |time|. */
long timeout;
+
+ /* time is the time the session was issued, measured in seconds from the UNIX
+ * epoch. */
long time;
const SSL_CIPHER *cipher;
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index d5a1003..1a0cbbf 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -1904,6 +1904,9 @@
}
}
+ /* |tlsext_tick_lifetime_hint| is measured from when the ticket was issued. */
+ ssl_session_refresh_time(ssl, session);
+
if (!CBS_stow(&ticket, &session->tlsext_tick, &session->tlsext_ticklen)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
goto err;
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 14229cb..285bd84 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1827,20 +1827,36 @@
return ssl->method->write_message(ssl);
}
+ const SSL_SESSION *session;
+ SSL_SESSION *session_copy = NULL;
+ if (ssl->session == NULL) {
+ /* Fix the timeout to measure from the ticket issuance time. */
+ ssl_session_refresh_time(ssl, ssl->s3->new_session);
+ session = ssl->s3->new_session;
+ } else {
+ /* We are renewing an existing session. Duplicate the session to adjust the
+ * timeout. */
+ session_copy = SSL_SESSION_dup(ssl->session, SSL_SESSION_INCLUDE_NONAUTH);
+ if (session_copy == NULL) {
+ return -1;
+ }
+
+ ssl_session_refresh_time(ssl, session_copy);
+ session = session_copy;
+ }
+
CBB cbb, body, ticket;
- if (!ssl->method->init_message(ssl, &cbb, &body,
- SSL3_MT_NEW_SESSION_TICKET) ||
- /* Ticket lifetime hint (advisory only): We leave this unspecified for
- * resumed session (for simplicity), and guess that tickets for new
- * sessions will live as long as their sessions. */
- !CBB_add_u32(&body,
- ssl->session != NULL ? 0 : ssl->s3->new_session->timeout) ||
- !CBB_add_u16_length_prefixed(&body, &ticket) ||
- !ssl_encrypt_ticket(ssl, &ticket, ssl->session != NULL
- ? ssl->session
- : ssl->s3->new_session) ||
- !ssl->method->finish_message(ssl, &cbb)) {
- CBB_cleanup(&cbb);
+ int ok =
+ ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_NEW_SESSION_TICKET) &&
+ CBB_add_u32(&body, session->timeout) &&
+ CBB_add_u16_length_prefixed(&body, &ticket) &&
+ ssl_encrypt_ticket(ssl, &ticket, session) &&
+ ssl->method->finish_message(ssl, &cbb);
+
+ SSL_SESSION_free(session_copy);
+ CBB_cleanup(&cbb);
+
+ if (!ok) {
return -1;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 2668559..ecf2d0c 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1654,6 +1654,10 @@
OPENSSL_EXPORT SSL_SESSION *SSL_SESSION_dup(SSL_SESSION *session,
int dup_flags);
+/* ssl_session_refresh_time updates |session|'s start time to the current time,
+ * adjusting the timeout so the expiration time is unchanged. */
+void ssl_session_refresh_time(SSL *ssl, SSL_SESSION *session);
+
void ssl_cipher_preference_list_free(
struct ssl_cipher_preference_list_st *cipher_list);
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index bb7198e..2c074b7 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -285,6 +285,31 @@
return 0;
}
+void ssl_session_refresh_time(SSL *ssl, SSL_SESSION *session) {
+ struct timeval now;
+ ssl_get_current_time(ssl, &now);
+
+ /* To avoid overflows and underflows, if we've gone back in time or any value
+ * is negative, update the time, but mark the session expired. */
+ if (session->time > now.tv_sec ||
+ session->time < 0 ||
+ now.tv_sec < 0) {
+ session->time = now.tv_sec;
+ session->timeout = 0;
+ return;
+ }
+
+ /* Adjust the session time and timeout. If the session has already expired,
+ * clamp the timeout at zero. */
+ long delta = now.tv_sec - session->time;
+ session->time = now.tv_sec;
+ if (session->timeout < delta) {
+ session->timeout = 0;
+ } else {
+ session->timeout -= delta;
+ }
+}
+
int SSL_SESSION_up_ref(SSL_SESSION *session) {
CRYPTO_refcount_inc(&session->references);
return 1;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 9198a28..5eede01 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2097,6 +2097,38 @@
return encrypt ? 1 : 2;
}
+static bool GetServerTicketTime(long *out, const SSL_SESSION *session) {
+ static const uint8_t kZeros[16] = {0};
+
+ if (session->tlsext_ticklen < 16 + 16 + SHA256_DIGEST_LENGTH) {
+ return false;
+ }
+
+ const uint8_t *iv = session->tlsext_tick + 16;
+ const uint8_t *ciphertext = session->tlsext_tick + 16 + 16;
+ size_t len = session->tlsext_ticklen - 16 - 16 - SHA256_DIGEST_LENGTH;
+ std::unique_ptr<uint8_t[]> plaintext(new uint8_t[len]);
+
+ bssl::ScopedEVP_CIPHER_CTX ctx;
+ int len1, len2;
+ if (!EVP_DecryptInit_ex(ctx.get(), EVP_aes_128_cbc(), nullptr, kZeros, iv) ||
+ !EVP_DecryptUpdate(ctx.get(), plaintext.get(), &len1, ciphertext, len) ||
+ !EVP_DecryptFinal_ex(ctx.get(), plaintext.get() + len1, &len2)) {
+ return false;
+ }
+
+ len = static_cast<size_t>(len1 + len2);
+
+ bssl::UniquePtr<SSL_SESSION> server_session(
+ SSL_SESSION_from_bytes(plaintext.get(), len));
+ if (!server_session) {
+ return false;
+ }
+
+ *out = server_session->time;
+ return true;
+}
+
static bool TestSessionTimeout() {
bssl::UniquePtr<X509> cert = GetTestCertificate();
bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
@@ -2192,6 +2224,25 @@
return false;
}
+ // Check the sessions have timestamps measured from issuance.
+ long session_time = 0;
+ if (server_test) {
+ if (!GetServerTicketTime(&session_time, new_session.get())) {
+ fprintf(stderr, "Failed to decode session ticket (version = %04x).\n",
+ version);
+ return false;
+ }
+ } else {
+ session_time = new_session->time;
+ }
+
+ if (session_time != g_current_time.tv_sec) {
+ fprintf(stderr,
+ "New session is not measured from issuance (version = %04x).\n",
+ version);
+ return false;
+ }
+
// The new session is usable just before the old expiration.
g_current_time.tv_sec = kStartTime + SSL_DEFAULT_SESSION_TIMEOUT - 1;
if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(),
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index 409dc24..fd60bd3 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -709,6 +709,8 @@
return 0;
}
+ ssl_session_refresh_time(ssl, session);
+
CBS cbs, ke_modes, auth_modes, ticket, extensions;
CBS_init(&cbs, ssl->init_msg, ssl->init_num);
if (!CBS_get_u32(&cbs, &session->tlsext_tick_lifetime_hint) ||
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index f6c70c6..2b82401 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -597,6 +597,10 @@
}
ssl->method->received_flight(ssl);
+
+ /* Refresh the session timestamp so that it is measured from ticket
+ * issuance. */
+ ssl_session_refresh_time(ssl, ssl->s3->new_session);
hs->state = state_send_new_session_ticket;
return ssl_hs_ok;
}
@@ -608,7 +612,6 @@
static enum ssl_hs_wait_t do_send_new_session_ticket(SSL *ssl,
SSL_HANDSHAKE *hs) {
SSL_SESSION *session = ssl->s3->new_session;
- session->tlsext_tick_lifetime_hint = session->timeout;
/* TODO(svaldez): Add support for sending 0RTT through TicketEarlyDataInfo
* extension. */
@@ -616,7 +619,7 @@
CBB cbb, body, ke_modes, auth_modes, ticket, extensions;
if (!ssl->method->init_message(ssl, &cbb, &body,
SSL3_MT_NEW_SESSION_TICKET) ||
- !CBB_add_u32(&body, session->tlsext_tick_lifetime_hint) ||
+ !CBB_add_u32(&body, session->timeout) ||
!CBB_add_u8_length_prefixed(&body, &ke_modes) ||
!CBB_add_u8(&ke_modes, SSL_PSK_DHE_KE) ||
!CBB_add_u8_length_prefixed(&body, &auth_modes) ||
