From 721e8b79a90ece5d49ac3c673127e9e9089f830e Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 3 Aug 2016 13:13:17 -0400 Subject: [PATCH] Test that servers enforce session timeouts. Extend the DTLS mock clock to apply to sessions too and test that resumption behaves as expected. Change-Id: Ib8fdec91b36e11cfa032872b63cf589f93b3da13 Reviewed-on: https://boringssl-review.googlesource.com/9110 Reviewed-by: Adam Langley Commit-Queue: Adam Langley CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/d1_lib.c | 30 ++-------------- ssl/handshake_client.c | 4 ++- ssl/handshake_server.c | 11 +++--- ssl/internal.h | 2 ++ ssl/ssl_lib.c | 27 ++++++++++++++- ssl/ssl_session.c | 11 ++++-- ssl/ssl_test.cc | 77 +++++++++++++++++++++++++++++++++++++++++- ssl/test/bssl_shim.cc | 4 ++- 8 files changed, 127 insertions(+), 39 deletions(-) diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index 901168c7..c3a9432b 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -66,13 +66,6 @@ #include "internal.h" -#if defined(OPENSSL_WINDOWS) -#include -#else -#include -#include -#endif - /* DTLS1_MTU_TIMEOUTS is the maximum number of timeouts to expire * before starting to decrease the MTU. */ @@ -82,8 +75,6 @@ * before failing the DTLS handshake. */ #define DTLS1_MAX_TIMEOUTS 12 -static void get_current_time(const SSL *ssl, struct timeval *out_clock); - int dtls1_new(SSL *ssl) { DTLS1_STATE *d1; @@ -139,7 +130,7 @@ void dtls1_start_timer(SSL *ssl) { } /* Set timeout to current time */ - get_current_time(ssl, &ssl->d1->next_timeout); + ssl_get_current_time(ssl, &ssl->d1->next_timeout); /* Add duration to current time */ ssl->d1->next_timeout.tv_sec += ssl->d1->timeout_duration_ms / 1000; @@ -162,9 +153,8 @@ int DTLSv1_get_timeout(const SSL *ssl, struct timeval *out) { return 0; } - /* Get current time */ struct timeval timenow; - get_current_time(ssl, &timenow); + ssl_get_current_time(ssl, &timenow); /* If timer already expired, set remaining time to 0 */ if (ssl->d1->next_timeout.tv_sec < timenow.tv_sec || @@ -273,22 +263,6 @@ int DTLSv1_handle_timeout(SSL *ssl) { return dtls1_retransmit_outgoing_messages(ssl); } -static void get_current_time(const SSL *ssl, struct timeval *out_clock) { - if (ssl->ctx->current_time_cb != NULL) { - ssl->ctx->current_time_cb(ssl, out_clock); - return; - } - -#if defined(OPENSSL_WINDOWS) - struct _timeb time; - _ftime(&time); - out_clock->tv_sec = time.time; - out_clock->tv_usec = time.millitm * 1000; -#else - gettimeofday(out_clock, NULL); -#endif -} - void dtls1_expect_flight(SSL *ssl) { dtls1_start_timer(ssl); } diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index 38fb3af7..7135a8f6 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -710,8 +710,10 @@ static int ssl3_send_client_hello(SSL *ssl) { if (ssl->session != NULL) { uint16_t session_version = ssl->method->version_from_wire(ssl->session->ssl_version); + struct timeval now; + ssl_get_current_time(ssl, &now); if (ssl->session->session_id_length == 0 || ssl->session->not_resumable || - ssl->session->timeout < (long)(time(NULL) - ssl->session->time) || + ssl->session->timeout < (long)now.tv_sec - ssl->session->time || session_version < min_version || session_version > max_version) { SSL_set_session(ssl, NULL); } diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index 4d464d34..8a28c18c 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -885,11 +885,12 @@ static int ssl3_send_server_hello(SSL *ssl) { ssl->s3->tlsext_channel_id_valid = 0; } - const uint32_t current_time = time(NULL); - ssl->s3->server_random[0] = current_time >> 24; - ssl->s3->server_random[1] = current_time >> 16; - ssl->s3->server_random[2] = current_time >> 8; - ssl->s3->server_random[3] = current_time; + struct timeval now; + ssl_get_current_time(ssl, &now); + ssl->s3->server_random[0] = now.tv_sec >> 24; + ssl->s3->server_random[1] = now.tv_sec >> 16; + ssl->s3->server_random[2] = now.tv_sec >> 8; + ssl->s3->server_random[3] = now.tv_sec; if (!RAND_bytes(ssl->s3->server_random + 4, SSL3_RANDOM_SIZE - 4)) { return -1; } diff --git a/ssl/internal.h b/ssl/internal.h index 5c8f32cc..c69bc5fa 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1479,6 +1479,8 @@ int tls12_check_peer_sigalg(SSL *ssl, int *out_alert, uint16_t signature_algorithm); void ssl_set_client_disabled(SSL *ssl); +void ssl_get_current_time(const SSL *ssl, struct timeval *out_clock); + #if defined(__cplusplus) } /* extern C */ diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index a9df4e82..64fca640 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -155,6 +155,13 @@ #include "internal.h" #include "../crypto/internal.h" +#if defined(OPENSSL_WINDOWS) +#include +#else +#include +#include +#endif + /* |SSL_R_UNKNOWN_PROTOCOL| is no longer emitted, but continue to define it * to avoid downstream churn. */ @@ -2102,7 +2109,9 @@ void ssl_update_cache(SSL *ssl, int mode) { CRYPTO_MUTEX_unlock_write(&ctx->lock); if (flush_cache) { - SSL_CTX_flush_sessions(ctx, (unsigned long)time(NULL)); + struct timeval now; + ssl_get_current_time(ssl, &now); + SSL_CTX_flush_sessions(ctx, (long)now.tv_sec); } } } @@ -3005,3 +3014,19 @@ int SSL_set_tmp_ecdh(SSL *ssl, const EC_KEY *ec_key) { int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec_key)); return SSL_set1_curves(ssl, &nid, 1); } + +void ssl_get_current_time(const SSL *ssl, struct timeval *out_clock) { + if (ssl->ctx->current_time_cb != NULL) { + ssl->ctx->current_time_cb(ssl, out_clock); + return; + } + +#if defined(OPENSSL_WINDOWS) + struct _timeb time; + _ftime(&time); + out_clock->tv_sec = time.time; + out_clock->tv_usec = time.millitm * 1000; +#else + gettimeofday(out_clock, NULL); +#endif +} diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c index 3bd52e02..0388661f 100644 --- a/ssl/ssl_session.c +++ b/ssl/ssl_session.c @@ -170,7 +170,7 @@ SSL_SESSION *SSL_SESSION_new(void) { session->verify_result = X509_V_ERR_INVALID_CALL; session->references = 1; session->timeout = SSL_DEFAULT_SESSION_TIMEOUT; - session->time = (unsigned long)time(NULL); + session->time = (long)time(NULL); CRYPTO_new_ex_data(&session->ex_data); return session; } @@ -419,6 +419,11 @@ int ssl_get_new_session(SSL *ssl, int is_server) { return 0; } + /* Fill in the time from the |SSL_CTX|'s clock. */ + struct timeval now; + ssl_get_current_time(ssl, &now); + session->time = now.tv_sec; + /* If the context has a default timeout, use it over the default. */ if (ssl->initial_ctx->session_timeout != 0) { session->timeout = ssl->initial_ctx->session_timeout; @@ -678,7 +683,9 @@ enum ssl_session_result_t ssl_get_prev_session( return ssl_session_error; } - if (session->timeout < (long)(time(NULL) - session->time)) { + struct timeval now; + ssl_get_current_time(ssl, &now); + if (session->timeout < (long)now.tv_sec - session->time) { if (from_cache) { /* The session was from the cache, so remove it. */ SSL_CTX_remove_session(ssl->initial_ctx, session); diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 97ade039..c2ce99ba 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -34,6 +34,15 @@ #include "test/scoped_types.h" #include "../crypto/test/test_util.h" +#if defined(OPENSSL_WINDOWS) +/* Windows defines struct timeval in winsock2.h. */ +OPENSSL_MSVC_PRAGMA(warning(push, 3)) +#include +OPENSSL_MSVC_PRAGMA(warning(pop)) +#else +#include +#endif + struct ExpectedCipher { unsigned long id; @@ -1840,6 +1849,71 @@ static bool TestSessionIDContext() { return true; } +static timeval g_current_time; + +static void CurrentTimeCallback(const SSL *ssl, timeval *out_clock) { + *out_clock = g_current_time; +} + +static bool TestSessionTimeout() { + ScopedX509 cert = GetTestCertificate(); + ScopedEVP_PKEY key = GetTestKey(); + if (!cert || !key) { + return false; + } + + for (uint16_t version : kVersions) { + // TODO(davidben): Enable this when TLS 1.3 resumption is implemented. + if (version == TLS1_3_VERSION) { + continue; + } + + ScopedSSL_CTX server_ctx(SSL_CTX_new(TLS_method())); + ScopedSSL_CTX client_ctx(SSL_CTX_new(TLS_method())); + if (!server_ctx || !client_ctx || + !SSL_CTX_use_certificate(server_ctx.get(), cert.get()) || + !SSL_CTX_use_PrivateKey(server_ctx.get(), key.get())) { + return false; + } + + SSL_CTX_set_min_version(client_ctx.get(), version); + SSL_CTX_set_max_version(client_ctx.get(), version); + SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_BOTH); + + SSL_CTX_set_min_version(server_ctx.get(), version); + SSL_CTX_set_max_version(server_ctx.get(), version); + SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_BOTH); + SSL_CTX_set_current_time_cb(server_ctx.get(), CurrentTimeCallback); + + ScopedSSL_SESSION session = + CreateClientSession(client_ctx.get(), server_ctx.get()); + if (!session) { + fprintf(stderr, "Error getting session (version = %04x).\n", version); + return false; + } + + // Advance the clock just behind the timeout. + g_current_time.tv_sec += SSL_DEFAULT_SESSION_TIMEOUT; + + if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), session.get(), + true /* expect session reused */)) { + fprintf(stderr, "Error resuming session (version = %04x).\n", version); + return false; + } + + // Advance the clock one more second. + g_current_time.tv_sec++; + + if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), session.get(), + false /* expect session not reused */)) { + fprintf(stderr, "Error resuming session (version = %04x).\n", version); + return false; + } + } + + return true; +} + int main() { CRYPTO_library_init(); @@ -1872,7 +1946,8 @@ int main() { !TestGetPeerCertificate() || !TestRetainOnlySHA256OfCerts() || !TestClientHello() || - !TestSessionIDContext()) { + !TestSessionIDContext() || + !TestSessionTimeout()) { ERR_print_errors_fp(stderr); return 1; } diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 9cc1277d..5694ac2e 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -864,7 +864,9 @@ static ScopedSSL_CTX SetupCtx(const TestConfig *config) { SSL_CTX_enable_tls_channel_id(ssl_ctx.get()); SSL_CTX_set_channel_id_cb(ssl_ctx.get(), ChannelIdCallback); - SSL_CTX_set_current_time_cb(ssl_ctx.get(), CurrentTimeCallback); + if (config->is_dtls) { + SSL_CTX_set_current_time_cb(ssl_ctx.get(), CurrentTimeCallback); + } SSL_CTX_set_info_callback(ssl_ctx.get(), InfoCallback); SSL_CTX_sess_set_new_cb(ssl_ctx.get(), NewSessionCallback);