detect/bsize: apply as content depth ##15825
Open
jlucovsky wants to merge 4 commits into
Open
Conversation
The bsize keyword only checked the buffer length at inspection time; the content patterns in the same buffer were still searched across the whole buffer before that length check ran. That made bsize noticeably more expensive than the equivalent depth/isdataat form. When bsize gives a usable upper bound (bsize:N, bsize:<N, or a range), apply it as a depth to every content in that buffer: a content can't match past the end of the buffer, and the buffer is at most bsize bytes, so the mpm and content inspection can bound their search instead of scanning the entire buffer. This mirrors the existing dsize and urilen optimizations. When a buffer carries more than one bsize, the tightest (smallest) upper bound is used, since the buffer must satisfy them all. When an exact bsize equals the length of a single content in the buffer, that content must span the whole buffer, so it is marked startswith/endswith -- setting the mpm anchoring and the endswith inspection short-circuit -- rather than only being bounded by depth. The depth is applied after DetectContentPropagateLimits, like dsize, because the DETECT_CONTENT_DEPTH flag feeds propagation and setting it earlier would change how limits chain across contents. The derived depth is recorded with a DETECT_CONTENT_BSIZE2DEPTH flag so engine-analysis can report that the content depth came from bsize. Issue: 4226
Both the content-depth optimization and the content-length validation need the usable bsize upper bound for a buffer. Extract that into a shared helper returning the tightest (smallest) bound across all of the buffer's bsize keywords, and use it in both places. Previously the validation callback inspected only the first bsize, so a signature whose required content exceeds a later, tighter bsize -- e.g. a 10-byte content with bsize:<20; bsize:<8 -- passed validation and became a silently never-matching rule. Validating against the tightest bound rejects it at load with a clear error, and both code paths now agree on which bound applies. A buffer's bsize keywords must all hold at once, so the same validation also rejects a buffer whose bounds share no satisfiable length -- two different exact bsizes, or a lower bound above an upper bound -- rather than letting it load as a silently never-matching rule. A lower and an upper bound that overlap (a range) stay valid. The bsize keyword reference documents this multiple-bsize behavior. Issue: 4226
Two engine-analysis hints for the bsize keyword: - When a sticky buffer holds a single content marked both startswith and endswith, the content must span the whole buffer, so its length is fixed. (isdataat:!1,relative folds into the same endswith flag.) bsize:<len> expresses that in one keyword and lets the engine prefilter on length, so the analyzer suggests it. - When a buffer combines a lower and an upper bsize bound (e.g. bsize:>N; bsize:<M), the analyzer suggests folding them into a single bsize range (bsize:lo<>hi), which expresses the two-sided bound more clearly. The suggestion is limited to genuine bound-pairs; same-direction or exact bsizes don't trigger it. These are rule-author-facing notes; neither rejects a rule. Issue: 4226
The JSON rule analysis report carries optional "notes" and "warnings" arrays, but they weren't described anywhere even though the surrounding report structure is. Add a short section explaining both fields and list a few representative notes, including the bsize-related ones, so signature writers know where to look for the optimizations the engine applied and the hints it offers. Issue: 4226
|
ERROR: ERROR: QA failed on ASAN_TLPR1_cfg. Pipeline = 32494 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #15825 +/- ##
==========================================
+ Coverage 82.93% 82.95% +0.02%
==========================================
Files 1003 1003
Lines 275204 275353 +149
==========================================
+ Hits 228239 228429 +190
+ Misses 46965 46924 -41
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continuation of #15806
Optimize content checks by using
bsizeas an upper bound, similar to whatdsizeandurilendo. This improves performance on large buffers; small buffers see less, if any, boost.Link to ticket: https://redmine.openinfosecfoundation.org/issues/4226
Describe changes:
bsizeas an upper bound and apply it as adepthso content inspection is bounded to the buffer instead of scanning the whole buffer.bsizeequals a lone content's length, mark the contentstartswith/endswithso the mpm is anchored and the engine prefilters on length.bsize, apply the tightest (smallest) upper bound, and reject at load a buffer whosebsizekeywords can never be satisfied together (two differing exact values, or a lower bound above an upper bound). A legitimate lower+upper pair forming a range stays valid.bsizewhere a content is pinned to the whole buffer the long way (startswith/endswith), and suggest folding a genuine lower+upperbsizepair into a singlebsize:lo<>hirange.bsizekeyword reference and the engine analysis notes/warnings section.Changes since #15806
Changes since #15674:
--strict-rule-keywordsidea for multiplebsize.Instead of rejecting multiple
bsize, a real lower+upper range is accepted(e.g.
bsize:<370; bsize:>359) and only infeasible combinations are rejected at load.genuine lower+upper pair, not for same-direction or exact bounds.
bug-4226-09 (shape-aware suggestion).
SV_BRANCH=OISF/suricata-verify#3155
Benchmark:
bsizeapplied as contentdepthMeasured with a standalone profiling harness (paired rules: each scenario has a
bsize-form rule and the hand-written equivalent), run over three captures. Reproduction kit (script + synthetic pcap) attached separately.smallFlows.pcap.gz
smallFlows.pcap.gz
bsize-bench.pcap.gz
bsize-bench.pcap.gz
bsize-bench.rules.gz
bsize-bench.rules.gz
Definitions
master(nobsize-as-depth optimization).bsizeupper bound applied as contentdepth; for anexact
bsizeequal to a lone content's length, the content is also markedstartswith+endswith).with
startswith; endswith.bsize-form rule ÷ the cost of itshand-written equivalent, computed within the same run.
~1.0means parity;>1.0means it is still paying extra. The within-run ratio cancels run-to-runand cross-binary tick noise.
bsizerule is evaluated. A drop from baseto opt means the depth bound made the MPM prefilter more selective.
1. Synthetic capture (uniform bodies — isolates the anchoring path)
2.
smallFlows.pcap(real mixed traffic, ~9 MB)3.
bigFlows.pcap(real mixed traffic, ~368 MB)Reading the results
The anchoring win (D) reproduces on every capture: the
bsize-form ruledrops from ~2x the cost of the manually-anchored equivalent to ~1.15x, and to
cheaper than an unconstrained content (0.79–0.90x of plain).
The
file.datadepth win (A) is prefilter-driven and only shows onrealistic traffic. On the synthetic capture the bodies are uniform, so the
fast-pattern can't become more selective and A is flat (100/100). On real
traffic the depth bound roughly halves how often the rule is evaluated —
smallFlows 699→396 (ratio 2.27→0.93), bigFlows 7006→3759 (ratio 2.56→1.17).
B (small
user_agent) and C (fixed-lengthhttp.method) are flat bydesign: the buffer is tiny or always equals the pinned length, so neither the
depth bound nor the
endswithshort-circuit has room to help.