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.

Author: jrha

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