Simplified the _gnutls13_psk_ext_parser interface and added unit tests

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

7 files changed, 247 insertions, 85 deletions

diff --git a/lib/auth/psk.h b/lib/auth/psk.h
index 783d4f99ad..3d204bc8c8 100644
--- a/lib/auth/psk.h
+++ b/lib/auth/psk.h
@@ -63,11 +63,10 @@ typedef struct psk_auth_info_st {
 	char hint[MAX_USERNAME_SIZE + 1];
 } *psk_auth_info_t;
 
+typedef struct psk_auth_info_st psk_auth_info_st;
 
 #ifdef ENABLE_PSK
 
-typedef struct psk_auth_info_st psk_auth_info_st;
-
 int
 _gnutls_set_psk_session_key(gnutls_session_t session, gnutls_datum_t * key,
 			    gnutls_datum_t * psk2);
diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index 02c2288528..920ae17398 100644
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -263,10 +263,9 @@ static int server_recv_params(gnutls_session_t session,
 	struct psk_st psk;
 
 	ret = _gnutls13_psk_ext_parser_init(&psk_parser, data, len);
-	if (ret == 0) {
-		/* No PSKs advertised by client */
-		return 0;
-	} else if (ret < 0) {
+	if (ret < 0) {
+		if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) /* No PSKs advertised by client */
+			return 0;
 		return gnutls_assert_val(ret);
 	}
 
@@ -295,10 +294,8 @@ static int server_recv_params(gnutls_session_t session,
 		return gnutls_assert_val(ret);
 
 	/* Get full ClientHello */
-	if (!_gnutls_ext_get_full_client_hello(session, &full_client_hello)) {
-		ret = 0;
-		goto cleanup;
-	}
+	if (!_gnutls_ext_get_full_client_hello(session, &full_client_hello))
+		return 0;
 
 	/* Compute the binder value for this PSK */
 	prf = pskcred->binder_algo;
@@ -306,16 +303,13 @@ static int server_recv_params(gnutls_session_t session,
 	ret = compute_psk_binder(GNUTLS_SERVER, prf, hash_size, hash_size, 0, 0,
 				 &key, &full_client_hello,
 				 binder_value);
-	if (ret < 0) {
-		gnutls_assert();
-		goto cleanup;
-	}
+	if (ret < 0)
+		return gnutls_assert_val(ret);
 
 	if (_gnutls_mac_get_algo_len(prf) != binder_recvd.size ||
 	    safe_memcmp(binder_value, binder_recvd.data, binder_recvd.size)) {
 		gnutls_free(key.data);
-		ret = gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
-		goto cleanup;
+		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 	}
 
 	if (session->internals.hsk_flags & HSK_PSK_KE_MODE_DHE_PSK)
@@ -335,11 +329,7 @@ static int server_recv_params(gnutls_session_t session,
 	session->key.proto.tls13.psk_index = psk_index;
 	session->key.proto.tls13.binder_prf = prf;
 
-	ret = 0;
- cleanup:
-	_gnutls_free_datum(&binder_recvd);
-
-	return ret;
+	return 0;
 }
 
 /*
diff --git a/lib/libgnutls.map b/lib/libgnutls.map
index 07aaf714cc..45aa9b9431 100644
--- a/lib/libgnutls.map
+++ b/lib/libgnutls.map
@@ -1218,6 +1218,7 @@ GNUTLS_3_6_3
 } GNUTLS_3_6_2;
 
 GNUTLS_FIPS140_3_4 {
+  global:
 	gnutls_cipher_self_test;
 	gnutls_pk_self_test;
 	gnutls_mac_self_test;
@@ -1256,6 +1257,11 @@ GNUTLS_PRIVATE_3_4 {
 	_gnutls_digest_exists;
 	_gnutls_log;
 
+	# PSK extension parser unit test
+	_gnutls13_psk_ext_parser_init;
+	_gnutls13_psk_ext_parser_next_psk;
+	_gnutls13_psk_ext_parser_find_binder;
+
 	# Internal symbols needed by gnutls-cli-debug:
 	_gnutls_rsa_pms_set_version;
 	_gnutls_record_set_default_version;
diff --git a/lib/tls13/psk_ext_parser.c b/lib/tls13/psk_ext_parser.c
index 7dd1427cf3..9f4087773d 100644
--- a/lib/tls13/psk_ext_parser.c
+++ b/lib/tls13/psk_ext_parser.c
@@ -20,23 +20,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
-#include "tls13/psk_ext_parser.h"
-
-
-static int advance_to_end_of_object(psk_ext_parser_st *p)
-{
-	size_t adv;
 
-	/* Advance the pointer to the end of the current object */
-	if (p->obj_read < p->obj_len) {
-		adv = p->obj_len - p->obj_read;
-		DECR_LEN(p->len, adv);
-		p->data += adv;
-	}
-
-	return 0;
-}
+#include "gnutls_int.h"
+#include "tls13/psk_ext_parser.h"
 
+/* Returns GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE when no identities
+ * are present, or >= 0, on success.
+ */
 int _gnutls13_psk_ext_parser_init(psk_ext_parser_st *p,
 				  const unsigned char *data, size_t _len)
 {
@@ -48,105 +38,106 @@ 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);
+	data += 2;
 
-	if (identities_len > 0) {
-		DECR_LEN(len, 2);
-		data += 2;
+	if (identities_len == 0)
+		return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
 
-		p->obj_len = identities_len;
-		p->data = (unsigned char *) data;
-		p->len = len;
-	}
+	p->id_len = identities_len;
+	p->data = (unsigned char *) data;
+	p->len = len;
+
+	DECR_LEN(len, p->id_len);
+	data += p->id_len;
 
-	return identities_len;
+	DECR_LEN(len, 2);
+	p->binder_len = _gnutls_read_uint16(data);
+
+	p->binder_data = p->data + p->id_len + 2;
+	DECR_LEN(len, p->binder_len);
+
+	return 0;
 }
 
 int _gnutls13_psk_ext_parser_next_psk(psk_ext_parser_st *p, struct psk_st *psk)
 {
-	if (p->obj_read >= p->obj_len)
+	if (p->id_read >= p->id_len)
 		return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
 	/* Read a PskIdentity structure */
+	DECR_LEN(p->len, 2);
 	psk->identity.size = _gnutls_read_uint16(p->data);
 	if (psk->identity.size == 0)
 		return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
-	DECR_LEN(p->len, 2);
 	p->data += 2;
-	p->obj_read += 2;
+	p->id_read += 2;
 
-	psk->identity.data = p->data;
+	psk->identity.data = (void*)p->data;
 
 	DECR_LEN(p->len, psk->identity.size);
 	p->data += psk->identity.size;
-	p->obj_read += psk->identity.size;
+	p->id_read += psk->identity.size;
 
-	psk->ob_ticket_age = _gnutls_read_uint32(p->data);
 	DECR_LEN(p->len, 4);
+	psk->ob_ticket_age = _gnutls_read_uint32(p->data);
+
 	p->data += 4;
-	p->obj_read += 4;
+	p->id_read += 4;
 
 	return p->next_index++;
 }
 
+/* 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)
 {
-	uint16_t binders_len;
 	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->obj_len == 0)
+	if (p->id_len == 0)
 		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
 
-	/* Place the pointer at the start of the binders */
-	if (advance_to_end_of_object(p) < 0)
-		return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET_LENGTH);
-
-	DECR_LEN(p->len, 2);
-	binders_len = _gnutls_read_uint16(p->data);
-	if (binders_len > 0) {
-		p->data += 2;
+	len = p->binder_len;
+	data = p->binder_data;
 
-		p->obj_len = binders_len;
-		p->obj_read = 0;
-	} else {
+	if (len == 0)
 		return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
-	}
 
 	/* Start traversing the binders */
