Merge branch 'tmp-rsa-psk-fix' into 'master'

RSA-PSK ciphersuites: only use under TLS1.2

See merge request gnutls/gnutls!624

5 files changed, 271 insertions, 10 deletions

diff --git a/lib/ext/psk_ke_modes.c b/lib/ext/psk_ke_modes.c
index afcbcb8ce1..872fec9fa3 100644
--- a/lib/ext/psk_ke_modes.c
+++ b/lib/ext/psk_ke_modes.c
@@ -112,12 +112,19 @@ psk_ke_modes_recv_params(gnutls_session_t session,
 	if (session->security_parameters.entity == GNUTLS_CLIENT)
 		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_EXTENSION);
 
-	if (!vers || !vers->tls13_sem)
-		return 0;
+	/* we set hsk_flags to HSK_PSK_KE_MODE_INVALID on failure to ensure that
+	 * when we parse the pre-shared key extension we detect PSK_KE_MODES as
+	 * received. */
+	if (!vers || !vers->tls13_sem) {
+		session->internals.hsk_flags |= HSK_PSK_KE_MODE_INVALID;
+		return gnutls_assert_val(0);
+	}
 
 	cred = (gnutls_psk_server_credentials_t)_gnutls_get_cred(session, GNUTLS_CRD_PSK);
-	if (cred == NULL)
-		return 0;
+	if (cred == NULL) {
+		session->internals.hsk_flags |= HSK_PSK_KE_MODE_INVALID;
+		return gnutls_assert_val(0);
+	}
 
 	DECR_LEN(len, 1);
 	ke_modes_len = *(data++);
diff --git a/lib/priority.c b/lib/priority.c
index 065728fa0b..0d2498d998 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -1199,7 +1199,10 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
 	const version_entry_st *tlsmin = NULL;
 	const version_entry_st *dtlsmin = NULL;
 	unsigned have_tls13 = 0, have_srp = 0;
-	unsigned have_psk = 0, have_null = 0;
+	unsigned have_psk = 0, have_null = 0, have_rsa_psk = 0;
+
+	/* have_psk indicates that a PSK key exchange compatible
+	 * with TLS1.3 is enabled. */
 
 	priority_cache->cs.size = 0;
 	priority_cache->sigalg.size = 0;
@@ -1217,7 +1220,10 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
 		if (IS_SRP_KX(priority_cache->_kx.priority[i])) {
 			have_srp = 1;
 		} else if (_gnutls_kx_is_psk(priority_cache->_kx.priority[i])) {
-			have_psk = 1;
+			if (priority_cache->_kx.priority[i] == GNUTLS_KX_RSA_PSK)
+				have_rsa_psk = 1;
+			else
+				have_psk = 1;
 		}
 	}
 
@@ -1226,9 +1232,9 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
 		if (!vers)
 			continue;
 
