Prevent applications from combining legacy versions of TLS with TLS1.3tmp-tls10-tls13-fix

It can happen that an application due to a misconfiguration, enables TLS1.3
in combination with TLS1.0 or TLS1.1 only. In that case a server which is
unaware of the TLS1.3 protocol will reply by selecting the TLS1.2 protocol
instead and that answer will be rejected by the client. With this change
we ensure that TLS1.3 is not enabled in these problematic scenarios.

Resolves: #621

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

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();