Fallback to TLS 1.2 when incompatible with signature certs are provided

This only takes into account certificates in the credentials structure.
If certificates are provided in a callback, these must be checked by
the provider. For that we assume that the credentials structure is
filled when associated with a session; if not then the fallback mechanism
will not work and the handshake will fail.

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

7 files changed, 70 insertions, 34 deletions

diff --git a/NEWS b/NEWS
index 80d5399630..af6aee6872 100644
--- a/NEWS
+++ b/NEWS
@@ -9,7 +9,9 @@ See the end for copying conditions.
 
 ** libgnutls: enforce key usage limitations on certificates more actively.
    Previously we would enforce it for TLS1.2 protocol, now we enforce it
-   even when TLS1.3 is negotiated, or on client certificates as well (#690).
+   even when TLS1.3 is negotiated, or on client certificates as well. When
+   an inappropriate for TLS1.3 certificate is seen on the credentials structure
+   GnuTLS will disable TLS1.3 support for that session (#690).
 
 ** API and ABI modifications:
 No changes since last version.
diff --git a/lib/algorithms/ciphersuites.c b/lib/algorithms/ciphersuites.c
index b97bbc82db..7269861ffe 100644
--- a/lib/algorithms/ciphersuites.c
+++ b/lib/algorithms/ciphersuites.c
@@ -1578,7 +1578,7 @@ _gnutls_figure_common_ciphersuite(gnutls_session_t session,
 	/* RFC7919 requires that we reply with insufficient security if we have
 	 * negotiated an FFDHE group, but cannot find a common ciphersuite. However,
 	 * we must also distinguish between not matching a ciphersuite due to an
-	 * incompatible certificate which we traditionally return GNUTLS_E_INSUFFICIENT_SECURITY.
+	 * incompatible certificate which we traditionally return GNUTLS_E_NO_CIPHER_SUITES.
 	 */
 	if (!no_cert_found && (session->internals.hsk_flags & HSK_HAVE_FFDHE) &&
 	    session->internals.priorities->groups.have_ffdhe && !version->tls13_sem)
diff --git a/lib/auth.c b/lib/auth.c
index 91a67c9afa..dd3fc861fb 100644
--- a/lib/auth.c
+++ b/lib/auth.c
@@ -138,6 +138,29 @@ gnutls_credentials_set(gnutls_session_t session,
 		}
 	}
 
+	/* sanity tests */
+	if (type == GNUTLS_CRD_CERTIFICATE) {
+		gnutls_certificate_credentials_t c = cred;
+		unsigned i;
+		bool allow_tls13 = 0;
+		unsigned key_usage;
+
+		if (c != NULL && c->ncerts != 0) {
+			for (i = 0; i < c->ncerts; i++) {
+				key_usage = get_key_usage(session, c->certs[i].cert_list[0].pubkey);
+				if (key_usage == 0 || (key_usage & GNUTLS_KEY_DIGITAL_SIGNATURE)) {
+					allow_tls13 = 1;
+					break;
+				}
+			}
+
+			if (!allow_tls13) {
+				/* to prevent the server random indicate TLS1.3 support */
+				session->internals.flags |= INT_FLAG_NO_TLS13;
+			}
+		}
+	}
+
 	return 0;
 }
 
diff --git a/lib/auth/cert.h b/lib/auth/cert.h
index 3d653cb81c..fd66628820 100644
--- a/lib/auth/cert.h
+++ b/lib/auth/cert.h
@@ -174,7 +174,8 @@ int _gnutls_proc_rawpk_crt(gnutls_session_t session,
 
 inline static unsigned get_key_usage(gnutls_session_t session, gnutls_pubkey_t pubkey)
 {
-	if (unlikely(session->internals.priorities->allow_server_key_usage_violation))
+	if (unlikely(session->internals.priorities &&
+	    session->internals.priorities->allow_server_key_usage_violation))
 		return 0;
 	else
 		return pubkey->key_usage;
diff --git a/lib/handshake.c b/lib/handshake.c
index 70b4486266..481210ebc0 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -444,6 +444,9 @@ _gnutls_negotiate_version(gnutls_session_t session,
 
 		if (aversion && aversion->id == GNUTLS_TLS1_2) {
 			vers = _gnutls_version_max(session);
+			if (unlikely(vers == NULL))
+				return gnutls_assert_val(GNUTLS_E_NO_CIPHER_SUITES);
+
 			if (vers->id >= GNUTLS_TLS1_2) {
 				session->security_parameters.pversion = aversion;
 				return 0;
@@ -2138,7 +2141,10 @@ static int send_client_hello(gnutls_session_t session, int again)
 
 		if (hver == NULL) {
 			gnutls_assert();
-			ret = GNUTLS_E_NO_PRIORITIES_WERE_SET;
+			if (session->internals.flags & INT_FLAG_NO_TLS13)
+				ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS;
+			else
+				ret = GNUTLS_E_NO_PRIORITIES_WERE_SET;
 			goto cleanup;
 		}
 
diff --git a/tests/common-cert-key-exchange.c b/tests/common-cert-key-exchange.c
index 468475f846..c0c27a4064 100644
--- a/tests/common-cert-key-exchange.c
+++ b/tests/common-cert-key-exchange.c
@@ -74,7 +74,7 @@ void try_with_key_fail(const char *name, const char *client_prio,
 
 	reset_buffers();
 	/* Init server */
-	gnutls_certificate_allocate_credentials(&serverx509cred);
+	assert(gnutls_certificate_allocate_credentials(&serverx509cred)>=0);
 
 	ret = gnutls_certificate_set_x509_key_mem(serverx509cred,
 						  serv_cert, serv_key,
@@ -82,16 +82,15 @@ void try_with_key_fail(const char *name, const char *client_prio,
 	if (ret < 0)
 		fail("Could not set key/cert: %s\n", gnutls_strerror(ret));
 
-	gnutls_init(&server, GNUTLS_SERVER);
-	gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
-			       serverx509cred);
-
-
+	assert(gnutls_init(&server, GNUTLS_SERVER)>=0);
 	if (server_priority)
 		assert(gnutls_priority_set_direct(server, server_priority, NULL) >= 0);
 	else
 		assert(gnutls_priority_set_direct(server, client_prio, NULL) >= 0);
 
+	assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
+				      serverx509cred)>=0);
+
 	gnutls_transport_set_push_function(server, server_push);
 	gnutls_transport_set_pull_function(server, server_pull);
 	gnutls_transport_set_ptr(server, server);
@@ -112,11 +111,6 @@ void try_with_key_fail(const char *name, const char *client_prio,
 	if (ret < 0)
 		exit(1);
 
-	ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
-				clientx509cred);
-	if (ret < 0)
-		exit(1);
-
 	gnutls_transport_set_push_function(client, client_push);
 	gnutls_transport_set_pull_function(client, client_pull);
 	gnutls_transport_set_ptr(client, client);
@@ -127,6 +121,12 @@ void try_with_key_fail(const char *name, const char *client_prio,
 			fprintf(stderr, "Error in %s\n", err);
 		exit(1);
 	}
+
+	ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
+				clientx509cred);
+	if (ret < 0)
+		exit(1);
+
 	success("negotiating %s\n", name);
 	HANDSHAKE_EXPECT(client, server, client_err, server_err);
 
@@ -173,8 +173,8 @@ void try_with_key_ks(const char *name, const char *client_prio, gnutls_kx_algori
 
 	reset_buffers();
 	/* Init server */
-	gnutls_anon_allocate_server_credentials(&s_anoncred);
-	gnutls_certificate_allocate_credentials(&server_cred);
+	assert(gnutls_anon_allocate_server_credentials(&s_anoncred)>=0);
+	assert(gnutls_certificate_allocate_credentials(&server_cred)>=0);
 
 	// Set server crt creds based on ctype
 	switch (server_ctype) {
@@ -201,11 +201,10 @@ void try_with_key_ks(const char *name, const char *client_prio, gnutls_kx_algori
 	gnutls_certificate_set_dh_params(server_cred, dh_params);
 	gnutls_anon_set_server_dh_params(s_anoncred, dh_params);
 
-	gnutls_init(&server, GNUTLS_SERVER | GNUTLS_ENABLE_RAWPK);
-	gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
-				server_cred);
-	gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred);
-
+	assert(gnutls_init(&server, GNUTLS_SERVER | GNUTLS_ENABLE_RAWPK)>=0);
+	assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
+				      server_cred)>=0);
+	assert(gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred)>=0);
 
 	if (server_priority)
 		assert(gnutls_priority_set_direct(server, server_priority, NULL) >= 0);
@@ -254,8 +253,8 @@ void try_with_key_ks(const char *name, const char *client_prio, gnutls_kx_algori
 		exit(1);
 
 
-	gnutls_anon_allocate_client_credentials(&c_anoncred);
-	gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred);
+	assert(gnutls_anon_allocate_client_credentials(&c_anoncred)>=0);
+	assert(gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred)>=0);
 	ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
 				client_cred);
 	if (ret < 0)
@@ -397,14 +396,14 @@ void dtls_try_with_key_mtu(const char *name, const char *client_prio, gnutls_kx_
 	gnutls_certificate_set_dh_params(serverx509cred, dh_params);
 	gnutls_anon_set_server_dh_params(s_anoncred, dh_params);
 
-	gnutls_init(&server, GNUTLS_SERVER|GNUTLS_DATAGRAM|GNUTLS_NONBLOCK);
-	gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
-				serverx509cred);
-	gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred);
+	assert(gnutls_init(&server, GNUTLS_SERVER|GNUTLS_DATAGRAM|GNUTLS_NONBLOCK)>=0);
+	assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
+				      serverx509cred)>=0);
+	assert(gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred)>=0);
 
