Ips handling/v24 #7359
Ips handling/v24 #7359
Conversation
Set correct direction for PORT mode, where the server connects to the client. The direction is not also strictly enforced. No data in the wrong direction will be accepted to setup the file or to be added to the file after setup. This also fixes files getting closed twice. Adds some general cleanups. Bug: OISF#3542.
Now that spurious retransmissions don't propegate into the reassembly code, error handling can be simplified.
Ticket: OISF#5202.
If stream session or reassembly memcaps are hit call the memcap policy on the packet and flow.
Apply policy when memcap is reached and no flow could be freed up.
Codecov Report
@@ Coverage Diff @@
## master #7359 +/- ##
==========================================
- Coverage 75.84% 75.80% -0.04%
==========================================
Files 656 657 +1
Lines 190093 190235 +142
==========================================
+ Hits 144172 144213 +41
- Misses 45921 46022 +101
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
On further thought I think the MemcapPolicy thing could even be a more general ExceptionPolicy, where possible exceptions include memcap hits, other resource limit hits, app errors and probably more |
| @@ -899,7 +918,7 @@ static inline void PacketSetAction(Packet *p, const uint8_t a) | |||
|
|
|||
| #define PACKET_ACCEPT(p) PACKET_SET_ACTION(p, ACTION_ACCEPT) | |||
|
|
|||
| #define PACKET_DROP(p) PACKET_SET_ACTION(p, ACTION_DROP) | |||
| //#define PACKET_DROP(p) PACKET_SET_ACTION(p, ACTION_DROP) | |||
There was a problem hiding this comment.
Remove if not used as this is a non-WIP commit.
| } else if ((strcmp(value_str, "bypass") == 0) || | ||
| (strcmp(value_str, "bypass-flow") == 0)) { |
There was a problem hiding this comment.
Both names required? Or can we just use one?
There was a problem hiding this comment.
Can do one. We use bypass in various places in config, but here I used flow-(packet|flow) pattern, so it seemed to make sense to have both.
There was a problem hiding this comment.
Being a new configuration option, I'd rather not see an alias right from the start.
There was a problem hiding this comment.
went with just bypass
| stream_config.ssn_memcap_policy = MemcapPolicyParse("stream.memcap-policy"); | ||
| stream_config.reassembly_memcap_policy = MemcapPolicyParse("stream.reassembly.memcap-policy"); |
There was a problem hiding this comment.
Or should policies be in their own configuration file section:
policy:
stream-memcap: ...
stream-reassembly-memcap: ...
Yes, I agree. I also think it would be more user friendly if this exception policy was configured in one place, rather than having it sprinkled among the various modules. |
Tough one. I feel like it makes sense to have them close to the memcap settings, but I also see the point of grouping them. Ultimately I feel most ppl would never touch these. They would get the implied values ( |
|
ERROR: ERROR: QA failed on tlpw1_files_sha256.
Pipeline 7353 |
|
continued in #7393 |
Only call ThreadInit and ThreadDeinit for custom eve filetypes if they exist. They are not required by all filetypes. Ticket: OISF#7359
Only call ThreadInit and ThreadDeinit for custom eve filetypes if they exist. They are not required by all filetypes. Ticket: OISF#7359
Only call ThreadInit and ThreadDeinit for custom eve filetypes if they exist. They are not required by all filetypes. Ticket: OISF#7359
Implements a new way of dealing with memcap hits: a per subsystem
memcap-policywhich can bedrop-packet,drop-flow,pass-packet,pass-flow,bypassorignore. Whereignoreis essentially the current behavior. Looking for a better name and general feedback on the approach.In OISF/suricata-verify#818 there are tests for most options in combination with stream reassembly memcap failure simulation.
continues work in #7234
(includes #7356)
suricata-verify-pr: 818