Bugfix 4700, 4269: Fix set and commit-time checks of ethernet address values

Changed the the set-time and commit-time check of ethernet interface address
values.  These checks need to prevent configuring both DHCP and static
IPv4 addresses on the same interfac.  The previous checks were comparing
against the running configuration tree, not the proposed config tree.  Now
the set-time check is purely a syntax check, and the commit-time check
only checks for both DHCP and static IPv4 addresses in the proposed config.
The system now allows DHCP and static IPv6 addresses to be configured on
the same interface.

2 files changed, 63 insertions, 19 deletions

diff --git a/scripts/vyatta-interfaces.pl b/scripts/vyatta-interfaces.pl
index 567e3b7..ac690c5 100755
--- a/scripts/vyatta-interfaces.pl
+++ b/scripts/vyatta-interfaces.pl
@@ -47,7 +47,7 @@ use warnings;
 
 my $dhcp_daemon = '/sbin/dhclient';
 
-my ($eth_update, $eth_delete, $addr, $dev, $mac, $mac_update, $op_dhclient);
+my ($eth_update, $eth_delete, $addr_set, @addr_commit, $dev, $mac, $mac_update, $op_dhclient);
 my ($check_name, $show_names, $intf_cli_path, $vif_name, $warn_name);
 my ($check_up, $show_path);
 my @speed_duplex;
@@ -59,7 +59,8 @@ Usage: $0 --dev=<interface> --check=<type>
        $0 --dev=<interface> --valid-mac=<aa:aa:aa:aa:aa:aa>
        $0 --dev=<interface> --eth-addr-update=<aa:aa:aa:aa:aa:aa>
        $0 --dev=<interface> --eth-addr-delete=<aa:aa:aa:aa:aa:aa>
-       $0 --dev=<interface> --valid-addr={<a.b.c.d>|dhcp}
+       $0 --dev=<interface> --valid-addr-set={<a.b.c.d>|dhcp}
+       $0 --dev=<interface> --valid-addr-commit={addr1 addr2 ...}
        $0 --dev=<interface> --speed-duplex=speed,duplex
        $0 --dev=<interface> --path
        $0 --dev=<interface> --isup
