Make gnutls compliant to RFC5280

Signed-off-by: Zoltan Fridrich <zfridric@redhat.com>

diff --git a/NEWS b/NEWS
index 39859a6a633cb8a676885d09d3adaf10d2a40c96..cfc8a7373af75e4b2cdfc2fe4b4edc5cf171ec9d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,10 @@ See the end for copying conditions.
 * Version 3.7.4 (unreleased)
 
 ** Added support for certificate compression as defined in RFC8879.
+** libgnutls: Added strict-x509 configure option to enforce stricter
+   certificate sanity checks.
+** libgnutls: Removed IA5String type from DirectoryString within issuer
+   and subject name to make DirectoryString RFC5280 compliant.
 
 ** API and ABI modifications:
 gnutls_compress_certificate_get_selected_method: Added

diff --git a/configure.ac b/configure.ac
index 351cf4593ee67db8b1bba013a2358789f2de693b..894b301047c9f353878071997d3bdf7ed5bf1ab8 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -622,6 +622,13 @@ if [ test "$enable_fips" = "yes" ];then
   fi
 fi
 
+AC_ARG_ENABLE(strict-x509,
+  AS_HELP_STRING([--enable-strict-x509], [enable stricter sanity checks for x509 certificates]),
+    enable_strict_x509=$enableval, enable_strict_x509=no)
+if test "$enable_strict_x509" != "no"; then
+       AC_DEFINE([STRICT_X509], 1, [Enable stricter sanity checks for x509 certificates])
+fi
+
 AC_ARG_WITH([pkcs12-iter-count],
        [AS_HELP_STRING([--with-pkcs12-iter-count],
          [specify iteration count for PKCS\#12 key derivation @<:@default=600000@:>@])],

diff --git a/lib/pkix.asn b/lib/pkix.asn
index 22b5b8d9f68086d2f1c6203de56bb306011059df..48eaf396506bad9204e729e9278206a70874a6bf 100644 (file)
--- a/lib/pkix.asn
+++ b/lib/pkix.asn
@@ -36,11 +36,7 @@ DirectoryString ::= CHOICE {
       printableString           PrintableString (SIZE (1..MAX)),
       universalString           UniversalString (SIZE (1..MAX)),
       utf8String              UTF8String (SIZE (1..MAX)),
-      bmpString               BMPString (SIZE(1..MAX)),
-      -- IA5String is added here to handle old UID encoded as ia5String --
-      -- See tests/userid/ for more information.  It shouldn't be here, --
-      -- so if it causes problems, considering dropping it. --
-      ia5String               IA5String (SIZE(1..MAX)) }
+      bmpString               BMPString (SIZE(1..MAX)) }
 
 SubjectAltName ::= GeneralNames
 

diff --git a/lib/pkix_asn1_tab.c b/lib/pkix_asn1_tab.c
index 50f4196da0959be0d54240aea3ba615728de1407..df17b430d6721b6f0f2d1b423b15ad7b6ba17a7d 100644 (file)
--- a/lib/pkix_asn1_tab.c
+++ b/lib/pkix_asn1_tab.c
@@ -22,21 +22,13 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "SubjectKeyIdentifier", 1073741831, NULL },
   { "KeyUsage", 1073741830, NULL },
   { "DirectoryString", 1610612754, NULL },
-  { "teletexString", 1612709918, NULL },
-  { "MAX", 524298, "1"},
-  { "printableString", 1612709919, NULL },
-  { "MAX", 524298, "1"},
-  { "universalString", 1612709920, NULL },
-  { "MAX", 524298, "1"},
-  { "utf8String", 1612709922, NULL },
-  { "MAX", 524298, "1"},
-  { "bmpString", 1612709921, NULL },
-  { "MAX", 524298, "1"},
-  { "ia5String", 538968093, NULL },
-  { "MAX", 524298, "1"},
+  { "teletexString", 1075839006, NULL },
+  { "printableString", 1075839007, NULL },
+  { "universalString", 1075839008, NULL },
+  { "utf8String", 1075839010, NULL },
+  { "bmpString", 2097185, NULL },
   { "SubjectAltName", 1073741826, "GeneralNames"},
   { "GeneralNames", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "GeneralName"},
   { "GeneralName", 1610612754, NULL },
   { "otherName", 1610620930, "AnotherName"},
@@ -67,10 +59,8 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "BasicConstraints", 1610612741, NULL },
   { "cA", 1610645508, NULL },
   { NULL, 131081, NULL },
-  { "pathLenConstraint", 537411587, NULL },
-  { "0", 10, "MAX"},
+  { "pathLenConstraint", 16387, NULL },
   { "CRLDistributionPoints", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "DistributionPoint"},
   { "DistributionPoint", 1610612741, NULL },
   { "distributionPoint", 1610637314, "DistributionPointName"},
@@ -86,10 +76,8 @@ const asn1_static_node pkix_asn1_tab[] = {
   { NULL, 4104, "1"},
   { "ReasonFlags", 1073741830, NULL },
   { "ExtKeyUsageSyntax", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 12, NULL },
   { "AuthorityInfoAccessSyntax", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "AccessDescription"},
   { "AccessDescription", 1610612741, NULL },
   { "accessMethod", 1073741836, NULL },
@@ -107,7 +95,6 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "DistinguishedName", 1610612747, NULL },
   { NULL, 2, "RelativeDistinguishedName"},
   { "RelativeDistinguishedName", 1612709903, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "AttributeTypeAndValue"},
   { "Certificate", 1610612741, NULL },
   { "tbsCertificate", 1073741826, "TBSCertificate"},
@@ -141,7 +128,6 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "algorithm", 1073741826, "AlgorithmIdentifier"},
   { "subjectPublicKey", 6, NULL },
   { "Extensions", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "Extension"},
   { "Extension", 1610612741, NULL },
   { "extnID", 1073741836, NULL },
