_gnutls_verify_crt_status: apply algorithm checks to trusted CAs

If a CA is found in the trusted list, check in addition to
time validity, whether the algorithms comply to the expected
level. This addresses the problem of accepting CAs which would
have been marked as insecure otherwise.

Resolves: #877

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

6 files changed, 160 insertions, 28 deletions

diff --git a/NEWS b/NEWS
index 049f3b4ee0..15e268f2f6 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,11 @@ See the end for copying conditions.
    verification profile. Use '--verify-profile low' for certificate verification
    to apply the 'NORMAL' verification profile.
 
+** libgnutls: If a CA is found in the trusted list, check in addition to
+   time validity, whether the algorithms comply to the expected level prior
+   to accepting it. This addresses the problem of accepting CAs which would
+   have been marked as insecure otherwise (#877).
+
 ** API and ABI modifications:
 gnutls_ocsp_req_const_t: Added
 
diff --git a/lib/x509/verify.c b/lib/x509/verify.c
index 4ca04eb48d..d381b4ee87 100644
--- a/lib/x509/verify.c
+++ b/lib/x509/verify.c
@@ -870,6 +870,36 @@ gnutls_x509_crt_check_issuer(gnutls_x509_crt_t cert,
 	return is_issuer(cert, issuer);
 }
 
+static
+unsigned check_ca_sanity(const gnutls_x509_crt_t issuer,
+			 time_t now, unsigned int flags)
+{
+	unsigned int status = 0;
+	unsigned sigalg;
+	int ret;
+
+	/* explicit time check for trusted CA that we remove from
+	 * list. GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS
+	 */
+	if (!(flags & GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS) &&
+	    !(flags & GNUTLS_VERIFY_DISABLE_TIME_CHECKS)) {
+		status |= check_time_status(issuer, now);
+	}
+
+	ret =
+	    _gnutls_x509_get_signature_algorithm(issuer->cert, "signatureAlgorithm");
+	sigalg = ret;
+
+	/* we explicitly allow CAs which we do not support their self-algorithms
+	 * to pass. */
+	if (ret >= 0 && !is_level_acceptable(issuer, NULL, sigalg, flags)) {
+		status |= GNUTLS_CERT_INSECURE_ALGORITHM|GNUTLS_CERT_INVALID;
+	}
+
+	return status;
+
+}
+
 /* Verify X.509 certificate chain.
  *
  * Note that the return value is an OR of GNUTLS_CERT_* elements.
@@ -928,25 +958,17 @@ _gnutls_verify_crt_status(const gnutls_x509_crt_t * certificate_list,
 			 * CA to self-signed CA at some point. */
 			if (_gnutls_check_if_same_key
 			    (certificate_list[i], trusted_cas[j], i) != 0) {
-				/* explicit time check for trusted CA that we remove from
-				 * list. GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS
-				 */
-
-				if (!(flags & GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS) &&
-					!(flags & GNUTLS_VERIFY_DISABLE_TIME_CHECKS)) {
-					status |=
-					    check_time_status(trusted_cas[j],
-						       now);
-					if (status != 0) {
-						if (func)
-							func(certificate_list[i], trusted_cas[j], NULL, status);
-						return status;
-					}
-				}
+
+				status |= check_ca_sanity(trusted_cas[j], now, flags);
 
 				if (func)
 					func(certificate_list[i],
 					     trusted_cas[j], NULL, status);
+
+				if (status != 0) {
+					return gnutls_assert_val(status);
+				}
+
 				clist_size = i;
 				break;
 			}
@@ -1176,20 +1198,16 @@ _gnutls_pkcs11_verify_crt_status(const char* url,
 
 		if (gnutls_pkcs11_crt_is_known (url, certificate_list[i], vflags) != 0) {
 
-			if (!(flags & GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS) &&
-				!(flags & GNUTLS_VERIFY_DISABLE_TIME_CHECKS)) {
-				status |=
-				    check_time_status(certificate_list[i], now);
-				if (status != 0) {
-					if (func)
-						func(certificate_list[i], certificate_list[i], NULL, status);
-					return status;
-				}
-			}
+			status |= check_ca_sanity(certificate_list[i], now, flags);
+
 			if (func)
 				func(certificate_list[i],
 				     certificate_list[i], NULL, status);
 
+			if (status != 0) {
+				return gnutls_assert_val(status);
+			}
+
 			clist_size = i;
 			break;
 		}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2e46290410..74c74b93d0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -38,7 +38,7 @@ EXTRA_DIST = suppressions.valgrind eagain-common.h cert-common.h test-chains.h \
 	certs/ca-ecc.pem certs/cert-ecc384.pem certs/cert-ecc.pem certs/ecc256.pem \
 	certs/ecc521.pem certs/rsa-2432.pem x509cert-dir/ca.pem psk.passwd \
 	certs/rawpk_priv.pem certs/rawpk_pub.pem \
-	certs/ed25519.pem certs/cert-ed25519.pem \
+	certs/ed25519.pem certs/cert-ed25519.pem certs/rsa-512.pem \
 	system.prio pkcs11/softhsm.h pkcs11/pkcs11-pubkey-import.c gnutls-asan.supp \
 	rsa-md5-collision/README safe-renegotiation/README starttls-smtp.txt starttls-ftp.txt \
 	starttls-lmtp.txt starttls-pop3.txt starttls-xmpp.txt starttls-nntp.txt starttls-sieve.txt \
@@ -502,7 +502,8 @@ dist_check_SCRIPTS += fastopen.sh pkgconfig.sh starttls.sh starttls-ftp.sh start
 	ocsp-tests/ocsp-test cipher-listings.sh sni-hostname.sh server-multi-keys.sh \
 	psktool.sh ocsp-tests/ocsp-load-chain gnutls-cli-save-data.sh gnutls-cli-debug.sh \
 	sni-resume.sh ocsp-tests/ocsptool cert-reencoding.sh pkcs7-cat.sh long-crl.sh \
-	serv-udp.sh logfile-option.sh gnutls-cli-resume.sh profile-tests.sh
+	serv-udp.sh logfile-option.sh gnutls-cli-resume.sh profile-tests.sh \
+	server-weak-keys.sh
 
 if !DISABLE_SYSTEM_CONFIG
 dist_check_SCRIPTS += system-override-sig-hash.sh system-override-versions.sh system-override-invalid.sh \
diff --git a/tests/certs/rsa-512.pem b/tests/certs/rsa-512.pem
new file mode 100644
index 0000000000..46fbe62589
--- /dev/null
+++ b/tests/certs/rsa-512.pem
@@ -0,0 +1,20 @@
+-----BEGIN PRIVATE KEY-----
+MIIBVwIBADANBgkqhkiG9w0BAQEFAASCAUEwggE9AgEAAkEAwZFO/Vz94lR3/TKz
+76qRCV2skqthX7PB6YxeLHH3ifWSYR2qCYTBikaASm6PGDvAliviIjGjKTkdDdqZ
+X2S94QIDAQABAkEAsV+L+FN8OieZBCWwCNBNsz1pY8Uzp1S7Pl3n9eZBJOKNc/tI
+Tr0/zwAR+5C7IE7xjfuYHZDWN+yXg0LhH+GYgQIhAP0rzSdsjuPJ9XA9wpnYLN4O
+fqXnA7mzW5QKzYuzy3RJAiEAw7sCwUSi7030NszYd7A63o2WrzqWRoX1V1vt6FMd
+zNkCIQDmsytXaY0r9bU6eo0CNANutjaiZ0j1x4MD/HQhgc08QQIhALdYYLZF4xKj
+RRZoQIWtURfULciq6sXZCf7xICQ2Z33RAiEA/M/OnKZijdWg13dchmdaXLgNGxJO
+N90VucFVWK8nXzo=
+-----END PRIVATE KEY-----
+-----BEGIN CERTIFICATE-----
+MIIBTjCB+aADAgECAhQcc65I8jSxWRjcS1czw4MRLIc8qDANBgkqhkiG9w0BAQsF
+ADAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMTkxMjE1MDI1NTU4WhcNMjkxMjEy
+MDI1NTU4WjAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwXDANBgkqhkiG9w0BAQEFAANL
+ADBIAkEAwZFO/Vz94lR3/TKz76qRCV2skqthX7PB6YxeLHH3ifWSYR2qCYTBikaA
+Sm6PGDvAliviIjGjKTkdDdqZX2S94QIDAQABoyMwITAJBgNVHRMEAjAAMBQGA1Ud
+EQQNMAuCCWxvY2FsaG9zdDANBgkqhkiG9w0BAQsFAANBAHslvfVxod5p+Gt7l4LV
+M2HBxOt4YM8mRCtyNSmJEGAe+aIzXaiSiRnVkVvjQvdxacu2D4yP52BUo1vzNnCq
+2UI=
+-----END CERTIFICATE-----
diff --git a/tests/server-weak-keys.sh b/tests/server-weak-keys.sh
new file mode 100755
index 0000000000..31c51a80bc
--- /dev/null
+++ b/tests/server-weak-keys.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+
+# Copyright (C) 2017 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 General Public License
+# along with GnuTLS; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+srcdir="${srcdir:-.}"
+SERV="${SERV:-../src/gnutls-serv${EXEEXT}}"
+CLI="${CLI:-../src/gnutls-cli${EXEEXT}}"
+unset RETCODE
+
+if ! test -x "${SERV}"; then
+	exit 77
+fi
+
+if ! test -x "${CLI}"; then
+	exit 77
+fi
+
+if test "${WINDIR}" != ""; then
+	exit 77
+fi
+
+if ! test -z "${VALGRIND}"; then
+	VALGRIND="${LIBTOOL:-libtool} --mode=execute ${VALGRIND} --error-exitcode=15"
+fi
+
+
+SERV="${SERV} -q"
+
+. "${srcdir}/scripts/common.sh"
+
+check_for_datefudge
+
+echo "Checking whether a client will refuse weak but trusted keys"
+
+KEY1=${srcdir}/certs/rsa-512.pem
+CERT1=${srcdir}/certs/rsa-512.pem
+
+eval "${GETPORT}"
+launch_server $$ --echo --priority "NORMAL" --x509keyfile ${KEY1} --x509certfile ${CERT1}
+PID=$!
+wait_server ${PID}
+
+timeout 1800 datefudge "2019-12-20" \
+"${CLI}" -d 4 -p "${PORT}" localhost --x509cafile ${CERT1} --priority NORMAL:-VERS-TLS-ALL:+VERS-TLS1.2 </dev/null && \
+	fail ${PID} "1. handshake with RSA should have failed!"
+
+timeout 1800 datefudge "2019-12-20" \
+"${CLI}" -d 4 -p "${PORT}" localhost --x509cafile ${CERT1} --priority NORMAL </dev/null && \
+	fail ${PID} "2. handshake with RSA should have failed!"
+
+kill ${PID}
+wait
+
+exit 0
diff --git a/tests/test-chains.h b/tests/test-chains.h
index cf4d0c442a..fe118717d4 100644
--- a/tests/test-chains.h
+++ b/tests/test-chains.h
@@ -3981,6 +3981,20 @@ static const char *gost12_512[] = {
 };
 #endif
 
+static const char *rsa_512[] = {
+	"-----BEGIN CERTIFICATE-----\n"
+	"MIIBTjCB+aADAgECAhQcc65I8jSxWRjcS1czw4MRLIc8qDANBgkqhkiG9w0BAQsF\n"
+	"ADAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMTkxMjE1MDI1NTU4WhcNMjkxMjEy\n"
+	"MDI1NTU4WjAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwXDANBgkqhkiG9w0BAQEFAANL\n"
+	"ADBIAkEAwZFO/Vz94lR3/TKz76qRCV2skqthX7PB6YxeLHH3ifWSYR2qCYTBikaA\n"
+	"Sm6PGDvAliviIjGjKTkdDdqZX2S94QIDAQABoyMwITAJBgNVHRMEAjAAMBQGA1Ud\n"
+	"EQQNMAuCCWxvY2FsaG9zdDANBgkqhkiG9w0BAQsFAANBAHslvfVxod5p+Gt7l4LV\n"
+	"M2HBxOt4YM8mRCtyNSmJEGAe+aIzXaiSiRnVkVvjQvdxacu2D4yP52BUo1vzNnCq\n"
+	"2UI=\n"
+	"-----END CERTIFICATE-----\n",
+	NULL
+};
+
 #if defined __clang__ || __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wunused-variable"
@@ -4141,10 +4155,12 @@ static struct
 #ifdef ENABLE_GOST
   { "gost 34.10-01 - ok", gost01, &gost01[2], 0, 0, 0, 1466612070, 1},
   { "gost 34.10-01 - not ok (due to profile)", gost01, &gost01[2], GNUTLS_PROFILE_TO_VFLAGS(GNUTLS_PROFILE_ULTRA),
-	GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID, NULL, 1466612070, 1},
+    GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID, NULL, 1466612070, 1},
   { "gost 34.10-12-256 - ok", gost12_256, &gost12_256[0], 0, 0, 0, 1466612070, 1},
   { "gost 34.10-12-512 - ok", gost12_512, &gost12_512[0], 0, 0, 0, 1466612070, 1},
 #endif
+  { "rsa-512 - not ok (due to profile)", rsa_512, &rsa_512[0], GNUTLS_PROFILE_TO_VFLAGS(GNUTLS_PROFILE_MEDIUM),
+    GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID, NULL, 1576759855, 1},
   { NULL, NULL, NULL, 0, 0}
 };