refactor SigningKey class for reusability (#1500)

* WIP: refactor SigningKey class

* cleanup interface

* fix flake8

* flake8 ignore N818

* fix sign method return type in ifc

* refactor challenge formatting code

autobahn/wamp/cryptosign.py

@@ -30,6 +30,7 @@
 import txaio
 
 from autobahn import util
+from autobahn.wamp.interfaces import ISigningKey
 from autobahn.wamp.types import Challenge
 
 __all__ = [
@@ -353,32 +354,49 @@ def _verify_signify_ed25519_signature(pubkey_file, signature_file, message):
 
 if HAS_CRYPTOSIGN:
 
-    @util.public
-    class SigningKey(object):
-        """
-        A cryptosign private key for signing, and hence usable for authentication or a
-        public key usable for verification (but can't be used for signing).
-        """
+    def format_challenge(session, challenge, channel_id_type) -> bytes:
+        if not isinstance(challenge, Challenge):
+            raise Exception(
+                "challenge must be instance of autobahn.wamp.types.Challenge, not {}".format(type(challenge)))
 
-        def __init__(self, key, comment=None):
-            """
+        if 'challenge' not in challenge.extra:
+            raise Exception("missing challenge value in challenge.extra")
 
-            :param key: A Ed25519 private signing key or a Ed25519 public verification key.
-            :type key: instance of nacl.signing.VerifyKey or instance of nacl.signing.SigningKey
-            """
-            if not (isinstance(key, signing.VerifyKey) or isinstance(key, signing.SigningKey)):
-                raise Exception("invalid type {} for key".format(type(key)))
+        # the challenge sent by the router (a 32 bytes random value)
+        challenge_hex = challenge.extra['challenge']
 
-            if not (comment is None or type(comment) == str):
-                raise Exception("invalid type {} for comment".format(type(comment)))
+        if type(challenge_hex) != str:
+            raise Exception("invalid type {} for challenge (expected a hex string)".format(type(challenge_hex)))
 
-            self._key = key
-            self._comment = comment
-            self._can_sign = isinstance(key, signing.SigningKey)
+        if len(challenge_hex) != 64:
+            raise Exception("unexpected challenge (hex) length: was {}, but expected 64".format(len(challenge_hex)))
 
-        def __str__(self):
-            comment = '"{}"'.format(self.comment()) if self.comment() else None
-            return 'Key(can_sign={}, comment={}, public_key={})'.format(self.can_sign(), comment, self.public_key())
+        # the challenge for WAMP-cryptosign is a 32 bytes random value in Hex encoding (that is, a unicode string)
+        challenge_raw = binascii.a2b_hex(challenge_hex)
+
+        if channel_id_type == 'tls-unique':
+            # get the TLS channel ID of the underlying TLS connection
+            channel_id_raw = session._transport.get_channel_id()
+            assert len(
+                channel_id_raw) == 32, 'unexpected TLS transport channel ID length (was {}, but expected 32)'.format(
+                len(channel_id_raw))
+
+            # with TLS channel binding of type "tls-unique", the message to be signed by the client actually
+            # is the XOR of the challenge and the TLS channel ID
+            data = util.xor(challenge_raw, channel_id_raw)
+        elif channel_id_type is None:
+            # when no channel binding was requested, the message to be signed by the client is the challenge only
+            data = challenge_raw
+        else:
+            assert False, 'invalid channel_id_type "{}"'.format(channel_id_type)
+
+        return data
+
+    class SigningKeyBase(object):
+
+        def __init__(self, signer, can_sign: bool) -> None:
+            self._can_sign = can_sign
+            self._key = signer
 
         @util.public
         def can_sign(self):
@@ -391,32 +409,41 @@ def can_sign(self):
             return self._can_sign
 
         @util.public
-        def comment(self):
+        def sign_challenge(self, session, challenge, channel_id_type='tls-unique'):
             """
-            Get the key comment (if any).
+            Sign WAMP-cryptosign challenge.
 
-            :returns: The comment (if any) from the key.
-            :rtype: str or None
-            """
-            return self._comment
+            :param session: The authenticating WAMP session.
+            :type session: :class:`autobahn.wamp.protocol.ApplicationSession`
 
-        @util.public
-        def public_key(self, binary=False):
-            """
-            Returns the public key part of a signing key or the (public) verification key.
+            :param challenge: The WAMP-cryptosign challenge object for which a signature should be computed.
+            :type challenge: instance of autobahn.wamp.types.Challenge
 
-            :returns: The public key in Hex encoding.
-            :rtype: str or None
+            :returns: A Deferred/Future that resolves to the computed signature.
+            :rtype: str
             """
-            if isinstance(self._key, signing.SigningKey):
-                key = self._key.verify_key
-            else:
-                key = self._key
+            data = format_challenge(session, challenge, channel_id_type)
 
-            if binary:
-                return key.encode()
-            else:
-                return key.encode(encoder=encoding.HexEncoder).decode('ascii')
+            # a raw byte string is signed, and the signature is also a raw byte string
+            d1 = self.sign(data)
+
+            # asyncio lacks callback chaining (and we cannot use co-routines, since we want
+            # to support older Pythons), hence we need d2
+            d2 = txaio.create_future()
+
+            def process(signature_raw):
+                # convert the raw signature into a hex encode value (unicode string)
+                signature_hex = binascii.b2a_hex(signature_raw).decode('ascii')
+
+                # we return the concatenation of the signature and the message signed (96 bytes)
+                data_hex = binascii.b2a_hex(data).decode('ascii')
+
+                sig = signature_hex + data_hex
+                txaio.resolve(d2, sig)
+
+            txaio.add_callbacks(d1, process, None)
+
+            return d2
 
         @util.public
         def sign(self, data):
@@ -444,72 +471,62 @@ def sign(self, data):
             # the signature
             return txaio.create_future_success(sig.signature)
 
-        @util.public
-        def sign_challenge(self, session, challenge, channel_id_type='tls-unique'):
-            """
-            Sign WAMP-cryptosign challenge.
-
-            :param session: The authenticating WAMP session.
-            :type session: :class:`autobahn.wamp.protocol.ApplicationSession`
-
-            :param challenge: The WAMP-cryptosign challenge object for which a signature should be computed.
-            :type challenge: instance of autobahn.wamp.types.Challenge
+    @util.public
+    class SigningKey(SigningKeyBase):
+        """
+        A cryptosign private key for signing, and hence usable for authentication or a
+        public key usable for verification (but can't be used for signing).
+        """
 
-            :returns: A Deferred/Future that resolves to the computed signature.
-            :rtype: str
+        def __init__(self, key, comment=None):
             """
-            if not isinstance(challenge, Challenge):
-                raise Exception("challenge must be instance of autobahn.wamp.types.Challenge, not {}".format(type(challenge)))
-
-            if 'challenge' not in challenge.extra:
-                raise Exception("missing challenge value in challenge.extra")
-
-            # the challenge sent by the router (a 32 bytes random value)
-            challenge_hex = challenge.extra['challenge']
 
-            if type(challenge_hex) != str:
-                raise Exception("invalid type {} for challenge (expected a hex string)".format(type(challenge_hex)))
-
-            if len(challenge_hex) != 64:
-                raise Exception("unexpected challenge (hex) length: was {}, but expected 64".format(len(challenge_hex)))
-
-            # the challenge for WAMP-cryptosign is a 32 bytes random value in Hex encoding (that is, a unicode string)
-            challenge_raw = binascii.a2b_hex(challenge_hex)
+            :param key: A Ed25519 private signing key or a Ed25519 public verification key.
+            :type key: instance of nacl.signing.VerifyKey or instance of nacl.signing.SigningKey
+            """
+            if not (isinstance(key, signing.VerifyKey) or isinstance(key, signing.SigningKey)):
+                raise Exception("invalid type {} for key".format(type(key)))
 
-            if channel_id_type == 'tls-unique':
-                # get the TLS channel ID of the underlying TLS connection
-                channel_id_raw = session._transport.get_channel_id()
-                assert len(channel_id_raw) == 32, 'unexpected TLS transport channel ID length (was {}, but expected 32)'.format(len(channel_id_raw))
+            if not (comment is None or type(comment) == str):
+                raise Exception("invalid type {} for comment".format(type(comment)))
 
-                # with TLS channel binding of type "tls-unique", the message to be signed by the client actually
-                # is the XOR of the challenge and the TLS channel ID
-                data = util.xor(challenge_raw, channel_id_raw)
-            elif channel_id_type is None:
-                # when no channel binding was requested, the message to be signed by the client is the challenge only
-                data = challenge_raw
-            else:
-                assert False, 'invalid channel_id_type "{}"'.format(channel_id_type)
+            self._key = key
+            self._comment = comment
+            can_sign = isinstance(key, signing.SigningKey)
 
-            # a raw byte string is signed, and the signature is also a raw byte string
-            d1 = self.sign(data)
+            super().__init__(key, can_sign)
 
-            # asyncio lacks callback chaining (and we cannot use co-routines, since we want
-            # to support older Pythons), hence we need d2
-            d2 = txaio.create_future()
+        def __str__(self):
+            comment = '"{}"'.format(self.comment()) if self.comment() else None
+            return 'Key(can_sign={}, comment={}, public_key={})'.format(self.can_sign(), comment, self.public_key())
 
-            def process(signature_raw):
-                # convert the raw signature into a hex encode value (unicode string)
-                signature_hex = binascii.b2a_hex(signature_raw).decode('ascii')
+        @util.public
+        def comment(self):
+            """
+            Get the key comment (if any).
 
-                # we return the concatenation of the signature and the message signed (96 bytes)
-                data_hex = binascii.b2a_hex(data).decode('ascii')
+            :returns: The comment (if any) from the key.
+            :rtype: str or None
+            """
+            return self._comment
 
-                sig = signature_hex + data_hex
-                txaio.resolve(d2, sig)
+        @util.public
+        def public_key(self, binary=False):
+            """
+            Returns the public key part of a signing key or the (public) verification key.
 
-            txaio.add_callbacks(d1, process, None)
+            :returns: The public key in Hex encoding.
+            :rtype: str or None
+            """
+            if isinstance(self._key, signing.SigningKey):
+                key = self._key.verify_key
+            else:
+                key = self._key
 
-            return d2
+            if binary:
+                return key.encode()
+            else:
+                return key.encode(encoder=encoding.HexEncoder).decode('ascii')
 
         @util.public
         @classmethod
@@ -587,6 +604,9 @@ def from_ssh_data(cls, keydata):
 
             return cls(key, comment)
 
+    ISigningKey.register(SigningKey)
+
+
 if __name__ == '__main__':
     import sys
     if not HAS_CRYPTOSIGN:

autobahn/wamp/interfaces.py

@@ -25,6 +25,7 @@
 ###############################################################################
 
 import abc
+from typing import Union
 
 from autobahn.util import public
 
@@ -35,7 +36,8 @@
     'ITransport',
     'ITransportHandler',
     'ISession',
-    'IPayloadCodec'
+    'IPayloadCodec',
+    'ISigningKey'
 )
 
 
@@ -716,6 +718,40 @@ def on_welcome(self, authextra):
         """
 
 
+@public
+class ISigningKey(abc.ABC):
+
+    @abc.abstractmethod
+    def can_sign(self):
+        """
+        Check if the key can be used to sign.
+
+        :returns: `True`, iff the key can sign.
+        :rtype: bool
+        """
+
+    @abc.abstractmethod
+    def public_key(self, binary=False) -> Union[str, bytes]:
+        """
+        Returns the public key part of a signing key or the (public) verification key.
+
+        :param binary: If the return type should be binary instead of hex
+        :return: The public key in hex or byte encoding.
+        """
+
+    @abc.abstractmethod
+    def sign(self, data: bytes):
+        """
+        Sign the given data.
+
+        :param data: The data to be signed.
+        :type data: bytes
+
+        :returns: The signature.
+        :rtype: txaio.Future object that resolves to bytes
+        """
+
+
 @public
 class IPayloadCodec(abc.ABC):
     """

tox.ini

@@ -120,7 +120,7 @@ commands =
     python -V
     flake8 --version
     flake8 -v --statistics \
-        --ignore=E402,E501,E722,E741,N801,N802,N803,N805,N806,N815 \
+        --ignore=E402,E501,E722,E741,N801,N802,N803,N805,N806,N815,N818 \
         --exclude "autobahn/wamp/message_fbs.py,autobahn/wamp/gen/*" \
         autobahn