Use ClientConfig.node_id as canonical UI node identity

daemon/ui/notifications.go

@@ -51,6 +51,7 @@ func (c *Client) getClientConfig() *protocol.ClientConfig {
 	return &protocol.ClientConfig{
 		Id:                uint64(ts.UnixNano()),
 		Name:              nodeName,
+		NodeId:            nodeName,
 		Version:           nodeVersion,
 		IsFirewallRunning: firewall.IsRunning(),
 		Config:            strings.Replace(string(raw), "\n", "", -1),

proto/ui.proto

@@ -258,6 +258,7 @@ message ClientConfig {
     uint32 logLevel = 6;
     repeated Rule rules = 7;
     SysFirewall systemFirewall = 8;
+    string node_id = 9;
 }
 
 /* Notification message is sent to the clients (daemons) from the GUI (server)

ui/opensnitch/nodes.py

@@ -32,6 +32,7 @@ def __init__(self):
         self._db = Database.instance()
         self._rules = Rules()
         self._nodes = {}
+        self._peer_map = {}
         self._notifications_sent = {}
         self._interfaces = NetworkInterfaces()
         self.logger = logger.get(__name__)
@@ -63,22 +64,27 @@ def add(self, _peer, client_config=None):
         https://grpc.io/docs/guides/keepalive/#keepalive-configuration-specification
         """
         try:
-            proto, addr = self.get_addr(_peer)
-            peer = proto+":"+addr
+            peer = self._get_node_key(_peer, client_config)
+            now = datetime.now()
             if peer not in self._nodes:
                 self._nodes[peer] = {
                     'session': {
-                        'peer': _peer, 'last_seen': datetime.now()
+                        'peer': _peer, 'last_seen': now
                     },
                     'notifications': Queue(),
                     'online':        True,
-                    'last_seen':     datetime.now()
+                    'last_seen':     now
                 }
             else:
-                self._nodes[peer]['last_seen'] = datetime.now()
+                prev_peer = self._nodes[peer]['session'].get('peer')
+                if prev_peer is not None and prev_peer != _peer:
+                    self._peer_map.pop(prev_peer, None)
+                self._nodes[peer]['last_seen'] = now
                 self._nodes[peer]['session']['peer'] = _peer
+                self._nodes[peer]['session']['last_seen'] = now
 
             self._nodes[peer]['online'] = True
+            self._peer_map[_peer] = peer
             self.add_data(peer, client_config)
             self.insert(peer)
 
@@ -174,27 +180,31 @@ def update_rule_time(self, time, rule_name, addr):
     def delete_all(self):
         self.stop_notifications()
         self._nodes = {}
+        self._peer_map = {}
         self.nodesUpdated.emit(self.count())
 
     def delete(self, peer):
+        addr = self._resolve_node_key(peer)
         try:
-            proto, addr = self.get_addr(peer)
-            addr = "%s:%s" % (proto, addr)
             # Force the node to get one new item from queue,
             # in order to loop and exit.
             self._nodes[addr]['notifications'].put(None)
-        except:
-            addr = peer
+        except Exception:
+            pass
 
         if addr in self._nodes:
             del self._nodes[addr]
+            for mapped_peer, node_key in list(self._peer_map.items()):
+                if mapped_peer == peer or node_key == addr:
+                    del self._peer_map[mapped_peer]
             self.nodesUpdated.emit(self.count())
 
     def get(self):
         return self._nodes
 
     def get_node(self, addr):
         try:
+            addr = self._resolve_node_key(addr)
             return self._nodes[addr]
         except Exception as e:
             self.logger.debug("exception get_node() %s: %s", addr, repr(e))
@@ -205,6 +215,7 @@ def get_nodes(self):
 
     def get_node_hostname(self, addr):
         try:
+            addr = self._resolve_node_key(addr)
             if addr not in self._nodes:
                 return ""
             return self._nodes[addr]['data'].name
@@ -214,6 +225,7 @@ def get_node_hostname(self, addr):
 
     def get_node_config(self, addr):
         try:
+            addr = self._resolve_node_key(addr)
             if addr not in self._nodes:
                 return None
             return self._nodes[addr]['data'].config
@@ -234,7 +246,7 @@ def get_client_config(self, client_config):
 
     def get_addr(self, peer):
         try:
-            peer = peer.split(":")
+            peer = self._split_peer(self._resolve_node_key(peer))
             # WA for backward compatibility
             if peer[0] == "unix" and peer[1] == "":
                 peer[1] = "/local"
@@ -243,6 +255,25 @@ def get_addr(self, peer):
             self.logger.warning("error getting addr %s: %s", repr(peer), repr(e))
             return peer
 
+    def _split_peer(self, peer):
+        return peer.split(":")
+
+    def _resolve_node_key(self, peer):
+        return self._peer_map.get(peer, peer)
+
+    def _get_node_key(self, peer, client_config=None):
+        proto, addr = self._split_peer(peer)[:2]
+        node_id = getattr(client_config, "node_id", "").strip() if client_config is not None else ""
+        node_name = getattr(client_config, "name", "").strip() if client_config is not None else ""
+
+        if proto in ("ipv4", "ipv6"):
+            if node_id != "":
+                return f"node:{node_id}"
+            if node_name != "" and self.is_local(f"{proto}:{addr}"):
+                return f"node:{node_name}"
+
+        return f"{proto}:{addr}"
+
     def is_connected(self, addr):
         try:
             nd = self.get_node(addr)
@@ -281,6 +312,7 @@ def get_notifications(self):
 
     def save_node_config(self, addr, config):
         try:
+            addr = self._resolve_node_key(addr)
             self._nodes[addr]['data'].config = config
         except Exception as e:
             self.logger.warning("exception saving node config %s: %s - %s", addr, repr(e), config)
@@ -319,6 +351,7 @@ def firewall(self, not_type=ui_pb2.ENABLE_INTERCEPTION, addr=None, callback=None
 
     def send_notification(self, addr, notification, callback_signal=None):
         try:
+            addr = self._resolve_node_key(addr)
             notification.id = int(str(time.time()).replace(".", ""))
             if addr not in self._nodes:
                 # FIXME: the reply is sent before we return the notification id
@@ -373,6 +406,7 @@ def send_notifications(self, notification, callback_signal=None):
 
     def reply_notification(self, addr, reply):
         try:
+            addr = self._resolve_node_key(addr)
             if reply == None:
                 self.logger.debug("reply notification None %s", addr)
                 return
@@ -407,8 +441,7 @@ def stop_notifications(self, addr=None):
 
     def insert(self, peer, status=ONLINE):
         try:
-            proto, addr = self.get_addr(peer)
-            naddr = "{0}:{1}".format(proto, addr)
+            naddr = self._resolve_node_key(peer)
             self._db.insert(
                 "nodes",
                 "(addr, status, hostname, daemon_version, daemon_uptime, " \
@@ -422,8 +455,7 @@ def insert(self, peer, status=ONLINE):
 
     def update(self, peer, status=ONLINE):
         try:
-            proto, addr = self.get_addr(peer)
-            naddr = "{0}:{1}".format(proto, addr)
+            naddr = self._resolve_node_key(peer)
             self._db.update("nodes",
                     "hostname=?,version=?,last_connection=?,status=?",
                     (

ui/opensnitch/service.py

@@ -896,7 +896,8 @@ def _disable_temp_rule(args):
                 ost.start()
 
         elif kwargs['action'] == self.DELETE_RULE:
-            self._db.delete_rule(kwargs['name'], kwargs['addr'])
+            proto, addr = self._get_peer(kwargs['addr'])
+            self._db.delete_rule(kwargs['name'], f"{proto}:{addr}")
 
         elif kwargs['action'] == self.NODE_DELETE:
             self._delete_node(kwargs['peer'])
@@ -1105,7 +1106,7 @@ def new_node_message():
                     if in_message is None:
                         continue
 
-                    self._nodes.reply_notification(addr, in_message)
+                    self._nodes.reply_notification(node_addr, in_message)
                 except StopIteration:
                     self.logger.info("Node %s exited", node_addr)
                     break

ui/tests/conftest.py

@@ -27,6 +27,9 @@ def init_test_environment():
     # Setup mock node with full structure
     from tests.dialogs import ClientConfig
     nodes = Nodes.instance()
+    nodes._nodes = {}
+    nodes._peer_map = {}
+    nodes._notifications_sent = {}
     nodes._nodes["unix:/tmp/osui.sock"] = {
         'data': ClientConfig,
         'notifications': Queue(),
@@ -67,6 +70,9 @@ def reset_node_before_each_test(qapp):
     from tests.dialogs import ClientConfig
 
     nodes = Nodes.instance()
+    nodes._nodes = {}
+    nodes._peer_map = {}
+    nodes._notifications_sent = {}
     nodes._nodes["unix:/tmp/osui.sock"] = {
         'data': ClientConfig,
         'notifications': Queue(),

ui/tests/dialogs/__init__.py

@@ -7,8 +7,10 @@
 class ClientConfig:
     version = "1.2.3"
     name = "bla"
+    node_id = ""
     logLevel = 0
     isFirewallRunning = False
+    systemFirewall = None
     rules = []
     config = '''{
     "Server":{

ui/tests/test_nodes.py

@@ -26,6 +26,7 @@ class TestNodes():
     def setup_method(self):
         self.nid = None
         self.daemon_config = ClientConfig
+        self.daemon_config.node_id = ""
         self.nodes = Nodes.instance()
         # Insert with full addr format "proto:addr" to match how update() queries
         self.nodes._db.insert("nodes",
@@ -50,6 +51,38 @@ def test_get_addr(self, qtbot):
         proto, addr = self.nodes.get_addr("peer:1.2.3.4")
         assert proto == "peer" and addr == "1.2.3.4"
 
+    def test_add_network_node_uses_node_id(self):
+        self.daemon_config.node_id = "vm-work"
+
+        node, addr = self.nodes.add("ipv4:127.0.0.1:52032", self.daemon_config)
+
+        assert node is not None
+        assert addr == "node:vm-work"
+        assert self.nodes.get_node("node:vm-work") is not None
+        assert self.nodes.get_node("ipv4:127.0.0.1:52032") is not None
+
+    def test_network_reconnect_updates_existing_node_id(self):
+        self.daemon_config.node_id = "vm-work"
+        base_count = self.nodes.count()
+
+        self.nodes.add("ipv4:127.0.0.1:52032", self.daemon_config)
+        self.nodes.add("ipv4:127.0.0.1:52033", self.daemon_config)
+
+        assert self.nodes.count() == base_count + 1
+        assert self.nodes.get_node("node:vm-work")['session']['peer'] == "ipv4:127.0.0.1:52033"
+        assert self.nodes.get_node("ipv4:127.0.0.1:52032") is None
+        assert self.nodes.get_node("ipv4:127.0.0.1:52033") is not None
+
+    def test_loopback_node_falls_back_to_name_without_node_id(self, monkeypatch):
+        monkeypatch.setattr(self.nodes, "is_local", lambda _addr: True)
+
+        node, addr = self.nodes.add("ipv4:127.0.0.1:52032", self.daemon_config)
+
+        assert node is not None
+        assert addr == "node:bla"
+        assert self.nodes.get_node("node:bla") is not None
+        assert self.nodes.get_node("ipv4:127.0.0.1:52032") is not None
+
     def test_get_nodes(self, qtbot):
         nodes = self.nodes.get_nodes()
         assert nodes.get("peer:1.2.3.4") is not None