Commit Graph

578 Commits (10150e95ad0c6aa9e5af6f39f709e87b3b3b0ba9)

Author SHA1 Message Date
Victor Julien ce727cf4b1 detect: remove unnecessary detect thread flags stores 1 year ago
Philippe Antoine 4bbe7d92dc detect: helper to have pure rust keywords
detect: make number of keywords dynamic

Ticket: 4683
1 year ago
Philippe Antoine 20423fdd38 style: remove some useless return
and remove empty line before end of function
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
Jason Ish d2537361f4 run-mode: remove duplicate var; add setter function
Remove the global "run_mode" var as it was a duplicate of the runmode on
the "instance" struct. For direct access outside of suricata.c, use the
getter function.

Also expose a setter function for unit tests that need to change it.
1 year ago
Philippe Antoine b113bdd9e3 src: remove unused headers-exported functions
+ remove double definition of IPPairLock

Ticket: #4083
1 year 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
Jason Ish b11bb1c412 detect: rename DetectAppLayerInspectEngineRegister2
Rename DetectAppLayerInspectEngineRegister2 to
DetectAppLayerInspectEngineRegister as there is no other variant of
this function, and the versioning with lack of supporting
documentation can lead to confusion.
2 years ago
Victor Julien b1fa975503 detect/content-inspect: remove const casting 2 years ago
Victor Julien 6a73b3c90b mpm: remove ac-bs implementation
Ticket: #6586.
2 years ago
Victor Julien ee7c140028 detect: minor cleanup
MPM_HS does not need a guard.
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
Victor Julien ee66a7246f detect-engine: minor content inspection cleanup 2 years ago
Jeff Lucovsky ec1482cf48 calloc: Use nmemb with SCCalloc
This commit modifies calls to SCCalloc that had a member count of 1 and
a size count calculated as: element_count * sizeof(element).
2 years ago
Jeff Lucovsky 193e0ea1a9 memory/alloc: Use SCCalloc instead of malloc/memset 2 years ago
Thomas Winter 7d40a9f178 rule-reload: Release excess memory freed during engine reload
The hot reload results in large chunks of memory being freed as the
as the old signature tables are discarded. Help the memory management
system along by telling to release as much memory as it can at this
point.

Bug: #6454.
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 4a079541b2 detect: fix inspect engine return codes
Use proper inspect engine codes instead of bool.
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
Jeff Lucovsky a46779d866 detect/transform: Clarify transformation validation
Issue: 6439

Clarify the transform validation step. When a transform indicates that
the content/byte-array is not compatible, validation will stop.

Content is incompatible is some cases -- e.g., following the
to_lowercase transform with content containing uppercase characters.
An alert is not possible since the content contains uppercase and the
transform has converted the buffer into all lowercase.
2 years ago
Jeff Lucovsky 6a41843035 detect/tenants: Add tenant context to rule loads
Issue: 1520

This commit adds the tenant id for context to rule and .config file
loads.
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
Jeff Lucovsky f293823ab3 detect: Count buffer id once
This commit removes a second, unnecessary increment of the de_ctx buffer
id.

Issue: 5211
2 years ago
Victor Julien de598f12a6 multi-tenant: add reload-tenants command
Command to reload all tenants. Their original yaml path will be
used.

Ticket: #6267.
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
Victor Julien c87803ea0e detect: add multi-detect.config-path
Add option to specify path from which to load the tenants.

Mostly meant to be used in testing.
2 years ago
Victor Julien a4d80bc7c4 detect: free all tenant detect engines
Free all tenants registered in the master.
2 years ago
Victor Julien 269f751d36 stat: add wrappers to isolate OS_WIN32 specifics 2 years ago
Victor Julien b130234b26 var-names: reimplement var name handling
Implement a new design for handling var name id's. The old logic
was aware of detection engine versions and generally didn't work
well for multi-tenancy cases. Other than memory leaks and crashes,
logging of var names worked or failed based on which tenant was
loaded last.

This patch implements a new approach, where there is a global store
of vars and their id's for the lifetime of the program.

Overall Design:

Base Store: "base"

Used during keyword registration. Operates under lock. Base is shared
between all detect engines, detect engine versions and tenants.
Each variable name is ref counted.

During the freeing of a detect engine / tenant, unregistration decreases
the ref cnt.

