* [PATCH] BUG12265: firewall: iptables rules are being created in the wrong chain Fixes: #12265
@ 2021-03-25 10:23 Alexander Marx
2021-03-27 20:45 ` Peter Müller
0 siblings, 1 reply; 2+ messages in thread
From: Alexander Marx @ 2021-03-25 10:23 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 7139 bytes --]
When creating a rule like Source:Orange and Target:green IPfire Interface,
the rule was created in the forward instead of input chain.
This patch sets correct chain and additionally checks
if a single target ip (when set) is one of the ipfire interface ip addresses.
If this is the case, the target is automatically changed to IPFIRE interface instead of single target ip.
---
html/cgi-bin/firewall.cgi | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
index 532f99f91..c680eed1d 100644
--- a/html/cgi-bin/firewall.cgi
+++ b/html/cgi-bin/firewall.cgi
@@ -2,7 +2,7 @@
###############################################################################
# #
# IPFire.org - A linux based firewall #
-# Copyright (C) 2013 Alexander Marx <amarx(a)ipfire.org> #
+# Copyright (C) 2021 Alexander Marx <amarx(a)ipfire.org> #
# #
# This program is free software: you can redistribute it and/or modify #
# it under the terms of the GNU General Public License as published by #
@@ -213,6 +213,7 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
&General::readhasharray("$configfwdfw", \%configfwdfw);
&General::readhasharray("$configinput", \%configinputfw);
&General::readhasharray("$configoutgoing", \%configoutgoingfw);
+ &General::readhash("/var/ipfire/ethernet/settings", \%netsettings);
my $maxkey;
#Set Variables according to the JQuery code in protocol section
if ($fwdfwsettings{'PROT'} eq 'TCP' || $fwdfwsettings{'PROT'} eq 'UDP')
@@ -231,6 +232,19 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
{
$fwdfwsettings{'USESRV'} = 'ON';
}
+ #Check if manual targetip is one of IPFire addresses
+ if ($fwdfwsettings{'grp2'} eq 'tgt_addr' && $fwdfwsettings{$fwdfwsettings{'grp2'}} eq $netsettings{'GREEN_ADDRESS'}){
+ $fwdfwsettings{'grp2'} = 'ipfire';
+ $fwdfwsettings{$fwdfwsettings{'grp2'}} = 'GREEN';
+ }
+ if ($fwdfwsettings{'grp2'} eq 'tgt_addr' && $fwdfwsettings{$fwdfwsettings{'grp2'}} eq $netsettings{'ORANGE_ADDRESS'}){
+ $fwdfwsettings{'grp2'} = 'ipfire';
+ $fwdfwsettings{$fwdfwsettings{'grp2'}} = 'ORANGE';
+ }
+ if ($fwdfwsettings{'grp2'} eq 'tgt_addr' && $fwdfwsettings{$fwdfwsettings{'grp2'}} eq $netsettings{'BLUE_ADDRESS'}){
+ $fwdfwsettings{'grp2'} = 'ipfire';
+ $fwdfwsettings{$fwdfwsettings{'grp2'}} = 'BLUE';
+ }
$errormessage=&checksource;
if(!$errormessage){&checktarget;}
if(!$errormessage){&checkrule;}
@@ -247,7 +261,7 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
$errormessage=$Lang::tr{'fwdfw err same'};
}
# INPUT part
- if ($fwdfwsettings{'grp2'} eq 'ipfire' && $fwdfwsettings{$fwdfwsettings{'grp1'}} ne 'ORANGE'){
+ if ($fwdfwsettings{'grp2'} eq 'ipfire'){
$fwdfwsettings{'config'}=$configinput;
$fwdfwsettings{'chain'} = 'INPUTFW';
$maxkey=&General::findhasharraykey(\%configinputfw);
@@ -1512,7 +1526,7 @@ sub newrule
$checked{'USE_NAT'}{$fwdfwsettings{'USE_NAT'}} = 'CHECKED';
$selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected';
$selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected';
- $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
+ $selected{'ipfire tgt'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
$selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected';
#check if update and get values
if($fwdfwsettings{'updatefwrule'} eq 'on' || $fwdfwsettings{'copyfwrule'} eq 'on' && !$errormessage){
@@ -1526,7 +1540,7 @@ sub newrule
$fwdfwsettings{'ACTIVE'} = $hash{$key}[2];
$fwdfwsettings{'grp1'} = $hash{$key}[3];
$fwdfwsettings{$fwdfwsettings{'grp1'}} = $hash{$key}[4];
- $fwdfwsettings{'grp2'} = $hash{$key}[5];
+ $fwdfwsettings{'grp2'} = $hash{$key}[5];
$fwdfwsettings{$fwdfwsettings{'grp2'}} = $hash{$key}[6];
$fwdfwsettings{'USE_SRC_PORT'} = $hash{$key}[7];
$fwdfwsettings{'PROT'} = $hash{$key}[8];
@@ -1584,7 +1598,7 @@ sub newrule
$checked{'RATE_LIMIT'}{$fwdfwsettings{'RATE_LIMIT'}} = 'CHECKED';
$selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected';
$selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected';
- $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
+ $selected{'ipfire tgt'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
$selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected';
$selected{'dnat'}{$fwdfwsettings{'dnat'}} ='selected';
$selected{'snat'}{$fwdfwsettings{'snat'}} ='selected';
@@ -1753,16 +1767,16 @@ END
<table width='100%' border='0'>
<tr><td width='1%'><input type='radio' name='grp2' value='tgt_addr' checked></td><td width='60%' nowrap='nowrap'>$Lang::tr{'fwdfw targetip'}<input type='TEXT' name='tgt_addr' value='$fwdfwsettings{'tgt_addr'}' size='16' maxlength='18'><td width='1%'><input type='radio' name='grp2' id='ipfire' value='ipfire' $checked{'grp2'}{'ipfire'}></td><td><b>Firewall</b></td>
END
- print"<td align='right'><select name='ipfire' style='width:200px;'>";
- print "<option value='ALL' $selected{'ipfire'}{'ALL'}>$Lang::tr{'all'}</option>";
- print "<option value='GREEN' $selected{'ipfire'}{'GREEN'}>$Lang::tr{'green'} ($ifaces{'GREEN_ADDRESS'})</option>" if $ifaces{'GREEN_ADDRESS'};
- print "<option value='ORANGE' $selected{'ipfire'}{'ORANGE'}>$Lang::tr{'orange'} ($ifaces{'ORANGE_ADDRESS'})</option>" if (&Header::orange_used());
- print "<option value='BLUE' $selected{'ipfire'}{'BLUE'}>$Lang::tr{'blue'} ($ifaces{'BLUE_ADDRESS'})</option>"if (&Header::blue_used());
- print "<option value='RED1' $selected{'ipfire'}{'RED1'}>$Lang::tr{'red1'} ($redip)" if ($redip);
+ print"<td align='right'><select name='ipfire tgt' style='width:200px;'>";
+ print "<option value='ALL' $selected{'ipfire tgt'}{'ALL'}>$Lang::tr{'all'}</option>";
+ print "<option value='GREEN' $selected{'ipfire tgt'}{'GREEN'}>$Lang::tr{'green'} ($ifaces{'GREEN_ADDRESS'})</option>" if $ifaces{'GREEN_ADDRESS'};
+ print "<option value='ORANGE' $selected{'ipfire tgt'}{'ORANGE'}>$Lang::tr{'orange'} ($ifaces{'ORANGE_ADDRESS'})</option>" if (&Header::orange_used());
+ print "<option value='BLUE' $selected{'ipfire tgt'}{'BLUE'}>$Lang::tr{'blue'} ($ifaces{'BLUE_ADDRESS'})</option>"if (&Header::blue_used());
+ print "<option value='RED1' $selected{'ipfire tgt'}{'RED1'}>$Lang::tr{'red1'} ($redip)" if ($redip);
if (! -z "${General::swroot}/ethernet/aliases"){
foreach my $alias (sort keys %aliases)
{
- print "<option value='$alias' $selected{'ipfire'}{$alias}>$alias ($aliases{$alias}{'IPT'})</option>";
+ print "<option value='$alias' $selected{'ipfire tgt'}{$alias}>$alias ($aliases{$alias}{'IPT'})</option>";
}
}
print<<END;
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] BUG12265: firewall: iptables rules are being created in the wrong chain Fixes: #12265
2021-03-25 10:23 [PATCH] BUG12265: firewall: iptables rules are being created in the wrong chain Fixes: #12265 Alexander Marx
@ 2021-03-27 20:45 ` Peter Müller
0 siblings, 0 replies; 2+ messages in thread
From: Peter Müller @ 2021-03-27 20:45 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 8373 bytes --]
Hello Alex,
thank you for submitting this one.
Unfortunately, this patch is not behaving correctly on my testing machine (running Core Update 155):
A firewall rule with source = ORANGE and destination = firewall | GREEN IP is still displayed in the
wrong section in the WebUI. Worse, the destination IP address is silently omitted, resulting in
destination = any instead.
This faulty behaviour can be reproduced by creating and modifying firewall rules for arbitrary source
networks and arbitrary IP addresses of IPFire's interface as a destination.
Oddly enough, when accidentally leaving "destination IP" checked without entering one (why does the
WebUI even accept such rules?!), the rule is created in the correct section ("incoming firewall access"),
with it's target section displayed in blue and without an IP address - the IPFire machine, however, does
not even _have_ a BLUE interface.
Whatever goes wrong here seems to go seriously wrong.
Thanks, and best regards,
Peter Müller
> When creating a rule like Source:Orange and Target:green IPfire Interface,
> the rule was created in the forward instead of input chain.
>
> This patch sets correct chain and additionally checks
> if a single target ip (when set) is one of the ipfire interface ip addresses.
> If this is the case, the target is automatically changed to IPFIRE interface instead of single target ip.
> ---
> html/cgi-bin/firewall.cgi | 38 ++++++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
> index 532f99f91..c680eed1d 100644
> --- a/html/cgi-bin/firewall.cgi
> +++ b/html/cgi-bin/firewall.cgi
> @@ -2,7 +2,7 @@
> ###############################################################################
> # #
> # IPFire.org - A linux based firewall #
> -# Copyright (C) 2013 Alexander Marx <amarx(a)ipfire.org> #
> +# Copyright (C) 2021 Alexander Marx <amarx(a)ipfire.org> #
> # #
> # This program is free software: you can redistribute it and/or modify #
> # it under the terms of the GNU General Public License as published by #
> @@ -213,6 +213,7 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
> &General::readhasharray("$configfwdfw", \%configfwdfw);
> &General::readhasharray("$configinput", \%configinputfw);
> &General::readhasharray("$configoutgoing", \%configoutgoingfw);
> + &General::readhash("/var/ipfire/ethernet/settings", \%netsettings);
> my $maxkey;
> #Set Variables according to the JQuery code in protocol section
> if ($fwdfwsettings{'PROT'} eq 'TCP' || $fwdfwsettings{'PROT'} eq 'UDP')
> @@ -231,6 +232,19 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
> {
> $fwdfwsettings{'USESRV'} = 'ON';
> }
> + #Check if manual targetip is one of IPFire addresses
> + if ($fwdfwsettings{'grp2'} eq 'tgt_addr' && $fwdfwsettings{$fwdfwsettings{'grp2'}} eq $netsettings{'GREEN_ADDRESS'}){
> + $fwdfwsettings{'grp2'} = 'ipfire';
> + $fwdfwsettings{$fwdfwsettings{'grp2'}} = 'GREEN';
> + }
> + if ($fwdfwsettings{'grp2'} eq 'tgt_addr' && $fwdfwsettings{$fwdfwsettings{'grp2'}} eq $netsettings{'ORANGE_ADDRESS'}){
> + $fwdfwsettings{'grp2'} = 'ipfire';
> + $fwdfwsettings{$fwdfwsettings{'grp2'}} = 'ORANGE';
> + }
> + if ($fwdfwsettings{'grp2'} eq 'tgt_addr' && $fwdfwsettings{$fwdfwsettings{'grp2'}} eq $netsettings{'BLUE_ADDRESS'}){
> + $fwdfwsettings{'grp2'} = 'ipfire';
> + $fwdfwsettings{$fwdfwsettings{'grp2'}} = 'BLUE';
> + }
> $errormessage=&checksource;
> if(!$errormessage){&checktarget;}
> if(!$errormessage){&checkrule;}
> @@ -247,7 +261,7 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
> $errormessage=$Lang::tr{'fwdfw err same'};
> }
> # INPUT part
> - if ($fwdfwsettings{'grp2'} eq 'ipfire' && $fwdfwsettings{$fwdfwsettings{'grp1'}} ne 'ORANGE'){
> + if ($fwdfwsettings{'grp2'} eq 'ipfire'){
> $fwdfwsettings{'config'}=$configinput;
> $fwdfwsettings{'chain'} = 'INPUTFW';
> $maxkey=&General::findhasharraykey(\%configinputfw);
> @@ -1512,7 +1526,7 @@ sub newrule
> $checked{'USE_NAT'}{$fwdfwsettings{'USE_NAT'}} = 'CHECKED';
> $selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected';
> $selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected';
> - $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
> + $selected{'ipfire tgt'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
> $selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected';
> #check if update and get values
> if($fwdfwsettings{'updatefwrule'} eq 'on' || $fwdfwsettings{'copyfwrule'} eq 'on' && !$errormessage){
> @@ -1526,7 +1540,7 @@ sub newrule
> $fwdfwsettings{'ACTIVE'} = $hash{$key}[2];
> $fwdfwsettings{'grp1'} = $hash{$key}[3];
> $fwdfwsettings{$fwdfwsettings{'grp1'}} = $hash{$key}[4];
> - $fwdfwsettings{'grp2'} = $hash{$key}[5];
> + $fwdfwsettings{'grp2'} = $hash{$key}[5];
> $fwdfwsettings{$fwdfwsettings{'grp2'}} = $hash{$key}[6];
> $fwdfwsettings{'USE_SRC_PORT'} = $hash{$key}[7];
> $fwdfwsettings{'PROT'} = $hash{$key}[8];
> @@ -1584,7 +1598,7 @@ sub newrule
> $checked{'RATE_LIMIT'}{$fwdfwsettings{'RATE_LIMIT'}} = 'CHECKED';
> $selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected';
> $selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected';
> - $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
> + $selected{'ipfire tgt'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected';
> $selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected';
> $selected{'dnat'}{$fwdfwsettings{'dnat'}} ='selected';
> $selected{'snat'}{$fwdfwsettings{'snat'}} ='selected';
> @@ -1753,16 +1767,16 @@ END
> <table width='100%' border='0'>
> <tr><td width='1%'><input type='radio' name='grp2' value='tgt_addr' checked></td><td width='60%' nowrap='nowrap'>$Lang::tr{'fwdfw targetip'}<input type='TEXT' name='tgt_addr' value='$fwdfwsettings{'tgt_addr'}' size='16' maxlength='18'><td width='1%'><input type='radio' name='grp2' id='ipfire' value='ipfire' $checked{'grp2'}{'ipfire'}></td><td><b>Firewall</b></td>
> END
> - print"<td align='right'><select name='ipfire' style='width:200px;'>";
> - print "<option value='ALL' $selected{'ipfire'}{'ALL'}>$Lang::tr{'all'}</option>";
> - print "<option value='GREEN' $selected{'ipfire'}{'GREEN'}>$Lang::tr{'green'} ($ifaces{'GREEN_ADDRESS'})</option>" if $ifaces{'GREEN_ADDRESS'};
> - print "<option value='ORANGE' $selected{'ipfire'}{'ORANGE'}>$Lang::tr{'orange'} ($ifaces{'ORANGE_ADDRESS'})</option>" if (&Header::orange_used());
> - print "<option value='BLUE' $selected{'ipfire'}{'BLUE'}>$Lang::tr{'blue'} ($ifaces{'BLUE_ADDRESS'})</option>"if (&Header::blue_used());
> - print "<option value='RED1' $selected{'ipfire'}{'RED1'}>$Lang::tr{'red1'} ($redip)" if ($redip);
> + print"<td align='right'><select name='ipfire tgt' style='width:200px;'>";
> + print "<option value='ALL' $selected{'ipfire tgt'}{'ALL'}>$Lang::tr{'all'}</option>";
> + print "<option value='GREEN' $selected{'ipfire tgt'}{'GREEN'}>$Lang::tr{'green'} ($ifaces{'GREEN_ADDRESS'})</option>" if $ifaces{'GREEN_ADDRESS'};
> + print "<option value='ORANGE' $selected{'ipfire tgt'}{'ORANGE'}>$Lang::tr{'orange'} ($ifaces{'ORANGE_ADDRESS'})</option>" if (&Header::orange_used());
> + print "<option value='BLUE' $selected{'ipfire tgt'}{'BLUE'}>$Lang::tr{'blue'} ($ifaces{'BLUE_ADDRESS'})</option>"if (&Header::blue_used());
> + print "<option value='RED1' $selected{'ipfire tgt'}{'RED1'}>$Lang::tr{'red1'} ($redip)" if ($redip);
> if (! -z "${General::swroot}/ethernet/aliases"){
> foreach my $alias (sort keys %aliases)
> {
> - print "<option value='$alias' $selected{'ipfire'}{$alias}>$alias ($aliases{$alias}{'IPT'})</option>";
> + print "<option value='$alias' $selected{'ipfire tgt'}{$alias}>$alias ($aliases{$alias}{'IPT'})</option>";
> }
> }
> print<<END;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-27 20:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 10:23 [PATCH] BUG12265: firewall: iptables rules are being created in the wrong chain Fixes: #12265 Alexander Marx
2021-03-27 20:45 ` Peter Müller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox