Skip to content
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

Services: Intrusion Detection - Suricata doesn't accept "." in vlan network device names #187

Closed
2 tasks done
Monviech opened this issue Oct 11, 2023 · 24 comments
Closed
2 tasks done
Assignees
Labels
upstream Third party issue

Comments

@Monviech
Copy link
Member

Monviech commented Oct 11, 2023

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug

I try to be careful when making a bug report, I think this one is reproducible.

When a vlan interface name contains a dot "." , suricata fails to load.

To Reproduce

Steps to reproduce the behavior:

  1. Go to Interfaces: Other Types: VLAN
  2. Create a new VLAN:
    • Device: "vlan0.301"
    • Parent: Choose an interface, like igb0 or igc0...
    • VLAN tag: 301
    • Save
  3. Assign the new VLAN interface with the description "vlan301" and activate it
  4. Go to Services: Intrusion Detection: Administration
    • Enabled: X
    • IPS mode: X
    • Promiscuous mode: X
    • Interfaces: vlan301
  5. See error in /var/log/suricata/latest.log and suricata service stops.
    <Error> -- [ERRCODE: SC_ERR_FATAL(171)] - opening devname netmap:vlan0.301/R failed: Invalid argument

Expected behavior

Suricata starting normally, like with a vlan interface without "."

Describe alternatives you considered

Create all vlans interfaces without "." Though then the vlan "Device" helptext and input validation shouldn't allow "."

Changing the device name from "vlan0.301" to "vlan0301" makes suricata start.

<Notice> -- all 4 packet processing threads, 4 management threads initialized, engine started.

Environment

DEC740
OPNsense 23.7.5-amd64
FreeBSD 13.2-RELEASE-p3
OpenSSL 1.1.1w 11 Sep 2023

Additional Information

I also have vlan devices with dots "." on: (e.g. "vlan01.48")
DEC2750
OPNsense 23.4.2-amd64
FreeBSD 13.1-RELEASE-p8
OpenSSL 1.1.1v 1 Aug 2023
I use suricata there too and it loads without errors.

@AdSchellevis
Copy link
Member

Can you compare the netmap section of the suricata yaml on both ends? It sounds odd that one works and one doesn't

@Monviech
Copy link
Member Author

Monviech commented Oct 11, 2023

I have compared the suricata.yaml files of 23.7.5 to 23.4.2 and the only differences I can see are a few spelling corrections (line 787, 902-903, 1311-1312) and that HOME_NET on (line 17) has been changed.

EDIT: Also tried reverting from Suricata 6.0.14 to Suricata 6.0.13, same behavior.
Going to look if I find something else to explain it.

EDIT2: I also tested it on a new VM in Hyper-V with 23.7.5 instead of hardware, it's the same behavior.

I'll try to see if I can debug more of what's happening, but I can't promise anything. :)

EDIT3: I reverted the VM to 23.7.1 and its not working there either. I'm going to install a 23.1 VM next.

@fichtner fichtner added the support Community support label Oct 11, 2023
@fichtner
Copy link
Member

Create all vlans interfaces without "." Though then the vlan "Device" helptext and input validation shouldn't allow "."
Changing the device name from "vlan0.301" to "vlan0301" makes suricata start.

Looking at it from a parser perspective vlan0.301 is a valid name, but if the parsing goes wrong it probably sees vlan0, which is because it scans for a number but doesn't know that interface names can contain dots and colons and underscores... I'm suspecting libnetmap in src.git causes this. It should be easy to trace and fix.

@fichtner
Copy link
Member

fichtner commented Oct 11, 2023

(you can actually test that theory by creating a vlan0 on the command line to see if the error goes away -- or the error changes to something else which points to the same conclusion)

@Monviech
Copy link
Member Author

Monviech commented Oct 11, 2023

@fichtner
Thank you very much for your support time.

