From 04a53f714104aed3a830d3d3279b13aae626da85 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 12 Dec 2022 17:30:14 +0200 Subject: [PATCH 01/11] GCPSigner: Implement public key import This is for setting up the signing support for this key: import_() should be called once per key only, and the public key should be stored. --- securesystemslib/signer/_gcp_signer.py | 71 +++++++++++++++++++++++++- tests/check_kms_signers.py | 48 +++++++++++------ 2 files changed, 101 insertions(+), 18 deletions(-) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index 1aa61bc4..4754ed4e 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -6,19 +6,69 @@ import securesystemslib.hash as sslib_hash from securesystemslib import exceptions +from securesystemslib.keys import _get_keyid from securesystemslib.signer._key import Key -from securesystemslib.signer._signer import SecretsHandler, Signature, Signer +from securesystemslib.signer._signer import ( + SecretsHandler, + Signature, + Signer, + SSlibKey, +) logger = logging.getLogger(__name__) GCP_IMPORT_ERROR = None try: from google.cloud import kms + from google.cloud.kms_v1.types import CryptoKeyVersion except ImportError: GCP_IMPORT_ERROR = ( "google-cloud-kms library required to sign with Google Cloud keys." ) +KEYTYPE_AND_SCHEME = { + CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P256_SHA256: ( + "ecdsa-sha2-nistp256", + "ecdsa-sha2-nistp256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P384_SHA384: ( + "ecdsa-sha2-nistp384", + "ecdsa-sha2-nistp384", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_2048_SHA256: ( + "rsa", + "rsassa-pss-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_3072_SHA256: ( + "rsa", + "rsassa-pss-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_4096_SHA256: ( + "rsa", + "rsassa-pss-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_4096_SHA512: ( + "rsa", + "rsassa-pss-sha512", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_2048_SHA256: ( + "rsa", + "rsa-pkcs1v15-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_3072_SHA256: ( + "rsa", + "rsa-pkcs1v15-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_4096_SHA256: ( + "rsa", + "rsa-pkcs1v15-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_4096_SHA512: ( + "rsa", + "rsa-pkcs1v15-sha512", + ), +} + class GCPSigner(Signer): """Google Cloud KMS Signer @@ -71,6 +121,25 @@ def from_priv_key_uri( return cls(uri.path, public_key) + @classmethod + def import_(cls, gcp_keyid: str): + """Load signer (including public key) from KMS""" + client = kms.KeyManagementServiceClient() + request = {"name": gcp_keyid} + kms_pubkey = client.get_public_key(request) + try: + keytype, scheme = KEYTYPE_AND_SCHEME[kms_pubkey.algorithm] + except KeyError as e: + raise exceptions.UnsupportedAlgorithmError( + f"{kms_pubkey.algorithm} is not a supported signing algorithm" + ) from e + + keyval = {"public": kms_pubkey.pem} + keyid = _get_keyid(keytype, scheme, keyval) + public_key = SSlibKey(keyid, keytype, scheme, keyval) + + return cls(gcp_keyid, public_key) + @staticmethod def _get_hash_algorithm(public_key: Key) -> str: """Helper function to return payload hash algorithm used for this key""" diff --git a/tests/check_kms_signers.py b/tests/check_kms_signers.py index 0674dc86..a566e7ab 100644 --- a/tests/check_kms_signers.py +++ b/tests/check_kms_signers.py @@ -18,13 +18,26 @@ import unittest from securesystemslib.exceptions import UnverifiedSignatureError -from securesystemslib.signer import Key, Signer +from securesystemslib.signer import GCPSigner, Key, Signer class TestKMSKeys(unittest.TestCase): """Test that KMS keys can be used to sign.""" - def test_gcp(self): + pubkey = Key.from_dict( + "abcd", + { + "keyid": "abcd", + "keytype": "ecdsa", + "scheme": "ecdsa-sha2-nistp256", + "keyval": { + "public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE/ptvrXYuUc2ZaKssHhtg/IKNbO1X\ncDWlbKqLNpaK62MKdOwDz1qlp5AGHZkTY9tO09iq1F16SvVot1BQ9FJ2dw==\n-----END PUBLIC KEY-----\n" + }, + }, + ) + gcp_id = "projects/python-tuf-kms/locations/global/keyRings/securesystemslib-tests/cryptoKeys/ecdsa-sha2-nistp256/cryptoKeyVersions/1" + + def test_gcp_sign(self): """Test that GCP KMS key works for signing NOTE: The KMS account is setup to only accept requests from the @@ -35,25 +48,26 @@ def test_gcp(self): """ data = "data".encode("utf-8") - pubkey = Key.from_dict( - "abcd", - { - "keyid": "abcd", - "keytype": "ecdsa", - "scheme": "ecdsa-sha2-nistp256", - "keyval": { - "public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE/ptvrXYuUc2ZaKssHhtg/IKNbO1X\ncDWlbKqLNpaK62MKdOwDz1qlp5AGHZkTY9tO09iq1F16SvVot1BQ9FJ2dw==\n-----END PUBLIC KEY-----\n" - }, - }, - ) - gcp_id = "projects/python-tuf-kms/locations/global/keyRings/securesystemslib-tests/cryptoKeys/ecdsa-sha2-nistp256/cryptoKeyVersions/1" - signer = Signer.from_priv_key_uri(f"gcpkms:{gcp_id}", pubkey) + signer = Signer.from_priv_key_uri(f"gcpkms:{self.gcp_id}", self.pubkey) sig = signer.sign(data) - pubkey.verify_signature(sig, data) + self.pubkey.verify_signature(sig, data) with self.assertRaises(UnverifiedSignatureError): - pubkey.verify_signature(sig, b"NOT DATA") + self.pubkey.verify_signature(sig, b"NOT DATA") + + def test_gcp_import(self): + """Test that GCP KMS key can be imported + + NOTE: The KMS account is setup to only accept requests from the + Securesystemslib GitHub Action environment: test cannot pass elsewhere. + + In case of problems with KMS account, please file an issue and + assign @jku. + """ + + signer = GCPSigner.import_(self.gcp_id) + self.assertEqual(self.pubkey, signer.public_key) if __name__ == "__main__": From 148a65522df38dd4cd9d72f9d592612e4691fd40 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 12 Dec 2022 17:35:02 +0200 Subject: [PATCH 02/11] GCPSigner: expose private key uri This is useful on initial signer setup: After import_(), the application will want to store in application configuration: * private key uri * the public key keyid --- securesystemslib/signer/_gcp_signer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index 4754ed4e..b12e4401 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -104,6 +104,7 @@ def __init__(self, gcp_keyid: str, public_key: Key): self.hash_algorithm = self._get_hash_algorithm(public_key) self.gcp_keyid = gcp_keyid + self.priv_key_uri = f"{self.SCHEME}:{gcp_keyid}" self.public_key = public_key self.client = kms.KeyManagementServiceClient() From 5e59251154a1bb426a104461c564e368c9787497 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 12 Dec 2022 17:55:42 +0200 Subject: [PATCH 03/11] GCPSigner: Modify expected public key values Get them to match the format produced from the GCP key content --- tests/check_kms_signers.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/check_kms_signers.py b/tests/check_kms_signers.py index a566e7ab..934141ed 100644 --- a/tests/check_kms_signers.py +++ b/tests/check_kms_signers.py @@ -25,10 +25,9 @@ class TestKMSKeys(unittest.TestCase): """Test that KMS keys can be used to sign.""" pubkey = Key.from_dict( - "abcd", + "23b650fcc538c0c6d423d0886448172fd51d81f43ef749851a0a84978cdcd8e0", { - "keyid": "abcd", - "keytype": "ecdsa", + "keytype": "ecdsa-sha2-nistp256", "scheme": "ecdsa-sha2-nistp256", "keyval": { "public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE/ptvrXYuUc2ZaKssHhtg/IKNbO1X\ncDWlbKqLNpaK62MKdOwDz1qlp5AGHZkTY9tO09iq1F16SvVot1BQ9FJ2dw==\n-----END PUBLIC KEY-----\n" From 5138105c4f0939402b4ab161454250eff10c3deb Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 12 Dec 2022 20:48:07 +0200 Subject: [PATCH 04/11] GCPSigner: Don't use google-cloud-kms before init Don't use the google-cloud-kms module API during import: Build the lookup table for keytype/scheme inside a method. --- securesystemslib/signer/_gcp_signer.py | 97 ++++++++++++++------------ 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index b12e4401..41886315 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -1,7 +1,7 @@ """Signer implementation for Google Cloud KMS""" import logging -from typing import Optional +from typing import Optional, Tuple from urllib import parse import securesystemslib.hash as sslib_hash @@ -26,49 +26,6 @@ "google-cloud-kms library required to sign with Google Cloud keys." ) -KEYTYPE_AND_SCHEME = { - CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P256_SHA256: ( - "ecdsa-sha2-nistp256", - "ecdsa-sha2-nistp256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P384_SHA384: ( - "ecdsa-sha2-nistp384", - "ecdsa-sha2-nistp384", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_2048_SHA256: ( - "rsa", - "rsassa-pss-sha256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_3072_SHA256: ( - "rsa", - "rsassa-pss-sha256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_4096_SHA256: ( - "rsa", - "rsassa-pss-sha256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_4096_SHA512: ( - "rsa", - "rsassa-pss-sha512", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_2048_SHA256: ( - "rsa", - "rsa-pkcs1v15-sha256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_3072_SHA256: ( - "rsa", - "rsa-pkcs1v15-sha256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_4096_SHA256: ( - "rsa", - "rsa-pkcs1v15-sha256", - ), - CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_4096_SHA512: ( - "rsa", - "rsa-pkcs1v15-sha512", - ), -} - class GCPSigner(Signer): """Google Cloud KMS Signer @@ -125,11 +82,14 @@ def from_priv_key_uri( @classmethod def import_(cls, gcp_keyid: str): """Load signer (including public key) from KMS""" + if GCP_IMPORT_ERROR: + raise exceptions.UnsupportedLibraryError(GCP_IMPORT_ERROR) + client = kms.KeyManagementServiceClient() request = {"name": gcp_keyid} kms_pubkey = client.get_public_key(request) try: - keytype, scheme = KEYTYPE_AND_SCHEME[kms_pubkey.algorithm] + keytype, scheme = cls._get_keytype_and_scheme(kms_pubkey.algorithm) except KeyError as e: raise exceptions.UnsupportedAlgorithmError( f"{kms_pubkey.algorithm} is not a supported signing algorithm" @@ -141,6 +101,53 @@ def import_(cls, gcp_keyid: str): return cls(gcp_keyid, public_key) + @staticmethod + def _get_keytype_and_scheme(algorithm: int) -> Tuple[str, str]: + """Return keytype and scheme for the KMS algorithm enum""" + keytypes_and_schemes = { + CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P256_SHA256: ( + "ecdsa-sha2-nistp256", + "ecdsa-sha2-nistp256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P384_SHA384: ( + "ecdsa-sha2-nistp384", + "ecdsa-sha2-nistp384", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_2048_SHA256: ( + "rsa", + "rsassa-pss-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_3072_SHA256: ( + "rsa", + "rsassa-pss-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_4096_SHA256: ( + "rsa", + "rsassa-pss-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_4096_SHA512: ( + "rsa", + "rsassa-pss-sha512", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_2048_SHA256: ( + "rsa", + "rsa-pkcs1v15-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_3072_SHA256: ( + "rsa", + "rsa-pkcs1v15-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_4096_SHA256: ( + "rsa", + "rsa-pkcs1v15-sha256", + ), + CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PKCS1_4096_SHA512: ( + "rsa", + "rsa-pkcs1v15-sha512", + ), + } + return keytypes_and_schemes[algorithm] + @staticmethod def _get_hash_algorithm(public_key: Key) -> str: """Helper function to return payload hash algorithm used for this key""" From 96ee4e1bd5a58c2f0d937428962cba3a93004833 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 13 Dec 2022 12:42:57 +0200 Subject: [PATCH 05/11] GCPSigner: Document auth and permissions better --- securesystemslib/signer/_gcp_signer.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index 41886315..5d7eb8c6 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -36,9 +36,14 @@ class GCPSigner(Signer): The signer uses "ambient" credentials: typically environment var GOOGLE_APPLICATION_CREDENTIALS that points to a file with valid credentials. These will be found by google.cloud.kms, see - https://cloud.google.com/docs/authentication/getting-started - (and https://github.com/google-github-actions/auth for the relevant - GitHub action). + https://cloud.google.com/docs/authentication/getting-started. + Some practical authentication options include: + * GitHub Action: https://github.com/google-github-actions/auth + * gcloud CLI: https://cloud.google.com/sdk/gcloud + + The specific permissions that GCPSigner needs are: + * roles/cloudkms.signer for sign() + * roles/cloudkms.publicKeyViewer for import() Arguments: gcp_keyid: Fully qualified GCP KMS key name, like From b59478b74726d0ff435f48fd71de9bce90fe877d Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 13 Dec 2022 13:48:50 +0200 Subject: [PATCH 06/11] GCPSigner: Tweak import return value Instead of being a constructor, import_() now returns the private key URI and the public Key. This makes sense as * it's still trivial to construct the Signer if needed * In many cases we don't actually use the Signer at import time * This works around the problem that a Signer instance might need a SecretsHandler * This setup likely works for key generation as well --- securesystemslib/signer/_gcp_signer.py | 10 +++++++--- tests/check_kms_signers.py | 5 +++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index 5d7eb8c6..8b084bbe 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -85,8 +85,12 @@ def from_priv_key_uri( return cls(uri.path, public_key) @classmethod - def import_(cls, gcp_keyid: str): - """Load signer (including public key) from KMS""" + def import_(cls, gcp_keyid: str) -> Tuple[str, Key]: + """Load key and signer details from KMS + + Returns the private key uri and the public key. This method should only + be called once per key: the uri and Key should be stored for later use. + """ if GCP_IMPORT_ERROR: raise exceptions.UnsupportedLibraryError(GCP_IMPORT_ERROR) @@ -104,7 +108,7 @@ def import_(cls, gcp_keyid: str): keyid = _get_keyid(keytype, scheme, keyval) public_key = SSlibKey(keyid, keytype, scheme, keyval) - return cls(gcp_keyid, public_key) + return f"{cls.SCHEME}:{gcp_keyid}", public_key @staticmethod def _get_keytype_and_scheme(algorithm: int) -> Tuple[str, str]: diff --git a/tests/check_kms_signers.py b/tests/check_kms_signers.py index 934141ed..49a19216 100644 --- a/tests/check_kms_signers.py +++ b/tests/check_kms_signers.py @@ -65,8 +65,9 @@ def test_gcp_import(self): assign @jku. """ - signer = GCPSigner.import_(self.gcp_id) - self.assertEqual(self.pubkey, signer.public_key) + uri, key = GCPSigner.import_(self.gcp_id) + self.assertEqual(key, self.pubkey) + self.assertEqual(uri, f"gcpkms:{self.gcp_id}") if __name__ == "__main__": From 9a7fabfc645d9a85543e611d60ee109fbf1b3163 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Thu, 15 Dec 2022 15:54:22 +0200 Subject: [PATCH 07/11] pyproject: gpsigner really needs crypto too The actual keys are ecdsa so we need to depend on cryptography --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 130e3e72..015b0a34 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,7 @@ Issues = "https://github.com/secure-systems-lab/securesystemslib/issues" [project.optional-dependencies] crypto = ["cryptography>=37.0.0"] -gcpkms = ["google-cloud-kms"] +gcpkms = ["google-cloud-kms", "cryptography>=37.0.0"] pynacl = ["pynacl>1.2.0"] PySPX = ["PySPX==0.5.0"] asn1 = ["asn1crypto"] From 3b66de23902ce41151f16d641c47b65cdf7fe424 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 11 Jan 2023 10:14:50 +0200 Subject: [PATCH 08/11] signer: standardize HSMSigner.import_() with GCPSigner There is no signature to conform to here but since the methods do work in a similar way, stanrdaize as much as possible: * same name * same return value Returning the private key uri is not super useful for HSM at this point (as the URI is a static string) but will be if e.g. slot is made user configurable. --- securesystemslib/signer/_hsm_signer.py | 13 +++++++++---- tests/test_hsm_signer.py | 8 +++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/securesystemslib/signer/_hsm_signer.py b/securesystemslib/signer/_hsm_signer.py index 95343c15..f749e4f8 100644 --- a/securesystemslib/signer/_hsm_signer.py +++ b/securesystemslib/signer/_hsm_signer.py @@ -191,10 +191,14 @@ def _find_key_values( return ECDomainParameters.load(bytes(params)), bytes(point) @classmethod - def pubkey_from_hsm( + def import_( cls, sslib_keyid: str, hsm_keyid: Optional[int] = None - ) -> SSlibKey: - """Export public key from HSM. + ) -> Tuple[str, SSlibKey]: + """Import public key and signer details from HSM. + + Returns a private key URI (for Signer.from_priv_key_uri()) and a public + key. import_() should be called once and the returned URI and public + key should be stored for later use. Arguments: sslib_keyid: Key identifier that is unique within the metadata it is used in. @@ -240,12 +244,13 @@ def pubkey_from_hsm( .decode() ) - return SSlibKey( + key = SSlibKey( sslib_keyid, KEY_TYPE_ECDSA, _SCHEME_FOR_CURVE[curve], {"public": public_pem}, ) + return "hsm:", key @classmethod def from_priv_key_uri( diff --git a/tests/test_hsm_signer.py b/tests/test_hsm_signer.py index 5e8dfbae..a2917174 100644 --- a/tests/test_hsm_signer.py +++ b/tests/test_hsm_signer.py @@ -139,7 +139,7 @@ def test_hsm(self): """Test HSM key export and signing.""" for hsm_keyid in [self.hsm_keyid, self.hsm_keyid_default]: - key = HSMSigner.pubkey_from_hsm(self.sslib_keyid, hsm_keyid) + _, key = HSMSigner.import_(self.sslib_keyid, hsm_keyid) signer = HSMSigner(hsm_keyid, key, lambda sec: self.hsm_user_pin) sig = signer.sign(b"DATA") key.verify_signature(sig, b"DATA") @@ -150,11 +150,9 @@ def test_hsm(self): def test_hsm_uri(self): """Test HSM default key export and signing from URI.""" - key = HSMSigner.pubkey_from_hsm( - self.sslib_keyid, self.hsm_keyid_default - ) + uri, key = HSMSigner.import_(self.sslib_keyid, self.hsm_keyid_default) signer = Signer.from_priv_key_uri( - "hsm:", key, lambda sec: self.hsm_user_pin + uri, key, lambda sec: self.hsm_user_pin ) sig = signer.sign(b"DATA") key.verify_signature(sig, b"DATA") From 7d4fe8e0fcd3f505222ec534d5f9e34bdd06dc6c Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 11 Jan 2023 10:27:04 +0200 Subject: [PATCH 09/11] signer: Remove keyid arg from HSMSigner.import_ Intent is to make GCP and HSM imports as similar as possible. --- securesystemslib/signer/_hsm_signer.py | 17 +++++++---------- tests/test_hsm_signer.py | 5 ++--- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/securesystemslib/signer/_hsm_signer.py b/securesystemslib/signer/_hsm_signer.py index f749e4f8..fc83f01f 100644 --- a/securesystemslib/signer/_hsm_signer.py +++ b/securesystemslib/signer/_hsm_signer.py @@ -11,6 +11,7 @@ from securesystemslib import KEY_TYPE_ECDSA from securesystemslib.exceptions import UnsupportedLibraryError +from securesystemslib.keys import _get_keyid from securesystemslib.signer._key import Key, SSlibKey from securesystemslib.signer._signature import Signature from securesystemslib.signer._signer import SecretsHandler, Signer @@ -191,9 +192,7 @@ def _find_key_values( return ECDomainParameters.load(bytes(params)), bytes(point) @classmethod - def import_( - cls, sslib_keyid: str, hsm_keyid: Optional[int] = None - ) -> Tuple[str, SSlibKey]: + def import_(cls, hsm_keyid: Optional[int] = None) -> Tuple[str, SSlibKey]: """Import public key and signer details from HSM. Returns a private key URI (for Signer.from_priv_key_uri()) and a public @@ -201,7 +200,6 @@ def import_( key should be stored for later use. Arguments: - sslib_keyid: Key identifier that is unique within the metadata it is used in. hsm_keyid: Key identifier on the token. Default is 2 (meaning PIV key slot 9c). Raises: @@ -244,12 +242,11 @@ def import_( .decode() ) - key = SSlibKey( - sslib_keyid, - KEY_TYPE_ECDSA, - _SCHEME_FOR_CURVE[curve], - {"public": public_pem}, - ) + keyval = {"public": public_pem} + scheme = _SCHEME_FOR_CURVE[curve] + keyid = _get_keyid(KEY_TYPE_ECDSA, scheme, keyval) + key = SSlibKey(keyid, KEY_TYPE_ECDSA, scheme, keyval) + return "hsm:", key @classmethod diff --git a/tests/test_hsm_signer.py b/tests/test_hsm_signer.py index a2917174..6d7cd859 100644 --- a/tests/test_hsm_signer.py +++ b/tests/test_hsm_signer.py @@ -51,7 +51,6 @@ class TestHSM(unittest.TestCase): See .github/workflows/hsm.yml for how this can be done on Linux, macOS and Windows. """ - sslib_keyid = "a" * 64 # Mock SSlibKey conform sha256 hex digest keyid hsm_keyid = 1 hsm_keyid_default = 2 hsm_user_pin = "123456" @@ -139,7 +138,7 @@ def test_hsm(self): """Test HSM key export and signing.""" for hsm_keyid in [self.hsm_keyid, self.hsm_keyid_default]: - _, key = HSMSigner.import_(self.sslib_keyid, hsm_keyid) + _, key = HSMSigner.import_(hsm_keyid) signer = HSMSigner(hsm_keyid, key, lambda sec: self.hsm_user_pin) sig = signer.sign(b"DATA") key.verify_signature(sig, b"DATA") @@ -150,7 +149,7 @@ def test_hsm(self): def test_hsm_uri(self): """Test HSM default key export and signing from URI.""" - uri, key = HSMSigner.import_(self.sslib_keyid, self.hsm_keyid_default) + uri, key = HSMSigner.import_(self.hsm_keyid_default) signer = Signer.from_priv_key_uri( uri, key, lambda sec: self.hsm_user_pin ) From b220c426450e39f3cb56965b1a9448e24ce7092b Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 11 Jan 2023 17:29:25 +0200 Subject: [PATCH 10/11] GCPSigner: Remove unused attribute private key uri is now available as return value from import(): it is not needed as attribute. --- securesystemslib/signer/_gcp_signer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index 8b084bbe..14d9b220 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -66,7 +66,6 @@ def __init__(self, gcp_keyid: str, public_key: Key): self.hash_algorithm = self._get_hash_algorithm(public_key) self.gcp_keyid = gcp_keyid - self.priv_key_uri = f"{self.SCHEME}:{gcp_keyid}" self.public_key = public_key self.client = kms.KeyManagementServiceClient() From f8c851fe833e3e060c693ee4e8e43984e29afd53 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 11 Jan 2023 17:33:55 +0200 Subject: [PATCH 11/11] GCPSigner: Use "ecdsa" as keytype The more specific keytypes may get deprecated at some point. --- securesystemslib/signer/_gcp_signer.py | 4 ++-- tests/check_kms_signers.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py index 14d9b220..5b8e7cac 100644 --- a/securesystemslib/signer/_gcp_signer.py +++ b/securesystemslib/signer/_gcp_signer.py @@ -114,11 +114,11 @@ def _get_keytype_and_scheme(algorithm: int) -> Tuple[str, str]: """Return keytype and scheme for the KMS algorithm enum""" keytypes_and_schemes = { CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P256_SHA256: ( - "ecdsa-sha2-nistp256", + "ecdsa", "ecdsa-sha2-nistp256", ), CryptoKeyVersion.CryptoKeyVersionAlgorithm.EC_SIGN_P384_SHA384: ( - "ecdsa-sha2-nistp384", + "ecdsa", "ecdsa-sha2-nistp384", ), CryptoKeyVersion.CryptoKeyVersionAlgorithm.RSA_SIGN_PSS_2048_SHA256: ( diff --git a/tests/check_kms_signers.py b/tests/check_kms_signers.py index 49a19216..236660cc 100644 --- a/tests/check_kms_signers.py +++ b/tests/check_kms_signers.py @@ -25,9 +25,9 @@ class TestKMSKeys(unittest.TestCase): """Test that KMS keys can be used to sign.""" pubkey = Key.from_dict( - "23b650fcc538c0c6d423d0886448172fd51d81f43ef749851a0a84978cdcd8e0", + "218611b80052667026c221f8774249b0f6b8b310d30a5c45a3b878aa3a02f39e", { - "keytype": "ecdsa-sha2-nistp256", + "keytype": "ecdsa", "scheme": "ecdsa-sha2-nistp256", "keyval": { "public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE/ptvrXYuUc2ZaKssHhtg/IKNbO1X\ncDWlbKqLNpaK62MKdOwDz1qlp5AGHZkTY9tO09iq1F16SvVot1BQ9FJ2dw==\n-----END PUBLIC KEY-----\n"