Commit Graph

705 Commits (647e878f7cec255c4f634dd1d9e27cd26f6aa9fb)

Author SHA1 Message Date
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.
1 year ago
Victor Julien ce727cf4b1 detect: remove unnecessary detect thread flags stores 1 year ago
Philippe Antoine dc043d0297 detect: remove unused field
content_inspect_window is used in app-layer-smtp, but
not directly in detect-file-data
1 year ago
Victor Julien 956c8bebd1 detect/prefilter: use sig mask to exclude pkt engines
Add an argument to the packet prefilter registration function to include
`SignatureMask` flags. This will be used at runtime to only call these
prefilter engines when the mask check passes.
1 year ago
Victor Julien 4c2960169c detect/prefilter: minor function ptr cleanup
Use a typedef'd function pointer for packet Prefilter callbacks to make
the code consistent with the other callbacks.
1 year ago
Victor Julien d03660a646 detect: skip pseudo packets if sig needs real pkt
If a signature uses a condition that requires a real packet, filter
out pseudo packets as early as possible. To do this, the SignatureMask
logic is used.

This allows for the removal of checks for pseudo packets in individual
keywords `Match` functions, which will be done in a follow up commit.

Update analyzer to output the new flag.

Ticket: #7002.
1 year ago
Philippe Antoine 4bbe7d92dc detect: helper to have pure rust keywords
detect: make number of keywords dynamic

Ticket: 4683
1 year ago
Victor Julien 92581dbc06 detect: set ACTION_ALERT for rules that should alert
Replaces default "alert" logic and removed SIG_FLAG_NOALERT.

Instead, "noalert" unsets ACTION_ALERT. Same for flowbits:noalert and
friends.

In signature ordering rules w/o action are sorted as if they have 'alert',
which is the same behavior as before, but now implemented explicitly.

Ticket: #5466.
1 year ago
Jason Ish 10e6028175 lua: track memory limit exceede errors
Update the Lua allocated to set a code on memory allocation limit
exceeded errors so an appropriate error message can be logged and a
state incremented.

Fixes the tracking of the allocated size by using the difference
between original size, and new size and toss in some debug
validations.
1 year ago
Jason Ish 5a1cba72f0 lua: add logging and counter for instruction limit being exceeded 1 year ago
Jason Ish c8fa454cb2 lua: add blocked functions as a special log type plus stat
Distinguish between a generic Lua script error and an error created by a
function being blocked, so each is logged once respective of each other.

Also add a stat that is incremented when a script fails due to a
blocked function.

NOTE: This does not catch calls to functions that are blocked by not
having the library loaded, such as "io.open", as they are blocked by
not even loading the "io" library.
1 year ago
Philippe Antoine ce16a56a1f detect: unify functions for multi-buffer
Ticket: 6575

Multi buffers keywords now use a single registration function
DetectAppLayerMultiRegister with a GetBuffer argument.

This GetBuffer function pointer is similar to the ones used by
single-buffer keyword, except that it takes an additional
parameter which is the index of the buffer to get.
Under the hood, an anonymous union between these 2 functions
pointers types is used.

In the end, this deduplicates code, especially the calls to
DetectEngineContentInspection
1 year ago
Jason Ish 224f55ba21 detect/lua: don't treat a crashed script as no match
If a rule script crashed, the return value was treated as a no
match. This would make a negation of the rule match and alert.

Instead cleanup and exit early if the rule script crashed and don't
run negation logic.

A stat, detect.lua.errors has been added to count how many times a
script crashes.

Also consolidates the running of the Lua script and return value
handling to a common function.

Bug: #6940
1 year ago
Philippe Antoine 910f6af54f output: do not use tx id 0 when there is no tx
Ticket: 6846

This led to packet rules logging irrelevant app-layer data
1 year ago
Sascha Steinbiss 120313f4da ja4: implement for TLS and QUIC
Ticket: OISF#6379
1 year ago
Philippe Antoine b113bdd9e3 src: remove unused headers-exported functions
+ remove double definition of IPPairLock

Ticket: #4083
1 year ago
Jason Ish 44388f1b69 src: make include guards more library friendly
Include guards for libraries should use a prefix that is meaningful for
the library to avoid conflicts with other user code. For Suricata, use
SURICATA.

Additionally, remove the pattern of leading and trailing underscores as
these are reserved for the language implementation per the C and C++
standards.
2 years ago
Shivani Bhardwaj 54558f1b4a util/interval-tree: add utility fns
Add new utility files to deal with the interval trees. These cover the
basic ops:
1. Creation/Destruction of the tree
2. Creation/Destruction of the nodes

