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] fwhosts.cgi: Fix check to limit amount of ports in custom
 service groups.
Date: Thu, 15 Jul 2021 16:45:38 +0100
Message-ID: <E8239A13-9ED4-43BD-B807-7EE95FE3F9F6@ipfire.org>
In-Reply-To: <20210715100737.3733-1-stefan.schantl@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0108611956144917645=="
List-Id: <development.lists.ipfire.org>

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

Hello,

I agree with the fix, but it would be better if we could hide this from the u=
ser.

I am not sure whether people have reported running into this, but the more se=
nsible approach would have been to simply generate two iptables rules if ther=
e are more than the maximum number of ports being used in a group.

Since this has not been reported yet, I guess we will just leave it as it is.

Best,
-Michael

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 15 Jul 2021, at 11:07, Stefan Schantl <stefan.schantl(a)ipfire.org> wrot=
e:
>=20
> iptables multiport only supports up to 15 elements for each protocol (TCP o=
r UDP).
> That can be single ports or portranges (they count doubble).
>=20
> This commit extends the check to calculate the amount of used TCP and/or
> UDP ports of all existing entries in a group, by increasing the amount
> for the service which should be added.
>=20
> If the amount of ports for TCP or UDP ports become greater than the
> limit of 15 the error message will be displayed.
>=20
> Fixes #11323.
>=20
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> html/cgi-bin/fwhosts.cgi | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>=20
> diff --git a/html/cgi-bin/fwhosts.cgi b/html/cgi-bin/fwhosts.cgi
> index 35611ac08..f6c7227ce 100644
> --- a/html/cgi-bin/fwhosts.cgi
> +++ b/html/cgi-bin/fwhosts.cgi
> @@ -818,10 +818,28 @@ if ($fwhostsettings{'ACTION'} eq 'saveservicegrp')
> 			}
> 		}
> 	}
> -	if ($tcpcounter > 14){
> +
> +	# Loop through the hash of configured services.
> +	foreach my $key (keys %customservice) {
> +		# Assign nice human-readable values.
> +		my $service_name =3D $customservice{$key}[0];
> +		my $service_port =3D $customservice{$key}[1];
> +		my $service_proto =3D $customservice{$key}[2];
> +
> +		# Skip services unless the processed one has found.
> +		next unless $service_name eq $fwhostsettings{'CUST_SRV'};
> +
> +		# Increase the counters.
> +		$tcpcounter++ if $service_proto eq 'TCP';
> +		$tcpcounter++ if $service_proto eq 'TCP' && $service_port =3D~ m/:/i;
> +		$udpcounter++ if $service_proto eq 'UDP';
> +		$udpcounter++ if $service_proto eq 'UDP' && $service_port =3D~ m/:/i;
> +	}
> +
> +	if ($tcpcounter > 15) {
> 		$errormessage=3D$Lang::tr{'fwhost err maxservicetcp'};
> 	}
> -	if ($udpcounter > 14){
> +	if ($udpcounter > 15) {
> 		$errormessage=3D$Lang::tr{'fwhost err maxserviceudp'};
> 	}
> 	$tcpcounter=3D0;
> --=20
> 2.30.2
>=20


--===============0108611956144917645==--