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

Allow using additional untrusted certificates for chain building in X509StoreContext #948

Merged
merged 1 commit into from
Oct 12, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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):
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