BUG11559: There was no possibillity to select single IpSec subnets (if any) in the firewall creation site. Now the dropdown for IpSec is adapted to reflect the single TARGET subnets.

Message ID 1525068720-8894-1-git-send-email-alexander.marx@ipfire.org
State Superseded
Headers
Series BUG11559: There was no possibillity to select single IpSec subnets (if any) in the firewall creation site. Now the dropdown for IpSec is adapted to reflect the single TARGET subnets. |

Commit Message

Alexander Marx April 30, 2018, 4:12 p.m. UTC
  Another patch will follow to make these changes in the firewall groups with the language changes ("All subnets" instead of "all")

fixes: #11559
---
 config/firewall/firewall-lib.pl | 19 +++++++++++++++----
 html/cgi-bin/firewall.cgi       | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 7 deletions(-)
  

Comments

Michael Tremer April 30, 2018, 9:12 p.m. UTC | #1
On Mon, 2018-04-30 at 08:12 +0200, Alexander Marx wrote:
> Another patch will follow to make these changes in the firewall groups with
> the language changes ("All subnets" instead of "all")

Please submit all patches that belong together as a single patchset.

-M

> 
> fixes: #11559
> ---
>  config/firewall/firewall-lib.pl | 19 +++++++++++++++----
>  html/cgi-bin/firewall.cgi       | 36 +++++++++++++++++++++++++++++++++---
>  2 files changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/config/firewall/firewall-lib.pl b/config/firewall/firewall-lib.pl
> index eabd9a4..668eb9e 100644
> --- a/config/firewall/firewall-lib.pl
> +++ b/config/firewall/firewall-lib.pl
> @@ -150,6 +150,9 @@ sub get_ipsec_net_ip
>  	my $val=shift;
>  	my $field=shift;
>  	foreach my $key (sort {$a <=> $b} keys %ipsecconf){
> +		#adapt $val to reflect real name without subnet (if rule with
> only one ipsec subnet is created)
> +		my @tmpval = split (/\|/, $val);
> +		$val = $tmpval[0];
>  		if($ipsecconf{$key}[1] eq $val){
>  			return $ipsecconf{$key}[$field];
>  		}
> @@ -390,10 +393,18 @@ sub get_address
>  
>  	# IPsec networks.
>  	} elsif ($key ~~ ["ipsec_net_src", "ipsec_net_tgt", "IpSec Network"])
> {
> -		my $network_address = &get_ipsec_net_ip($value, 11);
> -		my @nets = split(/\|/, $network_address);
> -		foreach my $net (@nets) {
> -			push(@ret, [$net, ""]);
> +		#Check if we have multiple subnets and only want one of them
> +		
> +		if ( $value =~ /\|/ ){
> +			my @parts = split(/\|/, $value);
> +			push(@ret, [$parts[1], ""]);
> +			
> +		}else{
> +			my $network_address = &get_ipsec_net_ip($value, 11);
> +			my @nets = split(/\|/, $network_address);
> +			foreach my $net (@nets) {
> +				push(@ret, [$net, ""]);
> +			}
>  		}
>  
>  	# The firewall's own IP addresses.
> diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
> index face0f4..65e43a1 100644
> --- a/html/cgi-bin/firewall.cgi
> +++ b/html/cgi-bin/firewall.cgi
> @@ -1161,11 +1161,31 @@ END
>  	#IPsec netze
>  	foreach my $key (sort { ncmp($ipsecconf{$a}[1],$ipsecconf{$b}[1]) }
> keys %ipsecconf) {
>  		if ($ipsecconf{$key}[3] eq 'net' ||
> ($optionsfw{'SHOWDROPDOWN'} eq 'on' && $ipsecconf{$key}[3] ne 'host')){
> -			print"<tr><td valign='top'><input type='radio'
> name='$grp' value='ipsec_net_$srctgt'
> $checked{$grp}{'ipsec_net_'.$srctgt}></td><td >$Lang::tr{'fwhost ipsec
> net'}</td><td align='right'><select name='ipsec_net_$srctgt'
> style='width:200px;'>" if ($show eq '');
> +			print"<tr><td valign='top'><input type='radio'
> name='$grp' id='ipsec_net_$srctgt' value='ipsec_net_$srctgt'
> $checked{$grp}{'ipsec_net_'.$srctgt}></td><td >$Lang::tr{'fwhost ipsec
> net'}</td><td align='right'><select name='ipsec_net_$srctgt'
> style='width:200px;'>" if ($show eq '');
>  			$show='1';
> +
> +			#Check if we have more than one REMOTE subnet in
> config
> +			my @arr1 = split /\|/, $ipsecconf{$key}[11];
> +			my $cnt1 += @arr1;
> +
>  			print "<option ";
> -			print "selected='selected'" if
> ($fwdfwsettings{$fwdfwsettings{$grp}} eq $ipsecconf{$key}[1]);
> -			print ">$ipsecconf{$key}[1]</option>";
> +			print "value=$ipsecconf{$key}[1]";
> +			print " selected " if
> ($fwdfwsettings{$fwdfwsettings{$grp}} eq "$ipsecconf{$key}[1]");
> +			print ">$ipsecconf{$key}[1] ";
> +			print "$Lang::tr{'all'}" if $cnt1 > 1; #If this
> Conenction has more than one subnet, print one option for all subnets
> +			print "</option>";
> +
> +			if ($cnt1 > 1){
> +				foreach my $val (@arr1){
> +					#normalize subnet to cidr notation
> +					my ($val1,$val2) = split /\//, $val;
> +					my $val3 =
> &General::iporsubtocidr($val2);
> +					print "<option ";
> +					print
> "value='$ipsecconf{$key}[1]|$val1/$val3'";
> +					print "selected " if
> ($fwdfwsettings{$fwdfwsettings{$grp}} eq "$ipsecconf{$key}[1]|$val1/$val3");
> +					print ">$ipsecconf{$key}[1]
> $val1/$val3</option>";
> +				}
> +			}
>  		}
>  	}
>  	if($optionsfw{'SHOWDROPDOWN'} eq 'on' && $show eq ''){
> @@ -2575,6 +2595,11 @@ END
>  			#SOURCE
>  			my $ipfireiface;
>  			&getcolor($$hash{$key}[3],$$hash{$key}[4],\%customhos
> t);
> +			# Check SRC Host and replace "|" with space
> +			if ($$hash{$key}[4] =~ /\|/){
> +				$$hash{$key}[4] =~ s/\|/ (/g;
> +				$$hash{$key}[4] = $$hash{$key}[4].")";
> +			}
>  			print"<td align='center' width='30%' $tdcolor>";
>  			if ($$hash{$key}[3] eq 'ipfire_src'){
>  				$ipfireiface=$Lang::tr{'fwdfw iface'};
> @@ -2640,6 +2665,11 @@ END
>  			print<<END;
>  					<td align='center' $tdcolor>
>  END
> +			# Check TGT Host and replace "|" with space
> +			if ($$hash{$key}[6] =~ /\|/){
> +				$$hash{$key}[6] =~ s/\|/ (/g;
> +				$$hash{$key}[6] = $$hash{$key}[6].")";
> +			}
>  			#Is this a DNAT rule?
>  			my $natstring;
>  			if ($$hash{$key}[31] eq 'dnat' && $$hash{$key}[28] eq
> 'ON'){
  

Patch

diff --git a/config/firewall/firewall-lib.pl b/config/firewall/firewall-lib.pl
index eabd9a4..668eb9e 100644
--- a/config/firewall/firewall-lib.pl
+++ b/config/firewall/firewall-lib.pl
@@ -150,6 +150,9 @@  sub get_ipsec_net_ip
 	my $val=shift;
 	my $field=shift;
 	foreach my $key (sort {$a <=> $b} keys %ipsecconf){
+		#adapt $val to reflect real name without subnet (if rule with only one ipsec subnet is created)
+		my @tmpval = split (/\|/, $val);
+		$val = $tmpval[0];
 		if($ipsecconf{$key}[1] eq $val){
 			return $ipsecconf{$key}[$field];
 		}
@@ -390,10 +393,18 @@  sub get_address
 
 	# IPsec networks.
 	} elsif ($key ~~ ["ipsec_net_src", "ipsec_net_tgt", "IpSec Network"]) {
-		my $network_address = &get_ipsec_net_ip($value, 11);
-		my @nets = split(/\|/, $network_address);
-		foreach my $net (@nets) {
-			push(@ret, [$net, ""]);
+		#Check if we have multiple subnets and only want one of them
+		
+		if ( $value =~ /\|/ ){
+			my @parts = split(/\|/, $value);
+			push(@ret, [$parts[1], ""]);
+			
+		}else{
+			my $network_address = &get_ipsec_net_ip($value, 11);
+			my @nets = split(/\|/, $network_address);
+			foreach my $net (@nets) {
+				push(@ret, [$net, ""]);
+			}
 		}
 
 	# The firewall's own IP addresses.
diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
index face0f4..65e43a1 100644
--- a/html/cgi-bin/firewall.cgi
+++ b/html/cgi-bin/firewall.cgi
@@ -1161,11 +1161,31 @@  END
 	#IPsec netze
 	foreach my $key (sort { ncmp($ipsecconf{$a}[1],$ipsecconf{$b}[1]) } keys %ipsecconf) {
 		if ($ipsecconf{$key}[3] eq 'net' || ($optionsfw{'SHOWDROPDOWN'} eq 'on' && $ipsecconf{$key}[3] ne 'host')){
-			print"<tr><td valign='top'><input type='radio' name='$grp' value='ipsec_net_$srctgt' $checked{$grp}{'ipsec_net_'.$srctgt}></td><td >$Lang::tr{'fwhost ipsec net'}</td><td align='right'><select name='ipsec_net_$srctgt' style='width:200px;'>" if ($show eq '');
+			print"<tr><td valign='top'><input type='radio' name='$grp' id='ipsec_net_$srctgt' value='ipsec_net_$srctgt' $checked{$grp}{'ipsec_net_'.$srctgt}></td><td >$Lang::tr{'fwhost ipsec net'}</td><td align='right'><select name='ipsec_net_$srctgt' style='width:200px;'>" if ($show eq '');
 			$show='1';
+
+			#Check if we have more than one REMOTE subnet in config
+			my @arr1 = split /\|/, $ipsecconf{$key}[11];
+			my $cnt1 += @arr1;
+
 			print "<option ";
-			print "selected='selected'" if ($fwdfwsettings{$fwdfwsettings{$grp}} eq $ipsecconf{$key}[1]);
-			print ">$ipsecconf{$key}[1]</option>";
+			print "value=$ipsecconf{$key}[1]";
+			print " selected " if ($fwdfwsettings{$fwdfwsettings{$grp}} eq "$ipsecconf{$key}[1]");
+			print ">$ipsecconf{$key}[1] ";
+			print "$Lang::tr{'all'}" if $cnt1 > 1; #If this Conenction has more than one subnet, print one option for all subnets
+			print "</option>";
+
+			if ($cnt1 > 1){
+				foreach my $val (@arr1){
+					#normalize subnet to cidr notation
+					my ($val1,$val2) = split /\//, $val;
+					my $val3 = &General::iporsubtocidr($val2);
+					print "<option ";
+					print "value='$ipsecconf{$key}[1]|$val1/$val3'";
+					print "selected " if ($fwdfwsettings{$fwdfwsettings{$grp}} eq "$ipsecconf{$key}[1]|$val1/$val3");
+					print ">$ipsecconf{$key}[1] $val1/$val3</option>";
+				}
+			}
 		}
 	}
 	if($optionsfw{'SHOWDROPDOWN'} eq 'on' && $show eq ''){
@@ -2575,6 +2595,11 @@  END
 			#SOURCE
 			my $ipfireiface;
 			&getcolor($$hash{$key}[3],$$hash{$key}[4],\%customhost);
+			# Check SRC Host and replace "|" with space
+			if ($$hash{$key}[4] =~ /\|/){
+				$$hash{$key}[4] =~ s/\|/ (/g;
+				$$hash{$key}[4] = $$hash{$key}[4].")";
+			}
 			print"<td align='center' width='30%' $tdcolor>";
 			if ($$hash{$key}[3] eq 'ipfire_src'){
 				$ipfireiface=$Lang::tr{'fwdfw iface'};
@@ -2640,6 +2665,11 @@  END
 			print<<END;
 					<td align='center' $tdcolor>
 END
+			# Check TGT Host and replace "|" with space
+			if ($$hash{$key}[6] =~ /\|/){
+				$$hash{$key}[6] =~ s/\|/ (/g;
+				$$hash{$key}[6] = $$hash{$key}[6].")";
+			}
 			#Is this a DNAT rule?
 			my $natstring;
 			if ($$hash{$key}[31] eq 'dnat' && $$hash{$key}[28] eq 'ON'){