Improve CRL cache usage

... by not downloading new CRLs until the cached ones have been shown to
be inadequate.

Author: MatthiasValvekens

pyhanko_certvalidator/revinfo/manager.py

@@ -1,10 +1,19 @@
 from datetime import datetime
-from typing import Dict, Iterable, List, Optional, Set
+from typing import (
+    AsyncGenerator,
+    AsyncIterable,
+    AsyncIterator,
+    Dict,
+    Iterable,
+    List,
+    Optional,
+    Set,
+)
 
 from asn1crypto import crl, ocsp, x509
 
 from pyhanko_certvalidator.authority import Authority
-from pyhanko_certvalidator.errors import OCSPFetchError
+from pyhanko_certvalidator.errors import CRLFetchError, OCSPFetchError
 from pyhanko_certvalidator.fetchers import Fetchers
 from pyhanko_certvalidator.ltv.poe import (
     KnownPOE,
@@ -189,9 +198,26 @@ def check_crl_issuer(self, certificate_list) -> Optional[x509.Certificate]:
 
         return self._crl_issuer_map.get(certificate_list.signature)
 
-    async def async_retrieve_crls(self, cert) -> List[CRLContainer]:
+    def currently_available_crls(self) -> List[CRLContainer]:
         """
-        .. versionadded:: 0.20.0
+        .. versionadded:: 0.27.0
+
+        Return all currently available CRLs.
+
+        :return:
+            A list of :class:`CRLContainer` objects
+        """
+        result = list(self._crls)
+        if self._fetchers:
+            crls = self._fetchers.crl_fetcher.fetched_crls()
+            result.extend(CRLContainer(crl_data) for crl_data in crls)
+        return result
+
+    async def fetch_crls(self, cert) -> List[CRLContainer]:
+        """
+        .. versionadded:: 0.27.0
+
+        Download all relevant CRLs for a given certificate.
 
         :param cert:
             An asn1crypto.x509.Certificate object
@@ -200,15 +226,30 @@ async def async_retrieve_crls(self, cert) -> List[CRLContainer]:
             A list of :class:`CRLContainer` objects
         """
         if not self._fetchers:
-            return self._crls
+            raise CRLFetchError("No CRL fetcher available")
 
         fetchers = self._fetchers
         try:
             crls = fetchers.crl_fetcher.fetched_crls_for_cert(cert)
         except KeyError:
             crls = await fetchers.crl_fetcher.fetch(cert)
         conts = [CRLContainer(crl_data) for crl_data in crls]
-        return conts + self._crls
+        return conts
+
+    async def async_retrieve_crls(self, cert) -> List[CRLContainer]:
+        """
+        .. versionadded:: 0.20.0
+
+        :param cert:
+            An asn1crypto.x509.Certificate object
+
+        :return:
+            A list of :class:`CRLContainer` objects
+        """
+        crls = self.currently_available_crls()
+        if self._fetchers:
+            crls.extend(await self.fetch_crls(cert))
+        return crls
 
     async def async_retrieve_ocsps(
         self, cert, authority: Authority

pyhanko_certvalidator/revinfo/validate_crl.py

@@ -21,6 +21,7 @@
     PSSParameterMismatch,
     RevokedError,
 )
+from pyhanko_certvalidator.ltv.poe import POEManager
 from pyhanko_certvalidator.ltv.types import ValidationTimingParams
 from pyhanko_certvalidator.path import ValidationPath
 from pyhanko_certvalidator.policy_decl import CertRevTrustPolicy
@@ -729,7 +730,11 @@ def _maybe_get_delta_crl(
         parent_crl_aki=certificate_list.authority_key_identifier,
     )
     if not delta_certificate_list_cont:
-        return None
+        raise CRLValidationIndeterminateError(
+            "Delta CRL matching Freshest CRL extension not available",
+            failures=[],
+            suspect_stale=None,
+        )
 
     delta_certificate_list = delta_certificate_list_cont.crl_data
 
@@ -924,17 +929,14 @@ def _check_cert_on_crl_and_delta(
 
 
 async def _classify_relevant_crls(
-    revinfo_manager: RevinfoManager,
-    cert: x509.Certificate,
+    certificate_lists: List[CRLContainer],
+    poe_manager: POEManager,
     errs: _CRLErrs,
     control_time: Optional[datetime] = None,
 ):
     # NOTE: the control_time parameter is only used in the time sliding
     # algorithm code path for AdES validation
 
-    certificate_lists = await revinfo_manager.async_retrieve_crls(cert)
-    poe_manager = revinfo_manager.poe_manager
-
     complete_lists_by_issuer = defaultdict(list)
     delta_lists_by_issuer = defaultdict(list)
     for certificate_list_cont in certificate_lists:
@@ -1046,11 +1048,6 @@ async def verify_crl(
 
     revinfo_manager = validation_context.revinfo_manager
     errs = _CRLErrs()
-    (
-        complete_lists_by_issuer,
-        delta_lists_by_issuer,
-    ) = await _classify_relevant_crls(revinfo_manager, cert, errs)
-
     try:
         cert_issuer_auth = path.find_issuing_authority(cert)
     except LookupError:
@@ -1059,6 +1056,14 @@ async def verify_crl(
             f"{proc_state.describe_cert()} in path."
         )
 
+    # First, make an attempt to validate without downloading any extra CRLs
+    certificate_lists = revinfo_manager.currently_available_crls()
+    poe_manager = revinfo_manager.poe_manager
+    (
+        complete_lists_by_issuer,
+        delta_lists_by_issuer,
+    ) = await _classify_relevant_crls(certificate_lists, poe_manager, errs)
+
     # In the main loop, only complete CRLs are processed, so delta CRLs are
     # weeded out of the to-do list
     crls_to_process = []
@@ -1068,27 +1073,76 @@ async def verify_crl(
 
     checked_reasons = set()
 
-    for certificate_list_cont in crls_to_process:
+    async def _process(crl_container, deltas):
+        nonlocal checked_reasons
+        nonlocal errs
         try:
             interim_reasons = await _handle_single_crl(
                 cert=cert,
                 cert_issuer_auth=cert_issuer_auth,
-                certificate_list_cont=certificate_list_cont,
+                certificate_list_cont=crl_container,
                 path=path,
                 validation_context=validation_context,
-                delta_lists_by_issuer=delta_lists_by_issuer,
+                delta_lists_by_issuer=deltas,
                 use_deltas=use_deltas,
                 errs=errs,
                 proc_state=proc_state,
             )
             if interim_reasons is not None:
                 # Step l
                 checked_reasons |= interim_reasons
+        except CRLValidationIndeterminateError as e:
+            errs.append(e.msg, certificate_list_cont)
         except ValueError as e:
             msg = "Generic processing error while validating CRL."
             logging.debug(msg, exc_info=e)
             errs.append(msg, certificate_list_cont)
 
+    for certificate_list_cont in crls_to_process:
+        await _process(certificate_list_cont, delta_lists_by_issuer)
+
+    exc = _process_crl_completeness(
+        checked_reasons, total_crls, errs, proc_state
+    )
+    if exc is None:
+        return
+    elif not revinfo_manager.fetching_allowed:
+        raise exc
+
+    # If we're not done checking CRLs, but we are allowed to fetch more,
+    #  let's go download some more CRLs...
+
+    # TODO scan Freshest CRL extensions for delta CRLs?
+
+    extra_certificate_lists = await revinfo_manager.fetch_crls(cert)
+    (
+        extra_complete_lists_by_issuer,
+        extra_delta_lists_by_issuer,
+    ) = await _classify_relevant_crls(
+        extra_certificate_lists, poe_manager, errs
+    )
+
+    combined_deltas = {
+        k: delta_lists_by_issuer.get(k, [])
+        + extra_delta_lists_by_issuer.get(k, [])
+        for k in set(delta_lists_by_issuer.keys()).union(
+            extra_delta_lists_by_issuer.keys()
+        )
+    }
+
+    crls_to_process = []
+    for issuer, issuer_crls in complete_lists_by_issuer.items():
+        # some of the new deltas might complement CRLs that we already had
+        if issuer in extra_delta_lists_by_issuer:
+            crls_to_process.extend(issuer_crls)
+    for issuer_crls in extra_complete_lists_by_issuer.values():
+        crls_to_process.extend(issuer_crls)
+
+    for certificate_list_cont in crls_to_process:
+        await _process(certificate_list_cont, combined_deltas)
+
+    total_crls += len(crls_to_process)
+
     exc = _process_crl_completeness(
         checked_reasons, total_crls, errs, proc_state
     )
@@ -1217,15 +1271,18 @@ async def _assess_crl_relevance(
         if interim_reasons is None:
             continue
 
+        delta = None
         if use_deltas:
-            delta = _maybe_get_delta_crl(
-                certificate_list=certificate_list,
-                crl_issuer=putative_issuer,
-                delta_lists_by_issuer=delta_lists_by_issuer,
-                errs=errs,
-            )
-        else:
-            delta = None
+            try:
+                delta = _maybe_get_delta_crl(
+                    certificate_list=certificate_list,
+                    crl_issuer=putative_issuer,
+                    delta_lists_by_issuer=delta_lists_by_issuer,
+                    errs=errs,
+                )
+            except CRLValidationIndeterminateError as e:
+                errs.append(e.msg, certificate_list)
+                continue
 
         prov = ProvisionalCRLTrust(path=cand_path, delta=delta)
         provisional_results.append(prov)
@@ -1271,8 +1328,12 @@ async def collect_relevant_crls_with_paths(
 
     proc_state = proc_state or ValProcState(cert_path_stack=ConsList.sing(path))
     errs = _CRLErrs()
+    candidate_crls = revinfo_manager.currently_available_crls()
     classify_job = _classify_relevant_crls(
-        revinfo_manager, cert, errs, control_time=control_time
+        candidate_crls,
+        revinfo_manager.poe_manager,
+        errs,
+        control_time=control_time,
     )
     complete_lists_by_issuer, delta_lists_by_issuer = await classify_job