Commit Graph

104 Commits (751f3eef3b9ae9ee262b7507646880428cc8a2cd)

Author SHA1 Message Date
Philippe Antoine 256804b7b1 detect/threshold: timeout handling precision improvement
As found by -Wshorten-64-to-32 warnings

Ticket: #6186

Use SCTime_t instead of u32, which increases memory usage for
the structures changed here, while making it more correct.
1 year ago
Jason Ish 60c429e04d detect: add callback for when rate filter changes action
This callback will be called when alert action has been changed due to a
rate filter. The user can then reset or customize the action in their
callback per their own logic.

As the callback is added to the current detection engine, make sure its
copied to the new detection engine on reload.

Ticket: #7673
2 years ago
Jason Ish 1b7496bf5b decode: use BIT macros for flags; consistent naming
Use the BIT_U8 macros for packet alert flags and rename
PACKET_ALERT_RATE_FILTER_MODIFIED to
PACKET_ALERT_FLAG_RATE_FILTER_MODIFIED for consistency.
2 years ago
Jason Ish 22b77b0c56 conf: prefix conf API with SC 2 years ago
Jeff Lucovsky 3d26f917ee var: Use 16-bit container for type
Issue: 6855: Match sigmatch type field in var and bit structs

Align the size and datatype of type, idx, and next members across:
- FlowVarThreshold
- FlowBit
- FlowVar
- GenericVar
- XBit
- DetectVarList

Note that the FlowVar structure has been intentionally constrained to
match the structure size prior to this commit. To achieve this, the
keylen member was restricted to 8 bits after it was confirmed its value
is checked against a max of 0xff.
2 years ago
Victor Julien 98423a0009 detect: constify arguments 2 years ago
Philippe Antoine 26da953f6d util/hash: use randomized hash algorithm
For datasets and http ranges

Ticket: 7209

Prevents abusive hash collisions from known djb2 algorithm
2 years ago
Shivani Bhardwaj 00f7038beb util/thash: add a length getter fn
In order to have access to the length of datatypes with variable lengths
to correctly update memuse to calculate memcaps.

Bug 3910
2 years ago
Victor Julien 12130df21c detect/threshold: implement backoff type
Implement new `type backoff` for thresholding. This allows alerts to be
limited.

A count of 1 with a multiplier of 10 would generate alerts for matching packets:
1, 10, 100, 1000, 10000, 100000, etc.

A count of 1 with a multiplier of 2 would generate alerts for matching packets:
1, 2, 4, 8, 16, 32, etc.

Like with other thresholds, rule actions like drop and setting of
flowbits will still be performed for each matching packet.

Current implementation is only for the by_flow tracker and for per rule
threshold statements.

Tracking is done using uint32_t. When it reaches this value, the rest of
the packets in the tracker will use the silent match.

Ticket: #7120.
2 years ago
Victor Julien 7d4fcc311c detect/threshold: make hash size and memcap configurable 2 years ago
Victor Julien 10eaf550b7 detect/threshold: includes cleanup 2 years ago
Victor Julien 7bcf364095 detect/threshold: expand cache support for rule tracking
Use the same hash key as for the regular threshold storage,
so include gid, rev, tentant id.
2 years ago
Victor Julien 1e9fdc4005 detect/threshold: consider tenant id in tracking
Ticket: #6967.
2 years ago
Victor Julien 2be998fbcd detect/threshold: include rev in threshold tracking 2 years ago
Victor Julien 3471c0f6ad detect/threshold: improve hash function 2 years ago
Victor Julien b8028bf386 thresholds: use dedicated storage
Instead of a Host and IPPair table thresholding layer, use a dedicated
THash to store both. This allows hashing on host+sid+tracker or
ippair+sid+tracker, to create more unique hash keys.

This allows for fewer hash collisions.

The per rule tracking also uses this, so that the single big lock is no
longer a single point of contention.

Reimplement storage for flow thresholds to reuse as much logic as
possible from the host/ippair/rule thresholds.

Ticket: #426.
2 years ago
Victor Julien 3a7247b1ed detect/threshold: minor rate filter cleanup 2 years ago
Victor Julien ab5e04525f detect/threshold: minor code cleanup
Packet pointer is not used during allocation.
2 years ago
Victor Julien 6622dc7444 detect/threshold: minor cleanup 2 years ago
Victor Julien c08c81cacf detect/threshold: implement per thread cache
Thresholding often has 2 stages:

1. recording matches
2. appling an action, like suppress

E.g. with something like:
threshold:type limit, count 10, seconds 3600, track by_src;
the recording state is about counting 10 first hits for an IP,
then followed by the "suppress" state that might last an hour.