@@ -219,7 +205,6 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "unsignedAttrs", 536895490, "SignedAttributes"},
   { NULL, 4104, "1"},
   { "SignedAttributes", 1612709903, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "Attribute"},
   { "SignerIdentifier", 1610612754, NULL },
   { "issuerAndSerialNumber", 1073741826, "IssuerAndSerialNumber"},
@@ -261,16 +246,11 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "encryptionAlgorithm", 1073741826, "AlgorithmIdentifier"},
   { "encryptedData", 2, "pkcs-8-EncryptedData"},
   { "pkcs-8-EncryptedData", 1073741831, NULL },
-  { "pkcs-5-des-CBC-params", 1612709895, NULL },
-  { NULL, 1048586, "8"},
-  { "pkcs-5-des-EDE3-CBC-params", 1612709895, NULL },
-  { NULL, 1048586, "8"},
-  { "pkcs-5-aes128-CBC-params", 1612709895, NULL },
-  { NULL, 1048586, "16"},
-  { "pkcs-5-aes192-CBC-params", 1612709895, NULL },
-  { NULL, 1048586, "16"},
-  { "pkcs-5-aes256-CBC-params", 1612709895, NULL },
-  { NULL, 1048586, "16"},
+  { "pkcs-5-des-CBC-params", 1075838983, NULL },
+  { "pkcs-5-des-EDE3-CBC-params", 1075838983, NULL },
+  { "pkcs-5-aes128-CBC-params", 1075838983, NULL },
+  { "pkcs-5-aes192-CBC-params", 1075838983, NULL },
+  { "pkcs-5-aes256-CBC-params", 1075838983, NULL },
   { "Gost28147-89-Parameters", 1610612741, NULL },
   { "iv", 1073741831, NULL },
   { "encryptionParamSet", 12, NULL },
@@ -284,10 +264,8 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "salt", 1610612754, NULL },
   { "specified", 1073741831, NULL },
   { "otherSource", 2, "AlgorithmIdentifier"},
-  { "iterationCount", 1611137027, NULL },
-  { "1", 10, "MAX"},
-  { "keyLength", 1611153411, NULL },
-  { "1", 10, "MAX"},
+  { "iterationCount", 1073741827, NULL },
+  { "keyLength", 1073758211, NULL },
   { "prf", 16386, "AlgorithmIdentifier"},
   { "pkcs-12-PFX", 1610612741, NULL },
   { "version", 1610874883, NULL },
