Commit Graph

746 Commits (60c429e04db651e92a37b7aadc330591ab387c41)

Author SHA1 Message Date
Jason Ish 5a1cba72f0 lua: add logging and counter for instruction limit being exceeded 2 years 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.
2 years 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
2 years 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
2 years 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
2 years ago
Sascha Steinbiss 120313f4da ja4: implement for TLS and QUIC
Ticket: OISF#6379
2 years ago
Philippe Antoine b113bdd9e3 src: remove unused headers-exported functions
+ remove double definition of IPPairLock

Ticket: #4083
2 years 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.
3 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
3 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.
3 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.
3 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.
3 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.
3 years ago
Victor Julien 3b8ed937d7 detect: remove DCERPC mask logic
Added nothing over alproto check already in place.
3 years ago
Victor Julien db2484276e detect: shrink sgh to have all runtime members on one cache line 3 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>
3 years ago
Jason Ish 66ff23f9bf detect: rename InspectEngineFuncPtr2 to InspectEngineFuncPtr
Version 1 of the API no longer exists.
3 years ago
Victor Julien 06c809573b detect/content-inspect: optimize struct layout
Move members used by DetectEngineContentInspection() to the same cache line.
3 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().
3 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.
3 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.
3 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".
3 years ago
Victor Julien 8ba7f23c9b detect/content: use const pointer where possible 3 years ago
Victor Julien 15b545d16f detect: improve explanation of offset tracking 3 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.
3 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.
3 years ago
Jeff Lucovsky 9bd2b7425d general/bool: Change Suricata int to bool
Change Suricata operational values from int to bool.
3 years ago
Victor Julien 68a2fcaad3 mpm: thread ctx cleanups
Remove unused thread ctx' from AC variants

Use single thread store in detection.

Minor cleanups.
3 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.
3 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")
3 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
3 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.
3 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.
3 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.
3 years ago
Victor Julien e2f4c751aa reference: fix multi-tenant loading issues
Bug: #4797.
3 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.
3 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.
3 years ago
Victor Julien 8417d407be detect: more compact layout of DetectEngineCtx 3 years ago
Victor Julien 377f2d7e1e detect: reduce failure_fatal to bool to save space 3 years ago
Victor Julien fd36459c96 spm: reduce spm_matcher size to uint8_t
No more space is needed.
3 years ago
Philippe Antoine f2a18e91c4 rust: define AppLayerEventType only in rust
And detect.h does no longer depend on app-layer-events.h
3 years ago
Victor Julien 2696fda041 detect: use explicit rule types
Instead of using flags to indicate a rule type, use an explicit `type`
field.

This will make it more clean in code paths what paths a rule is taking,
and will allow easier debugging as well as analyzer output.

Define the following fields:

- SIG_TYPE_IPONLY: sig meets IP-only criteria and is handled by the IP-only
  engine.
- SIG_TYPE_PDONLY: sig inspects protocol detection results only.
- SIG_TYPE_DEONLY: sig inspects decoder events only.
- SIG_TYPE_PKT:    sig is inspected per packet.
- SIG_TYPE_PKT_STREAM: sig is inspected against either packet payload or
  stream payload.
- SIG_TYPE_STREAM: sig is inspected against the reassembled stream
- SIG_TYPE_APPLAYER: sig is inspected against an app-layer property, but not
  against a tx engine.
- SIG_TYPE_APP_TX: sig is inspected the tx aware inspection engine(s).

Ticket: #6085.
3 years ago
Philippe Antoine d47dba27bf all: remove unused literals 3 years ago
Victor Julien b591813b86 profiling/rules: reduce sync logic scope
Use a simple once a second scheme.
3 years ago
Eric Leblond 020cfbcd61 profiling: introduce rules profiling
Performance measurement of rules is important on live Suricata
as bad rules can cause severe performance regression. This patch
introduces the --enable-profiling-rules that activate profiling
for the rules. This reduces the performance impact of full
profiling  and provide visiblity on the rules performance at
the same time.
3 years ago
Victor Julien da89164ba1 detect: spelling: DetectBufferMpmRegistry 3 years ago
Victor Julien 29d59ee155 detect: spelling: SIG_FLAG_INIT_PRIO_EXPLICIT 3 years ago
Victor Julien ad88efc2d8 detect: support multi buffer matching
Multi buffer matching is implemented as a way for a rule to match
on multiple buffers within the same transaction.