-		/* if we have NULL ciphersuites enabled, remove TLS1.3+ protocol versions;
-		 * they cannot be negotiated under TLS1.3. */
-		if (have_null || have_srp) {
+		/* if we have NULL ciphersuites, SRP or RSA-PSK enabled, remove TLS1.3+ protocol
+		 * versions; they cannot be negotiated under TLS1.3. */
+		if (have_null || have_srp || have_rsa_psk) {
 			if (vers->tls13_sem) {
 				for (j=i+1;j<priority_cache->protocol.algorithms;j++)
 					priority_cache->protocol.priority[j-1] = priority_cache->protocol.priority[j];
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 88f56455c8..77819a3777 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -192,7 +192,8 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei
 	 send-data-before-handshake recv-data-before-handshake crt_inv_write \
 	 x509sign-verify-error rng-op-nonce rng-op-random rng-op-key x509-dn-decode-compat \
 	 ip-check mini-x509-ipaddr trust-store base64-raw random-art dhex509self \
-	 dss-sig-val sign-pk-api tls-session-ext-override mini-record-pad
+	 dss-sig-val sign-pk-api tls-session-ext-override mini-record-pad \
+	 tls13-server-kx-neg
 
 if HAVE_SECCOMP_TESTS
 ctests += dtls-with-seccomp tls-with-seccomp dtls-client-with-seccomp tls-client-with-seccomp
diff --git a/tests/server-kx-neg-common.c b/tests/server-kx-neg-common.c
index de4c7ad91a..5f80a60b15 100644
--- a/tests/server-kx-neg-common.c
+++ b/tests/server-kx-neg-common.c
@@ -39,6 +39,7 @@ typedef struct test_case_st {
 	unsigned have_ed25519_sign_cert;
 	unsigned have_rsa_decrypt_cert;
 	unsigned not_on_fips;
+	unsigned exp_version;
 	const char *client_prio;
 	const char *server_prio;
 } test_case_st;
@@ -230,6 +231,13 @@ static void try(test_case_st *test)
 
 	HANDSHAKE_EXPECT(client, server, test->client_ret, test->server_ret);
 
+	if (test->client_ret == 0 && test->server_ret == 0 && test->exp_version) {
+		if (gnutls_protocol_get_version(client) != test->exp_version)
+			fail("expected version (%s) does not match %s\n",
+			     gnutls_protocol_get_name(test->exp_version),
+			     gnutls_protocol_get_name(gnutls_protocol_get_version(client)));
+	}
+
 	gnutls_deinit(server);
 	gnutls_deinit(client);
 	gnutls_anon_free_client_credentials(c_anon_cred);
diff --git a/tests/tls13-server-kx-neg.c b/tests/tls13-server-kx-neg.c
new file mode 100644
index 0000000000..aba7944b1c
--- /dev/null
+++ b/tests/tls13-server-kx-neg.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2017-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
+
+/* This program tests the negotiation for various key exchange
+ * methods and options which are considered legacy in TLS1.3. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <gnutls/gnutls.h>
+#include "utils.h"
+#include "cert-common.h"
+#include "eagain-common.h"
+
+#include "server-kx-neg-common.c"
+
+#define PVERSION "-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2"
+
+test_case_st tests[] = {
+	{
+		.name = "TLS 1.3 DHE-PSK without cred",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS,
+		.server_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_3,
+	},
+	{
+		.name = "TLS 1.3 DHE-PSK with cred but no DH params",
+		.client_ret = 0,
+		.server_ret = 0,
+		.have_psk_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_3,
+	},
+	{
+		.name = "TLS 1.3 DHE-PSK with cred and DH params (level)",
+		.client_ret = 0,
+		.server_ret = 0,
+		.have_psk_cred = 1,
+		.have_psk_dh_params = 1,
+		.server_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_3,
+	},
+	{
+		.name = "TLS 1.3 DHE-PSK with cred and DH params (explicit)",
+		.client_ret = 0,
+		.server_ret = 0,
+		.have_psk_cred = 1,
+		.have_psk_exp_dh_params = 1,
+		.server_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+DHE-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_3,
+	},
+	{
+		.name = "TLS 1.3 ECDHE-PSK with cred but no common curve",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_NO_COMMON_KEY_SHARE,
+		.have_psk_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+ECDHE-PSK:-CURVE-ALL:+CURVE-SECP256R1:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+ECDHE-PSK:-CURVE-ALL:+CURVE-SECP384R1:"PVERSION,
+		.exp_version = GNUTLS_TLS1_3,
+	},
+	{
+		.name = "TLS 1.3 ECDHE-PSK with cred and common curve",
+		.client_ret = 0,
+		.server_ret = 0,
+		.have_psk_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+ECDHE-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+ECDHE-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_3,
+	},
+	{
+		.name = "TLS 1.3 RSA-PSK without cert cred",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS,
+		.have_psk_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 RSA-PSK without psk cred",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_NO_CIPHER_SUITES,
+		.have_psk_cred = 0,
+		.have_cert_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 RSA-PSK with cred but invalid cert",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_NO_CIPHER_SUITES,
+		.have_psk_cred = 1,
+		.have_cert_cred = 1,
+		.have_rsa_sign_cert = 1,
+		.have_ecc_sign_cert = 1,
+		.server_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 RSA-PSK with cred",
+		.server_ret = 0,
+		.client_ret = 0,
+		.have_psk_cred = 1,
+		.have_cert_cred = 1,
+		.have_rsa_decrypt_cert = 1,
+		.server_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 RSA-PSK with cred and multiple certs",
+		.server_ret = 0,
+		.client_ret = 0,
+		.have_psk_cred = 1,
+		.have_cert_cred = 1,
+		.have_rsa_sign_cert = 1,
+		.have_ecc_sign_cert = 1,
+		.have_rsa_decrypt_cert = 1,
+		.server_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+RSA-PSK:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP-RSA without cert cred",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS,
+		.have_srp_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP-RSA without srp cred",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_NO_CIPHER_SUITES,
+		.have_srp_cred = 0,
+		.have_cert_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP-RSA with cred but invalid cert",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_NO_CIPHER_SUITES,
+		.have_srp_cred = 1,
+		.have_cert_cred = 1,
+		.have_rsa_decrypt_cert = 1,
+		.have_ecc_sign_cert = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP-RSA with cred",
+		.server_ret = 0,
+		.client_ret = 0,
+		.have_srp_cred = 1,
+		.have_cert_cred = 1,
+		.have_rsa_sign_cert = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP-RSA with cred and multiple certs",
+		.server_ret = 0,
+		.client_ret = 0,
+		.have_srp_cred = 1,
+		.have_cert_cred = 1,
+		.have_rsa_sign_cert = 1,
+		.have_ecc_sign_cert = 1,
+		.have_rsa_decrypt_cert = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP-RSA:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP without srp cred",
+		.client_ret = GNUTLS_E_AGAIN,
+		.server_ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS,
+		.have_srp_cred = 0,
+		.have_cert_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	},
+	{
+		.name = "TLS 1.3 SRP with cred",
+		.server_ret = 0,
+		.client_ret = 0,
+		.have_srp_cred = 1,
+		.server_prio = "NORMAL:-KX-ALL:+SRP:"PVERSION,
+		.client_prio = "NORMAL:-KX-ALL:+SRP:"PVERSION,
+		.exp_version = GNUTLS_TLS1_2,
+	}
+};
+
+void doit(void)
+{
+	unsigned i;
+	global_init();
+
+	for (i=0;i<sizeof(tests)/sizeof(tests[0]);i++) {
+		try(&tests[i]);
+	}
+
+	gnutls_global_deinit();
+}