start switching the CRL entry extensions to be full-fledged classes

first up: CertificateIssuer

7 files changed, 127 insertions, 12 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 3e24633e..ab6468d8 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -6,6 +6,17 @@ Changelog
 
 .. note:: This version is not yet released and is under active development.
 
+* **BACKWARDS INCOMPATIBLE:**
+  :class:`~cryptography.x509.RevokedCertificate`
+  :attr:`~cryptography.x509.RevokedCertificate.extensions` now uses extension
+  classes rather than returning raw values inside the
+  :class:`~cryptography.x509.Extension`
+  :attr:`~cryptography.x509.Extension.value`. The new classes
+  are:
+
+  * :class:`~cryptography.x509.CertificateIssuer`
+  * ``CRLReason``
+  * ``InvalidityDate``
 * The :class:`~cryptography.x509.Certificate` class now has
   :attr:`~cryptography.x509.Certificate.signature` and
   :attr:`~cryptography.x509.Certificate.tbs_certificate_bytes` attributes.
diff --git a/docs/x509/reference.rst b/docs/x509/reference.rst
index ea22ab0b..1f25ac14 100644
--- a/docs/x509/reference.rst
+++ b/docs/x509/reference.rst
@@ -1956,6 +1956,40 @@ These classes may be present within a :class:`CertificatePolicies` instance.
 
         A list of integers.
 
+CRL Entry Extensions
+~~~~~~~~~~~~~~~~~~~~
+
+These extensions are only valid within a :class:`RevokedCertificate` object.
+
+.. class:: CertificateIssuer(general_names)
+
+    .. versionadded:: 1.2
+
+    The certificate issuer is an extension that is only valid inside
+    :class:`~cryptography.x509.RevokedCertificate` objects.  If the
+    ``indirectCRL`` property of the parent CRL's IssuingDistributionPoint
+    extension is set, then this extension identifies the certificate issuer
+    associated with the revoked certificate. The object is iterable to get
+    every element.
+
+    :param list general_names: A list of :class:`GeneralName` instances.
+
+    .. attribute:: oid
+
+        :type: :class:`ObjectIdentifier`
+
+        Returns
+        :attr:`~cryptography.x509.oid.CRLEntryExtensionOID.CERTIFICATE_ISSUER`.
+
+    .. method:: get_values_for_type(type)
+
+        :param type: A :class:`GeneralName` instance. This is one of the
+            :ref:`general name classes <general_name_classes>`.
+
+        :returns: A list of values extracted from the matched general names.
+            The type of the returned values depends on the :class:`GeneralName`.
+
+
 Object Identifiers
 ~~~~~~~~~~~~~~~~~~
 
diff --git a/src/cryptography/hazmat/backends/openssl/x509.py b/src/cryptography/hazmat/backends/openssl/x509.py
index f3286b05..05390809 100644
--- a/src/cryptography/hazmat/backends/openssl/x509.py
+++ b/src/cryptography/hazmat/backends/openssl/x509.py
@@ -743,7 +743,7 @@ def _decode_cert_issuer(backend, ext):
                 CRLEntryExtensionOID.CERTIFICATE_ISSUER))
 
     gns = backend._ffi.gc(gns, backend._lib.GENERAL_NAMES_free)
-    return x509.GeneralNames(_decode_general_names(backend, gns))
+    return x509.CertificateIssuer(_decode_general_names(backend, gns))
 
 
 @utils.register_interface(x509.RevokedCertificate)
