Reject tickets from the future.
This shouldn't happen, but it is good to check to avoid the potential
underflow in ssl_session_is_time_valid.
This required tweaking the mock clock in bssl_shim to stop going back in
time.
Change-Id: Id3ab8755139e989190d0b53d4bf90fe1ac203022
Reviewed-on: https://boringssl-review.googlesource.com/11841
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index c2396b1..2c20bc3 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -599,6 +599,12 @@
struct timeval now;
ssl_get_current_time(ssl, &now);
+
+ /* Reject tickets from the future to avoid underflow. */
+ if ((long)now.tv_sec < session->time) {
+ return 0;
+ }
+
return session->timeout >= (long)now.tv_sec - session->time;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 1c4ba7f..55ac923 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2047,6 +2047,9 @@
}
for (uint16_t version : kTLSVersions) {
+ static const int kStartTime = 1000;
+ g_current_time.tv_sec = kStartTime;
+
bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
if (!server_ctx || !client_ctx ||
@@ -2088,6 +2091,15 @@
fprintf(stderr, "Error resuming session (version = %04x).\n", version);
return false;
}
+
+ // Rewind the clock to before the session was minted.
+ g_current_time.tv_sec = kStartTime - 1;
+
+ 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;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 2eee888..08b81f3 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -594,8 +594,10 @@
return config->psk.size();
}
+static timeval g_clock;
+
static void CurrentTimeCallback(const SSL *ssl, timeval *out_clock) {
- *out_clock = PacketedBioGetClock(GetTestState(ssl)->packeted_bio);
+ *out_clock = g_clock;
}
static void ChannelIdCallback(SSL *ssl, EVP_PKEY **out_pkey) {
@@ -923,9 +925,7 @@
SSL_CTX_enable_tls_channel_id(ssl_ctx.get());
SSL_CTX_set_channel_id_cb(ssl_ctx.get(), ChannelIdCallback);
- if (config->is_dtls) {
- SSL_CTX_set_current_time_cb(ssl_ctx.get(), CurrentTimeCallback);
- }
+ 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);
@@ -1473,7 +1473,7 @@
return false;
}
if (config->is_dtls) {
- bssl::UniquePtr<BIO> packeted = PacketedBioCreate(!config->async);
+ bssl::UniquePtr<BIO> packeted = PacketedBioCreate(&g_clock, !config->async);
if (!packeted) {
return false;
}
@@ -1748,6 +1748,11 @@
return Usage(argv[0]);
}
+ // Some code treats the zero time special, so initialize the clock to a
+ // non-zero time.
+ g_clock.tv_sec = 1234;
+ g_clock.tv_usec = 1234;
+
bssl::UniquePtr<SSL_CTX> ssl_ctx = SetupCtx(&config);
if (!ssl_ctx) {
ERR_print_errors_fp(stderr);
diff --git a/ssl/test/packeted_bio.cc b/ssl/test/packeted_bio.cc
index f7267fc..8331b4b 100644
--- a/ssl/test/packeted_bio.cc
+++ b/ssl/test/packeted_bio.cc
@@ -31,10 +31,9 @@
const uint8_t kOpcodeTimeoutAck = 't';
struct PacketedBio {
- explicit PacketedBio(bool advance_clock_arg)
- : advance_clock(advance_clock_arg) {
+ PacketedBio(timeval *clock_arg, bool advance_clock_arg)
+ : clock(clock_arg), advance_clock(advance_clock_arg) {
memset(&timeout, 0, sizeof(timeout));
- memset(&clock, 0, sizeof(clock));
memset(&read_deadline, 0, sizeof(read_deadline));
}
@@ -47,14 +46,14 @@
return true;
}
- if (clock.tv_sec == read_deadline.tv_sec) {
- return clock.tv_usec < read_deadline.tv_usec;
+ if (clock->tv_sec == read_deadline.tv_sec) {
+ return clock->tv_usec < read_deadline.tv_usec;
}
- return clock.tv_sec < read_deadline.tv_sec;
+ return clock->tv_sec < read_deadline.tv_sec;
}
timeval timeout;
- timeval clock;
+ timeval *clock;
timeval read_deadline;
bool advance_clock;
};
@@ -66,10 +65,6 @@
return (PacketedBio *)bio->ptr;
}
-const PacketedBio *GetData(const BIO *bio) {
- return GetData(const_cast<BIO*>(bio));
-}
-
// ReadAll reads |len| bytes from |bio| into |out|. It returns 1 on success and
// 0 or -1 on error.
static int ReadAll(BIO *bio, uint8_t *out, size_t len) {
@@ -272,19 +267,15 @@
} // namespace
-bssl::UniquePtr<BIO> PacketedBioCreate(bool advance_clock) {
+bssl::UniquePtr<BIO> PacketedBioCreate(timeval *clock, bool advance_clock) {
bssl::UniquePtr<BIO> bio(BIO_new(&g_packeted_bio_method));
if (!bio) {
return nullptr;
}
- bio->ptr = new PacketedBio(advance_clock);
+ bio->ptr = new PacketedBio(clock, advance_clock);
return bio;
}
-timeval PacketedBioGetClock(const BIO *bio) {
- return GetData(bio)->clock;
-}
-
bool PacketedBioAdvanceClock(BIO *bio) {
PacketedBio *data = GetData(bio);
if (data == nullptr) {
@@ -295,10 +286,10 @@
return false;
}
- data->clock.tv_usec += data->timeout.tv_usec;
- data->clock.tv_sec += data->clock.tv_usec / 1000000;
- data->clock.tv_usec %= 1000000;
- data->clock.tv_sec += data->timeout.tv_sec;
+ data->clock->tv_usec += data->timeout.tv_usec;
+ data->clock->tv_sec += data->clock->tv_usec / 1000000;
+ data->clock->tv_usec %= 1000000;
+ data->clock->tv_sec += data->timeout.tv_sec;
memset(&data->timeout, 0, sizeof(data->timeout));
return true;
}
diff --git a/ssl/test/packeted_bio.h b/ssl/test/packeted_bio.h
index 07930d4..9d4cdcb 100644
--- a/ssl/test/packeted_bio.h
+++ b/ssl/test/packeted_bio.h
@@ -28,21 +28,18 @@
// PacketedBioCreate creates a filter BIO which implements a reliable in-order
-// blocking datagram socket. It internally maintains a clock and honors
-// |BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT| based on it.
+// blocking datagram socket. It uses the value of |*clock| as the clock and
+// honors |BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT| based on it.
//
// During a |BIO_read|, the peer may signal the filter BIO to simulate a
// timeout. If |advance_clock| is true, it automatically advances the clock and
// continues reading, subject to the read deadline. Otherwise, it fails
// immediately. The caller must then call |PacketedBioAdvanceClock| before
// retrying |BIO_read|.
-bssl::UniquePtr<BIO> PacketedBioCreate(bool advance_clock);
+bssl::UniquePtr<BIO> PacketedBioCreate(timeval *clock, bool advance_clock);
-// PacketedBioGetClock returns the current time for |bio|.
-timeval PacketedBioGetClock(const BIO *bio);
-
-// PacketedBioAdvanceClock advances |bio|'s internal clock and returns true if
-// there is a pending timeout. Otherwise, it returns false.
+// PacketedBioAdvanceClock advances |bio|'s clock and returns true if there is a
+// pending timeout. Otherwise, it returns false.
bool PacketedBioAdvanceClock(BIO *bio);
