Merge branch 'tmp-fix-order-extensions' into 'master'

extensions: corrected order of pre-shared-key and dumbfw

Closes #473

See merge request gnutls/gnutls!659

8 files changed, 399 insertions, 8 deletions

diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index dce24d80a1..b12d853af8 100644
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -749,7 +749,7 @@ static int _gnutls_psk_recv_params(gnutls_session_t session,
 
 const hello_ext_entry_st ext_pre_shared_key = {
 	.name = "Pre Shared Key",
-	.tls_id = 41,
+	.tls_id = PRE_SHARED_KEY_TLS_ID,
 	.gid = GNUTLS_EXTENSION_PRE_SHARED_KEY,
 	.parse_type = GNUTLS_EXT_TLS,
 	.validity = GNUTLS_EXT_FLAG_TLS | GNUTLS_EXT_FLAG_CLIENT_HELLO | GNUTLS_EXT_FLAG_TLS13_SERVER_HELLO,
diff --git a/lib/ext/pre_shared_key.h b/lib/ext/pre_shared_key.h
index 2e830ff52e..1168750656 100644
--- a/lib/ext/pre_shared_key.h
+++ b/lib/ext/pre_shared_key.h
@@ -5,6 +5,8 @@
 #include <hello_ext.h>
 #include "tls13/session_ticket.h"
 
+#define PRE_SHARED_KEY_TLS_ID 41
+
 extern const hello_ext_entry_st ext_pre_shared_key;
 
 inline static
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index d2d417ee67..b27fa01130 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -335,10 +335,9 @@ typedef enum extensions_t {
 	GNUTLS_EXTENSION_PSK_KE_MODES,
 	/*
 	 * pre_shared_key and dumbfw must always be the last extensions,
-	 * in that order
-	 */
-	GNUTLS_EXTENSION_PRE_SHARED_KEY,
+	 * in that order */
 	GNUTLS_EXTENSION_DUMBFW,
+	GNUTLS_EXTENSION_PRE_SHARED_KEY,
 	GNUTLS_EXTENSION_MAX /* not real extension - used for iterators */
 } extensions_t;
 
diff --git a/lib/hello_ext.c b/lib/hello_ext.c
index d9f548457f..a3027130a6 100644
--- a/lib/hello_ext.c
+++ b/lib/hello_ext.c
@@ -195,6 +195,7 @@ typedef struct hello_ext_ctx_st {
 	gnutls_ext_flags_t msg;
 	gnutls_ext_parse_type_t parse_type;
 	const hello_ext_entry_st *ext; /* used during send */
+	unsigned seen_pre_shared_key;
 } hello_ext_ctx_st;
 
 static
@@ -205,6 +206,14 @@ int hello_ext_parse(void *_ctx, unsigned tls_id, const uint8_t *data, unsigned d
 	const hello_ext_entry_st *ext;
 	int ret;
 
+	if (tls_id == PRE_SHARED_KEY_TLS_ID) {
+		ctx->seen_pre_shared_key = 1;
+	} else if (ctx->seen_pre_shared_key) {
+		/* the pre-shared key extension must always be the last one,
+		 * draft-ietf-tls-tls13-28: 4.2.11 */
+		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+	}
+
 	ext = tls_id_to_ext_entry(session, tls_id, ctx->parse_type);
 	if (ext == NULL || ext->recv_func == NULL) {
 		goto ignore;
@@ -270,9 +279,9 @@ int hello_ext_parse(void *_ctx, unsigned tls_id, const uint8_t *data, unsigned d
 
 int
 _gnutls_parse_hello_extensions(gnutls_session_t session,
-			 gnutls_ext_flags_t msg,
-			 gnutls_ext_parse_type_t parse_type,
-			 const uint8_t * data, int data_size)
+			       gnutls_ext_flags_t msg,
+			       gnutls_ext_parse_type_t parse_type,
+			       const uint8_t * data, int data_size)
 {
 	int ret;
 	hello_ext_ctx_st ctx;
@@ -282,6 +291,7 @@ _gnutls_parse_hello_extensions(gnutls_session_t session,
 	ctx.session = session;
 	ctx.msg = msg;
 	ctx.parse_type = parse_type;
+	ctx.seen_pre_shared_key = 0;
 
 	ret = _gnutls_extv_parse(&ctx, hello_ext_parse, data, data_size);
 	if (ret < 0)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 14d75d42c5..c0beb5acda 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -124,6 +124,8 @@ ctests += tls13-cipher-neg
 
 ctests += tls13/no-psk-exts
 
+ctests += tls13/psk-dumbfw
+
 ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniqueid tls-neg-ext-key \
 	 mpi certificate_set_x509_crl dn parse_ca x509-dn x509-dn-decode record-sizes \
 	 hostname-check cve-2008-4989 pkcs12_s2k chainverify record-sizes-range \
diff --git a/tests/pskself.c b/tests/pskself.c
index 9580abe86d..10fe4c90cf 100644
--- a/tests/pskself.c
+++ b/tests/pskself.c
@@ -323,7 +323,7 @@ void doit(void)
 	run_test("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK", 0);
 	run_test("NORMAL:-VERS-ALL:+VERS-TLS1.3:-GROUP-ALL:+GROUP-FFDHE2048:+DHE-PSK", 0);
 	run_test("NORMAL:-VERS-ALL:+VERS-TLS1.3:-GROUP-ALL:+GROUP-SECP256R1:+ECDHE-PSK", 0);
-	/* the follow should work once we support PSK without DH */
+	/* the following should work once we support PSK without DH */
 	run_test("NORMAL:-VERS-ALL:+VERS-TLS1.3:-GROUP-ALL:+PSK", 0);
 
 	run_test("NORMAL:-KX-ALL:+PSK", 1);
diff --git a/tests/tls13/ext-parse.h b/tests/tls13/ext-parse.h
index ae9c1c8afb..d3f2275c4f 100644
--- a/tests/tls13/ext-parse.h
+++ b/tests/tls13/ext-parse.h
@@ -108,6 +108,60 @@ static unsigned find_client_extension(const gnutls_datum_t *msg, unsigned extnr,
 	return 0;
 }
 
+static unsigned is_client_extension_last(const gnutls_datum_t *msg, unsigned extnr)
+{
+	unsigned pos, found = 0;
+
+	if (msg->size < HANDSHAKE_SESSION_ID_POS)
+		fail("invalid client hello\n");
+
+	/* we expect the legacy version to be present */
+	/* ProtocolVersion legacy_version = 0x0303 */
+	if (msg->data[0] != 0x03) {
+		fail("ProtocolVersion contains %d.%d\n", (int)msg->data[0], (int)msg->data[1]);
+	}
+
+	pos = HANDSHAKE_SESSION_ID_POS;
+	/* legacy_session_id */
+	SKIP8(pos, msg->size);
+
+	/* CipherSuites */
+	SKIP16(pos, msg->size);
+
+	/* legacy_compression_methods */
+	SKIP8(pos, msg->size);
+
+	pos += 2;
+
+	while (pos < msg->size) {
+		uint16_t type;
+
+		if (pos+4 > msg->size)
+			fail("invalid client hello\n");
+
+		type = (msg->data[pos] << 8) | msg->data[pos+1];
+		pos+=2;
+
+		if (debug)
+			success("Found client extension %d\n", (int)type);
+
+		if (type != extnr) {
+			if (found) {
+				success("found extension %d after %d\n", type, extnr);
+				return 0;
+			}
+			SKIP16(pos, msg->size);
+		} else { /* found */
+			found = 1;
+			SKIP16(pos, msg->size);
+		}
+	}
+
+	if (found)
+		return 1;
+	return 0;
+}
+
 #define TLS_RANDOM_SIZE 32
 
 static unsigned find_server_extension(const gnutls_datum_t *msg, unsigned extnr, void *priv, ext_parse_func cb)
diff --git a/tests/tls13/psk-dumbfw.c b/tests/tls13/psk-dumbfw.c
new file mode 100644
index 0000000000..c9c995801e
--- /dev/null
+++ b/tests/tls13/psk-dumbfw.c
@@ -0,0 +1,324 @@
+/*
+ * Copyright (C) 2004-2012 Free Software Foundation, Inc.
+ * Copyright (C) 2013 Adam Sampson <ats@offog.org>
+ * Copyright (C) 2018 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 <stdint.h>
+
+#if defined(_WIN32)
+
+/* socketpair isn't supported on Win32. */
+int main(int argc, char **argv)
+{
+	exit(77);
+}
+
+#else
+
+#include <string.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#if !defined(_WIN32)
+#include <sys/wait.h>
+#endif
+#include <unistd.h>
+#include <gnutls/gnutls.h>
+#include <assert.h>
+
+#include "tls13/ext-parse.h"
+
+#include "utils.h"
+
+/* Tests whether the pre-shared key extension will always be last
+ * even if the dumbfw extension is present.
+ */
+
+const char *side = "";
+
+static void tls_log_func(int level, const char *str)
+{
+	fprintf(stderr, "%s|<%d>| %s", side, level, str);
+}
+
+#define MAX_BUF 1024
+#define MSG "Hello TLS"
+
+static void client(int sd, const char *prio)
+{
+	int ret, ii;
+	gnutls_session_t session;
+	char buffer[MAX_BUF + 1];
+	gnutls_psk_client_credentials_t pskcred;
+	/* Need to enable anonymous KX specifically. */
+	const gnutls_datum_t key = { (void *) "DEADBEEF", 8 };
+
+	global_init();
+	gnutls_global_set_log_function(tls_log_func);
+	if (debug)
+		gnutls_global_set_log_level(6);
+
+	side = "client";
+
+	gnutls_psk_allocate_client_credentials(&pskcred);
+	gnutls_psk_set_client_credentials(pskcred, "test", &key,
+					  GNUTLS_PSK_KEY_HEX);
+
+	assert(gnutls_init(&session, GNUTLS_CLIENT|GNUTLS_KEY_SHARE_TOP)>=0);
+
+	assert(gnutls_priority_set_direct(session, prio, NULL)>=0);
+	assert(gnutls_credentials_set(session, GNUTLS_CRD_PSK, pskcred)>=0);
+
+	gnutls_transport_set_int(session, sd);
+
+	/* Perform the TLS handshake
+	 */
+	ret = gnutls_handshake(session);
+
+	if (ret < 0) {
+		fail("client: Handshake failed\n");
+		gnutls_perror(ret);
+		goto end;
+	} else {
+		if (debug)
+			success("client: Handshake was completed\n");
+	}
+
+	assert(gnutls_record_send(session, MSG, strlen(MSG))>=0);
+
+	ret = gnutls_record_recv(session, buffer, MAX_BUF);
+	if (ret == 0) {
+		if (debug)
+			success
+			    ("client: Peer has closed the TLS connection\n");
+		goto end;
+	} else if (ret < 0) {
+		fail("client: Error: %s\n", gnutls_strerror(ret));
+		goto end;
+	}
+
+	if (debug) {
+		printf("- Received %d bytes: ", ret);
+		for (ii = 0; ii < ret; ii++) {
+			fputc(buffer[ii], stdout);
+		}
+		fputs("\n", stdout);
+	}
+
+	gnutls_bye(session, GNUTLS_SHUT_RDWR);
+
+      end:
+
+	close(sd);
+
+	gnutls_deinit(session);
+
+	gnutls_psk_free_client_credentials(pskcred);
+
+	gnutls_global_deinit();
+}
+
+static int
+pskfunc(gnutls_session_t session, const char *username,
+	gnutls_datum_t * key)
+{
+	if (debug)
+		printf("psk: username %s\n", username);
+	key->data = gnutls_malloc(4);
+	key->data[0] = 0xDE;
+	key->data[1] = 0xAD;
+	key->data[2] = 0xBE;
+	key->data[3] = 0xEF;
+	key->size = 4;
+	return 0;
+}
+
+#define EXT_CLIENTHELLO_PADDING 21
+#define EXT_PRE_SHARED_KEY 41
+
+struct ctx_st {
+	unsigned long pos;
+	void *base;
+};
+
+static
+void check_ext_pos(void *priv, gnutls_datum_t *msg)
+{
+	struct ctx_st *ctx = priv;
+
+	ctx->pos = (ptrdiff_t)((ptrdiff_t)msg->data - (ptrdiff_t)ctx->base);
+}
+
+static int client_hello_callback(gnutls_session_t session, unsigned int htype,
+	unsigned post, unsigned int incoming, const gnutls_datum_t *msg)
+{
+	unsigned long pos_psk;
+	unsigned long pos_pad;
+
+	if (htype == GNUTLS_HANDSHAKE_CLIENT_HELLO && post == GNUTLS_HOOK_POST) {
+		struct ctx_st ctx;
+
+		ctx.base = msg->data;
+		if (find_client_extension(msg, EXT_CLIENTHELLO_PADDING, &ctx, check_ext_pos) == 0)
+			fail("Could not find dumbfw/client hello padding extension!\n");
+		pos_pad = ctx.pos;
+
+		ctx.base = msg->data;
+		if (find_client_extension(msg, EXT_PRE_SHARED_KEY, &ctx, check_ext_pos) == 0)
+			fail("Could not find psk extension!\n");
+		pos_psk = ctx.pos;
+
+		if (pos_psk < pos_pad) {
+			fail("The dumbfw extension was sent after pre-shared key!\n");
+		}
+
+		/* check if we are the last extension in general */
+		if (!is_client_extension_last(msg, EXT_PRE_SHARED_KEY)) {
+			fail("pre-shared key extension wasn't the last one!\n");
+		}
+	}
+
+	return 0;
+}
+
+
+static void server(int sd, const char *prio)
+{
+	gnutls_psk_server_credentials_t server_pskcred;
+	int ret;
+	gnutls_session_t session;
+	char buffer[MAX_BUF + 1];
+
+	global_init();
+	gnutls_global_set_log_function(tls_log_func);
+	if (debug)
+		gnutls_global_set_log_level(6);
+
+	side = "server";
+
+
+	assert(gnutls_psk_allocate_server_credentials(&server_pskcred)>=0);
+	gnutls_psk_set_server_credentials_function(server_pskcred,
+						   pskfunc);
+
+	assert(gnutls_init(&session, GNUTLS_SERVER)>=0);
+
+	assert(gnutls_priority_set_direct(session, prio, NULL)>=0);
+	gnutls_credentials_set(session, GNUTLS_CRD_PSK, server_pskcred);
+
+	gnutls_handshake_set_hook_function(session, GNUTLS_HANDSHAKE_ANY,
+					   GNUTLS_HOOK_BOTH,
+					   client_hello_callback);
+
+	gnutls_transport_set_int(session, sd);
+	ret = gnutls_handshake(session);
+	if (ret < 0) {
+		close(sd);
+		gnutls_deinit(session);
+		fail("server: Handshake has failed (%s)\n\n",
+		     gnutls_strerror(ret));
+		return;
+	}
+	if (debug)
+		success("server: Handshake was completed\n");
+
+	for (;;) {
+		memset(buffer, 0, MAX_BUF + 1);
+		gnutls_record_set_timeout(session, 10000);
+		ret = gnutls_record_recv(session, buffer, MAX_BUF);
+
+		if (ret == 0) {
+			if (debug)
+				success
+				    ("server: Peer has closed the GnuTLS connection\n");
+			break;
+		} else if (ret < 0) {
+			fail("server: Received corrupted data(%d). Closing...\n", ret);
+			break;
+		} else if (ret > 0) {
+			/* echo data back to the client
+			 */
+			gnutls_record_send(session, buffer,
+					   strlen(buffer));
+		}
+	}
+
+	gnutls_bye(session, GNUTLS_SHUT_WR);
+
+	close(sd);
+	gnutls_deinit(session);
+
+	gnutls_psk_free_server_credentials(server_pskcred);
+
+	gnutls_global_deinit();
+
+	if (debug)
+		success("server: finished\n");
+}
+
+static
+void run_test(const char *prio)
+{
+	pid_t child;
+	int err;
+	int sockets[2];
+
+	success("trying with %s\n", prio);
+
+	err = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);
+	if (err == -1) {
+		perror("socketpair");
+		fail("socketpair failed\n");
+		return;
+	}
+
+	child = fork();
+	if (child < 0) {
+		perror("fork");
+		fail("fork");
+		return;
+	}
+
+	if (child) {
+		int status;
+		/* parent */
+		close(sockets[1]);
+		server(sockets[0], prio);
+		wait(&status);
+		check_wait_status(status);
+	} else {
+		close(sockets[0]);
+		client(sockets[1], prio);
+		exit(0);
+	}
+}
+
+void doit(void)
+{
+	run_test("NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2:+PSK:%DUMBFW:-GROUP-ALL:+GROUP-FFDHE2048");
+}
+
+#endif				/* _WIN32 */