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

Add support for cryptography CRLs to X509Store #1252

Merged
merged 2 commits into from Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -30,6 +30,10 @@ Deprecations:
Changes:
^^^^^^^^

- Changed ``OpenSSL.crypto.X509Store.add_crl`` to also accept
``cryptography``'s ``X509.CertificateRevocationList`` arguments in addition
to the now deprecated ``OpenSSL.crypto.CRL`` arguments.

23.2.0 (2023-05-30)
-------------------

Expand Down
34 changes: 27 additions & 7 deletions src/OpenSSL/crypto.py
Expand Up @@ -1720,7 +1720,9 @@ def add_cert(self, cert: X509) -> None:
res = _lib.X509_STORE_add_cert(self._store, cert._x509)
_openssl_assert(res == 1)

def add_crl(self, crl: "CRL") -> None:
def add_crl(
self, crl: Union["_CRLInternal", x509.CertificateRevocationList]
) -> None:
"""
Add a certificate revocation list to this store.

Expand All @@ -1730,11 +1732,29 @@ def add_crl(self, crl: "CRL") -> None:

.. versionadded:: 16.1.0

:param CRL crl: The certificate revocation list to add to this store.
:param crl: The certificate revocation list to add to this store.
:type crl: ``Union[CRL, cryptography.x509.CertificateRevocationList]``
:return: ``None`` if the certificate revocation list was added
successfully.
"""
_openssl_assert(_lib.X509_STORE_add_crl(self._store, crl._crl) != 0)
if isinstance(crl, x509.CertificateRevocationList):
from cryptography.hazmat.primitives.serialization import Encoding

bio = _new_mem_buf(crl.public_bytes(Encoding.DER))
openssl_crl = _lib.d2i_X509_CRL_bio(bio, _ffi.NULL)
if openssl_crl == _ffi.NULL:
_raise_current_error()

crl = _ffi.gc(openssl_crl, _lib.X509_CRL_free)
elif isinstance(crl, _CRLInternal):
crl = crl._crl
else:
raise TypeError(
"CRL must be of type OpenSSL.crypto.CRL or "
"cryptography.x509.CertificateRevocationList"
)

_openssl_assert(_lib.X509_STORE_add_crl(self._store, crl) != 0)

def set_flags(self, flags: int) -> None:
"""
Expand Down Expand Up @@ -2404,7 +2424,7 @@ def to_cryptography(self) -> x509.CertificateRevocationList:
@classmethod
def from_cryptography(
cls, crypto_crl: x509.CertificateRevocationList
) -> "CRL":
) -> "_CRLInternal":
"""
Construct based on a ``cryptography`` *crypto_crl*.

Expand Down Expand Up @@ -2445,7 +2465,7 @@ def get_revoked(self) -> Optional[Tuple[_RevokedInternal, ...]]:
return tuple(results)
return None

def add_revoked(self, revoked: Revoked) -> None:
def add_revoked(self, revoked: _RevokedInternal) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Did this fail mypy or are we avoiding deprecation warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's for the docs, see the current type signature for this function:

add_revoked(revoked: <cryptography.utils._DeprecatedValue object at 0x7f66c71ab1d0>) → None
Add a revoked (by value not reference) to the CRL structure

"""
Add a revoked (by value not reference) to the CRL structure

Expand Down Expand Up @@ -3222,7 +3242,7 @@ def verify(
)


def dump_crl(type: int, crl: CRL) -> bytes:
def dump_crl(type: int, crl: _CRLInternal) -> bytes:
"""
Dump a certificate revocation list to a buffer.

Expand Down Expand Up @@ -3264,7 +3284,7 @@ def dump_crl(type: int, crl: CRL) -> bytes:
)