-	while (!binder_found && p->len > 0) {
-		DECR_LEN(p->len, 1);
-		binder_len = *(p->data);
+	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);
 
-		p->data++;
-		p->obj_read++;
+		data++;
+		read_data++;
 
 		if (cur_index == psk_index) {
 			/* We found the binder with the supplied index */
-			binder_out->data = gnutls_malloc(binder_len);
-			if (!binder_out->data)
-				return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
-
+			DECR_LEN(len, binder_len);
+			binder_out->data = (void*)data;
 			binder_out->size = binder_len;
-			DECR_LEN(p->len, binder_len);
-			memcpy(binder_out->data, p->data, binder_len);
-			p->data += binder_len;
-			p->obj_read += binder_len;
+
+			data += binder_len;
+			read_data += binder_len;
 
 			binder_found = 1;
 		} else {
 			/* Not our binder - continue to the next one */
-			DECR_LEN(p->len, binder_len);
-			p->data += binder_len;
-			p->obj_read += binder_len;
+			DECR_LEN(len, binder_len);
+			data += binder_len;
+			read_data += binder_len;
 
 			cur_index++;
 		}
diff --git a/lib/tls13/psk_ext_parser.h b/lib/tls13/psk_ext_parser.h
index 14f3c89186..2c6ab603ae 100644
--- a/lib/tls13/psk_ext_parser.h
+++ b/lib/tls13/psk_ext_parser.h
@@ -22,19 +22,23 @@
 
 #ifndef PSK_PARSER_H
 #define PSK_PARSER_H
