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

[WIP] Add x509_certificate_revocation_info module #30

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Apr 11, 2020

SUMMARY

A new module which allows to check whether a certificate is revoked, via different sources. (As suggested by @ctrufan in ansible/ansible#67539 (comment)) Right now, only CRLs are supported. A certificate (given by a PEM file, PEM content, or by its serial number) can be checked against a CRL file, against a CRL URL, and/or against the CRL URLs contained in the certificate (only if PEM is given, of course).

I guess support for OCSP checking will be up next, though since I'm not sure when I have time to work this, I'd be happy if this can get merged before that is added as well.

Contains #27 and #29. Will be rebased and un-WIP'ed once that is merged.

Closes ansible/ansible#53789.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

x509_certificate_revocation_info

@felixfontein felixfontein force-pushed the x509_certificate_revocation_info branch 2 times, most recently from 2418d18 to 7a61b3f Compare April 13, 2020 12:01
@felixfontein felixfontein changed the title Add x509_certificate_revocation_info module [WIP] Add x509_certificate_revocation_info module Apr 13, 2020
@felixfontein felixfontein force-pushed the x509_certificate_revocation_info branch from 7a61b3f to 5e691f0 Compare April 23, 2020 20:21
@felixfontein felixfontein force-pushed the x509_certificate_revocation_info branch 2 times, most recently from 7e3f058 to b95fbb3 Compare May 15, 2020 07:58
@felixfontein felixfontein changed the title [WIP] Add x509_certificate_revocation_info module Add x509_certificate_revocation_info module May 15, 2020
@felixfontein
Copy link
Contributor Author

ready_for_review

@ctrufan
Copy link
Collaborator

ctrufan commented May 17, 2020

Sorry I've been too busy to comment on much. I will note that there were some OCSP checks in my very first "add entrust support" pull request that I removed because they over-complicated the module. Wasn't tested with a variety of OCSP responders though

ansible/ansible@689ddd1

result['crl_contained'] = False
result['crl_record'] = None
for cert in crl:
if cert.serial_number == self.cert_serial_number:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition for serial number match, to be considered revoked a certificate also must have either:

  • The same issuer as the CRL
  • The "certificateIssuer" field on the CRL entry set to the same issuer as the certificate (which is one or more entries in the issuer alternate names).

Also,there's sort of a "fall through" for CRL issuer values, where if it's not set, it defaults to whatever the value was for the previous entry in the CRL. So you could have CRL Entries (don't quote me on this):

  1. <CertIssuer=NotSet> Defaults to Z, which was the CRL Issuer.
  2. <CertIssuer=A> Users Cert Issuer A
  3. <CertIssuer=NotSet> Uses Cert Issuer A
  4. <CertIssuer=NotSet> Uses Cert Issuer A
  5. <CertIssuer=B> Uses Cert Issuer B
  6. <CertIssuer=NotSet> Uses Cert Issuer B

Relevant section from RFC5280 https://tools.ietf.org/html/rfc5280

5.3.3. Certificate Issuer
This CRL entry extension identifies the certificate issuer associated
with an entry in an indirect CRL, that is, a CRL that has the
indirectCRL indicator set in its issuing distribution point
extension. When present, the certificate issuer CRL entry extension
includes one or more names from the issuer field and/or issuer
alternative name extension of the certificate that corresponds to the
CRL entry. If this extension is not present on the first entry in an
indirect CRL, the certificate issuer defaults to the CRL issuer. On
subsequent entries in an indirect CRL, if this extension is not
present, the certificate issuer for the entry is the same as that for
the preceding entry. This field is defined as follows:

id-ce-certificateIssuer OBJECT IDENTIFIER ::= { id-ce 29 }

CertificateIssuer ::= GeneralNames

Conforming CRL issuers MUST include in this extension the
distinguished name (DN) from the issuer field of the certificate that
corresponds to this CRL entry. The encoding of the DN MUST be
identical to the encoding used in the certificate.

CRL issuers MUST mark this extension as critical since an
implementation that ignored this extension could not correctly
attribute CRL entries to certificates. This specification RECOMMENDS
that implementations recognize this extension.

if distribution_point.full_name is not None:
had_crl_url = False
for name in distribution_point.full_name:
if isinstance(name, x509.UniformResourceIdentifier):
Copy link
Collaborator

@ctrufan ctrufan May 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible for a full name to be an LDAP entry (e.g. would show up in windows cert browser as "URL=ldap://blah", not sure about how it gets decoded by python). Not sure what that would do to the getURL. bit in the subsequent check_crl call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would error out with "ldap is an unsupported URL schema", or something like that. Which I think is acceptable.

@felixfontein felixfontein mentioned this pull request Jun 27, 2020
@felixfontein felixfontein changed the base branch from master to main July 2, 2020 12:13
@pabelanger
Copy link

recheck

@felixfontein felixfontein force-pushed the x509_certificate_revocation_info branch from bbf7298 to 93944b5 Compare July 3, 2020 20:50
@felixfontein felixfontein changed the title Add x509_certificate_revocation_info module [WIP] Add x509_certificate_revocation_info module Dec 4, 2020
@felixfontein felixfontein marked this pull request as draft November 27, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support openssl_crl much like openssl_certificate
4 participants