config/mt: Add vlan-tuple MT selector #10015

jlucovsky · 2023-12-08T15:56:04Z

Continuation of #10005

Add a new MT selector type to support use cases where a VLAN tuple should be used to determine the MT tenant.

Packets with one VLAN id will never match as vlan-tuple requires at least QinQ.

The tuple can hold up to 3 values -- this is the max supported by Suricata atm.

Tenants are selected by specifying a VLAN tuple, e.g., [1010, 5]. A packet matches when:

It has double VLAN encapsulation
The outer VLAN id is 1015
The inner VLAN id is 5

Wild card values are supported; values of 0 match 'any VLAN' value in the same position as expressed in the tuple:
Tenants are selected by specifying a VLAN tuple, e.g., [1010, 0]. A packet matches when:

It has double VLAN encapsulation
The outer VLAN id is 1015
The inner VLAN id always matches since it's a wildcard value.

Link to redmine ticket: 6237

Describe changes:

Add and document a new MT selector -- vlan-tuple -- for use cases where a VLAN pair should determines the tenant.

Updates

Rebase

Provide values to any of the below to override the defaults.

To use a pull request use a branch name like pr/N where N is the
pull request number.

Alternatively, SV_BRANCH may also be a link to an
OISF/suricata-verify pull-request.

SV_REPO=
SV_BRANCH=pr/1354
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

Issue: 6237 The VLAN tuple selector uses a tuple of values to select a tenant. - [ vlan-outermost, vlan-innermost] The tuple can contain as many VLAN values as supported by Suricata - currently 3. Each of these can accept a wild-card value (0). The tenant is selected by matching packet VLAN values with the selector values.

Issue: 6237

Show the default value used for the unix control socket instead of a generic name that's not used: custom.socket

jufajardini

Tough work writing documentation! From my end, things are looking good :)

suricata-qa · 2023-12-08T21:25:03Z

Information: QA ran without warnings.

Pipeline 16965

catenacyber · 2024-03-14T21:08:43Z

doc/userguide/configuration/multi-tenant.rst

+the selector ``vlan`` (default); the selector ``vlan-tuple`` should be used if QinQ is deployed and requires both
+the inner and outer VLAN id values to match to determine the tenant.
+Multiple VLANs can have the same tenant id. VLAN id values must be between 1 and 4094 with the ``vlan`` selector.
+A wildcard value of ``00`` can be used with the ``vlan-tuple`` selector.


Why double 0 here ?

catenacyber · 2024-03-14T21:10:35Z

python/suricata/sc/specs.py

            "type": int,
+            "variable": 1,


What is this file for ?

Should this variable be a bool ?

catenacyber · 2024-03-14T21:12:45Z

src/detect-engine.c

+                        SCLogError("vlan value %s is invalid; must be 0-4094", field->val);
+                        goto bad_mapping;
+                    }
+                    traffic_id.vlan.tuple[idx++] = vlan_id;


We can overflow by specifying a bad config with too many clans ? (we should bounds check on idx)

Thanks -- will protect this.

catenacyber · 2024-03-14T21:17:47Z

src/runmode-unix-socket.c

+        } else {
+            args.vlan.tuple[count++] = (uint16_t)json_integer_value(hargs);
+        }
+    }


We should return early, should we not ?
We are not expecting hargs_tuple if we got hargs

src/runmode-unix-socket.c

catenacyber · 2024-03-14T21:23:24Z

src/runmode-unix-socket.c

+                            answer, "message", json_string("hargs_tuple invalid vlan id"));
+                    return false;
+                }
+                args.vlan.tuple[count++] = (uint16_t)vlan;


overflow on evil socket message

and not checking that there is at least one non-zero vlan

catenacyber · 2024-03-14T21:24:52Z

src/runmode-unix-socket.c

+            }
+        } else if (json_is_integer(hargs)) {
+            int vlan = json_integer_value(hargs);
+            if (vlan < 0 || vlan > 4094) {


0 should not be accepted, right ?

"0" is a wildcard value

But only for vlan_tuple, not for a single vlan, right ?

catenacyber · 2024-03-14T21:28:52Z

src/detect-engine.c

+        if (tenant_val.vlan.count == traffic_id.vlan.count) {                                      \
+            match = true;                                                                          \
+            for (int i = 0; i < tenant_val.vlan.count; i++) {                                      \
+                if (tenant_val.vlan.tuple[i] != traffic_id.vlan.tuple[i]) {                        \


Should this not use special case 0 ?

catenacyber · 2024-03-14T21:28:59Z

src/detect-engine.c

+}
+
+/* Match if the configured vlan match is a wildcard or the vlan ids match */
+#define VLAN_TUPLE_MATCH(tenant_val, traffic_val) ((tenant_val == 0) || (tenant_val == traffic_val))


Is this macro used ?

catenacyber

Some overflows to avoid at least

suricata-qa · 2024-04-18T15:00:03Z

Information: QA ran without warnings.

Pipeline 16965

jlucovsky · 2024-06-19T10:07:16Z

Closed for further evaluation.

jlucovsky added 3 commits December 8, 2023 10:31

detect/mt: Add support for vlan-tuple selector

379cc50

Issue: 6237

conf/socket: Include the default socket name

ce04bf2

Show the default value used for the unix control socket instead of a generic name that's not used: custom.socket

jlucovsky requested review from jasonish, inashivb, jufajardini, victorjulien and a team as code owners December 8, 2023 15:56

jlucovsky mentioned this pull request Dec 8, 2023

config/mt: Add vlan-tuple MT selector #10005

Closed

jufajardini reviewed Dec 8, 2023

View reviewed changes

catenacyber reviewed Mar 14, 2024

View reviewed changes

src/runmode-unix-socket.c Show resolved Hide resolved

catenacyber reviewed Mar 14, 2024

View reviewed changes

src/runmode-unix-socket.c Show resolved Hide resolved

catenacyber reviewed Mar 14, 2024

View reviewed changes

catenacyber mentioned this pull request Mar 20, 2024

multi-tenant: Basic multi-tenant selector tests OISF/suricata-verify#1354

Closed

catenacyber requested changes Mar 21, 2024

View reviewed changes

jlucovsky closed this Jun 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config/mt: Add vlan-tuple MT selector #10015

config/mt: Add vlan-tuple MT selector #10015

jlucovsky commented Dec 8, 2023 •

edited

Loading

jufajardini left a comment

suricata-qa commented Dec 8, 2023

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

jlucovsky Mar 16, 2024

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

jlucovsky Mar 16, 2024

catenacyber Mar 17, 2024

catenacyber Mar 14, 2024

catenacyber Mar 14, 2024

catenacyber left a comment

suricata-qa commented Apr 18, 2024

jlucovsky commented Jun 19, 2024

config/mt: Add vlan-tuple MT selector #10015

config/mt: Add vlan-tuple MT selector #10015

Conversation

jlucovsky commented Dec 8, 2023 • edited Loading

Provide values to any of the below to override the defaults.

jufajardini left a comment

Choose a reason for hiding this comment

suricata-qa commented Dec 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catenacyber left a comment

Choose a reason for hiding this comment

suricata-qa commented Apr 18, 2024

jlucovsky commented Jun 19, 2024

jlucovsky commented Dec 8, 2023 •

edited

Loading