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>

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);