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>

diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index 2352299cd8a9c92e26f63a21d7d129bbe7a28fc0..a340d3747d523dcf0ea49a42e0a2d03f8ad7e33c 100644 (file)
--- 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 fedef6cbbc0bc671662127563a655269944942cd..8f9ec7842c11aaf776a9841181857ee683ee167c 100644 (file)
--- 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 481210ebc0e29e19231d0bf164b9ee9b7f6cf809..c6762076aa4c4b03f07d06577bd6ac642df6abdc 100644 (file)
--- 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 272ac431b7ba0222da7259e151ae2e24e0b234a6..e7f8e4f01a028448144a04cf3cb10d409aee973f 100644 (file)
--- 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 f4ab818ca307c75be5fb62ac90f64a4acb3ecaa2..a00bd34ed9115935e05917d4e0c5af18137c5827 100644 (file)
--- 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);