Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It may not be necessary to check the "revocation date" #3340

Closed
yuemonangong opened this issue May 21, 2020 · 1 comment · Fixed by #3433
Closed

It may not be necessary to check the "revocation date" #3340

yuemonangong opened this issue May 21, 2020 · 1 comment · Fixed by #3433

Comments

@yuemonangong
Copy link

Description

  • Type: Bug

Bug

OS
ubuntu 16.04.1 [linux|

mbed TLS build:
Version: 2.16.5

I created a CRL whose revocation date is later than current time. MbedTLS does not use this CRL because it thought that the CRL is illegal (see the code in /library/x509_crt.c, line 1788). Comparatively, openssl does not check the "revocation date" field and revokes certificate(s).

The openssl guys explained that "revocation date” is useless in certificate validation and may only be used as meta data (see openssl/openssl#11859). I indeed checked RFC 5280 and did not find any words saying that "revocation date" is important (for certificate parsing and validation). Then do we still need to check the revocation date?

#if defined(MBEDTLS_X509_CRL_PARSE_C)
/*
 * Return 1 if the certificate is revoked, or 0 otherwise.
 */
int mbedtls_x509_crt_is_revoked( const mbedtls_x509_crt *crt, const mbedtls_x509_crl *crl )
{
    const mbedtls_x509_crl_entry *cur = &crl->entry;

    while( cur != NULL && cur->serial.len != 0 )
    {
        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 );
        }

        cur = cur->next;
    }

    return( 0 );
}

The command I used is:

cert_app mode='file' filename=leaf.pem ca_file=root.pem crl_file=test.crl

The verification returns

Result of MbedTLS:
  . Loading the CA root certificate ... ok (0 skipped)

  . Loading the certificate(s) ... ok
  . Peer certificate information    ...
      cert. version     : 3
      serial number     : 01
      issuer name       : C=CN, ST=SH, O=SJTU, OU=DDST, CN=NCRL
      subject name      : C=GB, ST=Berkshire, L=Newbury, O=My Company Ltd
      issued  on        : 1996-08-01 00:00:00
      expires on        : 2020-12-31 23:59:59
      signed using      : RSA with SHA-256
      RSA key size      : 2048 bits
      basic constraints : CA=true

  . Verifying X.509 certificate...
Verify requested for (Depth 1):
cert. version     : 3
serial number     : F5:34:01:4D:DA:77:4E:2F
issuer name       : C=CN, ST=SH, O=SJTU, OU=DDST, CN=NCRL
subject name      : C=CN, ST=SH, O=SJTU, OU=DDST, CN=NCRL
issued  on        : 2020-03-26 08:27:49
expires on        : 2023-01-14 08:27:49
signed using      : RSA with SHA-256
RSA key size      : 2048 bits
basic constraints : CA=true, max_pathlen=3
key usage         : Digital Signature, Non Repudiation, Key Encipherment, Key Cert Sign, CRL Sign
  This certificate has no flags

Verify requested for (Depth 0):
cert. version     : 3
serial number     : 01
issuer name       : C=CN, ST=SH, O=SJTU, OU=DDST, CN=NCRL
subject name      : C=GB, ST=Berkshire, L=Newbury, O=My Company Ltd
issued  on        : 1996-08-01 00:00:00
expires on        : 2020-12-31 23:59:59
signed using      : RSA with SHA-256
RSA key size      : 2048 bits
basic constraints : CA=true
  This certificate has no flags
 ok

Result of OpenSSL:

C = GB, ST = Berkshire, L = Newbury, O = My Company Ltd
error 23 at 0 depth lookup: certificate revoked
error leaf.pem: verification failed

root.pem

-----BEGIN CERTIFICATE-----
MIIDNDCCAhygAwIBAgIJAPU0AU3ad04vMA0GCSqGSIb3DQEBCwUAMEcxCzAJBgNV
BAYTAkNOMQswCQYDVQQIDAJTSDENMAsGA1UECgwEU0pUVTENMAsGA1UECwwERERT
VDENMAsGA1UEAwwETkNSTDAeFw0yMDAzMjYwODI3NDlaFw0yMzAxMTQwODI3NDla
MEcxCzAJBgNVBAYTAkNOMQswCQYDVQQIDAJTSDENMAsGA1UECgwEU0pUVTENMAsG
A1UECwwERERTVDENMAsGA1UEAwwETkNSTDCCASIwDQYJKoZIhvcNAQEBBQADggEP
ADCCAQoCggEBAPNLlu+KPCcjj1KiZ1/sUFvFRDt3Z7WZTWjOYeJUFvycHNcYN9cE
laGJ32hfjgaPw9u3cOgs0JJHwIhlQkNhSexUvgGatw336H/3FjPQCJKs48lDJG13
sDF7TK1MvG5wcF1pgRkfvoRSWOyr30aqoeQHaRnqMnT0lWBy26mmV6mgM7LPeNq8
E6jh6aLy7uep3+rO/Ef7LvFi/QWqy+vVVmr5MljXtFyWI+aLs4uFtZZ6rFw5Va4U
Y6OffwchjkVex1eML4D593fobkmkubxZEm2o2Upi/Eiech7CM8HuwgqrAwoIVxi6
FlObraD90sUSUKUwohvl03tkjCTXakXF2TUCAwEAAaMjMCEwEgYDVR0TAQH/BAgw
BgEB/wIBAzALBgNVHQ8EBAMCAeYwDQYJKoZIhvcNAQELBQADggEBACb6hOtUCqD5
sH4VucCO4FYFHM6nfBvB9vx+c2RPC/psam9clOvL5llrUhY070pXbZnd2hwxfnzj
cdr448sVyJkHosukzZj/MyEBV9BERTUMOaY4etQxM2L33uyzn5++/NeRC2Yd53AL
vY/s4znat7txqBK/izvLemLerp1Z5E58VFzLOvYNz+7vEoxMmNaU55TGh88VJIvo
THaZ3LflTc7hv9eUWin0LTV0mg7cvM+/qWrM2N2hyOukztF5gCcMEgoVkpEgUCJP
WsrvOumtDNuXnPr80r4N54n5TaQCTBG22Tj89klc6jUji63+UR9KKACCV44KT8hc
+0ecJPmXqEU=
-----END CERTIFICATE-----

