Enforce the certificate key usage restrictions on all cases

That is, we require a signing certificate when negotiating
TLS1.3, or when sending a client certificate (on all cases).

Before we would not perform any checks under TLS1.3 or when client
certificates are sent, assuming that the certificates used will always
be signing ones. However if the user sets up incorrectly a decryption
certificate we would use it for signing. This fix makes sure that an
error is returned early when these scenarios are detected.

Resolves: #690

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

10 files changed, 104 insertions, 16 deletions

diff --git a/NEWS b/NEWS
index bfc895f0f3..80d5399630 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,16 @@ Copyright (C) 2000-2016 Free Software Foundation, Inc.
 Copyright (C) 2013-2017 Nikos Mavrogiannopoulos
 See the end for copying conditions.
 
+* Version 3.6.7 (unreleased)
+
+** 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).
+
+** API and ABI modifications:
+No changes since last version.
+
+
 * Version 3.6.6 (released 2019-01-25)
 
 ** libgnutls: gnutls_pubkey_import_ecc_raw() was fixed to set the number bits
diff --git a/lib/auth/cert.c b/lib/auth/cert.c
index b6bd3bf91e..53ad91df6d 100644
--- a/lib/auth/cert.c
+++ b/lib/auth/cert.c
@@ -181,6 +181,7 @@ find_x509_client_cert(gnutls_session_t session,
 	ssize_t data_size = _data_size;
 	unsigned i, j;
 	int result, cert_pk;
+	unsigned key_usage;
 
 	*indx = -1;
 
@@ -191,6 +192,16 @@ find_x509_client_cert(gnutls_session_t session,
 	    (data_size == 0
 	     || (session->internals.flags & GNUTLS_FORCE_CLIENT_CERT))) {
 		if (cred->certs[0].cert_list[0].type == GNUTLS_CRT_X509) {
+
+			key_usage = get_key_usage(session, cred->certs[0].cert_list[0].pubkey);
+
+			/* For client certificates we require signatures */
+			result = _gnutls_check_key_usage_for_sig(session, key_usage, 1);
+			if (result < 0) {
+				_gnutls_debug_log("Client certificate is not suitable for signing\n");
+				return gnutls_assert_val(result);
+			}
+
 			/* This check is necessary to prevent sending other certificate
 			 * credentials that are set (e.g. raw public-key). */
 			*indx = 0;
@@ -221,6 +232,14 @@ find_x509_client_cert(gnutls_session_t session,
 				if (odn.size == 0 || odn.size != asked_dn.size)
 					continue;
 
+				key_usage = get_key_usage(session, cred->certs[i].cert_list[0].pubkey);
+
+				/* For client certificates we require signatures */
+				if (_gnutls_check_key_usage_for_sig(session, key_usage, 1) < 0) {
+					_gnutls_debug_log("Client certificate is not suitable for signing\n");
+					continue;
+				}
+
 				/* If the DN matches and
 				 * the *_SIGN algorithm matches
 				 * the cert is our cert!
@@ -1462,11 +1481,11 @@ int cert_select_sign_algorithm(gnutls_session_t session,
 		return gnutls_assert_val(GNUTLS_E_INSUFFICIENT_CREDENTIALS);
 	}
 
-	if (unlikely(session->internals.priorities->allow_server_key_usage_violation)) {
-		key_usage = 0;
-	} else {
-		key_usage = pubkey->key_usage;
-	}
+	key_usage = get_key_usage(session, pubkey);
+
+	/* In TLS1.3 we support only signatures; ensure the selected key supports them */
+	if (ver->tls13_sem && _gnutls_check_key_usage_for_sig(session, key_usage, 1) < 0)
+		return gnutls_assert_val(GNUTLS_E_INSUFFICIENT_CREDENTIALS);
 
 	if (!ver->tls13_sem && !_gnutls_kx_supports_pk_usage(cs->kx_algorithm, pk, key_usage)) {
 		return gnutls_assert_val(GNUTLS_E_INSUFFICIENT_CREDENTIALS);
diff --git a/lib/auth/cert.h b/lib/auth/cert.h
index 3f57ec1c74..3d653cb81c 100644
--- a/lib/auth/cert.h
+++ b/lib/auth/cert.h
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2002-2012 Free Software Foundation, Inc.
- * Copyright (C) 2016-2017 Red Hat, Inc.
+ * Copyright (C) 2016-2019 Red Hat, Inc.
  *
  * Author: Nikos Mavrogiannopoulos
  *
@@ -29,6 +29,7 @@
 #include <gnutls/abstract.h>
 #include <gnutls/compat.h>
 #include <str_array.h>
+#include "abstract_int.h"
 
 #define MAX_OCSP_RESPONSES 8
 
@@ -171,5 +172,12 @@ int _gnutls_gen_rawpk_crt(gnutls_session_t session, gnutls_buffer_st* data);
 int _gnutls_proc_rawpk_crt(gnutls_session_t session,
 				uint8_t * data, size_t data_size);
 
+inline static unsigned get_key_usage(gnutls_session_t session, gnutls_pubkey_t pubkey)
+{
+	if (unlikely(session->internals.priorities->allow_server_key_usage_violation))
+		return 0;
+	else
+		return pubkey->key_usage;
+}
 
 #endif
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index a0c47efa0f..93ffd7cee9 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -1566,9 +1566,9 @@ inline static size_t max_user_send_size(gnutls_session_t session,
  *
  * This function is made static inline for optimization reasons.
  */
-static inline gnutls_certificate_type_t
+inline static gnutls_certificate_type_t
 get_certificate_type(gnutls_session_t session,
-								gnutls_ctype_target_t target)
+		     gnutls_ctype_target_t target)
 {
 	switch (target) {
 		case GNUTLS_CTYPE_CLIENT:
diff --git a/lib/tls-sig.c b/lib/tls-sig.c
index 19357c06a1..73ca4314a3 100644
--- a/lib/tls-sig.c
+++ b/lib/tls-sig.c
@@ -40,8 +40,7 @@
 #include <x509/common.h>
 #include <abstract_int.h>
 
-static
-int check_key_usage_for_sig(gnutls_session_t session, unsigned key_usage, unsigned our_cert)
+int _gnutls_check_key_usage_for_sig(gnutls_session_t session, unsigned key_usage, unsigned our_cert)
 {
 	const char *lstr;
 	unsigned allow_key_usage_violation;
@@ -192,7 +191,7 @@ _gnutls_handshake_sign_data(gnutls_session_t session,
 
 	gnutls_pubkey_get_key_usage(cert->pubkey, &key_usage);
 
-	ret = check_key_usage_for_sig(session, key_usage, 1);
+	ret = _gnutls_check_key_usage_for_sig(session, key_usage, 1);
 	if (ret < 0)
 		return gnutls_assert_val(ret);
 
@@ -336,7 +335,7 @@ _gnutls_handshake_verify_data(gnutls_session_t session,
 
 	gnutls_pubkey_get_key_usage(cert->pubkey, &key_usage);
 
-	ret = check_key_usage_for_sig(session, key_usage, 0);
+	ret = _gnutls_check_key_usage_for_sig(session, key_usage, 0);
 	if (ret < 0)
 		return gnutls_assert_val(ret);
 
@@ -497,7 +496,7 @@ _gnutls_handshake_verify_crt_vrfy(gnutls_session_t session,
 
 	gnutls_pubkey_get_key_usage(cert->pubkey, &key_usage);
 
-	ret = check_key_usage_for_sig(session, key_usage, 0);
+	ret = _gnutls_check_key_usage_for_sig(session, key_usage, 0);
 	if (ret < 0)
 		return gnutls_assert_val(ret);
 
@@ -708,7 +707,7 @@ _gnutls_handshake_sign_crt_vrfy(gnutls_session_t session,
 
 	gnutls_pubkey_get_key_usage(cert->pubkey, &key_usage);
 
-	ret = check_key_usage_for_sig(session, key_usage, 1);
+	ret = _gnutls_check_key_usage_for_sig(session, key_usage, 1);
 	if (ret < 0)
 		return gnutls_assert_val(ret);
 
diff --git a/lib/tls-sig.h b/lib/tls-sig.h
index afea4ebaf9..6e0564de1b 100644
--- a/lib/tls-sig.h
+++ b/lib/tls-sig.h
@@ -32,6 +32,9 @@
  */
 #define MAX_SIG_SIZE (19 + MAX_HASH_SIZE)
 
+int _gnutls_check_key_usage_for_sig(gnutls_session_t session, unsigned key_usage,
+				    unsigned our_cert);
+
 int _gnutls_handshake_sign_crt_vrfy(gnutls_session_t session,
 				    gnutls_pcert_st * cert,
 				    gnutls_privkey_t pkey,
diff --git a/lib/tls13-sig.c b/lib/tls13-sig.c
index 8eea6166b3..1f3a74bb5e 100644
--- a/lib/tls13-sig.c
+++ b/lib/tls13-sig.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Red Hat, Inc.
+ * Copyright (C) 2017-2019 Red Hat, Inc.
  *
  * Author: Nikos Mavrogiannopoulos
  *
@@ -27,6 +27,7 @@
 #include <ext/signature.h>
 #include <abstract_int.h>
 #include "tls13-sig.h"
+#include "tls-sig.h"
 #include "hash_int.h"
 
 #undef PREFIX_SIZE
@@ -48,6 +49,7 @@ _gnutls13_handshake_verify_data(gnutls_session_t session,
 	const version_entry_st *ver = get_version(session);
 	gnutls_buffer_st buf;
 	uint8_t prefix[PREFIX_SIZE];
+	unsigned key_usage = 0;
 	gnutls_datum_t p;
 
 	_gnutls_handshake_log
@@ -75,6 +77,12 @@ _gnutls13_handshake_verify_data(gnutls_session_t session,
 	if (se->tls13_ok == 0) /* explicitly prohibited */
 		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 
+	gnutls_pubkey_get_key_usage(cert->pubkey, &key_usage);
+
+	ret = _gnutls_check_key_usage_for_sig(session, key_usage, 0);
+	if (ret < 0)
+		return gnutls_assert_val(ret);
+
 	_gnutls_buffer_init(&buf);
 
 	memset(prefix, 0x20, sizeof(prefix));
@@ -150,6 +158,7 @@ _gnutls13_handshake_sign_data(gnutls_session_t session,
 	if (unlikely(sign_supports_priv_pk_algorithm(se, pkey->pk_algorithm) == 0))
 		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 
+	/* when we reach here we know we have a signing certificate */
 	_gnutls_handshake_log
 	    ("HSK[%p]: signing TLS 1.3 handshake data: using %s and PRF: %s\n", session, se->name,
 	     session->security_parameters.prf->name);
diff --git a/tests/cipher-neg-common.c b/tests/cipher-neg-common.c
index bfbda8b05b..1fcd6048b3 100644
--- a/tests/cipher-neg-common.c
+++ b/tests/cipher-neg-common.c
@@ -54,8 +54,8 @@ static void try(test_case_st *test)
 	gnutls_certificate_set_known_dh_params(s_cert_cred, GNUTLS_SEC_PARAM_MEDIUM);
 
 	assert(gnutls_certificate_set_x509_key_mem(s_cert_cred, &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, GNUTLS_X509_FMT_PEM) >= 0);
-	assert(gnutls_certificate_set_x509_key_mem(s_cert_cred, &server_ca3_localhost_ecc_cert, &server_ca3_ecc_key, GNUTLS_X509_FMT_PEM) >= 0);
 	assert(gnutls_certificate_set_x509_key_mem(s_cert_cred, &server_ca3_localhost_rsa_sign_cert, &server_ca3_key, GNUTLS_X509_FMT_PEM) >= 0);
+	assert(gnutls_certificate_set_x509_key_mem(s_cert_cred, &server_ca3_localhost_ecc_cert, &server_ca3_ecc_key, GNUTLS_X509_FMT_PEM) >= 0);
 
 	gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, c_cert_cred);
 
diff --git a/tests/tls12-cert-key-exchange.c b/tests/tls12-cert-key-exchange.c
index 7811ae85bb..da26e87a3b 100644
--- a/tests/tls12-cert-key-exchange.c
+++ b/tests/tls12-cert-key-exchange.c
@@ -120,5 +120,25 @@ void doit(void)
 			GNUTLS_E_AGAIN, GNUTLS_E_UNWANTED_ALGORITHM,
 			&server_ca3_rsa_pss_cert, &server_ca3_rsa_pss_key, &cli_ca3_cert, &cli_ca3_key);
 
+	try_with_key_fail("TLS 1.2 with rsa encryption cert without RSA",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.2:-RSA",
+			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.2 with (forced) rsa encryption cert and no RSA - client should detect",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.2:-RSA:%DEBUG_ALLOW_KEY_USAGE_VIOLATIONS",
+			GNUTLS_E_AGAIN, GNUTLS_E_KEY_USAGE_VIOLATION,
+			&server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, NULL, NULL);
+
+	try_with_key_fail("TLS 1.2 with client rsa encryption cert",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.2",
+			GNUTLS_E_AGAIN, GNUTLS_E_KEY_USAGE_VIOLATION,
+			&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.2 with (forced) client rsa encryption cert - server should detect",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.2:%DEBUG_ALLOW_KEY_USAGE_VIOLATIONS",
+			GNUTLS_E_KEY_USAGE_VIOLATION, GNUTLS_E_AGAIN,
+			&server_ca3_rsa_pss_cert, &server_ca3_rsa_pss_key, &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key);
+
 	gnutls_global_deinit();
 }
diff --git a/tests/tls13-cert-key-exchange.c b/tests/tls13-cert-key-exchange.c
index 8b72b8a8d6..066c7d2fb0 100644
--- a/tests/tls13-cert-key-exchange.c
+++ b/tests/tls13-cert-key-exchange.c
@@ -138,5 +138,25 @@ void doit(void)
 			GNUTLS_E_AGAIN, GNUTLS_E_INCOMPATIBLE_SIG_WITH_KEY,
 			&server_ca3_rsa_pss_cert, &server_ca3_rsa_pss_key, &cli_ca3_cert, &cli_ca3_key);
 
+	try_with_key_fail("TLS 1.3 with rsa encryption cert",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.3",
+			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 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,
+			&server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, NULL, NULL);
+
+	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,
+			&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",
+			"NORMAL:-VERS-ALL:+VERS-TLS1.3:%DEBUG_ALLOW_KEY_USAGE_VIOLATIONS",
+			GNUTLS_E_KEY_USAGE_VIOLATION, GNUTLS_E_SUCCESS,
+			&server_ca3_rsa_pss_cert, &server_ca3_rsa_pss_key, &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key);
+
 	gnutls_global_deinit();
 }