From 0ccb2c8fc6139d8cb97bf22c0ec751a4e786f0f0 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 29 Aug 2023 13:44:58 +0300 Subject: [PATCH 1/7] SigstoreSigner: Update to Sigstore 2.x * detect_credentials() requires no arguments * token is now a IdentityToken instead of str * signing now happens in a SigningContext for the token The token improvement means that we could now also find out the identity and issuer from the token * would make sense to support this as a way to import a key * we can verify the token against the public key in the constructor The latter I intend to implement in a follow up commit, former is likely future work (not hard, just needs some testing to see how it should work). --- pyproject.toml | 2 +- requirements-sigstore.txt | 2 +- securesystemslib/signer/_sigstore_signer.py | 21 +++++++++++++-------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6969bd02..714a284b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,7 +50,7 @@ awskms = ["boto3", "botocore", "cryptography>=40.0.0"] hsm = ["asn1crypto", "cryptography>=40.0.0", "PyKCS11"] pynacl = ["pynacl>1.2.0"] PySPX = ["PySPX>=0.5.0"] -sigstore = ["sigstore==1.1.2"] +sigstore = ["sigstore~=2.0.0"] [tool.hatch.version] path = "securesystemslib/__init__.py" diff --git a/requirements-sigstore.txt b/requirements-sigstore.txt index 6a939765..cfbd8bba 100644 --- a/requirements-sigstore.txt +++ b/requirements-sigstore.txt @@ -1 +1 @@ -sigstore==1.1.2 +sigstore==2.0.0 diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py index 5e6e0a84..77e65832 100644 --- a/securesystemslib/signer/_sigstore_signer.py +++ b/securesystemslib/signer/_sigstore_signer.py @@ -130,11 +130,13 @@ class SigstoreSigner(Signer): SCHEME = "sigstore" - def __init__(self, token: str, public_key: Key): + def __init__(self, token: Any, public_key: Key): # TODO: Vet public key # - signer eligible for keytype/scheme? # - token matches identity/issuer? self.public_key = public_key + # token is of type sigstore.oidc.IdentityToken but the module should be usable + # without sigstore so it's not annotated self._token = token @classmethod @@ -146,7 +148,7 @@ def from_priv_key_uri( ) -> "SigstoreSigner": # pylint: disable=import-outside-toplevel try: - from sigstore.oidc import Issuer, detect_credential + from sigstore.oidc import Issuer, detect_credential, IdentityToken except ImportError as e: raise UnsupportedLibraryError(IMPORT_ERROR) from e @@ -166,9 +168,10 @@ def from_priv_key_uri( issuer = Issuer.production() token = issuer.identity_token() else: - # Note: this method signature only works with sigstore-python 1.1.2: - # dependencies must be updated when changing this - token = detect_credential("sigstore") + credential = detect_credential() + if not credential: + raise RuntimeError("Failed to detect Sigstore credentials") + token = IdentityToken(credential) return cls(token, public_key) @@ -217,12 +220,14 @@ def sign(self, payload: bytes) -> Signature: """ # pylint: disable=import-outside-toplevel try: - from sigstore.sign import Signer as _Signer + from sigstore.sign import SigningContext except ImportError as e: raise UnsupportedLibraryError(IMPORT_ERROR) from e - signer = _Signer.production() - result = signer.sign(io.BytesIO(payload), self._token) + context = SigningContext.production() + with context.signer(self._token) as sigstore_signer: + result = sigstore_signer.sign(io.BytesIO(payload)) + # TODO: Ask upstream if they can make this public bundle = result._to_bundle() # pylint: disable=protected-access From f6da0346d273d7848e7e68112a94decc975e7d36 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 29 Aug 2023 14:24:54 +0300 Subject: [PATCH 2/7] SigstoreSigner: Verify that signer token and key match This is now possible with sigstore 2.x. The verification is in from_priv_key_uri(). It could be in constructor but I was trying to avoid making the optional imports even more complicated... Note that there is a workaround for the fact that we do not know the signing identity at signing time in the ambient case: The signing certificate SAN is not the OIDC identity. --- securesystemslib/signer/_sigstore_signer.py | 22 ++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py index 77e65832..43e110f6 100644 --- a/securesystemslib/signer/_sigstore_signer.py +++ b/securesystemslib/signer/_sigstore_signer.py @@ -131,9 +131,6 @@ class SigstoreSigner(Signer): SCHEME = "sigstore" def __init__(self, token: Any, public_key: Key): - # TODO: Vet public key - # - signer eligible for keytype/scheme? - # - token matches identity/issuer? self.public_key = public_key # token is of type sigstore.oidc.IdentityToken but the module should be usable # without sigstore so it's not annotated @@ -148,7 +145,7 @@ def from_priv_key_uri( ) -> "SigstoreSigner": # pylint: disable=import-outside-toplevel try: - from sigstore.oidc import Issuer, detect_credential, IdentityToken + from sigstore.oidc import IdentityToken, Issuer, detect_credential except ImportError as e: raise UnsupportedLibraryError(IMPORT_ERROR) from e @@ -161,8 +158,9 @@ def from_priv_key_uri( raise ValueError(f"SigstoreSigner does not support {priv_key_uri}") params = dict(parse.parse_qsl(uri.query)) + ambient = params.get("ambient", "true") == "true" - if params.get("ambient") == "false": + if not ambient: # TODO: Restrict oauth flow to use identity/issuer from public_key # TODO: Use secrets_handler for identity_token() secret arg issuer = Issuer.production() @@ -173,6 +171,20 @@ def from_priv_key_uri( raise RuntimeError("Failed to detect Sigstore credentials") token = IdentityToken(credential) + key_identity = public_key.keyval["identity"] + key_issuer = public_key.keyval["issuer"] + if key_issuer != token.expected_certificate_subject: + raise ValueError( + f"Signer identity issuer {token.expected_certificate_subject} " + f"did not match key: {key_issuer}" + ) + # TODO: should check ambient identity too: unfortunately IdentityToken does + # not provide access to the expected identity value (cert SAN) in ambient case + if not ambient and key_identity != token.identity: + raise ValueError( + f"Signer identity {token.identity} did not match key: {key_identity}" + ) + return cls(token, public_key) @classmethod From 95c3c79555b613a3d86c67643e9d15458c38a6fb Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Thu, 31 Aug 2023 12:05:22 +0300 Subject: [PATCH 3/7] SigstoreSigner: lint workaround Cheeky rename of the unused argument: this takes us below the too-many-locals limit. --- securesystemslib/signer/_sigstore_signer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py index 43e110f6..95a0b150 100644 --- a/securesystemslib/signer/_sigstore_signer.py +++ b/securesystemslib/signer/_sigstore_signer.py @@ -141,7 +141,7 @@ def from_priv_key_uri( cls, priv_key_uri: str, public_key: Key, - secrets_handler: Optional[SecretsHandler] = None, + _secrets_handler: Optional[SecretsHandler] = None, ) -> "SigstoreSigner": # pylint: disable=import-outside-toplevel try: From 1434f835b3c99dc2bc836370a4a2c148b8da00a8 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Thu, 7 Sep 2023 14:00:05 +0300 Subject: [PATCH 4/7] Sigstore: Add an import method with no args This way the user has to authenticate to the identity they want to sign with later * removes possibility of typos or misunderstanding * Still allows storing the identity and issuer in the URI (this is not implemented here) --- securesystemslib/signer/_sigstore_signer.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py index 95a0b150..a1e32617 100644 --- a/securesystemslib/signer/_sigstore_signer.py +++ b/securesystemslib/signer/_sigstore_signer.py @@ -215,6 +215,25 @@ def import_( return uri, key + @classmethod + def import_via_auth(cls) -> Tuple[str, SigstoreKey]: + """Create public key and signer URI by interactive authentication + + Returns a private key URI (for Signer.from_priv_key_uri()) and a public + key. This method always uses the interactive authentication. + """ + # pylint: disable=import-outside-toplevel + try: + from sigstore.oidc import Issuer + except ImportError as e: + raise UnsupportedLibraryError(IMPORT_ERROR) from e + + # authenticate to get the identity and issuer + token = Issuer.production().identity_token() + return cls.import_( + token.identity, token.expected_certificate_subject, False + ) + def sign(self, payload: bytes) -> Signature: """Signs payload using the OIDC token on the signer instance. From 4a938c858c5b143c4f700a6025caa6fd1dd3ed9c Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 12 Sep 2023 11:45:58 +0300 Subject: [PATCH 5/7] Sigstore: Use new public SigningResult.to_bundle() --- securesystemslib/signer/_sigstore_signer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py index a1e32617..f65d1bc0 100644 --- a/securesystemslib/signer/_sigstore_signer.py +++ b/securesystemslib/signer/_sigstore_signer.py @@ -259,8 +259,7 @@ def sign(self, payload: bytes) -> Signature: with context.signer(self._token) as sigstore_signer: result = sigstore_signer.sign(io.BytesIO(payload)) - # TODO: Ask upstream if they can make this public - bundle = result._to_bundle() # pylint: disable=protected-access + bundle = result.to_bundle() return Signature( self.public_key.keyid, From c01f957cfe66e7919137896aa26d27688ca5b23d Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 2 Oct 2023 12:35:20 +0300 Subject: [PATCH 6/7] build: Fix sigstore compatible release" versioning "~=2.0.0" matches 2.0.x but we actually want 2.x --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 714a284b..6922602a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,7 +50,7 @@ awskms = ["boto3", "botocore", "cryptography>=40.0.0"] hsm = ["asn1crypto", "cryptography>=40.0.0", "PyKCS11"] pynacl = ["pynacl>1.2.0"] PySPX = ["PySPX>=0.5.0"] -sigstore = ["sigstore~=2.0.0"] +sigstore = ["sigstore~=2.0"] [tool.hatch.version] path = "securesystemslib/__init__.py" From 856aa5727899fcf971c81475faa4fb482892baa9 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 2 Oct 2023 15:34:38 +0300 Subject: [PATCH 7/7] sigstore: Tweak method signature underscore prefix was used to avoid a too-many-locals warning: Workaround that otherwise to make the method signature match the parent class. --- securesystemslib/signer/_sigstore_signer.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py index f65d1bc0..61f03b87 100644 --- a/securesystemslib/signer/_sigstore_signer.py +++ b/securesystemslib/signer/_sigstore_signer.py @@ -141,7 +141,7 @@ def from_priv_key_uri( cls, priv_key_uri: str, public_key: Key, - _secrets_handler: Optional[SecretsHandler] = None, + secrets_handler: Optional[SecretsHandler] = None, ) -> "SigstoreSigner": # pylint: disable=import-outside-toplevel try: @@ -163,8 +163,7 @@ def from_priv_key_uri( if not ambient: # TODO: Restrict oauth flow to use identity/issuer from public_key # TODO: Use secrets_handler for identity_token() secret arg - issuer = Issuer.production() - token = issuer.identity_token() + token = Issuer.production().identity_token() else: credential = detect_credential() if not credential: