validfqdn

  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
  

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-Z
> +        if ($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:
> 
>   http://data.iana.org/TLD/tlds-alpha-by-domain.txt
> 
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:

> >   http://data.iana.org/TLD/tlds-alpha-by-domain.txt
> > 
> 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.