@@ -341,22 +319,18 @@ const asn1_static_node pkix_asn1_tab[] = {
   { NULL, 4104, "0"},
   { "pkcs-7-ContentEncryptionAlgorithmIdentifier", 1073741826, "AlgorithmIdentifier"},
   { "pkcs-7-UnprotectedAttributes", 1612709903, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "Attribute"},
   { "ProxyCertInfo", 1610612741, NULL },
-  { "pCPathLenConstraint", 1611153411, NULL },
-  { "0", 10, "MAX"},
+  { "pCPathLenConstraint", 1073758211, NULL },
   { "proxyPolicy", 2, "ProxyPolicy"},
   { "ProxyPolicy", 1610612741, NULL },
   { "policyLanguage", 1073741836, NULL },
   { "policy", 16391, NULL },
   { "certificatePolicies", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "PolicyInformation"},
   { "PolicyInformation", 1610612741, NULL },
   { "policyIdentifier", 1073741836, NULL },
   { "policyQualifiers", 538984459, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "PolicyQualifierInfo"},
   { "PolicyQualifierInfo", 1610612741, NULL },
   { "policyQualifierId", 1073741836, NULL },
@@ -371,14 +345,10 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "noticeNumbers", 536870923, NULL },
   { NULL, 3, NULL },
   { "DisplayText", 1610612754, NULL },
-  { "ia5String", 1612709917, NULL },
-  { "200", 524298, "1"},
-  { "visibleString", 1612709923, NULL },
-  { "200", 524298, "1"},
-  { "bmpString", 1612709921, NULL },
-  { "200", 524298, "1"},
-  { "utf8String", 538968098, NULL },
-  { "200", 524298, "1"},
+  { "ia5String", 1075839005, NULL },
+  { "visibleString", 1075839011, NULL },
+  { "bmpString", 1075839009, NULL },
+  { "utf8String", 2097186, NULL },
   { "OCSPRequest", 1610612741, NULL },
   { "tbsRequest", 1073741826, "TBSRequest"},
   { "optionalSignature", 536895490, "Signature"},
@@ -472,7 +442,6 @@ const asn1_static_node pkix_asn1_tab[] = {
   { "excludedSubtrees", 536895490, "GeneralSubtrees"},
   { NULL, 4104, "1"},
   { "GeneralSubtrees", 1612709899, NULL },
-  { "MAX", 1074266122, "1"},
   { NULL, 2, "GeneralSubtree"},
   { "GeneralSubtree", 1610612741, NULL },
   { "base", 1073741826, "GeneralName"},

diff --git a/lib/x509/x509.c b/lib/x509/x509.c
index 4c5085e7a0dcee5675d8c634ebdff16664d8b48f..7a63d3e3679334a023fb40e47a2754812393b876 100644 (file)
--- a/lib/x509/x509.c
+++ b/lib/x509/x509.c
@@ -421,6 +421,50 @@ static size_t hhasher(const void *entry, size_t n)
        return hash_pjw_bare(e, strlen(e)) % n;
 }
 
+#ifdef STRICT_X509
+
+/* Check whether certificates serial number is RFC5280 compliant */
+static bool has_valid_serial(gnutls_x509_crt_t cert)
+{
+       int err, is_zero;
+       unsigned i;
+       unsigned char serial[128];
+       size_t serial_size = sizeof(serial);
+
+       err = gnutls_x509_crt_get_serial(cert, serial, &serial_size);
+       if (err < 0) {
+               _gnutls_debug_log("error: could not read serial number\n");
+               return false;
+       }
+
+       if (serial_size > 20) {
+               _gnutls_debug_log("error: serial number value is longer than 20 octets\n");
+               return false;
+       }
+
+       if (serial[0] & 0x80) {
+               _gnutls_debug_log("error: serial number is negative\n");
+               return false;
+       }
+
+       is_zero = 1;
+       for (i = 0; i < serial_size; ++i) {
+               if (serial[i]) {
+                       is_zero = 0;
+                       break;
+               }
+       }
+
+       if (is_zero) {
+               _gnutls_debug_log("error: serial number is zero\n");
+               return false;
+       }
+
+       return true;
+}
+
+#endif /* STRICT_X509 */
+
 int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
 {
        int ret = 0, version;
@@ -438,6 +482,14 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
 
        version = ret;
 
+#ifdef STRICT_X509
+       /* enforce upper bound on certificate version (RFC5280 compliant) */
+       if (version > 3) {
+               _gnutls_debug_log("error: invalid certificate version %d\n", version);
+               return gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
+       }
+#endif
+
        if (version < 3) {
                if (!cert->modified) {
                        ret = _gnutls_x509_get_raw_field2(cert->cert, &cert->der,
@@ -453,8 +505,7 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
                        }
                }
        } else {
-               /* Version is >= 3; ensure no duplicate extensions are
-                * present. */
+               /* Version is 3; ensure no duplicate extensions are present. */
                unsigned i;
                char oid[MAX_OID_SIZE];
                size_t oid_size;
@@ -518,6 +569,13 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
                }
        }
 
