Commit Graph

12908 Commits (8e4b18f4e41449201f00bcf87704ddffdf61290b)

Author SHA1 Message Date
Jeff Lucovsky d853972c74 output/log: Remove extraneous error message
Issue: 7447

When the output file can't be opened, 2 error messages are displayed
for the same problem. The second message doesn't add value and lacks
context (error reason, e.g., "Permission denied").

Retaining the second message as a debug message.

Without this commit:

Error: logopenfile: Error opening file: "/home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.1.json": Permission denied [SCLogOpenFileFp:util-logopenfile.c:428]
Error: logopenfile: Unable to open slot 1 for file /home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.json [LogFileEnsureExists:util-logopenfile.c:737]
Error: runmodes: unable to initialize sub-module eve-log.stats [RunModeInitializeEveOutput:runmodes.c:692]

With commit:

Error: logopenfile: Error opening file: "/home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.1.json": Permission denied [SCLogOpenFileFp:util-logopenfile.c:428]
Error: runmodes: unable to initialize sub-module eve-log.stats [RunModeInitializeEveOutput:runmodes.c:692]
2 years ago
Jeff Lucovsky e72fc39f83 output/log: Improve error handling
This commit improves error handling for cases when file(s) cannot be
opened.
- Return NULL if file object can't be opened
- checks whether the file object has been opened before
  dereferencing the per-file context.

Issue: 7447
2 years ago
Jeff Lucovsky 89c9cec0bb general/typo: Fix typo in stacksize msg 2 years ago
Victor Julien 779f9d8ba3 eve/flow: add per flow TCP oob urg data counter
If TCP urgent handling is set to the OOB processing, the number of OOB
bytes is tracked for SEQ offset calculations. If this offset is
non-zero, add the field to the flow record.

Ticket: #7411.
2 years ago
Victor Julien 6882bcb3e5 stream: add TCP urgent handling options
TCP urgent handling is a complex topic due to conflicting RFCs and
implementations.

Until now the URG flag and urgent pointer values were simply ignored,
leading to an effective "inline" processing of urgent data. Many
implementations however, do not default to this behavior.

Many actual implementations use the urgent mechanism to send 1 byte of
data out of band to the application.

Complicating the matter is that the way the urgent logic is handled is
generally configurable both of the OS and the app level. So from the
network it is impossible to know with confidence what the settings are.

This patch adds the following policies:

`stream.reassembly.urgent.policy`:

- drop: drop URG packets before they affect the stream engine

- inline: ignore the urgent pointer and process all data inline

- oob (out of band): treat the last byte as out of band

- gap: skip the last byte, but do no adjust sequence offsets, leading to
       gaps in the data

For the `oob` option, tracking of a sequence number offset is required,
as the OOB data does "consume" sequence number space. This is limited to
64k. For this reason, there is a second policy:

`stream.reassembly.urgent.oob-limit-policy`:

- drop: drop URG packets before they affect the stream engine

- inline: ignore the urgent pointer and process all data inline

- gap: skip the last byte, but do no adjust sequence offsets, leading to
       gaps in the data

Bug: #7411.
2 years ago
Victor Julien 4c1554f4f6 stream: remove unused function argument
Sequence number is taken from seg, not the func arg.
2 years ago
Victor Julien ac02a71479 decode/tcp: count urg flag 2 years ago
Victor Julien d1b0d00478 flow/timeout: add frame awareness
If there are still frames in the flow, the detection and logging logic
needs to be able to evaluate them. To do this, make the flow timeout
logic aware of the frames. If frames still exist in a direction, trigger
a FFR packet to be created.

Ticket: #7440.
2 years ago
Victor Julien 0de9eee04d eve/frame: require frame length to be known
Or reach logging threshold.

Avoids logging too early.

Ticket: #7440.
2 years ago
Victor Julien 95ac92f9aa eve/frame: run logging for flow end packets
If there are frames in the flow the flow manager will create flow
timeout packets to log the remaining frames. This requires the logger to
run for those flow timeout packets.

Ticket: #7440.
2 years ago
Victor Julien db589765e4 eve/frame: remove unreachable if branch 2 years ago
Philippe Antoine 9a53ec43b1 util/streaming-buffer: add extra safety check
Ticket: 7393

Check if GrowRegionToSize is called with an argument
trying to shrink the region size, and if so do nothing,
ie do not try to shrink, and just return ok.

This way, we avoid a buffer overflow from memeset using an
unsigned having underflowed.
2 years ago
Philippe Antoine 8900041405 util/streaming-buffer: check need to grow region
Ticket: 7393

As it was possible before earlier patches to get here
with mem_size lesser than start->buf_size,
which caused then an unsigned underflow and a buffer overflow.
2 years ago
Philippe Antoine 282509f70c util/streaming-buffer: fix regions intersection
This was not a problem for current callers in Suricata,
as RegionsIntersect is only called through StreamingBufferInsertAt
which is only used by TCP...

