From cf5ccc4850150fc6e87fe9924540151d72d7852c Mon Sep 17 00:00:00 2001 From: Daniel Kim Date: Wed, 21 Jul 2021 11:07:32 -0400 Subject: [PATCH 1/8] Change the way we parse the server values for agent server and upstream dest to ensure that contact endpoints without explicit protocols (e.g. http) get parsed correctly. --- app/objects/c_agent.py | 21 +++++++++++------ tests/objects/test_agent.py | 42 +++++++++++++++++++++++++++++++++ tests/services/test_rest_svc.py | 8 +++---- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/app/objects/c_agent.py b/app/objects/c_agent.py index d00b2e243..11b4d3257 100644 --- a/app/objects/c_agent.py +++ b/app/objects/c_agent.py @@ -108,8 +108,7 @@ def __init__(self, sleep_min, sleep_max, watchdog=0, platform='unknown', server= self.group = group self.architecture = architecture self.platform = platform.lower() - url = urlparse(server) - self.server = '%s://%s:%s' % (url.scheme, url.hostname, url.port) + self.server = self.parse_server(server) self.location = location self.pid = pid self.ppid = ppid @@ -133,11 +132,7 @@ def __init__(self, sleep_min, sleep_max, watchdog=0, platform='unknown', server= self.available_contacts = available_contacts if available_contacts else [self.contact] self.pending_contact = contact self.host_ip_addrs = host_ip_addrs if host_ip_addrs else [] - if upstream_dest: - upstream_url = urlparse(upstream_dest) - self.upstream_dest = '%s://%s:%s' % (upstream_url.scheme, upstream_url.hostname, upstream_url.port) - else: - self.upstream_dest = self.server + self.upstream_dest = self.parse_endpoint(upstream_dest) if upstream_dest else self.server self._executor_change_to_assign = None self.log = self.create_logger('agent') @@ -327,6 +322,18 @@ def assign_pending_executor_change(self): self._executor_change_to_assign = None return executor_change + @staticmethod + def parse_endpoint(server): + if re.search(r'^[^:/]+://[^:\s]+(:\d+)?/*$', server): + # E.g. http://127.0.0.1:8888 + url = urlparse(server) + parsed = '%s://%s' % (url.scheme, url.hostname) + return parsed + ':%s' % url.port if url.port else parsed + elif re.search(r'^[^\s:]+:\d+/*$', server): + # E.g. 127.0.0.1:7010 or 127.0.0.1:7011/ + return server.rstrip('/') + return 'unknown' + """ PRIVATE """ def _replace_payload_data(self, decoded_cmd, file_svc): diff --git a/tests/objects/test_agent.py b/tests/objects/test_agent.py index a8cf265e5..094fcb571 100644 --- a/tests/objects/test_agent.py +++ b/tests/objects/test_agent.py @@ -126,3 +126,45 @@ def test_heartbeat_modification_during_pending_executor_removal(self, loop): agent.set_pending_executor_removal('test') loop.run_until_complete(agent.heartbeat_modification(executors=original_executors)) assert agent.executors == ['cmd'] + + def test_server_value_with_protocol_ip_port(self): + test = 'http://127.0.0.1:8888' + test_slash = 'http://127.0.0.1:8888/' + want = 'http://127.0.0.1:8888' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_with_protocol_ip_without_port(self): + test = 'http://127.0.0.1' + test_slash = 'http://127.0.0.1/' + want = 'http://127.0.0.1' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_with_protocol_domain_port(self): + test = 'http://mydomain.tld:8888' + test_slash = 'http://mydomain.tld:8888/' + want = 'http://mydomain.tld:8888' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_with_protocol_domain_without_port(self): + test = 'http://mydomain.tld' + test_slash = 'http://mydomain.tld/' + want = 'http://mydomain.tld' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_without_protocol_with_ip_port(self): + test = '127.0.0.1:7010' + test_slash = '127.0.0.1:7010/' + want = '127.0.0.1:7010' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_without_protocol_with_domain_port(self): + test = 'mydomain.tld:7010' + test_slash = 'mydomain.tld:7010/' + want = 'mydomain.tld:7010' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want diff --git a/tests/services/test_rest_svc.py b/tests/services/test_rest_svc.py index d3f8bbb9b..3e4a052bc 100644 --- a/tests/services/test_rest_svc.py +++ b/tests/services/test_rest_svc.py @@ -82,13 +82,13 @@ def test_delete_operation(self, loop, rest_svc, data_svc): 'host_group': [{'trusted': True, 'architecture': 'unknown', 'watchdog': 0, 'contact': 'unknown', 'username': 'unknown', 'links': [], 'sleep_max': 8, 'exe_name': 'unknown', 'executors': ['pwsh', 'psh'], 'ppid': 0, - 'sleep_min': 2, 'server': '://None:None', 'platform': 'windows', + 'sleep_min': 2, 'server': 'unknown', 'platform': 'windows', 'host': 'unknown', 'paw': '123', 'pid': 0, 'display_name': 'unknown$unknown', 'group': 'red', 'location': 'unknown', 'privilege': 'User', 'proxy_receivers': {}, 'proxy_chain': [], 'origin_link_id': 0, 'deadman_enabled': False, 'available_contacts': ['unknown'], 'pending_contact': 'unknown', - 'host_ip_addrs': [], 'upstream_dest': '://None:None'}], + 'host_ip_addrs': [], 'upstream_dest': 'unknown'}], 'visibility': 50, 'autonomous': 1, 'chain': [], 'auto_close': False, 'obfuscator': 'plain-text', 'use_learning_parsers': False, 'objective': {'goals': [{'value': 'complete', @@ -154,12 +154,12 @@ def test_create_operation(self, loop, rest_svc, data_svc): 'host_group': [ {'trusted': True, 'architecture': 'unknown', 'watchdog': 0, 'contact': 'unknown', 'username': 'unknown', 'links': [], 'sleep_max': 8, 'exe_name': 'unknown', - 'executors': ['pwsh', 'psh'], 'ppid': 0, 'sleep_min': 2, 'server': '://None:None', + 'executors': ['pwsh', 'psh'], 'ppid': 0, 'sleep_min': 2, 'server': 'unknown', 'platform': 'windows', 'host': 'unknown', 'paw': '123', 'pid': 0, 'display_name': 'unknown$unknown', 'group': 'red', 'location': 'unknown', 'privilege': 'User', 'proxy_receivers': {}, 'proxy_chain': [], 'origin_link_id': 0, 'deadman_enabled': False, 'available_contacts': ['unknown'], 'pending_contact': 'unknown', - 'host_ip_addrs': [], 'upstream_dest': '://None:None'}], + 'host_ip_addrs': [], 'upstream_dest': 'unknown'}], 'visibility': 50, 'autonomous': 1, 'chain': [], 'auto_close': False, 'objective': '', 'obfuscator': 'plain-text', 'use_learning_parsers': False} internal_rest_svc = rest_svc(loop) From 873777d536b69dffd7396d23fe8c1a1e8777fd8f Mon Sep 17 00:00:00 2001 From: Daniel Kim Date: Wed, 21 Jul 2021 11:08:14 -0400 Subject: [PATCH 2/8] Fix function name --- app/objects/c_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/objects/c_agent.py b/app/objects/c_agent.py index 11b4d3257..ea34e37b7 100644 --- a/app/objects/c_agent.py +++ b/app/objects/c_agent.py @@ -108,7 +108,7 @@ def __init__(self, sleep_min, sleep_max, watchdog=0, platform='unknown', server= self.group = group self.architecture = architecture self.platform = platform.lower() - self.server = self.parse_server(server) + self.server = self.parse_endpoint(server) self.location = location self.pid = pid self.ppid = ppid From 3af6bb19b8aec28f57f20a385aba11965bb80b5a Mon Sep 17 00:00:00 2001 From: Daniel Kim Date: Wed, 21 Jul 2021 11:53:30 -0400 Subject: [PATCH 3/8] Update heartbeat modification for server updates --- app/objects/c_agent.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/objects/c_agent.py b/app/objects/c_agent.py index ea34e37b7..bf315e607 100644 --- a/app/objects/c_agent.py +++ b/app/objects/c_agent.py @@ -185,7 +185,8 @@ async def heartbeat_modification(self, **kwargs): self.last_trusted_seen = now self.update('pid', kwargs.get('pid')) self.update('ppid', kwargs.get('ppid')) - self.update('server', kwargs.get('server')) + new_server = kwargs.get('server') + self.update('server', self.parse_endpoint(new_server) if new_server else None) self.update('exe_name', kwargs.get('exe_name')) self.update('location', kwargs.get('location')) self.update('privilege', kwargs.get('privilege')) From e647cce375d6d4f1266cb62b912407b7eefd2cee Mon Sep 17 00:00:00 2001 From: Daniel Kim Date: Wed, 21 Jul 2021 11:56:07 -0400 Subject: [PATCH 4/8] Add heartbeat modification tests for server --- tests/objects/test_agent.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/objects/test_agent.py b/tests/objects/test_agent.py index 094fcb571..8d4a7d712 100644 --- a/tests/objects/test_agent.py +++ b/tests/objects/test_agent.py @@ -168,3 +168,18 @@ def test_server_value_without_protocol_with_domain_port(self): want = 'mydomain.tld:7010' assert Agent.parse_endpoint(test) == want assert Agent.parse_endpoint(test_slash) == want + + def test_heartbeat_modification_new_server(self, loop): + agent = Agent(paw='123', sleep_min=2, sleep_max=8, watchdog=0, executors=['cmd', 'test'], platform='windows', + server='unknown') + test = 'mydomain.tld:7010' + want = 'mydomain.tld:7010' + loop.run_until_complete(agent.heartbeat_modification(server=test)) + assert agent.server == want + + def test_heartbeat_modification_no_server_provided(self, loop): + want = 'mydomain.tld:7010' + agent = Agent(paw='123', sleep_min=2, sleep_max=8, watchdog=0, executors=['cmd', 'test'], platform='windows', + server=want) + loop.run_until_complete(agent.heartbeat_modification()) + assert agent.server == want From 57ac46bec5b02b247a09c0f463db18d89090ff3e Mon Sep 17 00:00:00 2001 From: Chris Lenk Date: Mon, 24 Jul 2023 12:18:43 -0400 Subject: [PATCH 5/8] Fix parsed agent str in operations test --- tests/api/v2/handlers/test_operations_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api/v2/handlers/test_operations_api.py b/tests/api/v2/handlers/test_operations_api.py index 9433a8de4..7117eada7 100644 --- a/tests/api/v2/handlers/test_operations_api.py +++ b/tests/api/v2/handlers/test_operations_api.py @@ -352,7 +352,7 @@ async def test_create_potential_link_with_globals(self, api_v2_client, api_cooki assert result['paw'] == payload['paw'] assert result['id'] assert result['ability']['name'] == 'Manual Command' - assert result['command'] == "://None:None 123" + assert result['command'] == "unknown 123" async def test_create_potential_link(self, api_v2_client, api_cookies, mocker, async_return): with mocker.patch('app.objects.c_operation.Operation.apply') as mock_apply: From 4c3a0117e9b86fdf23c3c3b7c0283fa2faf44c31 Mon Sep 17 00:00:00 2001 From: Chris Lenk Date: Mon, 24 Jul 2023 17:19:41 -0400 Subject: [PATCH 6/8] Update agent server regex --- app/objects/c_agent.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/objects/c_agent.py b/app/objects/c_agent.py index 5132019a0..1ff367705 100644 --- a/app/objects/c_agent.py +++ b/app/objects/c_agent.py @@ -329,12 +329,12 @@ def assign_pending_executor_change(self): @staticmethod def parse_endpoint(server): - if re.search(r'^[^:/]+://[^:\s]+(:\d+)?/*$', server): + if re.search(r'^[^:/]+://[^:\s]+(:\d{1,5})?/*$', server): # E.g. http://127.0.0.1:8888 url = urlparse(server) parsed = '%s://%s' % (url.scheme, url.hostname) return parsed + ':%s' % url.port if url.port else parsed - elif re.search(r'^[^\s:]+:\d+/*$', server): + elif re.search(r'^[^\s:]+:\d{1,5}/*$', server): # E.g. 127.0.0.1:7010 or 127.0.0.1:7011/ return server.rstrip('/') return 'unknown' From bf7bd3b06d13b6ce54b420165bb81f4a988d1806 Mon Sep 17 00:00:00 2001 From: Chris Lenk Date: Tue, 25 Jul 2023 17:36:41 -0400 Subject: [PATCH 7/8] Fix agent server string regex Prevent catastrophic backtracking using the workaround explained here: https://stackoverflow.com/a/13577411/ --- app/objects/c_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/objects/c_agent.py b/app/objects/c_agent.py index 1ff367705..aab42299c 100644 --- a/app/objects/c_agent.py +++ b/app/objects/c_agent.py @@ -329,7 +329,7 @@ def assign_pending_executor_change(self): @staticmethod def parse_endpoint(server): - if re.search(r'^[^:/]+://[^:\s]+(:\d{1,5})?/*$', server): + if re.search(r'^[^:/\s]+://(?=([^:\s]+))\1(:\d{1,5})?/*$', server): # E.g. http://127.0.0.1:8888 url = urlparse(server) parsed = '%s://%s' % (url.scheme, url.hostname) From 083e3d845eac7bc08865312e003db087a244f279 Mon Sep 17 00:00:00 2001 From: Chris Lenk Date: Tue, 25 Jul 2023 17:38:39 -0400 Subject: [PATCH 8/8] Add more agent server string parsing tests for 'localhost'. Note that it will be parsed as 'unknown' unless you have a protocol, port, or TLD. --- tests/objects/test_agent.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/objects/test_agent.py b/tests/objects/test_agent.py index 86cb7d503..a9e9a11f7 100644 --- a/tests/objects/test_agent.py +++ b/tests/objects/test_agent.py @@ -168,6 +168,34 @@ def test_server_value_without_protocol_with_domain_port(self): assert Agent.parse_endpoint(test) == want assert Agent.parse_endpoint(test_slash) == want + def test_server_value_with_protocol_localhost_port(self): + test = 'http://localhost:8888' + test_slash = 'http://localhost:8888/' + want = 'http://localhost:8888' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_with_protocol_localhost_without_port(self): + test = 'http://localhost' + test_slash = 'http://localhost/' + want = 'http://localhost' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_without_protocol_localhost_port(self): + test = 'localhost:8888' + test_slash = 'localhost:8888/' + want = 'localhost:8888' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + + def test_server_value_without_protocol_localhost_without_port(self): + test = 'localhost' + test_slash = 'localhost/' + want = 'unknown' + assert Agent.parse_endpoint(test) == want + assert Agent.parse_endpoint(test_slash) == want + def test_heartbeat_modification_new_server(self, loop): agent = Agent(paw='123', sleep_min=2, sleep_max=8, watchdog=0, executors=['cmd', 'test'], platform='windows', server='unknown')