From 33fe4a0d1406f423e7424ea7367e1d1a51c2edc1 Mon Sep 17 00:00:00 2001 From: Alessandro Ghedini Date: Mon, 6 Feb 2017 13:40:10 +0000 Subject: [PATCH] Remove support for setting per-connection default session timeout As previously discussed, it turns out we don't actually need this, so there's no point in keeping it. Change-Id: If549c917b6bd818cd36948e37cb7839c8d122b1a Reviewed-on: https://boringssl-review.googlesource.com/13641 Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- include/openssl/ssl.h | 14 ---- ssl/internal.h | 8 --- ssl/ssl_lib.c | 3 - ssl/ssl_session.c | 16 +---- ssl/ssl_test.cc | 146 ------------------------------------------ ssl/tls13_client.c | 2 +- ssl/tls13_server.c | 2 +- 7 files changed, 5 insertions(+), 186 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 263b2728..01aa3e8e 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1753,20 +1753,6 @@ OPENSSL_EXPORT void SSL_CTX_set_session_psk_dhe_timeout(SSL_CTX *ctx, * sessions created in |ctx|. */ OPENSSL_EXPORT long SSL_CTX_get_timeout(const SSL_CTX *ctx); -/* SSL_set_session_timeout sets the default lifetime, in seconds, of a TLS 1.2 - * (or earlier) session created in |ssl| to |timeout|, and returns the old - * value. - * - * By default the value |SSL_DEFAULT_SESSION_TIMEOUT| is used, which can be - * overridden at the context level by calling |SSL_CTX_set_timeout|. - * - * If |timeout| is zero the newly created session will not be resumable. */ -OPENSSL_EXPORT long SSL_set_session_timeout(SSL *ssl, long timeout); - -/* SSL_set_session_psk_dhe_timeout sets the lifetime, in seconds, of TLS 1.3 - * sessions created in |ssl| to |timeout|. */ -OPENSSL_EXPORT void SSL_set_session_psk_dhe_timeout(SSL *ssl, long timeout); - /* SSL_CTX_set_session_id_context sets |ctx|'s session ID context to |sid_ctx|. * It returns one on success and zero on error. The session ID context is an * application-defined opaque byte string. A session will not be used in a diff --git a/ssl/internal.h b/ssl/internal.h index a90294e2..50ab3cd3 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1836,14 +1836,6 @@ struct ssl_st { * session space. Only effective on the server side. */ unsigned retain_only_sha256_of_client_certs:1; - /* session_timeout is the default lifetime in seconds of the session - * created in this connection at TLS 1.2 and earlier. */ - long session_timeout; - - /* session_psk_dhe_timeout is the default lifetime in seconds of sessions - * created in this connection at TLS 1.3. */ - long session_psk_dhe_timeout; - /* OCSP response to be sent to the client, if requested. */ CRYPTO_BUFFER *ocsp_response; }; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 851c81f5..912d9344 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -471,9 +471,6 @@ SSL *SSL_new(SSL_CTX *ctx) { ssl->signed_cert_timestamps_enabled = ctx->signed_cert_timestamps_enabled; ssl->ocsp_stapling_enabled = ctx->ocsp_stapling_enabled; - ssl->session_timeout = ctx->session_timeout; - ssl->session_psk_dhe_timeout = ctx->session_psk_dhe_timeout; - /* If the context has an OCSP response, use it. */ if (ctx->ocsp_response != NULL) { CRYPTO_BUFFER_up_ref(ctx->ocsp_response); diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c index 7cddbdff..9221e925 100644 --- a/ssl/ssl_session.c +++ b/ssl/ssl_session.c @@ -518,13 +518,13 @@ int ssl_get_new_session(SSL_HANDSHAKE *hs, int is_server) { if (version >= TLS1_3_VERSION) { /* TLS 1.3 uses tickets as authenticators, so we are willing to use them for * longer. */ - session->timeout = ssl->session_psk_dhe_timeout; + session->timeout = ssl->initial_ctx->session_psk_dhe_timeout; session->auth_timeout = SSL_DEFAULT_SESSION_AUTH_TIMEOUT; } else { /* TLS 1.2 resumption does not incorporate new key material, so we use a * much shorter timeout. */ - session->timeout = ssl->session_timeout; - session->auth_timeout = ssl->session_timeout; + session->timeout = ssl->initial_ctx->session_timeout; + session->auth_timeout = ssl->initial_ctx->session_timeout; } if (is_server) { @@ -983,16 +983,6 @@ void SSL_CTX_set_session_psk_dhe_timeout(SSL_CTX *ctx, long timeout) { ctx->session_psk_dhe_timeout = timeout; } -long SSL_set_session_timeout(SSL *ssl, long timeout) { - long old_timeout = ssl->session_timeout; - ssl->session_timeout = timeout; - return old_timeout; -} - -void SSL_set_session_psk_dhe_timeout(SSL *ssl, long timeout) { - ssl->session_psk_dhe_timeout = timeout; -} - typedef struct timeout_param_st { SSL_CTX *ctx; long time; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index aa265c8f..8a1a6154 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -2410,151 +2410,6 @@ static bool TestSessionTimeout(bool is_dtls, const SSL_METHOD *method, return true; } -static int SetSessionTimeoutCallback(SSL *ssl, void *arg) { - long timeout = *(long *) arg; - SSL_set_session_timeout(ssl, timeout); - return 1; -} - -static int SetSessionTimeoutCallbackTLS13(SSL *ssl, void *arg) { - long timeout = *(long *) arg; - SSL_set_session_psk_dhe_timeout(ssl, timeout); - return 1; -} - -static bool TestSessionTimeoutCertCallback(bool is_dtls, - const SSL_METHOD *method, - uint16_t version) { - if (version == TLS1_3_VERSION) { - // |SSL_set_session_timeout| only applies to TLS 1.2 style resumption. - return true; - } - - static const int kStartTime = 1000; - g_current_time.tv_sec = kStartTime; - - bssl::UniquePtr cert = GetTestCertificate(); - bssl::UniquePtr key = GetTestKey(); - if (!cert || !key) { - return false; - } - - bssl::UniquePtr server_ctx(SSL_CTX_new(method)); - bssl::UniquePtr client_ctx(SSL_CTX_new(method)); - if (!server_ctx || !client_ctx || - !SSL_CTX_use_certificate(server_ctx.get(), cert.get()) || - !SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()) || - !SSL_CTX_set_min_proto_version(client_ctx.get(), version) || - !SSL_CTX_set_max_proto_version(client_ctx.get(), version) || - !SSL_CTX_set_min_proto_version(server_ctx.get(), version) || - !SSL_CTX_set_max_proto_version(server_ctx.get(), version)) { - return false; - } - - 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); - - SSL_CTX_set_current_time_cb(server_ctx.get(), CurrentTimeCallback); - - long timeout = 25; - if (version == TLS1_3_VERSION) { - SSL_CTX_set_cert_cb(server_ctx.get(), SetSessionTimeoutCallbackTLS13, - &timeout); - } else { - SSL_CTX_set_cert_cb(server_ctx.get(), SetSessionTimeoutCallback, &timeout); - } - - bssl::UniquePtr session = - CreateClientSession(client_ctx.get(), server_ctx.get()); - if (!session) { - fprintf(stderr, "Error getting session.\n"); - return false; - } - - // Advance the clock just behind the timeout. - g_current_time.tv_sec += timeout - 1; - - if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), session.get(), - true /* expect session reused */)) { - fprintf(stderr, "Error resuming session.\n"); - 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.\n"); - return false; - } - - // Set session timeout to 0 to disable resumption. - timeout = 0; - g_current_time.tv_sec = kStartTime; - - bssl::UniquePtr not_resumable_session = - CreateClientSession(client_ctx.get(), server_ctx.get()); - if (!not_resumable_session) { - fprintf(stderr, "Error getting session.\n"); - return false; - } - - if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), - not_resumable_session.get(), - false /* expect session not reused */)) { - fprintf(stderr, "Error resuming session with timeout of 0.\n"); - return false; - } - - // Set both context and connection (via callback) default session timeout. - // The connection one is the one that ends up being used. - timeout = 25; - g_current_time.tv_sec = kStartTime; - - if (version == TLS1_3_VERSION) { - SSL_CTX_set_session_psk_dhe_timeout(server_ctx.get(), timeout - 10); - } else { - SSL_CTX_set_timeout(server_ctx.get(), timeout - 10); - } - - bssl::UniquePtr ctx_and_cb_session = - CreateClientSession(client_ctx.get(), server_ctx.get()); - if (!ctx_and_cb_session) { - fprintf(stderr, "Error getting session.\n"); - return false; - } - - if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), - ctx_and_cb_session.get(), - true /* expect session reused */)) { - fprintf(stderr, "Error resuming session with timeout of 0.\n"); - return false; - } - - // Advance the clock just behind the timeout. - g_current_time.tv_sec += timeout - 1; - - if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), - ctx_and_cb_session.get(), - true /* expect session reused */)) { - fprintf(stderr, "Error resuming session.\n"); - return false; - } - - // Advance the clock one more second. - g_current_time.tv_sec++; - - if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), - ctx_and_cb_session.get(), - false /* expect session not reused */)) { - fprintf(stderr, "Error resuming session.\n"); - return false; - } - - return true; -} - static int SwitchContext(SSL *ssl, int *out_alert, void *arg) { SSL_CTX *ctx = reinterpret_cast(arg); SSL_set_SSL_CTX(ssl, ctx); @@ -3325,7 +3180,6 @@ TEST(SSLTest, AllTests) { !TestClientHello() || !ForEachVersion(TestSessionIDContext) || !ForEachVersion(TestSessionTimeout) || - !ForEachVersion(TestSessionTimeoutCertCallback) || !ForEachVersion(TestSNICallback) || !TestEarlyCallbackVersionSwitch() || !TestSetVersion() || diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index 74d3646e..85f47920 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c @@ -261,7 +261,7 @@ static enum ssl_hs_wait_t do_process_server_hello(SSL_HANDSHAKE *hs) { /* Resumption incorporates fresh key material, so refresh the timeout. */ ssl_session_renew_timeout(ssl, ssl->s3->new_session, - ssl->session_psk_dhe_timeout); + ssl->initial_ctx->session_psk_dhe_timeout); } else if (!ssl_get_new_session(hs, 0)) { ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return ssl_hs_error; diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index 319fe4c2..ed9191ae 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c @@ -224,7 +224,7 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) { /* Resumption incorporates fresh key material, so refresh the timeout. */ ssl_session_renew_timeout(ssl, ssl->s3->new_session, - ssl->session_psk_dhe_timeout); + ssl->initial_ctx->session_psk_dhe_timeout); } if (ssl->ctx->dos_protection_cb != NULL &&