If for a packet we have a TX N that has detect state and a TX N+1 that
has no detect state, but does have 'progress', we have a corner case
in stateful detection.
ContinueDetection inspects TX N, but cannot flag the rule in the
de_state_sig_array as the next (TX N+1) has already started and needs
to be inspected. 'StartDetection' however, is then unaware of the fact
that ContinueDetection already inspected the rule. It uses the per
session 'inspect_id' that is only moved forward at the end of the
detection run.
This patch adds a workaround. It uses the DetectEngineThreadCtx::
de_state_sig_array to store an offset between the 'base' inspect_id
and the inspect_id that StartDetection should use. The data type is
limited, so if the offset would be too big, a search based fall back
is implemented as well.
Fix storing state and bypassing detection. Previously we'd store
on a match only, meaning that StartDetection would rerun often.
Make sure StartDetection only stores if there is something to store.
The post match list was called with an unlocked flow until now.
However, recent de_state handling updates changed this. The stateful
detection code can now call the post match functions while keeping
the flow locked. The normal detection code still calls it with an
unlocked flow.
This patch adds a hint to the DetectEngineThreadCtx called
'flow_locked' that is set to true if the caller has already locked
the flow.
Use separate data structures for storing TX and FLOW (AMATCH) detect
state.
- move state storing into util funcs
- remove de_state_m
- simplify reset state logic on reload
Set actions that are set directly from Signatures using the new
utility function DetectSignatureApplyActions. This will apply
the actions and also store info about the 'drop' that first made
the rule drop.
When using AMATCH, continue detection would fail if the tx part
had already run. This lead to start detection rerunning, causing
multiple alerts for the same issue.
When using the inspection engines, track the current tx_id in the
thread storage the detect thread uses. As 0 is a valid tx_id, add
a simple bool that indicates if the tx_id field is set.
Previously, the alstate use in the main detect loop was unsafe. The
alstate pointer would be set duing a lock, but it would again be used
after one or more lock/unlock cycles. If the data pointed to would
disappear, a dangling pointer would be the result.
Due to they way flows are cleaned up using reference counting and
such, changes of this happening were very small. However, at least
one path can lead to this situation. So it had to be fixed.
Make DeStateDetectContinueDetection get it's own alstate pointer instead
of using the one that was passed to it. We now get and use it only
inside a flow lock.
Make DeStateDetectStartDetection get it's own alstate pointer instead
of using the one that was passed to it. We now get and use it only
inside a flow lock.
This is an intrusive change. This patch modifies the way AMATCH
inspection uses locking.
So far, each keyword did it's own locking. This lead to a situation
where a 'alstate' pointer was passed around that was not always
protected by a lock.
This patch moves the locking to the Stateful detection functions.
In case of multiple transactions, the stored AMATCH list would not have
been reset, but it would still be reconsidered. Even though none would
match, the engine would still conclude that the rule matched.
app-layer.[ch], app-layer-detect-proto.[ch] and app-layer-parser.[ch].
Things addressed in this commit:
- Brings out a proper separation between protocol detection phase and the
parser phase.
- The dns app layer now is registered such that we don't use "dnstcp" and
"dnsudp" in the rules. A user who previously wrote a rule like this -
"alert dnstcp....." or
"alert dnsudp....."
would now have to use,
alert dns (ipproto:tcp;) or
alert udp (app-layer-protocol:dns;) or
alert ip (ipproto:udp; app-layer-protocol:dns;)
The same rules extend to other another such protocol, dcerpc.
- The app layer parser api now takes in the ipproto while registering
callbacks.
- The app inspection/detection engine also takes an ipproto.
- All app layer parser functions now take direction as STREAM_TOSERVER or
STREAM_TOCLIENT, as opposed to 0 or 1, which was taken by some of the
functions.
- FlowInitialize() and FlowRecycle() now resets proto to 0. This is
needed by unittests, which would try to clean the flow, and that would
call the api, AppLayerParserCleanupParserState(), which would try to
clean the app state, but the app layer now needs an ipproto to figure
out which api to internally call to clean the state, and if the ipproto
is 0, it would return without trying to clean the state.
- A lot of unittests are now updated where if they are using a flow and
they need to use the app layer, we would set a flow ipproto.
- The "app-layer" section in the yaml conf has also been updated as well.
When generating an alert and storing it in the packet, store the tx_id
as well. This way the output modules can log the tx_id and access the
proper tx for logging.
Issue #904.
In debug validation mode, it is required to call application layer
parsing and other functions with a lock on flow. This patch updates
the code to do so.
The action field in Packet structure should not be accessed
directly as the tunneled packet needs to update the root packet
and not the initial packet.
This patch is fixing issue #819 where suricata was not able to
drop fragmented packets in AF_PACKET IPS mode. It also fixes
drop capability for tunneled packets.
Victor Julien
Ken Steele
Anoop Saldanha
Eric Leblond