Skip to content

Commit

Permalink
Always revoke certificate on CRL
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
raoulstrackx committed Jun 24, 2020
1 parent b21b1f5 commit 3529f78
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 4 deletions.
6 changes: 6 additions & 0 deletions ChangeLog.d/fix-crl-revocationDate.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix
* A CRL's "revocationDate" entry field is no longer checked to be in the
past. This brings the implementation in line with RFC 5280. Note that
this also is a security fix in environments where the local clock cannot
be trusted (e.g., in an Intel SGX enclave). Reported by Raoul Strackx &
Jethro Beekman.
2 changes: 2 additions & 0 deletions include/mbedtls/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
* MBEDTLS_PLATFORM_STD_TIME.
*
* Comment if your system does not support time functions
* \warning You should comment this out if the system's clock could be controlled by an attacker.
*/
#define MBEDTLS_HAVE_TIME

Expand All @@ -145,6 +146,7 @@
*
* Comment if your system does not have a correct clock.
*
* \warning You should comment this out if the system's clock could be controlled by an attacker.
* \note mbedtls_platform_gmtime_r() is an abstraction in platform_util.h that
* behaves similarly to the gmtime_r() function from the C standard. Refer to
* the documentation for mbedtls_platform_gmtime_r() for more information.
Expand Down
3 changes: 1 addition & 2 deletions library/x509_crt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2320,8 +2320,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;
Expand Down
5 changes: 4 additions & 1 deletion tests/data_files/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,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)
$(FAKETIME) '3000-01-01' $(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 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*

Expand Down
2 changes: 1 addition & 1 deletion tests/data_files/Readme-x509.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions tests/data_files/crl-futureRevocationDate.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-----BEGIN X509 CRL-----
MIIBrzCBmDANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDERMA8GA1UECgwI
UG9sYXJTU0wxGTAXBgNVBAMMEFBvbGFyU1NMIFRlc3QgQ0EYDzI5OTkxMjMxMjMw
MDAwWhgPMzAxMDAxMDEyMzAwMDBaMCgwEgIBARcNMTEwMjEyMTI0NDA3WjASAgED
Fw0xMTAyMTIxMjQ0MDdaMA0GCSqGSIb3DQEBBQUAA4IBAQCOs/sekrnS0yDIP5BE
MKYMgywqm4IT8hy/pmvqSxFNOC1MF8lLiQ4SPtQvEpCW9mwZ5w+sT/u01Yb2KmQB
OpZGV21o6tvil8byMb3hsc4O11xbMVSHBG0045MPRaW8Ik9SMLS5IE74hXaK5l+y
L9kf4RHZon/tl9OdiseRVb+iEMpGPsHLHZsF3NuPpXAVizu6jHaf4SMIAQ2IjLMy
mmHS9VaMSH+p20fDgDX42uNi+wYGiW9ezCD/oD38dY+XOoIhknWyOTNvzM3bpv0+
v1WF2ZVksiNcteSICPXcEAnEfmjmcwYoqVOYxd6XH8eQTmnxf7FoUv1jTD7FYEPR
NBLr
-----END X509 CRL-----
8 changes: 8 additions & 0 deletions tests/suites/test_suite_x509parse.data
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,14 @@ X509 CRT 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 CRT verification with ca callback: failure
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK
x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_FATAL_ERROR
Expand Down

0 comments on commit 3529f78

Please sign in to comment.