Re: [PATCH 1/2] aliases: Add support to assign aliases to multiple RED interfaces

["Peter Müller" <peter.mueller@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 11628 bytes --]

Acked-by: Peter Müller <peter.mueller(a)ipfire.org>

> This is a little patch which will extend the aliases page to offer an
> interface selection if there are more than one RED interfaces.
> 
> This is a little hack to make configuration easier for users who have
> manually set up more than one RED interface (e.g. for load balancing or
> fail-over) and want to use the UI to configure firewall rules.
> 
> As a little benefit on the side, I had to rewrite setaliases.c to use
> ip(8) instead of ifconfig(8).
> 
> Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
> ---
>  config/cfgroot/network-functions.pl | 20 +++++++++
>  html/cgi-bin/aliases.cgi            | 63 +++++++++++++++++++++++++----
>  langs/en/cgi-bin/en.pl              |  1 +
>  src/misc-progs/setaliases.c         | 38 ++++++++++-------
>  4 files changed, 99 insertions(+), 23 deletions(-)
> 
> diff --git a/config/cfgroot/network-functions.pl b/config/cfgroot/network-functions.pl
> index d50322823..4ac6d8670 100644
> --- a/config/cfgroot/network-functions.pl
> +++ b/config/cfgroot/network-functions.pl
> @@ -332,6 +332,26 @@ sub setup_upstream_proxy() {
>  	}
>  }
>  
> +sub get_red_interfaces() {
> +	my $default = &General::get_red_interface();
> +
> +	my @intfs = (
> +		$default,
> +	);
> +
> +	opendir(INTERFACES, "/sys/class/net");
> +
> +	while (my $intf = readdir(INTERFACES)) {
> +		if ($intf =~ m/^red[0-9]+$/) {
> +			push(@intfs, $intf);
> +		}
> +	}
> +
> +	closedir(INTERFACES);
> +
> +	return &General::uniq(@intfs);
> +}
> +
>  sub list_wireless_interfaces() {
>  	my %interfaces = ();
>  
> diff --git a/html/cgi-bin/aliases.cgi b/html/cgi-bin/aliases.cgi
> index 7b80b3c84..def03ff9b 100644
> --- a/html/cgi-bin/aliases.cgi
> +++ b/html/cgi-bin/aliases.cgi
> @@ -34,6 +34,7 @@ require '/var/ipfire/general-functions.pl';	# replace /var/ipcop with /var/ipcop
>  require "${General::swroot}/lang.pl";
>  require "${General::swroot}/header.pl";
>  require "${General::swroot}/ids-functions.pl";
> +require "${General::swroot}/network-functions.pl";
>  
>  my $configfwdfw		= "${General::swroot}/firewall/config";
>  my $configinput		= "${General::swroot}/firewall/input";
> @@ -52,6 +53,11 @@ undef (@dummy);
>  my $setting = "${General::swroot}/ethernet/settings";
>  our $datafile = "${General::swroot}/ethernet/aliases";
>  
> +# Fetch the name of the main RED interface
> +my $RED_INTERFACE = &General::get_red_interface();
> +
> +# Fetch all RED interfaces
> +my @RED_INTERFACES = &Network::get_red_interfaces();
>  
>  our %settings=();
>  #Settings1
> @@ -61,7 +67,8 @@ our %settings=();
>  $settings{'IP'} = '';
>  $settings{'ENABLED'} = 'off';		# Every check box must be set to off
>  $settings{'NAME'} = '';
> -my @nosaved=('IP','ENABLED','NAME');	# List here ALL setting2 fields. Mandatory
> +$settings{'INTERFACE'} = '';
> +my @nosaved=('IP','ENABLED','NAME','INTERFACE');	# List here ALL setting2 fields. Mandatory
>  
>  $settings{'ACTION'} = '';		# add/edit/remove
>  $settings{'KEY1'} = '';			# point record for ACTION
> @@ -215,10 +222,10 @@ if ($settings{'ACTION'} eq $Lang::tr{'add'}) {
>  	}
>      unless ($errormessage) {
>  	if ($settings{'KEY1'} eq '') { #add or edit ?
> -	    unshift (@current, "$settings{'IP'},$settings{'ENABLED'},$settings{'NAME'}\n");
> +	    unshift (@current, "$settings{'IP'},$settings{'ENABLED'},$settings{'NAME'},$settings{'INTERFACE'}\n");
>  	    &General::log($Lang::tr{'ip alias added'});
>  	} else {
> -	    @current[$settings{'KEY1'}] = "$settings{'IP'},$settings{'ENABLED'},$settings{'NAME'}\n";
> +	    @current[$settings{'KEY1'}] = "$settings{'IP'},$settings{'ENABLED'},$settings{'NAME'},$settings{'INTERFACE'}\n";
>  	    $settings{'KEY1'} = '';       # End edit mode
>  	    &General::log($Lang::tr{'ip alias changed'});
>  	}
> @@ -250,6 +257,7 @@ if ($settings{'ACTION'} eq $Lang::tr{'edit'}) {
>      $settings{'IP'}=$temp[0];			# Prepare the screen for editing
>      $settings{'ENABLED'}=$temp[1];
>      $settings{'NAME'}=$temp[2];
> +    $settings{'INTERFACE'}=$temp[3];
>  }
>  
>  if ($settings{'ACTION'} eq $Lang::tr{'remove'}) {
> @@ -295,6 +303,7 @@ if ($settings{'ACTION'} eq '' ) { # First launch from GUI
>  &Header::openpage($Lang::tr{'external aliases configuration'}, 1, '');
>  &Header::openbigbox('100%', 'left', '', $errormessage);
>  my %checked =();     # Checkbox manipulations
> +my %selected = ();
>  
>  if ($errormessage) {
>      &Header::openbox('100%', 'left', $Lang::tr{'error messages'});
> @@ -320,6 +329,11 @@ END
>  #
>  $checked{'ENABLED'}{'on'} = ($settings{'ENABLED'} eq 'on') ? "checked='checked'" : '' ;
>  
> +$selected{'INTERFACE'} = ();
> +foreach my $intf (@RED_INTERFACES) {
> +	$selected{'INTERFACE'}{$intf} = ($settings{'INTERFACE'} eq $intf) ? "selected" : "";
> +}
> +
>  my $buttontext = $Lang::tr{'add'};
>  if ($settings{'KEY1'} ne '') {
>      $buttontext = $Lang::tr{'update'};
> @@ -329,7 +343,7 @@ if ($settings{'KEY1'} ne '') {
>  }
>  
>  #Edited line number (KEY1) passed until cleared by 'save' or 'remove' or 'new sort order'
> -print <<END
> +print <<END;
>  <form method='post' action='$ENV{'SCRIPT_NAME'}'>
>  <input type='hidden' name='KEY1' value='$settings{'KEY1'}' />
>  <input type='hidden' name='OLDNAME' value='$settings{'NAME'}' />
> @@ -340,6 +354,33 @@ print <<END
>  <td><input type='text' name='NAME' value='$settings{'NAME'}' size='32' /></td>
>  <td class='base' style='text-align:right; color:${Header::colourred};'>$Lang::tr{'alias ip'}:&nbsp;</td>
>  <td><input type='text' name='IP' value='$settings{'IP'}' size='16' /></td>
> +END
> +
> +if (scalar @RED_INTERFACES >= 2) {
> +	print <<END;
> +		<td class='base' style='color:${Header::colourred};'>$Lang::tr{'interface'}:</td>
> +		<td>
> +			<select name="INTERFACE">
> +				<option value="">$Lang::tr{'aliases default interface'}</option>
> +END
> +
> +	# Print an option for each RED interface
> +	foreach my $intf (@RED_INTERFACES) {
> +		# Skip the default one
> +		next if ($RED_INTERFACE eq $intf);
> +
> +		print <<END;
> +				<option value="$intf" $selected{'INTERFACE'}{$intf}>$intf</option>
> +END
> +	}
> +
> +	print <<END;
> +			</select>
> +		</td>
> +END
> +}
> +
> +print <<END;
>  <td class='base' style='text-align:right;'>$Lang::tr{'enabled'}&nbsp;</td>
>  <td><input type='checkbox' name='ENABLED' $checked{'ENABLED'}{'on'} /></td>
>  </tr>
> @@ -353,7 +394,7 @@ print <<END
>  </table>
>  </form>
>  END
> -;
> +
>  &Header::closebox();
>  
>  # Add visual indicators to column headings to show sort order - EO
> @@ -419,9 +460,15 @@ foreach my $line (@current) {
>      }
>      print "<tr style='$col'>";
>  
> +	my $address = $temp[0];
> +
> +	if ($temp[3] ne "") {
> +		$address .= " @ $temp[3]";
> +	}
> +
>      print <<END
>  <td style='text-align:center; $col'>$temp[2]</td>
> -<td style='text-align:center; $col'>$temp[0]</td>
> +<td style='text-align:center; $col'>$address</td>
>  
>  <td style='text-align:center; $col'>
>  <form method='post' action='$ENV{'SCRIPT_NAME'}'>
> @@ -542,7 +589,7 @@ sub SortDataFile
>  	# The KEY,key record permits doublons. If removed, then F1 becomes the key without doublon permitted.
>  
>  
> -	my @record = ('KEY',$key++,'IP',$temp[0],'ENABLED',$temp[1],'NAME',$temp[2]);
> +	my @record = ('KEY',$key++,'IP',$temp[0],'ENABLED',$temp[1],'NAME',$temp[2],'INTERFACE',$temp[3]);
>  	my $record = {};                        	# create a reference to empty hash
>  	%{$record} = @record;                		# populate that hash with @record
>  	$entries{$record->{KEY}} = $record; 		# add this to a hash of hashes
> @@ -552,7 +599,7 @@ sub SortDataFile
>  
>      # Each field value is printed , with the newline ! Don't forget separator and order of them.
>      foreach my $entry (sort fixedleasesort keys %entries) {
> -	print FILE "$entries{$entry}->{IP},$entries{$entry}->{ENABLED},$entries{$entry}->{NAME}\n";
> +	print FILE "$entries{$entry}->{IP},$entries{$entry}->{ENABLED},$entries{$entry}->{NAME},$entries{$entry}->{INTERFACE}\n";
>      }
>  
>      close(FILE);
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index b4c24a535..eb3e5f14f 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -422,6 +422,7 @@
>  'alcatelusb upload' => 'Upload Speedtouch USB firmware',
>  'alias ip' => 'Alias IP',
>  'aliases' => 'Aliases',
> +'aliases default interface' => '- Default Interface -',
>  'aliases not active' => 'Aliases will not be active unless your RED interface is STATIC',
>  'all' => 'All',
>  'all interfaces' => 'All Interfaces',
> diff --git a/src/misc-progs/setaliases.c b/src/misc-progs/setaliases.c
> index 4ba6816af..4d59aa0a8 100644
> --- a/src/misc-progs/setaliases.c
> +++ b/src/misc-progs/setaliases.c
> @@ -28,6 +28,8 @@
>  struct keyvalue *kv = NULL;
>  FILE *file = NULL;
>  
> +#define SCOPE 128
> +
>  void exithandler(void)
>  {
>  	if (kv) freekeyvalues(kv);
> @@ -45,6 +47,7 @@ int main(void)
>  	char *enabled;
>  	char *sptr;
>  	char *comment;
> +	char* intf = NULL;
>  	int alias;
>  	int count;
>  
> @@ -118,13 +121,12 @@ int main(void)
>  		exit(1);
>  	}
>  
> -	/* down the aliases in turn until ifconfig complains */
> -	alias=0;
> -	do
> -	{
> -		memset(command, 0, STRING_SIZE);
> -		snprintf(command, STRING_SIZE-1, "/sbin/ifconfig %s:%d down 2>/dev/null", red_dev, alias++);
> -	} while (safe_system(command)==0);
> +	// Flush all previous aliases
> +	alias = 0;
> +	do {
> +		snprintf(command, STRING_SIZE - 1,
> +			"ip addr flush dev red%d scope %d 2>/dev/null", alias++, SCOPE);
> +	} while (safe_system(command) == 0);
>  
>  	/* Now set up the new aliases from the config file */
>          if (!(file = fopen(CONFIG_ROOT "/ethernet/aliases", "r")))
> @@ -144,15 +146,18 @@ int main(void)
>                  aliasip = NULL;
>                  enabled = NULL;
>                  comment = NULL;
> +                intf = NULL;
>                  sptr = strtok(s, ",");
>                  while (sptr)
>                  {
>                          if (count == 0)
>                                  aliasip = sptr;
> -                        if (count == 1)
> +                        else if (count == 1)
>                                  enabled = sptr;
> -                        else
> +                        else if (count == 2)
>                                  comment = sptr;
> +                        else if (count == 3)
> +                                intf = sptr;
>                          count++;
>  			sptr = strtok(NULL, ",");
>  		}
> @@ -175,15 +180,18 @@ int main(void)
>                          exit(1);
>                  }
>  
> -		memset(command, 0, STRING_SIZE);
> -		snprintf(command, STRING_SIZE-1,
> -				"/sbin/ifconfig %s:%d %s netmask %s up",
> -			     red_dev, alias, aliasip, red_netmask);
> +		// Default to RED_DEV if intf isn't set
> +		if (!intf)
> +			intf = red_dev;
> +
> +		snprintf(command, STRING_SIZE - 1, "ip addr add %s/%s dev %s scope %d",
> +			aliasip, red_netmask, intf, SCOPE);
>  		safe_system(command);
> -		memset(command, 0, STRING_SIZE);
> +
> +		// Send an ARP broadcast
>  		snprintf(command, STRING_SIZE-1,
>  				"/usr/sbin/arping -q -c 1 -w 1 -i %s -S %s %s",
> -				red_dev, aliasip, default_gateway);
> +				intf, aliasip, default_gateway);
>  		safe_system(command);
>  		alias++;
>  	}