Before this patch a rule like:

    dns.query; content:"example"; dns.query; content:".com";

would be equivalent to:

    dns.query; content:"example"; content:".com";

If a DNS query would request more than one name, e.g.:

    DNS: [example.net][something.com]

Eeach would be inspected to have both patterns present. Otherwise,
it would not be a match. So the rule above would not match, as neither
example.net and somthing.com satisfy both conditions at the same time.

This patch changes this behavior. Instead of the above, each time the
sticky buffer is specified, it creates a separate detection unit. Each
buffer is a "multi buffer" sticky buffer will now be evaluated against
each "instance" of the sticky buffer.

To continue with the above example:

    DNS: [example.net] <- matches 'dns.query; content:"example";'
    DNS: [something.com] <- matches 'dns.query; content:".com"'

So this would now be a match.

To make sure both patterns match in a single query string, the expression
'dns.query; content:"example"; content:".com";' still works for this.

This patch doesn't yet enable the behavior for the keywords. That is
done in a follow up patch.

To be able to implement this the internal storage of parsed rules
is changed. Until this patch and array of lists was used, where the
index was the buffer id (e.g. http_uri, dns_query). Therefore there
was only one list of matches per buffer id. As a side effect this
array was always very sparsely populated as many buffers could not
be mixed.

This patch changes the internal representation. The new array is densely
packed:

    dns.query; content:"1"; dns.query; bsize:1; content:"2";

    [type: dns_query][list: content:"1";]
    [type: dns_query][list: bsize:1; content:"2";]

The new scheme allows for multiple instances of the same buffer.
These lists are then translated into multiple inspection engines
during the final setup of the rule.

Ticket: #5784.
3 years ago
Victor Julien afd57dc845 detect: remove unused thread ctx members 3 years ago
Victor Julien 4f1d61169c detect: make DetectEngineCtx more compact 3 years ago
Victor Julien 0ff28a482b detect: remove unused group counters 3 years ago
Victor Julien 9f0bea5410 detect: remove DetectEngineCtx members
Members were set but not checked.
3 years ago
Victor Julien 9ce967da53 detect: remove unused field 3 years ago
Justin Azoff aacb7dc291 detect/iponly: remove DetectEngineIPOnlyThreadCtx
This is unused.

Issue: 4578
3 years ago
Justin Azoff dfbc3da0eb detect/iponly: Reduce the size of the SigNumArray bitsets
Instead of tracking ip only rules by the internal signum, track them by
a separate counter that starts at zero.  This results in dense
SigNumArrays instead of sparse ones and a much smaller max_idx.

Issue: 4578
3 years ago
Victor Julien e624328deb detect: split mpm per alproto for file.data & others
Instead of a shared mpm context for just "file.data" or "file.magic"
use per alproto mpms. This way http file.data rules won't affect smb
file.data performance.

Ticket: #4378.
3 years ago
Shivani Bhardwaj 901e99fa95 detect: add comments for DETECT_SM_LIST_* types 3 years ago
Victor Julien 9e41075d5d detect/frames: improve IPS and GAP handling
Inspect individual chunks in lossy traffic.

Don't use the frame idx as the inspection buffer idx. Engines are running
per frame, so multi inspection can be used for stream chunks instead.

Ticket: #4977.
4 years ago
Victor Julien 70125a29f6 detect/buffer: add initialized flag to simplify buffer logic 4 years ago
Victor Julien aa376a3b21 detect/frame: improve frame detection
Add a per frame progress tracker.
4 years ago
Todd Mortimer 7d1a8cc335 file/swf: Use lzma-rs decompression instead of libhtp.
Use the lzma-rs crate for decompressing swf/lzma files instead of
the lzma decompressor in libhtp. This decouples suricata from libhtp
except for actual http parsing, and means libhtp no longer has to
export a lzma decompression interface.

