Jason Ish [Fri, 10 Jan 2025 21:40:26 +0000 (15:40 -0600)]
lua/datasets: rework to be a "required" module
Re-work the Lua dataset lib to be required into a user script like:
local dataset = require("suricata.data")
The main difference from loading it into global space is providing a
custom require function (as we removed it in the sandbox) and load it on
demand, returning a table to the module.
Victor Julien [Thu, 11 Apr 2024 14:10:34 +0000 (16:10 +0200)]
detect/lua: add support for datasets
dataset.new
create a dataset object in lua
<dataset>:get
gets a reference to an existing dataset
<dataset>:add
returns 1 if a new entry was added
returns 0 if entry was already in the set
Example:
```
function init (args)
local needs = {}
needs["packet"] = tostring(true)
return needs
end
function thread_init (args)
conn_new, dataset.new()
ret, err conn_new:get("conn-seen")
if err ~= nil then
SCLogWarning("dataset warning: " .. err)
return 0
end
end
function match (args)
ipver, srcip, dstip, proto, sp, dp = SCFlowTuple()
str = ipver .. ":<" .. srcip .. ">:<" .. dstip .. ">:" .. dp
ret, err = conn_new:add(str, #str);
if ret == 1 then
SCLogInfo(str .. " => " .. ret)
end
return ret
end
```
Shivani Bhardwaj [Fri, 29 Nov 2024 08:31:12 +0000 (14:01 +0530)]
flow/pkts: make syntax cleaner and compact
Currently, the syntax includes direction as a part of the keyword which
is against how usually keywords are done. By making direction as a
mandatory argument, it is possible to make the syntax cleaner and the
implementation more compact and easily extendable.
Pros:
- Registration table sees lesser entries if newer options are added
- If the options have to be extended, it can be done trivially
- In accordance w existing keyword implementations
Note that this commit also retains the existing direction specific
keywords.
Victor Julien [Fri, 29 Nov 2024 13:37:08 +0000 (14:37 +0100)]
stream: RST no longer acks all data
Since forever (1578ef1e3e8a24d0cc615430c4e6bec1fefdad28) a valid RST
would update the internal `last_ack` representation to include all
unack'd data. This was originally done to make sure the unACK'd data was
inspected/processed at flow timeout.
It was observed however, that if GAPs existed in this unACK'd data, a
GAP could be reported in the stats and a GAP event would be raised. This
doesn't make sense, as missing segments in the unACK'd part of the
stream are completely normal. Segments simply do not all arrive in
order.
It turns out that the original behavior of updating `last_ack` to
include all unACK'd data is no longer needed.
For raw stream inspection, the detection engine will already include the
unACK'd data on flow end.
For app-layer updates the unACK'd data is often harmful, as the data
often has GAPs. Parser like the http parser would report these GAPs and
could also get confused about the post-GAP data being a new transaction
including a file. This lead to many reported errors and fantom txs and
files.
Since the GAP detection uses `last_ack` to determine GAPs, not moving
`last_ack` addresses the GAP false positives.
Victor Julien [Wed, 18 Sep 2024 09:50:59 +0000 (11:50 +0200)]
threads: align struct to CLS to avoid false sharing
Since `Thread` objects are part of a big allocation, more than one
Thread could be on a single cache line, leading to false sharing. Atomic
updates to one `Thread` could then lead to poor performance accessing
another `Thread`. Align to CLS (cache line size) to avoid this.
Victor Julien [Tue, 17 Sep 2024 09:02:00 +0000 (11:02 +0200)]
threads: fine grained locking for Thread
Until now many accesses to the Thread structure required taking a global
lock, leading to performance issues. In practice this only happened in
offline mode.
This patch adds a finer grained locking scheme. It assumes that the
Thread object itself cannot disappear, and adds a spinlock to protect
updates to the structure.
Additionally, the `pktts` field is made an atomic, so that it can be
read w/o taking the spinlock. Updates to it are still done under lock.
Victor Julien [Wed, 18 Sep 2024 10:21:42 +0000 (12:21 +0200)]
threads: use sleeping threads for minimum time a bit longer
If a thread doesn't receive packets for a while the packet timestamp
will no longer be used to determine a reasonable minimum timestamp for
flow timeout handling.
To avoid issues with the minimum timestamp to be set a bit too
aggressively, increase the time a thread can be inactive.
Victor Julien [Sun, 15 Sep 2024 17:15:56 +0000 (19:15 +0200)]
time: thread time update after flow update
The flow worker needs to get the opportunity to run the flow update
before globally making it's current timestamp available. This is to
avoid another thread using the time to evict the flow that is about to
get a legitimate update.
Philippe Antoine [Tue, 17 Dec 2024 10:20:51 +0000 (11:20 +0100)]
ldap: truly enforce max-tx
Ticket: 7465
If a bug chunk of data is parsed in one go, we could create many
transactions even if marking them as complete, and have
quadratic complexity calling find_request.
Proposed solution is to fail on creating a new transaction if too
many already exist.
Philippe Antoine [Wed, 11 Dec 2024 13:50:49 +0000 (14:50 +0100)]
detect: improve tx_id guessing for unidirectional protocols
So we get:
1. request arrives - buffered due to not ackd
2. response arrives, acks request - request is now parsed, response isn't
3. ack for response, response parsed. Then detect runs for request,
generates alert. We now have 2 txs. txid will be 0 from AppLayerParserGetTransactionInspectId
But txid 1 is unidirectional in the other way, so we can use txid 0
metadata for logging
Jeff Lucovsky [Thu, 12 Dec 2024 13:49:20 +0000 (08:49 -0500)]
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]
Jeff Lucovsky [Wed, 11 Dec 2024 13:26:01 +0000 (08:26 -0500)]
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.
Jason Ish [Wed, 18 Dec 2024 17:36:20 +0000 (11:36 -0600)]
rust/jsonbuilder: make set_uint generic over Into<u64>
Allow `set_uint` to accept any number value that can be converted to a
u64. Prevents callers from having to do `as u64`.
This required fixing up any callers that used `.into()` to just pass in
their value without the into conversion.
Most calls using `as u64` can have that cast removed, with the exception
of `usize` values which must still be cast is conversion can't be
guaranteed to be non-fallible.
Jeff Lucovsky [Tue, 19 Nov 2024 14:30:23 +0000 (09:30 -0500)]
doc/commit
Describe how to use the git commit template. The template helps ensure
that the information needed for evaluation and context is included in
the commit message.
Jeff Lucovsky [Tue, 16 Jul 2024 13:43:12 +0000 (09:43 -0400)]
git: Add commit template
Issue: none
This commit adds a template that identifies commit message elements that
we find important. The Suricata development team requests that
contributions use the template to help improve commit messages. We
reserve the right to strictly enforce adherence to the template in the
future.
Configure git to use this template with:
git config commit.template ..github/commit-template.txt
Victor Julien [Tue, 10 Dec 2024 09:16:51 +0000 (10:16 +0100)]
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.
Victor Julien [Thu, 10 Oct 2024 14:12:09 +0000 (16:12 +0200)]
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
Victor Julien [Fri, 6 Dec 2024 13:15:34 +0000 (14:15 +0100)]
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.
Victor Julien [Fri, 6 Dec 2024 13:11:38 +0000 (14:11 +0100)]
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.
Jason Ish [Fri, 1 Nov 2024 17:39:23 +0000 (11:39 -0600)]
dns: provide events for recoverable parse errors
Add events for the following resource name parsing issues:
- name truncated as its too long
- maximum number of labels reached
- infinite loop
Currently these events are only registered when encountered, but
recoverable. That is where we are able to return some of the name,
usually in a truncated state.
As name parsing has many code paths, we pass in a pointer to a flag
field that can be updated by the name parser, this is done in
addition to the flags being set on a specific name as when logging we
want to designate which fields are truncated, etc. But for alerts, we
just care that something happened during the parse. It also reduces
errors as it won't be forgotten to check for the flags and set the
event if some new parser is written that also parses names.
Jason Ish [Thu, 31 Oct 2024 21:40:40 +0000 (15:40 -0600)]
dns: truncate names larger than 1025 characters
Once a name has gone over 1025 chars it will be truncated to 1025
chars and no more labels will be added to it, however the name will
continue to be parsed up to the label limit in attempt to find the end
so parsing can continue.
This introduces a new struct, DNSName which contains the name and any
flags which indicate any name parsing errors which should not error
out parsing the complete message, for example, infinite recursion
after some labels are parsed can continue, or truncation of name where
compression was used so we know the start of the next data to be
parsed.
This limits the logged DNS messages from being over our maximum size
of 10Mb in the case of really long names.
Philippe Antoine [Thu, 21 Nov 2024 14:20:44 +0000 (15:20 +0100)]
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.