Always revoke certificate on CRL

RFC5280 does not state that the `revocationDate` should be checked.

In addition, when no time source is available (i.e., when MBEDTLS_HAVE_TIME_DATE is not defined), `mbedtls_x509_time_is_past` always returns 0. This results in the CRL not being checked at all.

https://tools.ietf.org/html/rfc5280
Signed-off-by: Raoul Strackx <raoul.strackx@fortanix.com>

ChangeLog.d/crl-revocationDate.txt

@@ -0,0 +1,11 @@
+Security
+   * When checking X.509 CRLs, a certificate was only considered as revoked if
+     its revocationDate was in the past according to the local clock if
+     available. In particular, on builds without MBEDTLS_HAVE_TIME_DATE,
+     certificates were never considered as revoked. On builds with
+     MBEDTLS_HAVE_TIME_DATE, an attacker able to control the local clock (for
+     example, an untrusted OS attacking a secure enclave) could prevent
+     revocation of certificates via CRLs. Fixed by no longer checking the
+     revocationDate field, in accordance with RFC 5280. Reported by
+     yuemonangong in #3340. Reported independently and fixed by
+     Raoul Strackx and Jethro Beekman in #3433.

library/x509_crt.c

@@ -1786,8 +1786,7 @@ int mbedtls_x509_crt_is_revoked( const mbedtls_x509_crt *crt, const mbedtls_x509
         if( crt->serial.len == cur->serial.len &&
             memcmp( crt->serial.p, cur->serial.p, crt->serial.len ) == 0 )
         {
-            if( mbedtls_x509_time_is_past( &cur->revocation_date ) )
+            return( 1 );
-                return( 1 );
         }
 
         cur = cur->next;

tests/data_files/Makefile

@@ -887,7 +887,10 @@ server1.v1.der.openssl: server1.v1.crt.openssl
 crl.pem: $(test_ca_crt) $(test_ca_key_file_rsa) $(test_ca_config_file)
 	$(OPENSSL) ca -gencrl -batch -cert $(test_ca_crt) -keyfile $(test_ca_key_file_rsa) -key $(test_ca_pwd_rsa) -config $(test_ca_server1_config_file) -md sha1 -crldays 3653 -out $@
 
-server1_all: crl.pem server1.csr server1.crt server1.noauthid.crt server1.crt.openssl server1.v1.crt server1.v1.crt.openssl server1.key_usage.crt server1.key_usage_noauthid.crt server1.key_usage.crt.openssl server1.cert_type.crt server1.cert_type_noauthid.crt server1.cert_type.crt.openssl server1.der server1.der.openssl server1.v1.der server1.v1.der.openssl server1.key_usage.der server1.key_usage.der.openssl server1.cert_type.der server1.cert_type.der.openssl
+crl-futureRevocationDate.pem: $(test_ca_crt) $(test_ca_key_file_rsa) $(test_ca_config_file) test-ca.server1.future-crl.db  test-ca.server1.future-crl.opensslconf
+	$(FAKETIME) '2028-12-31' $(OPENSSL) ca -gencrl -config test-ca.server1.future-crl.opensslconf -crldays 365 -passin "pass:$(test_ca_pwd_rsa)" -out $@
+
+server1_all: crl.pem crl-futureRevocationDate.pem server1.csr server1.crt server1.noauthid.crt server1.crt.openssl server1.v1.crt server1.v1.crt.openssl server1.key_usage.crt server1.key_usage_noauthid.crt server1.key_usage.crt.openssl server1.cert_type.crt server1.cert_type_noauthid.crt server1.cert_type.crt.openssl server1.der server1.der.openssl server1.v1.der server1.v1.der.openssl server1.key_usage.der server1.key_usage.der.openssl server1.cert_type.der server1.cert_type.der.openssl
 
 # To revoke certificate in the openssl database:
 #

tests/data_files/Readme-x509.txt

@@ -89,7 +89,7 @@ Signing CA in parentheses (same meaning as certificates).
 - crl-ec-sha*.pem: (2) server6.crt
 - crl-future.pem: (2) server6.crt + unknown
 - crl-rsa-pss-*.pem: (1) server9{,badsign,with-ca}.crt + cert_sha384.crt + unknown
-- crl.pem, crl_expired.pem: (1) server1{,.cert_type,.key_usage,.v1}.crt + unknown
+- crl.pem, crl-futureRevocationDate.pem, crl_expired.pem: (1) server1{,.cert_type,.key_usage,.v1}.crt + unknown
 - crl_md*.pem: crl_sha*.pem: (1) same as crl.pem
 - crt_cat_*.pem: (1+2) concatenations in various orders:
     ec = crl-ec-sha256.pem, ecfut = crl-future.pem

tests/data_files/crl-futureRevocationDate.pem

@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBqzCBlDANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDERMA8GA1UECgwI
+UG9sYXJTU0wxGTAXBgNVBAMMEFBvbGFyU1NMIFRlc3QgQ0EXDTI4MTIzMDIzMDAw
+MFoXDTI5MTIzMDIzMDAwMFowKDASAgEBFw0yOTAxMDExMjQ0MDdaMBICAQMXDTI5
+MDEwMTEyNDQwN1owDQYJKoZIhvcNAQEFBQADggEBAKbL1mDpzCbLJmRZKM2KHPvK
+ijS4UMnanzzYpLAwom1NI69v2fE1/EfiXv0empE6mFqnLwOG4ZP8fECfxjMXO2Ee
+VhxYiRjly6q9hfIUk1e+N9ct8unNnLEBvf6Syfy9+FSO3Q/ahljpYlXsXxg62WXl
+9xp5b5Ok+/0sCv0eL5uFQKXQa8hS9dZo6py7jvFDQC+wVau1mXjQW85iXMLm7vik
+4lR+kfZloeq1jIbsx8cdMi32YVt7uccaqoFcjtrdrWfGmi0wvlDc8K5J0l4tIxZY
+9P+T4fzSgQLdqGZ3xADheEaGTRVL/5oe5L4zRH32BZONMFCijv+j1SpWLxHE8cM=
+-----END X509 CRL-----

tests/data_files/test-ca.server1.future-crl.db

@@ -0,0 +1,2 @@
+R	210212144406Z	290101124407Z	01	unknown	/C=NL/O=PolarSSL/CN=PolarSSL Server 1
+R	210212144400Z	290101124407Z	03	unknown	/C=NL/O=PolarSSL/CN=PolarSSL Test CA

tests/data_files/test-ca.server1.future-crl.opensslconf

@@ -0,0 +1,18 @@
+ [ ca ]
+ default_ca             = test-ca
+
+ [ test-ca ]
+ certificate            = test-ca.crt
+ private_key            = test-ca.key
+ serial                 = test-ca.server1.serial
+ default_md             = sha1
+ default_startdate      = 110212144406Z
+ default_enddate        = 210212144406Z
+ new_certs_dir          = ./
+ database               = ./test-ca.server1.future-crl.db
+ policy                 = policy_match
+
+ [policy_match]
+ countryName            = supplied
+ organizationName       = supplied
+ commonName             = supplied

tests/scripts/all.sh

@@ -1124,6 +1124,16 @@ component_test_null_entropy () {
     make test
 }
 
+component_test_no_date_time () {
+    msg "build: default config without MBEDTLS_HAVE_TIME_DATE"
+    scripts/config.pl unset MBEDTLS_HAVE_TIME_DATE
+    CC=gcc cmake
+    make
+
+    msg "test: !MBEDTLS_HAVE_TIME_DATE - main suites"
+    make test
+}
+
 component_test_platform_calloc_macro () {
     msg "build: MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO enabled (ASan build)"
     scripts/config.pl set MBEDTLS_PLATFORM_MEMORY

tests/suites/test_suite_x509parse.data

@@ -831,6 +831,14 @@ X509 Certificate verification #97 (next profile Valid Cert SHA256 Digest)
 depends_on:MBEDTLS_SHA256_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_ECDSA_C:MBEDTLS_SHA1_C
 x509_verify:"data_files/cert_sha256.crt":"data_files/test-ca.crt":"data_files/crl-ec-sha256.pem":"NULL":0:0:"next":"NULL"
 
+X509 CRT verification #98 (Revoked Cert, revocation date in the future, _with_ MBEDTLS_HAVE_TIME_DATE)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_HAVE_TIME_DATE
+x509_verify:"data_files/server1.crt":"data_files/test-ca.crt":"data_files/crl-futureRevocationDate.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_REVOKED|MBEDTLS_X509_BADCRL_FUTURE:"compat":"NULL"
+
+X509 CRT verification #99 (Revoked Cert, revocation date in the future, _without_ MBEDTLS_HAVE_TIME_DATE)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:!MBEDTLS_HAVE_TIME_DATE
+x509_verify:"data_files/server1.crt":"data_files/test-ca.crt":"data_files/crl-futureRevocationDate.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_REVOKED:"compat":"NULL"
+
 X509 Certificate verification: domain identical to IPv4 in SubjectAltName
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C
 x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL"