Merge pull request #2572 from reaperhulk/crlbuilder-add-revoked-certificate

support revoked certificates in CertificateRevocationListBuilder

5 files changed, 98 insertions, 2 deletions

diff --git a/docs/x509/reference.rst b/docs/x509/reference.rst
index 8d8bda4b..e02d4b20 100644
--- a/docs/x509/reference.rst
+++ b/docs/x509/reference.rst
@@ -788,12 +788,18 @@ X.509 Certificate Revocation List Builder
         ... ]))
         >>> builder = builder.last_update(datetime.datetime.today())
         >>> builder = builder.next_update(datetime.datetime.today() + one_day)
+        >>> revoked_cert = x509.RevokedCertificateBuilder().serial_number(
+        ...     333
+        ... ).revocation_date(
+        ...     datetime.datetime.today()
+        ... ).build(default_backend())
+        >>> builder = builder.add_revoked_certificate(revoked_cert)
         >>> crl = builder.sign(
         ...     private_key=private_key, algorithm=hashes.SHA256(),
         ...     backend=default_backend()
         ... )
-        >>> isinstance(crl, x509.CertificateRevocationList)
-        True
+        >>> len(crl)
+        1
 
     .. method:: issuer_name(name)
 
@@ -832,6 +838,15 @@ X.509 Certificate Revocation List Builder
         :param critical: Set to ``True`` if the extension must be understood and
              handled by whoever reads the CRL.
 
+    .. method:: add_revoked_certificate(revoked_certificate)
+
+        Adds a revoked certificate to this CRL.
+
+        :param revoked_certificate: An instance of
+            :class:`~cryptography.x509.RevokedCertificate`. These can be
+            obtained from an existing CRL or created with
+            :class:`~cryptography.x509.RevokedCertificateBuilder`.
+
     .. method:: sign(private_key, algorithm, backend)
 
         Sign this CRL using the CA's private key.
diff --git a/src/_cffi_src/openssl/x509.py b/src/_cffi_src/openssl/x509.py
index b58a1a27..c5eb600a 100644
--- a/src/_cffi_src/openssl/x509.py
+++ b/src/_cffi_src/openssl/x509.py
@@ -270,6 +270,8 @@ void PKCS8_PRIV_KEY_INFO_free(PKCS8_PRIV_KEY_INFO *);
 """
 
 MACROS = """
+X509_REVOKED *Cryptography_X509_REVOKED_dup(X509_REVOKED *);
+
 int i2d_X509_CINF(X509_CINF *, unsigned char **);
 int i2d_X509_CRL_INFO(X509_CRL_INFO *, unsigned char **);
 int i2d_X509_REQ_INFO(X509_REQ_INFO *, unsigned char **);
@@ -365,4 +367,12 @@ int (*i2d_ECPrivateKey_bio)(BIO *, EC_KEY *) = NULL;
 EC_KEY *(*o2i_ECPublicKey)(EC_KEY **, const unsigned char **, long) = NULL;
 int (*i2o_ECPublicKey)(EC_KEY *, unsigned char **) = NULL;
 #endif
+
+/* X509_REVOKED_dup only exists on 1.0.2+. It is implemented using
+   IMPLEMENT_ASN1_DUP_FUNCTION. The below is the equivalent so we have
+   it available on all OpenSSLs. */
+X509_REVOKED *Cryptography_X509_REVOKED_dup(X509_REVOKED *rev) {
+    return ASN1_item_dup(ASN1_ITEM_rptr(X509_REVOKED), rev);
+}
+
 """
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index 81316da5..7d8460c6 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -1519,6 +1519,17 @@ class Backend(object):
             gc=True
         )
 
+        # add revoked certificates
+        for revoked_cert in builder._revoked_certificates:
+            # Duplicating because the X509_CRL takes ownership and will free
+            # this memory when X509_CRL_free is called.
+            revoked = self._lib.Cryptography_X509_REVOKED_dup(
+                revoked_cert._x509_revoked
+            )
+            self.openssl_assert(revoked != self._ffi.NULL)
+            res = self._lib.X509_CRL_add0_revoked(x509_crl, revoked)
+            self.openssl_assert(res == 1)
+
         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 e29a3105..bc927e87 100644
--- a/src/cryptography/x509/base.py
+++ b/src/cryptography/x509/base.py
@@ -591,6 +591,19 @@ class CertificateRevocationListBuilder(object):
             self._extensions + [extension], self._revoked_certificates
         )
 
+    def add_revoked_certificate(self, revoked_certificate):
+        """
+        Adds a revoked certificate to the CRL.
+        """
+        if not isinstance(revoked_certificate, RevokedCertificate):
+            raise TypeError("Must be an instance of RevokedCertificate")
+
+        return CertificateRevocationListBuilder(
+            self._issuer_name, self._last_update,
+            self._next_update, self._extensions,
+            self._revoked_certificates + [revoked_certificate]
+        )
+
     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 f2db5416..de3adcd4 100644
--- a/tests/test_x509_crlbuilder.py
+++ b/tests/test_x509_crlbuilder.py
@@ -104,6 +104,12 @@ class TestCertificateRevocationListBuilder(object):
                 object(), False
             )
 
+    def test_add_invalid_revoked_certificate(self):
+        builder = x509.CertificateRevocationListBuilder()
+
+        with pytest.raises(TypeError):
+            builder.add_revoked_certificate(object())
+
     @pytest.mark.requires_backend_interface(interface=RSABackend)
     @pytest.mark.requires_backend_interface(interface=X509Backend)
     def test_no_issuer_name(self, backend):
@@ -338,3 +344,44 @@ class TestCertificateRevocationListBuilder(object):
 
         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_with_revoked_certificates(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)
+        revoked_cert0 = x509.RevokedCertificateBuilder().serial_number(
+            38
+        ).revocation_date(
+            datetime.datetime(2011, 1, 1, 1, 1)
+        ).build(backend)
+        revoked_cert1 = x509.RevokedCertificateBuilder().serial_number(
+            2
+        ).revocation_date(
+            datetime.datetime(2012, 1, 1, 1, 1)
+        ).build(backend)
+        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_revoked_certificate(
+            revoked_cert0
+        ).add_revoked_certificate(
+            revoked_cert1
+        )
+
+        crl = builder.sign(private_key, hashes.SHA256(), backend)
+        assert len(crl) == 2
+        assert crl.last_update == last_update
+        assert crl.next_update == next_update
+        assert crl[0].serial_number == revoked_cert0.serial_number
+        assert crl[0].revocation_date == revoked_cert0.revocation_date
+        assert len(crl[0].extensions) == 0
+        assert crl[1].serial_number == revoked_cert1.serial_number
+        assert crl[1].revocation_date == revoked_cert1.revocation_date
+        assert len(crl[1].extensions) == 0