From 75475d8465660a2b3612e7ecf10fb43c79ca0c71 Mon Sep 17 00:00:00 2001 From: Raoul Strackx Date: Mon, 15 Jun 2020 17:03:13 +0200 Subject: [PATCH] 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 --- ChangeLog.d/crl-revocationDate.txt | 11 +++++++++++ library/x509_crt.c | 3 +-- tests/data_files/Makefile | 5 ++++- tests/data_files/Readme-x509.txt | 2 +- tests/data_files/crl-futureRevocationDate.pem | 11 +++++++++++ tests/data_files/test-ca.server1.future-crl.db | 2 ++ .../test-ca.server1.future-crl.opensslconf | 18 ++++++++++++++++++ tests/scripts/all.sh | 10 ++++++++++ tests/suites/test_suite_x509parse.data | 8 ++++++++ 9 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 ChangeLog.d/crl-revocationDate.txt create mode 100644 tests/data_files/crl-futureRevocationDate.pem create mode 100644 tests/data_files/test-ca.server1.future-crl.db create mode 100644 tests/data_files/test-ca.server1.future-crl.opensslconf diff --git a/ChangeLog.d/crl-revocationDate.txt b/ChangeLog.d/crl-revocationDate.txt new file mode 100644 index 000000000..a8ad53216 --- /dev/null +++ b/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. diff --git a/library/x509_crt.c b/library/x509_crt.c index d519a3fa1..fadd28eff 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1816,8 +1816,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; diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 0429bddd1..a4d124c9c 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -1003,7 +1003,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.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.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 # server2* diff --git a/tests/data_files/Readme-x509.txt b/tests/data_files/Readme-x509.txt index 6f54ed0c1..d07241a2c 100644 --- a/tests/data_files/Readme-x509.txt +++ b/tests/data_files/Readme-x509.txt @@ -111,7 +111,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 diff --git a/tests/data_files/crl-futureRevocationDate.pem b/tests/data_files/crl-futureRevocationDate.pem new file mode 100644 index 000000000..f147a8f80 --- /dev/null +++ b/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----- diff --git a/tests/data_files/test-ca.server1.future-crl.db b/tests/data_files/test-ca.server1.future-crl.db new file mode 100644 index 000000000..763aa1219 --- /dev/null +++ b/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 diff --git a/tests/data_files/test-ca.server1.future-crl.opensslconf b/tests/data_files/test-ca.server1.future-crl.opensslconf new file mode 100644 index 000000000..e9ce7543a --- /dev/null +++ b/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 diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d0a3edc1a..9194aa558 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1245,6 +1245,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 diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 502218626..bc6188ee0 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -843,6 +843,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"