From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] BUG11278: It is not possible to create subnets of internal networks in firewallgroups Date: Wed, 07 Jun 2017 17:22:04 +0100 Message-ID: <1496852524.21214.5.camel@ipfire.org> In-Reply-To: <1496841236-15315-1-git-send-email-alexander.marx@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6331447291566318762==" List-Id: --===============6331447291566318762== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 wit= h a paramter. That is hard to understand and document. Hence I would like to chan= ge this. We already have some good starting points and therefore I would like to aim f= or 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 >=20 > 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. >=20 > Signed-off-by: Alexander Marx > --- > =C2=A0config/cfgroot/general-functions.pl | 24 ++++++++++++++++++++++-- > =C2=A0html/cgi-bin/fwhosts.cgi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A02 +- > =C2=A02 files changed, 23 insertions(+), 3 deletions(-) >=20 > 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 > =C2=A0 my $ccdname=3D$_[0]; > =C2=A0 my $ccdnet=3D$_[1]; > =C2=A0 my $ownnet=3D$_[2]; > + my $checktype=3D$_[3]; > =C2=A0 my $errormessage; > =C2=A0 my ($ip,$cidr)=3Dsplit(/\//,$ccdnet); > =C2=A0 $cidr=3D&iporsubtocidr($cidr); > @@ -542,10 +543,15 @@ sub checksubnets > =C2=A0 } > =C2=A0=09 > =C2=A0 #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); > + } > =C2=A0} > =C2=A0 > -sub check_net_internal{ > +sub check_net_internal_range{ > =C2=A0 my $network=3Dshift; > =C2=A0 my ($ip,$cidr)=3Dsplit(/\//,$network); > =C2=A0 my %ownnet=3D(); > @@ -559,6 +565,20 @@ sub check_net_internal{ > =C2=A0 if (($ownnet{'RED_NETADDRESS'}=C2=A0 ne '' && > $ownnet{'RED_NETADDRESS'}=C2=A0 ne '0.0.0.0') && > &IpInSubnet($ip,$ownnet{'RED_NETADDRESS'},&iporsubtodec($ownnet{'RED_NETMAS= K'} > ))){ $errormessage=3D$Lang::tr{'ccd err red'};return $errormessage;} > =C2=A0} > =C2=A0 > +sub check_net_internal_exact{ > + my $network=3Dshift; > + my ($ip,$cidr)=3Dsplit(/\//,$network); > + my %ownnet=3D(); > + my $errormessage; > + $cidr=3D&iporsubtocidr($cidr); > + #check if we use one of ipfire's networks (green,orange,blue) > + &readhash("${General::swroot}/ethernet/settings", \%ownnet); > + if (($ownnet{'GREEN_NETADDRESS'}=C2=A0=C2=A0 ne '' && > $ownnet{'GREEN_NETADDRESS'}=C2=A0 ne '0.0.0.0') && > &Network::network_equal("$ownnet{'GREEN_NETADDRESS'}/$ownnet{'GREEN_NETMASK= '}" > , $network)){ $errormessage=3D$Lang::tr{'ccd err green'};return $errormessa= ge;} > + if (($ownnet{'ORANGE_NETADDRESS'} ne '' && > $ownnet{'ORANGE_NETADDRESS'}=C2=A0 ne '0.0.0.0') && > &Network::network_equal("$ownnet{'ORANGE_NETADDRESS'}/$ownnet{'ORANGE_NETMA= SK' > }", $network)){ $errormessage=3D$Lang::tr{'ccd err orange'};return > $errormessage;} > + if (($ownnet{'BLUE_NETADDRESS'}=C2=A0 ne '' && > $ownnet{'BLUE_NETADDRESS'}=C2=A0 ne '0.0.0.0') && > &Network::network_equal("$ownnet{'BLUE_NETADDRESS'}/$ownnet{'BLUE_NETMASK'}= ", > $network)){ $errormessage=3D$Lang::tr{'ccd err blue'};return $errormessage;} > + if (($ownnet{'RED_NETADDRESS'}=C2=A0 ne '' && > $ownnet{'RED_NETADDRESS'}=C2=A0 ne '0.0.0.0') && > &Network::network_equal("$ownnet{'RED_NETADDRESS'}/$ownnet{'RED_NETMASK'}", > $network)){ $errormessage=3D$Lang::tr{'ccd err red'};return $errormessage;} > +} > + > =C2=A0sub validport > =C2=A0{ > =C2=A0 $_ =3D $_[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' ) > =C2=A0 } > =C2=A0 if($fwhostsettings{'error'} ne 'on'){ > =C2=A0 my > $fullip=3D"$fwhostsettings{'IP'}/".&General::iporsubtocidr($fwhostsettings{= 'SUBN > ET'}); > - $errormessage=3D$errormessage.&General::checksu > bnets($fwhostsettings{'HOSTNAME'},$fullip,""); > + $errormessage=3D$errormessage.&General::checksu > bnets($fwhostsettings{'HOSTNAME'},$fullip,"","exact"); > =C2=A0 } > =C2=A0 #only check plausi when no error till now > =C2=A0 if (!$errormessage){ --===============6331447291566318762== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjIKCmlRSWNCQUFC Q2dBR0JRSlpPQ2dzQUFvSkVJQjU4UDl2a0FrSHJ5b1AvMDdpdXVKdUdVQzBUTUJkdy9RSkVsMUsK cXVGSjZzb1RRcmJHeS9yc3k0b2xwTGNYVjk3U1ZxeVJjbERHK29DL2d3WTVkMHVXQ3puNjJUd0lK K0JqL0MzKwpLc2R6ZDRXRXp4QU10NzBvZnFKY21EMy83MXV3MzFtSVpUNkhXU1NvZXgzOE1OQnpr WFYxZzgwR0hpNTIrQyt4CnZwRyt5TUlYcjA5NGJEY0lPQkxkUGxodWNTSFZUS2pLbzk3N1Bqdjdq d2wrVjczVGVkcGZ5Zi9NY3RadXNzN2IKaWYxNlhXR0F3aGJCWDNVelZ6TC85Tmo4UkRqOHRDY0NE aERsSlYzZ2oxbU1nS096NnlNa3AyTzJReU0yUGlkUQpkRzlGOUJJZFBEOGZtY3ZBbTZ5bUdGcTRS RWNmQ1dIaWVVaTVienE0Rkp2ejBWNEgyVW55WFpYbXZ0c2xMZzBYCngzSmxWVWRKeGo4ZUMzbTNr bGVwRWJyZmhXOURkYmRaU2g3ZnNwSXZGRjdwcTUwaWxaVGlxZ2d1VXhrMlI0bVQKOGY5Y2dOcm03 d2c4UW5teU5DWE44OGw3U0ZBaklISXU3VDhMbExjREFCZDJvZEJZTGpvQlZVZi92ZDROWnhQMQpQ dkhOR0dac08zQ2MvQVI0alBVQTZ6ODhDNlM4U25WZHVGRDVicnh5bkRKZzhGc0Y0alQ5NFA2ZzEv YVlHVy9WCjl0NVA1Y2tlUTFBZ1gxNmRNdFE2eTNKZVhXMG9UeHVUc01KUEZmdW9BdGwyZVhUcTNN cVlEVWhjdHEzNk4wdGIKbkZQS1hlL2RPRFM0U0JLZ0d0Si9QWkxmZzNSVlBNODV5Q09icjRoYk0v RmFsMmxyM3c0eWJTdVQvVkdNRXgrdApTd0FIQzFXQXp6REh4N0ozejlhNQo9eks2awotLS0tLUVO RCBQR1AgU0lHTkFUVVJFLS0tLS0K --===============6331447291566318762==--