diff options
author | Nikos Mavrogiannopoulos <nmav@redhat.com> | 2019-02-25 15:11:19 +0100 |
---|---|---|
committer | Nikos Mavrogiannopoulos <nmav@gnutls.org> | 2019-03-02 21:15:26 +0100 |
commit | e53bd8ed2010cc061a9d36cfc36cf8d076894ee1 (patch) | |
tree | c7db69a59f1f656a48114497479cbea38e26b016 | |
parent | 6868c57f0879e32f1a2d9da994bdf165609afc25 (diff) | |
download | gnutls-e53bd8ed2010cc061a9d36cfc36cf8d076894ee1.tar.gz |
gnutls_record_send2: try to ensure integrity of operations on false and early start
This adds a double check in the sanity check of gnutls_record_send2()
for the initial_negotiation_completed value, making sure that the
check will be successful even in parallel operation of send/recv.
Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>
-rw-r--r-- | lib/gnutls_int.h | 3 | ||||
-rw-r--r-- | lib/handshake-tls13.c | 9 | ||||
-rw-r--r-- | lib/handshake.c | 11 | ||||
-rw-r--r-- | lib/record.c | 18 | ||||
-rw-r--r-- | lib/state.c | 10 |
5 files changed, 41 insertions, 10 deletions
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h index 2352299cd8..a340d3747d 100644 --- a/lib/gnutls_int.h +++ b/lib/gnutls_int.h @@ -1281,6 +1281,9 @@ typedef struct { /* A handshake process has been completed */ bool initial_negotiation_completed; + void *post_negotiation_lock; /* protects access to the variable above + * in the cases where negotiation is incomplete + * after gnutls_handshake() - early/false start */ /* The type of transport protocol; stream or datagram */ transport_t transport; diff --git a/lib/handshake-tls13.c b/lib/handshake-tls13.c index fedef6cbbc..8f9ec7842c 100644 --- a/lib/handshake-tls13.c +++ b/lib/handshake-tls13.c @@ -56,6 +56,7 @@ #include "tls13/finished.h" #include "tls13/key_update.h" #include "ext/pre_shared_key.h" +#include "locks.h" static int generate_rms_keys(gnutls_session_t session); static int generate_hs_traffic_keys(gnutls_session_t session); @@ -202,8 +203,8 @@ int _gnutls13_handshake_client(gnutls_session_t session) return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR); } - - /* explicitly reset any false start flags */ + /* no lock of post_negotiation_lock is required here as this is not run + * after handshake */ session->internals.recv_state = RECV_STATE_0; session->internals.initial_negotiation_completed = 1; @@ -577,9 +578,11 @@ int _gnutls13_handshake_server(gnutls_session_t session) return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR); } - /* explicitly reset any false start flags */ + /* explicitly reset any early start flags */ + gnutls_mutex_lock(&session->internals.post_negotiation_lock); session->internals.recv_state = RECV_STATE_0; session->internals.initial_negotiation_completed = 1; + gnutls_mutex_unlock(&session->internals.post_negotiation_lock); SAVE_TRANSCRIPT; diff --git a/lib/handshake.c b/lib/handshake.c index 481210ebc0..c6762076aa 100644 --- a/lib/handshake.c +++ b/lib/handshake.c @@ -55,6 +55,7 @@ #include <dtls.h> #include "secrets.h" #include "tls13/session_ticket.h" +#include "locks.h" #define TRUE 1 #define FALSE 0 @@ -1008,8 +1009,6 @@ int _gnutls_recv_finished(gnutls_session_t session) } - session->internals.initial_negotiation_completed = 1; - cleanup: _gnutls_buffer_clear(&buf); @@ -3144,7 +3143,10 @@ static int handshake_client(gnutls_session_t session) } /* explicitly reset any false start flags */ + gnutls_mutex_lock(&session->internals.post_negotiation_lock); + session->internals.initial_negotiation_completed = 1; session->internals.recv_state = RECV_STATE_0; + gnutls_mutex_unlock(&session->internals.post_negotiation_lock); return 0; } @@ -3363,7 +3365,6 @@ static int recv_handshake_final(gnutls_session_t session, int init) break; } - return 0; } @@ -3560,6 +3561,10 @@ static int handshake_server(gnutls_session_t session) break; } + /* no lock of post_negotiation_lock is required here as this is not run + * after handshake */ + session->internals.initial_negotiation_completed = 1; + return _gnutls_check_id_for_change(session); } diff --git a/lib/record.c b/lib/record.c index 272ac431b7..e7f8e4f01a 100644 --- a/lib/record.c +++ b/lib/record.c @@ -53,6 +53,7 @@ #include <dh.h> #include <random.h> #include <xsize.h> +#include "locks.h" struct tls_record_st { uint16_t header_size; @@ -1986,14 +1987,23 @@ gnutls_record_send2(gnutls_session_t session, const void *data, if (unlikely(!session->internals.initial_negotiation_completed)) { /* this is to protect buggy applications from sending unencrypted - * data. We allow sending however, if we are in false start handshake - * state. */ - if (session->internals.recv_state != RECV_STATE_FALSE_START && + * data. We allow sending however, if we are in false or early start + * handshake state. */ + gnutls_mutex_lock(&session->internals.post_negotiation_lock); + + /* we intentionally re-check the initial_negotation_completed variable + * to avoid locking during normal operation of gnutls_record_send2() */ + if (!session->internals.initial_negotiation_completed && + session->internals.recv_state != RECV_STATE_FALSE_START && session->internals.recv_state != RECV_STATE_FALSE_START_HANDLING && session->internals.recv_state != RECV_STATE_EARLY_START && session->internals.recv_state != RECV_STATE_EARLY_START_HANDLING && - !(session->internals.hsk_flags & HSK_EARLY_DATA_IN_FLIGHT)) + !(session->internals.hsk_flags & HSK_EARLY_DATA_IN_FLIGHT)) { + + gnutls_mutex_unlock(&session->internals.post_negotiation_lock); return gnutls_assert_val(GNUTLS_E_UNAVAILABLE_DURING_HANDSHAKE); + } + gnutls_mutex_unlock(&session->internals.post_negotiation_lock); } if (unlikely(!vers)) diff --git a/lib/state.c b/lib/state.c index f4ab818ca3..a00bd34ed9 100644 --- a/lib/state.c +++ b/lib/state.c @@ -53,6 +53,7 @@ #include "dtls.h" #include "tls13/session_ticket.h" #include "ext/cert_types.h" +#include "locks.h" /* to be used by supplemental data support to disable TLS1.3 * when supplemental data have been globally registered */ @@ -451,6 +452,13 @@ int gnutls_init(gnutls_session_t * session, unsigned int flags) if (*session == NULL) return GNUTLS_E_MEMORY_ERROR; + ret = gnutls_mutex_init(&(*session)->internals.post_negotiation_lock); + if (ret < 0) { + gnutls_assert(); + gnutls_free(*session); + return ret; + } + ret = _gnutls_epoch_setup_next(*session, 1, NULL); if (ret < 0) { gnutls_free(*session); @@ -591,6 +599,8 @@ void gnutls_deinit(gnutls_session_t session) if (session == NULL) return; + gnutls_mutex_deinit(&session->internals.post_negotiation_lock); + /* remove auth info firstly */ _gnutls_free_auth_info(session); |