Ticket: #5638
4 years ago
Philippe Antoine 4706083929 detect: avoids memory leak on ja3 signature parsing
If a later keyword enforces a protocol incompatible with ja3
4 years ago
Victor Julien a6e86fef04 detect: remove unused data struct 4 years ago
Victor Julien 3263202094 detect/tx: add AppLayerTxData to PrefilterTx
In preparation of some file inspection optimizations, for which we need the
tx data.

Update all users.
4 years ago
Victor Julien 79499e4769 app-layer: move files into transactions
Update APIs to store files in transactions instead of the per flow state.

Goal is to avoid the overhead of matching up files and transactions in
cases where there are many of both.

Update all protocol implementations to support this.

Update file logging logic to account for having files in transactions. Instead
of it acting separately on file containers, it is now tied into the
transaction logging.

Update the filestore keyword to consider a match if filestore output not
enabled.
4 years ago
Victor Julien 040404b093 detect/profiling: track bytes scanned by prefilter engines 4 years ago
Philippe Antoine 02f2602dde src: rework includes as per cppclean 4 years ago
Victor Julien 8109b0017e detect/dcerpc: simplify keyword validation
Now that the engine understands the relation between SMB and DCERPC better
we can get rid of some of the special case handling in keywords.
4 years ago
Philippe Antoine b8524e70d9 detect: change InspectEngineFuncPtr2 to return uint8_t 4 years ago
Philippe Antoine c5cf2967b3 detect: fix integer warnings
Ticket: #4516
4 years ago
Jason Ish c8a5207083 detect: introduce "like" ip-only signature type
Rules that look like they should be IP-only but contain a negated rule
address are now marked with an LIKE_IPONLY flag. This is so they are
treated like IPONLY rules with respect to flow action, but don't
interfere with other IPONLY processing like using the radix tree.

Ticket: #5361
4 years ago
Victor Julien ffef10c5d7 detect: address parsing variable rename to match code style 4 years ago
Victor Julien 51ef6f4e3a detect/iponly: remove unused code 4 years ago
Juliana Fajardini 192360aa05 detect: update copyright years 4 years ago
Juliana Fajardini 877b32c1e4 detect/stats: log out total of suppressed alerts
Related to
Task #4943
Task #5179
4 years ago
Juliana Fajardini 8616c90fe7 detect/stats: log out total of discarded alerts
Add a counter to our stats log with the total of alerts that have been
discarded due to packet alert queue overflow.

Task #5179
4 years ago
Juliana Fajardini 88805f03ee detect/alert: add infra for new alert queue
Initial work to bring part of the alert queue processing to
DetectEngineThreadCtx.

Task #4943
4 years ago
Victor Julien 54a6dd09dd detect: pattern id assignment through hash table
Only consider active part of the pattern for mpm (so consider chop).

Move data structure to hash list table over the custom array logic.
4 years ago
Victor Julien a14854bce9 detect: keyword list to hash to improve perf
Since the switch to pcre2 this was much more heavily used, which
would lead to measurable time spent in list handling.
4 years ago
Victor Julien 3352c0bee4 detect: initialization optimization
A lot of time was spent in `SigMatchListSMBelongsTo` for the `mpm_sm`.

Optimize this by keeping the value at hand during Signature parsing and
detection engine setup.
4 years ago
Victor Julien f6f124f283 detect/engine: support frames
Implement the low level detect engine support for inspecting frames,
including MPM, transforms and inspect API's.
5 years ago
Victor Julien c0ec3984fa eve/alert: add support for logging frame
If detection was done in a frame, the frame will be added to the
eve.alert output.
5 years ago
Victor Julien ac11502629 detect/engine: store buffer name in local array
Instead of storing a name and description as a pointer in DetectBufferType
store them in fixed size arrays. This is in preparation of runtime registration
of buffer types, where a constant name/desc is not available.
5 years ago
Victor Julien ecce116117 detect/fast_pattern: allow for rule time registration
In preparation of more dynamic logic in rule loading also doing
some registration, allow for buffers to be registered as fast_patterns
during rule parsing.

