tests: verify pcap_filename on alert and flow events - v3#3013
tests: verify pcap_filename on alert and flow events - v3#3013oferda4 wants to merge 1 commit intoOISF:masterfrom
Conversation
| Verify that pcap_filename in EVE JSON output reflects the file each | ||
| connection's packets came from, not the last file the RX thread processed. | ||
|
|
||
| Two pcap files are fed with --pcap-file-recursive. alert.pcap is processed |
There was a problem hiding this comment.
You said
the order is actually determined by readdir() - which is filesystem-dependent and not guaranteed.
So ? (or can we make the order reproducible)
There was a problem hiding this comment.
You are right but the test outcome is deterministic: regardless of order, one pcap's flow events would get the wrong pcap_filename if the global fallback were used instead of the per-flow pfv. I'll update the README to not claim a specific order 👍
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
Besides the nit comment, I think these tests are good
@catenacyber How do you go with small fixes like that? Should I open a new PR version? |
It's ok to force-push minor changes on suricata-verify PRs. Thanks for asking and following contribution guidelines! 🙇🏽♀️ 🌟 |
Add three suricata-verify tests that together cover all code paths in the pcap_filename output for pcap-file mode: pcap-filename-alert-and-flow: single pcap, verifies pcap_filename is present in alert events (p != NULL path in OutputJsonBuilderBuffer) and flow events (p == NULL, falls back to FlowGetPcapFileVars). pcap-filename-pseudo-pkts: verifies pcap_filename is present in fileinfo events generated by stream pseudo-packets (PKT_SRC_STREAM_TCP_DETECTLOG_FLUSH), which carry pcap_v.pfv propagated from the flow. pcap-filename-recursive: two pcap files fed with --pcap-file-recursive. alert.pcap is processed first; http.pcap is processed last. Verifies that the flow event for the alert.pcap connection still reports alert.pcap (via the per-flow pfv), not the stale global that has advanced to http.pcap. This is the primary race-condition regression test for #5255. Ticket: #5255
oferda4
force-pushed
the
test/eve-pcap-filename-alert-and-flow
branch
from
de845a4 to
bae5324
Compare
3 participants
Add a regression test that checks the pcap_filename field is present and correct in EVE JSON output for both alert events (packet-based) and flow events (non-packet-based).
These two code paths use different sources for the filename in OutputJsonBuilderBuffer: per-packet pfv->filename when a packet is available, and the global PcapFileGetFilename() fallback for flow/netflow events where no packet exists.
Previous PR: #2932
v2:
- Fix recursive test and readme.
v3:
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/5255
Suricata PR: OISF/suricata#15176