Commit Graph

8496 Commits (bc27511e1a068c9a8126657c1cefaff66f0a9c28)
 

Author SHA1 Message Date
Victor Julien d5267c3020 uricontent: move debug func into unittests
Cleanup header, which lead to the app-layer-htp.h header needing to
be added in a few other places.
6 years ago
Victor Julien f502a52407 detect/replace: fix mem leak in error path 6 years ago
Victor Julien c66091cbb7 isdataat: fix mem leak in error path 6 years ago
Victor Julien ead1e9830b bits: avoid memory leak in case of adding types 6 years ago
Victor Julien f000b70b9a ipproto: fix memleak in error case 6 years ago
Victor Julien 985b6198d4 bytetest: don't leak memory in error condition 6 years ago
Victor Julien b60c469e7d yaml: fix potential memleak and suppress coverity issue 6 years ago
Victor Julien 39d6551437 outputs: fix memleaks in the error paths reported by coverity 6 years ago
Victor Julien 8433e9a8b1 coverity: suppress warning for intentional code 6 years ago
Victor Julien 6eba849068 rust/dns: don't compile unused C code if Rust is enabled 6 years ago
Victor Julien 8702fb9bea coverity: don't warn on fall back random 6 years ago
Victor Julien 7c4f0ac901 http: implement min size stream logic
Update HTTP parser to set the min inspect depth per transaction. This
allows for signatures to have their fast_pattern in the HTTP body,
while still being able to inspect the raw stream reliably with it.

The inspect depth is set per transaction as it:
- depends on the per personality config for min inspect size
- is set to the size of the actual body if it is smaller

After the initial inspection is done, it is set to 0 which disables
the feature for the rest of the transaction.

This removes the rescanning flush logic in commit
7e004f52c6 and provides an alternative
fix for bug #2522. The old approach caused too much rescanning of
HTTP body data leading to a performance degradation.

Bug #2522
6 years ago
Victor Julien a0734cf7e1 stream: introduce min inspect depth logic
Some rules need to inspect both raw stream data and higher level
buffers together. When this higher level buffer is a streaming
buffer itself, the risk of mismatch exists.

This patch allows an app-layer parser to set a 'min inspect depth'.
The value is used by the stream engine to keep at least this
depth worth of data, so that the detection engine can request
all of it for inspection.

For rules that have the SIG_FLAG_FLUSH flag set, data is inspected
not from offset raw_progress, but from raw_progress minus
min_inspect_depth.

At this time this is only used for sigs that have their fast_pattern
in a HTTP body and have raw stream match as well.
6 years ago
Victor Julien 005d876365 gcc8: fix format truncation warnings 6 years ago
Victor Julien 96869a367a stream/tcp: be more liberal in last_ack
Don't set even if seq is before next_seq, as this could still be
a valid packet that was sent before the state was reached.
6 years ago
Victor Julien 22a6372226 stream/tcp: add debug statements to state dispatcher 6 years ago
Victor Julien ad0794015a unittests: fix format-truncation warning 6 years ago
Victor Julien a0c37ff82d flow/timeout: code simplification and cleanup 6 years ago
Victor Julien 78345cc887 flow-manager: fix unittest initialization 6 years ago
Eric Leblond 37d10d3537 af-packet: close the socket in case of early fail 6 years ago
Eric Leblond 6d2fc70190 log-filestore: fix file descriptor leak
In the case we exceed the number of simultaneously open
file we can reach a state were we will not close the file
after writing.

Thanks to Steve Grubb <sgrubb@redhat.com> for the analysis.
6 years ago
Victor Julien 530203bfd5 detect/prefilter: speed up setup
If the global detect.prefilter.default setting is not "auto", it is
wasteful to run each prefilter setup routine. This patch tracks which
of the engines have been explicitly enabled in the rules and only
runs those.
6 years ago
Victor Julien df6b985f3c detect/prefilter: fix prefilter when setting is 'mpm'
When prefilter is not enabled globally, it is still possible to
enable it per signature. This was broken however, as the setup
code would never be called.