leaf.pem

-----BEGIN CERTIFICATE-----
MIIDITCCAgmgAwIBAgIBATANBgkqhkiG9w0BAQsFADBHMQswCQYDVQQGEwJDTjEL
MAkGA1UECAwCU0gxDTALBgNVBAoMBFNKVFUxDTALBgNVBAsMBEREU1QxDTALBgNV
BAMMBE5DUkwwHhcNOTYwODAxMDAwMDAwWhcNMjAxMjMxMjM1OTU5WjBMMQswCQYD
VQQGEwJHQjESMBAGA1UECBMJQmVya3NoaXJlMRAwDgYDVQQHEwdOZXdidXJ5MRcw
FQYDVQQKEw5NeSBDb21wYW55IEx0ZDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC
AQoCggEBAL+N0yePi18I/+MxN/31iBehb2rO5s8MzykUz3aGp3BG/5uEFueqoYZN
CNLA38wIUT/ry8wIw+jlTNj29L7Q9uOX8+10XgF4VTVtN14KT0s7tZ5dLjGRD7ft
fZF03ifbGYp39fW2Wjutjo4Jyop+Bm7g6SrSJB3uaioITpZh8Xf7MHo+kNjJKPsu
ZlVVNQ3T5WQWzoskcpRRIujv7U/NATuRzXODUzqnw+HGavu2qTX3falo5i0dzzrt
9yCtLKqtC+0oX+kZPIi3ib/o20fY3hEXwYstq5sKpvV25xgKTbtwRN1KlMIhfSQN
uFXIg/Rd6rbd9P60zPYxzOTwMsaEysECAwEAAaMTMBEwDwYDVR0TAQH/BAUwAwEB
/zANBgkqhkiG9w0BAQsFAAOCAQEAKhf5CQGxsJCzkFJv26ggzi2HxN/X/eXcwJyy
3gfPP0JZNLzRb6bmracLui58LyCX+0tmY5TA1G3V94Vdu2LIUMRoANwKszTxhW/n
8oNvXDji+E62EsivCtoPgYRAwFE0q4flvcWzDwGlqCfEdaG1uqYGLlLxW8gmHdFs
pKJf4yCzQOn04RmReXOhaAtyUT+xp9AUzawzr2PPGA75x7B07HT4ezLPWy+l1X0o
gMBOWm3AwrwTD8k1B488NiKivCYjBn6UPG0r9/gKxSvdEJEJ6SyM8+Jw+f7lij8i
55LYqy8oyPPknQOAWzB+KZkCbqkcBGJLEPR35agBN/SDSdioXA==
-----END CERTIFICATE-----

test.crl

-----BEGIN X509 CRL-----
MIIBtjCBnwIBATANBgkqhkiG9w0BAQ4FADBHMQswCQYDVQQGEwJDTjELMAkGA1UE
CAwCU0gxDTALBgNVBAoMBFNKVFUxDTALBgNVBAsMBEREU1QxDTALBgNVBAMMBE5D
UkwXDTIwMDMwODIwMjYwN1oXDTIxMDIyMTIwMjYwN1owFDASAgEBFw0yMTAyMjEy
MDI2MDdaoA4wDDAKBgNVHRQEAwIBADANBgkqhkiG9w0BAQ4FAAOCAQEAkqQPjXyG
FTyPmHi47KoMaeg0M7wU5KO8SXOHxUEU65+QuXOK5pOSpjAH+JhLk5H19dY+22gW
+Bec3N3PO2Iw7iK58t5jl+UywRl3VNf6g2Pf+7zrZje3XoTHATLp+LHfVkUJLflZ
/FiEtJS6NVsNemrAY+BNCCIocHqBVjMAX1zSmUw9lSUFIH6+9HOtVQ0FlmO5RBA5
6QoCmAO3t/5FZF3LqxGj5z3weBVsU340iebf20remV2pEFwWW9KKDBkx+Rd9njgW
dmrKXY4pLfAUZ+D2eCevHBU7vLSSjmgBOKu+74/l28wGF+7oFtj5txbK4hdc1j11
/gX0RqNhGLarQw==
-----END X509 CRL-----
@gilles-peskine-arm
Copy link
Contributor

Thanks for reporting this! As it happens there's a fix pending, which we hope to merge soon: #3433 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants