Don't use long for timestamps.
This is the first part to fixing the SSL stack to be 2038-clean.
Internal structures and functions are switched to use OPENSSL_timeval
which, unlike timeval and long, are suitable for timestamps on all
platforms.
It is generally accepted that the year is now sometime after 1970, so
use uint64_t for the timestamps to avoid worrying about serializing
negative numbers in SSL_SESSION.
A follow-up change will fix SSL_CTX_set_current_time_cb to use
OPENSSL_timeval. This will require some coordinating with WebRTC.
DTLSv1_get_timeout is left alone for compatibility and because it stores
time remaining rather than an absolute time.
BUG=155
Change-Id: I1a5054813300874b6f29e348f9cd8ca80f6b9729
Reviewed-on: https://boringssl-review.googlesource.com/13944
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 9f91949..ef15252 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -147,32 +147,43 @@
return 0;
}
- struct timeval timenow;
+ struct OPENSSL_timeval 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 ||
(ssl->d1->next_timeout.tv_sec == timenow.tv_sec &&
ssl->d1->next_timeout.tv_usec <= timenow.tv_usec)) {
- OPENSSL_memset(out, 0, sizeof(struct timeval));
+ OPENSSL_memset(out, 0, sizeof(*out));
return 1;
}
/* Calculate time left until timer expires */
- OPENSSL_memcpy(out, &ssl->d1->next_timeout, sizeof(struct timeval));
- out->tv_sec -= timenow.tv_sec;
- out->tv_usec -= timenow.tv_usec;
- if (out->tv_usec < 0) {
- out->tv_sec--;
- out->tv_usec += 1000000;
+ struct OPENSSL_timeval ret;
+ OPENSSL_memcpy(&ret, &ssl->d1->next_timeout, sizeof(ret));
+ ret.tv_sec -= timenow.tv_sec;
+ if (ret.tv_usec >= timenow.tv_usec) {
+ ret.tv_usec -= timenow.tv_usec;
+ } else {
+ ret.tv_usec = 1000000 + ret.tv_usec - timenow.tv_usec;
+ ret.tv_sec--;
}
/* If remaining time is less than 15 ms, set it to 0 to prevent issues
- * because of small devergences with socket timeouts. */
- if (out->tv_sec == 0 && out->tv_usec < 15000) {
- OPENSSL_memset(out, 0, sizeof(struct timeval));
+ * because of small divergences with socket timeouts. */
+ if (ret.tv_sec == 0 && ret.tv_usec < 15000) {
+ OPENSSL_memset(&ret, 0, sizeof(ret));
}
+ /* Clamp the result in case of overflow. */
+ if (ret.tv_sec > INT_MAX) {
+ assert(0);
+ out->tv_sec = INT_MAX;
+ } else {
+ out->tv_sec = ret.tv_sec;
+ }
+
+ out->tv_usec = ret.tv_usec;
return 1;
}
@@ -204,7 +215,7 @@
void dtls1_stop_timer(SSL *ssl) {
/* Reset everything */
ssl->d1->num_timeouts = 0;
- OPENSSL_memset(&ssl->d1->next_timeout, 0, sizeof(struct timeval));
+ OPENSSL_memset(&ssl->d1->next_timeout, 0, sizeof(ssl->d1->next_timeout));
ssl->d1->timeout_duration_ms = ssl->initial_timeout_duration_ms;
/* Clear retransmission buffer */
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 1b65662..e3a4e51 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1058,7 +1058,7 @@
ssl->s3->tlsext_channel_id_valid = 0;
}
- struct timeval now;
+ struct OPENSSL_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;
diff --git a/ssl/internal.h b/ssl/internal.h
index 144b680..dbeae92 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1720,6 +1720,11 @@
uint8_t *reassembly;
} hm_fragment;
+struct OPENSSL_timeval {
+ uint64_t tv_sec;
+ uint32_t tv_usec;
+};
+
typedef struct dtls1_state_st {
/* send_cookie is true if we are resending the ClientHello
* with a cookie from a HelloVerifyRequest. */
@@ -1768,7 +1773,7 @@
/* Indicates when the last handshake msg or heartbeat sent will
* timeout. */
- struct timeval next_timeout;
+ struct OPENSSL_timeval next_timeout;
/* timeout_duration_ms is the timeout duration in milliseconds. */
unsigned timeout_duration_ms;
@@ -2013,7 +2018,8 @@
/* ssl_session_renew_timeout calls |ssl_session_rebase_time| and renews
* |session|'s timeout to |timeout| (measured from the current time). The
* renewal is clamped to the session's auth_timeout. */
-void ssl_session_renew_timeout(SSL *ssl, SSL_SESSION *session, long timeout);
+void ssl_session_renew_timeout(SSL *ssl, SSL_SESSION *session,
+ uint32_t timeout);
void ssl_cipher_preference_list_free(
struct ssl_cipher_preference_list_st *cipher_list);
@@ -2203,7 +2209,7 @@
* call this function before the version is determined. */
uint16_t ssl3_protocol_version(const SSL *ssl);
-void ssl_get_current_time(const SSL *ssl, struct timeval *out_clock);
+void ssl_get_current_time(const SSL *ssl, 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_asn1.c b/ssl/ssl_asn1.c
index 3533225..cfcc12a 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -210,28 +210,10 @@
!CBB_add_bytes(&child, in->session_id,
for_ticket ? 0 : in->session_id_length) ||
!CBB_add_asn1(&session, &child, CBS_ASN1_OCTETSTRING) ||
- !CBB_add_bytes(&child, in->master_key, in->master_key_length)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- if (in->time < 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
- goto err;
- }
-
- if (!CBB_add_asn1(&session, &child, kTimeTag) ||
- !CBB_add_asn1_uint64(&child, in->time)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- if (in->timeout < 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
- goto err;
- }
-
- if (!CBB_add_asn1(&session, &child, kTimeoutTag) ||
+ !CBB_add_bytes(&child, in->master_key, in->master_key_length) ||
+ !CBB_add_asn1(&session, &child, kTimeTag) ||
+ !CBB_add_asn1_uint64(&child, in->time) ||
+ !CBB_add_asn1(&session, &child, kTimeoutTag) ||
!CBB_add_asn1_uint64(&child, in->timeout)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
goto err;
@@ -634,19 +616,17 @@
ret->master_key_length = CBS_len(&master_key);
CBS child;
- uint64_t time, timeout;
+ uint64_t timeout;
if (!CBS_get_asn1(&session, &child, kTimeTag) ||
- !CBS_get_asn1_uint64(&child, &time) ||
- time > LONG_MAX ||
+ !CBS_get_asn1_uint64(&child, &ret->time) ||
!CBS_get_asn1(&session, &child, kTimeoutTag) ||
!CBS_get_asn1_uint64(&child, &timeout) ||
- timeout > LONG_MAX) {
+ timeout > UINT32_MAX) {
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
goto err;
}
- ret->time = (long)time;
- ret->timeout = (long)timeout;
+ ret->timeout = (uint32_t)timeout;
CBS peer;
int has_peer;
@@ -811,8 +791,8 @@
kPeerSignatureAlgorithmTag, 0) ||
!SSL_SESSION_parse_u32(&session, &ret->ticket_max_early_data,
kTicketMaxEarlyDataTag, 0) ||
- !SSL_SESSION_parse_long(&session, &ret->auth_timeout, kAuthTimeoutTag,
- ret->timeout) ||
+ !SSL_SESSION_parse_u32(&session, &ret->auth_timeout, kAuthTimeoutTag,
+ ret->timeout) ||
!SSL_SESSION_parse_octet_string(&session, &ret->early_alpn,
&ret->early_alpn_len, kEarlyALPNTag) ||
CBS_len(&session) != 0) {
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index a37de06..75cac31 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1888,9 +1888,9 @@
CRYPTO_MUTEX_unlock_write(&ctx->lock);
if (flush_cache) {
- struct timeval now;
+ struct OPENSSL_timeval now;
ssl_get_current_time(ssl, &now);
- SSL_CTX_flush_sessions(ctx, (long)now.tv_sec);
+ SSL_CTX_flush_sessions(ctx, now.tv_sec);
}
}
}
@@ -2686,9 +2686,20 @@
return SSL_set1_curves(ssl, &nid, 1);
}
-void ssl_get_current_time(const SSL *ssl, struct timeval *out_clock) {
+void ssl_get_current_time(const SSL *ssl, struct OPENSSL_timeval *out_clock) {
if (ssl->ctx->current_time_cb != NULL) {
- ssl->ctx->current_time_cb(ssl, out_clock);
+ /* 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);
+ if (clock.tv_sec < 0) {
+ assert(0);
+ out_clock->tv_sec = 0;
+ out_clock->tv_usec = 0;
+ } else {
+ out_clock->tv_sec = (uint64_t)clock.tv_sec;
+ out_clock->tv_usec = (uint32_t)clock.tv_usec;
+ }
return;
}
@@ -2698,10 +2709,25 @@
#elif defined(OPENSSL_WINDOWS)
struct _timeb time;
_ftime(&time);
- out_clock->tv_sec = time.time;
- out_clock->tv_usec = time.millitm * 1000;
+ if (time.time < 0) {
+ assert(0);
+ out_clock->tv_sec = 0;
+ out_clock->tv_usec = 0;
+ } else {
+ out_clock->tv_sec = time.time;
+ out_clock->tv_usec = time.millitm * 1000;
+ }
#else
- gettimeofday(out_clock, NULL);
+ struct timeval clock;
+ gettimeofday(&clock, NULL);
+ if (clock.tv_sec < 0) {
+ assert(0);
+ out_clock->tv_sec = 0;
+ out_clock->tv_usec = 0;
+ } else {
+ out_clock->tv_sec = (uint64_t)clock.tv_sec;
+ out_clock->tv_usec = (uint32_t)clock.tv_usec;
+ }
#endif
}
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index bbe88c3..c9390d2 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -173,7 +173,7 @@
session->references = 1;
session->timeout = SSL_DEFAULT_SESSION_TIMEOUT;
session->auth_timeout = SSL_DEFAULT_SESSION_TIMEOUT;
- session->time = (long)time(NULL);
+ session->time = time(NULL);
CRYPTO_new_ex_data(&session->ex_data);
return session;
}
@@ -315,14 +315,12 @@
}
void ssl_session_rebase_time(SSL *ssl, SSL_SESSION *session) {
- struct timeval now;
+ struct OPENSSL_timeval now;
ssl_get_current_time(ssl, &now);
- /* To avoid overflows and underflows, if we've gone back in time or any value
- * is negative, update the time, but mark the session expired. */
- if (session->time > now.tv_sec ||
- session->time < 0 ||
- now.tv_sec < 0) {
+ /* To avoid overflows and underflows, if we've gone back in time, update the
+ * time, but mark the session expired. */
+ if (session->time > now.tv_sec) {
session->time = now.tv_sec;
session->timeout = 0;
session->auth_timeout = 0;
@@ -331,7 +329,7 @@
/* Adjust the session time and timeouts. If the session has already expired,
* clamp the timeouts at zero. */
- long delta = now.tv_sec - session->time;
+ uint64_t delta = now.tv_sec - session->time;
session->time = now.tv_sec;
if (session->timeout < delta) {
session->timeout = 0;
@@ -345,7 +343,8 @@
}
}
-void ssl_session_renew_timeout(SSL *ssl, SSL_SESSION *session, long timeout) {
+void ssl_session_renew_timeout(SSL *ssl, SSL_SESSION *session,
+ uint32_t timeout) {
/* Rebase the timestamp relative to the current time so |timeout| is measured
* correctly. */
ssl_session_rebase_time(ssl, session);
@@ -395,11 +394,11 @@
return session->session_id;
}
-long SSL_SESSION_get_timeout(const SSL_SESSION *session) {
+uint32_t SSL_SESSION_get_timeout(const SSL_SESSION *session) {
return session->timeout;
}
-long SSL_SESSION_get_time(const SSL_SESSION *session) {
+uint64_t SSL_SESSION_get_time(const SSL_SESSION *session) {
if (session == NULL) {
/* NULL should crash, but silently accept it here for compatibility. */
return 0;
@@ -424,7 +423,7 @@
return max_out;
}
-long SSL_SESSION_set_time(SSL_SESSION *session, long time) {
+uint64_t SSL_SESSION_set_time(SSL_SESSION *session, uint64_t time) {
if (session == NULL) {
return 0;
}
@@ -433,7 +432,7 @@
return time;
}
-long SSL_SESSION_set_timeout(SSL_SESSION *session, long timeout) {
+uint32_t SSL_SESSION_set_timeout(SSL_SESSION *session, uint32_t timeout) {
if (session == NULL) {
return 0;
}
@@ -528,7 +527,7 @@
session->ssl_version = ssl->version;
/* Fill in the time from the |SSL_CTX|'s clock. */
- struct timeval now;
+ struct OPENSSL_timeval now;
ssl_get_current_time(ssl, &now);
session->time = now.tv_sec;
@@ -689,15 +688,15 @@
return 0;
}
- struct timeval now;
+ struct OPENSSL_timeval now;
ssl_get_current_time(ssl, &now);
/* Reject tickets from the future to avoid underflow. */
- if ((long)now.tv_sec < session->time) {
+ if (now.tv_sec < session->time) {
return 0;
}
- return session->timeout > (long)now.tv_sec - session->time;
+ return session->timeout > now.tv_sec - session->time;
}
int ssl_session_is_resumable(const SSL_HANDSHAKE *hs,
@@ -933,7 +932,7 @@
}
}
-long SSL_CTX_set_timeout(SSL_CTX *ctx, long timeout) {
+uint32_t SSL_CTX_set_timeout(SSL_CTX *ctx, uint32_t timeout) {
if (ctx == NULL) {
return 0;
}
@@ -943,12 +942,12 @@
timeout = SSL_DEFAULT_SESSION_TIMEOUT;
}
- long old_timeout = ctx->session_timeout;
+ uint32_t old_timeout = ctx->session_timeout;
ctx->session_timeout = timeout;
return old_timeout;
}
-long SSL_CTX_get_timeout(const SSL_CTX *ctx) {
+uint32_t SSL_CTX_get_timeout(const SSL_CTX *ctx) {
if (ctx == NULL) {
return 0;
}
@@ -956,13 +955,13 @@
return ctx->session_timeout;
}
-void SSL_CTX_set_session_psk_dhe_timeout(SSL_CTX *ctx, long timeout) {
+void SSL_CTX_set_session_psk_dhe_timeout(SSL_CTX *ctx, uint32_t timeout) {
ctx->session_psk_dhe_timeout = timeout;
}
typedef struct timeout_param_st {
SSL_CTX *ctx;
- long time;
+ uint64_t time;
LHASH_OF(SSL_SESSION) *cache;
} TIMEOUT_PARAM;
@@ -970,6 +969,7 @@
TIMEOUT_PARAM *param = void_param;
if (param->time == 0 ||
+ session->time + session->timeout < session->time ||
param->time > (session->time + session->timeout)) {
/* timeout */
/* The reason we don't call SSL_CTX_remove_session() is to
@@ -984,7 +984,7 @@
}
}
-void SSL_CTX_flush_sessions(SSL_CTX *ctx, long time) {
+void SSL_CTX_flush_sessions(SSL_CTX *ctx, uint64_t time) {
TIMEOUT_PARAM tp;
tp.ctx = ctx;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index d6ef1ff..2a9f945 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1901,7 +1901,7 @@
return 1;
}
- struct timeval now;
+ struct OPENSSL_timeval now;
ssl_get_current_time(ssl, &now);
uint32_t ticket_age = 1000 * (now.tv_sec - ssl->session->time);
uint32_t obfuscated_ticket_age = ticket_age + ssl->session->ticket_age_add;
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index e178666..254c363 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -650,18 +650,9 @@
}
/* Cap the renewable lifetime by the server advertised value. This avoids
- * wasting bandwidth on 0-RTT when we know the server will reject it.
- *
- * TODO(davidben): This dance where we're not sure if long or uint32_t is
- * bigger is silly. session->timeout should not be a long to begin with.
- * https://crbug.com/boringssl/155. */
-#if LONG_MAX < 0xffffffff
- if (server_timeout > LONG_MAX) {
- server_timeout = LONG_MAX;
- }
-#endif
- if (session->timeout > (long)server_timeout) {
- session->timeout = (long)server_timeout;
+ * wasting bandwidth on 0-RTT when we know the server will reject it. */
+ if (session->timeout > server_timeout) {
+ session->timeout = server_timeout;
}
/* Parse out the extensions. */
