From 77f5a2560a2c637364467a5f74b60a0e70e177f9 Mon Sep 17 00:00:00 2001 From: Erik Trauschke Date: Wed, 14 Oct 2015 08:06:38 -0700 Subject: use X509ExtensionParser for Revoked extensions remove revoked_certificates property from RevokedCertificate class CRLExtensions should actually be RevokedExtensions doctest cleanup for RevokedCertificate --- docs/x509/reference.rst | 21 +-- src/cryptography/hazmat/backends/openssl/x509.py | 179 ++++++++++------------- src/cryptography/x509/__init__.py | 8 +- src/cryptography/x509/base.py | 6 - src/cryptography/x509/oid.py | 8 +- tests/test_x509.py | 29 ++-- 6 files changed, 106 insertions(+), 145 deletions(-) diff --git a/docs/x509/reference.rst b/docs/x509/reference.rst index 5f4c5314..67591d38 100644 --- a/docs/x509/reference.rst +++ b/docs/x509/reference.rst @@ -478,18 +478,6 @@ X.509 CRL (Certificate Revocation List) Object >>> crl.last_update datetime.datetime(2015, 1, 1, 0, 0) - .. attribute:: revoked_certificates - - :type: list of :class:`RevokedCertificate` - - The revoked certificates listed in this CRL. - - .. doctest:: - - >>> for r in crl.revoked_certificates: - ... print(r.serial_number) - 0 - .. attribute:: extensions :type: :class:`Extensions` @@ -712,6 +700,10 @@ X.509 Revoked Certificate Object .. versionadded:: 1.0 + .. doctest:: + + >>> revoked_certificate = crl[0] + .. attribute:: serial_number :type: :class:`int` @@ -720,7 +712,6 @@ X.509 Revoked Certificate Object .. doctest:: - >>> revoked_certificate = crl.revoked_certificates[0] >>> revoked_certificate.serial_number 0 @@ -732,7 +723,7 @@ X.509 Revoked Certificate Object .. doctest:: - >>> crl.revoked_certificates[0].revocation_date + >>> revoked_certificate.revocation_date datetime.datetime(2015, 1, 1, 0, 0) .. attribute:: extensions @@ -743,7 +734,7 @@ X.509 Revoked Certificate Object .. doctest:: - >>> for ext in crl.revoked_certificates[0].extensions: + >>> for ext in revoked_certificate.extensions: ... print(ext) , critical=False, value=2015-01-01 00:00:00)> , critical=False, value=ReasonFlags.key_compromise)> diff --git a/src/cryptography/hazmat/backends/openssl/x509.py b/src/cryptography/hazmat/backends/openssl/x509.py index 32678939..7f7be545 100644 --- a/src/cryptography/hazmat/backends/openssl/x509.py +++ b/src/cryptography/hazmat/backends/openssl/x509.py @@ -18,7 +18,9 @@ from six.moves import urllib_parse from cryptography import utils, x509 from cryptography.exceptions import UnsupportedAlgorithm from cryptography.hazmat.primitives import hashes, serialization -from cryptography.x509.oid import CertificatePoliciesOID, ExtensionOID +from cryptography.x509.oid import ( + CertificatePoliciesOID, ExtensionOID, RevokedExtensionOID +) def _obj2txt(backend, obj): @@ -173,10 +175,11 @@ def _decode_ocsp_no_check(backend, ext): class _X509ExtensionParser(object): - def __init__(self, ext_count, get_ext, handlers): + def __init__(self, ext_count, get_ext, handlers, supported_versions=None): self.ext_count = ext_count self.get_ext = get_ext self.handlers = handlers + self.supported_versions = supported_versions def parse(self, backend, x509_obj): extensions = [] @@ -187,6 +190,13 @@ class _X509ExtensionParser(object): crit = backend._lib.X509_EXTENSION_get_critical(ext) critical = crit == 1 oid = x509.ObjectIdentifier(_obj2txt(backend, ext.object)) + + # Filter out extensions we know are not supported by the backend + if (self.supported_versions and oid in self.supported_versions and + self.supported_versions[oid] > + backend._lib.OPENSSL_VERSION_NUMBER): + self.handlers.pop(oid, None) + if oid in seen_oids: raise x509.DuplicateExtension( "Duplicate {0} extension found".format(oid), oid @@ -196,7 +206,8 @@ class _X509ExtensionParser(object): except KeyError: if critical: raise x509.UnsupportedExtension( - "{0} is not currently supported".format(oid), oid + "Critical extension {0} is not currently supported" + .format(oid), oid ) else: d2i = backend._lib.X509V3_EXT_d2i(ext) @@ -639,6 +650,49 @@ def _decode_inhibit_any_policy(backend, asn1_int): return x509.InhibitAnyPolicy(skip_certs) +def _decode_crl_reason(backend, enum): + enum = backend._ffi.cast("ASN1_ENUMERATED *", enum) + enum = backend._ffi.gc(enum, backend._lib.ASN1_ENUMERATED_free) + code = backend._lib.ASN1_ENUMERATED_get(enum) + + try: + return { + 0: x509.ReasonFlags.unspecified, + 1: x509.ReasonFlags.key_compromise, + 2: x509.ReasonFlags.ca_compromise, + 3: x509.ReasonFlags.affiliation_changed, + 4: x509.ReasonFlags.superseded, + 5: x509.ReasonFlags.cessation_of_operation, + 6: x509.ReasonFlags.certificate_hold, + 8: x509.ReasonFlags.remove_from_crl, + 9: x509.ReasonFlags.privilege_withdrawn, + 10: x509.ReasonFlags.aa_compromise, + }[code] + except KeyError: + raise ValueError("Unsupported reason code: {0}".format(code)) + + +def _decode_invalidity_date(backend, inv_date): + generalized_time = backend._ffi.cast( + "ASN1_GENERALIZEDTIME *", inv_date + ) + generalized_time = backend._ffi.gc( + generalized_time, backend._lib.ASN1_GENERALIZEDTIME_free + ) + time = backend._ffi.string( + backend._lib.ASN1_STRING_data( + backend._ffi.cast("ASN1_STRING *", generalized_time) + ) + ).decode("ascii") + return datetime.datetime.strptime(time, "%Y%m%d%H%M%SZ") + + +def _decode_cert_issuer(backend, issuer): + gns = backend._ffi.cast("GENERAL_NAMES *", issuer) + gns = backend._ffi.gc(gns, backend._lib.GENERAL_NAMES_free) + return x509.GeneralNames(_decode_general_names(backend, gns)) + + @utils.register_interface(x509.RevokedCertificate) class _RevokedCertificate(object): def __init__(self, backend, x509_revoked): @@ -670,46 +724,9 @@ class _RevokedCertificate(object): @property def extensions(self): - if self._extensions: - return self._extensions - - extensions = [] - seen_oids = set() - extcount = self._backend._lib.X509_REVOKED_get_ext_count( - self._x509_revoked) - for i in range(extcount): - ext = self._backend._lib.X509_REVOKED_get_ext( - self._x509_revoked, i) - self._backend.openssl_assert(ext != self._backend._ffi.NULL) - crit = self._backend._lib.X509_EXTENSION_get_critical(ext) - critical = crit == 1 - oid = x509.ObjectIdentifier(_obj2txt(self._backend, ext.object)) - if oid in seen_oids: - raise x509.DuplicateExtension( - "Duplicate {0} extension found".format(oid), oid - ) - - if oid == x509.OID_CRL_REASON: - value = self._decode_crl_reason(ext) - elif oid == x509.OID_INVALIDITY_DATE: - value = self._decode_invalidity_date(ext) - elif (oid == x509.OID_CERTIFICATE_ISSUER and - self._backend._lib.OPENSSL_VERSION_NUMBER >= 0x10000000): - value = self._decode_cert_issuer(ext) - elif critical: - raise x509.UnsupportedExtension( - "{0} is not currently supported".format(oid), oid - ) - else: - # Unsupported non-critical extension, silently skipping for now - seen_oids.add(oid) - continue - - seen_oids.add(oid) - extensions.append(x509.Extension(oid, critical, value)) - - self._extensions = x509.Extensions(extensions) - return self._extensions + return _REVOKED_CERTIFICATE_EXTENSION_PARSER.parse( + self._backend, self._x509_revoked + ) def get_reason(self): """ @@ -741,57 +758,6 @@ class _RevokedCertificate(object): except x509.ExtensionNotFound: return None - def _decode_crl_reason(self, ext): - enum = self._backend._lib.X509V3_EXT_d2i(ext) - self._backend.openssl_assert(enum != self._backend._ffi.NULL) - enum = self._backend._ffi.cast("ASN1_ENUMERATED *", enum) - enum = self._backend._ffi.gc( - enum, self._backend._lib.ASN1_ENUMERATED_free - ) - code = self._backend._lib.ASN1_ENUMERATED_get(enum) - - try: - return { - 0: x509.ReasonFlags.unspecified, - 1: x509.ReasonFlags.key_compromise, - 2: x509.ReasonFlags.ca_compromise, - 3: x509.ReasonFlags.affiliation_changed, - 4: x509.ReasonFlags.superseded, - 5: x509.ReasonFlags.cessation_of_operation, - 6: x509.ReasonFlags.certificate_hold, - 8: x509.ReasonFlags.remove_from_crl, - 9: x509.ReasonFlags.privilege_withdrawn, - 10: x509.ReasonFlags.aa_compromise, - }[code] - except KeyError: - raise ValueError("Unsupported reason code: {0}".format(code)) - - def _decode_invalidity_date(self, ext): - generalized_time = self._backend._ffi.cast( - "ASN1_GENERALIZEDTIME *", - self._backend._lib.X509V3_EXT_d2i(ext) - ) - self._backend.openssl_assert( - generalized_time != self._backend._ffi.NULL - ) - generalized_time = self._backend._ffi.gc( - generalized_time, self._backend._lib.ASN1_GENERALIZEDTIME_free - ) - time = self._backend._ffi.string( - self._backend._lib.ASN1_STRING_data( - self._backend._ffi.cast("ASN1_STRING *", generalized_time) - ) - ).decode("ascii") - return datetime.datetime.strptime(time, "%Y%m%d%H%M%SZ") - - def _decode_cert_issuer(self, ext): - gns = self._backend._ffi.cast( - "GENERAL_NAMES *", self._backend._lib.X509V3_EXT_d2i(ext) - ) - self._backend.openssl_assert(gns != self._backend._ffi.NULL) - gns = self._backend._ffi.gc(gns, self._backend._lib.GENERAL_NAMES_free) - return x509.GeneralNames(_decode_general_names(self._backend, gns)) - @utils.register_interface(x509.CertificateRevocationList) class _CertificateRevocationList(object): @@ -865,8 +831,7 @@ class _CertificateRevocationList(object): self._last_update = self._backend._parse_asn1_time(lu) return self._last_update - @property - def revoked_certificates(self): + def _revoked_certificates(self): if self._revoked: return self._revoked @@ -884,13 +849,13 @@ class _CertificateRevocationList(object): return self._revoked def __iter__(self): - return iter(self.revoked_certificates) + return iter(self._revoked_certificates()) def __getitem__(self, idx): - return self.revoked_certificates[idx] + return self._revoked_certificates()[idx] def __len__(self): - return len(self.revoked_certificates) + return len(self._revoked_certificates()) @property def extensions(self): @@ -977,6 +942,15 @@ _EXTENSION_HANDLERS = { ExtensionOID.NAME_CONSTRAINTS: _decode_name_constraints, } +_REVOKED_EXTENSION_HANDLERS = { + RevokedExtensionOID.CRL_REASON: _decode_crl_reason, + RevokedExtensionOID.INVALIDITY_DATE: _decode_invalidity_date, + RevokedExtensionOID.CERTIFICATE_ISSUER: _decode_cert_issuer, +} + +_REVOKED_SUPPORTED_VERSIONS = { + RevokedExtensionOID.CERTIFICATE_ISSUER: 0x10000000, +} _CERTIFICATE_EXTENSION_PARSER = _X509ExtensionParser( ext_count=lambda backend, x: backend._lib.X509_get_ext_count(x), @@ -989,3 +963,10 @@ _CSR_EXTENSION_PARSER = _X509ExtensionParser( get_ext=lambda backend, x, i: backend._lib.sk_X509_EXTENSION_value(x, i), handlers=_EXTENSION_HANDLERS ) + +_REVOKED_CERTIFICATE_EXTENSION_PARSER = _X509ExtensionParser( + ext_count=lambda backend, x: backend._lib.X509_REVOKED_get_ext_count(x), + get_ext=lambda backend, x, i: backend._lib.X509_REVOKED_get_ext(x, i), + handlers=_REVOKED_EXTENSION_HANDLERS, + supported_versions=_REVOKED_SUPPORTED_VERSIONS +) diff --git a/src/cryptography/x509/__init__.py b/src/cryptography/x509/__init__.py index 70e1d3da..697df6f2 100644 --- a/src/cryptography/x509/__init__.py +++ b/src/cryptography/x509/__init__.py @@ -28,7 +28,7 @@ from cryptography.x509.general_name import ( ) from cryptography.x509.name import Name, NameAttribute from cryptography.x509.oid import ( - AuthorityInformationAccessOID, CRLExtensionOID, CertificatePoliciesOID, + AuthorityInformationAccessOID, RevokedExtensionOID, CertificatePoliciesOID, ExtendedKeyUsageOID, ExtensionOID, NameOID, ObjectIdentifier, SignatureAlgorithmOID, _SIG_OIDS_TO_HASH ) @@ -95,9 +95,9 @@ OID_ANY_POLICY = CertificatePoliciesOID.ANY_POLICY OID_CPS_QUALIFIER = CertificatePoliciesOID.CPS_QUALIFIER OID_CPS_USER_NOTICE = CertificatePoliciesOID.CPS_USER_NOTICE -OID_CERTIFICATE_ISSUER = CRLExtensionOID.CERTIFICATE_ISSUER -OID_CRL_REASON = CRLExtensionOID.CRL_REASON -OID_INVALIDITY_DATE = CRLExtensionOID.INVALIDITY_DATE +OID_CERTIFICATE_ISSUER = RevokedExtensionOID.CERTIFICATE_ISSUER +OID_CRL_REASON = RevokedExtensionOID.CRL_REASON +OID_INVALIDITY_DATE = RevokedExtensionOID.INVALIDITY_DATE OID_CA_ISSUERS = AuthorityInformationAccessOID.CA_ISSUERS OID_OCSP = AuthorityInformationAccessOID.OCSP diff --git a/src/cryptography/x509/base.py b/src/cryptography/x509/base.py index 9dc49a60..01eadfcb 100644 --- a/src/cryptography/x509/base.py +++ b/src/cryptography/x509/base.py @@ -176,12 +176,6 @@ class CertificateRevocationList(object): Returns the date of last update for this CRL. """ - @abc.abstractproperty - def revoked_certificates(self): - """ - Returns a list of RevokedCertificate objects for this CRL. - """ - @abc.abstractproperty def extensions(self): """ diff --git a/src/cryptography/x509/oid.py b/src/cryptography/x509/oid.py index ead40169..667045af 100644 --- a/src/cryptography/x509/oid.py +++ b/src/cryptography/x509/oid.py @@ -58,7 +58,7 @@ class ExtensionOID(object): OCSP_NO_CHECK = ObjectIdentifier("1.3.6.1.5.5.7.48.1.5") -class CRLExtensionOID(object): +class RevokedExtensionOID(object): CERTIFICATE_ISSUER = ObjectIdentifier("2.5.29.29") CRL_REASON = ObjectIdentifier("2.5.29.21") INVALIDITY_DATE = ObjectIdentifier("2.5.29.24") @@ -177,9 +177,9 @@ _OID_NAMES = { ExtensionOID.SUBJECT_ALTERNATIVE_NAME: "subjectAltName", ExtensionOID.ISSUER_ALTERNATIVE_NAME: "issuerAltName", ExtensionOID.BASIC_CONSTRAINTS: "basicConstraints", - CRLExtensionOID.CRL_REASON: "cRLReason", - CRLExtensionOID.INVALIDITY_DATE: "invalidityDate", - CRLExtensionOID.CERTIFICATE_ISSUER: "certificateIssuer", + RevokedExtensionOID.CRL_REASON: "cRLReason", + RevokedExtensionOID.INVALIDITY_DATE: "invalidityDate", + RevokedExtensionOID.CERTIFICATE_ISSUER: "certificateIssuer", ExtensionOID.NAME_CONSTRAINTS: "nameConstraints", ExtensionOID.CRL_DISTRIBUTION_POINTS: "cRLDistributionPoints", ExtensionOID.CERTIFICATE_POLICIES: "certificatePolicies", diff --git a/tests/test_x509.py b/tests/test_x509.py index 61e7a7d0..347ed1a6 100644 --- a/tests/test_x509.py +++ b/tests/test_x509.py @@ -153,17 +153,19 @@ class TestCertificateRevocationList(object): assert crl.next_update.isoformat() == "2016-01-01T00:00:00" assert crl.last_update.isoformat() == "2015-01-01T00:00:00" - def test_revoked_certs(self, backend): + def test_revoked_cert_retrieval(self, backend): crl = _load_cert( os.path.join("x509", "custom", "crl_all_reasons.pem"), x509.load_pem_x509_crl, backend ) - assert isinstance(crl.revoked_certificates, list) - for r in crl.revoked_certificates: + for r in crl: assert isinstance(r, x509.RevokedCertificate) + # Check that len() works for CRLs. + assert len(crl) == 12 + def test_extensions(self, backend): crl = _load_cert( os.path.join("x509", "custom", "crl_all_reasons.pem"), @@ -186,7 +188,7 @@ class TestRevokedCertificate(object): backend ) - for i, rev in enumerate(crl.revoked_certificates): + for i, rev in enumerate(crl): assert isinstance(rev, x509.RevokedCertificate) assert isinstance(rev.serial_number, int) assert isinstance(rev.revocation_date, datetime.datetime) @@ -204,7 +206,7 @@ class TestRevokedCertificate(object): # First revoked cert doesn't have extensions, test if it is handled # correctly. - rev0 = crl.revoked_certificates[0] + rev0 = crl[0] # It should return an empty Extensions object. assert isinstance(rev0.extensions, x509.Extensions) assert len(rev0.extensions) == 0 @@ -216,7 +218,7 @@ class TestRevokedCertificate(object): assert rev0.get_reason() is None # Test manual retrieval of extension values. - rev1 = crl.revoked_certificates[1] + rev1 = crl[1] assert isinstance(rev1.extensions, x509.Extensions) reason = rev1.extensions.get_extension_for_oid( @@ -232,18 +234,11 @@ class TestRevokedCertificate(object): assert rev1.get_invalidity_date().isoformat() == "2015-01-01T00:00:00" # Check if all reason flags can be found in the CRL. - # Also test if CRL as iterator works. flags = set(x509.ReasonFlags) for r in crl: flags.discard(r.get_reason()) assert len(flags) == 0 - # Check that len() works for CRLs. - assert len(crl) == 12 - - # Check that direct access to revoked cert in CRL works - assert isinstance(crl[0], x509.RevokedCertificate) - def test_duplicate_entry_ext(self, backend): crl = _load_cert( os.path.join("x509", "custom", "crl_dup_entry_ext.pem"), @@ -252,7 +247,7 @@ class TestRevokedCertificate(object): ) with pytest.raises(x509.DuplicateExtension): - crl.revoked_certificates[0].extensions + crl[0].extensions def test_unsupported_crit_entry_ext(self, backend): crl = _load_cert( @@ -264,7 +259,7 @@ class TestRevokedCertificate(object): ) with pytest.raises(x509.UnsupportedExtension): - crl.revoked_certificates[0].extensions + crl[0].extensions def test_unsupported_reason(self, backend): crl = _load_cert( @@ -276,7 +271,7 @@ class TestRevokedCertificate(object): ) with pytest.raises(ValueError): - crl.revoked_certificates[0].extensions + crl[0].extensions def test_cert_issuer_ext(self, backend): if backend._lib.OPENSSL_VERSION_NUMBER < 0x10000000: @@ -295,7 +290,7 @@ class TestRevokedCertificate(object): ])) ]) - rev = crl.revoked_certificates[1] + rev = crl[1] issuer = rev.extensions.get_extension_for_oid( x509.OID_CERTIFICATE_ISSUER).value assert issuer == exp_issuer -- cgit v1.2.3