Message ID | pus370$nkt$1@tuscan3.grantura.co.uk |
---|---|
State | Dropped |
Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (mail01.i.ipfire.org [172.28.1.200]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail01.ipfire.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by web07.i.ipfire.org (Postfix) with ESMTPS id 2642A8ABD89 for <patchwork@web07.i.ipfire.org>; Wed, 12 Dec 2018 22:48:13 +0000 (GMT) Received: from mail01.i.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id 2E9EC21B9E57; Wed, 12 Dec 2018 22:48:12 +0000 (GMT) Received: from tuscan3.grantura.co.uk (grantura.co.uk [217.169.17.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mail01.ipfire.org (Postfix) with ESMTPS id A941A21B9E4E for <development@lists.ipfire.org>; Wed, 12 Dec 2018 22:48:07 +0000 (GMT) Received: from tuscan3.grantura.co.uk (localhost [127.0.0.1]) by tuscan3.grantura.co.uk (8.15.2/8.15.2/Debian-12) with ESMTP id wBCMm0Li024884 for <development@lists.ipfire.org>; Wed, 12 Dec 2018 22:48:00 GMT Received: (from news@localhost) by tuscan3.grantura.co.uk (8.15.2/8.15.2/Submit) id wBCMm0tW024878 for development@lists.ipfire.org; Wed, 12 Dec 2018 22:48:00 GMT To: development@lists.ipfire.org From: Bob Brewer <ipfire-devel@grantura.co.uk> Newsgroups: grantura.local.ipfire-devel Subject: validfqdn Date: Wed, 12 Dec 2018 22:48 +0000 Organization: Megadodo Publications Lines: 48 Message-ID: <pus370$nkt$1@tuscan3.grantura.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7Bit User-Agent: KNode/4.14.10 X-Spam-Status: No, score=2.49 X-Rspamd-Server: mail01.i.ipfire.org Authentication-Results: mail01.ipfire.org X-Spamd-Result: default: False [2.49 / 11.00]; ARC_NA(0.00)[]; MX_INVALID(0.50)[greylisted]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MV_CASE(0.50)[]; MIME_GOOD(-0.10)[text/plain]; PREVIOUSLY_DELIVERED(0.00)[development@lists.ipfire.org]; TO_DN_NONE(0.00)[]; AUTH_NA(1.00)[]; RCPT_COUNT_ONE(0.00)[1]; HAS_ORG_HEADER(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; MIME_TRACE(0.00)[0:+]; RCVD_IN_DNSWL_MED(-0.20)[29.17.169.217.list.dnswl.org : 127.0.6.2]; DMARC_NA(0.00)[grantura.co.uk]; IP_SCORE(-0.01)[country: GB(-0.07)]; R_SPF_NA(0.00)[]; FORGED_SENDER(0.30)[ipfire-devel@grantura.co.uk,news@tuscan3.grantura.co.uk]; R_DKIM_NA(0.00)[]; CTE_CASE(0.50)[]; ASN(0.00)[asn:20712, ipnet:217.169.0.0/19, country:GB]; FROM_NEQ_ENVFROM(0.00)[ipfire-devel@grantura.co.uk,news@tuscan3.grantura.co.uk]; RCVD_TLS_LAST(0.00)[] X-Spam-Level: ** X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <https://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <https://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <https://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
Series |
validfqdn
|
|
Commit Message
Rob Brewer
Dec. 13, 2018, 9:48 a.m. UTC
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
Comments
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.
--- /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; }