Merge branch 'tmp-strict-x509-time' into 'master'

Do not tolerate invalid DER time

Closes #207

See merge request gnutls/gnutls!1141

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");
+}