x509: do not tolerate invalid DER time

This effectively reverts !400 and ensures that we no longer tolerate
invalid DER time. This complements the previous commit by Lili Quan
and ensures we provide the --disable-strict-der-time backwards compatibility
option.

Resolves: #207

Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org>

10 files changed, 201 insertions, 6 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2283ccc333..d1835a0a7f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -161,7 +161,7 @@ SSL-3.0.Fedora.x86_64:
   script:
   - ./bootstrap
   - mkdir -p build && cd build &&
-    dash ../configure --disable-tls13-interop --disable-gcc-warnings --cache-file ../cache/config.cache --enable-sha1-support --enable-ssl3-support --enable-seccomp-tests --disable-doc --disable-guile &&
+    dash ../configure --disable-tls13-interop --disable-gcc-warnings --cache-file ../cache/config.cache --enable-sha1-support --enable-ssl3-support --enable-seccomp-tests --disable-doc --disable-guile --disable-strict-der-time &&
     make -j$(nproc) && make check -j$(nproc)
   - cd ..
   tags:
diff --git a/NEWS b/NEWS
index 67051289ab..51f1f05779 100644
--- a/NEWS
+++ b/NEWS
@@ -10,7 +10,10 @@ See the end for copying conditions.
 ** libgnutls: Introduced the gnutls_ocsp_req_const_t which is compatible
    with gnutls_ocsp_req_t but const.
 
-** libgnutls: Reject certificates with invalid characters in Time fields (#870).
+** libgnutls: Reject certificates with invalid time fields. That is we reject
+   certificates with invalid characters in Time fields, or invalid time formatting
+   To continue accepting the invalid form compile with --disable-strict-der-time
+   (#207, #870).
 
 ** libgnutls: Added support for GOST CNT_IMIT ciphersuite (as defined by
    draft-smyshlyaev-tls12-gost-suites-06).
diff --git a/configure.ac b/configure.ac
index aaeb6bfb58..7f27ae6932 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1132,6 +1132,7 @@ if features are disabled)
   IDNA support:         $idna_support
   Non-SuiteB curves:    $enable_non_suiteb
   FIPS140 mode:         $enable_fips
+  Strict DER time:	$ac_strict_der_time
 ])
 
 AC_MSG_NOTICE([Optional libraries:
diff --git a/lib/x509/common.h b/lib/x509/common.h
index 5bbbdfaebd..d36c263a58 100644
--- a/lib/x509/common.h
+++ b/lib/x509/common.h
@@ -279,10 +279,10 @@ int _gnutls_check_if_sorted(gnutls_x509_crt_t * crt, int nr);
 inline static int _asn1_strict_der_decode (asn1_node * element, const void *ider,
 		       int len, char *errorDescription)
 {
-#ifdef ASN1_DECODE_FLAG_ALLOW_INCORRECT_TIME
-# define _ASN1_DER_FLAGS ASN1_DECODE_FLAG_ALLOW_INCORRECT_TIME|ASN1_DECODE_FLAG_STRICT_DER
-#else
+#if defined(STRICT_DER_TIME) || !defined(ASN1_DECODE_FLAG_ALLOW_INCORRECT_TIME)
 # define _ASN1_DER_FLAGS ASN1_DECODE_FLAG_STRICT_DER
+#else
+# define _ASN1_DER_FLAGS (ASN1_DECODE_FLAG_ALLOW_INCORRECT_TIME|ASN1_DECODE_FLAG_STRICT_DER)
 #endif
 	return asn1_der_decoding2(element, ider, &len, _ASN1_DER_FLAGS, errorDescription);
 }
diff --git a/lib/x509/time.c b/lib/x509/time.c
index 2843d32345..fa10a91002 100644
--- a/lib/x509/time.c
+++ b/lib/x509/time.c
@@ -184,12 +184,15 @@ time_t _gnutls_utcTime2gtime(const char *ttime)
 		gnutls_assert();
 		return (time_t) - 1;
 	}
+
+#ifdef STRICT_DER_TIME
 	/* Make sure everything else is digits. */
 	for (i = 0; i < len - 1; i++) {
 		if (c_isdigit(ttime[i]))
 			continue;
 		return gnutls_assert_val((time_t)-1);
 	}
+#endif
 	xx[2] = 0;
 
 /* get the year
diff --git a/m4/hooks.m4 b/m4/hooks.m4
index 34a5b38eb9..49367bd1da 100644
--- a/m4/hooks.m4
+++ b/m4/hooks.m4
@@ -144,6 +144,20 @@ LIBTASN1_MINIMUM=4.9
     AC_MSG_WARN([C99 macros not supported. This may affect compiling.])
   ])
 
+  ac_strict_der_time=yes
+  AC_MSG_CHECKING([whether to disable strict DER time encodings for backwards compatibility])
+  AC_ARG_ENABLE(strict-der-time,
+    AS_HELP_STRING([--disable-strict-der-time],
+                   [allow non compliant DER time values]),
+    ac_strict_der_time=$enableval)
+  if test x$ac_strict_der_time != xno; then
+   AC_MSG_RESULT(no)
+   AC_DEFINE([STRICT_DER_TIME], 1, [force strict DER time constraints])
+  else
+   AC_MSG_RESULT(yes)
+  fi
+  AM_CONDITIONAL(STRICT_DER_TIME, test "$ac_strict_der_time" != "no")
+
   ac_allow_sha1=no
   AC_MSG_CHECKING([whether to allow SHA1 as an acceptable hash for cert digital signatures])
   AC_ARG_ENABLE(sha1-support,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 74c74b93d0..5b1597a636 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -221,6 +221,10 @@ if HAVE_SECCOMP_TESTS
 ctests += dtls-with-seccomp tls-with-seccomp dtls-client-with-seccomp tls-client-with-seccomp
 endif
 
+if STRICT_DER_TIME
+ctests += strict-der
+endif
+
 if !DISABLE_SYSTEM_CONFIG
 ctests += system-prio-file
 endif
diff --git a/tests/cert-tests/Makefile.am b/tests/cert-tests/Makefile.am
index c8abdbf74a..38dd2fb53f 100644
--- a/tests/cert-tests/Makefile.am
+++ b/tests/cert-tests/Makefile.am
@@ -111,11 +111,16 @@ dist_check_SCRIPTS = pathlen aki invalid-sig email \
 	pkcs12 certtool-crl-decoding pkcs12-encode pkcs12-corner-cases inhibit-anypolicy \
 	smime cert-time alt-chain pkcs7-list-sign pkcs7-eddsa certtool-ecdsa \
 	key-id pkcs8 pkcs8-decode ecdsa illegal-rsa pkcs8-invalid key-invalid \
-	pkcs8-eddsa certtool-subca cert-non-digits-time certtool-verify-profiles
+	pkcs8-eddsa certtool-subca certtool-verify-profiles
 
 dist_check_SCRIPTS += key-id ecdsa pkcs8-invalid key-invalid pkcs8-decode pkcs8 pkcs8-eddsa \
 	certtool-utf8 crq
 
+if STRICT_DER_TIME
+dist_check_SCRIPTS += cert-non-digits-time reject-invalid-time
+else
+dist_check_SCRIPTS += tolerate-invalid-time
+endif
 
 if WANT_TEST_SUITE
 dist_check_SCRIPTS += provable-dh-default
diff --git a/tests/cert-tests/reject-invalid-time b/tests/cert-tests/reject-invalid-time
new file mode 100755
index 0000000000..39aa5c4ca5
--- /dev/null
+++ b/tests/cert-tests/reject-invalid-time
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# 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 Lesser General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>
+
+#set -e
+
+srcdir="${srcdir:-.}"
+CERTTOOL="${CERTTOOL:-../../src/certtool${EXEEXT}}"
+PKGCONFIG="${PKG_CONFIG:-$(which pkg-config)}"
+DIFF="${DIFF:-diff -b -B}"
+
+if ! test -x "${CERTTOOL}"; then
+	exit 77
+fi
+
+if ! test -z "${VALGRIND}"; then
+	VALGRIND="${LIBTOOL:-libtool} --mode=execute ${VALGRIND}"
+fi
+
+${PKGCONFIG} --version >/dev/null || exit 77
+
+${PKGCONFIG} --atleast-version=4.12 libtasn1 || exit 77
+
+# Check whether certificates with invalid time fields are accepted
+for file in openssl-invalid-time-format.pem;do
+	${VALGRIND} "${CERTTOOL}" -i --infile "${srcdir}/data/$file"
+	rc=$?
+
+	if test "${rc}" = "0";then
+		echo "file $file was accepted"
+		exit 1
+	fi
+done
+
+exit 0
diff --git a/tests/strict-der.c b/tests/strict-der.c
new file mode 100644
index 0000000000..8854c744d9
--- /dev/null
+++ b/tests/strict-der.c
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2011-2012 Free Software Foundation, Inc.
+ *
+ * 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
+ */
+
+/* Parts copied from GnuTLS example programs. */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#if !defined(_WIN32)
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <sys/wait.h>
+#include <arpa/inet.h>
+#endif
+#include <unistd.h>
+#include <gnutls/gnutls.h>
+#include <gnutls/x509.h>
+
+#include "utils.h"
+
+/* Test for gnutls_certificate_get_issuer() and implicitly for
+ * gnutls_trust_list_get_issuer().
+ */
+
+static void tls_log_func(int level, const char *str)
+{
+	fprintf(stderr, "<%d>| %s", level, str);
+}
+
+/* This certificate is modified to contain invalid DER. In older
+ * gnutls versions that would still be parsed and the wrong DER was
+ * "corrected" but now we should reject these */
+static unsigned char cert_pem[] =
+    "-----BEGIN CERTIFICATE-----\n"
+    "MIIFXzCCBEegAwIBAgIQHYWDpKNVUzEFx4Pq8yjxbTANBgkqhkiG9w0BAQUFADCBtTELMAkGA1UE\n"
+    "BhMCVVMxFzAVBgNVBAoTDlZlcmlTaWduLCBJbmMuMR8wHQYDVQQLExZWZXJpU2lnbiBUcnVzdCBO\n"
+    "ZXR3b3JrMTswOQYDVQQLEzJUZXJtcyBvZiB1c2UgYXQgaHR0cHM6Ly93d3cudmVyaXNpZ24uY29t\n"
+    "L3JwYSAoYykxMDEvMC0GA1UEAxMmVmVyaVNpZ24gQ2xhc3MgMyBTZWN1cmUgU2VydmVyIENBIC0g\n"
+    "RzMwHxcOMTQwMjI3MDAwMDAwWgAXDTE1MDIyODIzNTk1OVowZzELMAkGA1UEBhMCVVMxEzARBgNV\n"
+    "BAgTCldhc2hpbmd0b24xEDAOBgNVBAcUB1NlYXR0bGUxGDAWBgNVBAoUD0FtYXpvbi5jb20gSW5j\n"
+    "LjEXMBUGA1UEAxQOd3d3LmFtYXpvbi5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB\n"
+    "AQCXX4njj63+AK39SJXnf4ove+NO2Z46WgeccZuPUOD89/ucZg9C2K3uwo59QO1t2ZR5IucxVWaV\n"
+    "vSW/9z30hA2ObJco5Cw9o3ZdoFXn0rYUmbWMW+XmL+/bSBDdFPQGfP1WhsFKJJfJ9TIrXBAsTSzH\n"
+    "uC6qFZktvZ1yE0081+bdyOHVHjAQzSPsYFaSUqccMwPvy/sMaI+Um+GCf2PolJJwpI1+j6WmTEVg\n"
+    "RBNHarxtNqpcV3rAFdJ5imL427agMqFur4Iz/OYeoCRBEiKk02ctRzoBaTvF09OQqRg3I4T9bE71\n"
+    "xe1cdWo/sQ4nRiy1tfPBt+aBSiIRMh0Fdle780QFAgMBAAGjggG1MIIBsTBQBgNVHREESTBHghF1\n"
+    "ZWRhdGEuYW1hem9uLmNvbYIKYW1hem9uLmNvbYIIYW16bi5jb22CDHd3dy5hbXpuLmNvbYIOd3d3\n"
+    "LmFtYXpvbi5jb20wCQYDVR0TBAIwADAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYIKwYBBQUH\n"
+    "AwEGCCsGAQUFBwMCMEMGA1UdIAQ8MDowOAYKYIZIAYb4RQEHNjAqMCgGCCsGAQUFBwIBFhxodHRw\n"
+    "czovL3d3dy52ZXJpc2lnbi5jb20vY3BzMB8GA1UdIwQYMBaAFA1EXBZTRMGCfh0gqyX0AWPYvnml\n"
+    "MEUGA1UdHwQ+MDwwOqA4oDaGNGh0dHA6Ly9TVlJTZWN1cmUtRzMtY3JsLnZlcmlzaWduLmNvbS9T\n"
+    "VlJTZWN1cmVHMy5jcmwwdgYIKwYBBQUHAQEEajBoMCQGCCsGAQUFBzABhhhodHRwOi8vb2NzcC52\n"
+    "ZXJpc2lnbi5jb20wQAYIKwYBBQUHMAKGNGh0dHA6Ly9TVlJTZWN1cmUtRzMtYWlhLnZlcmlzaWdu\n"
+    "LmNvbS9TVlJTZWN1cmVHMy5jZXIwDQYJKoZIhvcNAQEFBQADggEBADnmX45CNMkf57rQjB6ef7gf\n"
+    "3r5AfKiGMYdSim4TwU5qcpJicYiyqwQXAQbvZFuZTGzT0jXJROLAsjdHcQiR8D5u7mzVMbJg0kz0\n"
+    "yTsdDM5dFmVWme3l958NZI/I0qCtH+Z/O0cyivOTMARbBJ+92dqQ78U3He9gRNE9VCS3FNgObhwC\n"
+    "cr5tkKTlgSESpSRyBwnLucY4+ci5xjvYndHIzoxII/X9TKOIc2sC+b0H5KP8RcQLAO9G5Nra7+eJ\n"
+    "IC74ZgFvgejqTd2f8QeJljTsNxvG4P7vqQi73fCkTuVfCk5YDtTU2joGAujgBd1EjTIbjWYeoebV\n"
+    "gN5gPKxa/GbGsoQ=\n"
+    "-----END CERTIFICATE-----\n";
+
+const gnutls_datum_t cert = { cert_pem, sizeof(cert_pem) - 1};
+
+void doit(void)
+{
+	int ret;
+	gnutls_x509_crt_t crt;
+
+	/* this must be called once in the program
+	 */
+	global_init();
+
+	gnutls_global_set_log_function(tls_log_func);
+	if (debug)
+		gnutls_global_set_log_level(6);
+
+	gnutls_x509_crt_init(&crt);
+
+	ret =
+	    gnutls_x509_crt_import(crt, &cert, GNUTLS_X509_FMT_PEM);
+	if (ret >= 0) {
+		fail("gnutls_x509_crt_import allowed loading a cert with invalid DER\n");
+		exit(1);
+	}
+	gnutls_x509_crt_deinit(crt);
+
+	gnutls_global_deinit();
+
+	if (debug)
+		success("success");
+}