This commit always call the setup code and lets that sort out
which signatures (if any) to enable prefiltering for.
6 years ago
Victor Julien c80aae1b5b detect: limit flush logic to sigs that need it
Limit the early 'flush' logic to sigs that actually need to match
on both stream and http bodies.
6 years ago
Victor Julien 4fc04f17d8 detect/prefilter: fix alias for fast_pattern
If prefilter is used on a content keyword, it acts as a simple
fast_pattern statement. This was broken because the SIG_FLAG_PREFILTER
flag bypasses MPM for a sig. This commits fixes this by not setting
the flag when it should act as fast_pattern.
6 years ago
Victor Julien 4ca131d9bc detect/http: flush bodies when inspecting stream
The HTTP bodies (http_client_body and http_server_body/file_data) use
settings to control how much data we have before doing first inspection:

    request-body-minimal-inspect-size
    response-body-minimal-inspect-size

These settings default to 32k as quite some existing rules need this.

At the same time, the 'raw stream' inspection uses its own limits. By
default it inspects the data in blocks of about 2.5k. This could lead
to a situation where rules would not match.

For example, with 2 rules like this:

    content:"abc"; content:"data="; http_client_body; depth:5; sid:1;
    content:"xyz"; sid:2;

Sid 1 would only be inspected when the POST body reached the 32k limit
or when it was complete. Observed case shows the POST body to be 18k.
Sid 2 is inspected as soon as the 2.5k limit is reached, and then again
for each 2.5k increment. This moves the raw stream tracker forward.

So by the time sid 1 is inspected, some 18/19k into the stream, the
raw stream tracker is actually already moved forward for approximately
17.5k, this leads to the stream match of sid 1 possibly not matching.
Since the body match is at the start of the buffer, it makes sense
that the body and stream are inspected together.

The body inspection uses a tracker 'body_inspected', that keeps track
of how far into the body both MPM and per signature inspection has
moved.

This patch updates the logic in 2 ways:

1. it triggers earlier HTTP body inspection, which is matched to the
   stream inspection. When the detection engine finds it has stream
   data available for inspection, it passes the new 'STREAM_FLUSH'
   flag to the HTTP body inspection code. Which will then do an
   early inspection, even if still before the min inspect size.

2. to still somewhat adhere to the min inspect size, the body
   tracker is not updated until the min inspect size is reached.
   This will lead to some re-evaluation of the same body data.

If raw stream reassembly is disabled, this 'STREAM_FLUSH' flag is
never set, and the old behavior is used.

Bug #2522.
6 years ago
Victor Julien 36dacde025 stream: improve TCP CLOSED handling
Trigger app layer reassembly in both directions as soon as we've set
the TCP state to closed.

In IDS mode, if a toserver packet would close the state, the app layer
would not get updated until the next toclient packet. However, in
detection, the raw stream inspection would already use all available
stream data in detection and move the 'raw stream progress' tracker
forward. When in later (a) packet(s) the app layer was updated and
inspection ran on the app layer, the stream progress was already
moved too far forward. This would lead to signatures that matched
on both stream and app layer to not match.

By triggering the app layer reassembly as soon as the TCP state is
set to closed, the inspection as both the stream and app layer data
available at the same time so these rules can match.

Bug: #2570
Bug: #2554
6 years ago
Victor Julien d58f81d01b detect/files: don't prune files for bad packets
A bad packet (rejected by stream engine) could still trigger a file
prune, even though (most of the) detection wouldn't happen for the
packet. The next valid packet would then not be able to match on the
file, as it was already freed.

This patch uses the same logic before file pruning as in the detect
engine.

Bug: 2576
6 years ago
Victor Julien acd30a4397 detect/filehash: try to open data file from rulefile dir
If the data file can't be found in the default location, which
normally is 'default-rule-path', try to see if it can be found
in the path of the rule file that references it.

This makes QA much easier.
6 years ago
Victor Julien 8f32fad935 flow: flag packets as established for async
If a stream is async we see only on side of the traffic. This would
lead to the flow engine not flagging packets as 'established' even
if the flow state was in fact established. The flow was tagged as
such by the TCP engine.

This patch considers the flow state for setting the packet flag.

