Traffic variables (flowvars, flowbits, xbits, etc) use a smaller int for
their type than detection types. As a workaround make sure the values fit
in a uint8_t.
Add support for 'by_flow' track option. This allows using the various
threshold options in the context of a single flow.
Example:
alert tcp ... stream-event:pkt_broken_ack; \
threshold:type limit, track by_flow, count 1, seconds 3600;
The example would limit the number of alerts to once per hour for
packets triggering the 'pkt_broken_ack' stream event.
Implemented as a special "flowvar" holding the threshold entries. This
means no synchronization is required, making this a cheaper option
compared to the other trackers.
Ticket: #6822.
Issue: 6487
To avoid ambiguity, a single definition for base 64 decoding modes will
be used. The Rust base64 transform contains the definitions for the
existing mode types: Strict, RFC2045, RFC4648
Issue: 6487
Implement the from_base64 transform:
[bytes value] [offset value] [mode strict|rfc4648|rfc2045]
The value for bytes and offset may be a byte_ variable or an
unsigned integer.
Ticket: 6390
This can happen with keyword filestore:both,flow
If one direction does not have a signature group with a filestore,
the file is set to nostore on opening, until a signature in
the other direction tries to set it to store.
Subsequent files will be stored in both directions as flow flags
are now set.
Add an argument to the packet prefilter registration function to include
`SignatureMask` flags. This will be used at runtime to only call these
prefilter engines when the mask check passes.
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.
Ticket: 4863
On the way, convert unit test DetectSNMPCommunityTest to a SV test.
And also, make snmp.pdu_type use a generic uint32 for detection,
allowing operators, instead of just equality.
When replaying a pcap file, it is not possible to get rules
profiling because it has to be activated from the unix socket.
This patch adds a new option to be able to activate profiling
collection at start so a pcap run can get rules profiling
information.
Implement special "isset" and "isnotset" modes.
"isset" matches if an IP address is part of an iprep category with any
value.
It is internally implemented as ">=,0", which should always be true if
there is a value to evaluate, as valid reputation values are 0-127.
"isnotset" matches if an IP address is not part of an iprep category.
Internally it is implemented outside the uint support.
Ticket: #6857.
Replaces default "alert" logic and removed SIG_FLAG_NOALERT.
Instead, "noalert" unsets ACTION_ALERT. Same for flowbits:noalert and
friends.
In signature ordering rules w/o action are sorted as if they have 'alert',
which is the same behavior as before, but now implemented explicitly.
Ticket: #5466.
Ticket: 3958
- transactions are now bidirectional
- there is a logger
- gap support is improved with probing for resync
- frames support
- app-layer events
- enip_command keyword accepts now string enumeration as values.
- add enip.status keyword
- add keywords :
enip.product_name, enip.protocol_version, enip.revision,
enip.identity_status, enip.state, enip.serial, enip.product_code,
enip.device_type, enip.vendor_id, enip.capabilities,
enip.cip_attribute, enip.cip_class, enip.cip_instance,
enip.cip_status, enip.cip_extendedstatus
Before, the JsonBuilder object for the pgsql event was being created
from the C-side function that actually called the Rust logger.
This resulted that if another module - such as the Json Alert called the
PGSQL logger, we wouldn't have the `pgsql` key present in the log output
- only its inner fields.
Bug #6983
Adds the following frames:
command_line
data
response_line
The *_line frames are per line, so in multi-line responses each line
will have it's own frame.
Ticket: #4905.
Add new flags to trigger FLOW_TS_APP_UPDATED/FLOW_TC_APP_UPDATED flags
to be set for the next packet in the relevant direction.
This allows for app relevant work to be done in the next packet in our
direction.
EVE logging has a direction parameter that can cause the logging
of an application layer to be done in a direction that is not linked
to the packet. As a result the source IP addres could be assigned the
MAC address of the destination IP and reverse.
This patch addresses this by propagating the direction to the ethernet
logging function and using it there to define the correct mapping.
Issue #6405
Use quoted include style for Lua includes ("lua.h" instead of <lua.h>)
as this could result in system includes being picked up instead of the
includes from our vendor directory.
In certain conditions, it can take a long time for threads to start up.
For example in af-packet, setting up the socket, rings, etc has been
observed to take close to half a second per thread, and since the
threads go one by one in a preset order, this means the start up can
take a lot of time if there are many threads. The old logic would just
allow a hard coded 60s. This was not always enough when the number of
threads was high.
This patch makes the wait time take the number of threads into account.
It adds a second of time budget to the base 60s for each thread.
So as an example, if a system has 112 af-packet threads, it would wait
172 seconds (60 + 112) for the threads to get ready.
Ticket: #7048.
When starting a large amount of threads, the loop was inefficient. It
would loop over the threads and if one wasn't yet ready it would sleep a
bit and then reevaluate all the threads. This reevaluation of threads
already checked was inefficient, and could lead to the time budget
running out.
This patch splits the check, and keeps track of the threads that have
already passed. This avoids the rescanning of already checked threads.
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.
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.
The Lua library surface area is small enough to manage an allow list,
which is generally better than a deny list, as we'll explicitly need
to opt-in to new functions provided by the Lua runtime.
Move prototypes for functions that exist in util-port-interval-tree.c
from detect-engine-port.h to util-port-interval-tree.h.
Fix header guard names while there.
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
Bug: https://redmine.openinfosecfoundation.org/issues/6782
Callers to these allocators often use ``sc_errno`` to provide context of
the error. And in the case of the above bug, they return ``sc_errno``,
but as it has not been set ``sc_errno = 0; == SC_OK``.
This patch simply sets this variable to ensure there is context provided
upon error.
The on-disk pcap pkthdr is 16 bytes. This was calculated using
`sizeof(struct pcap_pkthdr)`, which is 24 bytes on 64 bit Linux. On
Macos, it's even worse, as a comment field grows the struct to 280
bytes.
Address this by hardcoding the value of 16.
Bug: #7037.
In offline mode, a timestamp is kept per thread, and the lowest
timestamp of the active threads is used. This was also considering the
non-packet threads, which could lead to the used timestamp being further
behind that needed. This would happen at the start of the program, as
the non-packet threads were set up the same way as the packet threads.
This patch both no longer sets up the timestamp for non-packet threads
as well as not considering non-packet threads during timestamp
retrieval.
Fixes: 6f560144c1 ("time: improve offline time handling")
Bug: #7034.
HttpRangeOpenFileAux may return NULL in different cases, including
when memcap is reached.
But is only caller did not check it before calling HttpRangeAppendData
which would dereference the NULL value.
Ticket: 7029
So far, the SANs were available as a part of IssuerDN via x509_parser
crate but SANs were not available to the SSLState* to be directly used
to setup and match against a sticky buffer.
Expose it to SSLStateConnp.
Feature 5234
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
This commit enhances the JSON output by introducing a feature for conditional port logging.
Now, port logging is dependent on the underlying protocol
(such as TCP, UDP, or SCTP), where port information is pertinent, while it
avoids unnecessary logging for protocols where a port is not utilized (e.g. ARP).
Furthermore, this update ensures that IP addresses and the protocol have
meaningful values set, rather than being logged as empty strings.
These changes will make each log entry more precise, eliminating cases where
5-tuple fields are empty or set to zero, indicating the absence of a field.
This change exposes 'JSONFormatAndAddMACAddr' as a public function,
allowing it to be reused across modules, such as the ARP logger, for logging
MAC addresses extracted from ARP packets.
Cover all the calls to DecodeBase64Block with the check for enough
space. Found by the recently introduced fuzz target.
Bug 7028
Fixes: c0bc43c393 ("util/base64: use decoder fns per RFC")
In the default config iface bypass support is not enabled,
and storage API not initialized for it. Using it will lead to a crash.
This commit first checks if the device storage API is initialized.
Bug: #7022.
Rules would allow checking against value 0, but internally the value
was used to indicate "no value". To address this, the internals now
return negative values for not found. This way value 0 can be fully
supported.
Bug: #6834.
- Remove no_mangle and pub from FFI functions that are only accessed
with a function pointer.
- Rename all no_mangle FFI functions to our C naming scheme.
Store IPv6 decoder vars in a new Packet::l3 section in the packet.
Use inline functions instead of the often multi-layer macro's for
various IPv6 header getters.
Ticket: #6938.
Store IPv4 decoder vars in a new Packet::l3 section in the packet.
Use inline functions instead of the often multi-layer macro's for
various IPv4 header getters.
Ticket: #6938.
Improve readability by setting up data/data_len once before
passing on to the other decoders.
Work in preparation of other decoder changes.
Ticket: #5517.
Replace it with inline functions. Adds inline functions to
wrap PKT_IS_IPV4/PKT_IS_IPV6.
This is in preparation of removing the macro's, and cleaning up the
header pointers.
Ticket: #5517.
Use of potentially dangerous function: localtime
Windows localtime is thread safe and no localtime_r is availabe on
Windows, so use separate logic for Windows.
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.
Since there is code in place that skips over invalid base64 characters
and creates a new array out of the remainder vector, all test must be
made after that final array has been created and against the variable
that holds the actual length of the final array.
Also, don't treat a successful decoding that had invalid characters as
errors. The caller does not treat them as errors so it makes sense to
consider it successful if something was decoded.
Bug 6964
Base64 decoder tried to handle several different cases in one fn
including how the data should be decoded and corresponding consumed and
decoded bytes calculated for RFC 2045 and RFC 4648 which are very
different. This left window for mistakes and edge cases that one mode is
not supposed to take care of while the other can.
Separate the decoder per RFC so as to have each variable their own scope
and playground. This also makes it much easier to debug if there are any
issues in decoding w.r.t. different callers.
This also takes care of some known issues like making sure the consumed
bytes are correctly handled.
Bug 6964
Ticket: 6889
To avoid regexp dos with too much backtracking.
This is already done on pcre keyword, and pcrexform transform.
We use the same default limits for rules parsing.
The list of fragments may still contain overlaps, so adding up the
fragment lengths is flawed. Instead track the largest size of
contiguous data that can be re-assembled.
Bug: #6675
Fix the BSD policy case where a subsequent fragment starts before an
original fragment and overlaps the beginning of the original
fragment. In this case the overlapping data from the new fragment is
preferred.
Suricata was preferring the data from the original fragment, but it
should only do that when the original fragment has an offset <= to the
new fragment.
- Adds tests for this case.
Bug: #6669
Instead of breaking the loop when the current fragment does not have
any more fragments, set a flag and continue to the next fragment as
the next fragment may have data that occurs before this fragment, but
overlaps it.
Then break if the next fragment does not overlap the previous.
Bug: #6668
When we only have stream matches.
Ticket: 6846
This solves the case where another transaction was created
by parsing data in the other direction, before running the
detection.
Like
1. get data in direction 1
2. acked data: parse it, but do not run detection in dir 1
3. other data in direction 2
4. other data acked : parse it and create new tx,
then run detection for direction 1 with data from first packet
Unsafe handling of buffer offset and to be inserted data's length
could lead to a integer overflow. This in turn would skip growing
the target buffer, which then would be memcpy'd into, leading to
an out of bounds write.
This issue shouldn't be reachable through any of the consumers of
the API, but to be sure some debug validation checks have been
added.
Bug: #6903.
For the capture methods that support livedev and IPS,
livedev.use-for-tracking is not supported.
This setting causes major flow tracking issues, as both sides of
a flow would be tracked in different flows.
This patch disables the livedev.use-for-tracking setting if it
is set to true. A warning will be issued.
Ticket: #6726.
When we added feature #5976 (72146b969), we overlook that we also have
a config stats option for the human-readable stats logs to output
0 counters.
Due to not seeing this before, we now have two different setting names
for basically the same thing, but in different logs:
- zero-valued-counters for EVE
- null-values for stats.log
This ensures we use the same terminology, and change the recently added
one to `null-values`, as this one has been around for longer.
Task #6962
Issue: 6864
Multiple IP options were not handled properly as the value being OR'd
into the packet's ip option variable were enum values instead of bit
values.
Ticket: 6878
Follow up on 15649424a7
When adding many sequence nodes, either from start or scalar event
We add "sequence nodes" whose name is an integer cf sequence_node_name
and then run ConfNodeLookupChild to see if it had been already set
(from the command line cf comment in the code)
And ConfNodeLookupChild iterates the whole linked list...
1. We add node 1
2. To add node 2, we check if node 1 equals this new node
3. To add node 3, we check if nodes 1, or 2 equals this new node's name
And so on...
This commits avoids these checks ig the list is empty at the beginning
Datasets that hit the memcap limit need to be discarded if the memcap is
hit or otherwise the datasets are still loaded with partial data while
the signature is not loaded due to the memcap error.
Ticket: #6678
Issue: 6957
Rather than selecting the thread_id index by packets traveling to the
server, use the flow flags. If the flow has been reversed, the second
slot is represents the thread id to be used.
Recognize PPP_CCP, PPP_CBCP and PPP_COMP_DGRAM.
Does not implement decoders for these record types, so these
are logged as unsupported types. Was "wrong_type" before.
This commit moves the memcap pressure/pressure_max stats from the global
stats namespace into the memcap namespace.
With per-thread stats, they will be within the flow-manager's values.
Issue: 6398
Eve's packet_info.linktype should correctly indicated what the `packet`
field contains. Until now it was using DLT_RAW even if Ethernet or other
L2+ headers were present.
This commit records the datalink of the packet creating the first
fragment, which can include the L2+ header data.
Bug: #6887.
Some stats can be quite verbose if logging all zero valued-counters.
This allows users to disable logging such counters. Default is still
true, as that's the expected behavior for the engine.
Task #5976
With the addition of exception policy stats counters, the human readable
version of the sats log was mis-aligned, when counters for per-app-proto
were enabled.
Width change made large enough to accomodate a counter as long as
"app_layer.error.bittorrent-dht.exception_policy.pass_packet" which
could be valid.
Task #5816
We will register stats counters for all policies, even though for now
Suri only uses one possible configuration policy at a time. The idea is
that this could change in the near future, so we want to have this
ready.
Task #5816
Decode file needed ExceptionPolicy types and exception-policy file
needed Decode types, rendering some works quite difficult to work
around.
ExceptionPolicyToStr is useful for registering exception policy
counters, so make that public.
Part of
Task #5816
Commit 5592ec07 introduced a bug where ptv->slot was
assigned with the current one instead of the next one
in the list. This caused a Suricata crash in the DPDK mode.
Ticket: 6877
One of the lessons of the XZ backdoor story was that just linking to
libsystemd to call sd_notify is discouraged by the systemd project:
Lennart Poettering:
"PSA: In context of the xzpocalypse we now added an example reimplementation
of sd_notify() to our man page:
https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
It's pretty comprehensive (i.e. uses it for reload notification too), but
still relatively short.
In the past, I have been telling anyone who wanted to listen that if all you
want is sd_notify() then don't bother linking to libsystemd, since the
protocol is stable and should be considered the API, not our C wrapper
around it. After all, the protocol is so trivial"
From: https://mastodon.social/@pid_eins/112202687764571433
This commit takes the example code and uses it to reimplement the notify
logic.
The code is enabled if Linux is detected in configure. Since the code
won't do anything if the NOTIFY_SOCKET env var isn't set, this should
also work fine on systems w/o systemd.
Ticket: #6913.
Function prototype has changed in a recent release. Rather than dealing
with detecting that, fall back to our regular pattern of using
pcre2_substring_copy_bynumber().
Bug: #6918.
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
Expose LoadYamlConfig as SCLoadYamlConfig and remove it from
SuricataInit. This is required to allow the library user the ability
customize the loading of the configuration, for example doing some
programmatic configuration then loading a configuration file.
A configuration sequence node needs to have name, which is just its
index in the sequence.
Discovered by calling ConfDump() after Suricata was fully initialized.
Move the contents of SuricataMain into the `main()` function found in
main.c. This forces the Suricata application to bootstrap and run
Suricata through the same interfaces as a library user might do.
Required exposing StartInternalRunMode as SCStartInternalRunmode. Its
arguable whether those "actions" belong in the library or just the
application, but I think that is separation we can look at later.
For now the lib example and Suricata's own main are the same, however
the example will probably extend more into programmatically
configuring Suricata or dynamically registering a runmode, which
doesn't really belong the main Suricata application.
Expose the functions FinalizeRunMode and ParseCommandLine to library
users, renaming with the `SC` prefix in the process.
This involves moving "application" level details from SuricataInit
into SuricataMain, as parsing command line options should be opt-in
for a library user.
We want to be able to call ParseCommandLine from library users, but
currently library users don't have access to the `suricata` instance
type. Since this var is used other places as a global, use the global
one in ParseCommandLine as well.
Not ideal, but isolating SCInstance to a non-global will be another
challenge on its own.
Move WindowsInitService from SuricataInit() to SuricataMain(), as
initializing Suricata as a service is very specific to the application
and not something you'd want to happen in a library, and SuricataInit
is more common initialization for application and library usage.
Expose SuricataMainLoop and GlobalsDestroy so that SuricataMain can be
replicated by a library user of Suricata.
These removes the `suricata` instance as a function argument to some
of these functions, as the way we use it now, it serves no
purpose. However, it is a reminder that it should probably be
refactored to not be a global, as at some point it might be desirable
for to have multiple instances active without data sharing.
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
Issue: 6861
Without this commit, disabling rule profiling via suricatasc's command
'ruleset-profile-stop' may crash because profiling_rules_entered becomes
negative.
This can happen because
- There can be multiple rules evaluated for a single packet
- Each rule is profiled individually.
- Starting profiling is gated by a configuration setting and rule
profiling being active
- Ending profiling is gated by the same configuration setting and
whether the packet was marked as profiling.
The crash can occur when a rule is being profiled and rule profiling
is then disabled after one at least one rule was profiled for the packet
(which marks the packet as being profiled).
In this scenario, the value of profiling_rules_entered was
not incremented so the BUG_ON in the end profiling macro trips
because it is 0.
The changes to fix the problem are:
- In the profiling end macro, gate the actions taken there by the same
configuration setting and use the profiling_rues_entered (instead of
the per-packet profiling flag). Since the start and end macros are
tightly coupled, this will permit profiling to "finish" if started.
- Modify SCProfileRuleStart to only check the sampling values if the
packet hasn't been marked for profiling already. This change makes all
rules for a packet (once selected) to be profiled (without this change
sampling is applied to each *rule* that applies to the packet.
Ticket: 6578
When a protocol such as SSH sets no_inspection, we still have to
flush the current streams and packets that contain clear-text
for detection.
Ticket: #6305
When running FlowWorkerStreamTCPUpdate, one of the dequeued packet
may set the flow action to drop, without updating the not-pseudo
packet action, as is done usually with a previous call to
FlowHandlePacketUpdate
Victor Julien
Philippe Antoine
Jeff Lucovsky
Shivani Bhardwaj
Eric Leblond
Jason Ish
Philippe Antoine
Juliana Fajardini
Alexey Simakov
Jo Johnson
Richard McConnell
Giuseppe Longo
Andreas Herz
Sascha Steinbiss
Jeff Lucovsky
Lukas Sismis
Daniel Olatunji
jason taylor
Angelo Mirabella