From 11b4fd9d4ffd392b5b5a4ec0f0fefe713b2e1117 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Wed, 8 Feb 2012 10:16:45 +0100 Subject: [PATCH] DTLS fixes. Corrected bugs in DTLS sliding window code to account for lost packets arriving after an epoch change. The last handshake flight is now being kept by both parties in order to be used as a lost packet indication. --- lib/debug.c | 2 +- lib/gnutls_buffers.c | 2 +- lib/gnutls_dtls.c | 51 ++++++++++++++++++++++++++++++++------------------- lib/gnutls_record.c | 14 ++++++++++---- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/lib/debug.c b/lib/debug.c index fc350f351..1e5b0c6fb 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -49,7 +49,7 @@ _gnutls_packet2str (content_type_t packet) switch (packet) { case GNUTLS_CHANGE_CIPHER_SPEC: - return "Change Cipher Spec"; + return "ChangeCipherSpec"; case GNUTLS_ALERT: return "Alert"; case GNUTLS_HANDSHAKE: diff --git a/lib/gnutls_buffers.c b/lib/gnutls_buffers.c index 7cbd63526..d95719719 100644 --- a/lib/gnutls_buffers.c +++ b/lib/gnutls_buffers.c @@ -51,6 +51,7 @@ #include /* gnutls_epoch_get */ #include #include +#include "debug.h" #ifndef EAGAIN #define EAGAIN EWOULDBLOCK @@ -600,7 +601,6 @@ _gnutls_io_write_flush (gnutls_session_t session) return sent; } -#include "debug.h" /* Checks whether there are received data within * a timeframe. * diff --git a/lib/gnutls_dtls.c b/lib/gnutls_dtls.c index 5c6c333b7..5b945fd31 100644 --- a/lib/gnutls_dtls.c +++ b/lib/gnutls_dtls.c @@ -179,6 +179,7 @@ int ret; * non blocking way, check if it is time to retransmit or just * return. */ + if (session->internals.dtls.flight_init != 0 && session->internals.dtls.blocking == 0) { /* just in case previous run was interrupted */ @@ -205,11 +206,8 @@ int ret; goto cleanup; } } - else - return ret; } - do { if (now-session->internals.dtls.handshake_start_time >= session->internals.dtls.total_timeout_ms/1000) @@ -233,7 +231,7 @@ int ret; session->internals.dtls.handshake_last_call = now; session->internals.dtls.actual_retrans_timeout_ms = session->internals.dtls.retrans_timeout_ms; - if (last_type == GNUTLS_HANDSHAKE_FINISHED && _dtls_is_async(session)) + if (last_type == GNUTLS_HANDSHAKE_FINISHED) { /* we cannot do anything here. We just return 0 and * if a retransmission occurs because peer didn't receive it @@ -243,8 +241,6 @@ int ret; } else session->internals.dtls.last_flight = 0; - - } ret = _gnutls_io_write_flush (session); @@ -252,7 +248,7 @@ int ret; return gnutls_assert_val(ret); /* last message in handshake -> no ack */ - if (session->internals.dtls.last_flight != 0) + if (session->internals.dtls.last_flight != 0 && _dtls_is_async(session)) { /* we cannot do anything here. We just return 0 and * if a retransmission occurs because peer didn't receive it @@ -321,12 +317,13 @@ static void rot_window(gnutls_session_t session, int places) int _dtls_record_check(gnutls_session_t session, uint64 * _seq) { uint64_t seq = 0, diff; -int i, offset = 0; +unsigned int i, offset = 0; - for (i=0;i<8;i++) + seq |= _seq->i[0] & 0xff; + for (i=1;i<8;i++) { - seq |= _seq->i[i]; seq <<= 8; + seq |= _seq->i[i] & 0xff; } if (window_size == 0) @@ -341,9 +338,10 @@ int i, offset = 0; return -1; } - if (window_size == DTLS_RECORD_WINDOW_SIZE) { - rot_window(session, MOVE_SIZE); - } + if (window_size == DTLS_RECORD_WINDOW_SIZE) + { + rot_window(session, MOVE_SIZE); + } if (seq < window_table[window_size-1]) { @@ -352,19 +350,34 @@ int i, offset = 0; if (diff >= window_size) { + /* Probably an epoch change. Check if we can fit it */ + if (window_table[0] == 0) + { + window_table[0] = seq; + return 0; + } + + for (i=0;i= last */ { diff --git a/lib/gnutls_record.c b/lib/gnutls_record.c index 17cad7f25..7510fbf1d 100644 --- a/lib/gnutls_record.c +++ b/lib/gnutls_record.c @@ -669,6 +669,12 @@ record_add_to_buffers (gnutls_session_t session, */ if (IS_DTLS(session)) { + if (type == GNUTLS_CHANGE_CIPHER_SPEC) + { + ret = gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET); + goto unexpected_packet; + } + if (_dtls_is_async(session) && _dtls_async_timer_active(session)) { ret = _dtls_retransmit(session); @@ -996,6 +1002,8 @@ begin: if (ret < 0) { gnutls_assert(); + _gnutls_audit_log(session, "Discarded message[%u] due to invalid decryption\n", + (unsigned int)_gnutls_uint64touint32 (packet_sequence)); goto sanity_check_error; } @@ -1008,8 +1016,8 @@ begin: ret = _dtls_record_check(session, packet_sequence); if (ret < 0) { - _gnutls_audit_log(session, "Discarded duplicate message[%u]\n", - (unsigned int) _gnutls_uint64touint32 (packet_sequence)); + _gnutls_audit_log(session, "Discarded duplicate message[%u]: %s\n", + (unsigned int) _gnutls_uint64touint32 (packet_sequence), _gnutls_packet2str (record.type)); goto sanity_check_error; } } @@ -1077,8 +1085,6 @@ sanity_check_error: if (IS_DTLS(session)) { session->internals.dtls.packets_dropped++; - _gnutls_audit_log(session, "Discarded message[%u] due to invalid decryption\n", - (unsigned int)_gnutls_uint64touint32 (packet_sequence)); ret = gnutls_assert_val(GNUTLS_E_AGAIN); goto cleanup; } -- 2.11.4.GIT