Rotate the default ticket encryption key.
The ticket encryption key is rotated automatically once every 24 hours,
unless a key has been configured manually (i.e. using
|SSL_CTX_set_tlsext_ticket_keys|) or one of the custom ticket encryption
methods is used.
Change-Id: I0dfff28b33e58e96b3bbf7f94dcd6d2642f37aec
Reviewed-on: https://boringssl-review.googlesource.com/18924
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/crypto/internal.h b/crypto/internal.h
index 28ec3ee..83da68e 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -113,6 +113,7 @@
#include <openssl/stack.h>
#include <openssl/thread.h>
+#include <assert.h>
#include <string.h>
#if defined(_MSC_VER)
@@ -462,6 +463,42 @@
OPENSSL_EXPORT void CRYPTO_STATIC_MUTEX_unlock_write(
struct CRYPTO_STATIC_MUTEX *lock);
+#if defined(__cplusplus)
+extern "C++" {
+
+namespace bssl {
+
+namespace internal {
+
+/* MutexLockBase is a RAII helper for CRYPTO_MUTEX locking. */
+template <void (*LockFunc)(CRYPTO_MUTEX *), void (*ReleaseFunc)(CRYPTO_MUTEX *)>
+class MutexLockBase {
+ public:
+ explicit MutexLockBase(CRYPTO_MUTEX *mu) : mu_(mu) {
+ assert(mu_ != nullptr);
+ LockFunc(mu_);
+ }
+ ~MutexLockBase() { ReleaseFunc(mu_); }
+ MutexLockBase(const MutexLockBase<LockFunc, ReleaseFunc> &) = delete;
+ MutexLockBase &operator=(const MutexLockBase<LockFunc, ReleaseFunc> &) =
+ delete;
+
+ private:
+ CRYPTO_MUTEX *const mu_;
+};
+
+} // namespace internal
+
+using MutexWriteLock =
+ internal::MutexLockBase<CRYPTO_MUTEX_lock_write, CRYPTO_MUTEX_unlock_write>;
+using MutexReadLock =
+ internal::MutexLockBase<CRYPTO_MUTEX_lock_read, CRYPTO_MUTEX_unlock_read>;
+
+} // namespace bssl
+
+} // extern "C++"
+#endif // defined(__cplusplus)
+
/* Thread local storage. */
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 9a414ca..3168a81 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1975,10 +1975,14 @@
* An attacker that compromises a server's session ticket key can impersonate
* the server and, prior to TLS 1.3, retroactively decrypt all application
* traffic from sessions using that ticket key. Thus ticket keys must be
- * regularly rotated for forward secrecy. Note the default key is currently not
- * rotated.
- *
- * TODO(davidben): This is silly. Rotate the default key automatically. */
+ * regularly rotated for forward secrecy. Note the default key is rotated
+ * automatically once every 48 hours but manually configured keys are not. */
+
+/* SSL_DEFAULT_TICKET_KEY_ROTATION_INTERVAL is the interval with which the
+ * default session ticket encryption key is rotated, if in use. If any
+ * non-default ticket encryption mechanism is configured, automatic rotation is
+ * disabled. */
+#define SSL_DEFAULT_TICKET_KEY_ROTATION_INTERVAL (2 * 24 * 60 * 60)
/* SSL_CTX_get_tlsext_ticket_keys writes |ctx|'s session ticket key material to
* |len| bytes of |out|. It returns one on success and zero if |len| is not
@@ -3134,7 +3138,8 @@
/* SSL_CTX_set_current_time_cb configures a callback to retrieve the current
* time, which should be set in |*out_clock|. This can be used for testing
* purposes; for example, a callback can be configured that returns a time
- * set explicitly by the test. */
+ * set explicitly by the test. The |ssl| pointer passed to |cb| is always null.
+ */
OPENSSL_EXPORT void SSL_CTX_set_current_time_cb(
SSL_CTX *ctx, void (*cb)(const SSL *ssl, struct timeval *out_clock));
@@ -4217,6 +4222,17 @@
uint8_t *in_group_flags;
};
+struct tlsext_ticket_key {
+ uint8_t name[SSL_TICKET_KEY_NAME_LEN];
+ uint8_t hmac_key[16];
+ uint8_t aes_key[16];
+ /* next_rotation_tv_sec is the time (in seconds from the epoch) when the
+ * current key should be superseded by a new key, or the time when a previous
+ * key should be dropped. If zero, then the key should not be automatically
+ * rotated. */
+ uint64_t next_rotation_tv_sec;
+};
+
/* ssl_ctx_st (aka |SSL_CTX|) contains configuration common to several SSL
* connections. */
struct ssl_ctx_st {
@@ -4359,10 +4375,14 @@
/* TLS extensions servername callback */
int (*tlsext_servername_callback)(SSL *, int *, void *);
void *tlsext_servername_arg;
- /* RFC 4507 session ticket keys */
- uint8_t tlsext_tick_key_name[SSL_TICKET_KEY_NAME_LEN];
- uint8_t tlsext_tick_hmac_key[16];
- uint8_t tlsext_tick_aes_key[16];
+
+ /* RFC 4507 session ticket keys. |tlsext_ticket_key_current| may be NULL
+ * before the first handshake and |tlsext_ticket_key_prev| may be NULL at any
+ * time. Automatically generated ticket keys are rotated as needed at
+ * handshake time. Hence, all access must be synchronized through |lock|. */
+ struct tlsext_ticket_key *tlsext_ticket_key_current;
+ struct tlsext_ticket_key *tlsext_ticket_key_prev;
+
/* Callback to support customisation of ticket key setting */
int (*tlsext_ticket_key_cb)(SSL *ssl, uint8_t *name, uint8_t *iv,
EVP_CIPHER_CTX *ectx, HMAC_CTX *hctx, int enc);
@@ -4433,8 +4453,8 @@
void (*keylog_callback)(const SSL *ssl, const char *line);
/* current_time_cb, if not NULL, is the function to use to get the current
- * time. It sets |*out_clock| to the current time. See
- * |SSL_CTX_set_current_time_cb|. */
+ * time. It sets |*out_clock| to the current time. The |ssl| argument is
+ * always NULL. See |SSL_CTX_set_current_time_cb|. */
void (*current_time_cb)(const SSL *ssl, struct timeval *out_clock);
/* pool is used for all |CRYPTO_BUFFER|s in case we wish to share certificate
@@ -4633,6 +4653,7 @@
BORINGSSL_MAKE_DELETER(SSL, SSL_free)
BORINGSSL_MAKE_DELETER(SSL_CTX, SSL_CTX_free)
BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free)
+BORINGSSL_MAKE_DELETER(tlsext_ticket_key, OPENSSL_free);
enum class OpenRecordResult {
kOK,
diff --git a/ssl/internal.h b/ssl/internal.h
index 534a1ce..ea883e5 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2066,6 +2066,7 @@
int ssl_cert_check_private_key(const CERT *cert, const EVP_PKEY *privkey);
int ssl_get_new_session(SSL_HANDSHAKE *hs, int is_server);
int ssl_encrypt_ticket(SSL *ssl, CBB *out, const SSL_SESSION *session);
+int ssl_ctx_rotate_ticket_encryption_key(SSL_CTX *ctx);
/* ssl_session_new returns a newly-allocated blank |SSL_SESSION| or nullptr on
* error. */
@@ -2328,6 +2329,8 @@
int ssl_can_read(const SSL *ssl);
void ssl_get_current_time(const SSL *ssl, struct OPENSSL_timeval *out_clock);
+void ssl_ctx_get_current_time(const SSL_CTX *ctx,
+ struct OPENSSL_timeval *out_clock);
/* ssl_reset_error_state resets state for |SSL_get_error|. */
void ssl_reset_error_state(SSL *ssl);
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 10128d8..026e218 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -357,11 +357,18 @@
}
void ssl_get_current_time(const SSL *ssl, struct OPENSSL_timeval *out_clock) {
- if (ssl->ctx->current_time_cb != NULL) {
+ /* TODO(martinkr): Change callers to |ssl_ctx_get_current_time| and drop the
+ * |ssl| arg from |current_time_cb| if possible. */
+ ssl_ctx_get_current_time(ssl->ctx, out_clock);
+}
+
+void ssl_ctx_get_current_time(const SSL_CTX *ctx,
+ struct OPENSSL_timeval *out_clock) {
+ if (ctx->current_time_cb != NULL) {
/* TODO(davidben): Update current_time_cb to use OPENSSL_timeval. See
* https://crbug.com/boringssl/155. */
struct timeval clock;
- ssl->ctx->current_time_cb(ssl, &clock);
+ ctx->current_time_cb(nullptr /* ssl */, &clock);
if (clock.tv_sec < 0) {
assert(0);
out_clock->tv_sec = 0;
@@ -503,13 +510,6 @@
ret->max_send_fragment = SSL3_RT_MAX_PLAIN_LENGTH;
- /* Setup RFC4507 ticket keys */
- if (!RAND_bytes(ret->tlsext_tick_key_name, 16) ||
- !RAND_bytes(ret->tlsext_tick_hmac_key, 16) ||
- !RAND_bytes(ret->tlsext_tick_aes_key, 16)) {
- ret->options |= SSL_OP_NO_TICKET;
- }
-
/* Disable the auto-chaining feature by default. Once this has stuck without
* problems, the feature will be removed entirely. */
ret->mode = SSL_MODE_NO_AUTO_CHAIN;
@@ -571,6 +571,8 @@
OPENSSL_free(ctx->alpn_client_proto_list);
EVP_PKEY_free(ctx->tlsext_channel_id_private);
OPENSSL_free(ctx->verify_sigalgs);
+ OPENSSL_free(ctx->tlsext_ticket_key_current);
+ OPENSSL_free(ctx->tlsext_ticket_key_prev);
OPENSSL_free(ctx);
}
@@ -1587,10 +1589,18 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_TICKET_KEYS_LENGTH);
return 0;
}
+
+ /* The default ticket keys are initialized lazily. Trigger a key
+ * rotation to initialize them. */
+ if (!ssl_ctx_rotate_ticket_encryption_key(ctx)) {
+ return 0;
+ }
+
uint8_t *out_bytes = reinterpret_cast<uint8_t *>(out);
- OPENSSL_memcpy(out_bytes, ctx->tlsext_tick_key_name, 16);
- OPENSSL_memcpy(out_bytes + 16, ctx->tlsext_tick_hmac_key, 16);
- OPENSSL_memcpy(out_bytes + 32, ctx->tlsext_tick_aes_key, 16);
+ MutexReadLock lock(&ctx->lock);
+ OPENSSL_memcpy(out_bytes, ctx->tlsext_ticket_key_current->name, 16);
+ OPENSSL_memcpy(out_bytes + 16, ctx->tlsext_ticket_key_current->hmac_key, 16);
+ OPENSSL_memcpy(out_bytes + 32, ctx->tlsext_ticket_key_current->aes_key, 16);
return 1;
}
@@ -1602,10 +1612,22 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_TICKET_KEYS_LENGTH);
return 0;
}
+ if (!ctx->tlsext_ticket_key_current) {
+ ctx->tlsext_ticket_key_current =
+ (tlsext_ticket_key *)OPENSSL_malloc(sizeof(tlsext_ticket_key));
+ if (!ctx->tlsext_ticket_key_current) {
+ return 0;
+ }
+ }
+ OPENSSL_memset(ctx->tlsext_ticket_key_current, 0, sizeof(tlsext_ticket_key));
const uint8_t *in_bytes = reinterpret_cast<const uint8_t *>(in);
- OPENSSL_memcpy(ctx->tlsext_tick_key_name, in_bytes, 16);
- OPENSSL_memcpy(ctx->tlsext_tick_hmac_key, in_bytes + 16, 16);
- OPENSSL_memcpy(ctx->tlsext_tick_aes_key, in_bytes + 32, 16);
+ OPENSSL_memcpy(ctx->tlsext_ticket_key_current->name, in_bytes, 16);
+ OPENSSL_memcpy(ctx->tlsext_ticket_key_current->hmac_key, in_bytes + 16, 16);
+ OPENSSL_memcpy(ctx->tlsext_ticket_key_current->aes_key, in_bytes + 32, 16);
+ OPENSSL_free(ctx->tlsext_ticket_key_prev);
+ ctx->tlsext_ticket_key_prev = nullptr;
+ /* Disable automatic key rotation. */
+ ctx->tlsext_ticket_key_current->next_rotation_tv_sec = 0;
return 1;
}
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index a1c21dc..dad0656 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -437,6 +437,59 @@
return 1;
}
+int ssl_ctx_rotate_ticket_encryption_key(SSL_CTX *ctx) {
+ OPENSSL_timeval now;
+ ssl_ctx_get_current_time(ctx, &now);
+ {
+ /* Avoid acquiring a write lock in the common case (i.e. a non-default key
+ * is used or the default keys have not expired yet). */
+ MutexReadLock lock(&ctx->lock);
+ if (ctx->tlsext_ticket_key_current &&
+ (ctx->tlsext_ticket_key_current->next_rotation_tv_sec == 0 ||
+ ctx->tlsext_ticket_key_current->next_rotation_tv_sec > now.tv_sec) &&
+ (!ctx->tlsext_ticket_key_prev ||
+ ctx->tlsext_ticket_key_prev->next_rotation_tv_sec > now.tv_sec)) {
+ return 1;
+ }
+ }
+
+ MutexWriteLock lock(&ctx->lock);
+ if (!ctx->tlsext_ticket_key_current ||
+ (ctx->tlsext_ticket_key_current->next_rotation_tv_sec != 0 &&
+ ctx->tlsext_ticket_key_current->next_rotation_tv_sec <= now.tv_sec)) {
+ /* The current key has not been initialized or it is expired. */
+ auto new_key = bssl::MakeUnique<struct tlsext_ticket_key>();
+ if (!new_key) {
+ return 0;
+ }
+ OPENSSL_memset(new_key.get(), 0, sizeof(struct tlsext_ticket_key));
+ if (ctx->tlsext_ticket_key_current) {
+ /* The current key expired. Rotate it to prev and bump up its rotation
+ * timestamp. Note that even with the new rotation time it may still be
+ * expired and get droppped below. */
+ ctx->tlsext_ticket_key_current->next_rotation_tv_sec +=
+ SSL_DEFAULT_TICKET_KEY_ROTATION_INTERVAL;
+ OPENSSL_free(ctx->tlsext_ticket_key_prev);
+ ctx->tlsext_ticket_key_prev = ctx->tlsext_ticket_key_current;
+ }
+ ctx->tlsext_ticket_key_current = new_key.release();
+ RAND_bytes(ctx->tlsext_ticket_key_current->name, 16);
+ RAND_bytes(ctx->tlsext_ticket_key_current->hmac_key, 16);
+ RAND_bytes(ctx->tlsext_ticket_key_current->aes_key, 16);
+ ctx->tlsext_ticket_key_current->next_rotation_tv_sec =
+ now.tv_sec + SSL_DEFAULT_TICKET_KEY_ROTATION_INTERVAL;
+ }
+
+ /* Drop an expired prev key. */
+ if (ctx->tlsext_ticket_key_prev &&
+ ctx->tlsext_ticket_key_prev->next_rotation_tv_sec <= now.tv_sec) {
+ OPENSSL_free(ctx->tlsext_ticket_key_prev);
+ ctx->tlsext_ticket_key_prev = nullptr;
+ }
+
+ return 1;
+}
+
static int ssl_encrypt_ticket_with_cipher_ctx(SSL *ssl, CBB *out,
const uint8_t *session_buf,
size_t session_len) {
@@ -464,14 +517,19 @@
return 0;
}
} else {
+ /* Rotate ticket key if necessary. */
+ if (!ssl_ctx_rotate_ticket_encryption_key(tctx)) {
+ return 0;
+ }
+ MutexReadLock lock(&tctx->lock);
if (!RAND_bytes(iv, 16) ||
!EVP_EncryptInit_ex(ctx.get(), EVP_aes_128_cbc(), NULL,
- tctx->tlsext_tick_aes_key, iv) ||
- !HMAC_Init_ex(hctx.get(), tctx->tlsext_tick_hmac_key, 16,
+ tctx->tlsext_ticket_key_current->aes_key, iv) ||
+ !HMAC_Init_ex(hctx.get(), tctx->tlsext_ticket_key_current->hmac_key, 16,
tlsext_tick_md(), NULL)) {
return 0;
}
- OPENSSL_memcpy(key_name, tctx->tlsext_tick_key_name, 16);
+ OPENSSL_memcpy(key_name, tctx->tlsext_ticket_key_current->name, 16);
}
uint8_t *ptr;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 898cd04..135b37d 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -49,6 +49,31 @@
#endif
+namespace bssl {
+
+namespace {
+
+struct VersionParam {
+ uint16_t version;
+ enum { is_tls, is_dtls } ssl_method;
+ const char name[8];
+};
+
+static const size_t kTicketKeyLen = 48;
+
+static const VersionParam kAllVersions[] = {
+ {SSL3_VERSION, VersionParam::is_tls, "SSL3"},
+ {TLS1_VERSION, VersionParam::is_tls, "TLS1"},
+ {TLS1_1_VERSION, VersionParam::is_tls, "TLS1_1"},
+ {TLS1_2_VERSION, VersionParam::is_tls, "TLS1_2"},
+// TLS 1.3 requires RSA-PSS, which is disabled for Android system builds.
+#if !defined(BORINGSSL_ANDROID_SYSTEM)
+ {TLS1_3_VERSION, VersionParam::is_tls, "TLS1_3"},
+#endif
+ {DTLS1_VERSION, VersionParam::is_dtls, "DTLS1"},
+ {DTLS1_2_VERSION, VersionParam::is_dtls, "DTLS1_2"},
+};
+
struct ExpectedCipher {
unsigned long id;
int in_group_flag;
@@ -2232,6 +2257,23 @@
return std::move(g_last_session);
}
+static bool ExpectTicketKeyChanged(SSL_CTX *ctx, uint8_t *inout_key,
+ bool changed) {
+ uint8_t new_key[kTicketKeyLen];
+ int res = SSL_CTX_get_tlsext_ticket_keys(ctx, new_key, kTicketKeyLen) == 1;
+ if (res != 1) { /* May return 0, 1 or 48. */
+ fprintf(stderr, "SSL_CTX_get_tlsext_ticket_keys() returned %d.\n", res);
+ return false;
+ }
+ if (changed != !!OPENSSL_memcmp(inout_key, new_key, kTicketKeyLen)) {
+ fprintf(stderr, "Ticket key unexpectedly %s.\n",
+ changed ? "did not change" : "changed");
+ return false;
+ }
+ OPENSSL_memcpy(inout_key, new_key, kTicketKeyLen);
+ return true;
+}
+
static int SwitchSessionIDContextSNI(SSL *ssl, int *out_alert, void *arg) {
static const uint8_t kContext[] = {3};
@@ -2602,6 +2644,126 @@
return true;
}
+class DefaultSessionTicketKeyTest
+ : public ::testing::TestWithParam<VersionParam> {
+ public:
+ bssl::UniquePtr<SSL_CTX> CreateContext() const {
+ const VersionParam version = GetParam();
+ const SSL_METHOD *method = version.ssl_method == VersionParam::is_dtls
+ ? DTLS_method()
+ : TLS_method();
+ bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(method));
+ if (!ctx ||
+ !SSL_CTX_set_min_proto_version(ctx.get(), version.version) ||
+ !SSL_CTX_set_max_proto_version(ctx.get(), version.version)) {
+ return nullptr;
+ }
+ return ctx;
+ }
+};
+
+
+TEST_P(DefaultSessionTicketKeyTest, Initialization) {
+ bssl::UniquePtr<X509> cert = GetTestCertificate();
+ ASSERT_TRUE(cert);
+ bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+ ASSERT_TRUE(key);
+
+ bssl::UniquePtr<SSL_CTX> server_ctx = CreateContext();
+ ASSERT_TRUE(server_ctx);
+ static const uint8_t kZeroKey[kTicketKeyLen] = {};
+ uint8_t ticket_key[kTicketKeyLen];
+ ASSERT_EQ(1, SSL_CTX_get_tlsext_ticket_keys(server_ctx.get(), ticket_key,
+ kTicketKeyLen));
+ ASSERT_NE(0, OPENSSL_memcmp(ticket_key, kZeroKey, kTicketKeyLen));
+}
+
+TEST_P(DefaultSessionTicketKeyTest, Rotation) {
+ if (GetParam().version == SSL3_VERSION) {
+ return;
+ }
+
+ static const time_t kStartTime = 1001;
+ g_current_time.tv_sec = kStartTime;
+ uint8_t ticket_key[kTicketKeyLen];
+
+ bssl::UniquePtr<X509> cert = GetTestCertificate();
+ ASSERT_TRUE(cert);
+ bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+ ASSERT_TRUE(key);
+
+ bssl::UniquePtr<SSL_CTX> server_ctx = CreateContext();
+ ASSERT_TRUE(server_ctx);
+ bssl::UniquePtr<SSL_CTX> client_ctx = CreateContext();
+ ASSERT_TRUE(client_ctx);
+
+ /* We use session reuse as a proxy for ticket decryption success, hence
+ * disable session timeouts. */
+ SSL_CTX_set_timeout(server_ctx.get(), std::numeric_limits<uint32_t>::max());
+ SSL_CTX_set_session_psk_dhe_timeout(server_ctx.get(),
+ std::numeric_limits<uint32_t>::max());
+
+ SSL_CTX_set_current_time_cb(client_ctx.get(), FrozenTimeCallback);
+ SSL_CTX_set_current_time_cb(server_ctx.get(), CurrentTimeCallback);
+
+ 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_OFF);
+
+ ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
+ ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
+
+ /* Initialize ticket_key with the current key. */
+ ASSERT_TRUE(
+ ExpectTicketKeyChanged(server_ctx.get(), ticket_key, true /* changed */));
+
+ /* Verify ticket resumption actually works. */
+ bssl::UniquePtr<SSL> client, server;
+ bssl::UniquePtr<SSL_SESSION> session =
+ CreateClientSession(client_ctx.get(), server_ctx.get());
+ ASSERT_TRUE(session);
+ EXPECT_TRUE(ExpectSessionReused(client_ctx.get(), server_ctx.get(),
+ session.get(), true /* reused */));
+
+ /* Advance time to just before key rotation. */
+ g_current_time.tv_sec += SSL_DEFAULT_TICKET_KEY_ROTATION_INTERVAL - 1;
+ EXPECT_TRUE(ExpectSessionReused(client_ctx.get(), server_ctx.get(),
+ session.get(), true /* reused */));
+ ASSERT_TRUE(ExpectTicketKeyChanged(server_ctx.get(), ticket_key,
+ false /* NOT changed */));
+
+ /* Force key rotation. */
+ g_current_time.tv_sec += 1;
+ bssl::UniquePtr<SSL_SESSION> new_session =
+ CreateClientSession(client_ctx.get(), server_ctx.get());
+ ASSERT_TRUE(
+ ExpectTicketKeyChanged(server_ctx.get(), ticket_key, true /* changed */));
+
+ /* Resumption with both old and new ticket should work. */
+ EXPECT_TRUE(ExpectSessionReused(client_ctx.get(), server_ctx.get(),
+ session.get(), true /* reused */));
+ EXPECT_TRUE(ExpectSessionReused(client_ctx.get(), server_ctx.get(),
+ new_session.get(), true /* reused */));
+ ASSERT_TRUE(ExpectTicketKeyChanged(server_ctx.get(), ticket_key,
+ false /* NOT changed */));
+
+ /* Force key rotation again. Resumption with the old ticket now fails. */
+ g_current_time.tv_sec += SSL_DEFAULT_TICKET_KEY_ROTATION_INTERVAL;
+ EXPECT_TRUE(ExpectSessionReused(client_ctx.get(), server_ctx.get(),
+ session.get(), false /* NOT reused */));
+ ASSERT_TRUE(
+ ExpectTicketKeyChanged(server_ctx.get(), ticket_key, true /* changed */));
+
+ /* But resumption with the newer session still works. */
+ EXPECT_TRUE(ExpectSessionReused(client_ctx.get(), server_ctx.get(),
+ new_session.get(), true /* reused */));
+}
+
+INSTANTIATE_TEST_CASE_P(WithTLSVersion, DefaultSessionTicketKeyTest,
+ testing::ValuesIn(kAllVersions),
+ [](const testing::TestParamInfo<VersionParam> &i) {
+ return i.param.name;
+ });
+
static int SwitchContext(SSL *ssl, int *out_alert, void *arg) {
SSL_CTX *ctx = reinterpret_cast<SSL_CTX*>(arg);
SSL_set_SSL_CTX(ssl, ctx);
@@ -3313,39 +3475,21 @@
return true;
}
-
static bool ForEachVersion(bool (*test_func)(bool is_dtls,
const SSL_METHOD *method,
uint16_t version)) {
- static uint16_t kTLSVersions[] = {
- SSL3_VERSION,
- TLS1_VERSION,
- TLS1_1_VERSION,
- TLS1_2_VERSION,
-// TLS 1.3 requires RSA-PSS, which is disabled for Android system builds.
-#if !defined(BORINGSSL_ANDROID_SYSTEM)
- TLS1_3_VERSION,
-#endif
- };
-
- static uint16_t kDTLSVersions[] = {
- DTLS1_VERSION, DTLS1_2_VERSION,
- };
-
- for (uint16_t version : kTLSVersions) {
- if (!test_func(false, TLS_method(), version)) {
- fprintf(stderr, "Test failed at TLS version %04x.\n", version);
+ for (auto version : kAllVersions) {
+ const bool is_dtls = version.ssl_method == VersionParam::is_dtls;
+ const SSL_METHOD *method = is_dtls ? DTLS_method() : TLS_method();
+ if (!test_func(is_dtls, method, version.version)) {
+ if (is_dtls) {
+ fprintf(stderr, "Test failed at DTLS version %04x.\n", version.version);
+ } else {
+ fprintf(stderr, "Test failed at TLS version %04x.\n", version.version);
+ }
return false;
}
}
-
- for (uint16_t version : kDTLSVersions) {
- if (!test_func(true, DTLS_method(), version)) {
- fprintf(stderr, "Test failed at DTLS version %04x.\n", version);
- return false;
- }
- }
-
return true;
}
@@ -4064,3 +4208,6 @@
ADD_FAILURE() << "Tests failed";
}
}
+
+} // namespace
+} // namespace bssl
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 1dd834b..bbe6401 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -3004,60 +3004,20 @@
return 1;
}
-static enum ssl_ticket_aead_result_t
-ssl_decrypt_ticket_with_cipher_ctx(SSL *ssl, uint8_t **out, size_t *out_len,
- int *out_renew_ticket, const uint8_t *ticket,
- size_t ticket_len) {
- const SSL_CTX *const ssl_ctx = ssl->session_ctx;
-
- ScopedHMAC_CTX hmac_ctx;
- ScopedEVP_CIPHER_CTX cipher_ctx;
-
- /* Ensure there is room for the key name and the largest IV
- * |tlsext_ticket_key_cb| may try to consume. The real limit may be lower, but
- * the maximum IV length should be well under the minimum size for the
- * session material and HMAC. */
- if (ticket_len < SSL_TICKET_KEY_NAME_LEN + EVP_MAX_IV_LENGTH) {
- return ssl_ticket_aead_ignore_ticket;
- }
- const uint8_t *iv = ticket + SSL_TICKET_KEY_NAME_LEN;
-
- if (ssl_ctx->tlsext_ticket_key_cb != NULL) {
- int cb_ret = ssl_ctx->tlsext_ticket_key_cb(
- ssl, (uint8_t *)ticket /* name */, (uint8_t *)iv, cipher_ctx.get(),
- hmac_ctx.get(), 0 /* decrypt */);
- if (cb_ret < 0) {
- return ssl_ticket_aead_error;
- } else if (cb_ret == 0) {
- return ssl_ticket_aead_ignore_ticket;
- } else if (cb_ret == 2) {
- *out_renew_ticket = 1;
- }
- } else {
- /* Check the key name matches. */
- if (OPENSSL_memcmp(ticket, ssl_ctx->tlsext_tick_key_name,
- SSL_TICKET_KEY_NAME_LEN) != 0) {
- return ssl_ticket_aead_ignore_ticket;
- }
- if (!HMAC_Init_ex(hmac_ctx.get(), ssl_ctx->tlsext_tick_hmac_key,
- sizeof(ssl_ctx->tlsext_tick_hmac_key), tlsext_tick_md(),
- NULL) ||
- !EVP_DecryptInit_ex(cipher_ctx.get(), EVP_aes_128_cbc(), NULL,
- ssl_ctx->tlsext_tick_aes_key, iv)) {
- return ssl_ticket_aead_error;
- }
- }
- size_t iv_len = EVP_CIPHER_CTX_iv_length(cipher_ctx.get());
+static enum ssl_ticket_aead_result_t decrypt_ticket_with_cipher_ctx(
+ uint8_t **out, size_t *out_len, EVP_CIPHER_CTX *cipher_ctx,
+ HMAC_CTX *hmac_ctx, const uint8_t *ticket, size_t ticket_len) {
+ size_t iv_len = EVP_CIPHER_CTX_iv_length(cipher_ctx);
/* Check the MAC at the end of the ticket. */
uint8_t mac[EVP_MAX_MD_SIZE];
- size_t mac_len = HMAC_size(hmac_ctx.get());
+ size_t mac_len = HMAC_size(hmac_ctx);
if (ticket_len < SSL_TICKET_KEY_NAME_LEN + iv_len + 1 + mac_len) {
/* The ticket must be large enough for key name, IV, data, and MAC. */
return ssl_ticket_aead_ignore_ticket;
}
- HMAC_Update(hmac_ctx.get(), ticket, ticket_len - mac_len);
- HMAC_Final(hmac_ctx.get(), mac, NULL);
+ HMAC_Update(hmac_ctx, ticket, ticket_len - mac_len);
+ HMAC_Final(hmac_ctx, mac, NULL);
int mac_ok =
CRYPTO_memcmp(mac, ticket + (ticket_len - mac_len), mac_len) == 0;
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
@@ -3084,9 +3044,9 @@
return ssl_ticket_aead_ignore_ticket;
}
int len1, len2;
- if (!EVP_DecryptUpdate(cipher_ctx.get(), plaintext.get(), &len1, ciphertext,
+ if (!EVP_DecryptUpdate(cipher_ctx, plaintext.get(), &len1, ciphertext,
(int)ciphertext_len) ||
- !EVP_DecryptFinal_ex(cipher_ctx.get(), plaintext.get() + len1, &len2)) {
+ !EVP_DecryptFinal_ex(cipher_ctx, plaintext.get() + len1, &len2)) {
ERR_clear_error();
return ssl_ticket_aead_ignore_ticket;
}
@@ -3098,6 +3058,69 @@
return ssl_ticket_aead_success;
}
+static enum ssl_ticket_aead_result_t ssl_decrypt_ticket_with_cb(
+ SSL *ssl, uint8_t **out, size_t *out_len, int *out_renew_ticket,
+ const uint8_t *ticket, size_t ticket_len) {
+ assert(ticket_len >= SSL_TICKET_KEY_NAME_LEN + EVP_MAX_IV_LENGTH);
+ ScopedEVP_CIPHER_CTX cipher_ctx;
+ ScopedHMAC_CTX hmac_ctx;
+ const uint8_t *iv = ticket + SSL_TICKET_KEY_NAME_LEN;
+ int cb_ret = ssl->session_ctx->tlsext_ticket_key_cb(
+ ssl, (uint8_t *)ticket /* name */, (uint8_t *)iv, cipher_ctx.get(),
+ hmac_ctx.get(), 0 /* decrypt */);
+ if (cb_ret < 0) {
+ return ssl_ticket_aead_error;
+ } else if (cb_ret == 0) {
+ return ssl_ticket_aead_ignore_ticket;
+ } else if (cb_ret == 2) {
+ *out_renew_ticket = 1;
+ } else {
+ assert(cb_ret == 1);
+ }
+ return decrypt_ticket_with_cipher_ctx(out, out_len, cipher_ctx.get(),
+ hmac_ctx.get(), ticket, ticket_len);
+}
+
+static enum ssl_ticket_aead_result_t ssl_decrypt_ticket_with_ticket_keys(
+ SSL *ssl, uint8_t **out, size_t *out_len, const uint8_t *ticket,
+ size_t ticket_len) {
+ assert(ticket_len >= SSL_TICKET_KEY_NAME_LEN + EVP_MAX_IV_LENGTH);
+ SSL_CTX *ctx = ssl->session_ctx;
+
+ /* Rotate the ticket key if necessary. */
+ if (!ssl_ctx_rotate_ticket_encryption_key(ctx)) {
+ return ssl_ticket_aead_error;
+ }
+
+ /* Pick the matching ticket key and decrypt. */
+ ScopedEVP_CIPHER_CTX cipher_ctx;
+ ScopedHMAC_CTX hmac_ctx;
+ {
+ MutexReadLock lock(&ctx->lock);
+ const tlsext_ticket_key *key;
+ if (ctx->tlsext_ticket_key_current &&
+ !OPENSSL_memcmp(ctx->tlsext_ticket_key_current->name, ticket,
+ SSL_TICKET_KEY_NAME_LEN)) {
+ key = ctx->tlsext_ticket_key_current;
+ } else if (ctx->tlsext_ticket_key_prev &&
+ !OPENSSL_memcmp(ctx->tlsext_ticket_key_prev->name, ticket,
+ SSL_TICKET_KEY_NAME_LEN)) {
+ key = ctx->tlsext_ticket_key_prev;
+ } else {
+ return ssl_ticket_aead_ignore_ticket;
+ }
+ const uint8_t *iv = ticket + SSL_TICKET_KEY_NAME_LEN;
+ if (!HMAC_Init_ex(hmac_ctx.get(), key->hmac_key, sizeof(key->hmac_key),
+ tlsext_tick_md(), NULL) ||
+ !EVP_DecryptInit_ex(cipher_ctx.get(), EVP_aes_128_cbc(), NULL,
+ key->aes_key, iv)) {
+ return ssl_ticket_aead_error;
+ }
+ }
+ return decrypt_ticket_with_cipher_ctx(out, out_len, cipher_ctx.get(),
+ hmac_ctx.get(), ticket, ticket_len);
+}
+
static enum ssl_ticket_aead_result_t ssl_decrypt_ticket_with_method(
SSL *ssl, uint8_t **out, size_t *out_len, int *out_renew_ticket,
const uint8_t *ticket, size_t ticket_len) {
@@ -3141,8 +3164,20 @@
result = ssl_decrypt_ticket_with_method(
ssl, &plaintext, &plaintext_len, out_renew_ticket, ticket, ticket_len);
} else {
- result = ssl_decrypt_ticket_with_cipher_ctx(
- ssl, &plaintext, &plaintext_len, out_renew_ticket, ticket, ticket_len);
+ /* Ensure there is room for the key name and the largest IV
+ * |tlsext_ticket_key_cb| may try to consume. The real limit may be lower,
+ * but the maximum IV length should be well under the minimum size for the
+ * session material and HMAC. */
+ if (ticket_len < SSL_TICKET_KEY_NAME_LEN + EVP_MAX_IV_LENGTH) {
+ return ssl_ticket_aead_ignore_ticket;
+ }
+ if (ssl->session_ctx->tlsext_ticket_key_cb != NULL) {
+ result = ssl_decrypt_ticket_with_cb(ssl, &plaintext, &plaintext_len,
+ out_renew_ticket, ticket, ticket_len);
+ } else {
+ result = ssl_decrypt_ticket_with_ticket_keys(
+ ssl, &plaintext, &plaintext_len, ticket, ticket_len);
+ }
}
if (result != ssl_ticket_aead_success) {
