From 602126f6ae2ba43e7d8f0d77d03516a1b283df2c Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 13 Dec 2024 11:33:30 +0200 Subject: [PATCH 1/2] fulcio: Small refactor of how scts are extracted Functionality should not change: Except a few more errors are handled. The sct module API changes but it is internal. * Remove one unnecessary internal exception and one unused exception * We only support a single SCT: simplify the sct module API * Make sure all errors raised in sct.get_signed_certificate_timestamp() are actually handled (by only raising ValueError and handling that) * Handle SCT parsing errors during verify Signed-off-by: Jussi Kukkonen --- sigstore/_internal/fulcio/client.py | 19 +++---------------- sigstore/_internal/sct.py | 29 ++++++++++++----------------- sigstore/verify/verifier.py | 7 +++---- 3 files changed, 18 insertions(+), 37 deletions(-) diff --git a/sigstore/_internal/fulcio/client.py b/sigstore/_internal/fulcio/client.py index 062519186..ea7d9752c 100644 --- a/sigstore/_internal/fulcio/client.py +++ b/sigstore/_internal/fulcio/client.py @@ -36,10 +36,7 @@ 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._internal.sct import get_signed_certificate_timestamp from sigstore._utils import B64Str from sigstore.oidc import IdentityToken @@ -51,14 +48,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.""" @@ -149,10 +138,8 @@ def post( 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: + sct = get_signed_certificate_timestamp(cert) + except ValueError as ex: raise FulcioClientError(ex) return FulcioCertificateSigningResponse(cert, chain, sct) diff --git a/sigstore/_internal/sct.py b/sigstore/_internal/sct.py index 8fd6cc5b2..c146de6c9 100644 --- a/sigstore/_internal/sct.py +++ b/sigstore/_internal/sct.py @@ -148,32 +148,27 @@ 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 ValueError 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" + "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 ValueError( + f"Expected one certificate timestamp, found {len(timestamps)}." ) - return precert_scts_extension + return timestamps[0] def _cert_is_ca(cert: Certificate) -> bool: diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py index 59437caf1..bec76b647 100644 --- a/sigstore/verify/verifier.py +++ b/sigstore/verify/verifier.py @@ -43,7 +43,7 @@ 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, + get_signed_certificate_timestamp, verify_sct, ) from sigstore._internal.timestamp import TimestampSource, TimestampVerificationResult @@ -339,15 +339,14 @@ 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, + get_signed_certificate_timestamp(cert), cert, [parent_cert.to_cryptography() for parent_cert in chain], self._trusted_root.ct_keyring(KeyringPurpose.VERIFY), ) - except VerificationError as e: + except (ValueError, VerificationError) as e: raise VerificationError(f"failed to verify SCT on signing certificate: {e}") # (3): verify the signing certificate against the Sigstore From 41b86091416d205465432046b6800db90f8d54d4 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 13 Dec 2024 12:31:27 +0200 Subject: [PATCH 2/2] Remove sct from FulcioCertificateSigningResponse Simplify FulcioCertificateSigningResponse by removing sct field from it. This changes the (internal) fulcio module API. The only place that needs the SCT is verify_sct() and it already gets the certificate which it can trivially get the SCT from. This makes get_signed_certificate_timestamp() an internal implementation detail of verify_sct() which is nice. FulcioSigningCert.post() changes in that it could now return a response without an SCT: this is ok as it is immediately verified in the callsite in Signer._signing_cert(). Signed-off-by: Jussi Kukkonen --- sigstore/_internal/fulcio/client.py | 10 +--------- sigstore/_internal/sct.py | 14 ++++++++------ sigstore/sign.py | 12 +++--------- sigstore/verify/verifier.py | 4 +--- 4 files changed, 13 insertions(+), 27 deletions(-) diff --git a/sigstore/_internal/fulcio/client.py b/sigstore/_internal/fulcio/client.py index ea7d9752c..5fe6f0674 100644 --- a/sigstore/_internal/fulcio/client.py +++ b/sigstore/_internal/fulcio/client.py @@ -33,10 +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 get_signed_certificate_timestamp from sigstore._utils import B64Str from sigstore.oidc import IdentityToken @@ -58,7 +56,6 @@ class FulcioCertificateSigningResponse: cert: Certificate chain: List[Certificate] - sct: SignedCertificateTimestamp @dataclass(frozen=True) @@ -137,12 +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: - sct = get_signed_certificate_timestamp(cert) - except ValueError 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 index c146de6c9..e2d7c3b20 100644 --- a/sigstore/_internal/sct.py +++ b/sigstore/_internal/sct.py @@ -148,27 +148,28 @@ def _get_issuer_cert(chain: List[Certificate]) -> Certificate: return issuer -def get_signed_certificate_timestamp( +def _get_signed_certificate_timestamp( certificate: Certificate, ) -> SignedCertificateTimestamp: """Retrieve the embedded SCT from the certificate. - Raise ValueError if certificate does not contain exactly one SCT + Raise VerificationError if certificate does not contain exactly one SCT """ try: timestamps = certificate.extensions.get_extension_for_class( PrecertificateSignedCertificateTimestamps ).value except ExtensionNotFound: - raise ValueError( + raise VerificationError( "Certificate does not contain a signed certificate timestamp extension" ) if len(timestamps) != 1: - raise ValueError( + raise VerificationError( f"Expected one certificate timestamp, found {len(timestamps)}." ) - return timestamps[0] + sct: SignedCertificateTimestamp = timestamps[0] + return sct def _cert_is_ca(cert: Certificate) -> bool: @@ -182,7 +183,6 @@ def _cert_is_ca(cert: Certificate) -> bool: def verify_sct( - sct: SignedCertificateTimestamp, cert: Certificate, chain: List[Certificate], ct_keyring: CTKeyring, @@ -196,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 index 6afc7d74c..a87dcdb6e 100644 --- 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 index bec76b647..000e618b8 100644 --- 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_signed_certificate_timestamp, verify_sct, ) from sigstore._internal.timestamp import TimestampSource, TimestampVerificationResult @@ -341,12 +340,11 @@ def _verify_common_signing_cert( # (2): verify the signing certificate's SCT. try: verify_sct( - get_signed_certificate_timestamp(cert), cert, [parent_cert.to_cryptography() for parent_cert in chain], self._trusted_root.ct_keyring(KeyringPurpose.VERIFY), ) - except (ValueError, VerificationError) as e: + except VerificationError as e: raise VerificationError(f"failed to verify SCT on signing certificate: {e}") # (3): verify the signing certificate against the Sigstore