[change] Changed netjson ID of OpenVPN backend to avoid duplicate nodes

#93

The node ID of OpenVPN is now calculated in the following way:

- common_name,address in most cases
- common_name,address:port whenever there are multiple nodes
  having same common_name and address, which can happen when
  reusing the same common name is allowed and multiple nodes
  are behind the same public IP

Closes #93

netdiff/parsers/openvpn.py

@@ -40,14 +40,14 @@ def parse(self, data):
         else:
             clients = data.client_list.values()
             links = data.routing_table.values()
-        real_addresses = []
+        id_list = []
         special_cases = []
         for client in clients:
-            address = client.real_address
-            if address.host in real_addresses:
-                special_cases.append(address.host)
+            id_ = f'{client.common_name},{client.real_address.host}'
+            if id_ in id_list:
+                special_cases.append(id_)
                 continue
-            real_addresses.append(address.host)
+            id_list.append(id_)
         # add clients in graph as nodes
         for client in clients:
             if client.common_name == 'UNDEF':
@@ -70,21 +70,19 @@ def parse(self, data):
             ]
             if local_addresses:
                 client_properties['local_addresses'] = local_addresses
-            # use host:port as node ID only when
-            # there are more nodes with the same address
-            if address.host in special_cases:
-                node_id = '{}:{}'.format(address.host, address.port)
-            else:
-                node_id = str(address.host)
+            # if there are multiple nodes with the same common name
+            # and host address, add the port to the node ID
+            node_id = f'{client.common_name},{address.host}'
+            if node_id in special_cases:
+                node_id = f'{node_id}:{address.port}'
             graph.add_node(node_id, **client_properties)
         # add links in routing table to graph
         for link in links:
             if link.common_name == 'UNDEF':
                 continue
             address = link.real_address
-            if address.host in special_cases:
-                target_id = '{}:{}'.format(address.host, address.port)
-            else:
-                target_id = str(address.host)
+            target_id = f'{link.common_name},{address.host}'
+            if target_id in special_cases:
+                target_id = f'{target_id}:{address.port}'
             graph.add_edge(server, str(target_id), weight=1)
         return graph

tests/static/openvpn-2-links.txt

@@ -5,8 +5,8 @@ nodeA,87.18.10.87:49502,334948,1973012,Thu Jun 18 04:23:03 2015
 nodeB,93.40.230.50:64169,1817262,28981224,Thu Jun 18 04:08:39 2015
 ROUTING TABLE
 Virtual Address,Common Name,Real Address,Last Ref
-192.168.255.134,node1,87.18.10.87:49502,Thu Jun 18 08:12:09 2015
-192.168.255.126,node2,93.40.230.50:64169,Thu Jun 18 08:11:55 2015
+192.168.255.134,nodeA,87.18.10.87:49502,Thu Jun 18 08:12:09 2015
+192.168.255.126,nodeB,93.40.230.50:64169,Thu Jun 18 08:11:55 2015
 GLOBAL STATS
 Max bcast/mcast queue length,0
 END

tests/static/openvpn-special-case.txt

@@ -0,0 +1,15 @@
+OpenVPN CLIENT LIST
+Updated,Thu Jun 18 08:12:15 2015
+Common Name,Real Address,Bytes Received,Bytes Sent,Connected Since
+60c5a8fffe77607a,194.183.10.51:49794,334948,1973012,Thu Jun 18 04:23:03 2015
+60c5a8fffe77607a,194.183.10.51:60003,334948,1973012,Thu Jun 18 04:23:03 2015
+58a0cbeffe0156b0,217.72.97.67:59908,334948,1973012,Thu Jun 18 04:23:03 2015
+ROUTING TABLE
+Virtual Address,Common Name,Real Address,Last Ref
+172.29.0.42,60c5a8fffe77607a,194.183.10.51:49794,Tue Apr 28 14:25:10 2020
+172.29.0.34,60c5a8fffe77607a,194.183.10.51:60003,Tue Apr 28 14:25:10 2020
+172.29.0.35,60c5a8fffe77607a,217.72.97.66:40012,Tue Apr 28 14:25:10 2020
+172.29.0.22,58a0cbeffe0156b0,217.72.97.67:59908,Tue Apr 28 14:25:09 2020
+GLOBAL STATS
+Max bcast/mcast queue length,0
+END

