Fix channel id 2 (#1547)

* add missing module exports

* type hints and more docs for websocket.interfaces

* fix transport_channel_id for tls-unique on twisted

* bump dev version

* channel_id_type in sign_challenge is optional, and defaults to None

* reduce ab.wamp.protocol log noise, better excp handling

* use new style string interpolation

* obey websocket connection status when echo'ing in testee

* add type hints to interfaces

* fix module exports

* add type hints

* tighten is_attached/is_connected

* add type hints; some code style fixes

* add more type hints

* add even more type hints

* fix

* more type hints etc

* more type hints; move TransportDetails to autobahn.wamp.types; expand transport details

* fix use of TransportDetails

* default transport_details implementation on all wamp transports; more type hints; allow async onConnect

autobahn/_version.py

@@ -24,6 +24,6 @@
 #
 ###############################################################################
 
-__version__ = '22.4.1.dev4'
+__version__ = '22.4.1.dev5'
 
 __build__ = '00000000-0000000'

autobahn/asyncio/wamp.py

@@ -43,6 +43,8 @@
 from autobahn.websocket.compress import PerMessageDeflateOffer, \
     PerMessageDeflateResponse, PerMessageDeflateResponseAccept
 
+from autobahn.wamp.interfaces import ITransportHandler, ISession
+
 __all__ = (
     'ApplicationSession',
     'ApplicationSessionFactory',
@@ -64,12 +66,18 @@ class ApplicationSession(protocol.ApplicationSession):
     log = txaio.make_logger()
 
 
+ITransportHandler.register(ApplicationSession)
+
+# ISession.register collides with the abc.ABCMeta.register method
+ISession.abc_register(ApplicationSession)
+
+
 class ApplicationSessionFactory(protocol.ApplicationSessionFactory):
     """
     WAMP application session factory for asyncio-based applications.
     """
 
-    session = ApplicationSession
+    session: ApplicationSession = ApplicationSession
     """
     The application session class this application session factory will use.
     Defaults to :class:`autobahn.asyncio.wamp.ApplicationSession`.
@@ -276,6 +284,7 @@ def accept(response):
             loop.close()
 
 
+# new API
 class Session(protocol._SessionShim):
     # XXX these methods are redundant, but put here for possibly
     # better clarity; maybe a bad idea.

autobahn/asyncio/websocket.py

@@ -36,7 +36,7 @@
 from autobahn.asyncio.util import transport_channel_id, peer2str
 from autobahn.wamp import websocket
 from autobahn.websocket import protocol
-from autobahn.websocket.types import TransportDetails
+from autobahn.wamp.types import TransportDetails
 
 __all__ = (
     'WebSocketServerProtocol',
@@ -229,14 +229,20 @@ def _create_transport_details(self):
         Internal helper.
         Base class calls this to create a TransportDetails
         """
+        is_server = False
         is_secure = self.transport.get_extra_info('peercert', None) is not None
         if is_secure:
-            secure_channel_id = {
-                'tls-unique': transport_channel_id(self.transport, False, 'tls-unique'),
+            channel_id = {
+                'tls-unique': transport_channel_id(self.transport, is_server, 'tls-unique'),
             }
+            channel_type = TransportDetails.TRANSPORT_TYPE_TLS_TCP
+            peer_cert = None
         else:
-            secure_channel_id = {}
-        return TransportDetails(peer=self.peer, is_secure=is_secure, secure_channel_id=secure_channel_id)
+            channel_id = {}
+            channel_type = TransportDetails.TRANSPORT_TYPE_TCP
+            peer_cert = None
+        return TransportDetails(channel_type=channel_type, peer=self.peer, is_server=is_server, is_secure=is_secure,
+                                channel_id=channel_id, peer_cert=peer_cert)
 
 
 class WebSocketAdapterFactory(object):

autobahn/twisted/test/test_tx_protocol.py

@@ -30,7 +30,7 @@
 from autobahn.twisted.websocket import WebSocketServerFactory
 from autobahn.twisted.websocket import WebSocketServerProtocol
 from autobahn.twisted.websocket import WebSocketClientProtocol
-from autobahn.websocket.types import TransportDetails
+from autobahn.wamp.types import TransportDetails
 from autobahn.websocket.types import ConnectingRequest
 from twisted.python.failure import Failure
 from twisted.internet.error import ConnectionDone, ConnectionAborted, \
@@ -429,9 +429,10 @@ def onConnecting(self, transport_details):
 
     def test_str_transport(self):
         details = TransportDetails(
+            channel_type=TransportDetails.TRANSPORT_TYPE_FUNCTION,
             peer="example.com",
             is_secure=False,
-            secure_channel_id={},
+            channel_id={},
         )
         # we can str() this and it doesn't fail
         str(details)

autobahn/twisted/util.py

@@ -95,38 +95,48 @@ def peer2str(addr: Union[IPv4Address, IPv6Address, UNIXAddress, PipeAddress]) ->
 
 try:
     from twisted.protocols.tls import TLSMemoryBIOProtocol
+    from OpenSSL.SSL import Connection
 except ImportError:
-    def transport_channel_id(transport: object, is_server: bool, channel_id_type: Optional[str] = None) -> bytes:
+    def transport_channel_id(transport: object, is_server: bool, channel_id_type: Optional[str] = None) -> Optional[bytes]:
         if channel_id_type is None:
             return b'\x00' * 32
+        else:
+            raise RuntimeError('cannot determine TLS channel ID of type "{}" when TLS is not available on this system'.format(channel_id_type))
 else:
-    def transport_channel_id(transport: TLSMemoryBIOProtocol, is_server: bool, channel_id_type: Optional[str] = None) -> bytes:
+    def transport_channel_id(transport: object, is_server: bool, channel_id_type: Optional[str] = None) -> Optional[bytes]:
         """
-        Application-layer user authentication protocols are vulnerable to generic
-        credential forwarding attacks, where an authentication credential sent by
-        a client C to a server M may then be used by M to impersonate C at another
-        server S. To prevent such credential forwarding attacks, modern authentication
-        protocols rely on channel bindings. For example, WAMP-cryptosign can use
-        the tls-unique channel identifier provided by the TLS layer to strongly bind
-        authentication credentials to the underlying channel, so that a credential
-        received on one TLS channel cannot be forwarded on another.
-
-        :param transport: The Twisted TLS transport to extract the TLS channel ID from.
-        :param is_server: Flag indicating the transport is for a server.
-        :param channel_id_type: TLS channel ID type, currently only "tls-unique" is supported.
-        :returns: The TLS channel id (32 bytes).
+        Return TLS channel ID of WAMP transport of the given TLS channel ID type.
+
+        Application-layer user authentication protocols are vulnerable to generic credential forwarding attacks,
+        where an authentication credential sent by a client C to a server M may then be used by M to impersonate C at
+        another server S.
+        To prevent such credential forwarding attacks, modern authentication protocols rely on channel bindings.
+        For example, WAMP-cryptosign can use the tls-unique channel identifier provided by the TLS layer to strongly
+        bind authentication credentials to the underlying channel, so that a credential received on one TLS channel
+        cannot be forwarded on another.
+
+        :param transport: The Twisted TLS transport to extract the TLS channel ID from. If the transport isn't
+            TLS based, and non-empty ``channel_id_type`` is requested, ``None`` will be returned. If the transport
+            is indeed TLS based, an empty ``channel_id_type`` of ``None`` is requested, 32 NUL bytes will be returned.
+        :param is_server: Flag indicating that the transport is a server transport.
+        :param channel_id_type: TLS channel ID type, if set currently only ``"tls-unique"`` is supported.
+        :returns: The TLS channel ID (32 bytes).
         """
         if channel_id_type is None:
             return b'\x00' * 32
 
         if channel_id_type not in ['tls-unique']:
-            raise Exception("invalid channel ID type {}".format(channel_id_type))
+            raise RuntimeError('invalid TLS channel ID type "{}" requested'.format(channel_id_type))
 
-        # transport:                instance of :class:`twisted.protocols.tls.TLSMemoryBIOProtocol`
-        # transport._tlsConnection: instance of :class:`OpenSSL.SSL.Connection`
-        if not hasattr(transport, '_tlsConnection'):
-            print("TLS transport channel_id for tls-unique requested, but _tlsConnection not found on transport {}".format(dir(transport)))
-            return b'\x00' * 32
+        if not isinstance(transport, TLSMemoryBIOProtocol):
+            raise RuntimeError(
+                'cannot determine TLS channel ID of type "{}" when TLS is not available on this transport {}'.format(
+                    channel_id_type, type(transport)))
+
+        # get access to the OpenSSL connection underlying the Twisted protocol
+        # https://twistedmatrix.com/documents/current/api/twisted.protocols.tls.TLSMemoryBIOProtocol.html#getHandle
+        connection: Connection = transport.getHandle()
+        assert connection and isinstance(connection, Connection)
 
         # Obtain latest TLS Finished message that we expected from peer, or None if handshake is not completed.
         # http://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Connection.get_peer_finished
@@ -137,11 +147,11 @@ def transport_channel_id(transport: TLSMemoryBIOProtocol, is_server: bool, chann
             if is_server != is_not_resumed:
                 # for routers (=servers) XOR new sessions, the channel ID is based on the TLS Finished message we
                 # expected to receive from the client
-                tls_finished_msg = transport._tlsConnection.get_peer_finished()
+                tls_finished_msg = connection.get_peer_finished()
             else:
                 # for clients XOR resumed sessions, the channel ID is based on the TLS Finished message we sent
                 # to the router (=server)
-                tls_finished_msg = transport._tlsConnection.get_finished()
+                tls_finished_msg = connection.get_finished()
 
             if tls_finished_msg is None:
                 # this can occur if we made a successful connection (in a
@@ -153,4 +163,4 @@ def transport_channel_id(transport: TLSMemoryBIOProtocol, is_server: bool, chann
                 m.update(tls_finished_msg)
                 return m.digest()
         else:
-            assert False, 'should not arrive here'
+            raise NotImplementedError('should not arrive here (unhandled channel_id_type "{}")'.format(channel_id_type))

autobahn/twisted/wamp.py

@@ -46,10 +46,9 @@
     PerMessageDeflateResponse, PerMessageDeflateResponseAccept
 
 from autobahn.wamp import protocol, auth
-from autobahn.wamp.interfaces import IAuthenticator
+from autobahn.wamp.interfaces import ITransportHandler, ISession, IAuthenticator
 from autobahn.wamp.types import ComponentConfig
 
-
 __all__ = [
     'ApplicationSession',
     'ApplicationSessionFactory',
@@ -77,12 +76,18 @@ class ApplicationSession(protocol.ApplicationSession):
     log = txaio.make_logger()
 
 
+ITransportHandler.register(ApplicationSession)
+
+# ISession.register collides with the abc.ABCMeta.register method
+ISession.abc_register(ApplicationSession)
+
+
 class ApplicationSessionFactory(protocol.ApplicationSessionFactory):
     """
     WAMP application session factory for Twisted-based applications.
     """
 
-    session = ApplicationSession
+    session: ApplicationSession = ApplicationSession
     """
     The application session class this application session factory will use. Defaults to :class:`autobahn.twisted.wamp.ApplicationSession`.
     """

autobahn/twisted/websocket.py

@@ -42,11 +42,12 @@
 from autobahn.util import public
 from autobahn.util import _is_tls_error, _maybe_tls_reason
 from autobahn.wamp import websocket
-from autobahn.websocket.types import ConnectionRequest, ConnectionResponse, ConnectionDeny, \
-    TransportDetails
+from autobahn.websocket.types import ConnectionRequest, ConnectionResponse, ConnectionDeny
+from autobahn.wamp.types import TransportDetails
 from autobahn.websocket import protocol
 from autobahn.websocket.interfaces import IWebSocketClientAgent
 from autobahn.twisted.util import peer2str, transport_channel_id
+from autobahn.xbr._util import hltype
 
 from autobahn.websocket.compress import PerMessageDeflateOffer, \
     PerMessageDeflateOfferAccept, \
@@ -374,8 +375,9 @@ class WebSocketClientProtocol(WebSocketAdapterProtocol, protocol.WebSocketClient
 
     log = txaio.make_logger()
 
-    def _onConnect(self, response):
-        self.onConnect(response)
+    def _onConnect(self, response: ConnectionResponse):
+        self.log.debug('{meth}(response={response})', meth=hltype(self._onConnect), response=response)
+        return self.onConnect(response)
 
     def startTLS(self):
         self.log.debug("Starting TLS upgrade")
@@ -398,14 +400,20 @@ def _create_transport_details(self):
         # ISSLTransport at that point according to Twisted
         # documentation
         # the peer we are connected to
+        is_server = False
         is_secure = ISSLTransport.providedBy(self.transport)
         if is_secure:
-            secure_channel_id = {
-                'tls-unique': transport_channel_id(self.transport, False, 'tls-unique'),
+            channel_id = {
+                'tls-unique': transport_channel_id(self.transport, is_server, 'tls-unique'),
             }
+            channel_type = TransportDetails.TRANSPORT_TYPE_TLS_TCP
+            peer_cert = None
         else:
-            secure_channel_id = {}
-        return TransportDetails(peer=self.peer, is_secure=is_secure, secure_channel_id=secure_channel_id)
+            channel_id = {}
+            channel_type = TransportDetails.TRANSPORT_TYPE_TCP
+            peer_cert = None
+        return TransportDetails(channel_type=channel_type, peer=self.peer, is_server=is_server, is_secure=is_secure,
+                                channel_id=channel_id, peer_cert=peer_cert)
 
 
 class WebSocketAdapterFactory(object):

autobahn/wamp/cryptosign.py

@@ -447,7 +447,7 @@ def can_sign(self):
             return self._can_sign
 
         @util.public
-        def sign_challenge(self, session: ISession, challenge: Challenge, channel_id_type: str = 'tls-unique') -> bytes:
+        def sign_challenge(self, session: ISession, challenge: Challenge, channel_id_type: Optional[str] = None) -> bytes:
             """
             Sign WAMP-cryptosign challenge.
 
@@ -462,6 +462,7 @@ def sign_challenge(self, session: ISession, challenge: Challenge, channel_id_typ
             """
             # get the TLS channel ID of the underlying TLS connection. Could be None.
             channel_id_raw = session._transport.get_channel_id(channel_id_type)
+
             data = format_challenge(challenge, channel_id_raw, channel_id_type)
 
             return sign_challenge(data, self.sign)
@@ -627,28 +628,3 @@ def from_ssh_data(cls, keydata: str) -> 'SigningKey':
             return cls(key, comment)
 
     IEd25519Key.register(SigningKey)
-
-
-if __name__ == '__main__':
-    import sys
-    if not HAS_CRYPTOSIGN:
-        print('NaCl library must be installed for this to function.', file=sys.stderr)
-        sys.exit(1)
-
-    from optparse import OptionParser
-
-    parser = OptionParser()
-    parser.add_option('-f', '--file', dest='keyfile',
-                      help='file containing ssh key')
-    parser.add_option('-p', action='store_true', dest='printpub', default=False,
-                      help='print public key information')
-
-    options, args = parser.parse_args()
-
-    if not options.printpub:
-        print("Print public key must be specified as it's the only option.")
-        parser.print_usage()
-        sys.exit(1)
-
-    key = SigningKey.from_ssh_key(options.keyfile)
-    print(key.public_key())

autobahn/wamp/exception.py

@@ -30,6 +30,7 @@
 
 __all__ = (
     'Error',
+    'TypeCheckError',
     'SessionNotReady',
     'SerializationError',
     'ProtocolError',