From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 04/12] rules.pl: Destroy all ipset lists on rule reload.
Date: Wed, 16 Feb 2022 10:45:05 +0000
Message-ID: <56E22B21-4AF6-4FD9-BFAD-B4098B4BE646@ipfire.org>
In-Reply-To: <556eea03-d196-7ac4-b1f7-aad09939918b@tfitzgeorge.me.uk>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0405804688894772575=="
List-Id: <development.lists.ipfire.org>

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

Hello Tim,

> On 15 Feb 2022, at 19:28, Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk> wrote:
>=20
> Hello,
>=20
> I'm concerned about this as well.  Depending on when it does the ipset dest=
roy it may be OK (for example as part of shutting down the system or prior to=
 rebuilding the firewall from scratch, as in these cases it either won't matt=
er or the OP blocklist ipsets will be reloaded), but in general I would consi=
der it a bad idea to delete all the ipsets whether or not you 'own' them - ea=
ch 'package' should only touch it's own 'property', while this just deletes a=
ll the ipsets regardless.

This is quite hard to implement though. We could in theory iterate over all p=
ossible country codes and try to delete all sets, but that seems to be a very=
 slow and not elegant solution to the problem.

> Having said that, I think it will probably be alright as according to the d=
ocumentation ipset destroy won't delete lists which have references to them, =
and the IP blocklist ipsets should always have references.

This is good for us though. If we can consider the =E2=80=9Cdestroy=E2=80=9D =
command to be more of a cleanup and it is safe to call it, then we should not=
 run into any trouble here.

@Stefan: Can you confirm that any sets that are still referenced elsewhere wo=
n=E2=80=99t be destroyed and that there is no ugly output that could alarm an=
yone?

-Michael

>=20
> Tim
>=20
> On 15/02/2022 12:41, Michael Tremer wrote:
>> Hello,
>>=20
>> Looking at the other patchset that implements IP blocklists, could this in=
terfere with this in any way?
>>=20
>> -Michael
>>=20
>>> On 14 Feb 2022, at 18:42, Stefan Schantl <stefan.schantl(a)ipfire.org> wr=
ote:
>>>=20
>>> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
>>> ---
>>> config/firewall/rules.pl | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>=20
>>> diff --git a/config/firewall/rules.pl b/config/firewall/rules.pl
>>> index f685d08a7..da01b8775 100644
>>> --- a/config/firewall/rules.pl
>>> +++ b/config/firewall/rules.pl
>>> @@ -31,6 +31,7 @@ require "${General::swroot}/location-functions.pl";
>>> my $DEBUG =3D 0;
>>>=20
>>> my $IPTABLES =3D "iptables --wait";
>>> +my $IPSET =3D "ipset";
>>>=20
>>> # iptables chains
>>> my $CHAIN_INPUT           =3D "INPUTFW";
>>> @@ -114,6 +115,9 @@ sub main {
>>> 	# Flush all chains.
>>> 	&flush();
>>>=20
>>> +	# Destroy all existing ipsets.
>>> +	run("$IPSET destroy");
>>> +
>>> 	# Prepare firewall rules.
>>> 	if (! -z  "${General::swroot}/firewall/input"){
>>> 		&buildrules(\%configinputfw);
>>> --=20
>>> 2.30.2
>>>=20
>>=20
>=20


--===============0405804688894772575==--