Skip to content

Commit

Permalink
Make X509StoreContextError's message friendlier (#1133)
Browse files Browse the repository at this point in the history
* OpenSSL/crypto: make X509StoreContextError's message friendlier

Closes #1132.

Signed-off-by: William Woodruff <william@trailofbits.com>

* tests: update exception tests

Signed-off-by: William Woodruff <william@trailofbits.com>

* OpenSSL/crypto: blacken

Signed-off-by: William Woodruff <william@trailofbits.com>

* CHANGELOG: record changes

Signed-off-by: William Woodruff <william@trailofbits.com>
  • Loading branch information
woodruffw authored Jul 7, 2022
1 parent 02db1a0 commit 65ca53a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 16 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ Backward-incompatible changes:

- Remove support for SSLv2 and SSLv3.
- The minimum ``cryptography`` version is now 37.0.2.
- The ``OpenSSL.crypto.X509StoreContextError`` exception has been refactored,
changing its internal attributes.
`#1133 <https://github.com/pyca/pyopenssl/pull/1133>`_

Deprecations:
^^^^^^^^^^^^^
Expand Down
18 changes: 11 additions & 7 deletions src/OpenSSL/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -1776,8 +1776,11 @@ class X509StoreContextError(Exception):
:type certificate: :class:`X509`
"""

def __init__(self, message: Any, certificate: X509) -> None:
def __init__(
self, message: str, errors: List[Any], certificate: X509
) -> None:
super(X509StoreContextError, self).__init__(message)
self.errors = errors
self.certificate = certificate


Expand Down Expand Up @@ -1878,21 +1881,22 @@ def _exception_from_context(self) -> X509StoreContextError:
When a call to native OpenSSL X509_verify_cert fails, additional
information about the failure can be obtained from the store context.
"""
message = _ffi.string(
_lib.X509_verify_cert_error_string(
_lib.X509_STORE_CTX_get_error(self._store_ctx)
)
).decode("utf-8")
errors = [
_lib.X509_STORE_CTX_get_error(self._store_ctx),
_lib.X509_STORE_CTX_get_error_depth(self._store_ctx),
_ffi.string(
_lib.X509_verify_cert_error_string(
_lib.X509_STORE_CTX_get_error(self._store_ctx)
)
).decode("utf-8"),
message,
]
# A context error should always be associated with a certificate, so we
# expect this call to never return :class:`None`.
_x509 = _lib.X509_STORE_CTX_get_current_cert(self._store_ctx)
_cert = _lib.X509_dup(_x509)
pycert = X509._from_raw_x509_ptr(_cert)
return X509StoreContextError(errors, pycert)
return X509StoreContextError(message, errors, pycert)

def set_store(self, store: X509Store) -> None:
"""
Expand Down
18 changes: 9 additions & 9 deletions tests/test_crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -3874,7 +3874,7 @@ def test_verify_with_revoked(self):
store_ctx = X509StoreContext(store, self.intermediate_server_cert)
with pytest.raises(X509StoreContextError) as err:
store_ctx.verify_certificate()
assert err.value.args[0][2] == "certificate revoked"
assert str(err.value) == "certificate revoked"

def test_verify_with_missing_crl(self):
"""
Expand All @@ -3894,7 +3894,7 @@ def test_verify_with_missing_crl(self):
store_ctx = X509StoreContext(store, self.intermediate_server_cert)
with pytest.raises(X509StoreContextError) as err:
store_ctx.verify_certificate()
assert err.value.args[0][2] == "unable to get certificate CRL"
assert str(err.value) == "unable to get certificate CRL"
assert err.value.certificate.get_subject().CN == "intermediate-service"

def test_convert_from_cryptography(self):
Expand Down Expand Up @@ -4106,7 +4106,7 @@ def test_untrusted_self_signed(self):
store_ctx.verify_certificate()

# OpenSSL 1.1.x and 3.0.x have different error messages
assert exc.value.args[0][2] in [
assert str(exc.value) in [
"self signed certificate",
"self-signed certificate",
]
Expand All @@ -4124,7 +4124,7 @@ def test_invalid_chain_no_root(self):
with pytest.raises(X509StoreContextError) as exc:
store_ctx.verify_certificate()

assert exc.value.args[0][2] == "unable to get issuer certificate"
assert str(exc.value) == "unable to get issuer certificate"
assert exc.value.certificate.get_subject().CN == "intermediate"

def test_invalid_chain_no_intermediate(self):
Expand All @@ -4139,7 +4139,7 @@ def test_invalid_chain_no_intermediate(self):
with pytest.raises(X509StoreContextError) as exc:
store_ctx.verify_certificate()

assert exc.value.args[0][2] == "unable to get local issuer certificate"
assert str(exc.value) == "unable to get local issuer certificate"
assert exc.value.certificate.get_subject().CN == "intermediate-service"

def test_modification_pre_verify(self):
Expand All @@ -4157,7 +4157,7 @@ def test_modification_pre_verify(self):
with pytest.raises(X509StoreContextError) as exc:
store_ctx.verify_certificate()

assert exc.value.args[0][2] == "unable to get issuer certificate"
assert str(exc.value) == "unable to get issuer certificate"
assert exc.value.certificate.get_subject().CN == "intermediate"

store_ctx.set_store(store_good)
Expand All @@ -4182,7 +4182,7 @@ def test_verify_with_time(self):
with pytest.raises(X509StoreContextError) as exc:
store_ctx.verify_certificate()

assert exc.value.args[0][2] == "certificate has expired"
assert str(exc.value) == "certificate has expired"

def test_get_verified_chain(self):
"""
Expand Down Expand Up @@ -4216,7 +4216,7 @@ def test_get_verified_chain_invalid_chain_no_root(self):
with pytest.raises(X509StoreContextError) as exc:
store_ctx.get_verified_chain()

assert exc.value.args[0][2] == "unable to get issuer certificate"
assert str(exc.value) == "unable to get issuer certificate"
assert exc.value.certificate.get_subject().CN == "intermediate"

@pytest.fixture
Expand Down Expand Up @@ -4281,7 +4281,7 @@ def test_verify_failure_with_empty_ca_directory(self, tmpdir):
with pytest.raises(X509StoreContextError) as exc:
store_ctx.verify_certificate()

assert exc.value.args[0][2] == "unable to get local issuer certificate"
assert str(exc.value) == "unable to get local issuer certificate"


class TestSignVerify:
Expand Down

0 comments on commit 65ca53a

Please sign in to comment.