Make gnutls compliant to RFC5280

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

7 files changed, 92 insertions, 98 deletions

diff --git a/NEWS b/NEWS
index 39859a6a63..cfc8a7373a 100644
--- 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 351cf4593e..894b301047 100644
--- 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 22b5b8d9f6..48eaf39650 100644
--- 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 50f4196da0..df17b430d6 100644
--- 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 4c5085e7a0..7a63d3e367 100644
--- 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 0c78cd9a1d..3df478449a 100644
--- 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
index 39753c2753..0000000000
--- 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