rlinks: Allow custom cryptosign key

[om26er]

Currently the cryptosign key used for router to router links is of the Crossbar node. This should be configurable. When a router link is being started, it should be possible to pass private key hex.

[oberstet]

yes, this is by design:

• the identity of the node at the WAMP rlink authentication level is via the node key (there is exactly 1 per node, always)
• the identity of the node at the cross-domain (inter-operator) rlink level is via an Ethereum private key - pls see page 2 here https://github.com/wamp-proto/wamp-proto/blob/457b92a47e2615d4cdf67dee07d82eceb054bb94/wep/wep002/wamp_ap_r2r.pdf

I've marked the issue as "for discussion" nevertheless. Rgd the requirements I had in mind, a good question to start with is:

• Say you operate node-1, node-2 and node-3, and want to establish a full mesh of rlinks. all nodes are managed as one logical units (as eg in crossbar master). the pubkeys accepted and used should be tied to the nodes - otherwise it'll lead to potentially quadratic number of keys .. and why (for what gain)?
• Say someone else operates node-4 and you want to establish an rlink from each of your nodes (or any one) to that node-4. How do you know the pubkey of node-4 to accept, how does node-4 know the pubkeys of your nodes?

[om26er]

One scenario for that is the Edge, where the private key of the hardware (through secure element) is it's identity. In such scenarios it can be useful to have requested flexibility.

[oberstet]

no, private keys should not be part of the rlink api. and it would not work like that anyways for hardware wallets in my view:

• if the private key is in a hardware wallet, exporting that key is usually impossible, and importing the private key (even if it would be known) is wrong anyways, since the hardware wallet should sign
• hence, what we need is support for signing by hardware wallet in autobahn

[oberstet]

hardware signing can be added to CB via a new (sub)class for SigningKey

https://github.com/crossbario/autobahn-python/blob/a35f22eeaafca7568f1deb35c4a1b82ae78f77d4/autobahn/wamp/cryptosign.py#L357

[om26er]

@oberstet So there are two scenarios that I can think of

1. A real hardware wallet (for production)
2. A mock hardware wallet (for development and testing. This basically incorporates a private key hex)

Once we have something like that in place in autobahn. I guess CB would need to be changed somehow.

Would it make sense for CB to optionally support setting the "signing key" object on the controller ?

Below is what I am thinking

diff --git a/crossbar/worker/rlink.py b/crossbar/worker/rlink.py
index 4da87647..104a7820 100644
--- a/crossbar/worker/rlink.py
+++ b/crossbar/worker/rlink.py
@@ -14,6 +14,7 @@ from twisted.internet.defer import Deferred, inlineCallbacks

 from autobahn import util
 from autobahn.wamp.types import SessionIdent
+from autobahn.wamp.cryptosign import SigningKey

 from crossbar._util import hl, hlid, hltype, hluserid
 from crossbar.common.checkconfig import check_dict_args, check_realm_name, check_connecting_transport
@@ -492,10 +493,14 @@ class RLinkRemoteSession(BridgeSession):
     # directory in which events are flowing (published via this session
     DIR = hl('from local to remote', color='yellow', bold=True)

-    def __init__(self, config):
+    def __init__(self, config, signing_key: SigningKey = None):
         BridgeSession.__init__(self, config)
         self._subs = {}
         self._rlink_manager = self.config.extra['rlink_manager']
+        if signing_key is not None:
+            self._signing_key = signing_key
+        else:
+            self._signing_key = self._rlink_manager._controller._node_key

     def onConnect(self):
         self.log.debug('{klass}.onConnect()', klass=self.__class__.__name__)
@@ -508,7 +513,7 @@ class RLinkRemoteSession(BridgeSession):
         authextra.update({
             # forward the client pubkey: this allows us to omit authid as
             # the router can identify us with the pubkey already
-            'pubkey': self._rlink_manager._controller._node_key.public_key(),
+            'pubkey': self._signing_key.public_key(),

             # not yet implemented. a public key the router should provide
             # a trustchain for it's public key. the trustroot can eg be
@@ -550,7 +555,7 @@ class RLinkRemoteSession(BridgeSession):
             # is fine - the router is authentic wrt our trustroot.

             # sign the challenge with our private key.
-            signed_challenge = self._rlink_manager._controller._node_key.sign_challenge(self, challenge)
+            signed_challenge = self._signing_key.sign_challenge(self, challenge)

             # send back the signed challenge for verification
             return signed_challenge
@@ -796,7 +801,7 @@ class RLinkManager(object):
         return self._links.keys()

     @inlineCallbacks
-    def start_link(self, link_id, link_config, caller):
+    def start_link(self, link_id, link_config, caller, signing_key=None):
         assert type(link_id) == str
         assert isinstance(link_config, RLinkConfig)
         assert isinstance(caller, SessionIdent)
@@ -833,7 +838,7 @@ class RLinkManager(object):
         }
         remote_realm = link_config.realm
         remote_config = ComponentConfig(remote_realm, remote_extra)
-        remote_session = RLinkRemoteSession(remote_config)
+        remote_session = RLinkRemoteSession(remote_config, signing_key)

         # cross-connect the two sessions
         #
diff --git a/crossbar/worker/router.py b/crossbar/worker/router.py
index 394f6668..209f0d51 100644
--- a/crossbar/worker/router.py
+++ b/crossbar/worker/router.py
@@ -71,6 +71,8 @@ class RouterController(TransportController):
         # map: transport ID -> RouterTransport
         self.transports = {}

+        self._signing_key = None
+
     def realm_by_name(self, name):
         realm_id = self.realm_to_id.get(name, None)
         assert (realm_id in self.realms)
@@ -450,6 +452,9 @@ class RouterController(TransportController):
                        authrole=hlid(authrole))
         return succeed(session)

