refactor CryptoSigner (WIP)

Inheritance CrytoSigner -> _{RSA,...}Signer does not make sense. We want to just create CryptoSigner by throwing a pyca private key at it. Composition, seems to be better, if at all. The nesting now gets a bit deep. SslibSigner._crypto_signer._signer._private_key (sslibsigner will go away, but still) Is CryptoSigner._private_key enough? Yes, and we'd have to - do scheme check to original dispatch in init (okay), - case handle in sign (okay), and - either, - re-parse additional sign params from scheme on each sign (not nice), - or store different additional sign parameters (this could work) class SignArgs: class RSAArgs - padding - hash_algorithm class ECDSAArgs - signature_algorithm Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh · Nov 14, 2023 · c8e3165 · c8e3165
1 parent f4bea04
commit c8e3165
Showing 1 changed file with 52 additions and 60 deletions.
diff --git a/securesystemslib/signer/_crypto_signer.py b/securesystemslib/signer/_crypto_signer.py
@@ -1,7 +1,6 @@
 """Signer implementation for pyca/cryptography signing. """
 
 import logging
-from abc import ABCMeta
 from typing import Any, Dict, Optional, cast
 from urllib import parse
 
@@ -53,17 +52,42 @@
 logger = logging.getLogger(__name__)
 
 
-class CryptoSigner(Signer, metaclass=ABCMeta):
-    """Base class for PYCA/cryptography Signer implementations."""
+class CryptoSigner(Signer):
+    """PYCA/cryptography Signer implementations."""
 
     FILE_URI_SCHEME = "file"
 
-    def __init__(self, public_key: SSlibKey):
+    def __init__(
+        self,
+        private_key: "PrivateKeyTypes",
+        public_key: Optional[SSlibKey] = None,
+    ):
         if CRYPTO_IMPORT_ERROR:
             raise UnsupportedLibraryError(CRYPTO_IMPORT_ERROR)
 
+        if public_key is None:
+            public_key = SSlibKey._from_crypto_public_key(
+                private_key.public_key(), None, None
+            )
+
         self.public_key = public_key
 
