Correctly handle the lifetime signing EKU

docs/changelog.rst

@@ -6,7 +6,7 @@ v0.7.0 (unreleased)
 -------------------
 * Remove dependency of ``pyasn1`` and ``pyasn1-modules`` entirely to provide more robust
   parsing of ASN.1 structures, adding the ability to parse structures independent of
-  RFC version. Certain bugs bugs we've encountered in the past, have now been resolved
+  RFC version. Certain bugs we've encountered in the past, have now been resolved
   as a result of this. On top of that, structures defined in the replacement,
   ``asn1crypto``, are a lot more Pythonic, and parsing speed has been sliced in more
   than half.
@@ -37,6 +37,12 @@ v0.7.0 (unreleased)
 * Add support for ``SignerInfo`` versions other than v1
 * Fix bug that could cause out-of-bound reads during parsing of the PE file's
   certificate table
+* Correctly handle the lifetime-signing EKU (OID 1.3.6.1.4.1.311.10.3.13) by ignoring
+  the countersignature's timestamp during verification of the certification chain when
+  this is set on the end-entity's certificate. Note that the private
+  ``SignerInfo._verify_issuer`` has slightly changed semantics based on this.
+* Return the certificate chain(s) in ``AuthenticodeSignedData.verify`` and
+  the used ``AuthenticodeSignedData`` and chains in ``SignedPEFile.verify``
 
 * Parse the ``SpcPeImageData`` as part of the SpcInfo. This adds the attributes
   ``image_flags`` and ``image_publisher``, although this information is never used.

signify/authenticode/signed_pe.py

@@ -45,6 +45,7 @@
 from signify.asn1.hashing import ACCEPTED_DIGEST_ALGORITHMS
 from signify.authenticode import structures
 from signify.exceptions import AuthenticodeNotSignedError, SignedPEParseError
+from signify.x509 import Certificate
 
 logger = logging.getLogger(__name__)
 
@@ -365,7 +366,7 @@ def verify(
         expected_hashes: dict[str, bytes] | None = None,
         ignore_parse_errors: bool = True,
         **kwargs: Any,
-    ) -> bool:
+    ) -> list[tuple[structures.AuthenticodeSignedData, Iterable[list[Certificate]]]]:
         """Verifies the SignedData structures. This is a little bit more efficient than
         calling all verify-methods separately.
 
@@ -398,6 +399,8 @@ def verify(
             When :const:`False`, this will raise the :exc:`SignedPEParseError` as soon
             as one occurs. This often occurs before :exc:`AuthenticodeNotSignedError`
             is potentially raised.
+        :return: the used structure(s) in validation, as a list of tuples, in the form
+            (signed data object, certificate chain)
         :raises AuthenticodeVerificationError: when the verification failed
         :raises SignedPEParseError: for parse errors in the PEFile
         """
