_gnutls13_recv_async_handshake: process multiple and split handshake messagestmp-fix-multi-async-msg

It is permitted to concatenate multiple async handshake messages in a single
record message as well as split large messages (NST) into multiple records.
Modified _gnutls13_recv_async_handshake() to process them correctly, instead
of assuming that they are formatted as one message per record.

Resolves #510
Resolves #504

Relates #511

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

5 files changed, 145 insertions, 84 deletions

diff --git a/lib/buffers.c b/lib/buffers.c
index 1ea55e33c4..100390b5dc 100644
--- a/lib/buffers.c
+++ b/lib/buffers.c
@@ -1086,6 +1086,9 @@ static int merge_handshake_packet(gnutls_session_t session,
 inline static int cmp_hsk_types(gnutls_handshake_description_t expected,
 				gnutls_handshake_description_t recvd)
 {
+	if (expected == GNUTLS_HANDSHAKE_ANY)
+		return 1;
+
 #ifdef ENABLE_SSL2
 	if (expected == GNUTLS_HANDSHAKE_CLIENT_HELLO
 	     && recvd == GNUTLS_HANDSHAKE_CLIENT_HELLO_V2)
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index 99287b3726..6525282a69 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -300,7 +300,8 @@ typedef enum recv_state_t {
 	RECV_STATE_0 = 0,
 	RECV_STATE_DTLS_RETRANSMIT,
 	RECV_STATE_FALSE_START_HANDLING, /* we are calling gnutls_handshake() within record_recv() */
-	RECV_STATE_FALSE_START /* gnutls_record_recv() should complete the handshake */
+	RECV_STATE_FALSE_START, /* gnutls_record_recv() should complete the handshake */
+	RECV_STATE_ASYNC_HANDSHAKE /* an incomplete async handshake message was seen */
 } recv_state_t;
 
 #include "str.h"
diff --git a/lib/handshake-tls13.c b/lib/handshake-tls13.c
index 631665d7a7..ffd1b1531d 100644
--- a/lib/handshake-tls13.c
+++ b/lib/handshake-tls13.c
@@ -412,94 +412,136 @@ int _gnutls13_handshake_server(gnutls_session_t session)
 
 /* Processes handshake messages received asynchronously after initial handshake.
  *
- * It is called once per message, with a read-only buffer in @buf,
- * and should return success, or a fatal error code.
+ * It is called once per message and should return success, or a fatal error code.
  */
 int
-_gnutls13_recv_async_handshake(gnutls_session_t session, gnutls_buffer_st *buf)
+_gnutls13_recv_async_handshake(gnutls_session_t session)
 {
-	uint8_t type;
 	int ret;
-	size_t handshake_header_size = HANDSHAKE_HEADER_SIZE(session);
-	size_t length;
-
-	if (buf->length < handshake_header_size) {
-		gnutls_assert();
-		return GNUTLS_E_UNEXPECTED_PACKET_LENGTH;
-	}
+	handshake_buffer_st hsk;
 
 	/* The following messages are expected asynchronously after
 	 * the handshake process is complete */
 	if (unlikely(session->internals.handshake_in_progress))
 		return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
 
-	ret = _gnutls_buffer_pop_prefix8(buf, &type, 0);
-	if (ret < 0)
-		return gnutls_assert_val(ret);
+	do {
+		/* the received handshake message has already been pushed into
+		 * handshake buffers. As we do not need to use the handshake hash
+		 * buffers we call the lower level receive functions */
+		ret = _gnutls_handshake_io_recv_int(session, GNUTLS_HANDSHAKE_ANY, &hsk, 0);
+		if (ret < 0) {
+			gnutls_assert();
+			goto cleanup;
+		}
+		session->internals.last_handshake_in = hsk.htype;
 
-	ret = _gnutls_buffer_pop_prefix24(buf, &length, 1);
-	if (ret < 0)
-		return gnutls_assert_val(ret);
+		ret = _gnutls_call_hook_func(session, hsk.htype, GNUTLS_HOOK_PRE, 1,
+					     hsk.data.data, hsk.data.length);
+		if (ret < 0) {
+			gnutls_assert();
+			goto cleanup;
+		}
 
-	ret = _gnutls_call_hook_func(session, type, GNUTLS_HOOK_PRE, 1, buf->data, buf->length);
-	if (ret < 0)
-		return gnutls_assert_val(ret);
+		switch(hsk.htype) {
+			case GNUTLS_HANDSHAKE_CERTIFICATE_REQUEST:
+				if (!(session->security_parameters.entity == GNUTLS_CLIENT) ||
+				    !(session->internals.flags & GNUTLS_POST_HANDSHAKE_AUTH)) {
+					ret = gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
+					goto cleanup;
+				}
+
+				_gnutls_buffer_reset(&session->internals.reauth_buffer);
+
+				/* include the handshake headers in reauth buffer */
+				ret = _gnutls_buffer_append_data(&session->internals.reauth_buffer,
+								 hsk.header, hsk.header_size);
+				if (ret < 0) {
+					gnutls_assert();
+					goto cleanup;
+				}
+
+				ret = _gnutls_buffer_append_data(&session->internals.reauth_buffer,
+								 hsk.data.data, hsk.data.length);
+				if (ret < 0) {
+					gnutls_assert();
+					goto cleanup;
+				}
+
+				/* Application is expected to handle re-authentication
+				 * explicitly.  */
+				ret = GNUTLS_E_REAUTH_REQUEST;
+				goto cleanup;
+
+			case GNUTLS_HANDSHAKE_KEY_UPDATE:
+				ret = _gnutls13_recv_key_update(session, &hsk.data);
+				if (ret < 0) {
+					gnutls_assert();
+					goto cleanup;
+				}
+
+				/* Handshake messages MUST NOT span key changes, i.e., we
+				 * should not have any other pending handshake messages from
+				 * the same record. */
+				if (session->internals.handshake_recv_buffer_size != 0) {
+					ret = gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
+					goto cleanup;
+				}
+				break;
+			case GNUTLS_HANDSHAKE_NEW_SESSION_TICKET:
+				if (session->security_parameters.entity != GNUTLS_CLIENT) {
+					ret = gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
+					goto cleanup;
+				}
+
+				ret = _gnutls13_recv_session_ticket(session, &hsk.data);
+				if (ret < 0) {
+					gnutls_assert();
+					goto cleanup;
+				}
+
+				memcpy(session->internals.tls13_ticket.resumption_master_secret,
+				       session->key.proto.tls13.ap_rms,
+				       session->key.proto.tls13.temp_secret_size);
+
+				session->internals.tls13_ticket.prf = session->security_parameters.prf;
+				session->internals.hsk_flags |= HSK_TICKET_RECEIVED;
+				break;
+			default:
+				gnutls_assert();
+				ret = GNUTLS_E_UNEXPECTED_PACKET;
+				goto cleanup;
+		}
 
-	switch(type) {
-		case GNUTLS_HANDSHAKE_CERTIFICATE_REQUEST:
-			if (!(session->security_parameters.entity == GNUTLS_CLIENT) ||
-			    !(session->internals.flags & GNUTLS_POST_HANDSHAKE_AUTH)) {
-				return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
-			}
-
-			_gnutls_buffer_reset(&session->internals.reauth_buffer);
-
-			/* include the handshake headers in reauth buffer */
-			ret = _gnutls_buffer_append_data(&session->internals.reauth_buffer,
-							 buf->data-4, buf->length+4);
-			if (ret < 0)
-				return gnutls_assert_val(ret);
-
-			/* Application is expected to handle re-authentication
-			 * explicitly.  */
-			return GNUTLS_E_REAUTH_REQUEST;
-
-		case GNUTLS_HANDSHAKE_KEY_UPDATE:
-			ret = _gnutls13_recv_key_update(session, buf);
-			if (ret < 0)
-				return gnutls_assert_val(ret);
-			break;
-		case GNUTLS_HANDSHAKE_NEW_SESSION_TICKET:
-			if (session->security_parameters.entity != GNUTLS_CLIENT)
-				return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
-
-			ret = _gnutls13_recv_session_ticket(session, buf);
-			if (ret < 0)
-				return gnutls_assert_val(ret);
-
-			memcpy(session->internals.tls13_ticket.resumption_master_secret,
-			       session->key.proto.tls13.ap_rms,
-			       session->key.proto.tls13.temp_secret_size);
-
-			session->internals.tls13_ticket.prf = session->security_parameters.prf;
-			session->internals.hsk_flags |= HSK_TICKET_RECEIVED;
-			break;
-		default:
+		ret = _gnutls_call_hook_func(session, hsk.htype, GNUTLS_HOOK_POST, 1, hsk.data.data, hsk.data.length);
+		if (ret < 0) {
 			gnutls_assert();
-			return GNUTLS_E_UNEXPECTED_PACKET;
-	}
+			goto cleanup;
+		}
+		_gnutls_handshake_buffer_clear(&hsk);
 
-	ret = _gnutls_call_hook_func(session, type, GNUTLS_HOOK_POST, 1, buf->data, buf->length);
-	if (ret < 0)
-		return gnutls_assert_val(ret);
+	} while (_gnutls_record_buffer_get_size(session) > 0);
+
+	session->internals.recv_state = RECV_STATE_0;
 
 	return 0;
+
+ cleanup:
+	/* if we have pending/partial handshake data in buffers, ensure that
+	 * next read will read handshake data */
+	if (_gnutls_record_buffer_get_size(session) > 0)
+		session->internals.recv_state = RECV_STATE_ASYNC_HANDSHAKE;
+	else
+		session->internals.recv_state = RECV_STATE_0;
+
+	_gnutls_handshake_buffer_clear(&hsk);
+	return ret;
 }
 
 /**
  * gnutls_session_ticket_send:
  * @session: is a #gnutls_session_t type.
- * @nr: the number of tickets to send; must be a positive integer
+ * @nr: the number of tickets to send
  * @flags: must be zero
  *
  * Sends a fresh session ticket to the peer. This is relevant only
diff --git a/lib/handshake.h b/lib/handshake.h
index 390f5034f9..248d6a1896 100644
--- a/lib/handshake.h
+++ b/lib/handshake.h
@@ -164,6 +164,6 @@ _gnutls13_recv_hello_retry_request(gnutls_session_t session,
 				   gnutls_buffer_st *buf);
 
 int
-_gnutls13_recv_async_handshake(gnutls_session_t session, gnutls_buffer_st *buf);
+_gnutls13_recv_async_handshake(gnutls_session_t session);
 
 #endif
diff --git a/lib/record.c b/lib/record.c
index ed82db20a5..9b485fd293 100644
--- a/lib/record.c
+++ b/lib/record.c
@@ -790,6 +790,13 @@ record_add_to_buffers(gnutls_session_t session,
 			}
 		}
 
+		/* application data cannot be inserted between (async) handshake
+		 * messages */
+		if (type == GNUTLS_APPLICATION_DATA && session->internals.handshake_recv_buffer_size != 0) {
+			ret = gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET);
+			goto unexpected_packet;
+		}
+
 		_gnutls_record_buffer_put(session, type, seq, bufel);
 
 		/* if we received application data as expected then we
@@ -956,17 +963,14 @@ record_add_to_buffers(gnutls_session_t session,
 
 			/* retrieve async handshake messages */
 			if (ver && ver->tls13_sem) {
-				gnutls_buffer_st buf;
-
-				_gnutls_ro_buffer_from_datum(&buf, &bufel->msg);
-				ret = _gnutls13_recv_async_handshake(session,
-								     &buf);
-				if (ret < 0) {
-					gnutls_assert();
-				} else {
-					ret = GNUTLS_E_AGAIN;
-				}
-				goto cleanup;
+				_gnutls_record_buffer_put(session, recv->type, seq, bufel);
+
+				ret = _gnutls13_recv_async_handshake(session);
+				if (ret < 0)
+					return gnutls_assert_val(ret);
+
+				/* bufel is now accounted */
+				return GNUTLS_E_AGAIN;
 			}
 
 			/* This is legal if HELLO_REQUEST is received - and we are a client.
@@ -1014,13 +1018,14 @@ record_add_to_buffers(gnutls_session_t session,
 
 	return 0;
 
-      unexpected_packet:
+ unexpected_packet:
+
 	if (IS_DTLS(session) && ret != GNUTLS_E_REHANDSHAKE) {
 		_mbuffer_xfree(&bufel);
 		RETURN_DTLS_EAGAIN_OR_TIMEOUT(session, ret);
 	}
 
-      cleanup:
+ cleanup:
 	_mbuffer_xfree(&bufel);
 	return ret;
 
@@ -1491,7 +1496,7 @@ _gnutls_recv_in_buffers(gnutls_session_t session, content_type_t type,
 /* Returns a value greater than zero (>= 0) if buffers should be checked
  * for data. */
 static ssize_t
-check_session_status(gnutls_session_t session)
+check_session_status(gnutls_session_t session, unsigned ms)
 {
 	int ret;
 
@@ -1506,6 +1511,16 @@ check_session_status(gnutls_session_t session)
 	}
 
 	switch (session->internals.recv_state) {
+	case RECV_STATE_ASYNC_HANDSHAKE:
+		ret = _gnutls_recv_in_buffers(session, GNUTLS_HANDSHAKE, -1, ms);
+		if (ret < 0 && ret != GNUTLS_E_SESSION_EOF)
+			return gnutls_assert_val(ret);
+
+		ret = _gnutls13_recv_async_handshake(session);
+		if (ret < 0)
+			return gnutls_assert_val(ret);
+
+		return GNUTLS_E_AGAIN;
 	case RECV_STATE_FALSE_START_HANDLING:
 		return 1;
 	case RECV_STATE_FALSE_START:
@@ -1564,7 +1579,7 @@ _gnutls_recv_int(gnutls_session_t session, content_type_t type,
 	    && (data_size == 0 || data == NULL))
 		return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
 
-	ret = check_session_status(session);
+	ret = check_session_status(session, ms);
 	if (ret <= 0)
 		return ret;
 
@@ -1678,7 +1693,7 @@ gnutls_record_recv_packet(gnutls_session_t session,
 	if (packet == NULL)
 		return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
 
-	ret = check_session_status(session);
+	ret = check_session_status(session, session->internals.record_timeout_ms);
 	if (ret <= 0)
 		return ret;