I am porting the old ipcop addon 'Banish' to IPFire and during testing have found a problem in general-functions.pl which causes validfqdn to return 1 when testing valid and invalid ip addresses when it should return 0.
As this is not a problem with IPCop 2 a comparison of the validfqdn section in IPFire's general-functions.pl shows a missing segment that checks the TLD can only be a-z or A-Z.
Applying the patch below to general-functions.pl corrects the problem with my Banish port and I haven't found any problems affecting IPFire's operation.
Regards
Rob
--- /tmp/general-functions.pl 2018-09-19 10:32:37.000000000 +0100 +++ /tmp/general-functions.pl.new 2018-12-12 22:13:37.394653609 +0000 @@ -666,9 +666,13 @@ }
sub validfqdn +# modified to add addition test to confirm TL is only a-z or A-Z +# as per ipcop rwb 12/12/18 + { my $part; - + my $tld; + # Checks a fully qualified domain name against RFC1035 my $fqdn = $_[0]; my @parts = split (/./, $fqdn); # Split hostname at the '.' @@ -689,7 +693,14 @@ # Last character can only be a letter or a digit if (substr ($part, -1, 1) !~ /^[a-zA-Z0-9]*$/) { return 0;} - } + # Store for additional check on TLD + $tld = $part; + } + + # TLD valid characters are a-z, A-Z + if ($tld !~ /^[a-zA-Z]*$/) { + return 0; + } return 1; }
Hello Bob,
Thank you for submitting your patch.
On 12 Dec 2018, at 22:48, Bob Brewer ipfire-devel@grantura.co.uk wrote:
I am porting the old ipcop addon 'Banish' to IPFire and during testing have found a problem in general-functions.pl which causes validfqdn to return 1 when testing valid and invalid ip addresses when it should return 0.
What does the add-on do? I could not find an old version for IPCop on the Internet…
As this is not a problem with IPCop 2 a comparison of the validfqdn section in IPFire's general-functions.pl shows a missing segment that checks the TLD can only be a-z or A-Z.
What requires this change?
I do not know of any ASCII TLDs that have numbers, but there is no reason that they can’t in the future. Furthermore, we have some non-ASCII TLDs which will have to be encoded into ASCII using the puny-codes. That will result in something like this:
XN--FHBEI XN--FIQ228C5HS XN--FIQ64B XN--FIQS8S XN—FIQZ9S
This is just a couple of random TLDs I picked from here:
http://data.iana.org/TLD/tlds-alpha-by-domain.txt
I assume that those will no longer be usable after your patch. Can you confirm that?
Best, -Michael
Applying the patch below to general-functions.pl corrects the problem with my Banish port and I haven't found any problems affecting IPFire's operation.
Regards
Rob
--- /tmp/general-functions.pl 2018-09-19 10:32:37.000000000 +0100 +++ /tmp/general-functions.pl.new 2018-12-12 22:13:37.394653609 +0000 @@ -666,9 +666,13 @@ }
sub validfqdn +# modified to add addition test to confirm TL is only a-z or A-Z +# as per ipcop rwb 12/12/18
{ my $part;
my $tld;- # Checks a fully qualified domain name against RFC1035 my $fqdn = $_[0]; my @parts = split (/./, $fqdn); # Split hostname at the '.'
@@ -689,7 +693,14 @@ # Last character can only be a letter or a digit if (substr ($part, -1, 1) !~ /^[a-zA-Z0-9]*$/) { return 0;}
}
# Store for additional check on TLD$tld = $part;}# TLD valid characters are a-z, A-Zif ($tld !~ /^[a-zA-Z]*$/) {return 0;} return 1;}
Michael Tremer wrote:
Hi Michael,
Thank you for your reply.
Thank you for submitting your patch.
On 12 Dec 2018, at 22:48, Bob Brewer ipfire-devel@grantura.co.uk wrote:
I am porting the old ipcop addon 'Banish' to IPFire and during testing have found a problem in general-functions.pl which causes validfqdn to return 1 when testing valid and invalid ip addresses when it should return 0.
What does the add-on do? I could not find an old version for IPCop on the Internet…
Banish is a well written perl based IPCop addon that allows you to maintain a blocklist consisting of fqdn, mac address, ip range, CIDR and domain formats. It was written by Sid McLaurin and was last maintained about 10 years ago when IPCop was at version 1.4.18. I cannot currently find any trace of Sid McLauren and assume it has been abandoned and his servers are no longer in existence.
The program generates CUSTOMFORWARD, CUSTOMINPUT and CUSTOMOUTPUT iptables from the maintained GUI based blocklist.
I have a copy of Banish 1.4.7 which dates back to 2008 and have ported it to the last version of IPCop although I haven't produced an install script for it. If you would like a copy of Banish 1.4.7 I would be pleased to send it to you.
As this is not a problem with IPCop 2 a comparison of the validfqdn section in IPFire's general-functions.pl shows a missing segment that checks the TLD can only be a-z or A-Z.
What requires this change?
Banish checks new entries through the web interface to make sure that they confirm to formats stated above and gives error messages if they are not correct. In testing I was finding that invalid ip addresses (such as 192.168.0.256) were being validated by validfqdn as a fqdn and allowing it to be incorrectly used in the block list.
I do not know of any ASCII TLDs that have numbers, but there is no reason that they can’t in the future. Furthermore, we have some non-ASCII TLDs which will have to be encoded into ASCII using the puny-codes. That will result in something like this:
XN--FHBEI XN--FIQ228C5HS XN--FIQ64B XN--FIQS8S XN—FIQZ9S
This is just a couple of random TLDs I picked from here:
There are some weird domains in there and many are new to me so maybe my patch needs to be updated to allow for these but I notice that there aren't any purely numeric TLDs in that list.
I assume that those will no longer be usable after your patch. Can you confirm that?
MY patch brings IPfire's validfqdn to be the same as the one in the latest IPCop version. It looks like we need to add a '-' to the regex string to make comply with the domains listed above.
I have a hacked version of Banish almost working on IPFire but it still needs some more work to make an install script. I think it would make a good addition to IPFire and will make it available if you are interested.
Regards
Rob
On Thu, Dec 13, 2018 at 09:06:06PM +0000, Bob Brewer (ipfire-devel@grantura.co.uk) wrote:
There are some weird domains in there and many are new to me so maybe my patch needs to be updated to allow for these but I notice that there aren't any purely numeric TLDs in that list.
No, and I don't expect such in the next new gtld round either, though some with embedded numbers might appear.
I assume that those will no longer be usable after your patch. Can you confirm that?
MY patch brings IPfire's validfqdn to be the same as the one in the latest IPCop version. It looks like we need to add a '-' to the regex string to make comply with the domains listed above.
Dash (-) should be allowed except in the beginning and end (that applies to other parts of the fqdn, too, but there pure numeric ones can also occur).
It would also be good to think where non-ASCII names should be allowed without requiring user to convert them to punycode first.
-
Bob Brewer -
Michael Tremer -
Tapani Tarvainen