From dddbf4ef01bf5e584781161ce23f817c20f6d9ad Mon Sep 17 00:00:00 2001
From: Andrew Pan <3821575+tnytown@users.noreply.github.com>
Date: Fri, 17 Mar 2023 02:01:02 -0500
Subject: [PATCH] sct, keyring: specialize errors (#555)

* sct, keyring: specialize errors

Signed-off-by: Andrew Pan <a@tny.town>

* CHANGELOG: blurb for SCT changes

Signed-off-by: Andrew Pan <a@tny.town>

* CHANGELOG: add ref to PR

Signed-off-by: Andrew Pan <a@tny.town>

* Apply suggestions from code review

Signed-off-by: Andrew Pan <a@tny.town>

---------

Signed-off-by: Andrew Pan <a@tny.town>
---
 CHANGELOG.md                  |  2 +
 sigstore/_internal/keyring.py |  8 +++-
 sigstore/_internal/sct.py     | 69 ++++++++++++++++++++++++++++-------
 3 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6f223ee5..175b7f46 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,8 @@ All versions prior to 0.9.0 are untracked.
   ([#535](https://github.com/sigstore/sigstore-python/pull/535))
 * Revamped error diagnostics reporting. All errors with diagnostics now implement
   `sigstore.errors.Error`.
+* Improved diagnostics around Signed Certificate Timestamp verification failures.
+  ([#555](https://github.com/sigstore/sigstore-python/pull/555))
 
 ### Fixed
 
diff --git a/sigstore/_internal/keyring.py b/sigstore/_internal/keyring.py
index 4f006b28..0a5a38ec 100644
--- a/sigstore/_internal/keyring.py
+++ b/sigstore/_internal/keyring.py
@@ -44,6 +44,12 @@ class KeyringLookupError(KeyringError):
     pass
 
 
+class KeyringSignatureError(KeyringError):
+    """
+    Raised when `Keyring.verify()` is passed an invalid signature.
+    """
+
+
 class Keyring:
     """
     Represents a set of CT signing keys, each of which is a potentially
@@ -101,4 +107,4 @@ def verify(self, *, key_id: KeyID, signature: bytes, data: bytes) -> None:
                 # NOTE(ww): Unreachable without API misuse.
                 raise KeyringError(f"unsupported key type: {key}")
         except InvalidSignature as exc:
-            raise KeyringError("invalid signature") from exc
+            raise KeyringSignatureError("invalid signature") from exc
diff --git a/sigstore/_internal/sct.py b/sigstore/_internal/sct.py
index 84e4487d..8fbee283 100644
--- a/sigstore/_internal/sct.py
+++ b/sigstore/_internal/sct.py
@@ -32,7 +32,11 @@
 from cryptography.x509.oid import ExtendedKeyUsageOID
 
 from sigstore._internal.ctfe import CTKeyring
-from sigstore._internal.keyring import KeyringError, KeyringLookupError
+from sigstore._internal.keyring import (
+    KeyringError,
+    KeyringLookupError,
+    KeyringSignatureError,
+)
 from sigstore._utils import DERCert, KeyID, key_id
 from sigstore.errors import Error
 
@@ -142,13 +146,34 @@ class InvalidSCTError(Error):
 
     def diagnostics(self) -> str:
         """Returns diagnostics for the error."""
-        # We specialize this error case, since it usually indicates one of
-        # two conditions: either the current sigstore client is out-of-date,
-        # or that the SCT is well-formed but invalid for the current configuration
-        # (indicating that the user has asked for the wrong instance).
-        if isinstance(self.__cause__, KeyringLookupError):
-            return dedent(
-                f"""
+
+        ctx = f"\nContext: {self.__context__}" if self.__context__ else ""
+        return dedent(
+            f"""
+            SCT verification failed.
+
+            Additional context:
+
+            Message: {str(self)}
+            """
+            + ctx
+        )
+
+
+class InvalidSCTKeyError(InvalidSCTError):
+    """
+    Raised during SCT verification if the SCT can't be validated against the given keyring.
+
+    We specialize this error case, since it usually indicates one of
+    two conditions: either the current sigstore client is out-of-date,
+    or that the SCT is well-formed but invalid for the current configuration
+    (indicating that the user has asked for the wrong instance).
+    """
+
+    def diagnostics(self) -> str:
+        """Returns diagnostics for the error."""
+        return dedent(
+            f"""
                 Invalid key ID in SCT: not found in current keyring.
 
                 This may be a result of an outdated `sigstore` installation.
@@ -161,9 +186,27 @@ def diagnostics(self) -> str:
 
                 {self.__cause__}
                 """
-            )
+        )
 
-        return str(self)
+
+class SCTSignatureError(InvalidSCTError):
+    """
+    Raised during SCT verification if the signature of the SCT is invalid.
+    """
+
+    def diagnostics(self) -> str:
+        """Returns diagnostics for the error."""
+        return dedent(
+            f"""
+            Invalid signature on SCT.
+
+            If validating a certificate, the certificate associated with this SCT should not be trusted.
+
+            Additional context:
+
+            {self.__cause__}
+            """
+        )
 
 
 def verify_sct(
@@ -214,8 +257,8 @@ def verify_sct(
             key_id=KeyID(sct.log_id), signature=sct.signature, data=digitally_signed
         )
     except KeyringLookupError as exc:
-        raise InvalidSCTError(
-            "Invalid key ID in SCT: not found in current keyring"
-        ) from exc
+        raise InvalidSCTKeyError from exc
+    except KeyringSignatureError as exc:
+        raise SCTSignatureError from exc
     except KeyringError as exc:
         raise InvalidSCTError from exc