Merge pull request #678 from lukpueh/make-sslibkey-from-crypto-public

SSlibKey: make '_from_crypto_public_key' public API and remove 'from_pem'

docs/CRYPTO_SIGNER.md

@@ -82,13 +82,17 @@ os.environ.update({
 ```python
 import os
 from securesystemslib.signer import SSlibKey, Signer, CryptoSigner, SIGNER_FOR_URI_SCHEME
+from cryptography.hazmat.primitives.serialization import load_pem_public_key
+
 
 # NOTE: Registration becomes obsolete once CryptoSigner is the default file signer
 SIGNER_FOR_URI_SCHEME.update({CryptoSigner.FILE_URI_SCHEME: CryptoSigner})
 
 # Read signer details
 uri = os.environ["SIGNER_URI"]
-public_key = SSlibKey.from_pem(os.environ["SIGNER_PUBLIC"].encode())
+public_key = SSlibKey.from_crypto(
+    load_pem_public_key(os.environ["SIGNER_PUBLIC"].encode())
+)
 secrets_handler = lambda sec: os.environ["SIGNER_SECRET"]
 
 # Load and sign

securesystemslib/signer/_crypto_signer.py

@@ -131,9 +131,7 @@ def __init__(
             raise UnsupportedLibraryError(CRYPTO_IMPORT_ERROR)
 
         if public_key is None:
-            public_key = SSlibKey._from_crypto_public_key(
-                private_key.public_key(), None, None
-            )
+            public_key = SSlibKey.from_crypto(private_key.public_key())
 
         self._private_key: PrivateKeyTypes
         self._sign_args: Union[_RSASignArgs, _ECDSASignArgs, _NoSignArgs]
@@ -276,7 +274,7 @@ def generate_ed25519(
             raise UnsupportedLibraryError(CRYPTO_IMPORT_ERROR)
 
         private_key = Ed25519PrivateKey.generate()
-        public_key = SSlibKey._from_crypto_public_key(  # pylint: disable=protected-access
+        public_key = SSlibKey.from_crypto(
             private_key.public_key(), keyid, "ed25519"
         )
         return CryptoSigner(private_key, public_key)
@@ -307,7 +305,7 @@ def generate_rsa(
             public_exponent=65537,
             key_size=size,
         )
-        public_key = SSlibKey._from_crypto_public_key(  # pylint: disable=protected-access
+        public_key = SSlibKey.from_crypto(
             private_key.public_key(), keyid, scheme
         )
         return CryptoSigner(private_key, public_key)
@@ -331,7 +329,7 @@ def generate_ecdsa(
             raise UnsupportedLibraryError(CRYPTO_IMPORT_ERROR)
 
         private_key = generate_ec_private_key(SECP256R1())
-        public_key = SSlibKey._from_crypto_public_key(  # pylint: disable=protected-access
+        public_key = SSlibKey.from_crypto(
             private_key.public_key(), keyid, "ecdsa-sha2-nistp256"
         )
         return CryptoSigner(private_key, public_key)

securesystemslib/signer/_key.py

@@ -276,21 +276,31 @@ def _get_default_scheme(keytype: str) -> str:
         raise ValueError(f"unsupported 'keytype' {keytype}")
 
     @classmethod
-    def _from_crypto_public_key(
+    def from_crypto(
         cls,
         public_key: "PublicKeyTypes",
-        keyid: Optional[str],
-        scheme: Optional[str],
+        keyid: Optional[str] = None,
+        scheme: Optional[str] = None,
     ) -> "SSlibKey":
-        """Helper to create SSlibKey from pyca/cryptography public key.
+        """Create SSlibKey from pyca/cryptography public key.
+
+        Args:
+            public_key: pyca/cryptography public key object.
+            keyid: Key identifier. If not passed, a default keyid is computed.
+            scheme: SSlibKey signing scheme. Defaults are "rsassa-pss-sha256",
+                "ecdsa-sha2-nistp256", and "ed25519" according to the keytype
+
+        Raises:
+            UnsupportedLibraryError: pyca/cryptography not installed
+            ValueError: Key type not supported
 
-        NOTE: keytype (rsa, ecdsa, ed25519) assessed automatically. Defaults
-        exist for keyid and scheme, if not passed.
+        Returns:
+            SSlibKey
 
-        FIXME: also used in CryptoSigner keygen implementations, which requires
-        protected access. Should we make it public, or refactor and move to
-        an internal utils method?
         """
+        if CRYPTO_IMPORT_ERROR:
+            raise UnsupportedLibraryError(CRYPTO_IMPORT_ERROR)
+
         keytype = cls._get_keytype_for_crypto_key(public_key)
         if not scheme:
             scheme = cls._get_default_scheme(keytype)
@@ -314,42 +324,6 @@ def _from_crypto_public_key(
 
         return SSlibKey(keyid, keytype, scheme, keyval)
 
-    @classmethod
-    def from_pem(
-        cls,
-        pem: bytes,
-        scheme: Optional[str] = None,
-        keyid: Optional[str] = None,
-    ) -> "SSlibKey":
-        """Load SSlibKey from PEM.
-
-        NOTE: pyca/cryptography is used to decode the PEM payload. The expected
-        (and tested) format is subjectPublicKeyInfo (RFC 5280). Other formats
-        may but are not guaranteed to work.
-
-        Args:
-            pem: Public key PEM data.
-            scheme: SSlibKey signing scheme. Defaults are "rsassa-pss-sha256",
-                "ecdsa-sha2-nistp256", and "ed25519" according to the keytype
-            keyid: Key identifier. If not passed, a default keyid is computed.
-
-        Raises:
-            UnsupportedLibraryError: pyca/cryptography not installed
-            ValueError: Key type not supported
-            ValueError, \
-                    cryptography.exceptions.UnsupportedAlgorithm:
-                pyca/cryptography deserialization failed
-
-        Returns:
-            SSlibKey
-
-        """
-        if CRYPTO_IMPORT_ERROR:
-            raise UnsupportedLibraryError(CRYPTO_IMPORT_ERROR)
-
-        public_key = load_pem_public_key(pem)
-        return cls._from_crypto_public_key(public_key, keyid, scheme)
-
     @staticmethod
     def _get_hash_algorithm(name: str) -> "HashAlgorithm":
         """Helper to return hash algorithm for name."""

tests/check_public_interfaces.py

@@ -308,10 +308,10 @@ def test_gpg_functions(self):
             securesystemslib.gpg.functions.export_pubkey("f00")
         self.assertEqual(expected_error_msg, str(ctx.exception))
 
-    def test_sslib_key_from_pem(self):
-        """Assert raise UnsupportedLibraryError on SSlibKey.from_pem()."""
+    def test_sslib_key_from_crypto(self):
+        """Assert raise UnsupportedLibraryError on SSlibKey.from_crypto()."""
         with self.assertRaises(UnsupportedLibraryError):
-            SSlibKey.from_pem(b"fail")
+            SSlibKey.from_crypto("mock pyca/crypto pubkey")  # type: ignore
 
     def test_crypto_signer_from_priv_key_uri(self):
         """Assert raise UnsupportedLibraryError on 'from_priv_key_uri'."""

tests/test_migrate_key.py

@@ -7,6 +7,8 @@
 from pathlib import Path
 from unittest.mock import patch
 
+from cryptography.hazmat.primitives.serialization import load_pem_public_key
+
 from docs.migrate_key import main as migrate_key_cli
 from securesystemslib.exceptions import UnverifiedSignatureError
 from securesystemslib.interface import (
@@ -53,11 +55,16 @@ def setUpClass(cls):
     def tearDownClass(cls):
         shutil.rmtree(cls.new_keys)
 
+    def _from_file(self, algo):
+        with open(self.new_keys / f"{algo}_public", "rb") as f:
+            pem = f.read()
+        return load_pem_public_key(pem)
+
     def test_migrated_keys(self):
         for algo in ["rsa", "ecdsa", "ed25519"]:
             # Load public key
-            with open(self.new_keys / f"{algo}_public", "rb") as f:
-                public_key = SSlibKey.from_pem(f.read())
+            crypto_key = self._from_file(algo)
+            public_key = SSlibKey.from_crypto(crypto_key)
 
             # Load unencrypted private key
             path = self.new_keys / f"{algo}_private_unencrypted"
@@ -109,12 +116,12 @@ def test_old_signature_verifies_with_new_key(self):
             signer = SSlibSigner(private_key)
 
             # Load new public key
-            with open(self.new_keys / f"{algo}_public", "rb") as f:
-                # NOTE: The new auto-keyid would differ from the old keyid.
-                # Set it explicitly, to verify signatures with old keyid below
-                public_key = SSlibKey.from_pem(
-                    f.read(), keyid=private_key["keyid"]
-                )
+            crypto_key = self._from_file(algo)
+            # NOTE: The new auto-keyid would differ from the old keyid.
+            # Set it explicitly, to verify signatures with old keyid below
+            public_key = SSlibKey.from_crypto(
+                crypto_key, keyid=private_key["keyid"]
+            )
 
             # Sign and test signature
             sig = signer.sign(b"data")

tests/test_signer.py

@@ -8,7 +8,10 @@
 from pathlib import Path
 from typing import Any, Dict, Optional
 
-from cryptography.hazmat.primitives.serialization import load_pem_private_key
+from cryptography.hazmat.primitives.serialization import (
+    load_pem_private_key,
+    load_pem_public_key,
+)
 
 import securesystemslib.keys as KEYS
 from securesystemslib.exceptions import (
@@ -289,8 +292,8 @@ def to_dict(self) -> Dict[str, Any]:
 class TestSSlibKey(unittest.TestCase):
     """SSlibKey tests."""
 
-    def test_from_pem(self):
-        """Test load PEM/subjectPublicKeyInfo for each SSlibKey keytype"""
+    def test_from_crypto(self):
+        """Test load pyca/cryptography public key for each SSlibKey keytype"""
         test_data = [
             (
                 "rsa",
@@ -312,19 +315,21 @@ def test_from_pem(self):
         def _from_file(path):
             with open(path, "rb") as f:
                 pem = f.read()
-            return pem
+
+            crypto_key = load_pem_public_key(pem)
+            return crypto_key
 
         for keytype, default_scheme, default_keyid in test_data:
-            pem = _from_file(PEMS_DIR / f"{keytype}_public.pem")
-            key = SSlibKey.from_pem(pem)
+            crypto_key = _from_file(PEMS_DIR / f"{keytype}_public.pem")
+            key = SSlibKey.from_crypto(crypto_key)
             self.assertEqual(key.keytype, keytype)
             self.assertEqual(key.scheme, default_scheme)
             self.assertEqual(key.keyid, default_keyid)
 
         # Test with non-default scheme/keyid
-        pem = _from_file(PEMS_DIR / "rsa_public.pem")
-        key = SSlibKey.from_pem(
-            pem,
+        crypto_key = _from_file(PEMS_DIR / "rsa_public.pem")
+        key = SSlibKey.from_crypto(
+            crypto_key,
             scheme="rsa-pkcs1v15-sha224",
             keyid="abcdef",
         )