Merge pull request #2568 from reaperhulk/crlbuilder-extensions

add extension support to the CRLBuilder

4 files changed, 176 insertions, 2 deletions

diff --git a/docs/x509/reference.rst b/docs/x509/reference.rst
index 859bc838..e4711be3 100644
--- a/docs/x509/reference.rst
+++ b/docs/x509/reference.rst
@@ -822,6 +822,16 @@ X.509 Certificate Revocation List Builder
         :param time: The :class:`datetime.datetime` object (in UTC) that marks
             the next update time for this CRL.
 
+    .. method:: add_extension(extension, critical)
+
+        Adds an X.509 extension to this CRL.
+
+        :param extension: An extension with the
+            :class:`~cryptography.x509.ExtensionType` interface.
+
+        :param critical: Set to ``True`` if the extension must be understood and
+             handled by whoever reads the CRL.
+
     .. method:: sign(private_key, algorithm, backend)
 
         Sign this CRL using the CA's private key.
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index 86c1a813..65792c3b 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -153,6 +153,17 @@ def _encode_name_gc(backend, attributes):
     return subject
 
 
+def _encode_crl_number(backend, crl_number):
+    asn1int = _encode_asn1_int_gc(backend, crl_number.crl_number)
+    pp = backend._ffi.new('unsigned char **')
+    r = backend._lib.i2d_ASN1_INTEGER(asn1int, pp)
+    backend.openssl_assert(r > 0)
+    pp = backend._ffi.gc(
+        pp, lambda pointer: backend._lib.OPENSSL_free(pointer[0])
+    )
+    return pp, r
+
+
 def _encode_certificate_policies(backend, certificate_policies):
     cp = backend._lib.sk_POLICYINFO_new_null()
     backend.openssl_assert(cp != backend._ffi.NULL)
@@ -625,6 +636,15 @@ _EXTENSION_ENCODE_HANDLERS = {
     ExtensionOID.NAME_CONSTRAINTS: _encode_name_constraints,
 }
 
+_CRL_EXTENSION_ENCODE_HANDLERS = {
+    ExtensionOID.ISSUER_ALTERNATIVE_NAME: _encode_alt_name,
+    ExtensionOID.AUTHORITY_KEY_IDENTIFIER: _encode_authority_key_identifier,
+    ExtensionOID.AUTHORITY_INFORMATION_ACCESS: (
+        _encode_authority_information_access
+    ),
+    ExtensionOID.CRL_NUMBER: _encode_crl_number,
+}
+
 
 class _PasswordUserdata(object):
     def __init__(self, password):
@@ -1490,7 +1510,15 @@ class Backend(object):
         self.openssl_assert(res == 1)
         # TODO: support revoked certificates
 
-        # TODO: add support for CRL extensions
+        # Add extensions.
+        self._create_x509_extensions(
+            extensions=builder._extensions,
+            handlers=_CRL_EXTENSION_ENCODE_HANDLERS,
+            x509_obj=x509_crl,
+            add_func=self._lib.X509_CRL_add_ext,
+            gc=True
+        )
+
         res = self._lib.X509_CRL_sign(
             x509_crl, private_key._evp_pkey, evp_md
         )
diff --git a/src/cryptography/x509/base.py b/src/cryptography/x509/base.py
index 6bca2c52..49cbcf75 100644
--- a/src/cryptography/x509/base.py
+++ b/src/cryptography/x509/base.py
@@ -573,6 +573,24 @@ class CertificateRevocationListBuilder(object):
             self._extensions, self._revoked_certificates
         )
 
+    def add_extension(self, extension, critical):
+        """
+        Adds an X.509 extension to the certificate revocation list.
+        """
+        if not isinstance(extension, ExtensionType):
+            raise TypeError("extension must be an ExtensionType")
+
+        extension = Extension(extension.oid, critical, extension)
+
+        # TODO: This is quadratic in the number of extensions
+        for e in self._extensions:
+            if e.oid == extension.oid:
+                raise ValueError('This extension has already been set.')
+        return CertificateRevocationListBuilder(
+            self._issuer_name, self._last_update, self._next_update,
+            self._extensions + [extension], self._revoked_certificates
+        )
+
     def sign(self, private_key, algorithm, backend):
         if self._issuer_name is None:
             raise ValueError("A CRL must have an issuer name")
diff --git a/tests/test_x509_crlbuilder.py b/tests/test_x509_crlbuilder.py
index c6b23174..f2db5416 100644
--- a/tests/test_x509_crlbuilder.py
+++ b/tests/test_x509_crlbuilder.py
@@ -14,7 +14,7 @@ from cryptography.hazmat.backends.interfaces import (
 )
 from cryptography.hazmat.primitives import hashes
 from cryptography.hazmat.primitives.asymmetric import ec