It also adds the support for finding overlaps for a given set of ports.
This function is used by the detection engine is the Stage 2 of
signature preparation.

Ticket 6792
Bug 6414

Co-authored-by: Victor Julien <vjulien@oisf.net>
2 years ago
Victor Julien 976d8e65ae detect/iponly: move parsing only fields to init_data
IP-only parse results were not used at runtime.
2 years ago
Philippe Antoine db99c45d23 detect: errors on 65k filestore signatures
Errors when a detection engine gets 65k filestore signatures to
avoid the hard limit to have 65k filestore per signature group
head

Ticket: #6393
2 years ago
Shivani Bhardwaj 395c74d81e detect/engine: set max sig ID per SGH
Present scenario
----------------
Currently, as a part of setting signature count per SGH, a max_idx is
passed which could be as high as the highest signature number (internal
ID).

Issue
-----
Not every SGH needs to evaluate all the signatures while setting
the signature count or while creating the match_array.
In a nonideal scenario, when say, there are 2 SGHs and one SGH has 2
signatures and the other one has 60k, given the current scheme of
evaluating max_idx, the max_idx will be set to 60k, and this shall
later be passed on to SigGroupHeadSetSigCnt or
SigGroupHeadBuildMatchArra which shall traverse over all the 60k sigs
for either SGHs.

Other info
----------
This is a very fast operation as the internal arithmetic is done
bitwise.

Patch
-----
The functions SigGroupHeadSetSigCnt and SigGroupHeadBuildMatchArray can
be optimized by storing the max signature id (internal) per SGH (which
also seemed to be the initial intention as per fn comments).
As a result of this, the sig_array is only walked up until the max sig
id of that respective SGH.
2 years ago
Shivani Bhardwaj 264101ba22 detect: remove unused port in SigGroupHeadInitData
port is not used and logically makes sense to not be in this struct as
this struct is already referenced by DetectPort itself as a part of
SigGroupHead.
2 years ago
Shivani Bhardwaj 26b81ca007 detect: make SigMatch.is_last bool
It is used like bool so much so that nothing needs to be changed even
after changing its type.
2 years ago
Shivani Bhardwaj 588af05565 detect: remove unneeded size in DetectEngineCtx
sig_array_size can easily be calculated with length and is only used at
one place for debugging purposes. Remove it from the DetectEngineCtx
struct to avoid making it unnecessarily heavy.
2 years ago
Victor Julien 3b8ed937d7 detect: remove DCERPC mask logic
Added nothing over alproto check already in place.
2 years ago
Victor Julien db2484276e detect: shrink sgh to have all runtime members on one cache line 2 years ago
Jason Ish 5d5b0509a5 requires: add requires keyword
Add a new rule keyword "requires" that allows a rule to require specific
Suricata versions and/or Suricata features to be enabled.

Example:

  requires: feature geoip, version >= 7.0.0, version < 8;
  requires: version >= 7.0.3 < 8
  requires: version >= 7.0.3 < 8 | >= 8.0.3

Feature: #5972

Co-authored-by: Philippe Antoine <pantoine@oisf.net>
2 years ago
Jason Ish 66ff23f9bf detect: rename InspectEngineFuncPtr2 to InspectEngineFuncPtr
Version 1 of the API no longer exists.
2 years ago
Victor Julien 06c809573b detect/content-inspect: optimize struct layout
Move members used by DetectEngineContentInspection() to the same cache line.
2 years ago
Victor Julien 0014077a36 detect: optimize struct layout
Move reference count to top of DetectEngineThreadCtx, to move it to the
same cache line as the other members that are checked first in Detect().
2 years ago
Victor Julien 4cce7ba48b detect/content-inspect: localize recursion counting
Use stack local var instead of DetectEngineThreadCtx member. Instead
setup a stack local struct that both counts and holds the limit. Make sure
the limit is a const so we can avoid rereading it.