And TCP uses default region gap = 256kb, and only calls
StreamingBufferInsertAt with a u16, so TCP never inserts a new
data that will strictly contain an existing region augmented
with region gap, which was the only case where RegionsIntersect
returned the wrong result, which could later lead to a
buffer overflow.

Ticket: 7393
2 years ago
Victor Julien 0e4faba79a detect: don't run pkt sigs on ffr pkts
Last packet from the TLS TCP session moves TCP state to CLOSED.

This flags the app-layer with APP_LAYER_PARSER_EOF_TS or
APP_LAYER_PARSER_EOF_TC depending on the direction of the final packet.
This flag will just have been set in a single direction.

This leads to the last packet updating the inspect id in that packets
direction.

At the end of the TLS session a pseudo packet is created, because:
 - flow has ended
 - inspected tx id == 0, for at least one direction
 - total txs is 1

Then a packet rule matches:

```
alert tcp any any -> any 443 (flow: to_server;                  \
        flowbits:isset,tls_error;                               \
        sid:09901033; rev:1;                                    \
        msg:"Allow TLS error handling (outgoing packet)"; )
```

The `SIG_MASK_REQUIRE_REAL_PKT` is not preventing the match, as the
`flowbits` keyword doesn't set it.

To avoid this match. This patch skips signatures of the `SIG_TYPE_PKT`
for flow end packets.

Ticket: #7318.
2 years ago
Philippe Antoine f426ee3ee2 detect: rename stream_log variables
to better reflect their true meaning
2 years ago
Philippe Antoine f2c3776314 detect: log app-layer metadata in alert with single tx
Ticket: 7199

Uses a config parameter detect.guess-applayer-tx to enable
this behavior (off by default)

This feature is requested for use cases with signatures not
using app-layer keywords but still targetting application
layer transactions, such as pass/drop rule combination,
or lua usage.

This overrides the previous behavior of checking if the signature
has a content match, by checking if there is only one live
transaction, in addition to the config parameter being set.
2 years ago
Lukas Sismis 18ab9a6ccd dpdk: set ice PMD RSS key length to 52 bytes for all DPDK versions
ICE driver (Intel E810 NIC) requires/supports 52-byte long RSS key.
The 52 byte key length was mandatory from DPDK 23.11 when Suricata
was starting with independently configured ice PMD.

However, Suricata failed to start when ice PMD was part of
net_bonding PMD, requiring 52 byte RSS key even in DPDK versions
lower than 23.11. Since the support for the longer key is present
since DPDK 19.11 the key is set to 52 bytes for all versions.

Ticket: 7444
2 years ago
Philippe Antoine b02557ac7d app-layer: track modified/processed txs
To optimize detection, and logging, to avoid going through
all the live transactions when only a few were modified.

Two boolean fields are added to the tx data: updated_tc and ts
The app-layer parsers are now responsible to set these when
needed, and the logging and detection uses them to skip
transactions that were not updated.

There may some more optimization remaining by when we set
both updated_tc and updated_ts in functions returning
a mutable transaction, by checking if all the callers
are called in one direction only (request or response)

Ticket: 7087
2 years ago
Philippe Antoine e5b98be41f fuzz: simplify target for protocol detection
As too many cases are found when splitting tcp payload
2 years ago
Philippe Antoine 261c15d0e1 fuzz: better init for protocol detection
Ticket: 7435
2 years ago
Victor Julien 7fd707a876 flow/manager: add chunk debug output 2 years ago
Victor Julien ae072d5c07 flow/manager: fix multi instance row tracking
In multi instance flow manager setups, each flow manager gets a slice
of the hash table to manage. Due to a logic error in the chunked
scanning of the hash slice, instances beyond the first would always
rescan the same (first) subslice of their slice.

The `pos` variable that is used to keep the state of what the starting
position for the next scan was supposed to be, was treated as if it held
a relative value. Relative to the bounds of the slice. It was however,
holding an absolute position. This meant that when doing it's bounds
check it was always considered out of bounds. This would reset the sub-
slice to be scanned to the first part of the instances slice.

This patch addresses the issue by correctly handling the fact that the
value is absolute.

Bug: #7365.

Fixes: e9d2417e0f ("flow/manager: adaptive hash eviction timing")
2 years ago
Juliana Fajardini a9b36d88b8 detect/engine/flowint: apply clang format changes
Related to
Task #7426
2 years ago
Juliana Fajardini 6e4a501e7c flowint: add isnotset support
Similar keywords use `isnotset`, while `flowint` only accepted `notset`
Opted to change the code, not only the regex, to keep the underlying
code also following the same patterns.

Task #7426
2 years ago
Victor Julien 2fe2cf8553 eve/alert: enrich decoder event
Default decoder event alert was very sparse, not even logging packet
type and pcap_cnt. Expand support for this record type. It will be more
useful with the ethernet headers and packet field, but these are still
disabled by default.