Base has both a string to id and a id to string hash table. String to
id is used during parsing/registration. id to string during unregistration.

Active Store Pointer (atomic)

The "active" store atomic pointer points to the active lookup store. The call
to `VarNameStoreActivate` will build a new lookup store and hot swap
the pointer.

Ensuring memory safety. During the hot swap, the pointer is replaced, so
any new call to the lookup functions will automatically use the new store.
This leaves the case of any lookup happening concurrently with the pointer
swap. For this case we add the old store to a free list. It gets a timestamp
before which it cannot be freed.

Free List

The free list contains old stores that are waiting to get removed. They
contain a timestamp that is checked before they are freed.

Bug: #6044.
Bug: #6201.
2 years ago
Victor Julien 73b0efb03a detect: fix minor compile warning
detect-engine.c: In function ‘DetectKeywordCtxHashFunc’:
detect-engine.c:3550:75: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 3550 |     uint64_t hash = StringHashDjb2((const uint8_t *)name, strlen(name)) + (uint64_t)ctx->data;
      |
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 9ca4ef5dbe detect/engine: minor debug cleaup 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 377f2d7e1e detect: reduce failure_fatal to bool to save space 2 years ago
Philippe Antoine f2a18e91c4 rust: define AppLayerEventType only in rust
And detect.h does no longer depend on app-layer-events.h
2 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.
2 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.
2 years ago
Victor Julien 3c7ad453fe classification: spelling 2 years ago
Victor Julien da89164ba1 detect: spelling: DetectBufferMpmRegistry 2 years ago
Victor Julien bbcf6fe76b detect: spelling: multi-tenancy fixes 2 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.
2 years ago
Jason Ish 1c6644ef4e detect: fix setting of flag for rule reload
As part of 6d8b50b748, the settings of
THV_CAPTURE_INJECT_PKT ended up in a location unreachable by capture
methods that did not have PktAcqBreakLoop.

Instead, always call TmThreadsCaptureBreakLoop which handles the logic
for how the read loop should be broken.

This fixes the case where read threads won't "break" for rule reloads
until packets are seen.

Ticket: #6021
2 years ago
Victor Julien 021f626ab1 detect: add error message for transform w/o matches 2 years ago
Victor Julien 2c0e434bb8 detect: pass de_ctx to DetectBufferSetActiveList 2 years ago
Victor Julien 0b6ebd13ee detect: add util funcs to get first and last sigmatch for buffer 2 years ago
Justin Azoff aacb7dc291 detect/iponly: remove DetectEngineIPOnlyThreadCtx
This is unused.

Issue: 4578
2 years ago
Victor Julien 8a968faa04 detect: only breakloop threads that are lagging
Sleep after all threads have been checked.

Bug: #5969.
2 years ago
Philippe Antoine d313b5d605 detect: bump detect engine version for tenant reload
Because the engine version is used to free the old
variables and not the new ones.
As is done in DetectEngineReload.

Ticket: #5866
2 years ago
Victor Julien 977c5ea719 detect: initialize empty buffers 3 years ago
Victor Julien 70125a29f6 detect/buffer: add initialized flag to simplify buffer logic 3 years ago
Jason Ish dcfa6a6002 suricata: allow additional include files on command line
Add a new command line option, --include. This will merge additional
configuration files into the configuration specified in the main
suricata.yaml.  It can be provided multiple times and the files will be
included in the order they appear on the command line.

Ticket: 3912
3 years ago
Victor Julien ebd8728219 src: fix strict-prototype warnings
Tested on Fedora 37 with clang 15.

app-layer.c:1055:27: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
void AppLayerSetupCounters()
                          ^
                           void
app-layer.c:1176:29: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
void AppLayerDeSetupCounters()
                            ^
                             void
2 errors generated.
3 years ago
Victor Julien b31ffde6f4 output: remove error codes from output 3 years ago
Victor Julien 39f5c7f56a error: use SC_EINVAL for invalid input 3 years ago
Victor Julien 10c26bb628 unix-socket: don't print unix socket message twice 3 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
3 years ago
Philippe Antoine 3e4f58e375 detect: fix memory leak when parsing signature
Ticket: #5529
3 years ago
Victor Julien 39cf5b151a src: includes cleanup
Work towards making `suricata-common.h` only introduce system headers
and other things that are independent of complex internal Suricata
data structures.

