diff --git a/crypto/internal.h b/crypto/internal.h index 28ec3eeb..83da68e7 100644 --- a/crypto/internal.h +++ b/crypto/internal.h @@ -113,6 +113,7 @@ #include #include +#include #include #if defined(_MSC_VER) @@ -462,6 +463,42 @@ OPENSSL_EXPORT void CRYPTO_STATIC_MUTEX_unlock_read( 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 +class MutexLockBase { + public: + explicit MutexLockBase(CRYPTO_MUTEX *mu) : mu_(mu) { + assert(mu_ != nullptr); + LockFunc(mu_); + } + ~MutexLockBase() { ReleaseFunc(mu_); } + MutexLockBase(const MutexLockBase &) = delete; + MutexLockBase &operator=(const MutexLockBase &) = + delete; + + private: + CRYPTO_MUTEX *const mu_; +}; + +} // namespace internal + +using MutexWriteLock = + internal::MutexLockBase; +using MutexReadLock = + internal::MutexLockBase; + +} // namespace bssl + +} // extern "C++" +#endif // defined(__cplusplus) + /* Thread local storage. */ diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 9a414ca1..3168a810 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1975,10 +1975,14 @@ OPENSSL_EXPORT SSL_SESSION *SSL_magic_pending_session_ptr(void); * 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 @@ OPENSSL_EXPORT void (*SSL_CTX_get_keylog_callback(const SSL_CTX *ctx))( /* 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 @@ struct ssl_cipher_preference_list_st { 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 @@ struct ssl_ctx_st { /* 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 @@ struct ssl_ctx_st { 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 @@ namespace bssl { 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 534a1cec..ea883e53 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -2066,6 +2066,7 @@ int ssl_compare_public_and_private_key(const EVP_PKEY *pubkey, 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_write(const SSL *ssl); 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 10128d82..026e218b 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc @@ -357,11 +357,18 @@ void ssl_do_msg_callback(SSL *ssl, int is_write, int content_type, } 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 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *method) { 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 @@ void SSL_CTX_free(SSL_CTX *ctx) { 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 @@ int SSL_CTX_get_tlsext_ticket_keys(SSL_CTX *ctx, void *out, size_t len) { 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(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 @@ int SSL_CTX_set_tlsext_ticket_keys(SSL_CTX *ctx, const void *in, size_t len) { 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(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 a1c21dc8..dad0656f 100644 --- a/ssl/ssl_session.cc +++ b/ssl/ssl_session.cc @@ -437,6 +437,59 @@ int ssl_get_new_session(SSL_HANDSHAKE *hs, int is_server) { 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(); + 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 @@ static int ssl_encrypt_ticket_with_cipher_ctx(SSL *ssl, CBB *out, 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 898cd04b..135b37d9 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -49,6 +49,31 @@ OPENSSL_MSVC_PRAGMA(warning(pop)) #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 @@ static bssl::UniquePtr ExpectSessionRenewed(SSL_CTX *client_ctx, 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 @@ static bool TestSessionTimeout(bool is_dtls, const SSL_METHOD *method, return true; } +class DefaultSessionTicketKeyTest + : public ::testing::TestWithParam { + public: + bssl::UniquePtr CreateContext() const { + const VersionParam version = GetParam(); + const SSL_METHOD *method = version.ssl_method == VersionParam::is_dtls + ? DTLS_method() + : TLS_method(); + bssl::UniquePtr 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 cert = GetTestCertificate(); + ASSERT_TRUE(cert); + bssl::UniquePtr key = GetTestKey(); + ASSERT_TRUE(key); + + bssl::UniquePtr 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 cert = GetTestCertificate(); + ASSERT_TRUE(cert); + bssl::UniquePtr key = GetTestKey(); + ASSERT_TRUE(key); + + bssl::UniquePtr server_ctx = CreateContext(); + ASSERT_TRUE(server_ctx); + bssl::UniquePtr 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::max()); + SSL_CTX_set_session_psk_dhe_timeout(server_ctx.get(), + std::numeric_limits::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 client, server; + bssl::UniquePtr 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 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 &i) { + return i.param.name; + }); + static int SwitchContext(SSL *ssl, int *out_alert, void *arg) { SSL_CTX *ctx = reinterpret_cast(arg); SSL_set_SSL_CTX(ssl, ctx); @@ -3313,39 +3475,21 @@ static bool TestRecordCallback(bool is_dtls, const SSL_METHOD *method, 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); - return false; - } - } - - for (uint16_t version : kDTLSVersions) { - if (!test_func(true, DTLS_method(), version)) { - fprintf(stderr, "Test failed at DTLS 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; } } - return true; } @@ -4064,3 +4208,6 @@ TEST(SSLTest, AllTests) { ADD_FAILURE() << "Tests failed"; } } + +} // namespace +} // namespace bssl diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 1dd834bd..bbe64019 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc @@ -3004,60 +3004,20 @@ int ssl_parse_serverhello_tlsext(SSL_HANDSHAKE *hs, CBS *cbs) { 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 @@ ssl_decrypt_ticket_with_cipher_ctx(SSL *ssl, uint8_t **out, size_t *out_len, 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 @@ ssl_decrypt_ticket_with_cipher_ctx(SSL *ssl, uint8_t **out, size_t *out_len, 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 @@ enum ssl_ticket_aead_result_t ssl_process_ticket( 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) {