Commit Graph

112 Commits (0a953fe1ce2a030916b094aac7618d039ecff485)

Author SHA1 Message Date
Philippe Antoine 0a953fe1ce detect: add to signature mask for decode events
Ticket: 6291
2 years ago
Philippe Antoine 4e584ed201 detect: fix check for app_layer events
Ticket: 7106
2 years 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.
2 years ago
Philippe Antoine 20423fdd38 style: remove some useless return
and remove empty line before end of function
2 years ago
Victor Julien 68804b8c4b decode/tcp: move tcph into L4 packet data
To reduce Packet size.

Ticket: #6938.
2 years ago
Victor Julien cca912e9b3 decode/tcp: add and use PacketIsTCP inline func
To prepare future changes to the Packet header pointers.

Ticket: #5517.
2 years ago
Shivani Bhardwaj 4227e52c4b detect/port: handle range and upper boundary ports
So far, if a port was found to be single which was earlier a part of the
range, port + 1 was added to the list to honor the range that it was a
part of. But, this is incorrect in case the port is 65535 or if the port
was found to be of range when it was earlier a single port.

Bug 6896
2 years ago
Shivani Bhardwaj 7d937db5cb detect/port: fix grouping of ports w gaps
If a single port happens before a range port, the port groups created
were incorrect. Fix it to use smarter range check.

For example, given,
80:80 - SGH1
100:120 - SGH2

Range created should be
80:80 - SGH1
100:120 - SGH2

Bug 6881
2 years ago
Shivani Bhardwaj 632ca75dd3 detect/port: handle single port that is range too
If a port point is single but later on also a part of a range, it ends
up only creating the port groups for single points and not the range.
Fix it by adding the port next to current single one to unique points
and marking it a range port.

Bug 6843
2 years ago
Victor Julien e7e4305d91 detect/port: use qsort instead of insert sort
Instead of using in place insertion sort on linked list based on two
keys, convert the linked list to an array, perform sorting on it using
qsort and convert it back to a linked list. This turns out to be much
faster.

Ticket #6795
2 years ago
Shivani Bhardwaj 643ae85b5f detect/port: merge port ranges for same signatures
To avoid getting multiple entries in the final port list and to also
make the next step more efficient by reducing the size of the items to
traverse over.

Ticket 6792
Bug 6414
2 years ago
Shivani Bhardwaj 83aba93f40 detect/port: remove the port cut/insertion stage
As this is already taken care of and a list of ports is available for
use by the next stage.

Ticket 6792
Bug 6414
2 years ago
Shivani Bhardwaj 4ac2382f26 detect/port: create list of small port ranges
Using the unique port points, create a list of small port ranges which
contain the DetectPort objects and the designated SGHs found by finding
the overlaps with the existing ports and copying the SGHs accordingly.

Ticket 6792
Bug 6414
2 years ago
Shivani Bhardwaj a02c44a3a4 detect/port: create a tree of given ports
After all the SGHs have been appropriately copied to the designated
ports, create an interval tree out of it for a faster lookup when later
a search for overlaps is made.

Ticket 6792
Bug 6414
2 years ago
Shivani Bhardwaj c9a911b6f8 detect/port: find unique port points
In order to create the smallest possible port ranges, it is convenient
to first have a list of unique ports. Then, the work becomes simple. See
below:

Given, a port range P1 = [1, 8]; SGH1
and another, P2 = [3, 94]; SGH2

right now, the code will follow a logic of recursively cutting port
ranges until we create the small ranges. But, with the help of unique
port points, we get, unique_port_points = [1, 3, 8, 94]

So, now, in a later stage, we can create the ranges as
[1, 2], [3, 7], [8, 8], [9, 94] and copy the designated SGHs where they
belong. Note that the intervals are closed which means that the range
is inclusive of both the points.

The final result becomes:
1. [1, 2]; SGH1
2. [3, 7]; SGH1 + SGH2
3. [8, 8]; SGH1 + SGH2
4. [9, 94]; SGH2

There would be 3 unique rule groups made for the case above.
Group 1: [1, 2]
Group 2: [3, 7], [8, 8]
Group 3: [9, 94]

Ticket 6792
Bug 6414
2 years ago
Victor Julien fb9680bb7b detect/engine: fix whitelisting check
In the commit 4a00ae607, the whitelisting check was updated in a quest
to make use of the conditional better but it made things worse as every
range would be whitelisted as long as it had any of the default
whitelisted port which is very common.
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
Shivani Bhardwaj 7f89aaf772 detect: remove unneeded max_idx 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 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 75c1b7fb10 detect: consolidate per rule group file loops
Don't loop multiple times over the per group sig array.
3 years ago
Victor Julien 3b8ed937d7 detect: remove DCERPC mask logic
Added nothing over alproto check already in place.
3 years ago
Victor Julien bd66504a43 detect: implement --qa-skip-prefilter
Option meant for testing performance of rule engine w/o prefilter
optimizations.
3 years ago
Shivani Bhardwaj 34858808c1 detect-engine: use flag SIG_FLAG_MPM_NEG
The flag SIG_FLAG_MPM_NEG is set before whitelisting the rules. Make it
better by checking for the flag in the beginning and return immediately.
3 years ago
Shivani Bhardwaj 47c9a14543 detect-engine: use bool return type 3 years ago
Shivani Bhardwaj bd41b31005 detect: rename SigAddressPrepare fns to SigPrepare
There is nothing Address specific going on in the preparations.
Stage 1: Preprocessing happens. Sigs classified as IP Only, Masks
applied, content specific limits applied, etc and sig array built.
Stage 2: Sigs grouped by IPOnly, ports and protocols.
Stage 3: Decoder Events SGH built.
Stage 4: File flags set, sig grouping done per prefilter, etc.
3 years ago
Shivani Bhardwaj c1bf955326 detect-engine: use ports only after edge case handling
Also, add comments to clarify what's happening in the code.
3 years ago
Shivani Bhardwaj d8a887e518 detect/engine: defensive check and comment update 3 years ago
Shivani Bhardwaj 945ec4bc0a detect: remove redundant null setting
de_ctx->dport_hash_table is already set to NULL in the fn
DetectPortHashFree which is called right before this setting.
Remove the redundant setting.
3 years ago
Shivani Bhardwaj 6076b9e2f0 detect: use proper names for whitelist score criteria 3 years ago
Shivani Bhardwaj 4a00ae6076 detect/engine: fix whitelisted port range check
So far, the condition for checking if the whitelisted port was in the
port range of "a" said

a->port >= w->port && a->port2 <= w->port

But, if a->port <= a->port2, this condition could only be true when
a->port == w->port == a->port2. However, the motivation for this fn was
to be able to find if the whitelisted port for a carrier proto already
was in the range of the given protocol and calculate a score for the
port accordingly.
Fix the range check such that a->port <= w->port <= a->port2.
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
Shivani Bhardwaj 1f78a4fcd8 detect-engine: use only the exact match fn
DetectFlagsSignatureNeedsSynPackets checks if TCP SYN flag is set among
other flags.
DetectFlagsSignatureNeedsSynOnlyPackets checks if only TCP SYN flag is
set and no other flag.

Since DetectFlagsSignatureNeedsSynOnlyPackets also already checks for
TCP SYN flag, it does not need to be used in combination with
DetectFlagsSignatureNeedsSynPackets as this fn seems to be the superset
of the former.
3 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).
3 years ago
Jeff Lucovsky 193e0ea1a9 memory/alloc: Use SCCalloc instead of malloc/memset 3 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.
3 years ago
Philippe Antoine ad5f41c95c detect: remove code writing unused values
Coveridy ID 1546822 1546823 and 1546824
3 years ago
Jeff Lucovsky 59d050a067 detect/gen: Minor cleanup
Issue: 5211
3 years ago
Jeff Lucovsky 01d232b6be detect/rule: Use de_ctx buffer id
This commit uses the detect engine buffer id instead of the global
value.

Issue: 5211
3 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.
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
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
Victor Julien 681b4c3a23 detect: minor cleanup 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 549f7873df detect: spelling 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 0ff28a482b detect: remove unused group counters 3 years ago
Victor Julien b31ffde6f4 output: remove error codes from output 4 years ago
Victor Julien cef0a84bd2 detect: turn cleanup message into debug message 4 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.
4 years ago