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

next/635/20241112/v1 #12115

Closed
wants to merge 11 commits into from
Closed

Conversation

jlucovsky and others added 11 commits November 12, 2024 13:29
Remove hard-coded value for the memcap array and substitute compile-time
value for array sizing.

Issue: 845
Issue: 845

Maintain the memcap as an atomic counter so changes through the
unix-socket interface can be supported.
This commit adds memcap/memuse handling to the unix-socket interface:
- ftp
- http-byterange
- host

New stats:
- ippair: memuse, memcap
- host: memuse, memcap
- http-byterange: memuse, memcap
Adds user registerable callbacks for flow initialization, flow
update and flow finish.

Some plugins, such as other DPI libraries like nDPI need a way to hook
into these flow lifecycle events.

Ticket: OISF#7319
Ticket: OISF#7320
For library users and plugins that need to hook into the thread life
cycle, perhaps to initialize some thread storage.
Provide a way for library/plugin users to register a callback that
will be called prior to an EVE record being closed. The callback will
be passed ThreadVars, Packet, and Flow pointers if available, as well
as private user data.
Allows initialization to be done early, so the table is ready for
dynamic registration by plugins which are loaded before signature
setup.
rust-bindings.h was not being installed with "make install-headers",
and its now pulled in by a header used for plugin support, so make
sure its installed.

We first attempt to install the "dist" version if exists, otherwise
install the "gen" one. Also install the "gen" even if the "dist" one
exists, as its going to be newer.
Add documentation about the rule types introduced by 2696fda.

Add doc tags around code definitions that are referenced in the docs.

Task #https://redmine.openinfosecfoundation.org/issues/7031
Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

  • original PRs approved
  • number of commits matches
  • number of line changes matches
  • (CI checks not completed yet)

Copy link

codecov bot commented Nov 12, 2024

Codecov Report

Attention: Patch coverage is 65.33333% with 104 lines in your changes missing coverage. Please review.

Project coverage is 83.26%. Comparing base (278dc24) to head (64a5c10).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12115      +/-   ##
==========================================
+ Coverage   83.23%   83.26%   +0.02%     
==========================================
  Files         906      909       +3     
  Lines      257647   257883     +236     
==========================================
+ Hits       214458   214724     +266     
+ Misses      43189    43159      -30     
Flag Coverage Δ
fuzzcorpus 61.28% <29.61%> (+0.07%) ⬆️
livemode 19.43% <23.18%> (+<0.01%) ⬆️
pcap 44.40% <31.88%> (-0.03%) ⬇️
suricata-verify 62.69% <42.51%> (-0.01%) ⬇️
unittests 59.27% <39.46%> (-0.01%) ⬇️

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

@victorjulien
Copy link
Member Author

Thanks. Waiting for CI and QAlab results.

@suricata-qa
Copy link

Information:

ERROR: QA failed on SURI_TLPR1_alerts_cmp.

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.flow.dcerpc_tcp 40 42 105.0%

Pipeline 23310

@jufajardini
Copy link
Contributor

Thanks. Waiting for CI and QAlab results.

May I also request a re-review of the signature types table, before merge? Columns Inspected and Matches, especially.


.. container:: example-rule

alert :example-rule-emphasis:`http` any any -> any any (msg:"http, no content"; sid:601;)
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I run a test just like that, it still renders as 'app_layer to me'. Only if we add flow:established do I see the change in behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I think this should be mentioned in the docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I just noticed the same:

["pkt","pass tls any any -> any 443 (flow:established; sid:5;)"]
["pkt","pass tcp any any -> any 443 (flow:established; app-layer-protocol:tls; sid:6;)"]
["app_layer","alert http any any -> any any (sid:1;)"]
["app_layer","alert http any any -> any any (sid:2; threshold:type limit, track by_src, count 1, seconds 60;)"]
["pd_only","alert tcp any any -> any any (sid:3; app-layer-protocol:http;)"]
["pd_only","alert tcp any any -> any any (sid:4; app-layer-protocol:http; threshold:type limit, track by_src, count 1, seconds 60;)"]

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add such examples to the rule-types set of SV tests, and have a v8 for the docs with a note about this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reminder: also document the app-layer-protocol and app-layer in the proto field difference.

@victorjulien
Copy link
Member Author

2025200 is the only open rule I see with app_layer, but I don't yet see how it is different other than the threshold keyword.

@victorjulien
Copy link
Member Author

Doc PR needs another revision.

@victorjulien victorjulien deleted the next/635/20241112/v1 branch November 13, 2024 08:34
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.

5 participants