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

Ilya bakhtin/dns dcerpc reversed v5 #12503

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/7111

Describe changes:

  • The first one is intended to improve DCERPC UDP detection. False positives resulted in improper work of the detection framework.
  • The second commit simplifies the detection framework function AppLayerProtoDetectGetProto.
    It previously contained a bug that combined with a false positive in DCERPC resulted in incorrect reporting of DNS flow direction.

SV_BRANCH=OISF/suricata-verify#2266

Rebase of #12134 that I approve of to get a clean green CI

Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.57%. Comparing base (cfbf8fd) to head (317b93c).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #12503   +/-   ##
=======================================
  Coverage   80.56%   80.57%           
=======================================
  Files         925      925           
  Lines      259292   259287    -5     
=======================================
+ Hits       208906   208922   +16     
+ Misses      50386    50365   -21     
Flag Coverage Δ
fuzzcorpus 56.14% <100.00%> (-0.01%) ⬇️
livemode 19.39% <0.00%> (-0.01%) ⬇️
pcap 44.22% <100.00%> (+0.02%) ⬆️
suricata-verify 63.39% <100.00%> (+<0.01%) ⬆️
unittests 58.44% <50.00%> (-0.01%) ⬇️

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

Several additional checks are added to the probing parser to avoid false
detection of DNS as DCERPC

Ticket - 7111
Protocol detection code is simplified. Removed dependency on explicit
alproto constants from the common part of code that must not be aware of
the each specific protocol features.

Ticket - 7111
@catenacyber catenacyber force-pushed the ilya-bakhtin/dns-dcerpc-reversed-v5 branch from d2c55a3 to 317b93c Compare January 29, 2025 21:12
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24434

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.uptime 629 656 104.29%

Pipeline 24440

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24442

@victorjulien victorjulien added this to the 8.0 milestone Jan 30, 2025
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.

4 participants