@@ -430,13 +433,21 @@ def verify(
         expected_hashes = self._calculate_expected_hashes(signed_datas, expected_hashes)
 
         # Now iterate over all SignedDatas
+        chains = []
         last_error = None
         assert signed_datas
         for signed_data in signed_datas:
             try:
-                signed_data.verify(
-                    expected_hash=expected_hashes[signed_data.digest_algorithm().name],
-                    **kwargs,
+                chains.append(
+                    (
+                        signed_data,
+                        signed_data.verify(
+                            expected_hash=expected_hashes[
+                                signed_data.digest_algorithm().name
+                            ],
+                            **kwargs,
+                        ),
+                    )
                 )
             except Exception as e:  # noqa: PERF203
                 # best and any are interpreted as any; first doesn't matter either way,
@@ -446,9 +457,10 @@ def verify(
                 last_error = e
             else:
                 if multi_verify_mode not in ("all", "first"):
-                    return True
+                    # only return the last one, as we are in mode any/best
+                    return chains[-1:]
         if last_error is None:
-            return True
+            return chains
         raise last_error
 
     def explain_verify(

signify/authenticode/structures.py

@@ -275,6 +275,22 @@ def _parse(self) -> None:
 
             self.countersigner = RFC3161SignedData(ts_data["content"])
 
+    def _verify_issuer(
+        self,
+        issuer: Certificate,
+        context: VerificationContext,
+        signing_time: datetime.datetime | None = None,
+    ) -> list[Certificate]:
+        """Check whether the lifetime signing EKU is set. if that is the case, we can
+        only use the timestamp for revocation checking, not for extending the lifetime
+        of the signature. Revocation checking currently does not work.
+        """
+        if "microsoft_lifetime_signing" in issuer.extensions.get(
+            "extended_key_usage", []
+        ):
+            signing_time = None
+        return super()._verify_issuer(issuer, context, signing_time)
+
 
 class SpcInfo:
     """The Authenticode's SpcIndirectDataContent information, and their children. This
@@ -417,7 +433,7 @@ def verify(
         trusted_certificate_store: CertificateStore = TRUSTED_CERTIFICATE_STORE,
         verification_context_kwargs: dict[str, Any] | None = None,
         countersignature_mode: Literal["strict", "permit", "ignore"] = "strict",
-    ) -> None:
+    ) -> Iterable[list[Certificate]]:
         """Verifies the SignedData structure:
 
         * Verifies that the digest algorithms match across the structure
@@ -468,7 +484,7 @@ def verify(
             checked, but when it errors, it is verified as if the countersignature was
             never set. When set to 'ignore', countersignatures are never checked.
         :raises AuthenticodeVerificationError: when the verification failed
-        :return: :const:`None`
+        :return: A list of valid certificate chains for this SignedData.
         """
 
         if verification_context_kwargs is None:
@@ -532,6 +548,7 @@ def verify(
         # Can't check authAttr hash against encrypted hash, done implicitly in
         # M2's pubkey.verify.
 
+        signing_time = None
         if self.signer_info.countersigner and countersignature_mode != "ignore":
             assert cs_verification_context is not None
 
@@ -565,11 +582,9 @@ def verify(
             else:
                 # If no errors occur, we should be fine setting the timestamp to the
                 # countersignature's timestamp
-                verification_context.timestamp = (
-                    self.signer_info.countersigner.signing_time
-                )
+                signing_time = self.signer_info.countersigner.signing_time
 
-        self.signer_info.verify(verification_context)
+        return self.signer_info.verify(verification_context, signing_time)
 
     def explain_verify(
         self, *args: Any, **kwargs: Any

signify/pkcs7/signerinfo.py

@@ -268,16 +268,17 @@ def _encode_attributes(cls, data: cms.CMSAttributes) -> bytes:
         new_attrs = type(data)(contents=data.contents)
         return cast(bytes, new_attrs.dump())
 
-    def _verify_issuer(self, issuer: Certificate, context: VerificationContext) -> None:
-        """Verifies whether the given issuer is valid for the given context. Similar to
-        :meth:`Certificate._verify_issuer`. Does not support legacy verification method.
+    def _verify_issuer_signature(
+        self, issuer: Certificate, context: VerificationContext
+    ) -> None:
+        """Check the issuer signature against the information in the class. Use
+        :meth:`_verify_issuer` for full verification.
 
         :param Certificate issuer: The Certificate to verify
-        :param VerificationContext context: The
+        :param VerificationContext context: The context for verification
+        :raises SignerInfoVerificationError: If the issuer signature is invalid
         """
 
-        issuer.verify(context)
-
         try:
             issuer.verify_signature(
                 self.encrypted_digest,
@@ -291,9 +292,31 @@ def _verify_issuer(self, issuer: Certificate, context: VerificationContext) -> N
                 f" attributes in {type(self).__name__}: {e}"
             )
 
+    def _verify_issuer(
+        self,
+        issuer: Certificate,
+        context: VerificationContext,
+        signing_time: datetime.datetime | None = None,
+    ) -> list[Certificate]:
+        """Verifies whether the given issuer is valid for this :class:`SignerInfo`,
+        and valid in the given context. Similar to :meth:`Certificate._verify_issuer`.
+
+        It adds the ``signing_time`` to the context if necessary.
+        """
+
+        # _verify_issuer_signature may fail when it is not a valid issuer for
+        # this SignedInfo
+        self._verify_issuer_signature(issuer, context)
+
+        if signing_time is not None:
+            context.timestamp = signing_time
+        return context.verify(issuer)
+
     def _build_chain(
-        self, context: VerificationContext
-    ) -> Iterable[Iterable[Certificate]]:
+        self,
+        context: VerificationContext,
+        signing_time: datetime.datetime | None = None,
+    ) -> Iterable[list[Certificate]]:
         """Given a context, builds a chain up to a trusted certificate. This is a
         generator function, generating all valid chains.
 
@@ -303,6 +326,7 @@ def _build_chain(
         :param VerificationContext context: The context for building the chain. Most
             importantly, contains all certificates to build the chain from, but also
             their properties are relevant.
+        :param signing_time: The time to be used as timestamp when creating the chain
         :return: Iterable of all of the valid chains from this SignedInfo up to and
             including a trusted anchor. Note that this may be an empty iteration if no
             candidate parent certificate was found.
@@ -323,12 +347,8 @@ def _build_chain(
             issuer=self.issuer, serial_number=self.serial_number
         ):
             try:
-                # _verify_issuer may fail when it is not a valid issuer for this
-                # SignedInfo
-                self._verify_issuer(issuer, context)
-
-                # _build_chain may fail when anywhere up its chain an error occurs
-                yield context.verify(issuer)
+                # may fail when anywhere up its chain an error occurs
+                yield self._verify_issuer(issuer, context, signing_time)
             except VerificationError as e:  # noqa: PERF203
                 if first_error is None:
                     first_error = e
@@ -338,18 +358,23 @@ def _build_chain(
         if first_error:
             raise first_error
 
-    def verify(self, context: VerificationContext) -> Iterable[Iterable[Certificate]]:
+    def verify(
+        self,
+        context: VerificationContext,
+        signing_time: datetime.datetime | None = None,
+    ) -> Iterable[list[Certificate]]:
         """Verifies that this :class:`SignerInfo` verifies up to a chain with the root
         of a trusted certificate.
 
         :param VerificationContext context: The context for verifying the SignerInfo.
+        :param signing_time: The time to be used as timestamp when creating the chain
         :return: A list of valid certificate chains for this SignerInfo.
         :rtype: Iterable[Iterable[Certificate]]
         :raises AuthenticodeVerificationError: When the SignerInfo could not be
             verified.
         """
 
-        chains = list(self._build_chain(context))
+        chains = list(self._build_chain(context, signing_time))
 
         if not chains:
             raise SignerInfoVerificationError(
@@ -361,7 +386,7 @@ def verify(self, context: VerificationContext) -> Iterable[Iterable[Certificate]
 
     def potential_chains(
         self, context: VerificationContext
-    ) -> Iterable[Iterable[Certificate]]:
+    ) -> Iterable[list[Certificate]]:
         """Retrieves all potential chains from this SignerInfo instance.
 
         :param VerificationContext context: The context

signify/x509/context.py

@@ -310,7 +310,7 @@ def potential_chains(
                 else:
                     yield [*chain, certificate]
 
-    def verify(self, certificate: Certificate) -> Iterable[Certificate]:
+    def verify(self, certificate: Certificate) -> list[Certificate]:
         """Verifies the certificate, and its chain.
 
         :param Certificate certificate: The certificate to verify

tests/test_authenticode.py

@@ -33,12 +33,13 @@
 )
 from signify.authenticode.signed_pe import SignedPEFile
 from signify.exceptions import (
+    AuthenticodeNotSignedError,
     AuthenticodeVerificationError,
     SignedPEParseError,
     VerificationError,
-    AuthenticodeNotSignedError,
 )
 from signify.fingerprinter import AuthenticodeFingerprinter
+from signify.x509 import Certificate
 from signify.x509.context import (
     CertificateStore,
     FileSystemCertificateStore,
@@ -340,6 +341,45 @@ def test_signerinfo_v0(self):
             pefile = SignedPEFile(f)
             pefile.verify()
 
+    def test_lifetime_signing(self):
+        """this tests a sample that has a valid countersignature and a lifetime signing
+        flag. it is self-signed, so we need to load that one as well
+        """
+        certificate_store = CertificateStore(
+            list(TRUSTED_CERTIFICATE_STORE_NO_CTL)
+            + list(
+                Certificate.from_pems(
+                    (root_dir / "test_data" / "kdbazis.dll.crt").read_bytes()
+                )
+            ),
+            trusted=True,
+        )
+
+        with open(
+            str(root_dir / "test_data" / "kdbazis.dll"),
+            "rb",
+        ) as f:
+            pefile = SignedPEFile(f)
+            # should verify on this date
+            pefile.verify(
+                trusted_certificate_store=certificate_store,
+                verification_context_kwargs={
+                    "timestamp": datetime.datetime(
+                        2024, 1, 1, tzinfo=datetime.timezone.utc
+                    )
+                },
+            )
+            # should not verify on this date
+            with self.assertRaises(VerificationError):
+                pefile.verify(
+                    trusted_certificate_store=certificate_store,
+                    verification_context_kwargs={
+                        "timestamp": datetime.datetime(
+                            2060, 1, 1, tzinfo=datetime.timezone.utc
+                        )
+                    },
+                )
+
 
 class CertificateTestCase(unittest.TestCase):
     def test_all_trusted_certificates_are_trusted(self):

tests/test_data/kdbazis.dll.crt

@@ -0,0 +1,30 @@
+-----BEGIN CERTIFICATE-----
+MIIFJzCCAw+gAwIBAgIQNbHe1o7U14hPfhG6QHfQ0jANBgkqhkiG9w0BAQsFADAd
+MRswGQYDVQQDExJWaXJ0dWFsS0QtUmVkdXggQ0EwHhcNMjAxMTA3MTUyMDA2WhcN
+MzkxMjMxMjM1OTU5WjAdMRswGQYDVQQDExJWaXJ0dWFsS0QtUmVkdXggQ0EwggIi
+MA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQCw4HhzBkL9X8i8mIa5Xol9L2Il
+T289kTPS91Im0iJuMCdJanzCqPj/WjiVqL0ptjun0GNnPu3N26cImD+bdO/xEr0/
+ESnYzR6xgqbNEEL7mw9Jw6iIV91XBSguOCu8Rhibrcddh85qvcwDiUQjANeMxPd8
+dUNajJaDAg8bdu2EKXQkO6VAHf6J1RvBJW2jjNu8R6Tjyj+HdkS4zedD5t/aJ8wI
+TPasW843xqPOzUknzzYK2MW+IeXzmWU82T4QvNpJQnFF7ChB+y9x0IxfklddBWUN
+ZTNo2Dc4i3VSB41jxfmF96Hy8lT7TgKB+ab8hjHT0japbSOMgFh/jsjUIOo/uglW
+9EIbWPh0gL2kdGs8qPBPRO20vXrOnjbeh8s83Wd6GLxTTXTwuMaOY8A47zkCZlWE
+f0rf4kix7MNVXNvsNvOjT0FUQiE/OhhdFKRgViaQsKIcQtKVyL9Osr5hygOtLvkx
+/17nd7+NaZkzPIIagUd1YJN9JhlCs7KAuyGeWcsFKKDNxGScJrLrZvick8eL10/6
+GR4lJfUKSIx+1+MT6hvB5incWLhaKKcUF2vPib+x7TCabtZMymq0IdHdpiKcPzlZ
+ZnTQQAKHW1HYr6o+sNJRR8QKd2T1JnGLQ0Vg/I0/OJnIu5HkFrtbpuFKVKnG5LeR
+3b7VmxjPGev2fkIm7QIDAQABo2MwYTAPBgNVHRMBAf8EBTADAQH/ME4GA1UdAQRH
+MEWAEIXZMI6puXhWlnEv8ouzxC+hHzAdMRswGQYDVQQDExJWaXJ0dWFsS0QtUmVk
+dXggQ0GCEDWx3taO1NeIT34RukB30NIwDQYJKoZIhvcNAQELBQADggIBAHUC3v/L
+YIFpT4MISPMmMnLMVv8ra53Qvt2Y3ykqCcq3SO7jDAseMk27oNzdRW7hsP66BfO5
+uYLAyjAsgw+wta2h5L6jWlvPZQFEXgKhzd4cPnFZnQs4KxaUabW2HIrNHnJkgmss
+tNmT74YrwTHbg5oI/+RVf+24VLunmnngqmTS37R3bGMZtrmi0lTkTIQ1NrWASoAG
+zZTKQQiF1dUvDkC3uNQfgJ2b/sYVohUIJiVncLWnXA30ElX6wlhuZNnr72dCzbD/
++23uCWHY2h1FQvVzxFo+tInsz0ijjXAOAwYtjQm1jw7QwbWIQVSvARAMsD0coM9V
+LgZ1G+hdOeQjt/h3CJ1I7jWDaDY9ssgVqEAEr8eYHkUvmJBzdX07tutytCT+aFQP
+/D8Fsj/5KHw5f0zwEoUJjzgrIksaMS048ERifSkrE1UmQCbcmDW8VJM+p6hUC5/z
+bkBPHJrDcauc64f5SmIPqeP3fPQYykvI1DrVa31JmP8yITWEvzETetA21BHk3ArR
+PqRHYepdLPkpBfwiW5nHUln118aajp4B1npMGli2rSpXW0sTVjLQQaqVO5YsW8ea
+pG3p7LV39NWVJ8BjCQCTEXnkgn7PW6n3QV5kNz2vHE4PC4AhqUN1CXV6B0k3X2tR
+wocCMeN+1tepOsR9Ut/zIf2ISb8AF+NH3/3Z
+-----END CERTIFICATE-----

tests/test_fingerprinter.py

@@ -22,6 +22,7 @@
 import hashlib
 import io
 import json
+import os.path
 import pathlib
 import unittest
 
@@ -40,7 +41,9 @@ class FingerPrinterTestCase(unittest.TestCase):
 
     def test_entire_blobs(self):
         for filename in (root_dir / "test_data").iterdir():
-            if str(filename).endswith(".res") or str(filename).endswith("README.rst"):
+            if str(filename).endswith(".res") or not os.path.exists(
+                str(filename) + ".res"
+            ):
                 continue
             with self.subTest(filename):
                 with open(str(filename), "rb") as file_obj: