diff mbox series

general-functions.pl: Update to fix bug #12428

Message ID	20201230093441.3296-1-ahb.ipfire@gmail.com
State	Accepted
Headers	From: Adolf Belka <ahb.ipfire@gmail.com> To: development@lists.ipfire.org Subject: [PATCH] general-functions.pl: Update to fix bug #12428 Date: Wed, 30 Dec 2020 10:34:41 +0100 Message-Id: <20201230093441.3296-1-ahb.ipfire@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Analysis: v=2.4 cv=Ff+JeLy6 c=1 sm=1 tr=0 ts=5fec49c3 a=N0UC3/faf55XGTeY5t7zSQ==:17 a=9+rZDBEiDlHhcck0kWbJtElFXBc=:19 a=zTNgK-yGK50A:10 a=x7bEGLp0ZPQA:10 a=6yxbeI8x3IIA:10 a=pGLkceISAAAA:8 a=Z_ibbLdTpEcLSuRw3zcA:9 country: NL(-0.00), ip: 212.54.42.166(-0.58)]; BROKEN_CONTENT_TYPE(1.50)[]; R_SPF_SOFTFAIL(0.00)[~all]; ARC_SIGNED(0.00)[lists.ipfire.org:s=202003rsa:i=1]; RCVD_COUNT_THREE(0.00)[4]; RCPT_COUNT_TWO(0.00)[2]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:33915, ipnet:212.54.32.0/20, country:NL]; R_DKIM_NA(0.00)[]; HAS_X_AS(0.00)[]; RCVD_IN_DNSWL_LOW(-0.10)[212.54.42.166:from]; TAGGED_FROM(0.00)[]; ARC_NA(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; BAYES_HAM(-3.00)[99.99%]; TAGGED_RCPT(0.00)[]; MIME_GOOD(-0.10)[text/plain]; RECEIVED_SPAMHAUS_PBL(0.00)[24.132.103.33:received]; TO_MATCH_ENVRCPT_SOME(0.00)[]; FREEMAIL_CC(0.00)[gmail.com]; RCVD_TLS_ALL(0.00)[]; DMARC_POLICY_SOFTFAIL(0.10)[gmail.com : No valid SPF, No valid DKIM,none] Precedence: list Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org>
Series	general-functions.pl: Update to fix bug #12428 \| general-functions.pl: Update to fix bug #12428

Commit Message

Adolf Belka Dec. 30, 2020, 9:34 a.m. UTC

  - Patch of general-functions.pl for implementation of fix provided
	by Bernhard Bitsch. Prevents spaces being put into hostnames
- Patch implemented into testbed system and confirmed working

Signed-off-by: Adolf Belka <ahb.ipfire@gmail.com>
---
 config/cfgroot/general-functions.pl | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Peter Müller Dec. 31, 2020, 10:46 a.m. UTC | #1

Hello Adolf,

thank you for working on this.

(In my point of view, the web interface lacks some input validation quite often, so it's
good to see some improvements here... :-) )

Your patch looks fine to me, except when it comes to underscores in host- or domain names.
While I have never seen them in production, Michael commited something allowing underscores
in 2016: https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=03306ff6a25238e01a1f2b39fbb929cf56615934

Thanks, and best regards,
Peter Müller