def load_crl(type: int, buffer: Union[str, bytes]) -> CRL:
def load_crl(type: int, buffer: Union[str, bytes]) -> _CRLInternal:
"""
Load Certificate Revocation List (CRL) data from a string *buffer*.
*buffer* encoded with the type *type*.
Expand Down
76 changes: 69 additions & 7 deletions tests/test_crypto.py
Expand Up @@ -13,7 +13,7 @@
import flaky
import pytest
from cryptography import x509
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.primitives import hashes, serialization
from cryptography.hazmat.primitives.asymmetric import ec, ed448, ed25519, rsa

from OpenSSL._util import ffi as _ffi
Expand Down Expand Up @@ -3505,7 +3505,8 @@ def test_dump_crl(self):
buf = dump_crl(FILETYPE_PEM, crl)
assert buf == crlData

def _make_test_crl(self, issuer_cert, issuer_key, certs=()):
@staticmethod
def _make_test_crl(issuer_cert, issuer_key, certs=()):
"""
Create a CRL.

Expand All @@ -3531,18 +3532,66 @@ def _make_test_crl(self, issuer_cert, issuer_key, certs=()):
crl.sign(issuer_cert, issuer_key, digest=b"sha512")
return crl

def test_verify_with_revoked(self):
@staticmethod
def _make_test_crl_cryptography(issuer_cert, issuer_key, certs=()):
"""
Create a CRL using cryptography's API.

:param list[X509] certs: A list of certificates to revoke.
:rtype: ``cryptography.x509.CertificateRevocationList``
"""
from cryptography.x509.extensions import CRLReason, ReasonFlags

builder = x509.CertificateRevocationListBuilder()
builder = builder.issuer_name(
X509.to_cryptography(issuer_cert).subject
)
for cert in certs:
revoked = (
x509.RevokedCertificateBuilder()
.serial_number(cert.get_serial_number())
.revocation_date(datetime(2014, 6, 1, 0, 0, 0))
.add_extension(CRLReason(ReasonFlags.unspecified), False)
.build()
)
builder = builder.add_revoked_certificate(revoked)

builder = builder.last_update(datetime(2014, 6, 1, 0, 0, 0))
# The year 5000 is far into the future so that this CRL isn't
# considered to have expired.
builder = builder.next_update(datetime(5000, 6, 1, 0, 0, 0))

crl = builder.sign(
private_key=PKey.to_cryptography_key(issuer_key),
algorithm=hashes.SHA512(),
)
return crl

@pytest.mark.parametrize(
"create_crl",
[
pytest.param(
_make_test_crl.__func__,
id="pyOpenSSL CRL",
),
pytest.param(
_make_test_crl_cryptography.__func__,
id="cryptography CRL",
),
],
)
def test_verify_with_revoked(self, create_crl):
"""
`verify_certificate` raises error when an intermediate certificate is
revoked.
"""
store = X509Store()
store.add_cert(self.root_cert)
store.add_cert(self.intermediate_cert)
root_crl = self._make_test_crl(
root_crl = create_crl(
self.root_cert, self.root_key, certs=[self.intermediate_cert]
)
intermediate_crl = self._make_test_crl(
intermediate_crl = create_crl(
self.intermediate_cert, self.intermediate_key, certs=[]
)
store.add_crl(root_crl)
Expand All @@ -3555,15 +3604,28 @@ def test_verify_with_revoked(self):
store_ctx.verify_certificate()
assert str(err.value) == "certificate revoked"

def test_verify_with_missing_crl(self):
@pytest.mark.parametrize(
"create_crl",
[
pytest.param(
_make_test_crl.__func__,
id="pyOpenSSL CRL",
),
pytest.param(
_make_test_crl_cryptography.__func__,
id="cryptography CRL",
),
],
)
def test_verify_with_missing_crl(self, create_crl):
"""
`verify_certificate` raises error when an intermediate certificate's
CRL is missing.
"""
store = X509Store()
store.add_cert(self.root_cert)
store.add_cert(self.intermediate_cert)
root_crl = self._make_test_crl(
root_crl = create_crl(
self.root_cert, self.root_key, certs=[self.intermediate_cert]
)
store.add_crl(root_crl)
Expand Down