This is part of an effort to reduce the size of the DetectEngineThreadCtx
structure and reduce the number of memory writes to it. Additionally, it
is part of an effect to reduce the number of places where detection
tracks various forms of state.
2 years ago
Shivani Bhardwaj 77eb85e224 detect: remove misleading comment
The comment seems to have come from the enum for addresses where IPv4
and IPv6 matters.
2 years ago
Shivani Bhardwaj 2b73a17bb0 detect: rename whitelist to score
The term "whitelist" is actually used to store a list of DetectPort type
items for tcp and udp in detect.h. Using the same term for also keeping
the score that affects the grouping of rules is confusing. So, rename
the variable to "score".
2 years ago
Victor Julien 8ba7f23c9b detect/content: use const pointer where possible 2 years ago
Victor Julien 15b545d16f detect: improve explanation of offset tracking 2 years ago
Victor Julien 7f42506760 detect: reimplement discontinue matching logic
Previously various steps in the content inspection logic would use
a variable in the DetectEngineThreadCtx to flag that matching should
be discontinued.

This patch reimplements this logic by using a new return code instead.

Split content inspection into public and private version, so that
common initialization can be done in a single place.

Update the callsites.
2 years ago
Philippe Antoine c272a646c5 detect: SigMatchAppendSMToList can fail
Ticket: #6104

And failures should be handled to say that the rule failed to load

Reverts the fix by 299ee6ed55
that was simple, but not complete (memory leak),
to have this bigger API change which simplifies code.
2 years ago
Jeff Lucovsky 9bd2b7425d general/bool: Change Suricata int to bool
Change Suricata operational values from int to bool.
2 years ago
Victor Julien 68a2fcaad3 mpm: thread ctx cleanups
Remove unused thread ctx' from AC variants

Use single thread store in detection.

Minor cleanups.
2 years ago
Victor Julien 58c7a438ed detect/flow: optimize only_stream/no_stream options
Until now the implementation would scan the stream, fallback to the
packet payload in exception cases, then keep track of where the match
was and in the flow match logic reject the match if it was in the wrong
buffer.

This patch simplifies this logic, by refusing to inspect the packet
payload when `only_stream` is set.

To do this the `only_stream`/`no_stream` options are now translated
to the pseudo protocols `tcp-stream` and `tcp-pkt` at parsing, so that
the `flow` keyword doesn't have to evaluate these conditions anymore.
2 years ago
Victor Julien e9c1ca2804 detect: fix legacy modifiers leading to multi-buffer
Fix non-continious matches with content and pcre modifiers setting up
multiple buffers.

To address this store whether a buffer is multi-capable and if not reuse
an earlier buffer if possible.

Bug: #6397.

Fixes: ad88efc2d8 ("detect: support multi buffer matching")
2 years ago
Philippe Antoine 299ee6ed55 detect: check if signature uses too many buffers
Ticket: #6104

The approach in master branch is to change the prototype of
SigMatchAppendSMToList so that it allocates itself the new SigMatch
This approach requires to change all the 100-ish calls to
SigMatchAppendSMToList and is thus quite a big change.

For branch 7, we still wanted to avoid the buffer overflow, but
did not want such an intrusive change, and still wanted to make
the signature invalid. Instead of changing the prototype of the
function, we make it return early, and set a flag in the signature
which can be later checked by SigValidate
2 years ago
Victor Julien 6ba0956a75 multi-tenant: allow reload w/o yaml path
Store yaml path in de ctx, for reloads w/o path.

This allows for a simpler `reload-tenant N`, where the previously
used yaml is reloaded.
2 years ago
Jeff Lucovsky c8615bcd47 detect/analysis: Move globals to engine ctx
Issue: 6239

This commit moves the global variables associated with engine analysis
into the detect engine context. Doing so provides encapsulation of the
analysis components as well as thread-safe operation in a multi-tenant
(context) deployment.
2 years ago
Jeff Lucovsky 9fd77c737f detect/multi-tenant: Make tenant_id 32 bits everywhere
Issue: 6047

This commit ensures that the tenant id is contained in a unsigned 32 bit
container.
2 years ago
Victor Julien e2f4c751aa reference: fix multi-tenant loading issues
Bug: #4797.
2 years ago
Victor Julien 2859eeae81 classification: fix multi-tenant loading issues
Move pcre2 data structures used for parsing into the detect engine
context, so that multiple tenant loading threads don't use the same
data structures.

Bug: #4797.
2 years ago
Victor Julien 2cac440f7d detect/filemagic: fix thread ctx registration; reloads
Make sure thread ctx registration happens and id remains correct
in case of reloads.

To do so, move id var into the detect ctx.
2 years ago
Victor Julien 8417d407be detect: more compact layout of DetectEngineCtx 2 years ago
Victor Julien 377f2d7e1e detect: reduce failure_fatal to bool to save space 2 years ago