From a6ffdf47ee54721ab49107aa4da376d2f5c7d5f8 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Wed, 15 Jul 2020 18:31:55 -0500 Subject: [PATCH] [change] OpenVPN: just use common_name as ID #94 Allow duplicate common names with duplicate_cn. Closes #94 --- README.rst | 21 +++++++++++ netdiff/parsers/openvpn.py | 71 +++++++++++++++++++++++++++++--------- tests/test_openvpn.py | 54 ++++++++++++++++++++++++----- 3 files changed, 121 insertions(+), 25 deletions(-) diff --git a/README.rst b/README.rst index 7a7eb59..8a8ea0d 100644 --- a/README.rst +++ b/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 ------------ diff --git a/netdiff/parsers/openvpn.py b/netdiff/parsers/openvpn.py index 3618196..f10dbbd 100644 --- a/netdiff/parsers/openvpn.py +++ b/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() - 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_) + special_cases = self._find_special_cases(clients) # add clients in graph as nodes for client in clients: if client.common_name == 'UNDEF': @@ -62,6 +60,7 @@ def parse(self, data): ), 'bytes_received': int(client.bytes_received), 'bytes_sent': int(client.bytes_sent), + 'common_name': client.common_name, } local_addresses = [ str(route.virtual_address) @@ -70,19 +69,57 @@ def parse(self, data): ] if local_addresses: client_properties['local_addresses'] = local_addresses - # 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}' + 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 - target_id = f'{link.common_name},{address.host}' - if target_id in special_cases: - target_id = f'{target_id}:{address.port}' + 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 diff --git a/tests/test_openvpn.py b/tests/test_openvpn.py index 2c93e85..5ea83fb 100644 --- a/tests/test_openvpn.py +++ b/tests/test_openvpn.py @@ -107,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) @@ -143,14 +143,51 @@ def test_parse_bug(self): } self.assertEqual(expected, set(targets)) - def test_parse_special_case(self): + 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) + 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'): @@ -188,9 +225,10 @@ def test_common_name_as_id(self): id_list = [] for node in result['added']['nodes']: id_list.append(node['id']) + self.assertEqual(node['id'], node['properties']['common_name']) 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) + 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)