Skip to content

Commit

Permalink
Handle general OCSPValidationErrors cleanly
Browse files Browse the repository at this point in the history
  • Loading branch information
MatthiasValvekens committed Nov 18, 2023
1 parent 3733fa3 commit 2866e53
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
5 changes: 5 additions & 0 deletions pyhanko_certvalidator/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
NotYetValidError,
OCSPFetchError,
OCSPNoMatchesError,
OCSPValidationError,
OCSPValidationIndeterminateError,
PathBuildingError,
PathValidationError,
Expand Down Expand Up @@ -1328,6 +1329,10 @@ async def _check_revocation(
else:
failures.append(e.args[0])
revocation_check_failed = True
except OCSPValidationError as e:
failures.append(e.args[0])
revocation_check_failed = True
ocsp_matched = True
if not ocsp_status_good and rev_rule.ocsp_mandatory:
if failures:
err_str = '; '.join(str(f) for f in failures)
Expand Down
42 changes: 41 additions & 1 deletion tests/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import Iterable, List, Optional, Type

import pytest
import pytz
from asn1crypto import crl, ocsp, x509
from asn1crypto.util import timezone

Expand All @@ -18,6 +19,7 @@
CRLFetchError,
InsufficientRevinfoError,
OCSPFetchError,
OCSPValidationError,
PathValidationError,
RevokedError,
StaleRevinfoError,
Expand Down Expand Up @@ -64,6 +66,11 @@ async def fetch(self, cert: x509.Certificate, authority: Authority):
raise OCSPFetchError("No connection")


class MockOCSPFetcherWithValidationError(MockOCSPFetcher):
async def fetch(self, cert: x509.Certificate, authority: Authority):
raise OCSPValidationError("Something went wrong")


class MockCRLFetcher(CRLFetcher):
def fetched_crls_for_cert(
self, cert: x509.Certificate
Expand Down Expand Up @@ -100,6 +107,15 @@ def get_fetchers(self) -> Fetchers:
)


class MockFetcherBackendWithValidationError(FetcherBackend):
def get_fetchers(self) -> Fetchers:
return Fetchers(
ocsp_fetcher=MockOCSPFetcherWithValidationError(),
crl_fetcher=MockCRLFetcher(),
cert_fetcher=MockCertFetcher(),
)


ERR_CLASSES = {
cls.__name__: cls
for cls in (
Expand All @@ -117,7 +133,6 @@ class PKITSTestCaseErrorResult:
msg_regex: str


@pytest.mark.skip("annoying to maintain; replace with certomancer test")
def test_revocation_mode_soft():
cert = load_cert_object(
'digicert-ecc-p384-root-g5-revoked-chain-demos-digicert-com.crt'
Expand All @@ -130,6 +145,7 @@ def test_revocation_mode_soft():
context = ValidationContext(
trust_roots=ca_certs,
other_certs=other_certs,
moment=datetime(2023, 1, 10, tzinfo=pytz.UTC),
allow_fetching=True,
weak_hash_algos={'md2', 'md5'},
fetcher_backend=MockFetcherBackend(),
Expand All @@ -142,6 +158,30 @@ def test_revocation_mode_soft():
validate_path(context, path)


def test_revocation_mode_soft_fail():
cert = load_cert_object(
'digicert-ecc-p384-root-g5-revoked-chain-demos-digicert-com.crt'
)
ca_certs = [load_cert_object('digicert-root-g5.crt')]
other_certs = [
load_cert_object('digicert-g5-ecc-sha384-2021-ca1.crt'),
]

context = ValidationContext(
trust_roots=ca_certs,
other_certs=other_certs,
moment=datetime(2023, 1, 10, tzinfo=pytz.UTC),
allow_fetching=True,
weak_hash_algos={'md2', 'md5'},
fetcher_backend=MockFetcherBackendWithValidationError(),
)
paths = context.path_builder.build_paths(cert)
path = paths[0]

with pytest.raises(InsufficientRevinfoError, match="Something went wrong"):
validate_path(context, path)


@pytest.mark.skip("annoying to maintain; replace with certomancer test")
def test_revocation_mode_hard():
cert = load_cert_object(
Expand Down

0 comments on commit 2866e53

Please sign in to comment.