From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] BUG10940: remove leading zeros in ip address Date: Mon, 09 Nov 2015 22:16:09 +0000 Message-ID: <1447107369.2699.76.camel@ipfire.org> In-Reply-To: <5640F09E.6070708@oab.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6343786918669959276==" List-Id: --===============6343786918669959276== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit On Mon, 2015-11-09 at 20:14 +0100, Alexander Marx wrote: > > FULL ACK, Michael. > > But unfortunately it seems not to be possible to catch that issue > with modification of central functions like check_ip_address, becaus > ethat functions only return true or false not the "normalized ip" > without zero's. > > I am open for better solutions.... anyone ideas?! It is too late for that now. I think the best option would simply to consider those inputs as invalid and maybe fix this at some places where we can and where we find this makes sense. -Michael P.S. Please make sure to reply to the list. Especially if you are asking a question to the group :) > > cheers, > > Alex > > Am 09.11.2015 um 19:33 schrieb Michael Tremer: > > Hi, > > > > this does not look like the cleanest solution to be honest, but I > > guess > > this is acceptable. It solves the issue at hand, but generally I > > would > > like to have this solved one time so that one does not have to keep > > this problem in mind when writing new code. > > > > Merged. > > > > Best, > > -Michael > > > > On Mon, 2015-11-09 at 12:42 +0100, Alexander Marx wrote: > > > in firewallgroups (hosts) an error was created when using ip > > > adresses > > > like 192.168.000.008. Now all leading zeros are deleted in > > > firewallgroups and in the firewall itself when using single ip > > > addresses > > > as source or target. > > > > > > Signed-off-by: Alexander Marx > > > --- > > > config/cfgroot/network-functions.pl | 13 +++++++++++++ > > > html/cgi-bin/firewall.cgi | 8 ++++++++ > > > html/cgi-bin/fwhosts.cgi | 13 +++++++------ > > > 3 files changed, 28 insertions(+), 6 deletions(-) > > > > > > diff --git a/config/cfgroot/network-functions.pl > > > b/config/cfgroot/network-functions.pl > > > index cb4ca3d..70fa5ed 100644 > > > --- a/config/cfgroot/network-functions.pl > > > +++ b/config/cfgroot/network-functions.pl > > > @@ -122,6 +122,19 @@ sub network2bin($) { > > > return ($network_start, $netmask_bin); > > > } > > > > > > +# Deletes leading zeros in ip address > > > +sub ip_remove_zero{ > > > + my $address = shift; > > > + my @ip = split (/\./, $address); > > > + > > > + foreach my $octet (@ip) { > > > + $octet = int($octet); > > > + } > > > + > > > + $address = join (".", @ip); > > > + > > > + return $address; > > > +} > > > # Returns True for all valid IP addresses > > > sub check_ip_address($) { > > > my $address = shift; > > > diff --git a/html/cgi-bin/firewall.cgi b/html/cgi > > > -bin/firewall.cgi > > > index 682c285..8007182 100644 > > > --- a/html/cgi-bin/firewall.cgi > > > +++ b/html/cgi-bin/firewall.cgi > > > @@ -31,6 +31,7 @@ no warnings 'uninitialized'; > > > #use CGI::Carp 'fatalsToBrowser'; > > > > > > require '/var/ipfire/general-functions.pl'; > > > +require '/var/ipfire/network-functions.pl'; > > > require "${General::swroot}/lang.pl"; > > > require "${General::swroot}/header.pl"; > > > require "${General::swroot}/geoip-functions.pl"; > > > @@ -465,6 +466,9 @@ sub checksource > > > } > > > } > > > if ($fwdfwsettings{'isip'} eq 'on'){ > > > + #remove leading zero > > > + $ip = &Network::ip_remove_zero($ip); > > > + > > > ##check if ip is valid > > > if (! &General::validip($ip)){ > > > $errormessage.=$Lang::tr{'fwdfw > > > err > > > src_addr'}."
"; > > > @@ -569,11 +573,15 @@ sub checktarget > > > ($ip,$subnet)=split > > > (/\//,$fwdfwsettings{'tgt_addr'}); > > > $subnet = > > > &General::iporsubtocidr($subnet); > > > } > > > + > > > #check if only ip > > > if($fwdfwsettings{'tgt_addr'}=~/^(\d{1,3})\.(\d{ > > > 1,3} > > > )\.(\d{1,3})\.(\d{1,3})$/){ > > > $ip=$fwdfwsettings{'tgt_addr'}; > > > $subnet='32'; > > > } > > > + #remove leading zero > > > + $ip = &Network::ip_remove_zero($ip); > > > + > > > #check if ip is valid > > > if (! &General::validip($ip)){ > > > $errormessage.=$Lang::tr{'fwdfw err > > > tgt_addr'}."
"; > > > diff --git a/html/cgi-bin/fwhosts.cgi b/html/cgi-bin/fwhosts.cgi > > > index 994a50a..35afad3 100644 > > > --- a/html/cgi-bin/fwhosts.cgi > > > +++ b/html/cgi-bin/fwhosts.cgi > > > @@ -27,6 +27,7 @@ use Sort::Naturally; > > > use CGI::Carp 'fatalsToBrowser'; > > > no warnings 'uninitialized'; > > > require '/var/ipfire/general-functions.pl'; > > > +require '/var/ipfire/network-functions.pl'; > > > require "/var/ipfire/geoip-functions.pl"; > > > require "/usr/lib/firewall/firewall-lib.pl"; > > > require "${General::swroot}/lang.pl"; > > > @@ -277,6 +278,9 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' ) > > > &addnet; > > > &viewtablenet; > > > }else{ > > > + #convert ip if leading '0' exists > > > + $fwhostsettings{'IP'} = > > > &Network::ip_remove_zero($fwhostsettings{'IP'}); > > > + > > > #check valid ip > > > if > > > (!&General::validipandmask($fwhostsettings{'IP'}."/".$fwhostsetti > > > ngs{ > > > 'SUBNET'})) > > > { > > > @@ -372,9 +376,6 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' ) > > > foreach my $i (0 .. 3) { > > > $customnetwork{$key}[$i] = "";} > > > $fwhostsettings{'SUBNET'} = > > > &General::iporsubtocidr($fwhostsettings{'SUBNET'}); > > > $customnetwork{$key}[0] = > > > $fwhostsettings{'HOSTNAME'}; > > > - #convert ip when leading '0' in byte > > > - $fwhostsettings{'IP'} =&G > > > ener > > > al::ip2dec($fwhostsettings{'IP'}); > > > - $fwhostsettings{'IP'} =&G > > > ener > > > al::dec2ip($fwhostsettings{'IP'}); > > > $customnetwork{$key}[1] = > > > &General::getnetworkip($fwhostsettings{'IP'},$fwhostsettings{'SUB > > > NET' > > > }) ; > > > $customnetwork{$key}[2] = > > > &General::iporsubtodec($fwhostsettings{'SUBNET'}) ; > > > $customnetwork{$key}[3] = > > > $fwhostsettings{'NETREMARK'}; > > > @@ -423,6 +424,9 @@ if ($fwhostsettings{'ACTION'} eq 'savehost') > > > } > > > #CHECK IP-PART > > > if ($fwhostsettings{'type'} eq 'ip'){ > > > + #convert ip if leading '0' exists > > > + $fwhostsettings{'IP'} = > > > &Network::ip_remove_zero($fwhostsettings{'IP'}); > > > + > > > #check for subnet > > > if (rindex($fwhostsettings{'IP'},'/') eq > > > ' > > > -1' ){ > > > if($fwhostsettings{'type'} eq > > > 'ip' > > > && !&General::validipandmask($fwhostsettings{'IP'}."/32")) > > > @@ -503,9 +507,6 @@ if ($fwhostsettings{'ACTION'} eq 'savehost') > > > $customhost{$key}[0] = > > > $fwhostsettings{'HOSTNAME'} ; > > > $customhost{$key}[1] = > > > $fwhostsettings{'type'} ; > > > if ($fwhostsettings{'type'} eq 'ip'){ > > > - #convert ip when leading '0' in > > > byte > > > - $fwhostsettings{'IP'}=&General:: > > > ip2d > > > ec($fwhostsettings{'IP'}); > > > - $fwhostsettings{'IP'}=&General:: > > > dec2 > > > ip($fwhostsettings{'IP'}); > > > $customhost{$key}[2] = > > > $fwhostsettings{'IP'}."/".&General::iporsubtodec($fwhostsettings{ > > > 'SUB > > > NET'}); > > > }else{ > > > $customhost{$key}[2] = > > > $fwhostsettings{'IP'}; --===============6343786918669959276== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjEKCmlRSWNCQUFC Q2dBR0JRSldRUnNwQUFvSkVJQjU4UDl2a0FrSEwzc1AvUnVjSkNPQnNPWEFLRVBVOGlYS1poc04K QzNqUkJiV2ZqRjBhb2x6NXZrQ096ZGRnc0ZUTDEzL2lFZUNjOVVEQWVuNVBUTU5EclhGVXdURXht MVdmNCtLZQpNMXB3RU8xVVF1ZWpUQW5PajlTREFMd3YwSmM5YzBzWUgyTkl2cTlJZG9teTc4WW1t ZVRFSUQ0SnpvcGRFdFhtCjNkWno1YitNTE1ETnZiaUZoaERJbCt0cjkvQTBTR1JYYk4wMG9CRkRr MkdDMkFRbWZRS0poQWYxN2ptenkrME0KNnlwYkhYV2g5RDFvSXRhYThtUnk2VDIrRXRxWE40QVc5 SElGdlhsa1JZd3hwMGVlU2RlZWJ0bGFUaEVINC9iQgpWMkVpTjJFV28welA3MXk1Z1FlYjJOOFpU anE1WCtXOUNUd1NvR1VIN1l5WGpobWYrU0RuekhnQkFwSXkvekxRClUyQTVYWnJiS1BaQ3JFbnB3 dFdqVEdSb0cwcHRwSk1SNElxR2FYcU1kQmFiT1hoNlA4aEErN1VvRzdHOU0vT0MKNERzY3AzVG8r ZGtkUWFic1gxaW9yYzh3Y2VOS2pYWlZPTHk2WS95OWZxWFRqMHJ5b2JKVVZFQncxZ0FDUHpQYQpQ aFE2U1FpNkJWbitaL2VCSk9nclE0SGZHNXFjRU1UM2w2ZHJ0Zjg3NUx6ZFU1VjdlVUU2djZNVHBF R1lPMEdVCkJsYWI5SGRRYi9abVVmTUEyYUV4Z3VManEyTHpyUHg3b2JEdDZBRVg1d0dadmEzaUxu NXdCNFlTTTVYTXZhQmUKRC9SbUM5TGdSNnJKT0RuZEp2NU40S2ovaXJ3eU92NXlvV1ZTd1VOWVhr ZmxmcEFFWS9wMFVPazNUd0cza0ZZaApCbVM0bzlhTHBNYXRkWUt4QW91awo9YU53bwotLS0tLUVO RCBQR1AgU0lHTkFUVVJFLS0tLS0K --===============6343786918669959276==--