From 62ef33c0b97a36f370903d5e8717800ccb78f8cb Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 21 May 2019 08:32:21 +0200 Subject: record_add_to_buffers: check if there is an incomplete handshake header The function checks if a Handshake message is interleaved with an Application Data, but the check was insuffient because it assumed that a complete header is received in the buffer. Signed-off-by: Daiki Ueno --- lib/record.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/record.c b/lib/record.c index 7d661e2309..e17bebacdf 100644 --- a/lib/record.c +++ b/lib/record.c @@ -824,7 +824,9 @@ record_add_to_buffers(gnutls_session_t session, /* application data cannot be inserted between (async) handshake * messages */ - if (type == GNUTLS_APPLICATION_DATA && session->internals.handshake_recv_buffer_size != 0) { + if (type == GNUTLS_APPLICATION_DATA && + (session->internals.handshake_recv_buffer_size != 0 || + session->internals.handshake_header_recv_buffer.length != 0)) { ret = gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET); goto unexpected_packet; } -- cgit v1.2.1 From 65e2aa80d114d4bef095d129c2eda475e473244a Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 22 May 2019 10:39:27 +0200 Subject: tls13/key_update: increase handling limit from 1 to 8 The limit was too small when testing the capability of handling multiple KeyUpdate messages with tlsfuzzer. This requires a change in the rate limit logic, as previously it doesn't count the KeyUpdate messages despite the name of KEY_UPDATES_PER_SEC. Signed-off-by: Daiki Ueno --- lib/gnutls_int.h | 3 ++- lib/tls13/key_update.c | 25 ++++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h index 50251a356a..867e4d2d76 100644 --- a/lib/gnutls_int.h +++ b/lib/gnutls_int.h @@ -1368,7 +1368,8 @@ typedef struct { /* The hsk_flags are for use within the ongoing handshake; * they are reset to zero prior to handshake start by gnutls_handshake. */ unsigned hsk_flags; - time_t last_key_update; + struct timespec last_key_update; + unsigned key_update_count; /* Read-only pointer to the full ClientHello message */ gnutls_buffer_st full_client_hello; /* The offset at which extensions start in the ClientHello buffer */ diff --git a/lib/tls13/key_update.c b/lib/tls13/key_update.c index abd7c93ec6..d542a214b7 100644 --- a/lib/tls13/key_update.c +++ b/lib/tls13/key_update.c @@ -28,7 +28,8 @@ #include "mbuffers.h" #include "secrets.h" -#define KEY_UPDATES_PER_SEC 1 +#define KEY_UPDATES_WINDOW 1000 +#define KEY_UPDATES_PER_WINDOW 8 static int update_keys(gnutls_session_t session, hs_stage_t stage) { @@ -60,18 +61,28 @@ static int update_keys(gnutls_session_t session, hs_stage_t stage) int _gnutls13_recv_key_update(gnutls_session_t session, gnutls_buffer_st *buf) { int ret; - time_t now = gnutls_time(0); + struct timespec now; if (buf->length != 1) return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET_LENGTH); - if (unlikely(now - session->internals.last_key_update < KEY_UPDATES_PER_SEC)) { - _gnutls_debug_log("reached maximum number of key updates per second (%d)\n", - KEY_UPDATES_PER_SEC); - return gnutls_assert_val(GNUTLS_E_TOO_MANY_HANDSHAKE_PACKETS); + gnutls_gettime(&now); + + /* Roll over the counter if the time window has elapsed */ + if (session->internals.key_update_count == 0 || + timespec_sub_ms(&now, &session->internals.last_key_update) > + KEY_UPDATES_WINDOW) { + session->internals.last_key_update = now; + session->internals.key_update_count = 0; } - session->internals.last_key_update = now; + if (unlikely(++session->internals.key_update_count > + KEY_UPDATES_PER_WINDOW)) { + _gnutls_debug_log("reached maximum number of key updates per %d milliseconds (%d)\n", + KEY_UPDATES_WINDOW, + KEY_UPDATES_PER_WINDOW); + return gnutls_assert_val(GNUTLS_E_TOO_MANY_HANDSHAKE_PACKETS); + } _gnutls_epoch_gc(session); -- cgit v1.2.1