Fix address negation detection not resolving variables when
looking for the negation.
This patch makes use of the actual parsing routines to relay this
information to the signature parser.
Bug #3389.
Fixes: 92f08d85aa ("detect/iponly: improve negation handling in parsing")
If file prune is called inspect has already run. So if file is closed
we can just prune. No need to consider a window anymore.
When still in progress, fix the left_edge calculation.
Now that we call the file prune loop very regularly, we can move the
SMTP specific inspection pruning logic into this loop. Helps with
cases there we don't (often) update a files inspection trackers.
If we already know that the boundary exists, we can start looking
there. Otherwise, we can skip trying as the boundary is a subset
of the form end marker.
This commit ensures direction warnings for ICMP v4 and v6
are suppressed and corrects check so that both protocols
are checked (instead of the same protocol being checked twice).
Update the default packet size computation to use LiveDeviceName
instead of LiveDevice as the LiveDevice list is not built when
the default packet size is built.
Coccinelle test was doing a false positive on the function
AppLayerParserStateSetFlag and AppLayerParserStateIssetFlag.
To address that, this patch adds a new coccinelle markup:
/* coccinelle: AppLayerParserStateSetFlag():2,2:APP_LAYER_PARSER_ */
It indicates that AppLayerParserStateSetFlag is a setter and getter
and that the checks should be disabled inside the function.
Currently this markup is only used for that but following patch will
add some checks on option value.
When transforms are part of a rule, improve information displayed with
fast patterns to include the original buffer name and whether any
transform(s) are applied.
When registing a detection engine, check that the app-layer
protocol supports tx detect flags. Exit with a fatal
error if it does not as this is a code implementation
error that should be resolved during development.
Add method to check if a parser for an app-layer protocol
supports tx detect flags.
This is a bit of a hack for now as where we need to run
this check from we do not have the IP protocol.
When HTTP pipelining was in use, the transaction id used for events
and files could be off. If the request side was several requests ahead
of the responses, it would use the HtpState::transaction_cnt for events
and files, even though that is only incremented on complete requests.
Split request and response tx id tracking. The response is still handled
by the HtpState::transaction_cnt, but the request side is now handled by
its own logic.
Since ebcc4db84a the flow worker runs
file pruning after parsing, detection and loging. This means we can
simplify the pruning logic. If a file is in state >= CLOSED, we can
prune it. Detection and outputs will have had a final chance to
process it.
Remove the calls to the pruning code from Rust. They are no longer
needed.
If a protocol does not support TxDetectFlags, don't try to use them.
The consequence of trying to use them was that a TX would never be
considered done, and it would never be freed. This would lead to excessive
memory use and performance problems due to walking an ever increasing
list.
When a BPF filter is given on the command line when reading a
pcap file, the BPF filter is not honored.
The regression has been introduced in:
commit 3ab9120821
Author: Dana Helwig <dana.helwig@protectwise.com>
Date: Thu Apr 27 11:17:16 2017 -0600
source-pcap-file: Pcap Directory Mode (Feature #2222)
Reported-By: Tim Colin <tcolin@et.esiea.fr>
A BUG_ON statement would seemingly randomly trigger during the threading
shutdown logic. After a packet thread reached the THV_RUNNING_DONE state,
it would sometimes still receive flow timeout packets which would then
remain unprocessed.
1 main: TmThreadDisableReceiveThreads(); <- stop capturing packets
2 worker: -> TmThreadTimeoutLoop (THV_FLOW_LOOP) phase starts
3 main: FlowForceReassembly(); <- inject packets from flow engine
4 main: TmThreadDisablePacketThreads(); <- then disable packet threads
5 main: -> checks if 'worker' is ready processing packets
6 main: -> sends THV_KILL to worker
7 worker: breaks out of TmThreadTimeoutLoop and changes to THV_RUNNING_DONE.
Part of the problem was with (5) above. When checking if the worker was
already done with its work, TmThreadDisablePacketThreads would not consider
the injected flow timeout packets. The second part of the problem was with (7),
where the worker checked if it was ready with the TmThreadTimeoutLoop in a
thread unsafe way.
As a result TmThreadDisablePacketThreads would not wait long enough for the
worker(s) to finish its work and move the threads to the THV_RUNNING_DONE
phase by issuing the THV_KILL command.
When waiting for packet processing threads to process all in-flight packets,
also consider the 'stream_pq'. This will have received the flow timeout
packets.
Bug #1871.
This commit adds `ARRAY_SIZE` as an helper for determining the number of
elements in an initialized array. The calculation is the same but the
macro provides a convenient shortcut. The implementation was borrowed
from the kernel sources.
Create a single function to return the version string, to avoid lots
of ifdefs in multiple places.
Make the version determine the 'release' status. If the version from
autoconf has '-dev' in the name, it is not a release. If it hasn't
it is considered a release version.
Avoids using uninitialized memory. Show showed itself
in nonsense values in counters, and in nfq_handle_packet
errors that were likely the result of passing uninitialized
memory to the nfq API.
Bug 3263.
Bug 3120.
Fixes: b2a6c60dee ("source-nfq: increase maximum queues number to 65535")
NFQ can generate warnings/errors with a delay. After Suricata has
succesfully passed a verdict to the kernel, there are still things
that can go wrong for that verdict. This is then passed to the
queue through a netlink error message, which leads to nfq_handle_packet
returning an error code.
Suppress the warning. Also remove the errno/strerror use as
nfq_handle_packet does not set the errno.
Thanks to Florian Westphal.
Bug 3120.
TCP_OPT_INVALID_LEN was set if the opt len was 2. While useless
an empty SACK is not uncommon.
Seen on an iOS device talking to an Apple server.
Bug #3254.
If the DNS log version is not set, we default to v2. This should
not be warning, but better logged at the config level.
A warning will still be logged if the value is set but is not
1 or 2.
Implement min_inspect_depth for SMTP so that file_data and
regular stream matches don't go out of sync on the stream start.
Added toserver bytes tracking.
Bug #3190.
Change the meaning of the verbosity flag to change the log
level to fixed levels instead of being relative to whats
configured.
-v => INFO
-vv => PERF
-vvv => CONIFG
-vvvv => DEBUG
But do now allow -v to decrease the verbosity.
Bug #1851
The log level of individual loggers (console, file, syslog) was
being capped by the default log level. For example, if the
default log level was notice, setting the file level to info
would still result in notice level logging.
Bug #3210
Ensure that RETR (STOR) have a filename -- otherwise, treat the command
string as malformed.
Added unittests for each command and verified that SEGV's occur without
parser change and no longer occur with the parser change.
This reverts commit 6dca50a322.
The test mode should actually test in system mode by default as
that is what tools like Suricata-Update need before issuing a
reload command.
A sigmatches 'Setup' function may indicate it intends to fail
silently after the first error. It will return -2 instead of -1
in this case.
This is tracked in the DetectEngineCtx object, so errors will
be shown again at rule reloads.
Add --strict-rule-keywords commandline option to enable strict rule
parsing.
It can be used without options or with a comma separated list:
--strict-rule-keywords
--strict-rule-keywords=all
--strict-rule-keywords=classtype,reference
Parsing implementations can use SigMatchStrictEnabled to check
if strict parsing is enabled for them and act accordingly.
References are currently not used in Suricata, so erroring out on
rules using a undefined reference is too harsh.
Just issue a warning once per unique missing reference.
Still initialize the classtype hash table so that the classtypes
rules use can be added to it.
The file missing now reports a warning instead of error, as we
will continue to work.
Effect of classification on Suricata's working is minimal. Impact
of adding undefined classtypes is large: rules will fail to load
completely. This also leads multiple lines of log output per rule,
which in a large ruleset can lead to excessive output.
This patch changes the classtype keyword behavior. Instead of erroring
and invalidating a rule, we will merely warn.
The undefined classtype is then defined with a default priority,
so other rules using the classtype will not also warn. This way
there will be just a single warning per missing classtype.
Detect duplicate instances and use the one with the highest
priority.
Use new priority flag to make the logic around explicit priority
sets easier to follow.
Minor code cleanups. Also clean up unittests.
Introduce Signature init_flag to indicate priority has been set.
This will be needed in a follow-up classtype update.
Detect duplicate priority instances in a keyword, and use the
highest priority in the rule. Do issue a warning in this case.
As the file prune is now moved to the flow worker, the file
prune is run later, meaning the first file has not yet
been pruned from the file container list.
Adjust test to look for a second file, and check the
flags on that file.
For commit addressing bug 2490.
If a keyword like filemd5 was being used without a filestore,
or a file output enabled, it would be pruned before detection
had a chance to match.
Consolidate file pruning to the end of the flow worker so files
are available for detection even when a file output is not
enabled.
Redmine issue:
https://redmine.openinfosecfoundation.org/issues/2490
At the startup, if the default log dir provided either by command line
options or suricat.yaml is not writable, the error comes quite later.
This patch makes suricata exit if there is such an error in the
beginning itself.
Closes redmine ticket #2386.
In file included from suricata-common.h:471,
from app-layer-enip-common.c:27:
app-layer-enip-common.c: In function ‘DecodeCIPRequestPathPDU’:
util-debug.h:222:31: warning: ‘req_path_class8’ may be used uninitialized in this function [-Wmaybe-uninitialized]
int _sc_log_ret = snprintf(_sc_log_msg, SC_LOG_MAX_LOG_MSG_LEN, __VA_ARGS__); \
^~~~~~~~
app-layer-enip-common.c:589:13: note: ‘req_path_class8’ was declared here
uint8_t req_path_class8;
^~~~~~~~~~~~~~~
app-layer-enip-common.c:607:9: warning: ‘segment’ may be used uninitialized in this function [-Wmaybe-uninitialized]
switch (segment)
^~~~~~
app-layer-enip-common.c: In function ‘DecodeCIPResponsePDU’:
app-layer-enip-common.c:773:13: warning: ‘service’ may be used uninitialized in this function [-Wmaybe-uninitialized]
service &= 0x7f; //strip off top bit to get service code. Responses have first bit as 1
^~
app-layer-enip-common.c: In function ‘DecodeCIPRequestPDU’:
app-layer-enip-common.c:503:25: warning: ‘path_size’ may be used uninitialized in this function [-Wmaybe-uninitialized]
offset += path_size * sizeof(uint16_t); //move offset past pathsize
~~~~~~~~~~^~~~~~~~~~~~~~~~~~
app-layer-enip-common.c:506:5: warning: ‘service’ may be used uninitialized in this function [-Wmaybe-uninitialized]
switch (service)
^~~~~~
Bug #3139.
Improve warnings when eve.stats can't work because of the global config
missing or disabled.
Issue warning if global config is missing but stats are still enabled due
to the legacy stats.log.
Issue clearer warning when stats are disabled and unix socket dump-counters
command is issued.
Warnings include links to docs.
Bug #2465.
bzero(3): The bzero() function is deprecated (marked as LEGACY in
POSIX.1-2001); use memset(3) in new programs. POSIX.1-2008 removes
the specification of bzero().
Use memset instead.
Replace index by strchr and rindex by strrchr.
index(3) states "POSIX.1-2008 removes the specifications of index() and
rindex(), recommending strchr(3) and strrchr(3) instead."
Add index/rindex to banned function check so they don't get reintroduced.
Bug #1443.
OpenSSL uses 30, so this seems a reasonable limit.
Set a smaller limit than before to reduce the resources spent on
specially crafted input designed to be maximally expensive.
Set and Sequence parsers would pass on max available data instead
of the size of their object.
Malformed data could trigger massive recursion this way, leading
to spending much more resources than necessary.
Found using AFL.
Bug #3185.
Field is at data+1 offset, not +3. Also makes sure we always stay
within checked data bounds.
Reported-by: Sirko Höer -- Code Intelligence for DCSO.
Bug #3176.
Before re-assembling, check that the first fragment is large
enough to contain the IPv4 or IPv6 header to prevent
an out of bounds read (IPv4) or write (IPv6).
Reported-by: Sirko Höer -- Code Intelligence for DCSO.
Bug #3171.
Restructure code to make it clearer that either 'basic', 'extended'
or 'custom' is being printed, by creating one function for each of
the possibilities.
Add a rule keyword, dns.opcode to match on the opcode flag
found in the DNS request and response headers.
Only exact matches are allowed with negation.
Examples:
- dns.opcode:4;
- dns.opcode:!1;
This permits to use stream-depth value set for file-store.
Currently if a file is being stored and hits a limit,
such as request or response body, it will be truncated
although file-store.stream-depth is enabled but the file should be
closed and not truncated.
Two unit tests have been added to verify that:
- a file is stored correctly
- chunk's length computation doesn’t cause an underflow
output-json-ftp.c: In function ‘JsonFTPLogger’:
output-json-ftp.c:129:9: warning: ‘js_respcode_list’ may be used uninitialized in this function [-Wmaybe-uninitialized]
129 | json_object_set_new(cjs, "completion_code", js_respcode_list);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
output-json-ftp.c:74:13: note: ‘js_respcode_list’ was declared here
74 | json_t *js_respcode_list;
| ^~~~~~~~~~~~~~~~
output-json-ftp.c:128:9: warning: ‘js_resplist’ may be used uninitialized in this function [-Wmaybe-uninitialized]
128 | json_object_set_new(cjs, "reply", js_resplist);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
output-json-ftp.c:73:13: note: ‘js_resplist’ was declared here
73 | json_t *js_resplist;
| ^~~~~~~~~~~
Datasets are sets/lists of data that can be accessed or added from
the rule language.
This patch implements 3 data types:
1. string (or buffer)
2. md5
3. sha256
The patch also implements 2 new rule keywords:
1. dataset
2. datarep
The dataset keyword allows matching against a list of values to see if
it exists or not. It can also add the value to the set. The set can
optionally be stored to disk on exit.
The datarep support matching/lookups only. With each item in the set a
reputation value is stored and this value can be matched against. The
reputation value is unsigned 16 bit, so values can be between 0 and 65535.
Datasets can be registered in 2 ways:
1. through the yaml
2. through the rules
The goal of this rules based approach is that rule writers can start using
this without the need for config changes.
A dataset is implemented using a thash hash table. Each dataset is its own
separate thash.
Thread safe hash table implementation based on the Flow hash, IP Pair
hash and others.
Hash is array of buckets with per bucket locking. Each bucket has a
list of elements which also individually use locking.
1. Set WARN_UNUSED macro on DetectSignatureSetAppProto.
2. Replace all direct 'sets' of Signature::alproto from keyword registration.
Closes redmine ticket #3006.