Ticket: #7433.
2 years ago
Victor Julien b23fa51e33 detect: fix decoder only events
Add missing setup part of the decoder event sgh.

Bug: #7414.
2 years ago
Philippe Antoine 09ba69cfb0 output/http: log invalid status as a string
Ticket: 7311

If response_status_number is not a valid poisitive integer,
we should not try to parse it again, and fail again,
but just log the raw string.
2 years ago
Victor Julien 923ad6af77 af-packet: speed up thread sync during startup
Threads are initialized sequentially to allow for a predictable mapping
of threads and queues. Not all parts of the start up need to be done
sequentially. The setting up of the rings can be very expensive, taking
of a couple of hundred milliseconds. The ring setup doesn't need to be
done sequentially though.

This patch releases the thread early, after bind but before the ring
setups.

Ticket: #7272.
2 years ago
Jason Ish 8bcc844b6f sigtable: add function to test for a keyword
To be used by the requires keyword to check for keyword support.

Ticket: #7403
2 years ago
Jason Ish 287d8360e7 util-buffer: expand by multiples of 4k 2 years ago
Jason Ish 2e2eaac0b2 output-json: cleanup, have OutputJsonBuilderBuffer return void
The return value was never used.
2 years ago
Jason Ish d39e42728a output-json: drop eve records that are too long
In the situation where the mem buffer cannot be expanded to the
requested size, drop the log message.

For each JSON log context, a warning will be emitted once with a partial
bit of the log record being dropped to identify what event types may be
leading to large log records.

This also fixes the call to MemBufferExpand which is supposed be
passed the amount to expand by, not the new size required.

Ticket: #7300
2 years ago
Philippe Antoine 4ec90bd227 detect: absent keyword to test absence of sticky buffer
Ticket: 2224

It takes an argument to match only if the buffer is absent,
or it can still match if the buffer is present, but we test
the absence of some content.

For multi buffers, absent matches if there are 0 buffers.

For file keywords, absent matches if there is no file.
2 years ago
Philippe Antoine 7682816ef9 http1/detect: code simplification
- DetectEngineInspectBufferHttpHeader is only used with ALPROTO_HTTP1
- engine->progress should be HTP_REQUEST_HEADERS or HTP_RESPONSE_HEADERS based on the direction
2 years ago
Victor Julien 13f420c793 detect/ip-only: code cleanups
Move repeated pattern into helper function.
2 years ago
Victor Julien a8c63992fb detect/sigorder: remove data structs from global namespace
Rename types enum to reflect it is not using a radix tree anymore.
2 years ago
Victor Julien 4aeb606a97 detect/ip-only: remove dead code 2 years ago
Victor Julien 3eb2f6a584 radix: remove old radix tree implementation
No longer used.
2 years ago
Victor Julien 0ea6fe668e detect/iponly: reimplement with radix4 for ipv6
Use a more compact ipv6 specific structure.
2 years ago
Victor Julien 4b6296c848 detect/iponly: reimplement with radix4 for ipv4
Use a more compact ipv4 specific structure.
2 years ago
Victor Julien 8536dd51b5 host-os-policy: switch to radix4/6
Splits the unified tree into a ipv4 specific and ipv6 specific tree.
2 years ago
Victor Julien 82cd053fc8 htp: switch config tree to radix4/6
Splits the unified tree into a ipv4 specific and ipv6 specific tree.
2 years ago
Victor Julien 46fb7bb162 defrag/config: switch to radix4/6
Splits the unified tree into a ipv4 specific and ipv6 specific tree.
2 years ago
Victor Julien c75b29c74f reputation: switch cidr handling to radix4/6
Splits the unified tree into a ipv4 specific and ipv6 specific tree.
2 years ago
Victor Julien 8f03a2eb9b radix: implement more compact trees
Implement a more compact set of trees specifically for IPv4
and IPv6 addresses. This allows for more compact data structures
and fewer memory allocations.

Based on the existing radix tree implementation.
2 years ago
Victor Julien e9a2352b99 host-os-info: test cleanup 2 years ago
Eric Leblond be7cea4d32 suricata: fix list-keywords option
The list keywords option was crashing due to improper init.

Ticket: 7397
2 years ago
Shivani Bhardwaj d0427a2833 detect/smb: fix the sigmatch table for url
sigmatch_table for url entry was incorrectly set to DETECT_FLOW_AGE.
2 years ago
Jason Ish 5d766df12b make: install-headers: rust-bindings.h
rust-bindings.h was not being installed with "make install-headers",
and its now pulled in by a header used for plugin support, so make
sure its installed.

We first attempt to install the "dist" version if exists, otherwise
install the "gen" one. Also install the "gen" even if the "dist" one
exists, as its going to be newer.
2 years ago