Merge pull request #2770 from webmin/dev/net-module-fixes

Fix network module edge cases

net/edit_aifc.cgi

@@ -128,16 +128,21 @@ else {
 print &ui_table_row($text{'ifcs_mtu'}, $mtufield);
 
 # Current status
-if (!$access{'up'}) {
+if (($in{'new'} && $in{'virtual'}) || ($a && $a->{'virtual'} ne "")) {
+	# Virtual aliases are IP addresses, not links with independent status.
+	print &ui_hidden("up", 1);
+	}
+elsif (!$access{'up'}) {
 	# Cannot edit
 	$upfield = !$a ? $text{'ifcs_up'} :
 		   $a->{'up'} ? $text{'ifcs_up'} : $text{'ifcs_down'};
+	print &ui_table_row($text{'ifcs_status'}, $upfield);
 	}
 else {
 	$upfield = &ui_radio("up", $in{'new'} || $a->{'up'} ? 1 : 0,
 			[ [ 1, $text{'ifcs_up'} ], [ 0, $text{'ifcs_down'} ] ]);
+	print &ui_table_row($text{'ifcs_status'}, $upfield);
 	}
-print &ui_table_row($text{'ifcs_status'}, $upfield);
 
 # Hardware address, if non-virtual
 if ((!$a && $in{'virtual'} eq "") ||

net/lang/en

@@ -79,6 +79,7 @@ aifc_err2=Failed to save interface
 aifc_evirt=Missing or invalid virtual interface number
 aifc_evirtmin=Virtual interface number must be at least $1
 aifc_evirtdup=The virtual interface $1 already exists
+aifc_evirtdown=Virtual interfaces cannot be created with down status
 aifc_edup=The interface $1 already exists
 aifc_ename=Missing or invalid interface name
 aifc_eip='$1' is not a valid IP address

net/linux-lib.pl

@@ -252,6 +252,12 @@ if(($a->{'vlan'} == 1) && !(($gconfig{'os_type'} eq 'debian-linux') && ($gconfig
 	if ($?) { &error($vonconfigout); }
 	}
 
+if (&has_command("ip") && $a->{'virtual'} ne '' && !$a->{'up'}) {
+	# Linux virtual aliases are addresses, not independent links.
+	&deactivate_interface($old) if ($old && $old->{'address'});
+	return;
+	}
+
 if (!&has_command("ifconfig") && &has_command("ip")) {
 	# For a real interface, activate or de-activate the link
 	if ($a->{'virtual'} eq '' && $a->{'up'} && (!$old || !$old->{'up'})) {
@@ -942,7 +948,8 @@ close(SWITCH);
 &open_tempfile(SWITCH, ">/etc/nsswitch.conf");
 foreach (@switch) {
 	if (/^\s*hosts:\s+/) {
-		&print_tempfile(SWITCH, "hosts:\t$conf->{'order'}\n");
+		&print_tempfile(SWITCH,
+			&linux_nsswitch_hosts_line($_, $conf->{'order'}));
 		}
 	else {
 		&print_tempfile(SWITCH, $_);
@@ -1007,4 +1014,20 @@ else {
 	}
 }
 
+# linux_nsswitch_hosts_line(line, order)
+# Returns an updated nsswitch hosts line preserving existing spacing
+sub linux_nsswitch_hosts_line
+{
+my ($line, $order) = @_;
+$line =~ s/\r?\n$//;
+my $comment = "";
+if ($line =~ s/(\s+#.*)$//) {
+	# Keep inline comments while replacing only the lookup order.
+	$comment = $1;
+	}
+return $1.$2.$order.$comment."\n"
+	if ($line =~ /^(\s*hosts:)(\s+)\S/);
+return "hosts:\t$order$comment\n";
+}
+
 1;

net/net-lib.pl

@@ -52,18 +52,34 @@ local $line="";
 &open_readfile(HOSTS, $config{'hosts_file'});
 while($line=<HOSTS>) {
 	local $comment = 0;
+	local $comment_prefix = "";
+	local $leading = "";
+	local $inline_comment = "";
 	$line =~ s/\r|\n//g;
-	if ($line =~ s/^\s*#+\s*//) {
+	if ($line =~ s/^(\s*#+\s*)//) {
 		$comment = 1;
+		$comment_prefix = $1;
+		}
+	elsif ($line =~ s/^(\s+)//) {
+		# Preserve indentation if this file uses it for host rows.
+		$leading = $1;
+		}
+	if ($line =~ s/(\s+#.*)$//) {
+		# Keep inline comments attached to edited host rows.
+		$inline_comment = $1;
 		}
-	$line =~ s/#.*$//g;
 	$line =~ s/\s+$//g;
+	local @seps = &host_line_separators($line);
 	local @f = split(/\s+/, $line);
 	local $ipaddr = shift(@f);
 	if (check_ipaddress_any($ipaddr)) {
 		push(@rv, { 'address' => $ipaddr,
 			    'hosts' => [ @f ],
 			    'active' => !$comment,
+			    'comment_prefix' => $comment_prefix,
+			    'leading' => $leading,
+			    'comment' => $inline_comment,
+			    'seps' => \@seps,
 			    'line', $lnum,
 			    'index', scalar(@rv) });
 		}
@@ -73,13 +89,35 @@ close(HOSTS);
 return @rv;
 }
 
+# host_line_separators(line)
+# Returns the field separators from a parsed /etc/hosts line
+sub host_line_separators
+{
+local ($line) = @_;
+local @seps;
+while($line =~ /\S+(\s+)/g) {
+	push(@seps, $1);
+	}
+return @seps;
+}
+
 # make_host_line(&host)
 # Internal function to return a line for the hosts file
 sub make_host_line
 {
 local ($host) = @_;
-return ($host->{'active'} ? "" : "# ").
-       $host->{'address'}."\t".join(" ",@{$host->{'hosts'}})."\n";
+local $prefix = $host->{'active'} ? $host->{'leading'} || "" :
+		$host->{'comment_prefix'} || "# ";
+local @seps = @{$host->{'seps'} || [ ]};
+local @hosts = @{$host->{'hosts'} || [ ]};
+local $line = $prefix.$host->{'address'};
+for(local $i=0; $i<@hosts; $i++) {
+	# Reuse original spacing by field position, then fall back to defaults.
+	local $sep = $seps[$i] || ($i == 0 ? "\t" : " ");
+	$line .= $sep.$hosts[$i];
+	}
+$line .= $host->{'comment'} if ($host->{'comment'});
+return $line."\n";
 }
 
 # create_host(&host)

net/save_aifc.cgi

@@ -49,14 +49,14 @@ else {
 		# also creating a virtual interface
 		foreach $ea (@acts) {
 			if ($ea->{'name'} eq $1 &&
-			    $ea->{'virtual'} eq $3) {
+			    $ea->{'virtual'} eq $4) {
 				&error(&text('aifc_evirtdup', &html_escape($in{'name'})));
 				}
 			}
-		$3 >= $min_virtual_number ||
+		$4 >= $min_virtual_number ||
 			&error(&text('aifc_evirtmin', &html_escape($min_virtual_number)));
 		$a->{'name'} = $1;
-		$a->{'virtual'} = $3;
+		$a->{'virtual'} = $4;
 		$a->{'fullname'} = $a->{'name'}.":".$a->{'virtual'};
 		&can_create_iface() || &error($text{'ifcs_ecannot'});
 		&can_iface($a) || &error($text{'ifcs_ecannot'});
@@ -131,7 +131,14 @@ else {
 		}
 
 	# Save active flag
-	if (!$access{'up'}) {
+	if ($a->{'virtual'} ne "") {
+		# Virtual aliases are addresses only, so present means up.
+		if ($access{'up'} && defined($in{'up'}) && !$in{'up'}) {
+			&error($text{'aifc_evirtdown'});
+			}
+		$a->{'up'} = 1;
+		}
+	elsif (!$access{'up'}) {
 		$a->{'up'} = $in{'new'} ? 1 : $olda->{'up'};
 		}
 	elsif ($in{'up'}) {
@@ -175,7 +182,8 @@ else {
 		delete($a->{'netmask6'});
 		}
 
-	if (!$in{'ether_def'} && $a->{'virtual'} eq "" &&
+	if (defined($in{'ether'}) && $in{'ether'} ne '' &&
+	    !$in{'ether_def'} && $a->{'virtual'} eq "" &&
 	    &iface_hardware($a->{'name'})) {
 		$in{'ether'} =~ /^[A-Fa-f0-9:]+$/ ||
 			&error(&text('aifc_ehard', &html_escape($in{'ether'})));

net/save_bifc.cgi

@@ -59,19 +59,19 @@ else {
 		&can_create_iface() || &error($text{'ifcs_ecannot'});
 		&can_iface($b) || &error($text{'ifcs_ecannot'});
 		}
-	elsif ($in{'name'} =~ /^([a-z]+\d*(s\d*)?(\.\d+)?):(\d+)$/ ||
-	       $in{'name'} =~ /^(en[0-9a-z]+(s\d*)?(\.\d+)?):(\d+)$/) {
+	elsif ($in{'name'} =~ /^((?:[a-z]+\d*(?:s\d*)?(?:\.\d+)?)|(?:en[0-9a-z]+(?:s\d*)?(?:\.\d+)?)):(\d+)$/) {
 		# also creating a virtual interface
+		local ($vname, $vnum) = ($1, $2);
 		foreach $eb (@boot) {
-			if ($eb->{'name'} eq $2 &&
-			    $eb->{'virtual'} eq $4) {
+			if ($eb->{'name'} eq $vname &&
+			    $eb->{'virtual'} eq $vnum) {
 				&error(&text('bifc_evirtdup', &html_escape($in{'name'})));
 				}
 			}
-		$4 >= $min_virtual_number ||
+		$vnum >= $min_virtual_number ||
 			&error(&text('aifc_evirtmin', &html_escape($min_virtual_number)));
-		$b->{'name'} = $1;
-		$b->{'virtual'} = $4;
+		$b->{'name'} = $vname;
+		$b->{'virtual'} = $vnum;
 		$b->{'fullname'} = $b->{'name'}.":".$b->{'virtual'};
 		}
 	elsif ($in{'bridge'}) {

net/save_dns.cgi

@@ -124,6 +124,19 @@ if (&foreign_installed("postfix") && $in{'hostname'} ne $old_hostname) {
 		&postfix::set_current_value("mydestination",
 					    join(", ", @mydests));
 		}
+	$old_domain = $old_hostname =~ /^[^\.]+\.(.*)$/ ? $1 :
+		      $old_fqdn =~ /^[^\.]+\.(.*)$/ ? $1 : undef;
+	$new_domain = $in{'hostname'} =~ /^[^\.]+\.(.*)$/ ? $1 :
+		      $new_fqdn =~ /^[^\.]+\.(.*)$/ ? $1 : undef;
+	if ($old_domain && $new_domain &&
+	    lc($old_domain) ne lc($new_domain)) {
+		$idx = &indexoflc("localhost.$old_domain", @mydests);
+		if ($idx >= 0) {
+			$mydests[$idx] = "localhost.$new_domain";
+			&postfix::set_current_value("mydestination",
+						    join(", ", @mydests));
+			}
+		}
 
 	# Update postfix myorigin
 	$myorigin = &postfix::get_current_value("myorigin");

net/t/run-tests.t

@@ -122,6 +122,15 @@ do "$root/net/netplan-lib.pl" || die "netplan-lib.pl: $@ $!";
 	$main::netplan_dir = $tmp;
 }
 
+is(main::linux_nsswitch_hosts_line("hosts:          files dns\n",
+				   "files dns"),
+   "hosts:          files dns\n",
+   "Linux DNS save preserves nsswitch hosts spacing");
+is(main::linux_nsswitch_hosts_line("hosts:\tfiles dns # local policy\n",
+				   "files mdns4 dns"),
+   "hosts:\tfiles mdns4 dns # local policy\n",
+   "Linux DNS save preserves nsswitch hosts comments");
+
 my $netplan = "$tmp/50-cloud-init.yaml";
 write_text($netplan, <<'YAML');
 network:
@@ -232,4 +241,56 @@ main::save_interface($nmiface, [ $nmiface ]);
 like(join("\n", @commands), qr/ipv6\.dns/,
      "NetworkManager save_interface writes IPv6 nameservers");
 
+do "$root/net/linux-lib.pl" || die "linux-lib.pl: $@ $!";
+
+@commands = ( );
+{
+	no warnings 'redefine';
+	local *main::has_command = sub {
+		return $_[0] eq "ip" ? "/sbin/ip" : undef;
+		};
+	local *main::active_interfaces = sub {
+		return ( );
+		};
+	main::activate_interface({ 'name' => 'enp0s5',
+				   'fullname' => 'enp0s5:1',
+				   'virtual' => 1,
+				   'address' => '10.211.55.25',
+				   'netmask' => '255.255.255.0',
+				   'address6' => [ ],
+				   'netmask6' => [ ],
+				   'up' => 0 });
+	}
+is_deeply(\@commands, [ ],
+	  "Linux active virtual interface stays absent when created down");
+
+@commands = ( );
+{
+	no warnings 'redefine';
+	local *main::has_command = sub {
+		return $_[0] eq "ip" ? "/sbin/ip" : undef;
+		};
+	local *main::active_interfaces = sub {
+		return ({ 'name' => 'enp0s5',
+			  'fullname' => 'enp0s5:1',
+			  'virtual' => 1,
+			  'address' => '10.211.55.25',
+			  'netmask' => '255.255.255.0',
+			  'address6' => [ ],
+			  'netmask6' => [ ],
+			  'up' => 1 });
+		};
+	main::activate_interface({ 'name' => 'enp0s5',
+				   'fullname' => 'enp0s5:1',
+				   'virtual' => 1,
+				   'address' => '10.211.55.25',
+				   'netmask' => '255.255.255.0',
+				   'address6' => [ ],
+				   'netmask6' => [ ],
+				   'up' => 0 });
+	}
+is_deeply(\@commands,
+	  [ "ip addr del 10\\.211\\.55\\.25\\/24 dev enp0s5 2>&1" ],
+	  "Linux active virtual interface is removed when saved down");
+
 done_testing();