From f4501347c9f709fe3dad745ac96479513a1c9a8d Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 14 Aug 2014 17:24:37 -0400 Subject: [PATCH] Remove default_timeout hook. Of the remaining implementations left, ssl3_, dtls1_, and ssl23_, dtls1_ is redundant and can be folded into ssl3_. ssl23_ actually isn't; it sets 5 minutes rather than 2 hours. Two hours seems to be what everything else uses and seems a saner default. Most consumers seem to override it anyway (SSL_CTX_set_timeout). But it is a behavior change. The method is called at two points: - SSL_get_default_timeout - SSL_CTX_new Incidentally, the latter call actually makes the former never called internally and the value it returns a lie. SSL_get_default_timeout returns the default timeout of the /current/ method, but in ssl_get_new_session, the timeout is shadowed by session_timeout on the context. That is initialized when SSL_CTX_new is called. So, unless you go out of your way to SSL_CTX_set_timeout(0), it always overrides. (And it actually used to a difference because, for SSL23, the SSL_CTX's method is SSL23, but, when session creation happens, the SSL's method is the version-specific one.) Change-Id: I331d3fd69b726242b36492402717b6d0b521c6ee Reviewed-on: https://boringssl-review.googlesource.com/1521 Reviewed-by: Adam Langley --- include/openssl/ssl.h | 3 ++- ssl/d1_lib.c | 7 ------- ssl/s23_lib.c | 5 ----- ssl/s3_lib.c | 7 ------- ssl/ssl_lib.c | 4 ++-- ssl/ssl_locl.h | 8 -------- ssl/ssl_sess.c | 8 +++----- 7 files changed, 7 insertions(+), 35 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index b3b2d292..6e44b93b 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -387,7 +387,6 @@ struct ssl_method_st int (*num_ciphers)(void); const SSL_CIPHER *(*get_cipher)(unsigned ncipher); const struct ssl_method_st *(*get_ssl_method)(int version); - long (*get_timeout)(void); struct ssl3_enc_method *ssl3_enc; /* Extra SSLv3/TLS stuff */ int (*ssl_version)(void); long (*ssl_callback_ctrl)(SSL *s, int cb_id, void (*fp)(void)); @@ -725,6 +724,8 @@ typedef struct ssl_aead_ctx_st SSL_AEAD_CTX; #define SSL_SESSION_CACHE_MAX_SIZE_DEFAULT (1024*20) +#define SSL_DEFAULT_SESSION_TIMEOUT (2 * 60 * 60) + /* This callback type is used inside SSL_CTX, SSL, and in the functions that set * them. It is used to override the generation of SSL/TLS session IDs in a * server. Return value should be zero on an error, non-zero to proceed. Also, diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index 65cd7dad..d4c32335 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -116,13 +116,6 @@ SSL3_ENC_METHOD DTLSv1_2_enc_data={ dtls1_handshake_write }; -long dtls1_default_timeout(void) - { - /* 2 hours, the 24 hours mentioned in the DTLSv1 spec - * is way too long for http, the cache would over fill */ - return(60*60*2); - } - int dtls1_new(SSL *s) { DTLS1_STATE *d1; diff --git a/ssl/s23_lib.c b/ssl/s23_lib.c index 15c312b4..36f3ef1d 100644 --- a/ssl/s23_lib.c +++ b/ssl/s23_lib.c @@ -61,11 +61,6 @@ #include "ssl_locl.h" -long ssl23_default_timeout(void) - { - return(300); - } - int ssl23_read(SSL *s, void *buf, int len) { int n; diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 01de20fb..4e98f33d 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -1754,13 +1754,6 @@ SSL3_ENC_METHOD SSLv3_enc_data={ ssl3_handshake_write }; -long ssl3_default_timeout(void) - { - /* 2 hours, the 24 hours mentioned in the SSLv3 spec - * is way too long for http, the cache would over fill */ - return(60*60*2); - } - int ssl3_num_ciphers(void) { return(SSL3_NUM_CIPHERS); diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 86f7873a..ecc0c276 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1033,7 +1033,7 @@ int SSL_connect(SSL *s) long SSL_get_default_timeout(const SSL *s) { - return(s->method->get_timeout()); + return SSL_DEFAULT_SESSION_TIMEOUT; } int SSL_read(SSL *s,void *buf,int num) @@ -1929,7 +1929,7 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) ret->session_cache_tail=NULL; /* We take the system default */ - ret->session_timeout=meth->get_timeout(); + ret->session_timeout = SSL_DEFAULT_SESSION_TIMEOUT; ret->new_session_cb=0; ret->remove_session_cb=0; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index e7962451..8a549d13 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -729,7 +729,6 @@ const SSL_METHOD *func_name(void) \ ssl3_num_ciphers, \ ssl3_get_cipher, \ s_get_meth, \ - ssl3_default_timeout, \ &enc_data, \ ssl_undefined_void_function, \ ssl3_callback_ctrl, \ @@ -764,7 +763,6 @@ const SSL_METHOD *func_name(void) \ ssl3_num_ciphers, \ ssl3_get_cipher, \ s_get_meth, \ - ssl3_default_timeout, \ &SSLv3_enc_data, \ ssl_undefined_void_function, \ ssl3_callback_ctrl, \ @@ -799,7 +797,6 @@ const SSL_METHOD *func_name(void) \ ssl3_num_ciphers, \ ssl3_get_cipher, \ s_get_meth, \ - ssl23_default_timeout, \ &TLSv1_2_enc_data, \ ssl_undefined_void_function, \ ssl3_callback_ctrl, \ @@ -835,7 +832,6 @@ const SSL_METHOD *func_name(void) \ ssl3_num_ciphers, \ dtls1_get_cipher, \ s_get_meth, \ - dtls1_default_timeout, \ &enc_data, \ ssl_undefined_void_function, \ ssl3_callback_ctrl, \ @@ -963,7 +959,6 @@ int ssl3_pending(const SSL *s); void ssl3_record_sequence_update(unsigned char *seq); int ssl3_do_change_cipher_spec(SSL *ssl); -long ssl3_default_timeout(void ); void ssl3_set_handshake_header(SSL *s, int htype, unsigned long len); int ssl3_handshake_write(SSL *s); @@ -971,9 +966,7 @@ int ssl3_handshake_write(SSL *s); int ssl23_read(SSL *s, void *buf, int len); int ssl23_peek(SSL *s, void *buf, int len); int ssl23_write(SSL *s, const void *buf, int len); -long ssl23_default_timeout(void ); -long tls1_default_timeout(void); int dtls1_do_write(SSL *s,int type); int ssl3_read_n(SSL *s, int n, int max, int extend); int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek); @@ -999,7 +992,6 @@ void dtls1_clear_record_buffer(SSL *s); void dtls1_get_message_header(unsigned char *data, struct hm_header_st *msg_hdr); void dtls1_get_ccs_header(unsigned char *data, struct ccs_header_st *ccs_hdr); void dtls1_reset_seq_numbers(SSL *s, int rw); -long dtls1_default_timeout(void); int dtls1_check_timeout_num(SSL *s); int dtls1_handle_timeout(SSL *s); const SSL_CIPHER *dtls1_get_cipher(unsigned int u); diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 312a9a20..5749574c 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -209,7 +209,7 @@ SSL_SESSION *SSL_SESSION_new(void) ss->verify_result = 1; /* avoid 0 (= X509_V_OK) just in case */ ss->references=1; - ss->timeout=60*5+4; /* 5 minute timeout by default */ + ss->timeout = SSL_DEFAULT_SESSION_TIMEOUT; ss->time=(unsigned long)time(NULL); ss->prev=NULL; ss->next=NULL; @@ -282,10 +282,8 @@ int ssl_get_new_session(SSL *s, int session) if ((ss=SSL_SESSION_new()) == NULL) return(0); - /* If the context has a default timeout, use it */ - if (s->session_ctx->session_timeout == 0) - ss->timeout=SSL_get_default_timeout(s); - else + /* If the context has a default timeout, use it over the default. */ + if (s->session_ctx->session_timeout != 0) ss->timeout=s->session_ctx->session_timeout; if (s->session != NULL)