From 59d833c5749ac69b871627332841f4ff890edfb9 Mon Sep 17 00:00:00 2001 From: James Adams Date: Thu, 5 Dec 2024 20:38:39 +0000 Subject: [PATCH] ncm-network: Restrictions on device naming should match kernel That is: - Maximum 15 characters (16 including null) - No whitespace - No forward-slashes - No colons (but they _are_ allowed in filenames in order to label alias IPs) While we're at it, make the regexp in the module absolute, as we're actually matching filenames there. Similar validation should also happen in the schema as only throwing errors at runtime is _really_ unfriendly. --- .../pan/components/network/types/network.pan | 11 ++++- ncm-network/src/main/perl/network.pm | 44 +++++++++--------- ncm-network/src/test/perl/valid_interfaces.t | 45 +++++++++++-------- 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/ncm-network/src/main/pan/components/network/types/network.pan b/ncm-network/src/main/pan/components/network/types/network.pan index cfda10b86c..268ef2d8d1 100644 --- a/ncm-network/src/main/pan/components/network/types/network.pan +++ b/ncm-network/src/main/pan/components/network/types/network.pan @@ -53,8 +53,15 @@ type structure_network = { "gatewaydev" ? valid_interface @{Per interface network settings. These values are used to generate the /etc/sysconfig/network-scripts/ifcfg- files - when using ncm-network.} - "interfaces" : network_interface{} + when using ncm-network. + Interface names must be no more than 15 characters in and cannot contain whitespace, ".", "/" or ":". + } + "interfaces" : network_interface{} with { + foreach (i; _; SELF) { + match(i, '^[^\s\/:]{1,15}$') || error('Device name "%s" is invalid', i); + }; + true; + } "nameserver" ? type_ip[] "nisdomain" ? string(1..64) with match(SELF, '^\S+$') @{Setting nozeroconf to true stops an interface from being assigned an automatic address in the 169.254.0.0 subnet.} diff --git a/ncm-network/src/main/perl/network.pm b/ncm-network/src/main/perl/network.pm index 7f936210be..770624e6bc 100755 --- a/ncm-network/src/main/perl/network.pm +++ b/ncm-network/src/main/perl/network.pm @@ -95,6 +95,7 @@ use CAF::FileEditor; use CAF::FileWriter; use CAF::Path 17.7.0; use NetAddr::IP; +use File::Basename; use POSIX qw(WIFEXITED WEXITSTATUS); use Readonly; @@ -153,25 +154,16 @@ Readonly my $HARDWARE_PATH => '/hardware/cards/nic'; # Regexp for the supported ifcfg- devices. # $1 must match the device name +# Note that device names cannot contain ":", but the filenames generated may use ":" to delimit named alias IPs Readonly my $DEVICE_REGEXP => qr{ + ^ + (?:ifcfg|route6?|rule6?) - # separator from e.g. ifcfg or route ( # start whole match group $1 ( # start devicename group $2 - (?: - eth|seth|em| - bond|br|ovirtmgmt| - vlan|usb|vxlan| - ib| - tun| - p\d+p| - en(?: - o(?:\d+d)?| # onboard - (?:p\d+)?s(?:\d+f)?(?:\d+d)? # [pci]slot[function][device] - )(?:\d+np)? # [partition] - )\d+| # mandatory numbering - enx[[:xdigit:]]{12} # enx MAC address + [^\s_:.]+ ) - (?:_(\w+))? # opional suffix group $3 + (?:_(\w+))? # optional suffix group $3 (?:\.\d+)? # optional VLAN (?::\w+)? # optional alias ) # end whole matching group @@ -222,26 +214,34 @@ sub _is_executable return -x $fn; } -# Given the configuration ifcfg/route[6]/rule[6] filename, +# Given the configuration ifcfg/route[6]/rule[6] file, # Determine if this is a valid interface for ncm-network to manage, # Return arrayref tuple [interface name, ifdown/ifup name] when valid, # undef otherwise. sub is_valid_interface { - my ($self, $filename) = @_; + my ($self, $filepath) = @_; + + my $filename = basename($filepath); # Very primitive, based on regex only - # Not even the full filename (eg ifcfg) or anything if ($filename =~ m/$DEVICE_REGEXP/) { my $ifupdownname = $1; - my $name = $2; + my $devicename = $2; + + if (length($devicename) >= 16) { + return; + }; + my $suffix = $3; if ($suffix && $suffix =~ m/^\d+$/) { - $name .= "_$suffix"; - $self->verbose("Found digit-only suffix $suffix for device $name ($filename), ", - "added it to the interface name"); + $devicename .= "_$suffix"; + $self->verbose( + "Found digit-only suffix $suffix for device $devicename ($filename), added it to the interface name" + ); } - return [$name, $ifupdownname]; + + return [$devicename, $ifupdownname]; } else { return; }; diff --git a/ncm-network/src/test/perl/valid_interfaces.t b/ncm-network/src/test/perl/valid_interfaces.t index 231192c974..70d4c9a800 100644 --- a/ncm-network/src/test/perl/valid_interfaces.t +++ b/ncm-network/src/test/perl/valid_interfaces.t @@ -20,27 +20,36 @@ my @valid_ifs = qw(eth0 seth1 em2 em2_2 enxAABBCCDDEEFF); foreach my $valid (@valid_ifs) { - foreach my $type (qw(ifcfg route)) { - is_deeply($cmp->is_valid_interface("$basedir/$type-$valid"), [$valid, $valid], - "valid interface $valid from $type"); - is_deeply($cmp->is_valid_interface("$basedir/$type-$valid.123"), [$valid, "$valid.123"], - "valid interface $valid from $type with vlan"); - is_deeply($cmp->is_valid_interface("$basedir/$type-$valid:alias"), [$valid, "$valid:alias"], - "valid interface $valid from $type with alias"); - is_deeply($cmp->is_valid_interface("$basedir/$type-$valid.456:myalias"), [$valid, "$valid.456:myalias"], - "valid interface $valid from $type with vlan and alias"); - is_deeply($cmp->is_valid_interface("$basedir/$type-${valid}_whatever.456:myalias"), - [$valid =~ m/^(.*)_\d+$/ ? $1 : $valid, "${valid}_whatever.456:myalias"], - "valid interface $valid from $type with suffix, vlan and alias"); + foreach my $type (qw(ifcfg route route6)) { + is_deeply( + $cmp->is_valid_interface("$basedir/$type-$valid"), [$valid, $valid], + "valid interface $valid from $type" + ); + is_deeply( + $cmp->is_valid_interface("$basedir/$type-$valid.123"), [$valid, "$valid.123"], + "valid interface $valid from $type with vlan" + ); + is_deeply( + $cmp->is_valid_interface("$basedir/$type-$valid:alias"), [$valid, "$valid:alias"], + "valid interface $valid from $type with alias" + ); + is_deeply( + $cmp->is_valid_interface("$basedir/$type-$valid.456:myalias"), [$valid, "$valid.456:myalias"], + "valid interface $valid from $type with vlan and alias" + ); + is_deeply( + $cmp->is_valid_interface("$basedir/$type-${valid}_whatever.456:myalias"), + [$valid =~ m/^(.*)_\d+$/ ? $1 : $valid, "${valid}_whatever.456:myalias"], + "valid interface $valid from $type with suffix, vlan and alias" + ); }; }; -my @invalid_ifs = ('madeup', # arbitrary name - 'eth', # no number - 'enop1', # onboard with pci - 'enp2', # pci without slot - 'enxAABBCCDDEEF', # too short MAC - 'enxAABBCCDDEEFFF', # too long mac +my @invalid_ifs = ( + 'contains/slash', + 'too-many-characters', + 'multiple::colons', + 'space in_name', ); foreach my $invalid (@invalid_ifs) { ok(!defined($cmp->is_valid_interface("$basedir/ifcfg-$invalid")), "invalid interface $invalid");