diff --git a/src/cryptography/x509/__init__.py b/src/cryptography/x509/__init__.py
index 32543e67..9946daa0 100644
--- a/src/cryptography/x509/__init__.py
+++ b/src/cryptography/x509/__init__.py
@@ -15,9 +15,10 @@ from cryptography.x509.base import (
 from cryptography.x509.extensions import (
     AccessDescription, AuthorityInformationAccess,
     AuthorityKeyIdentifier, BasicConstraints, CRLDistributionPoints,
-    CRLNumber, CertificatePolicies, DistributionPoint, DuplicateExtension,
-    ExtendedKeyUsage, Extension, ExtensionNotFound, ExtensionType, Extensions,
-    GeneralNames, InhibitAnyPolicy, IssuerAlternativeName, KeyUsage,
+    CRLNumber, CertificateIssuer, CertificatePolicies,
+    DistributionPoint, DuplicateExtension, ExtendedKeyUsage, Extension,
+    ExtensionNotFound, ExtensionType, Extensions, GeneralNames,
+    InhibitAnyPolicy, IssuerAlternativeName, KeyUsage,
     NameConstraints, NoticeReference, OCSPNoCheck, PolicyInformation,
     ReasonFlags, SubjectAlternativeName, SubjectKeyIdentifier,
     UnsupportedExtension, UserNotice
@@ -165,4 +166,5 @@ __all__ = [
     "OID_OCSP",
     "_GENERAL_NAMES",
     "CRLExtensionOID",
+    "CertificateIssuer",
 ]
diff --git a/src/cryptography/x509/extensions.py b/src/cryptography/x509/extensions.py
index 15feb717..8eb1d34e 100644
--- a/src/cryptography/x509/extensions.py
+++ b/src/cryptography/x509/extensions.py
@@ -18,7 +18,9 @@ from cryptography import utils
 from cryptography.hazmat.primitives import constant_time, serialization
 from cryptography.x509.general_name import GeneralName, IPAddress, OtherName
 from cryptography.x509.name import Name
-from cryptography.x509.oid import ExtensionOID, ObjectIdentifier
+from cryptography.x509.oid import (
+    CRLEntryExtensionOID, ExtensionOID, ObjectIdentifier
+)
 
 
 class _SubjectPublicKeyInfo(univ.Sequence):
@@ -942,3 +944,32 @@ class IssuerAlternativeName(object):
 
     def __ne__(self, other):
         return not self == other
+
+
+@utils.register_interface(ExtensionType)
+class CertificateIssuer(object):
+    oid = CRLEntryExtensionOID.CERTIFICATE_ISSUER
+
+    def __init__(self, general_names):
+        self._general_names = GeneralNames(general_names)
+
+    def __iter__(self):
+        return iter(self._general_names)
+
+    def __len__(self):
+        return len(self._general_names)
+
+    def get_values_for_type(self, type):
+        return self._general_names.get_values_for_type(type)
+
+    def __repr__(self):
+        return "<CertificateIssuer({0})>".format(self._general_names)
+
+    def __eq__(self, other):
+        if not isinstance(other, CertificateIssuer):
+            return NotImplemented
+
+        return self._general_names == other._general_names
+
+    def __ne__(self, other):
+        return not self == other
diff --git a/tests/test_x509.py b/tests/test_x509.py
index 755b65fa..c91f08ba 100644
--- a/tests/test_x509.py
+++ b/tests/test_x509.py
@@ -355,12 +355,12 @@ class TestRevokedCertificate(object):
             backend
         )
 
-        exp_issuer = x509.GeneralNames([
+        exp_issuer = [
             x509.DirectoryName(x509.Name([
                 x509.NameAttribute(x509.OID_COUNTRY_NAME, u"US"),
                 x509.NameAttribute(x509.OID_COMMON_NAME, u"cryptography.io"),
             ]))
-        ])
+        ]
 
         # First revoked cert doesn't have extensions, test if it is handled
         # correctly.
@@ -383,14 +383,13 @@ class TestRevokedCertificate(object):
             x509.OID_CRL_REASON).value
         assert reason == x509.ReasonFlags.unspecified
 
-        issuer = rev1.extensions.get_extension_for_oid(
-            x509.OID_CERTIFICATE_ISSUER).value
-        assert issuer == exp_issuer
+        issuer = rev1.extensions.get_extension_for_class(
+            x509.CertificateIssuer).value
+        assert issuer == x509.CertificateIssuer(exp_issuer)
 
         date = rev1.extensions.get_extension_for_oid(
             x509.OID_INVALIDITY_DATE).value
-        assert isinstance(date, datetime.datetime)
-        assert date.isoformat() == "2015-01-01T00:00:00"
+        assert date == datetime.datetime(2015, 1, 1, 0, 0)
 
         # Check if all reason flags can be found in the CRL.
         flags = set(x509.ReasonFlags)
diff --git a/tests/test_x509_ext.py b/tests/test_x509_ext.py
index d9743c8e..f124a286 100644
--- a/tests/test_x509_ext.py
+++ b/tests/test_x509_ext.py
@@ -74,6 +74,44 @@ class TestExtension(object):
         assert ext1 != object()
 
 
+class TestCertificateIssuer(object):
+    def test_iter_names(self):
+        ci = x509.CertificateIssuer([
+            x509.DNSName(u"cryptography.io"),
+            x509.DNSName(u"crypto.local"),
+        ])
+        assert len(ci) == 2
+        assert list(ci) == [
+            x509.DNSName(u"cryptography.io"),
+            x509.DNSName(u"crypto.local"),
+        ]
+
+    def test_eq(self):
+        ci1 = x509.CertificateIssuer([x509.DNSName(u"cryptography.io")])
+        ci2 = x509.CertificateIssuer([x509.DNSName(u"cryptography.io")])
+        assert ci1 == ci2
+
+    def test_ne(self):
+        ci1 = x509.CertificateIssuer([x509.DNSName(u"cryptography.io")])
+        ci2 = x509.CertificateIssuer([x509.DNSName(u"somethingelse.tld")])
+        assert ci1 != ci2
+        assert ci1 != object()
+
+    def test_repr(self):
+        ci = x509.CertificateIssuer([x509.DNSName(u"cryptography.io")])
+        assert repr(ci) == (
+            "<CertificateIssuer(<GeneralNames([<DNSName(value=cryptography.io"
+            ")>])>)>"
+        )
+
+    def test_get_values_for_type(self):
+        ci = x509.CertificateIssuer(
+            [x509.DNSName(u"cryptography.io")]
+        )
+        names = ci.get_values_for_type(x509.DNSName)
+        assert names == [u"cryptography.io"]
+
+
 class TestNoticeReference(object):
     def test_notice_numbers_not_all_int(self):
         with pytest.raises(TypeError):