> - Patch of general-functions.pl for implementation of fix provided
> 	by Bernhard Bitsch. Prevents spaces being put into hostnames
> - Patch implemented into testbed system and confirmed working
> 
> Signed-off-by: Adolf Belka <ahb.ipfire@gmail.com>
> ---
>  config/cfgroot/general-functions.pl | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
> index 9be1e7708..318be2c01 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -640,13 +640,9 @@ sub validhostname
>  	if (length ($hostname) < 1 || length ($hostname) > 63) {
>  		return 0;}
>  	# Only valid characters are a-z, A-Z, 0-9 and -
> -	if ($hostname !~ /^[a-zA-Z0-9-\s]*$/) {
> -		return 0;}
> -	# First character can only be a letter or a digit
> -	if (substr ($hostname, 0, 1) !~ /^[a-zA-Z0-9]*$/) {
> -		return 0;}
> -	# Last character can only be a letter or a digit
> -	if (substr ($hostname, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
> +	# First and last character can only be letter or decimal digit
> +	# else letter, decimal digits and hyphen are allowed
> +	if ($hostname !~ /^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/) {
>  		return 0;}
>  	return 1;
>  }
>

Bernhard Bitsch Dec. 31, 2020, 11:56 a.m. UTC | #2

RFC1035 and RFC1101 demand the syntax proposed by me.
I didn't find an updated version of this, yet.

- Bernhard

> Gesendet: Donnerstag, 31. Dezember 2020 um 11:46 Uhr
> Von: "Peter Müller" <peter.mueller@ipfire.org>
> An: "Adolf Belka" <ahb.ipfire@gmail.com>
> Cc: development@lists.ipfire.org
> Betreff: Re: [PATCH] general-functions.pl: Update to fix bug #12428
>
> Hello Adolf,
> 
> thank you for working on this.
> 
> (In my point of view, the web interface lacks some input validation quite often, so it's
> good to see some improvements here... :-) )
> 
> Your patch looks fine to me, except when it comes to underscores in host- or domain names.
> While I have never seen them in production, Michael commited something allowing underscores
> in 2016: https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=03306ff6a25238e01a1f2b39fbb929cf56615934
> 
> Thanks, and best regards,
> Peter Müller
> 
> 
> > - Patch of general-functions.pl for implementation of fix provided
> > 	by Bernhard Bitsch. Prevents spaces being put into hostnames
> > - Patch implemented into testbed system and confirmed working
> > 
> > Signed-off-by: Adolf Belka <ahb.ipfire@gmail.com>
> > ---
> >  config/cfgroot/general-functions.pl | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
> > index 9be1e7708..318be2c01 100644
> > --- a/config/cfgroot/general-functions.pl
> > +++ b/config/cfgroot/general-functions.pl
> > @@ -640,13 +640,9 @@ sub validhostname
> >  	if (length ($hostname) < 1 || length ($hostname) > 63) {
> >  		return 0;}
> >  	# Only valid characters are a-z, A-Z, 0-9 and -
> > -	if ($hostname !~ /^[a-zA-Z0-9-\s]*$/) {
> > -		return 0;}
> > -	# First character can only be a letter or a digit
> > -	if (substr ($hostname, 0, 1) !~ /^[a-zA-Z0-9]*$/) {
> > -		return 0;}
> > -	# Last character can only be a letter or a digit
> > -	if (substr ($hostname, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
> > +	# First and last character can only be letter or decimal digit
> > +	# else letter, decimal digits and hyphen are allowed
> > +	if ($hostname !~ /^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/) {
> >  		return 0;}
> >  	return 1;
> >  }
> > 
>

Adolf Belka Dec. 31, 2020, 3:18 p.m. UTC | #3

Hi Peter,

Yes, when I was working on the patch I saw that the domain-name check includes an underscore. I therefore did a search and found references to several RFC's. What I found indicated that Domain names are allowed to have underscores but host names are not allowed to have underscores.

I found a lot of questions marks regarding FQDN's. Some information suggested that the hostname portion must not have underscores but the domain name portion could but there was also some information saying that if used as a FQDN all of it must not have underscores.

As the FQDN is so unclear, and I could not find an RFC related to FQDN's that mentioned the allowed syntax, I decided to leave the FQDN alone for now.

However for the hostname it seems clear that there must be no underscores.

Regards,
Adolf.


On 31/12/2020 12:56, Bernhard Bitsch wrote:
> RFC1035 and RFC1101 demand the syntax proposed by me.
> I didn't find an updated version of this, yet.
> 
> - Bernhard
> 
>> Gesendet: Donnerstag, 31. Dezember 2020 um 11:46 Uhr
>> Von: "Peter Müller" <peter.mueller@ipfire.org>
>> An: "Adolf Belka" <ahb.ipfire@gmail.com>
>> Cc: development@lists.ipfire.org
>> Betreff: Re: [PATCH] general-functions.pl: Update to fix bug #12428
>>
>> Hello Adolf,
>>
>> thank you for working on this.
>>
>> (In my point of view, the web interface lacks some input validation quite often, so it's
>> good to see some improvements here... :-) )
>>
>> Your patch looks fine to me, except when it comes to underscores in host- or domain names.
>> While I have never seen them in production, Michael commited something allowing underscores
>> in 2016: https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=03306ff6a25238e01a1f2b39fbb929cf56615934
>>
>> Thanks, and best regards,
>> Peter Müller
>>
>>
>>> - Patch of general-functions.pl for implementation of fix provided
>>> 	by Bernhard Bitsch. Prevents spaces being put into hostnames
>>> - Patch implemented into testbed system and confirmed working
>>>
>>> Signed-off-by: Adolf Belka <ahb.ipfire@gmail.com>
>>> ---
>>>   config/cfgroot/general-functions.pl | 10 +++-------
>>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
>>> index 9be1e7708..318be2c01 100644
>>> --- a/config/cfgroot/general-functions.pl
>>> +++ b/config/cfgroot/general-functions.pl
>>> @@ -640,13 +640,9 @@ sub validhostname
>>>   	if (length ($hostname) < 1 || length ($hostname) > 63) {
>>>   		return 0;}
>>>   	# Only valid characters are a-z, A-Z, 0-9 and -
>>> -	if ($hostname !~ /^[a-zA-Z0-9-\s]*$/) {
>>> -		return 0;}
>>> -	# First character can only be a letter or a digit
>>> -	if (substr ($hostname, 0, 1) !~ /^[a-zA-Z0-9]*$/) {
>>> -		return 0;}
>>> -	# Last character can only be a letter or a digit
>>> -	if (substr ($hostname, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
>>> +	# First and last character can only be letter or decimal digit
>>> +	# else letter, decimal digits and hyphen are allowed
>>> +	if ($hostname !~ /^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/) {
>>>   		return 0;}
>>>   	return 1;
>>>   }
>>>
>>

Peter Müller Dec. 31, 2020, 6:21 p.m. UTC | #4

Hello Adolf, hello Bernhard, hello *,

thank you for your replies.

I see, this patch makes sense to me then. :-)

