Merge branch 'tmp-psk-ext-refactor' into 'master'

tls13/psk_ext_parser: simplify the iterator interface

See merge request gnutls/gnutls!736

9 files changed, 150 insertions, 139 deletions

diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index f3bd9c5973..c6bb20c688 100644
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -201,8 +201,8 @@ client_send_params(gnutls_session_t session,
 	unsigned next_idx;
 	const mac_entry_st *prf_res = NULL;
 	const mac_entry_st *prf_psk = NULL;
-	time_t cur_time, ticket_age;
-	uint32_t ob_ticket_age;
+	time_t cur_time;
+	uint32_t ticket_age, ob_ticket_age;
 	int free_username = 0;
 	psk_auth_info_t info = NULL;
 	unsigned psk_id_len = 0;
@@ -235,16 +235,16 @@ client_send_params(gnutls_session_t session,
 
 		prf_res = session->internals.tls13_ticket.prf;
 
-		/* Check whether the ticket is stale */
 		cur_time = gnutls_time(0);
-		ticket_age = cur_time - session->internals.tls13_ticket.timestamp;
-		if (ticket_age < 0 || ticket_age > cur_time) {
+		if (unlikely(cur_time < session->internals.tls13_ticket.timestamp)) {
 			gnutls_assert();
 			_gnutls13_session_ticket_unset(session);
 			goto ignore_ticket;
 		}
 
-		if ((unsigned int) ticket_age > session->internals.tls13_ticket.lifetime) {
+		/* Check whether the ticket is stale */
+		ticket_age = cur_time - session->internals.tls13_ticket.timestamp;
+		if (ticket_age > session->internals.tls13_ticket.lifetime) {
 			_gnutls13_session_ticket_unset(session);
 			goto ignore_ticket;
 		}
@@ -469,24 +469,35 @@ static int server_recv_params(gnutls_session_t session,
 	const mac_entry_st *prf;
 	gnutls_datum_t full_client_hello;
 	uint8_t binder_value[MAX_HASH_SIZE];
-	int psk_index;
+	uint16_t psk_index, i;
 	gnutls_datum_t binder_recvd = { NULL, 0 };
 	gnutls_datum_t key = {NULL, 0};
 	psk_ext_parser_st psk_parser;
+	psk_ext_iter_st psk_iter;
 	struct psk_st psk;
 	psk_auth_info_t info;
 	tls13_ticket_t ticket_data;
-	time_t ticket_age;
+	uint32_t ticket_age;
 	bool resuming;
 
 	ret = _gnutls13_psk_ext_parser_init(&psk_parser, data, len);
 	if (ret < 0) {
-		if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) /* No PSKs advertised by client */
+		/* No PSKs advertised by client */
+		if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE)
 			return 0;
 		return gnutls_assert_val(ret);
 	}
 
-	while ((psk_index = _gnutls13_psk_ext_parser_next_psk(&psk_parser, &psk)) >= 0) {
+	_gnutls13_psk_ext_iter_init(&psk_iter, &psk_parser);
+	for (psk_index = 0; ; psk_index++) {
+		ret = _gnutls13_psk_ext_iter_next_identity(&psk_iter, &psk);
+		if (ret < 0) {
+			/* We couldn't find any usable PSK */
+			if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE)
+				return 0;
+			return gnutls_assert_val(ret);
+		}
+
 		/* This will unpack the session ticket if it is well
 		 * formed and has the expected name */
 		if (!(session->internals.flags & GNUTLS_NO_TICKETS) &&
@@ -496,19 +507,22 @@ static int server_recv_params(gnutls_session_t session,
 			session->internals.resumption_requested = 1;
 
 			/* Check whether ticket is stale or not */
-			ticket_age = psk.ob_ticket_age - ticket_data.age_add;
-			if (ticket_age < 0) {
+			if (psk.ob_ticket_age < ticket_data.age_add) {
+				gnutls_assert();
 				tls13_ticket_deinit(&ticket_data);
 				continue;
 			}
 
-			if ((unsigned int) (ticket_age / 1000) > ticket_data.lifetime) {
+			ticket_age = psk.ob_ticket_age - ticket_data.age_add;
+			if (ticket_age / 1000 > ticket_data.lifetime) {
+				gnutls_assert();
 				tls13_ticket_deinit(&ticket_data);
 				continue;
 			}
 
 			ret = compute_psk_from_ticket(&ticket_data, &key);
 			if (ret < 0) {
+				gnutls_assert();
 				tls13_ticket_deinit(&ticket_data);
 				continue;
 			}
@@ -539,14 +553,16 @@ static int server_recv_params(gnutls_session_t session,
 		}
 	}
 
-	if (psk_index < 0)
-		return 0;
-
-	ret = _gnutls13_psk_ext_parser_find_binder(&psk_parser, psk_index,
-						   &binder_recvd);
-	if (ret < 0) {
-		gnutls_assert();
-		goto fail;
+	_gnutls13_psk_ext_iter_init(&psk_iter, &psk_parser);
+	for (i = 0; i <= psk_index; i++) {
+		ret = _gnutls13_psk_ext_iter_next_binder(&psk_iter, &binder_recvd);
+		if (ret < 0) {
+			gnutls_assert();
+			/* We couldn't extract binder */
+			if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE)
+				ret = GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER;
+			goto fail;
+		}
 	}
 
 	/* Get full ClientHello */
@@ -557,7 +573,7 @@ static int server_recv_params(gnutls_session_t session,
 	}
 
 	/* Compute the binder value for this PSK */
-	ret = compute_psk_binder(session, prf, psk_parser.binder_len+2, 0, 0,
+	ret = compute_psk_binder(session, prf, psk_parser.binders_len+2, 0, 0,
 				 &key, &full_client_hello, resuming,
 				 binder_value);
 	if (ret < 0) {
@@ -582,11 +598,7 @@ static int server_recv_params(gnutls_session_t session,
 	/* save the username in psk_auth_info to make it available
 	 * using gnutls_psk_server_get_username() */
 	if (!resuming) {
-		if (psk.identity.size >= sizeof(info->username)) {
-			gnutls_assert();
-			ret = GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER;
-			goto fail;
-		}
+		assert(psk.identity.size < sizeof(info->username));
 
 		ret = _gnutls_auth_info_set(session, GNUTLS_CRD_PSK, sizeof(psk_auth_info_st), 1);
 		if (ret < 0) {
@@ -756,7 +768,7 @@ static int _gnutls_psk_recv_params(gnutls_session_t session,
 	}
 }
 
-const hello_ext_entry_st ext_pre_shared_key = {
+const hello_ext_entry_st ext_mod_pre_shared_key = {
 	.name = "Pre Shared Key",
 	.tls_id = PRE_SHARED_KEY_TLS_ID,
 	.gid = GNUTLS_EXTENSION_PRE_SHARED_KEY,
diff --git a/lib/ext/pre_shared_key.h b/lib/ext/pre_shared_key.h
index 1168750656..4ad7b240f3 100644
--- a/lib/ext/pre_shared_key.h
+++ b/lib/ext/pre_shared_key.h
@@ -7,7 +7,7 @@
 
 #define PRE_SHARED_KEY_TLS_ID 41
 
-extern const hello_ext_entry_st ext_pre_shared_key;
+extern const hello_ext_entry_st ext_mod_pre_shared_key;
 
 inline static
 unsigned _gnutls_have_psk_credentials(const gnutls_psk_client_credentials_t cred, gnutls_session_t session)
diff --git a/lib/ext/psk_ke_modes.c b/lib/ext/psk_ke_modes.c
index dacfba7ef6..a2fa7377e7 100644
--- a/lib/ext/psk_ke_modes.c
+++ b/lib/ext/psk_ke_modes.c
@@ -194,7 +194,7 @@ psk_ke_modes_recv_params(gnutls_session_t session,
 	}
 }
 
-const hello_ext_entry_st ext_psk_ke_modes = {
+const hello_ext_entry_st ext_mod_psk_ke_modes = {
 	.name = "PSK Key Exchange Modes",
 	.tls_id = 45,
 	.gid = GNUTLS_EXTENSION_PSK_KE_MODES,
diff --git a/lib/ext/psk_ke_modes.h b/lib/ext/psk_ke_modes.h
index bd06139ff5..82c2c10bba 100644
--- a/lib/ext/psk_ke_modes.h
+++ b/lib/ext/psk_ke_modes.h
@@ -3,6 +3,6 @@
 
 #include <hello_ext.h>
 
-extern const hello_ext_entry_st ext_psk_ke_modes;
+extern const hello_ext_entry_st ext_mod_psk_ke_modes;
 
 #endif
diff --git a/lib/hello_ext.c b/lib/hello_ext.c
index 28e8e8c08e..c4907aaced 100644
--- a/lib/hello_ext.c
+++ b/lib/hello_ext.c
@@ -95,8 +95,8 @@ static hello_ext_entry_st const *extfunc[MAX_EXT_TYPES+1] = {
 #ifdef ENABLE_ALPN
 	[GNUTLS_EXTENSION_ALPN] = &ext_mod_alpn,
 #endif
-	[GNUTLS_EXTENSION_PSK_KE_MODES] = &ext_psk_ke_modes,
-	[GNUTLS_EXTENSION_PRE_SHARED_KEY] = &ext_pre_shared_key,
+	[GNUTLS_EXTENSION_PSK_KE_MODES] = &ext_mod_psk_ke_modes,
+	[GNUTLS_EXTENSION_PRE_SHARED_KEY] = &ext_mod_pre_shared_key,
 	[GNUTLS_EXTENSION_RECORD_SIZE_LIMIT] = &ext_mod_record_size_limit,
 	/* This must be the last extension registered.
 	 */
diff --git a/lib/libgnutls.map b/lib/libgnutls.map
index c20dd33cfe..dd77025f07 100644
--- a/lib/libgnutls.map
+++ b/lib/libgnutls.map
@@ -1290,8 +1290,8 @@ GNUTLS_PRIVATE_3_4 {
 
 	# PSK extension parser unit test
 	_gnutls13_psk_ext_parser_init;
-	_gnutls13_psk_ext_parser_next_psk;
-	_gnutls13_psk_ext_parser_find_binder;
+	_gnutls13_psk_ext_iter_next_identity;
+	_gnutls13_psk_ext_iter_next_binder;
 
 	# Internal symbols needed by gnutls-cli-debug:
 	_gnutls_rsa_pms_set_version;
diff --git a/lib/tls13/psk_ext_parser.c b/lib/tls13/psk_ext_parser.c
index 9f4087773d..b588d6668e 100644
--- a/lib/tls13/psk_ext_parser.c
+++ b/lib/tls13/psk_ext_parser.c
@@ -25,12 +25,11 @@
 #include "tls13/psk_ext_parser.h"
 
 /* Returns GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE when no identities
- * are present, or >= 0, on success.
+ * are present, or 0, on success.
  */
 int _gnutls13_psk_ext_parser_init(psk_ext_parser_st *p,
 				  const unsigned char *data, size_t _len)
 {
-	uint16_t identities_len;
 	ssize_t len = _len;
 
 	if (!p || !data || !len)
@@ -39,112 +38,76 @@ int _gnutls13_psk_ext_parser_init(psk_ext_parser_st *p,
 	memset(p, 0, sizeof(*p));
 
 	DECR_LEN(len, 2);
-	identities_len = _gnutls_read_uint16(data);
+	p->identities_len = _gnutls_read_uint16(data);
 	data += 2;
 
-	if (identities_len == 0)
+	if (p->identities_len == 0)
 		return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
 
-	p->id_len = identities_len;
-	p->data = (unsigned char *) data;
-	p->len = len;
+	p->identities_data = (unsigned char *) data;
 
-	DECR_LEN(len, p->id_len);
-	data += p->id_len;
+	DECR_LEN(len, p->identities_len);
+	data += p->identities_len;
 
 	DECR_LEN(len, 2);
-	p->binder_len = _gnutls_read_uint16(data);
+	p->binders_len = _gnutls_read_uint16(data);
+	data += 2;
 
-	p->binder_data = p->data + p->id_len + 2;
-	DECR_LEN(len, p->binder_len);
+	p->binders_data = data;
+	DECR_LEN(len, p->binders_len);
 
 	return 0;
 }
 
-int _gnutls13_psk_ext_parser_next_psk(psk_ext_parser_st *p, struct psk_st *psk)
+/* Extract PSK identity and move to the next iteration.
+ *
+ * Returns GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE when no more identities
+ * are present, or 0, on success.
+ */
+int _gnutls13_psk_ext_iter_next_identity(psk_ext_iter_st *iter,
+					 struct psk_st *psk)
 {
-	if (p->id_read >= p->id_len)
+	if (iter->identities_len == 0)
 		return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
-	/* Read a PskIdentity structure */
-	DECR_LEN(p->len, 2);
-	psk->identity.size = _gnutls_read_uint16(p->data);
+	DECR_LEN(iter->identities_len, 2);
+	psk->identity.size = _gnutls_read_uint16(iter->identities_data);
 	if (psk->identity.size == 0)
-		return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
-
-	p->data += 2;
-	p->id_read += 2;
-
-	psk->identity.data = (void*)p->data;
+		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 
-	DECR_LEN(p->len, psk->identity.size);
-	p->data += psk->identity.size;
-	p->id_read += psk->identity.size;
+	iter->identities_data += 2;
+	psk->identity.data = (void*)iter->identities_data;
 
-	DECR_LEN(p->len, 4);
-	psk->ob_ticket_age = _gnutls_read_uint32(p->data);
+	DECR_LEN(iter->identities_len, psk->identity.size);
+	iter->identities_data += psk->identity.size;
 
-	p->data += 4;
-	p->id_read += 4;
+	DECR_LEN(iter->identities_len, 4);
+	psk->ob_ticket_age = _gnutls_read_uint32(iter->identities_data);
+	iter->identities_data += 4;
 
-	return p->next_index++;
+	return 0;
 }
 
-/* Output is a pointer to data, which shouldn't be de-allocated. */
-int _gnutls13_psk_ext_parser_find_binder(psk_ext_parser_st *p, int psk_index,
-					 gnutls_datum_t *binder_out)
+/* Extract PSK binder and move to the next iteration.
+ *
+ * Returns GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE when no more identities
+ * are present, or 0, on success.
+ */
+int _gnutls13_psk_ext_iter_next_binder(psk_ext_iter_st *iter,
+				       gnutls_datum_t *binder)
 {
-	uint8_t binder_len;
-	int cur_index = 0, binder_found = 0;
-	ssize_t len;
-	const uint8_t *data;
-	ssize_t read_data = 0;
-
-	if (p == NULL || psk_index < 0 || binder_out == NULL)
-		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
-
-	if (p->id_len == 0)
-		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
-
-	len = p->binder_len;
-	data = p->binder_data;
-
-	if (len == 0)
-		return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
-
-	/* Start traversing the binders */
-	while (!binder_found && len > 0) {
-		DECR_LEN(len, 1);
-		binder_len = *(data);
-
-		if (binder_len == 0)
-			return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
-
-		data++;
-		read_data++;
-
-		if (cur_index == psk_index) {
-			/* We found the binder with the supplied index */
-			DECR_LEN(len, binder_len);
-			binder_out->data = (void*)data;
-			binder_out->size = binder_len;
-
-			data += binder_len;
-			read_data += binder_len;
+	if (iter->binders_len == 0)
+		return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
-			binder_found = 1;
-		} else {
-			/* Not our binder - continue to the next one */
-			DECR_LEN(len, binder_len);
-			data += binder_len;
-			read_data += binder_len;
+	DECR_LEN(iter->binders_len, 1);
+	binder->size = *iter->binders_data;
+	if (binder->size == 0)
+		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 
-			cur_index++;
-		}
-	}
+	iter->binders_data++;
+	binder->data = (uint8_t *)iter->binders_data;
+	DECR_LEN(iter->binders_len, binder->size);
+	iter->binders_data += binder->size;
 
-	if (binder_found)
-		return 0;
-	else
-		return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
+	return 0;
 }
diff --git a/lib/tls13/psk_ext_parser.h b/lib/tls13/psk_ext_parser.h
index 2c6ab603ae..fcd6c67d28 100644
--- a/lib/tls13/psk_ext_parser.h
+++ b/lib/tls13/psk_ext_parser.h
@@ -20,22 +20,16 @@
  *
  */
 
-#ifndef PSK_PARSER_H
-#define PSK_PARSER_H
-#include <gnutls/gnutls.h>
-
 struct psk_ext_parser_st {
-	const unsigned char *data;
-	ssize_t len;
-	size_t id_len;
-	size_t id_read;
-	int next_index;
+	const unsigned char *identities_data;
+	ssize_t identities_len;
 
-	const unsigned char *binder_data;
-	ssize_t binder_len;
+	const unsigned char *binders_data;
+	ssize_t binders_len;
 };
 
 typedef struct psk_ext_parser_st psk_ext_parser_st;
+typedef struct psk_ext_parser_st psk_ext_iter_st;
 
 struct psk_st {
 	/* constant values */
@@ -45,8 +39,15 @@ struct psk_st {
 
 int _gnutls13_psk_ext_parser_init(psk_ext_parser_st *p,
 				  const unsigned char *data, size_t len);
-int _gnutls13_psk_ext_parser_next_psk(psk_ext_parser_st *p, struct psk_st *psk);
-int _gnutls13_psk_ext_parser_find_binder(psk_ext_parser_st *p, int psk_index,
-					 gnutls_datum_t *binder_out);
 
-#endif
+inline static
+void _gnutls13_psk_ext_iter_init(psk_ext_iter_st *iter,
+				 const psk_ext_parser_st *p)
+{
+	memcpy(iter, p, sizeof(*p));
+}
+
+int _gnutls13_psk_ext_iter_next_identity(psk_ext_iter_st *iter,
+					 struct psk_st *psk);
+int _gnutls13_psk_ext_iter_next_binder(psk_ext_iter_st *iter,
+				       gnutls_datum_t *binder);
diff --git a/tests/tls13/psk-ext.c b/tests/tls13/psk-ext.c
index 3d5c5956ed..27dca45eda 100644
--- a/tests/tls13/psk-ext.c
+++ b/tests/tls13/psk-ext.c
@@ -40,9 +40,11 @@ static void decode(const char *test_name, const gnutls_datum_t *raw, const gnutl
 {
 	int ret;
 	psk_ext_parser_st p;
+	psk_ext_iter_st iter;
 	struct psk_st psk;
 	gnutls_datum_t binder;
 	unsigned found = 0;
+	unsigned i, j;
 
 	ret = _gnutls13_psk_ext_parser_init(&p, raw->data, raw->size);
 	if (ret < 0) {
@@ -52,8 +54,16 @@ static void decode(const char *test_name, const gnutls_datum_t *raw, const gnutl
 		exit(1);
 	}
 
-	while ((ret = _gnutls13_psk_ext_parser_next_psk(&p, &psk)) >= 0) {
-		if (ret == (int)idx) {
+	_gnutls13_psk_ext_iter_init(&iter, &p);
+	for (i = 0; ; i++) {
+		ret = _gnutls13_psk_ext_iter_next_identity(&iter, &psk);
+		if (ret < 0) {
+			if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE)
+				break;
+			if (res == ret) /* expected */
+				return;
+		}
+		if (i == idx) {
 			if (psk.identity.size == id->size && memcmp(psk.identity.data, id->data, id->size) == 0) {
 				if (debug)
 					success("%s: found id\n", test_name);
@@ -68,11 +78,20 @@ static void decode(const char *test_name, const gnutls_datum_t *raw, const gnutl
 	if (found == 0)
 		fail("%s: did not found identity!\n", test_name);
 
-	ret = _gnutls13_psk_ext_parser_find_binder(&p, idx, &binder);
-	if (ret < 0) {
-		fail("%s: could not extract binder: %s\n", test_name, gnutls_strerror(ret));
+	_gnutls13_psk_ext_iter_init(&iter, &p);
+	for (j = 0; j <= i; j++) {
+		ret = _gnutls13_psk_ext_iter_next_binder(&iter, &binder);
+		if (ret < 0) {
+			if (res == ret) /* expected */
+				return;
+			fail("%s: could not extract binder: %s\n",
+			     test_name, gnutls_strerror(ret));
+		}
 	}
 
+	if (debug)
+		success("%s: found binder\n", test_name);
+
 	if (binder.size != b->size || memcmp(binder.data, b->data, b->size) != 0) {
 		hexprint(binder.data, binder.size);
 		fail("%s: did not match binder on index %d\n", test_name, idx);
@@ -155,6 +174,22 @@ struct decode_tests_st decode_tests[] = {
 		.binder = { (unsigned char*)"\x71\x83\x89\x3d\xcc\x46\xad\x83\x18\x98\x59\x46\x0b\xb2\x51\x24\x53\x41\xb4\x35\x04\x22\x90\x02\xac\x5e\xc1\xe7\xbc\xca\x52\x16", 32},
 		.idx = 2,
 		.res = 0
+	},
+	{
+		.name = "multiple psks id3",
+		.psk = { (unsigned char*)"\x00\x20\x00\x04\x70\x73\x6b\x31\x00\x00\x00\x00"
+				"\x00\x06\x70\x73\x6b\x69\x64\x00\x00\x00\x00\x00"
+				"\x00\x04\x74\x65\x73\x74\x00\x00\x00\x00\x00\x42"
+				"\x20\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x01\x00"
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+				"\x00\x00\x00\x00\x00\x00", 102},
+		.id = { (unsigned char*)"test", 4 },
+		.binder = { NULL, 0 },
+		.idx = 2,
+		.res = GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE
 	}
 };