Skip to content

Commit c77c422

Browse files
committed
Address PR review feedback
* Gate _suricata_http.html and _suricata_files.html Process columns on NETWORK_PROC_MAP (consistent with _hosts.html and per the PR description). * _hosts.html: fall back to legacy host.process_name / host.process_id when host.processes is missing (preserves existing process_map enrichment for users who don't run the network_etw module). * network_etw.py: include IPv6 unspecified address ":" in the localhost filter; lowercase hostnames in for_http / set_http_owner; strip whitespace from XML element text in _read_evt_data; correct docstring on _parse_kernel_network_etw (was naming the wrong auxiliary); store basename + path separately when hoisting sigma matched_events processes. * analyzer/network_etw.py: same IPv6 ":" filter; clean up the random C:\<dir> output directory after the final upload so it doesn't accumulate on VMs that aren't reverted from snapshot. * test_network_capture_integration.py: assert against the actually-patched open mock (and that open_exclusive is NOT called for replaceable uploads); document the sys.modules stub pattern. Note: the gemini-code-assist suggestion to rename ProcessID -> ProcessId in sigma matched_events lookups was checked against real sigma output on three different reports — sigma's matched_events use ProcessID (capital D). Existing code is correct; suggestion not applied.
1 parent 56b79e7 commit c77c422

6 files changed

Lines changed: 47 additions & 12 deletions

File tree

analyzer/windows/modules/auxiliary/network_etw.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def _should_filter(self, event, event_id):
9595
return True
9696
if dst_port in self._filter_ports or src_port in self._filter_ports:
9797
return True
98-
if dst_ip in ("127.0.0.1", "::1", "0.0.0.0", ""):
98+
if dst_ip in ("127.0.0.1", "::1", "0.0.0.0", "::", ""):
9999
return True
100100
return False
101101

@@ -240,3 +240,11 @@ def upload_results(self):
240240
upload_to_host(self.log_file_path, os.path.join("aux", "network_etw.json"))
241241
except Exception as e:
242242
log.error("Final network_etw upload failed: %s", e)
243+
244+
# Clean up the random C:\<dir> we created so it doesn't accumulate on
245+
# VMs that aren't reverted from snapshot between analyses.
246+
if self.output_dir and os.path.isdir(self.output_dir):
247+
try:
248+
shutil.rmtree(self.output_dir, ignore_errors=True)
249+
except Exception as e:
250+
log.debug("network_etw output_dir cleanup failed: %s", e)

modules/processing/network_etw.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def add_connection(self, pid, dst_ip, dst_port=None, src_ip="",
116116
return
117117
pid = str(pid)
118118
dst_ip = _clean_ip(dst_ip)
119-
if not dst_ip or dst_ip in ("127.0.0.1", "::1", "0.0.0.0"):
119+
if not dst_ip or dst_ip in ("127.0.0.1", "::1", "0.0.0.0", "::"):
120120
return
121121
if process_name:
122122
self.add_pid_name(pid, process_name)
@@ -250,7 +250,9 @@ def for_host(self, hostname):
250250

251251
def for_http(self, host, uri):
252252
"""(pid, name) from an already-enriched HTTP transaction. Prefer an
253-
exact (host, uri) match; fall back to host alone; finally DNS."""
253+
exact (host, uri) match; fall back to host alone; finally DNS.
254+
Hostnames are normalised to lowercase per RFC 4343."""
255+
host = host.lower() if host else ""
254256
if host and uri:
255257
hit = self._http_by_uri.get((host, uri))
256258
if hit:
@@ -266,6 +268,7 @@ def set_http_owner(self, host, uri, pid, name):
266268
if not pid:
267269
return
268270
pid = str(pid)
271+
host = host.lower() if host else ""
269272
if host and uri:
270273
self._http_by_uri.setdefault((host, uri), (pid, name))
271274
if host:
@@ -343,7 +346,7 @@ def _read_evt_data(event_elem):
343346
# normalise to "" so callers can distinguish "missing" via .get()
344347
# default vs "present but empty" via "" — same as before, but now
345348
# entity-decoded (&amp; → &, &lt; → <, &#xNN; → unicode char).
346-
out[name] = d.text or ""
349+
out[name] = (d.text or "").strip()
347350
return out
348351

349352
def _parse_sysmon_evtx(self):
@@ -422,7 +425,7 @@ def _parse_sysmon_evtx(self):
422425

423426
def _parse_kernel_network_etw(self, pid_to_name):
424427
"""Parse aux/network_etw.json from the Microsoft-Windows-Kernel-Network
425-
ETW provider (captured by the dns_etw auxiliary at analysis time)."""
428+
ETW provider (captured by the network_etw auxiliary at analysis time)."""
426429
connections = []
427430
etw_path = os.path.join(self.analysis_path, "aux", "network_etw.json")
428431
if not os.path.exists(etw_path):
@@ -604,7 +607,7 @@ def run(self):
604607
for c in merged:
605608
pid = c["pid"]
606609
dst = c["dst_ip"]
607-
if not dst or dst in ("127.0.0.1", "::1", "0.0.0.0"):
610+
if not dst or dst in ("127.0.0.1", "::1", "0.0.0.0", "::"):
608611
continue
609612
by_pid.setdefault(pid, {
610613
"pid": pid,
@@ -717,7 +720,8 @@ def apply(rec, hit):
717720
seen_procs.add(key)
718721
procs.append({
719722
"pid": pid,
720-
"process_name": image,
723+
"process_name": os.path.basename(image) if image else "",
724+
"process_path": image,
721725
"command_line": ev.get("CommandLine", ""),
722726
"parent_pid": ev.get("ParentProcessId"),
723727
"parent_image": ev.get("ParentImage", ""),

tests/test_network_capture_integration.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,17 @@
44

55
import pytest
66

7+
8+
# Tests for the network_etw / decryptpcap / resultserver code paths run in
9+
# isolation from the full CAPE runtime. The modules-under-test transitively
10+
# import gevent + a handful of CAPE-internal helpers that we don't want to
11+
# bring into pytest just to exercise pure logic.
12+
#
13+
# `setdefault` here means: only stub if pytest hasn't already imported the
14+
# real module via another collected test. That keeps these stubs from
15+
# clobbering an installed module when running the full suite, while still
16+
# letting `python -m pytest tests/test_network_capture_integration.py` work
17+
# in a stripped-down environment.
718
def _stub_module(name):
819
module = ModuleType(name)
920
sys.modules.setdefault(name, module)
@@ -130,15 +141,20 @@ def test_resultserver_allows_overwrite_for_periodic_aux_logs(tmp_path):
130141
upload = FileUpload(task_id=7, ctx=ctx)
131142
upload.init()
132143

133-
fake_fd = mock_open().return_value
144+
open_mock = mock_open()
134145

135146
with patch("lib.cuckoo.core.resultserver.path_exists", return_value=True), patch(
136-
"lib.cuckoo.core.resultserver.open", mock_open()
147+
"lib.cuckoo.core.resultserver.open_exclusive"
148+
) as exclusive_mock, patch(
149+
"lib.cuckoo.core.resultserver.open", open_mock
137150
) as patched_open:
138151
upload.handle()
139152

153+
# Existing replaceable path -> truncate-write via plain open(..., "wb")
140154
patched_open.assert_any_call(str(tmp_path / "aux/network_etw.json"), "wb")
141-
fake_fd.write.assert_not_called()
155+
# And NOT via open_exclusive — that would EEXIST and silently drop the
156+
# upload, which is exactly the bug REPLACEABLE_RESULT_UPLOADS fixes.
157+
exclusive_mock.assert_not_called()
142158

143159

144160
def test_pcap_selector_prefers_mixed_pcap_when_configured(tmp_path):

web/templates/analysis/network/_hosts.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ <h5 class="mb-0 text-white"><i class="fas fa-server me-2 text-info"></i>Hosts</h
4444
{% if p.process_name %}{{ p.process_name }}{% else %}(unknown){% endif %}{% if p.pid %} ({{ p.pid }}){% endif %}
4545
</span>
4646
{% endfor %}
47+
{% elif host.process_name or host.process_id %}
48+
{# Legacy single-owner attribution from CAPE's existing process_map enrichment in network.py — used when network_etw module isn't enabled #}
49+
<span class="badge bg-warning text-dark">
50+
{% if host.process_name %}{{ host.process_name }}{% else %}(unknown){% endif %}{% if host.process_id %} ({{ host.process_id }}){% endif %}
51+
</span>
4752
{% else %}
4853
<span class="text-muted">-</span>
4954
{% endif %}

web/templates/analysis/network/_suricata_files.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
<th style="border-top: 0;" width="15%">File name</th>
88
<td style="border-top: 0;"><b>{{file.filename}}</b></td>
99
</tr>
10+
{% if settings.NETWORK_PROC_MAP %}
1011
<tr>
1112
<th>Process</th>
1213
<td>{% if file.process_name %}<span class="badge bg-warning text-dark" title="PID: {{file.process_id}}">{{file.process_name}} ({{file.process_id}})</span>{% else %}<span class="text-muted">-</span>{% endif %}</td>
1314
</tr>
15+
{% endif %}
1416
<tr>
1517
<th>Expected File Size</th>
1618
<td>{{file.size}} bytes</td>

web/templates/analysis/network/_suricata_http.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ <h5 class="mb-0 text-white"><i class="fas fa-globe me-2 text-info"></i>Suricata
88
<table class="table table-dark table-striped table-bordered">
99
<tr>
1010
<th>Timestamp</th>
11-
<th>Process</th>
11+
{% if settings.NETWORK_PROC_MAP %}<th>Process</th>{% endif %}
1212
<th>Source IP</th>
1313
<th>Source Port</th>
1414
<th>Destination IP</th>
@@ -25,7 +25,7 @@ <h5 class="mb-0 text-white"><i class="fas fa-globe me-2 text-info"></i>Suricata
2525
{% for http in suricata.http %}
2626
<tr>
2727
<td>{{http.timestamp}}</td>
28-
<td>{% if http.process_name %}<span class="badge bg-warning text-dark" title="PID: {{http.process_id}}">{{http.process_name}} ({{http.process_id}})</span>{% else %}<span class="text-muted">-</span>{% endif %}</td>
28+
{% if settings.NETWORK_PROC_MAP %}<td>{% if http.process_name %}<span class="badge bg-warning text-dark" title="PID: {{http.process_id}}">{{http.process_name}} ({{http.process_id}})</span>{% else %}<span class="text-muted">-</span>{% endif %}</td>{% endif %}
2929
<td>{{http.srcip}}
3030
<a href="https://www.virustotal.com/en/ip-address/{{http.srcip}}/information/">[VT]</a>
3131
{% if config.display_et_portal %}

0 commit comments

Comments
 (0)