-	gnutls_priority_set_direct(server,
-				   "NORMAL:+ANON-ECDH:+ANON-DH:+ECDHE-RSA:+DHE-RSA:+RSA:+ECDHE-ECDSA:+CURVE-X25519",
-				   NULL);
+	assert(gnutls_priority_set_direct(server,
+					  "NORMAL:+ANON-ECDH:+ANON-DH:+ECDHE-RSA:+DHE-RSA:+RSA:+ECDHE-ECDSA:+CURVE-X25519",
+					   NULL)>=0);
 	gnutls_transport_set_push_function(server, server_push);
 	gnutls_transport_set_pull_function(server, server_pull);
 	gnutls_transport_set_pull_timeout_function(server, server_pull_timeout_func);
@@ -440,8 +439,8 @@ void dtls_try_with_key_mtu(const char *name, const char *client_prio, gnutls_kx_
 	if (ret < 0)
 		exit(1);
 
-	gnutls_anon_allocate_client_credentials(&c_anoncred);
-	gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred);
+	assert(gnutls_anon_allocate_client_credentials(&c_anoncred)>=0);
+	assert(gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred)>=0);
 	ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
 				clientx509cred);
 	if (ret < 0)
diff --git a/tests/tls13-cert-key-exchange.c b/tests/tls13-cert-key-exchange.c
index 066c7d2fb0..3a214f9ad1 100644
--- a/tests/tls13-cert-key-exchange.c
+++ b/tests/tls13-cert-key-exchange.c
@@ -143,6 +143,11 @@ void doit(void)
 			GNUTLS_E_NO_CIPHER_SUITES, GNUTLS_E_AGAIN,
 			&server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, NULL, NULL);
 
+	try_with_key_fail("TLS 1.3 and TLS 1.2 with rsa encryption cert",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2",
+			GNUTLS_E_SUCCESS, GNUTLS_E_SUCCESS,
+			&server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, NULL, NULL);
+
 	try_with_key_fail("TLS 1.3 with (forced) rsa encryption cert - client should detect",
 			"NORMAL:-VERS-ALL:+VERS-TLS1.3:%DEBUG_ALLOW_KEY_USAGE_VIOLATIONS",
 			GNUTLS_E_AGAIN, GNUTLS_E_KEY_USAGE_VIOLATION,
@@ -150,7 +155,7 @@ void doit(void)
 
 	try_with_key_fail("TLS 1.3 with client rsa encryption cert",
 			"NORMAL:-VERS-ALL:+VERS-TLS1.3",
-			GNUTLS_E_AGAIN, GNUTLS_E_KEY_USAGE_VIOLATION,
+			GNUTLS_E_AGAIN, GNUTLS_E_INSUFFICIENT_CREDENTIALS,
 			&server_ca3_rsa_pss_cert, &server_ca3_rsa_pss_key, &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key);
 
 	try_with_key_fail("TLS 1.3 with (forced) client rsa encryption cert - server should detect",