From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bernhard Bitsch <bbitsch@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] fwhosts.cgi: Fix check to limit amount of ports in custom
 service groups.
Date: Fri, 16 Jul 2021 09:02:05 +0200
Message-ID: <d7b6e8dd-d4a1-d2ac-5fec-71e81909bc78@ipfire.org>
In-Reply-To: <E8239A13-9ED4-43BD-B807-7EE95FE3F9F6@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============8207876375820755791=="
List-Id: <development.lists.ipfire.org>

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

I agree with Michael's statement.
A set of ports greater than 15 should be very unusual and should be=20
structured logically further.
A error messages reminds to this case.

Best,
Bernhard

Reviewed-by: Bernhard Bitsch <bbitsch(a)ipfire.org>

Am 15.07.2021 um 17:45 schrieb Michael Tremer:
> Hello,
>=20
> I agree with the fix, but it would be better if we could hide this from the=
 user.
>=20
> I am not sure whether people have reported running into this, but the more =
sensible approach would have been to simply generate two iptables rules if th=
ere are more than the maximum number of ports being used in a group.
>=20
> Since this has not been reported yet, I guess we will just leave it as it i=
s.
>=20
> Best,
> -Michael
>=20
> Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>
>=20
>> On 15 Jul 2021, at 11:07, Stefan Schantl <stefan.schantl(a)ipfire.org> wro=
te:
>>
>> iptables multiport only supports up to 15 elements for each protocol (TCP =
or UDP).
>> That can be single ports or portranges (they count doubble).
>>
>> 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.
>>
>> If the amount of ports for TCP or UDP ports become greater than the
>> limit of 15 the error message will be displayed.
>>
>> Fixes #11323.
>>
>> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
>> ---
>> html/cgi-bin/fwhosts.cgi | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> 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

--===============8207876375820755791==--