gnutls_priority_set: do not override version on handshaketmp-fix-priority-set-call

When handshake is in progress, do not override the default TLS
version in the session. This allows gnutls_priority_set to be called
in the post_client_hello function without breaking the handshake.

Resolves #580

Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org>

5 files changed, 192 insertions, 14 deletions

diff --git a/lib/handshake.c b/lib/handshake.c
index 08481cca07..b513872ec3 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -491,7 +491,8 @@ _gnutls_user_hello_func(gnutls_session_t session,
 			uint8_t major, uint8_t minor)
 {
 	int ret, sret = 0;
-	const version_entry_st *vers;
+	const version_entry_st *vers, *old_vers;
+	const version_entry_st *new_max;
 
 	if (session->internals.user_hello_func != NULL) {
 		ret = session->internals.user_hello_func(session);
@@ -504,17 +505,34 @@ _gnutls_user_hello_func(gnutls_session_t session,
 			return ret;
 		}
 
-		vers = get_version(session);
-		if (!vers->tls13_sem) {
-			/* Here we need to renegotiate the version since the callee might
-			 * have disabled some TLS versions. We only do it for TLS1.2 or
-			 * earlier, as TLS1.3 uses a different set of ciphersuites, and
-			 * thus we cannot fallback.
-			 */
-			ret = _gnutls_negotiate_version(session, major, minor, 0);
-			if (ret < 0) {
-				gnutls_assert();
-				return ret;
+		/* This callback is often used to switch the priority string of the
+		 * server, and that includes switching version which we have already
+		 * negotiated; note that this doesn't apply when resuming as the version
+		 * negotiation is already complete. */
+		if (session->internals.resumed != RESUME_TRUE) {
+			new_max = _gnutls_version_max(session);
+			old_vers = get_version(session);
+
+			if (!old_vers->tls13_sem || (new_max && !new_max->tls13_sem)) {
+#if GNUTLS_TLS_VERSION_MAX != GNUTLS_TLS1_3
+# error "Need to update the following logic"
+#endif
+				/* Here we need to renegotiate the version since the callee might
+				 * have disabled some TLS versions. This logic does not cope for
+				 * protocols later than TLS1.3 if they have the tls13_sem set */
+				ret = _gnutls_negotiate_version(session, major, minor, 0);
+				if (ret < 0)
+					return gnutls_assert_val(ret);
+
+				vers = get_version(session);
+				if (old_vers != vers) {
+					/* at this point we need to regenerate the random value to
+					 * avoid the peer detecting this session as a rollback
+					 * attempt. */
+					ret = _gnutls_gen_server_random(session, vers->id);
+					if (ret < 0)
+						return gnutls_assert_val(ret);
+				}
 			}
 		}
 	}
diff --git a/lib/priority.c b/lib/priority.c
index a9f0403d26..afd4b1a680 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -593,7 +593,8 @@ gnutls_priority_set(gnutls_session_t session, gnutls_priority_t priority)
 	/* set the current version to the first in the chain.
 	 * This will be overridden later.
 	 */
-	if (session->internals.priorities->protocol.algorithms > 0) {
+	if (session->internals.priorities->protocol.algorithms > 0 &&
+	    !session->internals.handshake_in_progress) {
 		if (_gnutls_set_current_version(session,
 					    session->internals.priorities->
 					    protocol.priority[0]) < 0) {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3680437f16..95f7db4fd0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -167,7 +167,7 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei
 	 tls13-cert-key-exchange x509-cert-callback-ocsp gnutls_ocsp_resp_list_import2 \
 	 server-sign-md5-rep privkey-keygen mini-tls-nonblock no-signal pkcs7-gen dtls-etm \
 	 x509sign-verify-rsa x509sign-verify-ecdsa x509sign-verify-gost \
-	 mini-alignment oids atfork prf psk-file priority-init2 \
+	 mini-alignment oids atfork prf psk-file priority-init2 post-client-hello-change-prio \
 	 status-request status-request-ok status-request-missing sign-verify-ext \
 	 fallback-scsv pkcs8-key-decode urls dtls-rehandshake-cert \
 	 key-usage-rsa key-usage-ecdhe-rsa mini-session-verify-function auto-verify \
diff --git a/tests/post-client-hello-change-prio.c b/tests/post-client-hello-change-prio.c
new file mode 100644
index 0000000000..d2ff4cad85
--- /dev/null
+++ b/tests/post-client-hello-change-prio.c
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2018 Nikos Mavrogiannopoulos
+ *
+ * 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 <errno.h>
+#include <assert.h>
+#include <gnutls/gnutls.h>
+#include "utils.h"
+#include "eagain-common.h"
+#include "cert-common.h"
+
+/* Tests whether the post_client_hello callback can modify
+ * the avalable priorities. This is used by apache's mod_gnutls.
+ */
+
+const char *side;
+static int pch_ok = 0;
+const char *override_prio = NULL;
+
+static int post_client_hello_callback(gnutls_session_t session)
+{
+	assert(gnutls_priority_set_direct(session, override_prio, NULL) >= 0);
+	pch_ok = 1;
+	return 0;
+}
+
+static void tls_log_func(int level, const char *str)
+{
+	fprintf(stderr, "%s|<%d>| %s", side, level, str);
+}
+
+static
+void start(const char *name, const char *prio, gnutls_protocol_t exp_version)
+{
+	/* Server stuff. */
+	gnutls_certificate_credentials_t serverx509cred;
+	gnutls_session_t server;
+	int sret = GNUTLS_E_AGAIN;
+	/* Client stuff. */
+	gnutls_certificate_credentials_t clientx509cred;
+	gnutls_session_t client;
+	int cret = GNUTLS_E_AGAIN;
+
+	success("trying %s\n", name);
+
+	pch_ok = 0;
+
+	/* General init. */
+	global_init();
+	gnutls_global_set_log_function(tls_log_func);
+	if (debug)
+		gnutls_global_set_log_level(4);
+
+	/* Init server */
+	assert(gnutls_certificate_allocate_credentials(&serverx509cred)>=0);
+	assert(gnutls_certificate_set_x509_key_mem(serverx509cred,
+					    &server_cert, &server_key,
+					    GNUTLS_X509_FMT_PEM)>=0);
+	assert(gnutls_init(&server, GNUTLS_SERVER)>=0);
+	gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
+				serverx509cred);
+	assert(gnutls_priority_set_direct(server, prio, NULL)>=0);
+	gnutls_transport_set_push_function(server, server_push);
+	gnutls_transport_set_pull_function(server, server_pull);
+	gnutls_transport_set_ptr(server, server);
+	gnutls_handshake_set_post_client_hello_function(server,
+							post_client_hello_callback);
+
+	assert(gnutls_certificate_allocate_credentials(&clientx509cred)>=0);
+	assert(gnutls_init(&client, GNUTLS_CLIENT)>=0);
+	gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
+				clientx509cred);
+	assert(gnutls_priority_set_direct(client, prio, NULL)>=0);
+	gnutls_transport_set_push_function(client, client_push);
+	gnutls_transport_set_pull_function(client, client_pull);
+	gnutls_transport_set_ptr(client, client);
+
+	HANDSHAKE(client, server);
+
+	assert(exp_version == gnutls_protocol_get_version(client));
+	assert(exp_version == gnutls_protocol_get_version(server));
+
+	gnutls_bye(client, GNUTLS_SHUT_RDWR);
+	gnutls_bye(server, GNUTLS_SHUT_RDWR);
+
+	gnutls_deinit(client);
+	gnutls_deinit(server);
+
+	gnutls_certificate_free_credentials(serverx509cred);
+	gnutls_certificate_free_credentials(clientx509cred);
+
+	gnutls_global_deinit();
+
+	if (pch_ok == 0)
+		fail("Post client hello callback wasn't called\n");
+
+	reset_buffers();
+}
+
+void doit(void)
+{
+	override_prio = "NORMAL";
+	start("tls1.2-only", "NORMAL:-VERS-ALL:+VERS-TLS1.2", GNUTLS_TLS1_2);
+	start("tls1.3-only", "NORMAL:-VERS-ALL:+VERS-TLS1.3", GNUTLS_TLS1_3);
+	start("default", "NORMAL", GNUTLS_TLS1_3);
+	override_prio = "NORMAL:-VERS-ALL:+VERS-TLS1.2";
+	start("default overriden to TLS1.2-only", "NORMAL", GNUTLS_TLS1_2);
+}
diff --git a/tests/resume.c b/tests/resume.c
index 84314b836c..0e582f603d 100644
--- a/tests/resume.c
+++ b/tests/resume.c
@@ -73,6 +73,7 @@ struct params_res {
 	int enable_session_ticket_server;
 	int enable_session_ticket_client;
 	int expect_resume;
+	int call_post_client_hello;
 	int client_cert;
 	int first_no_ext_master;
 	int second_no_ext_master;
@@ -95,6 +96,12 @@ struct params_res resume_tests[] = {
 	 .enable_session_ticket_server = 0,
 	 .enable_session_ticket_client = 0,
 	 .expect_resume = 1},
+	{.desc = "try to resume from db with post_client_hello",
+	 .enable_db = 1,
+	 .enable_session_ticket_server = 0,
+	 .enable_session_ticket_client = 0,
+	 .call_post_client_hello = 1,
+	 .expect_resume = 1},
 	{.desc = "try to resume from db using resumed session's data",
 	 .enable_db = 1,
 	 .enable_session_ticket_server = 0,
@@ -131,6 +138,12 @@ struct params_res resume_tests[] = {
 	 .enable_session_ticket_client = 1,
 	 .change_ciphersuite = 1,
 	 .expect_resume = 1},
+	{.desc = "try to resume from session ticket with post_client_hello",
+	 .enable_db = 0,
+	 .enable_session_ticket_server = 1,
+	 .enable_session_ticket_client = 1,
+	 .call_post_client_hello = 1,
+	 .expect_resume = 1},
 #endif
 #if defined(TLS13) && !defined(USE_PSK)
 	{.desc = "try to resume from session ticket (early start)",
@@ -241,6 +254,13 @@ static void tls_log_func(int level, const char *str)
 		str);
 }
 
+static int post_client_hello_callback(gnutls_session_t session)
+{
+	/* switches the supported ciphersuites to something compatible */
+	assert(gnutls_priority_set_direct(session, gnutls_session_get_ptr(session), NULL) >= 0);
+	return 0;
+}
+
 static int hsk_hook_cb(gnutls_session_t session, unsigned int htype, unsigned post,
 			unsigned int incoming, const gnutls_datum_t *_msg)
 {
@@ -809,6 +829,13 @@ static void server(int sds[], struct params_res *params)
 		gnutls_transport_set_int(session, sd);
 		gnutls_handshake_set_timeout(session, 20 * 1000);
 
+		if (params->call_post_client_hello) {
+			gnutls_session_set_ptr(session, PRIO_STR);
+			gnutls_handshake_set_post_client_hello_function(session,
+									post_client_hello_callback);
+		}
+
+
 		do {
 			ret = gnutls_handshake(session);
 		} while (ret < 0 && gnutls_error_is_fatal(ret) == 0);