Leaves the register time registrations mostly as-is, but copies the
resulting list into the DetectEngineCtx and works with that onwards.
This list can then be extended.
5 years ago
Victor Julien 5bcaae0a01 detect: use hashes for all buffer to id
Instead of a map that is constantly realloc'd, use 2 hash tables for
DetectBufferType entries: one by name (+transforms), the other by
id. Use these everywhere.
5 years ago
Victor Julien 51dcf3d76a detect: increase SigMatch type from u8 to u16 5 years ago
Victor Julien bb3d49d5bf detect: use bool for uint16_t used as bool 5 years ago
Victor Julien 88bb23b7cf detect/prefilter: update tx_min_progress to uint8_t
Now that our make progress value is 47, we don't need an int.
5 years ago
Victor Julien 932cf0b6a6 detect: track prefilter by progress, not engine
Fix FNs in case of too many prefilter engines. A transaction was tracking
which engines have run using a u64 bit array. The engines 'local_id' was
used to set and check this bit. However the bit checking code didn't
handle int types correctly, leading to an incorrect left shift result of
a u32 to a u64 bit value.

This commit addresses that by fixing the int handling, but also by
changing how the engines are tracked.

To avoid wasting prefilter engine tracking bit space, track what
ran by the progress they are registered at, instead of the individual
engine id's. While we can have many engines, the protocols use far
fewer unique progress values. So instead of tracking for dozens of
prefilter id's, we track for the handful of progress values.

To allow for this the engine array is sorted by tx_min_progress, then
app_proto and finally local_id. A new field is added to "know" when
the last relevant engine for a progress value is reached, so that we
can set the prefilter bit then.

A consquence is that the progress values have a ceiling now that
needs to fit in a 64 bit bitarray. The values used by parsers currently
does not exceed 5, so that seems to be ok.

Bug: #4685.
5 years ago
Victor Julien aa93984b7e detect: unify alert handling; fix bugs
Unify handling of signature matches between various rule types and
between noalert and regular rules.

"noalert" sigs are added to the alert queue initially, but removed
from it after handling their actions. This way all actions are applied
from a single place.

Make sure flow drop and pass are mutually exclusive.

The above addresses issue with pass and drops not getting applied
correctly in various cases.

Bug: #4663
Bug: #4670
5 years ago
Victor Julien dfe71bb773 detect: remove ticker
Last consumer of it has been converted.
5 years ago
Joshua Lumb cf9b2b5fd1 detect-dsize: Add ! operator for dsize matching 5 years ago
Victor Julien 86681c9d7c detect: move init only array to init data 5 years ago
Victor Julien a2e37522bb detect/analyze: dump patterns facility
Dump all patterns to `patterns.json`, with the pattern, a total count (`cnt`),
count of how many times this pattern is the mpm (`mpm`) and some of the flags.

Patterns are listed per buffer. So payload, http_uri, etc.
5 years ago
Victor Julien e611adf3dc detect: set event if max inspect buffers exceeded
If a parser exceeds 1024 buffers we stop processing them and
set a detect event instead. This is to avoid parser bugs as well as
crafted bad traffic leading to resources starvation due to excessive
loops.
5 years ago
Victor Julien 3dc50322db detect: fix multi inspect buffer issue; clean up
Fix multi inspect buffer API causing cleanup logic in the single
inspect buffer paths. This could lead to a buffer overrun in the
"to clear" logic.

Multi buffers now use InspectionBufferSetupMulti instead of
InspectionBuffer. This is enforced by a check in debug validation.

Simplify the multi inspect buffer setup code and update the callers.
5 years ago
Victor Julien 975062cf40 detect: track base id for xform buffers
Buffers with transforms are based on the non-transformed "base"
buffer, with a new ID assigned and the transform callbacks added.

This patch stores the id of the original buffer in the new buffer
inspect and prefilter structures. This way the buffers with and
without transforms can share some of the logic are progression
of file and body inspection trackers.

Related tickets: #4361 #4199 #3616
6 years ago
Victor Julien d694dac5a8 detect: remove inspect v1 API 6 years ago