Merge branch 'tmp-tls10-tls13-fix' into 'master'

Prevent applications from combining legacy versions of TLS with TLS1.3

Closes #621

See merge request gnutls/gnutls!815

3 files changed, 34 insertions, 10 deletions

diff --git a/NEWS b/NEWS
index 30f6ffffc7..87862db2a5 100644
--- a/NEWS
+++ b/NEWS
@@ -21,17 +21,23 @@ See the end for copying conditions.
 ** libgnutls: gnutls_priority_init() and friends, allow the CTYPE-OPENPGP keyword
    in the priority string. It is only accepted as legacy option and is ignored.
 
+** libgnutls: Added support for TLS 1.3 zero round-trip (0-RTT) mode (#127)
+
+** libgnutls: Added support for GOST key unmasking and unwrapped GOST private
+   keys parsing, as specified in R 50.1.112-2016.
+
+** libgnutls: The priority functions will ignore and not enable TLS1.3 if
+   requested with legacy TLS versions enabled but not TLS1.2. That is because
+   if such a priority string is used in the client side (e.g., TLS1.3+TLS1.0 enabled)
+   servers which do not support TLS1.3 will negotiate TLS1.2 which will be
+   rejected by the client as disabled (#621).
+
 ** p11tool: Fix initialization of security officer's PIN with the --initialize-so-pin
    option (#561)
 
 ** gnutls-serv: It applies the default settings when no --priority option is given,
    using gnutls_set_default_priority().
 
-** libgnutls: Added support for TLS 1.3 zero round-trip (0-RTT) mode (#127)
-
-** libgnutls: Added support for GOST key unmasking and unwrapped GOST private
-   keys parsing, as specified in R 50.1.112-2016.
-
 ** certtool: Add parameter --no-text that prevents certtool from outputting
    text before PEM-encoded private key, public key, certificate, CRL or CSR.
 
diff --git a/lib/priority.c b/lib/priority.c
index 11ff9ddce5..a8223a5308 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -1226,6 +1226,7 @@ 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_pre_tls12 = 0, have_tls12 = 0;
 	unsigned have_psk = 0, have_null = 0, have_rsa_psk = 0;
 
 	/* have_psk indicates that a PSK key exchange compatible
@@ -1270,14 +1271,20 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
 			if (vers->tls13_sem)
 				have_tls13 = 1;
 
+			if (vers->id == GNUTLS_TLS1_2)
+				have_tls12 = 1;
+			else if (vers->id < GNUTLS_TLS1_2)
+				have_pre_tls12 = 1;
+
 			if (tlsmax == NULL || vers->age > tlsmax->age)
 				tlsmax = vers;
 			if (tlsmin == NULL || vers->age < tlsmin->age)
 				tlsmin = vers;
 		} else { /* dtls */
 			tls_sig_sem |= vers->tls_sig_sem;
-			if (vers->tls13_sem)
-				have_tls13 = 1;
+
+			/* we need to introduce similar handling to above
+			 * when DTLS1.3 is supported */
 
 			if (dtlsmax == NULL || vers->age > dtlsmax->age)
 				dtlsmax = vers;
@@ -1400,11 +1407,15 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
 	if (unlikely(priority_cache->cs.size == 0))
 		return gnutls_assert_val(GNUTLS_E_NO_PRIORITIES_WERE_SET);
 
-	/* when TLS 1.3 is available we must have groups set */
-	if (unlikely(!have_psk && tlsmax && tlsmax->id >= GNUTLS_TLS1_3 && priority_cache->groups.size == 0)) {
+	/* when TLS 1.3 is available we must have groups set; additionally
+	 * we require TLS1.2 to be enabled if TLS1.3 is asked for, and
+	 * a pre-TLS1.2 protocol is there; that is because servers which
+	 * do not support TLS1.3 will negotiate TLS1.2 if seen a TLS1.3 handshake */
+	if (unlikely((!have_psk && tlsmax && tlsmax->id >= GNUTLS_TLS1_3 && priority_cache->groups.size == 0)) ||
+	    (!have_tls12 && have_pre_tls12 && have_tls13)) {
 		for (i = 0; i < priority_cache->protocol.num_priorities; i++) {
 			vers = version_to_entry(priority_cache->protocol.priorities[i]);
-			if (!vers)
+			if (!vers || vers->transport != GNUTLS_STREAM)
 				continue;
 
 			REMOVE_TLS13_IN_LOOP(vers, i);
diff --git a/tests/version-checks.c b/tests/version-checks.c
index c02bdbe79e..4f9bac2261 100644
--- a/tests/version-checks.c
+++ b/tests/version-checks.c
@@ -164,6 +164,13 @@ void doit(void)
 	reset_buffers();
 	try("NORMAL:-VERS-TLS-ALL:+VERS-TLS1.3", GNUTLS_TLS1_3);
 	reset_buffers();
+	try("NORMAL:-VERS-TLS-ALL:+VERS-TLS1.3:+VERS-TLS1.0", GNUTLS_TLS1_0);
+	reset_buffers();
+	/* similar to above test, but checks a different syntax */
+	try("NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.1", GNUTLS_TLS1_1);
+	reset_buffers();
+	try("NORMAL:-VERS-TLS-ALL:+VERS-TLS1.3:+VERS-TLS1.2", GNUTLS_TLS1_3);
+	reset_buffers();
 #ifdef ENABLE_SSL3
 	try("NORMAL:-VERS-TLS-ALL:+VERS-SSL3.0", -1);
 	reset_buffers();