+#ifdef STRICT_X509
+       if (!has_valid_serial(cert)) {
+               ret = gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
+               goto cleanup;
+       }
+#endif
+
        if (gnutls_x509_crt_get_expiration_time(cert) == -1 ||
            gnutls_x509_crt_get_activation_time(cert) == -1) {
                _gnutls_debug_log("error: invalid expiration or activation time in certificate\n");

diff --git a/tests/cert-tests/Makefile.am b/tests/cert-tests/Makefile.am
index 0c78cd9a1d58714b95e581bb0f083e6dcba7d7c6..3df478449a81a9bf0d0d172a4158a5b957e59325 100644 (file)
--- a/tests/cert-tests/Makefile.am
+++ b/tests/cert-tests/Makefile.am
@@ -46,7 +46,7 @@ EXTRA_DIST = data/ca-no-pathlen.pem data/no-ca-or-pathlen.pem data/aki-cert.pem
        data/name-constraints-ip2.pem data/chain-md5.pem data/pubkey-ecc256.pem \
        templates/template-dates-after2038.tmpl data/template-dates-after2038.pem \
        data/gost-cert.pem data/gost-cert-nogost.pem data/gost94-cert.pem \
-       templates/template-tlsfeature.tmpl data/userid.pem data/cert-with-crl.p12 \
+       templates/template-tlsfeature.tmpl data/cert-with-crl.p12 \
        data/template-tlsfeature.pem data/template-tlsfeature.csr \
        templates/template-tlsfeature-crq.tmpl templates/arb-extensions.tmpl data/arb-extensions.pem \
        data/arb-extensions.csr data/pkcs1-pad-ok.pem data/pkcs1-pad-broken.pem \
@@ -106,7 +106,7 @@ EXTRA_DIST = data/ca-no-pathlen.pem data/no-ca-or-pathlen.pem data/aki-cert.pem
 
 dist_check_SCRIPTS = pathlen.sh aki.sh invalid-sig.sh email.sh \
        pkcs7.sh pkcs7-broken-sigs.sh privkey-import.sh name-constraints.sh certtool-long-cn.sh crl.sh provable-privkey.sh \
-       provable-dh.sh userid.sh sha2-test.sh sha2-dsa-test.sh provable-privkey-dsa2048.sh \
+       provable-dh.sh sha2-test.sh sha2-dsa-test.sh provable-privkey-dsa2048.sh \
        provable-privkey-rsa2048.sh provable-privkey-gen-default.sh pkcs7-constraints.sh \
        pkcs7-constraints2.sh certtool-long-oids.sh pkcs7-cat.sh cert-sanity.sh cert-critical.sh \
        pkcs12.sh certtool-crl-decoding.sh pkcs12-encode.sh pkcs12-corner-cases.sh inhibit-anypolicy.sh \

diff --git a/tests/cert-tests/userid.sh b/tests/cert-tests/userid.sh
deleted file mode 100755 (executable)
index 39753c2..0000000
--- a/tests/cert-tests/userid.sh
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/bin/sh
-
-# Copyright (C) 2006, 2008, 2010, 2012 Free Software Foundation, Inc.
-#
-# Author: Simon Josefsson
-#
-# 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=.}
-: ${CERTTOOL=../../src/certtool${EXEEXT}}
-
-if ! test -x "${CERTTOOL}"; then
-       exit 77
-fi
-
-"${CERTTOOL}" --certificate-info --infile "${srcdir}/data/userid.pem" >out 2>&1
-RET=$?
-if [ ${RET} !=  0 ]; then
-       echo "Error in userid:"
-       cat out
-       exit 1
-fi
-
-rm -f out
-
-exit 0