Conversation
| #ifndef SURICATA_DETECT_VLAN_ID_H | ||
| #define SURICATA_DETECT_VLAN_ID_H | ||
|
|
||
| #define ANY 0 |
There was a problem hiding this comment.
I do not think we want to export this
| typedef struct DetectVlanIdData_ { | ||
| uint16_t id; /* id to match */ | ||
| uint8_t layer; /* id layer to match */ | ||
| } DetectVlanIdData; |
There was a problem hiding this comment.
This structure is only used in detect-vlan-id.c: it can live there
| static void DetectVlanIdRegisterTests(void); | ||
| #endif | ||
|
|
||
| #define PARSE_REGEX "^([0-9]+)(?:,\\s*([0-9]|any))?$" |
There was a problem hiding this comment.
I think we do not want anymore parsing with regex, rust would be welcome
| @@ -0,0 +1,21 @@ | |||
| Vlan.id Keyword | |||
| ============== | |||
|
|
||
| :: | ||
|
|
||
| alert ip any any -> any any (msg:"Vlan ID is equal to 200"; vlan.id:200,any; sid:1;) No newline at end of file |
There was a problem hiding this comment.
Reading the doc, what is the difference between vlan.id:200,any; and vlan.id:200; ?
There was a problem hiding this comment.
There’s no difference, both match any of the VLAN IDs. Should I keep them both?
There was a problem hiding this comment.
I would keep only vlan.id:200; and document it
|
|
||
| const DetectVlanIdData *vdata = (const DetectVlanIdData *)ctx; | ||
| for (int i = 0; i < p->vlan_idx; i++) { | ||
| if (p->vlan_id[i] == vdata->id && (vdata->layer == ANY || vdata->layer - 1 == i)) |
There was a problem hiding this comment.
I would like to use generic integer support here, to support negation for instance.
| } | ||
|
|
||
| const DetectVlanIdData *vdata = (const DetectVlanIdData *)ctx; | ||
| for (int i = 0; i < p->vlan_idx; i++) { |
There was a problem hiding this comment.
Do not need the loop of vdata->layer != ANY
| */ | ||
| static int DetectVlanIdParseTest03(void) | ||
| { | ||
| DetectVlanIdData *vdata = DetectVlanIdParse(NULL, "200,any"); |
There was a problem hiding this comment.
Should we support 200,last ? or 200,-1 ?
| @@ -0,0 +1,30 @@ | |||
| /* Copyright (C) 2022 Open Information Security Foundation | |||
|
|
||
| Syntax:: | ||
|
|
||
| vlan.id: id[,layer]; |
There was a problem hiding this comment.
What is the default behavior when we do not specify a layer ?
There was a problem hiding this comment.
It behaves just like specifying 'any', it matches any of the VLAN IDs.
There was a problem hiding this comment.
Then it's good to have that added to the docs :)
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
Thanks for the nice work, I think we should get rid of PCRE here
|
|
||
| Syntax:: | ||
|
|
||
| vlan.id: id[,layer]; |
There was a problem hiding this comment.
Then it's good to have that added to the docs :)
| @@ -0,0 +1,276 @@ | |||
| /* Copyright (C) 2022 Open Information Security Foundation | |||
There was a problem hiding this comment.
nit: copyright year should be 2024
3 participants
Ticket: #1065
Make sure these boxes are checked accordingly before submitting your Pull Request -- thank you.
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
Describe changes:
SV_BRANCH= OISF/suricata-verify#2134
Previous PR: #12070