Skip to content

Commit

Permalink
Allow using additional untrusted certificates for chain building in X…
Browse files Browse the repository at this point in the history
…509StoreContext

The additional certificates provided in the new `chain` parameter will be
untrusted but may be used to build the chain.

This makes it easier to validate a certificate against a store which
contains only root ca certificates, and the intermediates come from e.g.
the same untrusted source as the certificate to be verified.
  • Loading branch information
Sandor Oroszi committed Oct 9, 2020
1 parent 43c9776 commit 79b5ea8
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ Deprecations:
Changes:
^^^^^^^^

- Added a new optional ``chain`` parameter to ``OpenSSL.crypto.X509StoreContext()``
where additional untrusted certificates can be specified to help chain building.
`#948 <https://github.com/pyca/pyopenssl/pull/948>`_
- Added ``OpenSSL.crypto.X509Store.load_locations`` to set trusted
certificate file bundles and/or directories for verification.
`#943 <https://github.com/pyca/pyopenssl/pull/943>`_
Expand Down
36 changes: 34 additions & 2 deletions src/OpenSSL/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -1750,22 +1750,54 @@ class X509StoreContext(object):
collected.
:ivar _store: See the ``store`` ``__init__`` parameter.
:ivar _cert: See the ``certificate`` ``__init__`` parameter.
:ivar _chain: See the ``chain`` ``__init__`` parameter.
:param X509Store store: The certificates which will be trusted for the
purposes of any verifications.
:param X509 certificate: The certificate to be verified.
:param chain: List of untrusted certificates that may be used for building
the certificate chain. May be ``None``.
:type chain: :class:`list` of :class:`X509`
"""

def __init__(self, store, certificate):
def __init__(self, store, certificate, chain=None):
store_ctx = _lib.X509_STORE_CTX_new()
self._store_ctx = _ffi.gc(store_ctx, _lib.X509_STORE_CTX_free)
self._store = store
self._cert = certificate
self._chain = _ffi.NULL
self._chain = self._build_certificate_stack(chain)
# Make the store context available for use after instantiating this
# class by initializing it now. Per testing, subsequent calls to
# :meth:`_init` have no adverse affect.
self._init()

@staticmethod
def _build_certificate_stack(certificates):
def cleanup(s):
# Equivalent to sk_X509_pop_free, but we don't
# currently have a CFFI binding for that available
for i in range(_lib.sk_X509_num(s)):
x = _lib.sk_X509_value(s, i)
_lib.X509_free(x)
_lib.sk_X509_free(s)

if certificates is None or len(certificates) == 0:
return _ffi.NULL

stack = _lib.sk_X509_new_null()
_openssl_assert(stack != _ffi.NULL)
stack = _ffi.gc(stack, cleanup)

for cert in certificates:
if not isinstance(cert, X509):
raise TypeError("One of the elements is not an X509 instance")

_openssl_assert(_lib.X509_up_ref(cert._x509) > 0)
if _lib.sk_X509_push(stack, cert._x509) <= 0:
_lib.X509_free(cert._x509)
_raise_current_error()

return stack

def _init(self):
"""
Set up the store context for a subsequent verification operation.
Expand Down
139 changes: 139 additions & 0 deletions tests/test_crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -3824,6 +3824,145 @@ def test_reuse(self):
assert store_ctx.verify_certificate() is None
assert store_ctx.verify_certificate() is None

@pytest.mark.parametrize(
"root_cert, chain, verified_cert",
[
pytest.param(
root_cert,
[intermediate_cert],
intermediate_server_cert,
id="intermediate in chain",
),
pytest.param(
root_cert,
[],
intermediate_cert,
id="empty chain",
),
pytest.param(
root_cert,
[root_cert, intermediate_server_cert, intermediate_cert],
intermediate_server_cert,
id="extra certs in chain",
),
],
)
def test_verify_success_with_chain(self, root_cert, chain, verified_cert):
store = X509Store()
store.add_cert(root_cert)
store_ctx = X509StoreContext(store, verified_cert, chain=chain)
assert store_ctx.verify_certificate() is None

def test_valid_untrusted_chain_reuse(self):
"""
`verify_certificate` using an untrusted chain can be called multiple
times with the same ``X509StoreContext`` instance to produce the same
result.
"""
store = X509Store()
store.add_cert(self.root_cert)
chain = [self.intermediate_cert]

store_ctx = X509StoreContext(
store, self.intermediate_server_cert, chain=chain
)
assert store_ctx.verify_certificate() is None
assert store_ctx.verify_certificate() is None

def test_chain_reference(self):
"""
``X509StoreContext`` properly keeps references to the untrusted chain
certificates.
"""
store = X509Store()
store.add_cert(self.root_cert)
chain = [load_certificate(FILETYPE_PEM, intermediate_cert_pem)]

store_ctx = X509StoreContext(
store, self.intermediate_server_cert, chain=chain
)

del chain
assert store_ctx.verify_certificate() is None

@pytest.mark.parametrize(
"root_cert, chain, verified_cert",
[
pytest.param(
root_cert,
[],
intermediate_server_cert,
id="intermediate missing",
),
pytest.param(
None,
[intermediate_cert],
intermediate_server_cert,
id="no trusted root",
),
pytest.param(
None,
[root_cert, intermediate_cert],
intermediate_server_cert,
id="untrusted root, full chain is available",
),
pytest.param(
intermediate_cert,
[root_cert, intermediate_cert],
intermediate_server_cert,
id="untrusted root, intermediate is trusted and in chain",
),
],
)
def test_verify_fail_with_chain(self, root_cert, chain, verified_cert):
store = X509Store()
if root_cert:
store.add_cert(root_cert)

store_ctx = X509StoreContext(store, verified_cert, chain=chain)

with pytest.raises(X509StoreContextError) as exc:
store_ctx.verify_certificate()

@pytest.mark.parametrize(
"chain, expected_error",
[
pytest.param(
[intermediate_cert, "This is not a certificate"],
TypeError,
id="non-certificate in chain",
),
pytest.param(
42,
TypeError,
id="non-list chain",
),
],
)
def test_untrusted_chain_wrong_args(self, chain, expected_error):
"""
Creating ``X509StoreContext`` with wrong chain raises an exception.
"""
store = X509Store()
store.add_cert(self.root_cert)

with pytest.raises(expected_error):
X509StoreContext(store, self.intermediate_server_cert, chain=chain)

def test_failure_building_untrusted_chain_raises(self, monkeypatch):
"""
Creating ``X509StoreContext`` raises ``OpenSSL.crypto.Error`` when
the underlying lib fails to add the certificate to the stack.
"""
monkeypatch.setattr(_lib, "sk_X509_push", lambda _stack, _x509: -1)

store = X509Store()
store.add_cert(self.root_cert)
chain = [self.intermediate_cert]

with pytest.raises(Error):
X509StoreContext(store, self.intermediate_server_cert, chain=chain)

def test_trusted_self_signed(self):
"""
`verify_certificate` returns ``None`` when called with a self-signed
Expand Down

0 comments on commit 79b5ea8

Please sign in to comment.