Merge branch 'tmp-handshake-interleave' into 'master'

Fix interleaved handshake handling in TLS 1.3

Closes #272

See merge request gnutls/gnutls!708

9 files changed, 93 insertions, 47 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index aa06a55cd5..116f600fc1 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -211,6 +211,7 @@ asan.Fedora.x86_64:
     when: on_failure
     paths:
       - ./*.log
+      - fuzz/*.log
       - tests/*.log
       - tests/*/*.log
       - tests/suite/*/*.log
diff --git a/lib/buffers.c b/lib/buffers.c
index 100390b5dc..cee0d5fc59 100644
--- a/lib/buffers.c
+++ b/lib/buffers.c
@@ -882,7 +882,7 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel,
 {
 	uint8_t *dataptr = NULL;	/* for realloc */
 	size_t handshake_header_size =
-	    HANDSHAKE_HEADER_SIZE(session), data_size;
+	    HANDSHAKE_HEADER_SIZE(session), data_size, frag_size;
 
 	/* Note: SSL2_HEADERS == 1 */
 	if (_mbuffer_get_udata_size(bufel) < handshake_header_size)
@@ -898,7 +898,7 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel,
 	     && bufel->htype == GNUTLS_HANDSHAKE_CLIENT_HELLO_V2)) {
 		handshake_header_size = SSL2_HEADERS;	/* we've already read one byte */
 
-		hsk->length = _mbuffer_get_udata_size(bufel) - handshake_header_size;	/* we've read the first byte */
+		frag_size = _mbuffer_get_udata_size(bufel) - handshake_header_size;	/* we've read the first byte */
 
 		if (dataptr[0] != GNUTLS_HANDSHAKE_CLIENT_HELLO)
 			return
@@ -908,7 +908,7 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel,
 
 		hsk->sequence = 0;
 		hsk->start_offset = 0;
-		hsk->end_offset = hsk->length;
+		hsk->length = frag_size;
 	} else
 #endif
 	{	/* TLS or DTLS handshake headers */
@@ -925,13 +925,12 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel,
 			hsk->sequence = _gnutls_read_uint16(&dataptr[4]);
 			hsk->start_offset =
 			    _gnutls_read_uint24(&dataptr[6]);
-			hsk->end_offset =
-			    hsk->start_offset +
+			frag_size =
 			    _gnutls_read_uint24(&dataptr[9]);
 		} else {
 			hsk->sequence = 0;
 			hsk->start_offset = 0;
-			hsk->end_offset =
+			frag_size =
 			    MIN((_mbuffer_get_udata_size(bufel) -
 				 handshake_header_size), hsk->length);
 		}
@@ -947,25 +946,25 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel,
 	}
 	data_size = _mbuffer_get_udata_size(bufel) - handshake_header_size;
 
-	/* make the length offset */
-	if (hsk->end_offset > 0)
-		hsk->end_offset--;
+	if (frag_size > 0)
+		hsk->end_offset = hsk->start_offset + frag_size - 1;
+	else
+		hsk->end_offset = 0;
 
 	_gnutls_handshake_log
 	    ("HSK[%p]: %s (%u) was received. Length %d[%d], frag offset %d, frag length: %d, sequence: %d\n",
 	     session, _gnutls_handshake2str(hsk->htype),
 	     (unsigned) hsk->htype, (int) hsk->length, (int) data_size,
-	     hsk->start_offset, hsk->end_offset - hsk->start_offset + 1,
+	     hsk->start_offset, (int) frag_size,
 	     (int) hsk->sequence);
 
 	hsk->header_size = handshake_header_size;
 	memcpy(hsk->header, _mbuffer_get_udata_ptr(bufel),
 	       handshake_header_size);
 
-	if (hsk->length > 0 && (hsk->start_offset > hsk->end_offset ||
-				hsk->end_offset - hsk->start_offset >=
-				data_size
-				|| hsk->end_offset >= hsk->length)) {
+	if (hsk->length > 0 && (frag_size > data_size ||
+				(frag_size > 0 &&
+				 hsk->end_offset >= hsk->length))) {
 		return
 		    gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET_LENGTH);
 	}