+    def set_rlink_signing_key(self, signing_key):
+        self._signing_key = signing_key
+
     @wamp.register(None)
     def get_router_realm_roles(self, realm_id, details=None):
         """
@@ -1143,7 +1148,7 @@ class RouterController(TransportController):
                                        'router link {} already running'.format(link_id))
             link_config = RLinkConfig.parse(self.personality, link_config, id=link_id)
             caller = SessionIdent.from_calldetails(details)
-            rlink = yield rlink_manager.start_link(link_id, link_config, caller)
+            rlink = yield rlink_manager.start_link(link_id, link_config, caller, self._signing_key)
             started = rlink.marshal()
         except:
             self.log.failure()

[om26er]

The above basically show that before starting the rlink, the set_rlink_signing_key method could be called on the controller. The SigningKey object can be of a hardware wallet, a mock wallet or None

[om26er]

I guess there might be better places to do that, that I don't have in mind.

[oberstet]

@om26er thanks for the suggestion above! it doesn't fit into the design though: all rlinks must use exactly 1 private key, and that key must be the node key. all remote pub keys accepted by rlinks (not yet implemented, that is use of server cert checking in wamp-cryptosign in CB) must use a common database.

the solution is a change in AB. once we have that, the needed change in CB is make the node private key to either use a file-based priv key or a HW-based priv key or a dummy-whatever-based priv key. the selection of the respective AB class is then controlled from node config. so we need new config in CB first. eg how to configure use of HW node key? etc

one way to structure this work .. the AB parts .. would be 2 new issues on AB:

1. add proper interface abstraction for SigningKey; refactor existing file-based implementation into 1 impl. of the former interface
2. add HW based implementation (eg for the NXP chip using the python lib for that) as another impl. of said interface

[oberstet]

rgd the interface: obviously, the essential method that is called in CB would be SigningKey.sign_challenge. so another HW based impl. will provide its own impl. of that interface method. the methods to read the priv key from a file in the current impl. is indeed called in CB of course, but that should not be part of the interface - as it is specific to the impl .. that is file-based vs HW-based

[oberstet]

A real hardware wallet (for production) A mock hardware wallet (for development and testing. This basically incorporates a private key hex)

rgd the first: yes! pls see above .. go into AB rgd the second: not sure if I understand .. "mock hardware" for me would involve simulating talking to a chip or simulating the chip itself. if it is file-based-key, then we have that already ..

Once we have something like that in place in autobahn. I guess CB would need to be changed somehow.

yes, pls see above. node config + plug in the new impl classes

[om26er]

So here is the summary of what will happen.

1. Keep the functionality that we have today
2. Extend crossbar config to support optional signing key class
3. Also support providing file name of private key (if using SigningKey class)
4. For hardware wallet TBD

So the open question is: At what level of config should this option reside. Should it live at the worker config level OR do we need to support a top-level "options" stanza ?

[oberstet]

yes, that's a great plan!

rgd 2./3.: currently, the node key is stored in Node._node_key

https://github.com/crossbario/crossbar/blob/93f3931eb3fb34bae59c6a8d3d4bcbbef7131d67/crossbar/node/node.py#L102

and read/generated using this helper during node startup in load_keys()

https://github.com/crossbario/crossbar/blob/93f3931eb3fb34bae59c6a8d3d4bcbbef7131d67/crossbar/common/key.py#L158

the config though is read via load_config() - and since both are currently independent, the node key file or node key HW settings could not be set via node config. so we need to restructure the code and the calling code appropriately to allow for that possibility

once we have that, we then need to add new config stanzas to

https://github.com/crossbario/crossbar/blob/master/crossbar/common/checkconfig.py

since there is always exactly one node key per node, the config stanza for configuring the node key should be in the controller section (there is at most one such section per node config)

https://github.com/crossbario/crossbar/blob/93f3931eb3fb34bae59c6a8d3d4bcbbef7131d67/crossbar/common/checkconfig.py#L3371

we could have configuration stuff for node keys added to options - or we could add a new section controller.nodekey ... I am kinda undecided ... both would work technically of course ..

[om26er]

the config though is read via load_config() - and since both are currently independent, the node key file or node key HW settings could not be set via node config. so we need to restructure the code and the calling code appropriately to allow for that possibility

One option could be to swap the order of load_config() and load_keys(). The load_config() could check if the loaded config contains the signing key config and if so, construct that. The node class could then just expose a method has_keys(), if that returns false then node.load_keys() should be called, otherwise ignore.

[om26er]

Non tested, however here is what I am thinking. (We'd also need a similar change in edge and even master node code)

(venv) om26er@HomePC:~/scm/crossbario/crossbar/testing$ git diff
diff --git a/crossbar/node/main.py b/crossbar/node/main.py
index b38db80b..e09db306 100644
--- a/crossbar/node/main.py
+++ b/crossbar/node/main.py
@@ -794,10 +794,6 @@ def _run_command_start(options, reactor, personality):

     log.debug('Running on realm="{realm}" from cbdir="{cbdir}"', realm=hlid(node.realm), cbdir=hlid(options.cbdir))

-    # possibly generate new node key
-    #
-    node.load_keys(options.cbdir)
-
     # check and load the node configuration
     #
     try:
@@ -815,6 +811,11 @@ def _run_command_start(options, reactor, personality):
                  config_source=hl(config_source, bold=True, color='green'),
                  config_path=hlid(config_path))

+    # possibly generate new node key
+    #
+    if not node.has_key():
+        node.load_keys(options.cbdir)
+
     # if vmprof global profiling is enabled via command line option, this will carry
     # the file where vmprof writes its profile data
     if _HAS_VMPROF:
diff --git a/crossbar/node/node.py b/crossbar/node/node.py
index 52c27fa4..fb13eb57 100644
--- a/crossbar/node/node.py
+++ b/crossbar/node/node.py
@@ -200,8 +200,16 @@ class Node(object):

             self.personality.check_config(self.personality, self._config)

+        if 'nodekey' in self._config['controller']:
+            # construct node key object here
+            # self._node_key =
+            pass
+
         return config_source, config_path

+    def has_key(self):
+        return self._node_key is not None
+
     def _add_global_roles(self):
         controller_role_config = {
             # there is exactly 1 WAMP component authenticated under authrole "controller": the node controller

[oberstet]

One option could be ...

I like that! Makes sense. Lazy setup.

on a side note: the detailed setup of a node, eg config loading or wait for active mgmt from master node, load of node key, generally setup like this => this has grown over time, and I would expect there is potential to make it less convoluted, more straight forward etc .. and that the current detailed order of what happens when isn't easy to grasp (and has potential to break things;)

[om26er]

This is a bit complex than initially anticipated. We need to do some tweaking in the code for the worker processes to support custom signing key classes as well.

Currently it's read from the key files https://github.com/crossbario/crossbar/blob/93f3931eb3fb34bae59c6a8d3d4bcbbef7131d67/crossbar/worker/controller.py#L59

One way would be to extend the crossbar _exec_worker cli option to pass the verifying key class and it's arguments. Doing that we can then change the above code to construct the SigningKey dynamically.

[om26er]

A work-in-progress PR is up, which at least implements it for the controller process. https://github.com/crossbario/crossbar/pull/1906

[oberstet]

We need to do some tweaking in the code for the worker processes to support custom signing key classes as well.

yes, this is true. unfortunately, it's even trickier.

the node controller process is the only process guaranteed to always exist exactly once, and which is responsible for reading the node config.

every router worker process must be able to sign using the node key - whether that is file based or HW based

a wrapper for HW based key is very likely only able to have one running instance - eg it needs to open the Serial channel to the HW and process coms over that

hence, what we need is: allow multiple router workers to all use one SigningKey instance running in the controller worker process

the easiest way to make this work is to expose SigningKey methods via WAMP over the controller worker => router worker WAMP transport which runs over pipes between the processes. this is the way all worker methods are exposed to the controller worker (for local and remote node management), and of course it can be used in the other direction as well. call into the controller worker from the router worker. we already do that for other stuff.

cli option to pass the verifying key class and it's arguments.

in general, that would also be my first stop. however, in this case, it isn't sufficient, since we need to have the HW wrapping SigningKey be a single instance per node ... above

[oberstet]

let me add another node rgd above: you will need to understand how the controller=>worker WAMP comms works. and also where/how the worker is actually a WAMP client, and where it registers/subscribes, and how it can call/publish to the controller. might be tricky to understand first, but this mechanism is essential in CB as it allows a multi-process architecture with dynamic management over WAMP within CB itself ... kinda "eat your own dog food" ;)

if above sounds too confusing, np, pls let me know, I can guide you through the relevant code. it's nothing magic and "just WAMP" once you understand how the bits fit together ..

[oberstet]

one more note: if we expose SigningKey methods from the controller process, we need to ensure those methods are only exposed on the local node router, but not remoted via the management API towards the master node. This is possible .. I think we already do that. in any case: this is an important detail we should not forget;)

[oberstet]

So the open question is: At what level of config should this option reside. Should it live at the worker config level OR do we need to support a top-level "options" stanza ?

here is a proposal: https://github.com/crossbario/crossbar/pull/1906#issuecomment-974537954

[om26er]

This is done now. More to follow