Minor refactoring of Fulcio SCT handling (#1259)

Co-authored-by: William Woodruff <[email protected]>
sigstore · Jan 6, 2025 · 8b4b3fe · 8b4b3fe
1 parent 52846d9
commit 8b4b3fe
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 53 deletions.
diff --git a/sigstore/_internal/fulcio/client.py b/sigstore/_internal/fulcio/client.py
@@ -33,13 +33,8 @@
     CertificateSigningRequest,
     load_pem_x509_certificate,
 )
-from cryptography.x509.certificate_transparency import SignedCertificateTimestamp
 
 from sigstore._internal import USER_AGENT
-from sigstore._internal.sct import (
-    UnexpectedSctCountException,
-    _get_precertificate_signed_certificate_timestamps,
-)
 from sigstore._utils import B64Str
 from sigstore.oidc import IdentityToken
 
@@ -51,14 +46,6 @@
 TRUST_BUNDLE_ENDPOINT = "/api/v2/trustBundle"
 
 
-class FulcioSCTError(Exception):
-    """
-    Raised on errors when constructing a `FulcioSignedCertificateTimestamp`.
-    """
-
-    pass
-
-
 class ExpiredCertificate(Exception):
     """An error raised when the Certificate is expired."""
 
@@ -69,7 +56,6 @@ class FulcioCertificateSigningResponse:
 
     cert: Certificate
     chain: List[Certificate]
-    sct: SignedCertificateTimestamp
 
 
 @dataclass(frozen=True)
@@ -148,14 +134,7 @@ def post(
         cert = load_pem_x509_certificate(certificates[0].encode())
         chain = [load_pem_x509_certificate(c.encode()) for c in certificates[1:]]
 
-        try:
-            # The SignedCertificateTimestamp should be accessed by the index 0
-            sct = _get_precertificate_signed_certificate_timestamps(cert)[0]
-
-        except UnexpectedSctCountException as ex:
-            raise FulcioClientError(ex)
-
-        return FulcioCertificateSigningResponse(cert, chain, sct)
+        return FulcioCertificateSigningResponse(cert, chain)
 
 
 class FulcioTrustBundle(_Endpoint):

diff --git a/sigstore/_internal/sct.py b/sigstore/_internal/sct.py
@@ -148,32 +148,28 @@ def _get_issuer_cert(chain: List[Certificate]) -> Certificate:
     return issuer
 
 
-class UnexpectedSctCountException(Exception):
-    """
-    Number of percerts scts is wrong
-    """
-
-    pass
-
-
-def _get_precertificate_signed_certificate_timestamps(
+def _get_signed_certificate_timestamp(
     certificate: Certificate,
-) -> PrecertificateSignedCertificateTimestamps:
-    # Try to retrieve the embedded SCTs within the cert.
+) -> SignedCertificateTimestamp:
+    """Retrieve the embedded SCT from the certificate.
+
+    Raise VerificationError if certificate does not contain exactly one SCT
+    """
     try:
-        precert_scts_extension = certificate.extensions.get_extension_for_class(
+        timestamps = certificate.extensions.get_extension_for_class(
             PrecertificateSignedCertificateTimestamps
         ).value
     except ExtensionNotFound:
-        raise ValueError(
-            "No PrecertificateSignedCertificateTimestamps found for the certificate"
+        raise VerificationError(
+            "Certificate does not contain a signed certificate timestamp extension"
         )
 
-    if len(precert_scts_extension) != 1:
-        raise UnexpectedSctCountException(
-            f"Unexpected embedded SCT count in response: {len(precert_scts_extension)} != 1"
+    if len(timestamps) != 1:
+        raise VerificationError(
+            f"Expected one certificate timestamp, found {len(timestamps)}"
         )
-    return precert_scts_extension
+    sct: SignedCertificateTimestamp = timestamps[0]
+    return sct
 
 
 def _cert_is_ca(cert: Certificate) -> bool:
@@ -187,7 +183,6 @@ def _cert_is_ca(cert: Certificate) -> bool:
 
 
 def verify_sct(
-    sct: SignedCertificateTimestamp,
     cert: Certificate,
     chain: List[Certificate],
     ct_keyring: CTKeyring,
@@ -201,6 +196,8 @@ def verify_sct(
     log to sign SCTs).
     """
 
+    sct = _get_signed_certificate_timestamp(cert)
+
     issuer_key_id = None
     if sct.entry_type == LogEntryType.PRE_CERTIFICATE:
         # If we're verifying an SCT for a precertificate, we need to

diff --git a/sigstore/sign.py b/sigstore/sign.py
@@ -161,21 +161,15 @@ def _signing_cert(
                 certificate_request, self._identity_token
             )
 
-            # Verify the SCT
-            sct = certificate_response.sct
-            cert = certificate_response.cert
-            chain = certificate_response.chain
-
             verify_sct(
-                sct,
-                cert,
-                chain,
+                certificate_response.cert,
+                certificate_response.chain,
                 self._signing_ctx._trusted_root.ct_keyring(KeyringPurpose.SIGN),
             )
 
             _logger.debug("Successfully verified SCT...")
 
-            return cert
+            return certificate_response.cert
 
     def _finalize_sign(
         self,

diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py
@@ -43,7 +43,6 @@
 from sigstore._internal.rekor import _hashedrekord_from_parts
 from sigstore._internal.rekor.client import RekorClient
 from sigstore._internal.sct import (
-    _get_precertificate_signed_certificate_timestamps,
     verify_sct,
 )
 from sigstore._internal.timestamp import TimestampSource, TimestampVerificationResult
@@ -339,10 +338,8 @@ def _verify_common_signing_cert(
             chain = self._verify_chain_at_time(cert_ossl, vts)
 
         # (2): verify the signing certificate's SCT.
-        sct = _get_precertificate_signed_certificate_timestamps(cert)[0]
         try:
             verify_sct(
-                sct,
                 cert,
                 [parent_cert.to_cryptography() for parent_cert in chain],
                 self._trusted_root.ct_keyring(KeyringPurpose.VERIFY),