By_src/by_dst are expensive, as they do a host table lookup and lock
the host. If many threads require this access, lock contention becomes
a serious problem.

This patch adds a thread local cache to avoid the synchronization
overhead. When the threshold for a host enters the "apply" stage,
a thread local hash entry is added. This entry knows the expiry
time and the action to apply. This way the action can be applied
w/o the synchronization overhead.

A rbtree is used to handle expiration.

Implemented for IPv4.
2 years ago
Victor Julien cfd55ead74 threshold: add by_flow support for global thresholds
Allow rate_filter and thresholds from the global config to specify
tracking "by_flow".
2 years ago
Victor Julien 1552f0953a detect/threshold: implement tracking 'by_flow'
Add support for 'by_flow' track option. This allows using the various
threshold options in the context of a single flow.

Example:

    alert tcp ... stream-event:pkt_broken_ack; \
        threshold:type limit, track by_flow, count 1, seconds 3600;

The example would limit the number of alerts to once per hour for
packets triggering the 'pkt_broken_ack' stream event.

Implemented as a special "flowvar" holding the threshold entries. This
means no synchronization is required, making this a cheaper option
compared to the other trackers.

Ticket: #6822.
2 years ago
Victor Julien 549f7873df detect: spelling 4 years ago
Victor Julien ef5ec842e5 detect/threshold: reword alloc failure message 4 years ago
Jeff Lucovsky 9fbe683642 time: Rework SCTime_t into a struct
Issue: 5718

This commit changes SCTime_t to a struct with members setup as
bitfields.
4 years ago
Jeff Lucovsky 31793aface time: Replace struct timeval with scalar value
Issue: 5718

This commit switches the majority of time handling to a new type --
SCTime_t -- which is a 64 bit container for time:
- 44 bits -- seconds
- 20 bits -- useconds
4 years ago
Victor Julien b31ffde6f4 output: remove error codes from output 4 years ago
Victor Julien 39f5c7f56a error: use SC_EINVAL for invalid input 4 years ago
Victor Julien e042cd785e error: use SC_ENOMEM for alloc errors 4 years ago
Philippe Antoine 62352ad030 src: fix remaining cppclean warnings 4 years ago
Victor Julien 6c200c7793 detect: issue drop to root packet in all cases
Update DROP action handling in tunnel packets. DROP/REJECT action is set
to outer (root) and inner packet.

Check action flags both against outer (root) and inner packet.

Remove PACKET_SET_ACTION macro. Replace with RESET for the one reset usecase.
The reason to remove is to make the logic easier to understand.

Reduce scope of RESET macros.

Rename PacketTestAction to PacketCheckAction except in unittests. Keep
PacketTestAction as a wrapper around PacketCheckAction. This makes it
easier to trace the action handling in the real code.

Fix rate_filter setting actions directly.

General code cleanups.

Bug: #5571.
4 years ago
Victor Julien e250ef6402 debug: remove empty header 4 years ago
Juliana Fajardini e7727c3744 decode: remove unused macros, replace w/ functions
With the recent changes, these macros weren't being used anymore.

Related to
Bug #5458
4 years ago
Juliana Fajardini 1f54e8611a detect/alert: ensure reject action is applied
Bug 5458 states that the reject action is no longer working. While SV
tests that use the reject action still pass, it indeed seems that a
regression has happened with commit aa93984, because while the
function that applies rule actions to the flow (RuleActionToFlow) does
check for the reject action, the newly added function PacketApply
SignatureActions only checks for ACTION_DROP or ACTION_PASS when
deciding to call RuleActionToFlow.

Bug #5458
4 years ago
Juliana Fajardini 1774ff18a6 decode: make PacketDrop use action as parameter
A Packet may be dropped due to several different reasons. This change
adds action as a parameter, so we can update the packet action when we
drop it, instead of setting it to drop.

Related to
Bug #5458
4 years ago
Victor Julien df2e408d96 detect/threshold: fix offline time handling issue
Due to the TIMEVAL_DIFF_SEC calculating the delta into an unsigned
integer, it would underflow to a high positive value leading to
and incorrect result if the packet timestamp was below the timestamp
for the threshold entry. In normal conditions this shouldn't happen,
but in offline mode each thread has its own concept of time which
might differ significantly based on the pcap. In this case the
overflow would be very common.

Changing it to a signed value calculation triggered fuzz undefined
behavior if the packet timeval was very high, so this patch takes a
new approach where it no longer calculates a diff but sets up the
"seconds" value we compare against as a timeval itself, and uses
that to compare.

