_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>

diff --git a/NEWS b/NEWS
index 049f3b4ee0c46c64c88f1d432bc4802490b80fbe..15e268f2f6532dfd7bf15254e5fc545b21fe0cb7 100644 (file)
--- 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 4ca04eb48d31c913203cf0eb4512b5d166dd95bd..d381b4ee87a290e6b0348fc642a2ffd7e59a8ff9 100644 (file)
--- 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 2e46290410315bc885977a8f31d39c910e260e69..74c74b93d0cc3b3e5ceafdb4293afe610ae153ff 100644 (file)
--- 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 (file)
index 0000000..46fbe62
--- /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 (executable)
index 0000000..31c51a8
--- /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 cf4d0c442a1a9cd375154c1b1bd6945a3c1d0e1a..fe118717d4442f3de7f24eac956e41fb632f8e2d 100644 (file)
--- 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}
 };