Skip to content

Commit

Permalink
ncm-network: Restrictions on device naming should match kernel
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jrha committed Dec 6, 2024
1 parent 124f057 commit 59d833c
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 42 deletions.
11 changes: 9 additions & 2 deletions ncm-network/src/main/pan/components/network/types/network.pan
Original file line number Diff line number Diff line change
Expand Up @@ -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-<interface> 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.}
Expand Down
44 changes: 22 additions & 22 deletions ncm-network/src/main/perl/network.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -153,25 +154,16 @@ Readonly my $HARDWARE_PATH => '/hardware/cards/nic';

# Regexp for the supported ifcfg-<device> 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
Expand Down Expand Up @@ -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;
};
Expand Down
45 changes: 27 additions & 18 deletions ncm-network/src/test/perl/valid_interfaces.t
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 59d833c

Please sign in to comment.