@@ -1197,7 +1196,7 @@ int _gnutls_parse_record_buffered_msgs(gnutls_session_t session)
 		return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
 	if (!IS_DTLS(session)) {
-		ssize_t remain, append, header_size;
+		ssize_t append, header_size;
 
 		do {
 			if (bufel->type != GNUTLS_HANDSHAKE)
@@ -1205,18 +1204,21 @@ int _gnutls_parse_record_buffered_msgs(gnutls_session_t session)
 				    gnutls_assert_val
 				    (GNUTLS_E_UNEXPECTED_PACKET);
 
-			/* if we have a half received message then complete it.
-			 */
-			remain = recv_buf[0].length -
-			    recv_buf[0].data.length;
-
-			/* this is the rest of a previous message */
-			if (session->internals.handshake_recv_buffer_size >
-			    0 && recv_buf[0].length > 0 && remain > 0) {
-				if ((ssize_t) msg.size <= remain)
-					append = msg.size;
-				else
-					append = remain;
+			if (unlikely
+			    (session->internals.handshake_recv_buffer_size == 0 &&
+			     msg.size < HANDSHAKE_HEADER_SIZE(session) &&
+			     session->internals.handshake_header_recv_buffer.byte_length <
+			     HANDSHAKE_HEADER_SIZE(session) - msg.size)) {
+				bufel = _mbuffer_head_pop_first(&session->internals.record_buffer);
+				_mbuffer_enqueue(&session->internals.handshake_header_recv_buffer,
+						 bufel);
+				break;
+			} else if (session->internals.handshake_recv_buffer_size >
+				   0 && recv_buf[0].length > recv_buf[0].data.length) {
+				/* this is the rest of a previous message */
+				append = MIN(msg.size,
+					     recv_buf[0].length -
+					     recv_buf[0].data.length);
 
 				ret =
 				    _gnutls_buffer_append_data(&recv_buf
@@ -1231,6 +1233,25 @@ int _gnutls_parse_record_buffered_msgs(gnutls_session_t session)
 							   record_buffer,
 							   append);
 			} else {	/* received new message */
+				if (unlikely
+				    (session->internals.
+				     handshake_header_recv_buffer.length > 0)) {
+					bufel = _mbuffer_head_pop_first(&session->internals.
+									record_buffer);
+					_mbuffer_enqueue(&session->internals.
+							 handshake_header_recv_buffer,
+							 bufel);
+					ret = _mbuffer_linearize_align16(&session->internals.
+									 handshake_header_recv_buffer,
+									 get_total_headers(session));
+					if (ret < 0)
+						return gnutls_assert_val(ret);
+					bufel = _mbuffer_head_pop_first(&session->internals.
+									handshake_header_recv_buffer);
+					_mbuffer_head_push_first(&session->internals.
+								 record_buffer,
+								 bufel);
+				}
 
 				ret =
 				    parse_handshake_header(session, bufel,
diff --git a/lib/buffers.h b/lib/buffers.h
index e03c45d40d..e76c452e43 100644
--- a/lib/buffers.h
+++ b/lib/buffers.h
@@ -22,6 +22,8 @@
 #ifndef GNUTLS_BUFFERS_H
 #define GNUTLS_BUFFERS_H
 
+#include "mbuffers.h"
+
 #define MBUFFER_FLUSH 1
 
 void
@@ -98,6 +100,7 @@ inline static void _gnutls_handshake_recv_buffer_clear(gnutls_session_t
 		_gnutls_handshake_buffer_clear(&session->internals.
 					       handshake_recv_buffer[i]);
 	session->internals.handshake_recv_buffer_size = 0;
+	_mbuffer_head_clear(&session->internals.handshake_header_recv_buffer);
 }
 
 inline static void _gnutls_handshake_recv_buffer_init(gnutls_session_t
@@ -109,6 +112,7 @@ inline static void _gnutls_handshake_recv_buffer_init(gnutls_session_t
 					      handshake_recv_buffer[i]);
 	}
 	session->internals.handshake_recv_buffer_size = 0;
+	_mbuffer_head_init(&session->internals.handshake_header_recv_buffer);
 }
 
 int _gnutls_parse_record_buffered_msgs(gnutls_session_t session);
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index fc60f7cc3e..412bb368a1 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -1097,6 +1097,7 @@ typedef struct {
 	 * protocol only. freed using _gnutls_handshake_io_buffer_clear();
 	 */
 	mbuffer_head_st handshake_send_buffer;
+	mbuffer_head_st handshake_header_recv_buffer;
 	handshake_buffer_st handshake_recv_buffer[MAX_HANDSHAKE_MSGS];
 	int handshake_recv_buffer_size;
 
diff --git a/lib/mbuffers.c b/lib/mbuffers.c
index 2bcd57e426..2e4c62a295 100644
--- a/lib/mbuffers.c
+++ b/lib/mbuffers.c
@@ -126,6 +126,25 @@ mbuffer_st *_mbuffer_dequeue(mbuffer_head_st * buf, mbuffer_st * bufel)
 	return ret;
 }
 
+/* Append a segment to the beginning of this buffer.
+ *
+ * Cost: O(1)
+ */
+void _mbuffer_head_push_first(mbuffer_head_st * buf, mbuffer_st * bufel)
+{
+	bufel->prev = NULL;
+	bufel->next = buf->head;
+
+	buf->length++;
+	buf->byte_length += bufel->msg.size - bufel->mark;
+
+	if (buf->head != NULL)
+		buf->head->prev = bufel;
+	else
+		buf->tail = bufel;
+	buf->head = bufel;
+}
+
 /* Get a reference to the first segment of the buffer and
  * remove it from the list.
  *
diff --git a/lib/mbuffers.h b/lib/mbuffers.h
index 034f8a3cb9..bbbe079daf 100644
--- a/lib/mbuffers.h
+++ b/lib/mbuffers.h
@@ -39,6 +39,8 @@ mbuffer_st *_mbuffer_head_get_first(mbuffer_head_st * buf,
 				    gnutls_datum_t * msg);
 mbuffer_st *_mbuffer_head_get_next(mbuffer_st * cur, gnutls_datum_t * msg);
 
+void _mbuffer_head_push_first(mbuffer_head_st * buf, mbuffer_st * bufel);
+
 mbuffer_st *_mbuffer_head_pop_first(mbuffer_head_st * buf);
 
 /* This is dangerous since it will replace bufel with a new
diff --git a/lib/record.c b/lib/record.c
index 4589765524..96bf5736a9 100644
--- a/lib/record.c
+++ b/lib/record.c
@@ -1190,8 +1190,15 @@ static int recv_headers(gnutls_session_t session,
 		    (session, "Received packet with illegal length: %u\n",
 		     (unsigned int) record->length);
 
-		if (record->length == 0)
+		if (record->length == 0) {
+			/* Empty, unencrypted records are always unexpected. */
+			if (record_params->cipher->id == GNUTLS_CIPHER_NULL)
+				return
+				    gnutls_assert_val
+				    (GNUTLS_E_UNEXPECTED_PACKET);
+
 			return gnutls_assert_val(GNUTLS_E_DECRYPTION_FAILED);
+		}
 		return
 		    gnutls_assert_val(GNUTLS_E_RECORD_OVERFLOW);
 	}
diff --git a/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json b/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json
index db351652bb..4b01fe4531 100644
--- a/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json
+++ b/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json
@@ -43,12 +43,9 @@
 	 {"name" : "test-tls13-version-negotiation.py",
 	  "arguments": ["-p", "@PORT@"]},
 	 {"name" : "test-tls13-zero-length-data.py",
-	  "comment" : "in these tests tlsfuzzer splits ClientHello into the first 2 bytes and the remainder, which gnutls doesn't support",
+	  "comment" : "gnutls sends NST before receiving client Finished, that is not expected in the disabled test",
 	  "arguments": ["-p", "@PORT@",
-			"-e", "zero-length app data interleaved in handshake",
-			"-e", "zero-len app data with large padding during handshake",
-			"-e", "zero-len app data with large padding interleaved in handshake",
-			"-e", "zero-len app data with padding interleaved in handshake"]},
+			"-e", "zero-len app data with large padding during handshake"]},
 	 {"name" : "test-tls13-finished.py",
 	  "commoent" : "the disabled tests timeout very often due to slow tls-fuzzer implementation",
 	  "arguments": ["-p", "@PORT@", "-n", "5",
diff --git a/tests/suite/tls-fuzzer/gnutls-nocert.json b/tests/suite/tls-fuzzer/gnutls-nocert.json
index c69ecfaf7e..d99c17414b 100644
--- a/tests/suite/tls-fuzzer/gnutls-nocert.json
+++ b/tests/suite/tls-fuzzer/gnutls-nocert.json
@@ -17,20 +17,14 @@
           "ciphers even 8199",
           "ciphers odd 8090",
           "ext padding, 16130 bytes",
-          "ext padding, 65367 bytes"]},
-         {"name" : "test-large-hello.py",
-          "arguments" :
-                 ["multiple extensions 9212",
-                  "multiple extensions 1",
-                  "multiple extensions 16353"]},
-         {"name" : "test-large-hello.py",
-          "comment" : "These tests rely on fragmenting the first bytes of the handshake header. Gnutls is limited on that, and doesn't accept handshake header fragmentation.",
-          "arguments" :
-	          ["sanity check - fragmented",
-	           "fragmented, padding ext 0 bytes",
-	           "fragmented, padding ext 65354 bytes",
-	           "fragmented, padding ext 16213 bytes"],
-          "exp_pass" : false},
+          "ext padding, 65367 bytes",
+          "multiple extensions 9212",
+          "multiple extensions 1",
+          "multiple extensions 16353",
+          "sanity check - fragmented",
+          "fragmented, padding ext 0 bytes",
+          "fragmented, padding ext 65354 bytes",
+          "fragmented, padding ext 16213 bytes"]},
          {"name" : "test-ecdsa-sig-flexibility.py"},
          {"name" : "test-ocsp-stapling.py",
           "arguments" : ["--no-status"] },