ncm-network: Restrictions on device naming should match kernel

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-<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.}

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-<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
@@ -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;
     };

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");