+        if public_key.keytype == "rsa":
+            self._signer = _RSASigner(
+                public_key, cast(RSAPrivateKey, private_key)
+            )
+
+        elif public_key.keytype == "ecdsa":
+            self._signer = _ECDSASigner(
+                public_key, cast(EllipticCurvePrivateKey, private_key)
+            )
+        elif public_key.keytype == "ed25519":
+            self._signer = _Ed25519Signer(
+                public_key, cast(Ed25519PrivateKey, private_key)
+            )
+        else:
+            raise ValueError(f"unsupported keytype {public_key.keytype}")
+
     @classmethod
     def from_securesystemslib_key(
         cls, key_dict: Dict[str, Any]
@@ -73,49 +97,18 @@ def from_securesystemslib_key(
         public_key = SSlibKey.from_securesystemslib_key(key_dict)
 
         private_key: PrivateKeyTypes
-        if public_key.keytype == "rsa":
-            private_key = cast(
-                RSAPrivateKey,
-                load_pem_private_key(private.encode(), password=None),
-            )
-            return _RSASigner(public_key, private_key)
-
-        if public_key.keytype == "ecdsa":
-            private_key = cast(
-                EllipticCurvePrivateKey,
-                load_pem_private_key(private.encode(), password=None),
-            )
-            return _ECDSASigner(public_key, private_key)
+        if public_key.keytype in ["rsa", "ecdsa"]:
+            private_key = load_pem_private_key(private.encode(), password=None)
 
-        if public_key.keytype == "ed25519":
+        elif public_key.keytype == "ed25519":
             private_key = Ed25519PrivateKey.from_private_bytes(
                 bytes.fromhex(private)
             )
-            return _Ed25519Signer(public_key, private_key)
-
-        raise ValueError(f"unsupported keytype: {public_key.keytype}")
-
-    @classmethod
-    def _from_pem(
-        cls, private_pem: bytes, secret: Optional[bytes], public_key: SSlibKey
-    ):
-        """Helper factory to create CryptoSigner from private PEM."""
-        private_key = load_pem_private_key(private_pem, secret)
-
-        if public_key.keytype == "rsa":
-            return _RSASigner(public_key, cast(RSAPrivateKey, private_key))
-
-        if public_key.keytype == "ecdsa":
-            return _ECDSASigner(
-                public_key, cast(EllipticCurvePrivateKey, private_key)
-            )
 
-        if public_key.keytype == "ed25519":
-            return _Ed25519Signer(
-                public_key, cast(Ed25519PrivateKey, private_key)
-            )
+        else:
+            raise ValueError(f"unsupported keytype: {public_key.keytype}")
 
-        raise ValueError(f"unsupported keytype: {public_key.keytype}")
+        return CryptoSigner(private_key, public_key)
 
     @classmethod
     def from_priv_key_uri(
@@ -167,7 +160,8 @@ def from_priv_key_uri(
         with open(uri.path, "rb") as f:
             private_pem = f.read()
 
-        return cls._from_pem(private_pem, secret, public_key)
+        private_key = load_pem_private_key(private_pem, secret)
+        return CryptoSigner(private_key, public_key)
 
     @staticmethod
     def generate_ed25519(
@@ -191,7 +185,7 @@ def generate_ed25519(
         public_key = SSlibKey._from_crypto_public_key(  # pylint: disable=protected-access
             private_key.public_key(), keyid, "ed25519"
         )
-        return _Ed25519Signer(public_key, private_key)
+        return CryptoSigner(private_key, public_key)
 
     @staticmethod
     def generate_rsa(
@@ -222,7 +216,7 @@ def generate_rsa(
         public_key = SSlibKey._from_crypto_public_key(  # pylint: disable=protected-access
             private_key.public_key(), keyid, scheme
         )
-        return _RSASigner(public_key, private_key)
+        return CryptoSigner(private_key, public_key)
 
     @staticmethod
     def generate_ecdsa(
@@ -246,10 +240,14 @@ def generate_ecdsa(
         public_key = SSlibKey._from_crypto_public_key(  # pylint: disable=protected-access
             private_key.public_key(), keyid, "ecdsa-sha2-nistp256"
         )
-        return _ECDSASigner(public_key, private_key)
+        return CryptoSigner(private_key, public_key)
+
+    def sign(self, payload: bytes) -> Signature:
+        raw_signature = self._signer.sign(payload)
+        return Signature(self.public_key.keyid, raw_signature.hex())
 
 
-class _RSASigner(CryptoSigner):
+class _RSASigner:
     """Internal pyca/cryptography rsa signer implementation"""
 
     def __init__(self, public_key: SSlibKey, private_key: "RSAPrivateKey"):
@@ -265,7 +263,6 @@ def __init__(self, public_key: SSlibKey, private_key: "RSAPrivateKey"):
         ]:
             raise ValueError(f"unsupported scheme {public_key.scheme}")
 
-        super().__init__(public_key)
         self._private_key = private_key
         padding_name, hash_name = public_key.scheme.split("-")[1:]
         self._algorithm = self._get_hash_algorithm(hash_name)
@@ -302,12 +299,11 @@ def _get_rsa_padding(
 
         return padding
 
-    def sign(self, payload: bytes) -> Signature:
-        sig = self._private_key.sign(payload, self._padding, self._algorithm)
-        return Signature(self.public_key.keyid, sig.hex())
+    def sign(self, payload: bytes) -> bytes:
+        return self._private_key.sign(payload, self._padding, self._algorithm)
 
 
-class _ECDSASigner(CryptoSigner):
+class _ECDSASigner:
     """Internal pyca/cryptography ecdsa signer implementation"""
 
     def __init__(
@@ -316,25 +312,21 @@ def __init__(
         if public_key.scheme != "ecdsa-sha2-nistp256":
             raise ValueError(f"unsupported scheme {public_key.scheme}")
 
-        super().__init__(public_key)
         self._private_key = private_key
         self._signature_algorithm = ECDSA(SHA256())
 
-    def sign(self, payload: bytes) -> Signature:
-        sig = self._private_key.sign(payload, self._signature_algorithm)
-        return Signature(self.public_key.keyid, sig.hex())
+    def sign(self, payload: bytes) -> bytes:
+        return self._private_key.sign(payload, self._signature_algorithm)
 
 
-class _Ed25519Signer(CryptoSigner):
+class _Ed25519Signer:
     """Internal pyca/cryptography ecdsa signer implementation"""
 
     def __init__(self, public_key: SSlibKey, private_key: "Ed25519PrivateKey"):
         if public_key.scheme != "ed25519":
             raise ValueError(f"unsupported scheme {public_key.scheme}")
 
-        super().__init__(public_key)
         self._private_key = private_key
 
-    def sign(self, payload: bytes) -> Signature:
-        sig = self._private_key.sign(payload)
-        return Signature(self.public_key.keyid, sig.hex())
+    def sign(self, payload: bytes) -> bytes:
+        return self._private_key.sign(payload)