These tests are done in a VM with OPNsense 23.7.1 (Since I don't want to punish my poor new DEC740 x3):

I think your hint shows into the right direction:

root@OPNsense:~ # ifconfig vlan0 create
ifconfig: interface vlan0 already exists
root@OPNsense:~ # ifconfig vlan0 destroy
ifconfig: interface vlan0 does not exist
root@OPNsense:~ # ifconfig vlan0.2 create vlandev hn0
ifconfig: ambiguous vlan specification
root@OPNsense:~ # ifconfig hn0.1 create vlandev hn0
root@OPNsense:~ # ifconfig | grep -i hn0
hn0: flags=8943<UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST> metric 0 mtu 1500
        vlan: 301 vlanproto: 802.1q vlanpcp: 0 parent interface: hn0
hn0.1: flags=8842<BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        vlan: 1 vlanproto: 802.1q vlanpcp: 0 parent interface: hn0

The syntax seems to be ifconfig <device>.<vlanid> create vlandev <device>
But I can't create something like hn0.1 from the gui since it only allows the name "vlan"
I can't create vlan0 since it already exists, but it's invisible. Can't find it. I can't destroy it either.

But, using VLAN hn0.1 in OPNsense 23.7.X in suricata fails too, even though its parent interface exists.

<171>1 2023-10-11T12:38:29+00:00 OPNsense.localdomain suricata 27000 - [meta sequenceId="11"] [100695] <Error> -- [ERRCODE: SC_ERR_FATAL(171)] - opening devname netmap:hn0.1/R failed: Invalid argument

Version specific tests:

  • OPNsense 23.1.X - Suricata loads all interfaces like vlan0.1 etc...
  • OPNsense 23.7.X - Suricata refuses to load vlan0.1

EDIT: Corrected the version numbers, got confused.

EDIT2:

root@OPNsense:~ # suricata --netmap=vlan0
11/10/2023 -- 13:46:13 - <Info> - Including configuration file installed_rules.yaml.
11/10/2023 -- 13:46:13 - <Info> - Configuration node 'rule-files' redefined.
11/10/2023 -- 13:46:13 - <Info> - Including configuration file custom.yaml.
root@OPNsense:~ # suricata --netmap=vlan0.301
11/10/2023 -- 13:46:22 - <Info> - Including configuration file installed_rules.yaml.
11/10/2023 -- 13:46:22 - <Info> - Configuration node 'rule-files' redefined.
11/10/2023 -- 13:46:22 - <Info> - Including configuration file custom.yaml.
netmap:vlan0.301/R: invalid port name 'vlan0.301'

@fichtner
Copy link
Member

fichtner commented Oct 11, 2023

root@OPNsense:~ # ifconfig vlan0.2 create vlandev hn0
ifconfig: ambiguous vlan specification

That sort of thing happens when you make ifconfig aware of special meanings for ".", but discarding the reality that it could be used anywhere. Not a huge fan, but it is what it is.

OPNsense 23.1 - Suricata loads all interfaces like vlan0.1 etc...
Opnsense 27.1 - Suricata refuses to load vlan0.1

It still seems that the most likely change is the NETMAP v14 API change in Suricata which makes Suricata use libnetmap from src.git which fails here for other reasons.. maybe it ends up having issues with ".1/R" as opposed to "/R" which would make sense for it.

Let's look at the source code real quick:

https://github.com/opnsense/src/blob/08aa869336f6a7c19d5bbcaa61269a3f21ca1993/lib/libnetmap/nmreq.c#L53L63

This could be it... it tries to find 0-9a-zA-Z and _ but not . and : which are both valid to my knowledge.

This was just by eye, I haven't run this at all just traced suricata's nmport_prepare() call via grep.

Cheers,
Franco

@Monviech
Copy link
Member Author

Thank you, I'm trying to rebuild libnetmap.a after changing the nm_is_identifier() function to also allow dots. I'm curious what happens. ^^

@fichtner
Copy link
Member

.a is the static file, .so is the one that is probably used

@fichtner
Copy link
Member

PS: make clean && make all && make install should probably work.. you can load the source tree with opnsense-code src and go to it via cd /usr/src/lib/libnetmap

@Monviech
Copy link
Member Author

Monviech commented Oct 11, 2023

/usr/src/lib/libnetmap/nmreq.c

Added that '.' are valid too.

nm_is_identifier(const char *s, const char *e)
{
        for (; s != e; s++) {
                if (!isalnum(*s) && *s != '_' && *s != '.') {
                        return 0;
                }
        }

        return 1;
}
root@OPNsense:/usr/src/lib/libnetmap # suricata --netmap=vlan0.301
11/10/2023 -- 15:36:39 - <Info> - Including configuration file installed_rules.yaml.
11/10/2023 -- 15:36:39 - <Info> - Configuration node 'rule-files' redefined.
11/10/2023 -- 15:36:39 - <Info> - Including configuration file custom.yaml.
<173>1 2023-10-11T15:36:39+00:00 OPNsense.localdomain suricata 27337 - [meta sequenceId="11"] [100645] <Notice> -- all 1 packet processing threads, 4 management threads initialized, engine started.

It works, your guess was spot on Franco. Thank you for your help. How to rebuild certain parts in the opnsense this quickly will help me in the future.

@fichtner
Copy link
Member

Happy to help. Just a lucky guess. You did all the hard work :) I'll pick this up for 23.7.7 when we will update the kernel anyway. Moving to src repo.

@fichtner fichtner transferred this issue from opnsense/core Oct 11, 2023
@fichtner fichtner added upstream Third party issue and removed support Community support labels Oct 11, 2023
@fichtner fichtner self-assigned this Oct 11, 2023
@Monviech
Copy link
Member Author

Great, I'm always happy when my work here is valued. :)

@Monviech
Copy link
Member Author

I would like to know if there are plans to implement it in the current business release 23.10 after it has been picked up in the community release. Right now I have to wait with the update because I need the suricata to continue to work. (I don't want to compile it on production firewalls).

Thank you for your hard work, the business release looks great and I'm eager to try it out soon. :)

@fichtner
Copy link
Member

I want to add it to 23.7.7. Fixing the business edition is a problem, but I can provide a snapshot of the 23.7.7 base/kernel on the business mirror so you can run it in production. Sounds good?

@Monviech
Copy link
Member Author

Yes, thank you a lot. It's great that you do this (even though I'm probably a really small edge case who created the VLANs with dots xD)

@fichtner
Copy link
Member

@Monviech while adding "." may suffice in our use case the whole thing seems to be to prevent characters from being used that are (in theory perfectly valid) so I've gone around and only excluded the characters that libnetmap treats differently: f79d10f

Upstream may opt for a different change but I've found no restriction on device names whatsoever, even a space character works but obviously leads to a lot of trouble everywhere ;)

Can you try the patch? Want to build yourself or try a test build?

Opening this again, automation is good but final confirmation is better.

Cheers,
Franco

@fichtner fichtner reopened this Oct 19, 2023
@Monviech
Copy link
Member Author

Monviech commented Oct 19, 2023

Yeah I will test this on my DEC at home later and give you feedback. I will build the patch myself.

@fichtner
Copy link
Member

In any case I've set up a test base/kernel for the community edition:

# opnsense -zbkr 23.7.4-next

Cheers,
Franco

@Monviech
Copy link
Member Author

I have built the patch and suricata starts with no hiccups. Good job ^^

@fichtner
Copy link
Member

Let’s ship it then. I will try to do a review over at FreeBSD. Thanks for the help!

@fichtner
Copy link
Member

@Monviech as promised this now works from the business mirror when you're on 23.10:

# opnsense-update -zkbr 23.7.7

23.7.7 isn't out until tomorrow but the kernel is already done and won't change. It's probably also the kernel that 23.10.1 would use anyway.

Cheers,
Franco

@Monviech
Copy link
Member Author

@fichtner That's awesome, thanks for the great support.

@Monviech
Copy link
Member Author

Monviech commented Oct 26, 2023

Thanks, worked great without hiccups on a VM and a hardware. Only side effect is that the firewalls now think that there's an update still, even though the current kernel version is newer than the new version that's offered. But I expected something like this, it doesn't really matter. Just thought I'd share my experience. 👍

Package name | Current version | New version | Required action | Repository
-- | -- | -- | -- | --
base | 23.7.7 | 23.7.4 | upgrade | OPNsense
kernel | 23.7.7 | 23.7.4 | upgrade | OPNsense

@fichtner
Copy link
Member

fichtner commented Oct 26, 2023

Yep it wants to go back to the actual kernel/base belonging to 23.10 (that’s 23.7.4 indeed).

You can lock base/kernel but once you want it to update you don’t want to forget to unlock again, otherwise the version sticks until the 24.4 upgrade is executed which unlocks everything.

fichtner added a commit that referenced this issue Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Third party issue
Development

No branches or pull requests

3 participants