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 revflow 7552 v3 #12676

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7552

Describe changes:

  • Fix detection when reversing flow

SV_BRANCH=OISF/suricata-verify#2320

#12672 with review taken into account, using Flow::de_ctx_version to have only one code doing the flow/detect reset

Ticket: 7552

When we use midstream, and the first packet we see of a flow is
a response from server, and we want to match on some signature
to client :
- we had first set sgh_toserver/FLOW_SGH_TOSERVER as we first
  thought this was a packet to server
- we then swap/reverse the flow, so sgh_toclient becomes sgh_toserver
  but it contains signatures to server and cannot match our
  to_client signature

The detect engine with DetectRunSetup will set again the
signatures group heads properly
Ticket: 7552

f->sgh_toserver may be NULL but because FLOW_SGH_TOSERVER is unset
and thus, we want to delay cleanup until detection has really been
run with the right signature group head.

This may happen for a rule using
`alert tcp any any -> any any` and
a app-layer keyword to client
with a app-layer supporting both udp and tcp
with stream.midstream=true
and with the first packet of a flow being a server response

In this case, we swap the flow and reset its signature group heads
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.77%. Comparing base (3bc2a14) to head (43c2134).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12676      +/-   ##
==========================================
- Coverage   80.77%   80.77%   -0.01%     
==========================================
  Files         932      932              
  Lines      259517   259523       +6     
==========================================
- Hits       209629   209626       -3     
- Misses      49888    49897       +9     
Flag Coverage Δ
fuzzcorpus 57.02% <100.00%> (+0.03%) ⬆️
livemode 19.36% <0.00%> (-0.01%) ⬇️
pcap 44.15% <80.00%> (-0.01%) ⬇️
suricata-verify 63.52% <100.00%> (+0.01%) ⬆️
unittests 58.33% <87.50%> (+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 24894

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.

2 participants