tests/test_openvpn.py

@@ -11,6 +11,7 @@
 links2undef = open('{0}/static/openvpn-2-links-undef.txt'.format(CURRENT_DIR)).read()
 links5_tap = open('{0}/static/openvpn-5-links-tap.txt'.format(CURRENT_DIR)).read()
 bug = open('{0}/static/openvpn-bug.txt'.format(CURRENT_DIR)).read()
+special_case = open('{0}/static/openvpn-special-case.txt'.format(CURRENT_DIR)).read()
 
 
 class TestOpenvpnParser(TestCase):
@@ -117,27 +118,79 @@ def test_parse_bug(self):
         labels = []
         for node in data['nodes']:
             labels.append(node['label'])
-        expected = [
+        expected = {
             '60c5a8fffe77607a',
             '60c5a8fffe77606b',
             '60C5A8FFFE74CB6D',
             '60c5a8fffe77607a',
             '58a0cbeffe0176d4',
             '58a0cbeffe0156b0',
             '',
-        ]
+        }
         with self.subTest('Check contents of nodes'):
-            self.assertEqual(expected, labels)
+            self.assertEqual(expected, set(labels))
 
         targets = []
         for link in data['links']:
             targets.append(link['target'])
-        expected = [
-            '185.211.160.5',
-            '185.211.160.87',
-            '194.183.10.51:49794',
-            '194.183.10.51:60003',
-            '195.94.160.52',
-            '217.72.97.67',
-        ]
-        self.assertEqual(expected, targets)
+        expected = {
+            '60c5a8fffe77607a,185.211.160.5',
+            '60c5a8fffe77606b,185.211.160.87',
+            '60C5A8FFFE74CB6D,194.183.10.51',
+            '60c5a8fffe77607a,194.183.10.51',
+            '58a0cbeffe0176d4,195.94.160.52',
+            '58a0cbeffe0156b0,217.72.97.67',
+        }
+        self.assertEqual(expected, set(targets))
+
+    def test_parse_special_case(self):
+        """
+        Tests behavior when the topology contains
+        nodes that have the same common name and same address
+        (it can happen when allowing reusing the same certificate
+         and multiple clients are connected behind the same public IP)
+        """
+        p = OpenvpnParser(special_case)
+        data = p.json(dict=True)
+        self.assertIsInstance(p.graph, networkx.Graph)
+        with self.subTest('Count nodes and links'):
+            self.assertEqual(len(data['nodes']), 5)
+            self.assertEqual(len(data['links']), 4)
+
+        id_list = []
+        for node in data['nodes']:
+            id_list.append(node['id'])
+        expected = {
+            '60c5a8fffe77607a,194.183.10.51:49794',
+            '60c5a8fffe77607a,194.183.10.51:60003',
+            '60c5a8fffe77607a,217.72.97.66',
+            '58a0cbeffe0156b0,217.72.97.67',
+            'openvpn-server',
+        }
+        with self.subTest('Check contents of nodes'):
+            self.assertEqual(expected, set(id_list))
+
+        targets = []
+        for link in data['links']:
+            targets.append(link['target'])
+        expected = {
+            '60c5a8fffe77607a,194.183.10.51:49794',
+            '60c5a8fffe77607a,194.183.10.51:60003',
+            '60c5a8fffe77607a,217.72.97.66',
+            '58a0cbeffe0156b0,217.72.97.67',
+        }
+        self.assertEqual(expected, set(targets))
+
+    def test_common_name_as_id(self):
+        old = OpenvpnParser({})
+        new = OpenvpnParser(links5_tap)
+        result = diff(old, new)
+        id_list = []
+        for node in result['added']['nodes']:
+            id_list.append(node['id'])
+        self.assertEqual(len(id_list), 5)
+        self.assertIn('nodeA,2.226.154.66', id_list)
+        self.assertIn('nodeB,93.40.230.50', id_list)
+        self.assertIn('nodeC,95.250.161.57', id_list)
+        self.assertIn('nodeD,79.18.21.144', id_list)
+        self.assertIn('nodeE,87.3.36.166', id_list)