Merge c447796 into 1fe7109

.travis.yml

@@ -12,6 +12,8 @@ branches:
 install:
   - python setup.py develop
   - pip install -r requirements-test.txt
+  # TODO: remove when openwisp-utils with isort 5 is released
+  - pip install -U https://github.com/openwisp/openwisp-utils/tarball/master#egg=openwisp_utils[qa]
 
 before_script:
   - ./run-qa-checks

CHANGES.rst

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+Version 0.9.0 [unreleased]
+--------------------------
+
+WIP
+
 Version 0.8.0 [28-06-2020]
 --------------------------
 

README.rst

@@ -389,6 +389,27 @@ TopologyRetrievalError
 Raised when it is not possible to retrieve the topology data
 (eg: the URL might be temporary unreachable).
 
+Specialized features
+--------------------
+
+OpenVPN
+~~~~~~~
+
+By default, the OpenVPN parser uses the common name to identify a client,
+this was chosen because if the public IP address is used, the same client
+will not be recognized if it connects with a different IP address
+(very probable since many ISPs use dynamic public IP addresses).
+
+This does not work when the vpn server configuration allows different clients
+to use the same common name (which is generally not recommended anyway).
+
+If you need to support legacy systems which are configured with the OpenVPN
+``duplicate-cn`` feature enabled, you can pass ``duplicate_cn=True`` during
+the initialization of ``OpenvpnParser``.
+This will change the behavior of the parser so that each client is identified
+by their common name and IP address (and additionally the port used if there
+are multiple clients with same common name and IP).
+
 Known Issues
 ------------
 

netdiff/info.py

@@ -1,4 +1,4 @@
-VERSION = (0, 8, 0, 'final')
+VERSION = (0, 9, 0, 'alpha')
 __version__ = VERSION
 
 

netdiff/parsers/base.py

@@ -10,7 +10,7 @@
 try:
     import urlparse
 except ImportError:  # pragma: no cover
-    import urllib.parse as urlparse  # pragma: no cover
+    from urllib import parse as urlparse  # pragma: no cover
 
 
 class BaseParser(object):

netdiff/parsers/cnml.py

@@ -8,7 +8,7 @@
 try:
     import urlparse
 except ImportError:
-    import urllib.parse as urlparse
+    from urllib import parse as urlparse
 
 
 class CnmlParser(BaseParser):

netdiff/parsers/openvpn.py

@@ -11,9 +11,14 @@ class OpenvpnParser(BaseParser):
     protocol = 'OpenVPN Status Log'
     version = '1'
     metric = 'static'
+    duplicate_cn = False
     # for internal use only
     _server_common_name = 'openvpn-server'
 
+    def __init__(self, *args, **kwargs):
+        self.duplicate_cn = kwargs.pop('duplicate_cn', OpenvpnParser.duplicate_cn)
+        super().__init__(*args, **kwargs)
+
     def to_python(self, data):
         if not data:
             return None
@@ -40,14 +45,7 @@ def parse(self, data):
         else:
             clients = data.client_list.values()
             links = data.routing_table.values()
-        real_addresses = []
-        special_cases = []
-        for client in clients:
-            address = client.real_address
-            if address.host in real_addresses:
-                special_cases.append(address.host)
-                continue
-            real_addresses.append(address.host)
+        special_cases = self._find_special_cases(clients)
         # add clients in graph as nodes
         for client in clients:
             if client.common_name == 'UNDEF':
@@ -70,21 +68,57 @@ 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)
+            node_id = self.get_node_id(client, special_cases)
             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 = self.get_target_id(link, special_cases)
             graph.add_edge(server, str(target_id), weight=1)
         return graph
+
+    def get_node_id(self, client, special_cases):
+        """
+        when duplicate_cn is True
+            if there are multiple nodes with the same common name
+            and host address, add the port to the node ID
+        when self.duplicate_cn is False:
+            just use the common_name as node ID
+        """
+        if not self.duplicate_cn:
+            return client.common_name
+        address = client.real_address
+        node_id = f'{client.common_name},{address.host}'
+        if node_id in special_cases:
+            node_id = f'{node_id}:{address.port}'
+        return node_id
+
+    def get_target_id(self, link, special_cases):
+        """
+        when duplicate_cn is True
+            if there are multiple nodes with the same common name
+            and host address, add the port to the target ID
+        when self.duplicate_cn is False:
+            just use the common_name as target ID
+        """
+        if not self.duplicate_cn:
+            return link.common_name
+        address = link.real_address
+        target_id = f'{link.common_name},{address.host}'
+        if target_id in special_cases:
+            target_id = f'{target_id}:{address.port}'
+        return target_id
+
+    def _find_special_cases(self, clients):
+        if not self.duplicate_cn:
+            return []
+        id_list = []
+        special_cases = []
+        for client in clients:
+            id_ = f'{client.common_name},{client.real_address.host}'
+            if id_ in id_list:
+                special_cases.append(id_)
+                continue
+            id_list.append(id_)
+        return special_cases

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):
@@ -106,7 +107,7 @@ def test_label_diff_added(self):
         self.assertIn('nodeE', labels)
 
     def test_parse_bug(self):
-        p = OpenvpnParser(bug)
+        p = OpenvpnParser(bug, duplicate_cn=True)
         data = p.json(dict=True)
         self.assertIsInstance(p.graph, networkx.Graph)
 
@@ -117,27 +118,116 @@ 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_bug_duplicate_cn(self):
+        p = OpenvpnParser(bug, duplicate_cn=True)
+        data = p.json(dict=True)
+        self.assertIsInstance(p.graph, networkx.Graph)
+
+        with self.subTest('Count nodes and links'):
+            self.assertEqual(len(data['nodes']), 7)
+            self.assertEqual(len(data['links']), 6)
+
+        labels = []
+        for node in data['nodes']:
+            labels.append(node['label'])
+        expected = {
+            '60c5a8fffe77607a',
+            '60c5a8fffe77606b',
+            '60C5A8FFFE74CB6D',
+            '60c5a8fffe77607a',
+            '58a0cbeffe0176d4',
+            '58a0cbeffe0156b0',
+            '',
+        }
+        with self.subTest('Check contents of nodes'):
+            self.assertEqual(expected, set(labels))
+
+        targets = []
+        for link in data['links']:
+            targets.append(link['target'])
+        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_duplicate_cn(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, duplicate_cn=True)
+        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', id_list)
+        self.assertIn('nodeB', id_list)
+        self.assertIn('nodeC', id_list)
+        self.assertIn('nodeD', id_list)
+        self.assertIn('nodeE', id_list)