From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Schantl <stefan.schantl@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] BUG12301: Iptables =?utf-8?q?=E2=80=9Chost/network_?=
 =?utf-8?b?4oCYbm9uZeKAmSBub3QgZm91bmTigJ0=?=
Date: Fri, 16 Jul 2021 16:56:59 +0200
Message-ID: <2a53ccfb95a762dd2c3617e39b5eb340b1226ae9.camel@ipfire.org>
In-Reply-To: <65b773ad-1fb7-24a0-9a06-4d1ce6428244@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1840288241696907082=="
List-Id: <development.lists.ipfire.org>

--===============1840288241696907082==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Michael,

the patch looks fine to me too.

Technically the solution for "none" will work pretty fine.
>=20
>=20
> Am 12.04.21 um 12:23 schrieb Michael Tremer:
> > Hello,
> >=20
> > > On 12 Apr 2021, at 11:23, Alexander Marx <
> > > alexander.marx(a)ipfire.org> wrote:
> > >=20
> > >=20
> > >=20
> > > Am 12.04.21 um 12:18 schrieb Michael Tremer:
> > > > Hi,
> > > >=20
> > > > > On 12 Apr 2021, at 07:05, Alexander Marx <
> > > > > alexander.marx(a)ipfire.org> wrote:
> > > > >=20
> > > > > Fixes: #12301
> > > > >=20
> > > > > When using hosts with MAC-addresses in a hostgroup,
> > > > > the rule won't be generated if those hosts are selected as
> > > > > target.
> > > > > There is a hint but due to a wrong hashparameter the hint was
> > > > > not shown.
> > > > >=20
> > > > > With this patch the hint is shown again.
> > > > > Additionally the rule is skipped when rules.pl creates rules.
> > > > >=20
> > > > > There are no bootmessages with failed target "none" anymore.
> > > > > ---
> > > > > config/firewall/firewall-lib.pl | 4 ++--
> > > > > html/cgi-bin/firewall.cgi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 2 +-
> > > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > > >=20
> > > > > diff --git a/config/firewall/firewall-lib.pl
> > > > > b/config/firewall/firewall-lib.pl
> > > > > index bc0b30ca5..e7ec30ae0 100644
> > > > > --- a/config/firewall/firewall-lib.pl
> > > > > +++ b/config/firewall/firewall-lib.pl
> > > > > @@ -2,7 +2,7 @@
> > > > > #############################################################
> > > > > ##################
> > > > > #=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=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=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=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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #
> > > > > # IPFire.org - A linux based
> > > > > firewall=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=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=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=A0 #
> > > > > -# Copyright (C) 2013 Alexander Marx
> > > > > <amarx(a)ipfire.org>=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 #
> > > > > +# Copyright (C) 2021 Alexander Marx
> > > > > <amarx(a)ipfire.org>=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=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=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=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=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=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=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=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #
> > > > > # This program is free software: you can redistribute it
> > > > > and/or modify=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #
> > > > > # it under the terms of the GNU General Public License as
> > > > > published by=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #
> > > > > @@ -315,7 +315,7 @@ sub get_addresses
> > > > > =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=A0=C2=A0=C2=A0foreach my $grp (sort {$a <=3D> $b} keys
> > > > > %customgrp) {
> > > > > =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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0if ($customgrp{$grp}[0] eq $value) {
> > > > > =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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0my @address =3D
> > > > > &get_address($customgrp{$grp}[3], $customgrp{$grp}[2],
> > > > > $type);
> > > > > -
> > > > > +=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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0next if ($address[0][0] eq
> > > > > 'none');
> > > > A comment for these rather obscure things would not hurt, but
> > > > technically I agree with how this is solved.
> > > >=20
> > > >=20
> > > >=20
> > > > > =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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (@address) {
> > > > > =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=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=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=A0push(@addresses,
> > > > > @address);
> > > > > =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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > > > diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-
> > > > > bin/firewall.cgi
> > > > > index 1483e779f..b0851dd3e 100644
> > > > > --- a/html/cgi-bin/firewall.cgi
> > > > > +++ b/html/cgi-bin/firewall.cgi
> > > > > @@ -592,7 +592,7 @@ sub checktarget
> > > > > =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=A0=C2=A0=C2=A0&General::readhasharray("$confighost",
> > > > > \%customhost);
> > > > > =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=A0=C2=A0=C2=A0foreach my $grpkey (sort keys %customgrp){
> > > > > =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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0foreach my $hostkey (sort keys
> > > > > %customhost){
> > > > > -=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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0if ($customgrp{$grpkey}[2] eq
> > > > > $customhost{$hostkey}[0] && $customgrp{$grpkey}[2] eq
> > > > > $fwdfwsettings{$fwdfwsettings{'grp2'}} &&
> > > > > $customhost{$hostkey}[1] eq 'mac'){
> > > > > +=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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0if ($customgrp{$grpkey}[2] eq
> > > > > $customhost{$hostkey}[0] && $customgrp{$grpkey}[0] eq
> > > > > $fwdfwsettings{$fwdfwsettings{'grp2'}} &&
> > > > > $customhost{$hostkey}[1] eq 'mac'){
> > > > What has changed here?
> > > only the hashfield
> > >=20
> > > $customgrp{$grpkey}[0] (was 2 before)
> > Yes I saw that, but what does that change?
> >=20
> > -Michael
> >=20
> > P.S. Do not forget to CC the list
> Thats the indicator to show the Hint. When someone has hostgroups
> with=20
> macaddresses as target, the hint is shown.
> Because this Value was 2 instead of 0, the hint was never shown....


Previously the check was performed against the hostgroup name which
never would contain a valid MAC address.

With the changed value now the check for a MAC address will be
performed on each configured host inside the group what is what we
want.
>=20
> >=20
> > > > > =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=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=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=A0$hint=3D$Lang::tr{'fwdf
> > > > > w hint mac'};
> > > > > =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=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=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=A0return $hint;
> > > > > =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=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > > > =E2=80=94
> > > > > 2.25.1
> > > > >=20
> > > > Best,
> > > > -Michael
>=20

Acked-by: Stefan Schantl <stefan.schantl(a)ipfire.org>

Best regards,

-Stefan


--===============1840288241696907082==--