-from cryptography.x509.oid import NameOID
+from cryptography.x509.oid import AuthorityInformationAccessOID, NameOID
 
 from .hazmat.primitives.fixtures_dsa import DSA_KEY_2048
 from .hazmat.primitives.fixtures_rsa import RSA_KEY_2048, RSA_KEY_512
@@ -88,6 +88,22 @@ class TestCertificateRevocationListBuilder(object):
         with pytest.raises(ValueError):
             builder.next_update(datetime.datetime(2001, 1, 1, 12, 1))
 
+    def test_add_extension_checks_for_duplicates(self):
+        builder = x509.CertificateRevocationListBuilder().add_extension(
+            x509.CRLNumber(1), False
+        )
+
+        with pytest.raises(ValueError):
+            builder.add_extension(x509.CRLNumber(2), False)
+
+    def test_add_invalid_extension(self):
+        builder = x509.CertificateRevocationListBuilder()
+
+        with pytest.raises(TypeError):
+            builder.add_extension(
+                object(), False
+            )
+
     @pytest.mark.requires_backend_interface(interface=RSABackend)
     @pytest.mark.requires_backend_interface(interface=X509Backend)
     def test_no_issuer_name(self, backend):
@@ -144,6 +160,108 @@ class TestCertificateRevocationListBuilder(object):
         assert crl.last_update == last_update
         assert crl.next_update == next_update
 
+    @pytest.mark.parametrize(
+        "extension",
+        [
+            x509.CRLNumber(13),
+            x509.AuthorityKeyIdentifier(
+                b"\xc3\x9c\xf3\xfc\xd3F\x084\xbb\xceF\x7f\xa0|[\xf3\xe2\x08"
+                b"\xcbY",
+                None,
+                None
+            ),
+            x509.AuthorityInformationAccess([
+                x509.AccessDescription(
+                    AuthorityInformationAccessOID.CA_ISSUERS,
+                    x509.DNSName(u"cryptography.io")
+                )
+            ]),
+            x509.IssuerAlternativeName([
+                x509.UniformResourceIdentifier(u"https://cryptography.io"),
+            ])
+        ]
+    )
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_sign_extensions(self, backend, extension):
+        private_key = RSA_KEY_2048.private_key(backend)
+        last_update = datetime.datetime(2002, 1, 1, 12, 1)
+        next_update = datetime.datetime(2030, 1, 1, 12, 1)
+        builder = x509.CertificateRevocationListBuilder().issuer_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.COMMON_NAME, u"cryptography.io CA")
+            ])
+        ).last_update(
+            last_update
+        ).next_update(
+            next_update
+        ).add_extension(
+            extension, False
+        )
+
+        crl = builder.sign(private_key, hashes.SHA256(), backend)
+        assert len(crl) == 0
+        assert len(crl.extensions) == 1
+        ext = crl.extensions.get_extension_for_class(type(extension))
+        assert ext.critical is False
+        assert ext.value == extension
+
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_sign_multiple_extensions_critical(self, backend):
+        private_key = RSA_KEY_2048.private_key(backend)
+        last_update = datetime.datetime(2002, 1, 1, 12, 1)
+        next_update = datetime.datetime(2030, 1, 1, 12, 1)
+        ian = x509.IssuerAlternativeName([
+            x509.UniformResourceIdentifier(u"https://cryptography.io"),
+        ])
+        crl_number = x509.CRLNumber(13)
+        builder = x509.CertificateRevocationListBuilder().issuer_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.COMMON_NAME, u"cryptography.io CA")
+            ])
+        ).last_update(
+            last_update
+        ).next_update(
+            next_update
+        ).add_extension(
+            crl_number, False
+        ).add_extension(
+            ian, True
+        )
+
+        crl = builder.sign(private_key, hashes.SHA256(), backend)
+        assert len(crl) == 0
+        assert len(crl.extensions) == 2
+        ext1 = crl.extensions.get_extension_for_class(x509.CRLNumber)
+        assert ext1.critical is False
+        assert ext1.value == crl_number
+        ext2 = crl.extensions.get_extension_for_class(
+            x509.IssuerAlternativeName
+        )
+        assert ext2.critical is True
+        assert ext2.value == ian
+
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_add_unsupported_extension(self, backend):
+        private_key = RSA_KEY_2048.private_key(backend)
+        last_update = datetime.datetime(2002, 1, 1, 12, 1)
+        next_update = datetime.datetime(2030, 1, 1, 12, 1)
+        builder = x509.CertificateRevocationListBuilder().issuer_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.COMMON_NAME, u"cryptography.io CA")
+            ])
+        ).last_update(
+            last_update
+        ).next_update(
+            next_update
+        ).add_extension(
+            x509.OCSPNoCheck(), False
+        )
+        with pytest.raises(NotImplementedError):
+            builder.sign(private_key, hashes.SHA256(), backend)
+
     @pytest.mark.requires_backend_interface(interface=RSABackend)
     @pytest.mark.requires_backend_interface(interface=X509Backend)
     def test_sign_rsa_key_too_small(self, backend):