Bug #2491.
6 years ago
Victor Julien d036374749 enip: support gaps
Due to a bug in the GAP handling the TCP layer the parser would already
get data after GAPs before.
6 years ago
Victor Julien efaa7f13de stream: improve overlap detection
Improve detection of overlapping different data. Keep some data around
even if it was already ACK'd to check if packets have overlap.
6 years ago
Eric Leblond 778e0d1fa7 stream-tcp: fix stream depth computation
The stream depth computation was partly done with the stream_config
depth instead of using the value in the TCP session. As a result,
some configuration were resulting in abnormal behavior.

In particular, when stream depth was 0 and the file store depth was
not 0, Suricata was stopping the streaming on the flow as soon as
the filestore was started.

Reported-by: Pascal Delalande <pdl35@free.fr>
6 years ago
Victor Julien 076c02a223 stream: expand GAP detection 6 years ago
Victor Julien b6d0a4ee42 stream/app-layer: fix GAP handling issue
Fix case where data after GAP was processed as in order data by app-layer.
This happened even if protocol parser did not register to accept GAPs.
7 years ago
Maurizio Abba 8ec005afd6 detect: fix fileext and filename negated match
fix bug in fileext and filename preventing negated match to work
correctly. Previously, negated fileext (such as !"php") would cause a
match anyway on files that have extension php, as the last if would not
be accessed.

Using the same workflow as detect-filemagic we remove the final
isolated if and set it as a branch of the previous if.
7 years ago
Victor Julien 8771a02989 dcerpc: fix dce_iface not matching 7 years ago
Victor Julien 279d328439 changlog: update for 4.0.5 7 years ago
Victor Julien 186391423e mpm/hs: fix minor coverity warning
CID 1428797 (#1 of 1): Unchecked return value (CHECKED_RETURN)
    check_return: Calling HashTableAdd without checking return value
    (as is done elsewhere 5 out of 6 times).
7 years ago
Victor Julien 7e666c3443 enip: harden byte parsing code
Make sure we never read more than we have.

Reported-by: Henning Perl
7 years ago
Victor Julien 4dc26b3ae8 ssh: fix out of bounds read in banner parsing
Reported-by: Henning Perl
7 years ago
Jason Ish a3e0889ce3 rust/dns - remove extra parantheses
Removes rust compiler warning.

Redmine issue:
https://redmine.openinfosecfoundation.org/issues/2521
7 years ago
Victor Julien fbec94b79a detect/debug: suppress noisy info messages 7 years ago
Jason Ish 1b70e6a3ed yaml-loader: fix memory leak on fail include
Redmine issue:
https://redmine.openinfosecfoundation.org/issues/1929

If an include failed to load, either by the file not existing or
a parse error, the file pointer and yaml parser instance were
leaked.
7 years ago
Victor Julien 82fa3bc124 detect: fix memory leak in app-layer-event keyword
Bug #2515.
7 years ago
Victor Julien 0a6ff87761 http: fix setting event on the last tx 7 years ago
Victor Julien 92bc5766f4 http: set events for too many layers of compression
libhtp would already issue warnings, but these were not mapped
to events yet.
7 years ago
Victor Julien 23e93b1264 stream: support RST getting lost/ignored
In case of a valid RST on a SYN, the state is switched to 'TCP_CLOSED'.
However, the target of the RST may not have received it, or may not
have accepted it. Also, the RST may have been injected, so the supposed
sender may not actually be aware of the RST that was sent in it's name.

In this case the previous behavior was to switch the state to CLOSED and
accept no further TCP updates or stream reassembly.

This patch changes this. It still switches the state to CLOSED, as this
is by far the most likely to be correct. However, it will reconsider
the state if the receiver continues to talk.

To do this on each state change the previous state will be recorded in
TcpSession::pstate. If a non-RST packet is received after a RST, this
TcpSession::pstate is used to try to continue the conversation.

If the (supposed) sender of the RST is also continueing the conversation
as normal, it's highly likely it didn't send the RST. In this case
a stream event is generated.

Ticket: #2501

Reported-By: Kirill Shipulin
7 years ago
Victor Julien f4d5af76a8 stream-events: fix mapping 7 years ago
Victor Julien f2ba4864d6 detect/stream_size: code cleanups 7 years ago