Fixes: 9fafc1031c ("time: Add TIMEVAL_EARLIER and TIMEVAL_DIFF_SEC macros.")
Fixes: 82dc61f4c3 ("detect/threshold: Refactor threshold calculation to handle by_rule and by_both.")

Uses add `timeradd` specific version where available.

Bug: #5386.
4 years ago
Victor Julien 0035673208 eve/drop: log drop reason
Ticket: #5202.
4 years ago
Philippe Antoine 6fadb97d5d alert: fixes leak in ThresholdHandlePacketRule
ThresholdHandlePacketRule may take ownership of an allocated
DetectThresholdEntry, and places it in a position of the
array th_entry. But it never got released
5 years ago
Mats Klepsland 2a326421aa thresholds: Fix buffer overflow in threshold context
th_entry is resized using ThresholdHashRealloc() every time a rule with
a threshold using by_rule tracking is added. The problem is that this is
done before the rules are reordered, so occasionally a rule with by_rule
tracking gets a higher signature number (after reordering) than the
number of th_entries allocated, causing Suricata to crash.

This commit fixes this by allocating th_entries after all the rules are
loaded and reordered.

Backtrace from core dump:

  Program terminated with signal SIGSEGV, Segmentation fault.

  #0  0x000000000051b381 in ThresholdHandlePacket (p=p@entry=0x7fb0080f3960, lookup_tsh=0x51, new_tsh=new_tsh@entry=0x7fb016c316e0, td=td@entry=0x14adedf0, sid=9800979, gid=1, pa=0x7fb0080f3b18)
      at detect-engine-threshold.c:415
  415>----                if (TIMEVAL_DIFF_SEC(p->ts, lookup_tsh->tv1) < td->seconds) {

Bug #4503.
5 years ago
Mats Klepsland f47e4375b3 thresholds: syntax fixes
Fix syntax of if statement in SigGetThresholdTypeIter()
5 years ago
Mats Klepsland b0b4fab794 thresholds: remove unneeded function argument
Remove packet pointer from SigGetThresholdTypeIter() as it is
unused.
5 years ago
Juliana Fajardini b807059c34 host/storage: use dedicated 'id' type
- Wrap the id in a HostStorageId struct to avoid id confusion
with other storage API calls.
- Fix formatting with clang script.
6 years ago
Juliana Fajardini cf516de587 ippair/storage: use dedicated 'id' type
- Wrap the id in a new IPPairStorageId struct, to avoid id
confusion with other storage API calls.
- Formatting fixes by clang.
6 years ago
Shivani Bhardwaj 6f7d8e50c8 src: use FatalError whenever possible
Replaces all patterns of SCLogError() followed by exit() with
FatalError(). Cocci script to do this:

@@
constant C;
constant char[] msg;
@@

- SCLogError(C,
+ FatalError(SC_ERR_FATAL,
  msg);
- exit(EXIT_FAILURE);

Closes redmine ticket 3188.
6 years ago
Todd Mortimer 50e5b80463 detect/threshold: Add a common function to (re)allocate the by_rule threshold table.
Ensure that the by_rule threshold table is initialized if a rule
is thresholded by_rule. Replace manual table reallocaton with calls
to the common function.
7 years ago
Todd Mortimer 82dc61f4c3 detect/threshold: Refactor threshold calculation to handle by_rule and by_both.
The only difference between threshold calculations for by_src/by_dst,
by_rule or by_both is which table stores the DetectThresholdEntry.
Refactor the ThresholdHandlePacket* functions to do table lookup and
storage individually, but calculate thresholds in a common function.
7 years ago
Victor Julien 990e53222e threshold: minor cleanups 9 years ago
Ruslan Usmanov fb87d21ec7 rate_filter: by_rule fixed triggering algorithm
Fixes issue #2258

Program was triggering rate_filter by_rule earlier than needed
and generally behaved like a threshold.
9 years ago
Victor Julien 50a762bfd1 thresholds: fix issues with host based thresholds
The flow manager thread (that also runs the host cleanup code) would
sometimes free a host before it's thresholds are timed out. This would
lead to misdetection or too many alerts.

This was mostly (only?) visible on slower systems. And was caused by a
mismatch between time concepts of the async flow manager thread and the
packet threads, resulting in the flow manager using a timestamp that
was before the threshold entry creation ts. This would lead to an
integer underflow in the timeout check, leading to a incorrect conclusion
that the threshold entry was timed out.

To address this,  check if the 'check' timestamp is not before the creation
timestamp.
9 years ago
Ruslan Usmanov 1090ee9d8d rate_filter by_both through IPPair storage
Ticket https://redmine.openinfosecfoundation.org/issues/2127
9 years ago