New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[preview] Feature/newdetect/v17.3.5 #3147

Closed
wants to merge 26 commits into
base: master
from

Conversation

3 participants
@victorjulien

victorjulien commented Jan 12, 2018

Rewrite of the detect engine, esp the stateful logic. Much cleaner 'detect.c'.

Also includes preview of 'starts_with' and 'ends_with' keywords.

Adds new app-layer API call to register u64 with the protocol parser for keeping track of which prefilter engines already ran for a TX.

Changes TX tracking and cleanup. Much more aggressively frees TXs, and can free transactions out of order.

Since it's a lot of code changing and moving reviewing it is a bit challenging. Sorry about that.

PRScript output (if applicable):

inliniac added some commits Oct 13, 2017

flowbits: analyze and dump to json
Analyze flowbits to find which bits are only checked.

Track whether they are set and checked on the same level of 'statefulness'
for later used.

Dump flowbits to json including the sids that set/check etc the bit.
detect: rewrite of the detect engine
Use per tx detect_flags to track prefilter. Detect flags are used for 2
things:
1. marking tx as fully inspected
2. tracking already run prefilter (incl mpm) engines

This supercedes the MpmIDs API for directionless tracking
of the prefilter engines.

When we have no SGH we have to flag the txs that are 'complete'
as inspected as well.

Special handling for the stream engine:

If a rule mixes TX inspection and STREAM inspection, we can encounter
the case where the rule is evaluated against multiple transactions
during a single inspection run. As the stream data is exactly the same
for each of those runs, it's wasteful to rerun inspection of the stream
portion of the rule.

This patch enables caching of the stream 'inspect engine' result in
the local 'RuleMatchCandidateTx' array. This is valid only during the
live of a single inspection run.

Remove stateful inspection from 'mask' (SignatureMask). The mask wasn't
used in most cases for those rules anyway, as there we rely on the
prefilter. Add a alproto check to catch the remaining cases.

When building the active non-mpm/non-prefilter list check not just
the mask, but also the alproto. This especially helps stateful rules
with negated mpm.

Simplify AppLayerParserHasDecoderEvents usage in detection to only
return true if protocol detection events are set. Other detection is done
in inspect engines.

Move rule group lookup and handling into it's own function. Handle
'post lookup' tasks immediately, instead of after the first detect
run. The tasks were independent of the initial detection.

Many cleanups and much refactoring.
app-layer: detect flags API calls
Add API meant to replace the MpmIDs API. It uses a u64 for each direction
in a tx to keep track of 2 things:

1. is inspection done?
2. which prefilter engines (like mpm) are already completed
detect/flowbits: apply state knowledge
When stateless rules are depending on a flowbit being set by a stateful
rule, the inspection order is almost certainly wrong.

Switch stateless rules depending on stateful rules to being stateful.
This is used to turn 'TCP stream' inspecting rules (which are stateless
unless mixed with stateful keywords) into stateful rules.
app-layer: improve async and out of order txs
Free txs that are done out of order if we can. Some protocol
implementations have transactions running in parallel, where it is
possible that a tx that started later finishes earlier than other
transactions. Support freeing those.

Also improve handling on asynchronious transactions. If transactions
are unreplied, e.g. in the dns flood case, the parser may at some
point free transactions on it's own. Handle this case in
the app-layer engine so that the various tracking id's (inspect, log,
and 'min') are updated accordingly.

Next, free txs much more aggressively. Instead of freeing old txs
at the app-layer parsing stage, free all complete txs at the end
of the flow-worker. This frees txs much sooner in many cases.
detect: fix multiple files per tx inspect
Fix the inspection of multiple files in a single TX, where new files
may be added to the TX after inspection started.

Assign the hard coded id DE_STATE_FLAG_FILE_INSPECT to the file
inspect engine.

Make sure that sigs that do file inspection and don't match on the
current file always store a detailed state. This state will include
the DE_STATE_FLAG_FILE_INSPECT flag.

When the app-layer indicates a new file is available, for each sig
that has the DE_STATE_FLAG_FILE_INSPECT flag set, reset part of the
state so that the sig is evaluated again.
output/file: run file loggers in both directions
This avoids the wait for injected packets when file is already ready
to be logged.
detect/profiling: postpone setup
Do this to allow for including of runtime buffer registrations.
detect/content: introduce starts_with modifier
Add starts_with modifier to simplify matching patterns at the start
of a buffer.

Instead of:
    content:"abc"; depth:3;
This enables:
    content:"abc"; starts_with;

Especially with longer patterns this makes the intention of the rule
more clear and eases writing the rules.

Internally it's simply a shorthand for 'depth:<pattern len>;'.

Ticket https://redmine.openinfosecfoundation.org/issues/742

@victorjulien victorjulien requested review from jasonish, norg and OISF/core-team as code owners Jan 12, 2018

@jasonish

This comment has been minimized.

Show comment
Hide comment
@jasonish

jasonish Jan 16, 2018

Member

Does the non-Rust DNS parser need updates as well?

Member

jasonish commented Jan 16, 2018

Does the non-Rust DNS parser need updates as well?

@victorjulien

This comment has been minimized.

Show comment
Hide comment
@victorjulien

victorjulien Jan 17, 2018

Yup, adding it.

victorjulien commented Jan 17, 2018

Yup, adding it.

@victorjulien

This comment has been minimized.

Show comment
Hide comment
@victorjulien

victorjulien commented Jan 17, 2018

Replaced by #3169

@victorjulien victorjulien deleted the victorjulien:feature/newdetect/v17.3.5 branch Jan 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment