[PATCH] BUG11278: It is not possible to create subnets of internal networks in firewallgroups

[PATCH] BUG11278: It is not possible to create subnets of internal networks in firewallgroups

[Alexander Marx]

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

Fixes: #11278

When creating networks which are part of an internal network, there was an errormessage displayed and the creation was prohibited.
Now it is possible to create such subnets. This is used at own risk! Users have to take care of the firewallrule sequence.
It is possible to create situations that are not wanted.

Signed-off-by: Alexander Marx <alexander.marx(a)ipfire.org>
---
 config/cfgroot/general-functions.pl | 24 ++++++++++++++++++++++--
 html/cgi-bin/fwhosts.cgi            |  2 +-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
index 5e5417d..f448c34 100644
--- a/config/cfgroot/general-functions.pl
+++ b/config/cfgroot/general-functions.pl
@@ -465,6 +465,7 @@ sub checksubnets
 	my $ccdname=$_[0];
 	my $ccdnet=$_[1];
 	my $ownnet=$_[2];
+	my $checktype=$_[3];
 	my $errormessage;
 	my ($ip,$cidr)=split(/\//,$ccdnet);
 	$cidr=&iporsubtocidr($cidr);
@@ -542,10 +543,15 @@ sub checksubnets
 	}
 	
 	#call check_net_internal
-	&General::check_net_internal($ccdnet);
+	if ($checktype eq "exact")
+	{
+		&General::check_net_internal_exact($ccdnet);
+	}else{
+		&General::check_net_internal_range($ccdnet);
+	}
 }
 
-sub check_net_internal{
+sub check_net_internal_range{
 	my $network=shift;
 	my ($ip,$cidr)=split(/\//,$network);
 	my %ownnet=();
@@ -559,6 +565,20 @@ sub check_net_internal{
 	if (($ownnet{'RED_NETADDRESS'} 		ne '' && $ownnet{'RED_NETADDRESS'} 		ne '0.0.0.0') && &IpInSubnet($ip,$ownnet{'RED_NETADDRESS'},&iporsubtodec($ownnet{'RED_NETMASK'}))){ $errormessage=$Lang::tr{'ccd err red'};return $errormessage;}
 }
 
+sub check_net_internal_exact{
+	my $network=shift;
+	my ($ip,$cidr)=split(/\//,$network);
+	my %ownnet=();
+	my $errormessage;
+	$cidr=&iporsubtocidr($cidr);
+	#check if we use one of ipfire's networks (green,orange,blue)
+	&readhash("${General::swroot}/ethernet/settings", \%ownnet);
+	if (($ownnet{'GREEN_NETADDRESS'}  	ne '' && $ownnet{'GREEN_NETADDRESS'} 	ne '0.0.0.0') && &Network::network_equal("$ownnet{'GREEN_NETADDRESS'}/$ownnet{'GREEN_NETMASK'}", $network)){ $errormessage=$Lang::tr{'ccd err green'};return $errormessage;}
+	if (($ownnet{'ORANGE_NETADDRESS'}	ne '' && $ownnet{'ORANGE_NETADDRESS'} 	ne '0.0.0.0') && &Network::network_equal("$ownnet{'ORANGE_NETADDRESS'}/$ownnet{'ORANGE_NETMASK'}", $network)){ $errormessage=$Lang::tr{'ccd err orange'};return $errormessage;}
+	if (($ownnet{'BLUE_NETADDRESS'} 	ne '' && $ownnet{'BLUE_NETADDRESS'} 	ne '0.0.0.0') && &Network::network_equal("$ownnet{'BLUE_NETADDRESS'}/$ownnet{'BLUE_NETMASK'}", $network)){ $errormessage=$Lang::tr{'ccd err blue'};return $errormessage;}
+	if (($ownnet{'RED_NETADDRESS'} 		ne '' && $ownnet{'RED_NETADDRESS'} 		ne '0.0.0.0') && &Network::network_equal("$ownnet{'RED_NETADDRESS'}/$ownnet{'RED_NETMASK'}", $network)){ $errormessage=$Lang::tr{'ccd err red'};return $errormessage;}
+}
+
 sub validport
 {
 	$_ = $_[0];
diff --git a/html/cgi-bin/fwhosts.cgi b/html/cgi-bin/fwhosts.cgi
index 1b0fe07..25ab489 100644
--- a/html/cgi-bin/fwhosts.cgi
+++ b/html/cgi-bin/fwhosts.cgi
@@ -301,7 +301,7 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' )
 		}
 		if($fwhostsettings{'error'} ne 'on'){
 				my $fullip="$fwhostsettings{'IP'}/".&General::iporsubtocidr($fwhostsettings{'SUBNET'});
-				$errormessage=$errormessage.&General::checksubnets($fwhostsettings{'HOSTNAME'},$fullip,"");
+				$errormessage=$errormessage.&General::checksubnets($fwhostsettings{'HOSTNAME'},$fullip,"","exact");
 		}
 		#only check plausi when no error till now
 		if (!$errormessage){
-- 
2.7.4

Re: [PATCH] BUG11278: It is not possible to create subnets of internal networks in firewallgroups

[Michael Tremer]

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

Hey,

although I am not perfectly happy with the code quality, I merged this patch.

The entire code base here needs to become cleaner and we need to break it down
better into small functions that serve a single and easy purpose.

Right now it is possible to completely change the behaviour of a function with a
paramter. That is hard to understand and document. Hence I would like to change
this.

We already have some good starting points and therefore I would like to aim for
improving this in the month of June.

I created a ticket on BZ to keep track of this and assigned it to you.

Any additional help is of course appreciated.

Best,
-Michael

On Wed, 2017-06-07 at 15:13 +0200, Alexander Marx wrote:
> Fixes: #11278
> 
> When creating networks which are part of an internal network, there was an
> errormessage displayed and the creation was prohibited.
> Now it is possible to create such subnets. This is used at own risk! Users
> have to take care of the firewallrule sequence.
> It is possible to create situations that are not wanted.
> 
> Signed-off-by: Alexander Marx <alexander.marx(a)ipfire.org>
> ---
>  config/cfgroot/general-functions.pl | 24 ++++++++++++++++++++++--
>  html/cgi-bin/fwhosts.cgi            |  2 +-
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-
> functions.pl
> index 5e5417d..f448c34 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -465,6 +465,7 @@ sub checksubnets
>  	my $ccdname=$_[0];
>  	my $ccdnet=$_[1];
>  	my $ownnet=$_[2];
> +	my $checktype=$_[3];
>  	my $errormessage;
>  	my ($ip,$cidr)=split(/\//,$ccdnet);
>  	$cidr=&iporsubtocidr($cidr);
> @@ -542,10 +543,15 @@ sub checksubnets
>  	}
>  	
>  	#call check_net_internal
> -	&General::check_net_internal($ccdnet);
> +	if ($checktype eq "exact")
> +	{
> +		&General::check_net_internal_exact($ccdnet);
> +	}else{
> +		&General::check_net_internal_range($ccdnet);
> +	}
>  }
>  
> -sub check_net_internal{
> +sub check_net_internal_range{
>  	my $network=shift;
>  	my ($ip,$cidr)=split(/\//,$network);
>  	my %ownnet=();
> @@ -559,6 +565,20 @@ sub check_net_internal{
>  	if (($ownnet{'RED_NETADDRESS'} 		ne '' &&
> $ownnet{'RED_NETADDRESS'} 		ne '0.0.0.0') &&
> &IpInSubnet($ip,$ownnet{'RED_NETADDRESS'},&iporsubtodec($ownnet{'RED_NETMASK'}
> ))){ $errormessage=$Lang::tr{'ccd err red'};return $errormessage;}
>  }
>  
> +sub check_net_internal_exact{
> +	my $network=shift;
> +	my ($ip,$cidr)=split(/\//,$network);
> +	my %ownnet=();
> +	my $errormessage;
> +	$cidr=&iporsubtocidr($cidr);
> +	#check if we use one of ipfire's networks (green,orange,blue)
> +	&readhash("${General::swroot}/ethernet/settings", \%ownnet);
> +	if (($ownnet{'GREEN_NETADDRESS'}  	ne '' &&
> $ownnet{'GREEN_NETADDRESS'} 	ne '0.0.0.0') &&
> &Network::network_equal("$ownnet{'GREEN_NETADDRESS'}/$ownnet{'GREEN_NETMASK'}"
> , $network)){ $errormessage=$Lang::tr{'ccd err green'};return $errormessage;}
> +	if (($ownnet{'ORANGE_NETADDRESS'}	ne '' &&
> $ownnet{'ORANGE_NETADDRESS'} 	ne '0.0.0.0') &&
> &Network::network_equal("$ownnet{'ORANGE_NETADDRESS'}/$ownnet{'ORANGE_NETMASK'
> }", $network)){ $errormessage=$Lang::tr{'ccd err orange'};return
> $errormessage;}
> +	if (($ownnet{'BLUE_NETADDRESS'} 	ne '' &&
> $ownnet{'BLUE_NETADDRESS'} 	ne '0.0.0.0') &&
> &Network::network_equal("$ownnet{'BLUE_NETADDRESS'}/$ownnet{'BLUE_NETMASK'}",
> $network)){ $errormessage=$Lang::tr{'ccd err blue'};return $errormessage;}
> +	if (($ownnet{'RED_NETADDRESS'} 		ne '' &&
> $ownnet{'RED_NETADDRESS'} 		ne '0.0.0.0') &&
> &Network::network_equal("$ownnet{'RED_NETADDRESS'}/$ownnet{'RED_NETMASK'}",
> $network)){ $errormessage=$Lang::tr{'ccd err red'};return $errormessage;}
> +}
> +
>  sub validport
>  {
>  	$_ = $_[0];
> diff --git a/html/cgi-bin/fwhosts.cgi b/html/cgi-bin/fwhosts.cgi
> index 1b0fe07..25ab489 100644
> --- a/html/cgi-bin/fwhosts.cgi
> +++ b/html/cgi-bin/fwhosts.cgi
> @@ -301,7 +301,7 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' )
>  		}
>  		if($fwhostsettings{'error'} ne 'on'){
>  				my
> $fullip="$fwhostsettings{'IP'}/".&General::iporsubtocidr($fwhostsettings{'SUBN
> ET'});
> -				$errormessage=$errormessage.&General::checksu
> bnets($fwhostsettings{'HOSTNAME'},$fullip,"");
> +				$errormessage=$errormessage.&General::checksu
> bnets($fwhostsettings{'HOSTNAME'},$fullip,"","exact");
>  		}
>  		#only check plausi when no error till now
>  		if (!$errormessage){

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]