-#include "gnutls_int.h"
+#include <gnutls/gnutls.h>
 
 struct psk_ext_parser_st {
-	unsigned char *data;
+	const unsigned char *data;
 	ssize_t len;
-	size_t obj_len;
-	size_t obj_read;
+	size_t id_len;
+	size_t id_read;
 	int next_index;
+
+	const unsigned char *binder_data;
+	ssize_t binder_len;
 };
 
 typedef struct psk_ext_parser_st psk_ext_parser_st;
 
 struct psk_st {
+	/* constant values */
 	gnutls_datum_t identity;
 	uint32_t ob_ticket_age;
 };
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5abd976ff8..2f66ed5fc7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -108,6 +108,8 @@ ctests = tls13/supported_versions tls13/tls12-no-tls13-exts \
 	tls12-rollback-detection tls11-rollback-detection \
 	tls12-check-rollback-val tls11-check-rollback-val tls13/hello_random_value
 
+ctests += tls13/psk-ext
+
 ctests += tls13/key_update
 
 ctests += tls13/key_limits
diff --git a/tests/tls13/psk-ext.c b/tests/tls13/psk-ext.c
new file mode 100644
index 0000000000..3d5c5956ed
--- /dev/null
+++ b/tests/tls13/psk-ext.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2016, 2017 Red Hat, Inc.
+ *
+ * Author: Nikos Mavrogiannopoulos
+ *
+ * This file is part of GnuTLS.
+ *
+ * GnuTLS is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GnuTLS is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <string.h>
+#include <gnutls/gnutls.h>
+#include <stdint.h>
+#include "../lib/tls13/psk_ext_parser.h"
+
+#include "utils.h"
+
+/* Tests the PSK-extension decoding part */
+
+static void decode(const char *test_name, const gnutls_datum_t *raw, const gnutls_datum_t *id,
+		   const gnutls_datum_t *b, unsigned idx, int res)
+{
+	int ret;
+	psk_ext_parser_st p;
+	struct psk_st psk;
+	gnutls_datum_t binder;
+	unsigned found = 0;
+
+	ret = _gnutls13_psk_ext_parser_init(&p, raw->data, raw->size);
+	if (ret < 0) {
+		if (res == ret) /* expected */
+			return;
+		fail("%s: _gnutls13_psk_ext_parser_init: %d/%s\n", test_name, ret, gnutls_strerror(ret));
+		exit(1);
+	}
+
+	while ((ret = _gnutls13_psk_ext_parser_next_psk(&p, &psk)) >= 0) {
+		if (ret == (int)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);
+				found = 1;
+				break;
+			} else {
+				fail("%s: did not found identity on index %d\n", test_name, idx);
+			}
+		}
+	}
+
+	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));
+	}
+
+	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);
+	}
+
+	return;
+}
+
+struct decode_tests_st {
+	const char *name;
+	gnutls_datum_t psk;
+	unsigned idx; /* the ID index */
+	gnutls_datum_t id;
+	gnutls_datum_t binder;
+	int res;
+};
+
+struct decode_tests_st decode_tests[] = {
+	{
+		.name = "single PSK",
+		.psk = { (unsigned char*)"\x00\x0a\x00\x04\x6e\x6d\x61\x76\x00\x00\x00\x00\x00\x21\x20\xc4\xda\xe5\x7e\x05\x59\xf7\xae\x9b\xba\x90\xd2\x6e\x12\x68\xf6\xc1\xc7\xb9\x7e\xdc\xed\x9e\x67\x4e\xa5\x91\x2d\x7c\xb4\xf0\xab", 47},
+		.id = { (unsigned char*)"nmav", 4 },
+		.binder = { (unsigned char*)"\xc4\xda\xe5\x7e\x05\x59\xf7\xae\x9b\xba\x90\xd2\x6e\x12\x68\xf6\xc1\xc7\xb9\x7e\xdc\xed\x9e\x67\x4e\xa5\x91\x2d\x7c\xb4\xf0\xab", 32 },
+		.idx = 0,
+		.res = 0
+	},
+	{
+		.name = "multiple psks id0",
+		.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\x63"
+				"\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\x20\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", 135},
+		.id = { (unsigned char*)"psk1", 4 },
+		.binder = { (unsigned char*)"\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", 32},
+		.idx = 0,
+		.res = 0
+	},
+	{
+		.name = "multiple psks id1",
+		.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\x63"
+				"\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\x20\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", 135},
+		.id = { (unsigned char*)"pskid", 6 },
+		.binder = { (unsigned char*)"\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", 32},
+		.idx = 1,
+		.res = 0
+	},
+	{
+		.name = "multiple psks id2",
+		.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\x63"
+				"\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\x20\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", 135},
+		.id = { (unsigned char*)"test", 4 },
+		.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
+	}
+};
+
+void doit(void)
+{
+	unsigned i;
+
+	for (i=0;i<sizeof(decode_tests)/sizeof(decode_tests[0]);i++) {
+		decode(decode_tests[i].name, &decode_tests[i].psk, &decode_tests[i].id,
+		       &decode_tests[i].binder, decode_tests[i].idx, decode_tests[i].res);
+	}
+}
+