Merge pull request #557 from lukpueh/mv-_get_keyid

Move _get_keyid helper to Signer base class and use in implementations
secure-systems-lab · Mar 31, 2023 · 09e092e · 09e092e
2 parents 289102e + 7400ed7
commit 09e092e
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 25 deletions.
diff --git a/securesystemslib/signer/_gcp_signer.py b/securesystemslib/signer/_gcp_signer.py
@@ -6,7 +6,6 @@
 
 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,
@@ -104,7 +103,7 @@ def import_(cls, gcp_keyid: str) -> Tuple[str, Key]:
             ) from e
 
         keyval = {"public": kms_pubkey.pem}
-        keyid = _get_keyid(keytype, scheme, keyval)
+        keyid = cls._get_keyid(keytype, scheme, keyval)
         public_key = SSlibKey(keyid, keytype, scheme, keyval)
 
         return f"{cls.SCHEME}:{gcp_keyid}", public_key

diff --git a/securesystemslib/signer/_hsm_signer.py b/securesystemslib/signer/_hsm_signer.py
@@ -12,7 +12,6 @@
 from securesystemslib import KEY_TYPE_ECDSA
 from securesystemslib.exceptions import UnsupportedLibraryError
 from securesystemslib.hash import digest
-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
@@ -322,7 +321,7 @@ def import_(
 
         keyval = {"public": public_pem}
         scheme = _SCHEME_FOR_CURVE[curve]
-        keyid = _get_keyid(KEY_TYPE_ECDSA, scheme, keyval)
+        keyid = cls._get_keyid(KEY_TYPE_ECDSA, scheme, keyval)
         key = SSlibKey(keyid, KEY_TYPE_ECDSA, scheme, keyval)
 
         return uri, key

diff --git a/securesystemslib/signer/_signer.py b/securesystemslib/signer/_signer.py
@@ -3,10 +3,12 @@
 import logging
 import os
 from abc import ABCMeta, abstractmethod
-from typing import Callable, Dict, Optional, Type
+from typing import Any, Callable, Dict, Optional, Type
 from urllib import parse
 
 import securesystemslib.keys as sslib_keys
+from securesystemslib.formats import encode_canonical
+from securesystemslib.hash import digest
 from securesystemslib.signer._key import Key, SSlibKey
 from securesystemslib.signer._signature import Signature
 
@@ -117,6 +119,20 @@ def from_priv_key_uri(
             priv_key_uri, public_key, secrets_handler
         )
 
+    @staticmethod
+    def _get_keyid(keytype: str, scheme, keyval: Dict[str, Any]) -> str:
+        """Get keyid as sha256 hexdigest of the cjson representation of key fields."""
+        data = encode_canonical(
+            {
+                "keytype": keytype,
+                "scheme": scheme,
+                "keyval": keyval,
+            }
+        ).encode("utf-8")
+        hasher = digest("sha256")
+        hasher.update(data)
+        return hasher.hexdigest()
+
 
 class SSlibSigner(Signer):
     """A securesystemslib signer implementation.

diff --git a/securesystemslib/signer/_sigstore_signer.py b/securesystemslib/signer/_sigstore_signer.py
@@ -12,8 +12,6 @@
     UnverifiedSignatureError,
     VerificationError,
 )
-from securesystemslib.formats import encode_canonical
-from securesystemslib.hash import digest
 from securesystemslib.signer._signer import (
     Key,
     SecretsHandler,
@@ -178,23 +176,6 @@ def from_priv_key_uri(
     def _get_uri(cls, ambient: bool) -> str:
         return f"{cls.SCHEME}:{'' if ambient else '?ambient=false'}"
 
-    @classmethod
-    def _get_keyid(cls, keytype: str, scheme, keyval: Dict[str, Any]) -> str:
-        """Compute keyid as hexdigest over canonical json representation of key.
-
-        NOTE: Not compatible with ``securesystemslib.keys._get_keyid()``
-        """
-        data = encode_canonical(
-            {
-                "keytype": keytype,
-                "scheme": scheme,
-                "keyval": keyval,
-            }
-        ).encode("utf-8")
-        hasher = digest()
-        hasher.update(data)
-        return hasher.hexdigest()
-
     @classmethod
     def import_(
         cls, identity: str, issuer: str, ambient: bool = True

diff --git a/tests/check_kms_signers.py b/tests/check_kms_signers.py
@@ -23,7 +23,7 @@ class TestKMSKeys(unittest.TestCase):
     """Test that KMS keys can be used to sign."""
 
     pubkey = Key.from_dict(
-        "218611b80052667026c221f8774249b0f6b8b310d30a5c45a3b878aa3a02f39e",
+        "ab45d8d98992a4128efaea284c7ef0459557db199aeadf237ae41b915b9b5a1c",
         {
             "keytype": "ecdsa",
             "scheme": "ecdsa-sha2-nistp256",

diff --git a/tests/test_signer.py b/tests/test_signer.py
@@ -507,6 +507,27 @@ def test_gpg_key__eq__(self):
         self.assertNotEqual(key1, other_key)
 
 
+class TestUtils(unittest.TestCase):
+    """Test Signer utility methods."""
+
+    def test_get_keyid(self):
+        # pylint: disable=protected-access
+        self.assertEqual(
+            Signer._get_keyid("rsa", "rsassa-pss-sha256", {"public": "abcd"}),
+            "7b56b88ae790729d4e359d3fc5e889f1e0669a2e71a12d00e87473870c73fbcf",
+        )
+
+        # Unsupported keys can have default keyids too
+        self.assertEqual(
+            Signer._get_keyid("foo", "bar", {"baz": "qux"}),
+            "e3471be0598305190ba82f6f8043f4df52f3fbe471fdc187223bd9ade92abebb",
+        )
+
+        # Invalid keys cannot
+        with self.assertRaises(FormatError):
+            Signer._get_keyid("foo", "bar", {"baz": 1.1})
+
+
 # Run the unit tests.
 if __name__ == "__main__":
     unittest.main()