@@ -70,7 +71,8 @@ EOF
 
 GetOptions("eth-addr-update=s" => \$eth_update,
 	   "eth-addr-delete=s" => \$eth_delete,
-	   "valid-addr=s"      => \$addr,
+	   "valid-addr-set=s"  => \$addr_set,
+	   "valid-addr-commit=s{,}" => \@addr_commit,
            "dev=s"             => \$dev,
 	   "valid-mac=s"       => \$mac,
 	   "set-mac=s"	       => \$mac_update,
@@ -86,7 +88,8 @@ GetOptions("eth-addr-update=s" => \$eth_update,
 
 update_eth_addrs($eth_update, $dev)	if ($eth_update);
 delete_eth_addrs($eth_delete, $dev)	if ($eth_delete);
-is_valid_addr($addr, $dev)		if ($addr);
+is_valid_addr_set($addr_set, $dev)	if ($addr_set);
+is_valid_addr_commit($dev, @addr_commit) if (@addr_commit);
 is_valid_mac($mac, $dev)		if ($mac);
 update_mac($mac_update, $dev)		if ($mac_update);
 op_dhcp_command($op_dhclient, $dev)	if ($op_dhclient);
@@ -339,7 +342,13 @@ sub is_valid_mac {
     exit 0;
 }
 
-sub is_valid_addr {
+# Validate an address parameter at the time the user enters it via
+# a "set" command.  This validates the parameter for syntax only.
+# It does not validate it in combination with other parameters.
+# Valid values are:  "dhcp", <ipv4-address>/<prefix-len>, or 
+# <ipv6-address>/<prefix-len>
+#
+sub is_valid_addr_set {
     my ($addr_net, $intf) = @_;
 
     if ($addr_net eq "dhcp") { 
@@ -347,14 +356,6 @@ sub is_valid_addr {
 	    print "Error: can't use dhcp client on loopback interface\n";
 	    exit 1;
 	}
-	if (is_dhcp_enabled($intf)) {
-	    print "Error: dhcp already configured for $intf\n";
-	    exit 1;
-	}
-	if (is_address_enabled($intf)) {
-	    print "Error: remove static addresses before enabling dhcp for $intf\n";
-	    exit 1;
-	}
 	exit 0; 
     }
 
@@ -393,11 +394,6 @@ sub is_valid_addr {
        }
     }
 
-    if (is_dhcp_enabled($intf)) {
-	print "Error: remove dhcp before adding static addresses for $intf\n";
-	exit 1;
-    }
-
     if (is_ip_duplicate($intf, $addr_net)) {
 	print "Error: duplicate address/prefix [$addr_net]\n";
 	exit 1;
@@ -417,6 +413,39 @@ sub is_valid_addr {
     exit 1;
 }
 
+# Validate the set of address values configured on an interface at commit
+# time.  Syntax of address values is checked at set time, so is not
+# checked here.  Instead, we check that full set of address address
+# values are consistent.  The only rule that we enforce here is that
+# one may not configure an interface with both a DHCP address and a static
+# IPv4 address.
+#
+sub is_valid_addr_commit {
+    my ($intf, @addrs) = @_;
+
+    my $static_v4 = 0;
+    my $dhcp = 0;
+
+    foreach my $addr (@addrs) {
+	if ($addr eq "dhcp") {
+	    $dhcp = 1;
+	} else {
+	    my $version = is_ip_v4_or_v6($addr);
+	    if ($version == 4) {
+		$static_v4 = 1;
+	    }
+	}
+    }
+
+    if ($static_v4 == 1 && $dhcp == 1) {
+	printf("Error configuring interface $intf: Can't configure static\n");
+	printf("IPv4 address and DHCP on the same interface.\n");
+	exit 1;
+    }
+
+    exit 0;
+}
+
 sub op_dhcp_command {
     my ($op_command, $intf) = @_;
 
diff --git a/templates/interfaces/ethernet/node.tag/address/node.def b/templates/interfaces/ethernet/node.tag/address/node.def
index dd4d5fa..db87ff0 100644
--- a/templates/interfaces/ethernet/node.tag/address/node.def
+++ b/templates/interfaces/ethernet/node.tag/address/node.def
@@ -1,11 +1,26 @@
 multi:
+
 type: txt
+
 help: Set an IP address for this interface
-syntax:expression: exec "/opt/vyatta/sbin/vyatta-interfaces.pl --valid-addr $VAR(@) --dev $VAR(../@)"\
+
+# Syntax check at "set" time.  Give curent address value to script
+# so that it can perform syntax check.
+#
+syntax:expression: exec "/opt/vyatta/sbin/vyatta-interfaces.pl --valid-addr-set $VAR(@) --dev $VAR(../@)"\
      ; "Invalid IP address/prefix [$VAR(@)] for interface $VAR(../@)"
+
+# Syntax check at "commit" time.  Pass all address values to script so that
+# it can perform consistency check.
+#
+commit:expression: exec "/opt/vyatta/sbin/vyatta-interfaces.pl --valid-addr-commit $VAR(@@) --dev $VAR(../@)"
+
 create:sudo /opt/vyatta/sbin/vyatta-interfaces.pl --eth-addr-update $VAR(@) --dev $VAR(../@)
+
 delete:sudo /opt/vyatta/sbin/vyatta-interfaces.pl --eth-addr-delete $VAR(@) --dev $VAR(../@)
+
 allowed: echo "dhcp <>"
+
 comp_help:Possible completions:
   <x.x.x.x/x>		Set the IP address and prefix length
   <h:h:h:h:h:h:h:h/x>   Set the IPv6 address and prefix length