Update files to compile after this.

Remove special DPDK handling for strlcpy and strlcat, as this caused
many compilation failures w/o including DPDK headers for all files.

Remove packet macros from decode.h and move them into their own file,
turn them into functions and rename them to match our function naming
policy.
3 years ago
Victor Julien e250ef6402 debug: remove empty header 3 years ago
Jeff Lucovsky e566563875 classification/config: Propagate validation errors
Issue: #4554

This commit modifies the workflow to propagate classification parsing
errors when in test mode.

When not in test mode, errors continue to be displayed but they do not
halt Suricata execution.
3 years ago
Philippe Antoine 83a8cd80b2 detect: remove wrappers around DetectEngineInspectGenericList 3 years ago
Philippe Antoine 02f2602dde src: rework includes as per cppclean 3 years ago
Jufajardini Reichow 93c2c9743d detect/engine: init alert queue counters on reload
alert_queue_overflow and alerts_suppressed were not being
reinitialized when there was a reload of Suricata rules, leading to
non-valid stats counters if that happened.

Bug #5457
3 years ago
Philippe Antoine b8524e70d9 detect: change InspectEngineFuncPtr2 to return uint8_t 3 years ago
Philippe Antoine c5cf2967b3 detect: fix integer warnings
Ticket: #4516
3 years ago
Juliana Fajardini 877b32c1e4 detect/stats: log out total of suppressed alerts
Related to
Task #4943
Task #5179
3 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
3 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
3 years ago
Victor Julien 4fcb8740e7 detect/multi-tentancy: minor format string fixes
cppcheck:

src/detect-engine.c:3643:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
    snprintf(prefix, sizeof(prefix), "multi-detect.%d", tenant_id);
    ^
src/detect-engine.c:3707:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
    snprintf(prefix, sizeof(prefix), "multi-detect.%d.reload.%d", tenant_id, reload_cnt);
    ^
src/detect-engine.c:4086:17: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
                snprintf(prefix, sizeof(prefix), "multi-detect.%d", tenant_id);
                ^

Bug: #5291.
3 years ago
Philippe Antoine 45d1a9ae77 detect: faster linked list copy
In DetectAppLayerInspectEngineCopyListToDetectCtx
Avoid quadratic complexity by remembering last element
of the linked list we are inserting into
3 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.
3 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.
3 years ago
Victor Julien ec01a94a5f detect: minor debug fixup 3 years ago
Jeff Lucovsky be2155b4ed config/ref: Raise errors for ref.config parsing
This commit raises an error in configuration test mode if there was an
error parsing reference.config.

Issue: 4659
4 years ago
Victor Julien 40c315aa35 detect/frames: fix coverity warning
Harmless warning, but it was correct in that the code made no sense:
1497420 Dereference before null check
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.
4 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.
4 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.
4 years ago
Victor Julien db27244379 detect: add buffer helper functions 4 years ago
Victor Julien 707b75ccda detect: split register time and detect load time buffer funcs 4 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.
4 years ago
Philippe Antoine 689ac97d72 inspect: debug validation to ensure correct argument 4 years ago
Sam Muhammed 42d4eb6943 detect-engine: convert unittests to FAIL/PASS APIs 4 years ago
Philippe Antoine d21a252238 fuzz: target must use the rules it parsed
DetectEngineReloadThreads does not work for the fuzz targets
as there is no_of_detect_tvs = 0 as we did not register
real threads and slots.

So, we force the flow worker module to use the newly detect engine
conetxt with all it needs
4 years ago
Victor Julien 13a6eb3522 detect: suppress scan-build warning 4 years ago
Victor Julien ed87784907 detect: enforce max app-layer progress
Allow progress values in the range 0-47 so we have 48 bits to track
prefilter engines.

Mark bits 48-62 as reserved explicitly.

Add debug validation checks to make sure the reserved space isn't used.
4 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
4 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.
4 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.
4 years ago
Victor Julien 23d7beb458 detect: reformat events table 4 years ago
Jeff Lucovsky 1eeb96696b general: Cleanup bool usage 4 years ago
Victor Julien c3075cba42 detect/analyzer: fix mpm display on payload only rules 4 years ago