Reviewed-by: Peter Müller <peter.mueller@ipfire.org>

Thanks, and best regards,
Peter Müller


> Hi Peter,
> 
> Yes, when I was working on the patch I saw that the domain-name check includes an underscore. I therefore did a search and found references to several RFC's. What I found indicated that Domain names are allowed to have underscores but host names are not allowed to have underscores.
> 
> I found a lot of questions marks regarding FQDN's. Some information suggested that the hostname portion must not have underscores but the domain name portion could but there was also some information saying that if used as a FQDN all of it must not have underscores.
> 
> As the FQDN is so unclear, and I could not find an RFC related to FQDN's that mentioned the allowed syntax, I decided to leave the FQDN alone for now.
> 
> However for the hostname it seems clear that there must be no underscores.
> 
> Regards,
> Adolf.
> 
> 
> On 31/12/2020 12:56, Bernhard Bitsch wrote:
>> RFC1035 and RFC1101 demand the syntax proposed by me.
>> I didn't find an updated version of this, yet.
>>
>> - Bernhard
>>
>>> Gesendet: Donnerstag, 31. Dezember 2020 um 11:46 Uhr
>>> Von: "Peter Müller" <peter.mueller@ipfire.org>
>>> An: "Adolf Belka" <ahb.ipfire@gmail.com>
>>> Cc: development@lists.ipfire.org
>>> Betreff: Re: [PATCH] general-functions.pl: Update to fix bug #12428
>>>
>>> Hello Adolf,
>>>
>>> thank you for working on this.
>>>
>>> (In my point of view, the web interface lacks some input validation quite often, so it's
>>> good to see some improvements here... :-) )
>>>
>>> Your patch looks fine to me, except when it comes to underscores in host- or domain names.
>>> While I have never seen them in production, Michael commited something allowing underscores
>>> in 2016: https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=03306ff6a25238e01a1f2b39fbb929cf56615934
>>>
>>> Thanks, and best regards,
>>> Peter Müller
>>>
>>>
>>>> - Patch of general-functions.pl for implementation of fix provided
>>>>     by Bernhard Bitsch. Prevents spaces being put into hostnames
>>>> - Patch implemented into testbed system and confirmed working
>>>>
>>>> Signed-off-by: Adolf Belka <ahb.ipfire@gmail.com>
>>>> ---
>>>>   config/cfgroot/general-functions.pl | 10 +++-------
>>>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
>>>> index 9be1e7708..318be2c01 100644
>>>> --- a/config/cfgroot/general-functions.pl
>>>> +++ b/config/cfgroot/general-functions.pl
>>>> @@ -640,13 +640,9 @@ sub validhostname
>>>>       if (length ($hostname) < 1 || length ($hostname) > 63) {
>>>>           return 0;}
>>>>       # Only valid characters are a-z, A-Z, 0-9 and -
>>>> -    if ($hostname !~ /^[a-zA-Z0-9-\s]*$/) {
>>>> -        return 0;}
>>>> -    # First character can only be a letter or a digit
>>>> -    if (substr ($hostname, 0, 1) !~ /^[a-zA-Z0-9]*$/) {
>>>> -        return 0;}
>>>> -    # Last character can only be a letter or a digit
>>>> -    if (substr ($hostname, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
>>>> +    # First and last character can only be letter or decimal digit
>>>> +    # else letter, decimal digits and hyphen are allowed
>>>> +    if ($hostname !~ /^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/) {
>>>>           return 0;}
>>>>       return 1;
>>>>   }
>>>>
>>>

diff mbox series

Patch

diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
index 9be1e7708..318be2c01 100644
--- a/config/cfgroot/general-functions.pl
+++ b/config/cfgroot/general-functions.pl
@@ -640,13 +640,9 @@  sub validhostname
 	if (length ($hostname) < 1 || length ($hostname) > 63) {
 		return 0;}
 	# Only valid characters are a-z, A-Z, 0-9 and -
-	if ($hostname !~ /^[a-zA-Z0-9-\s]*$/) {
-		return 0;}
-	# First character can only be a letter or a digit
-	if (substr ($hostname, 0, 1) !~ /^[a-zA-Z0-9]*$/) {
-		return 0;}
-	# Last character can only be a letter or a digit
-	if (substr ($hostname, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
+	# First and last character can only be letter or decimal digit
+	# else letter, decimal digits and hyphen are allowed
+	if ($hostname !~ /^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/) {
 		return 0;}
 	return 1;
 }