detect: add vlan.id keyword - v8 #12333
Conversation
vlan.id matches on Virtual Local Area Network IDs It is an unsigned 16-bit integer Valid range for the default configuration = [1-4094] Supports prefiltering Ticket: OISF#1065
AkakiAlice
requested review from
jasonish,
jufajardini and
victorjulien
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12333 +/- ##
==========================================
- Coverage 83.26% 83.20% -0.06%
==========================================
Files 912 914 +2
Lines 257643 257864 +221
==========================================
+ Hits 214521 214564 +43
- Misses 43122 43300 +178
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
|
||
| static int PrefilterSetupVlanId(DetectEngineCtx *de_ctx, SigGroupHead *sgh) | ||
| { | ||
| return PrefilterSetupPacketHeader(de_ctx, sgh, DETECT_VLAN_ID, SIG_MASK_REQUIRE_FLOW, |
There was a problem hiding this comment.
why do we require flow here for a packet matching keyword? Shouldn't this be SIG_MASK_REQUIRE_REAL_PKT?
| =============== ================================================ | ||
| [default] Match all layers | ||
| 0 - 2 Match specific layer | ||
| ``-3`` - ``-1`` Match specific layer with back to front indexing |
There was a problem hiding this comment.
I think it would be good to clarify what front indexing means here. Perhaps with a small illustration to indicate how the indexing works, e.g.
[ethernet]
[vlan 666 (index 0 and -2)]
[vlan 123 (index 1 and -1)]
[ipv4]
[udp]
| #[derive(Debug, PartialEq)] | ||
| pub struct DetectVlanIdData { | ||
| pub du16: DetectUintData<u16>, | ||
| pub layer: i8, |
There was a problem hiding this comment.
this need some documentation
| 0 - 2 Match specific layer | ||
| ``-3`` - ``-1`` Match specific layer with back to front indexing | ||
| all Match only if all layers match | ||
| count Match on the number of layers |
There was a problem hiding this comment.
while I like that we can match on this, I do question integrating it with a vlan.id keyword. Should we have a vlan.layers keyword instead? I feel that these options are not only not about the id, they also bring their own syntax.
Ticket: #1065
Contribution style:
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
Our Contribution agreements:
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Changes (if applicable):
(including schema descriptions)
https://redmine.openinfosecfoundation.org/projects/suricata/issues
Link to ticket: https://redmine.openinfosecfoundation.org/issues/1065
Description:
vlan_id.rs changes:
SV_BRANCH=OISF/suricata-verify#2208
Previous PR: #12324