Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detect/iponly: use flow first flags #12460

Closed
wants to merge 1 commit into from

Conversation

victorjulien
Copy link
Member

Instead of ip-only specific flags, reuse the FLOW_PKT_TOSERVER_FIRST and FLOW_PKT_TOCLIENT_FIRST flags.

Fixes false positives on one sided streams that trigger a opposing flow timeout packet at the flow's end. That pseudo packet would trigger a match even though it shouldn't.

Ticket: #7521.

SV_BRANCH=OISF/suricata-verify#2251

https://redmine.openinfosecfoundation.org/issues/7521

Instead of ip-only specific flags, reuse the FLOW_PKT_TOSERVER_FIRST and
FLOW_PKT_TOCLIENT_FIRST flags.

Fixes false positives on one sided streams that trigger a opposing flow
timeout packet at the flow's end. That pseudo packet would trigger a
match even though it shouldn't.

Ticket: OISF#7521.
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.65%. Comparing base (95e8427) to head (897d1f3).
Report is 50 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12460      +/-   ##
==========================================
+ Coverage   80.63%   80.65%   +0.01%     
==========================================
  Files         920      920              
  Lines      258704   258687      -17     
==========================================
+ Hits       208595   208632      +37     
+ Misses      50109    50055      -54     
Flag Coverage Δ
fuzzcorpus 56.83% <33.33%> (+0.01%) ⬆️
livemode 19.39% <25.00%> (-0.01%) ⬇️
pcap 44.29% <33.33%> (-0.04%) ⬇️
suricata-verify 63.26% <100.00%> (-0.01%) ⬇️
unittests 58.51% <35.71%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24333

Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work

CI : ✅
Code : good, nice for adding vacancy
Commits segmentation : ok
Commit messages : good
Git ID set : looks fine for me
CLA : you already contributed
Doc update : not needed
Redmine ticket : ok
Rustfmt : no rust
Tests : nice, thanks
Dependencies added: none

@victorjulien victorjulien added this to the 8.0 milestone Jan 28, 2025
@victorjulien
Copy link
Member Author

Merged in #12499, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants