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

updated HariSekhonUtils.pm allow vlans in isInterface #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

samsk
Copy link

@samsk samsk commented Aug 23, 2024

This PR modifies isInterface() to allow any alfanumeric interface name with vlan suffix (ie. eth0.100).

# TODO: consider checking if the interface actually exists on the system
$interface =~ /^((?:em|eth|bond|lo|docker)\d+|lo|veth[A-Fa-f0-9]+)$/ or return;
Copy link
Owner

@HariSekhon HariSekhon Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing this line checking the format on the interface name needs some kind of reinstating to avoid weakening the check.

Unit tests need to be extended a bit to test failure of a fake interface called hari0 instead of eth0 for example, which fails on this branch, which highlights the weakening without the regex validation.

Also, the regex test should be done to untaint the returned interface. The code base runs entirely in taint security mode and relies on known good specific regexes to untaint user inputs. This patch inadvertently removes the untainting.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the response.
I've dared to removed the regex check, because its IMHO not possible to craft a regex to match interface names other than alnum - ie. the former regex did not worked for brX, tailscaleX, tunX, wlanX, wmnetX just to name a few. btw. interface hari0 is also valid interface name ;-)

$ ip link add link eth0 name hari0 type vlan id 101
hari0: flags=4098<BROADCAST,MULTICAST>  mtu 1500
        ether 96:00:03:6c:82:23  txqueuelen 1000  (Ethernet)
        RX packets 0  bytes 0 (0.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 0  bytes 0 (0.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

Also, the isInterface() was not used till now anywhere except of validate_interface(), that was mentioned only in the comment of check_linux_interface.pl (https://github.com/HariSekhon/Nagios-Plugins/blob/master/check_linux_interface.pl#L61).

The untainting is done in isAlnum and isInt (as it was till now, directly in check_linux_interface.pl), but I can improve it more if you have some suggestion how. One possibility can be, to close this todo
# TODO: consider checking if the interface actually exists on the system
and check interface name against /proc/net/dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants