From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] BUG10963: implement a better email verification
Date: Mon, 16 Nov 2015 14:12:19 +0000
Message-ID: <1447683139.2699.153.camel@ipfire.org>
In-Reply-To: <1447671201-9157-1-git-send-email-alexander.marx@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============6359621746186202142=="
List-Id: <development.lists.ipfire.org>

--===============6359621746186202142==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

Hi,

okay, this looks better, but I still would like to request a change:

On Mon, 2015-11-16 at 11:53 +0100, Alexander Marx wrote:
> We now check all allowed chars in the address before the @ sign.
> 
> To check the fqdn of an email the function validfqdn has been adapted
> as
> well. Here a valid domain part is for example: user(a)ipfire or 
> user(a)localhost.localdomain
> 
> Signed-off-by: Alexander Marx <alexander.marx(a)ipfire.org>
> ---
>  config/cfgroot/general-functions.pl | 30 +++++++++++++++++++--------
> ---
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/config/cfgroot/general-functions.pl
> b/config/cfgroot/general-functions.pl
> index 2b5cd19..564a904 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -653,9 +653,6 @@ sub validfqdn
>  	# Checks a fully qualified domain name against RFC1035
>          my $fqdn = $_[0];
>  	my @parts = split (/\./, $fqdn);	# Split hostname at
> the '.'
> -	if (scalar(@parts) < 2) {		# At least two
> parts should
> -		return 0;}			# exist in a FQDN
> -						# (i.e.
> hostname.domain)
>  	foreach $part (@parts) {
>  		# Each part should be at least one character in
> length
>  		# but no more than 63 characters
> @@ -747,14 +744,25 @@ sub ipcidr2msk {
>  }

This function above is called validfqdn and is supposed (according to
the comment) to check for a FQDN as defined in RFC1035. You change
changes that. Therefore I think this function should be left as it is
so other things that use this function don't break.

>  sub validemail {
> -    my $mail = shift;
> -    return 0 if ( $mail !~ /^[0-9a-zA-Z\.\-\_]+\@[0-9a-zA-Z\.\-]+$/
> );
> -    return 0 if ( $mail =~ /^[^0-9a-zA-Z]|[^0-9a-zA-Z]$/);
> -    return 0 if ( $mail !~ /([0-9a-zA-Z]{1})\@./ );
> -    return 0 if ( $mail !~ /.\@([0-9a-zA-Z]{1})/ );
> -    return 0 if ( $mail =~ /.\.\-.|.\-\..|.\.\..|.\-\-./g );
> -    return 0 if ( $mail =~ /.\.\_.|.\-\_.|.\_\..|.\_\-.|.\_\_./g );
> -    return 0 if ( $mail !~ /\.([a-zA-Z]{2,4})$/ );
> +    my $address = shift;
> +    my @parts = split( /\@/, $address );
> +    my $count=@parts;
> +
> +    #check if we have one part before and after '@'
> +    return 0 if ( $count != 2 );
> +
> +    #check if one of the parts starts or ends with a dot
> +    return 0 if ( substr($parts[0],0,1) eq '.' );
> +    return 0 if ( substr($parts[0],-1,1) eq '.' );
> +    return 0 if ( substr($parts[1],0,1) eq '.' );
> +    return 0 if ( substr($parts[1],-1,1) eq '.' );
> +
> +    #check first addresspart (before '@' sign)
> +    return 0 if  ( $parts[0] !~ m/^[a-zA-Z0-9\.!\-\+#]+$/ );
> +
> +    #check second addresspart (after '@' sign)
> +    return 0 if  ( !&validfqdn( $parts[1] ) );

If the FQDN function is not amended, the domain name should just be
checked for invalid characters.

> +
>      return 1;
>  }
>  

-Michael

--===============6359621746186202142==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjEKCmlRSWNCQUFC
Q2dBR0JRSldTZVJEQUFvSkVJQjU4UDl2a0FrSGRXVVFBS09mVVNGdUNZU1hpYk5qd1FPWXhiYzEK
cEYvZXYxcGkzRXh1Ly81TUsxdGI3TDE3UytsMUVJKzlsMjdxc0xRSGxBV1h0ekc2SUx5N1RTVzRY
RHBVaWI1Wgp3UGdFcnpGYWJPRnNoS0hkM2JxOFhSdE03bmduSW9XWldqaS9CMjNCK1VhVVlWdVlr
SEtueXE1T1ZWOGVwc1NjCnZZNGt3UXJrV1NLMlFQRkJXQTVSQVczYjkvSmNKWGV1UGVSSEJzd3JG
bHRVV2gxSGwrK3Qza3NsUy8rU0c3dm8KSGxxR1FkTWRMNk9Odm9yZ1U1dXNPY2d0RFNONHQ0bzgv
WEZvamRqN3BhamRnWmdjTnpMZDQ5d05uQjVMeVdaZwp1VnZCYTJSSXd0Qk5welVpK3VzUlJMRlZS
a24wbGV1TFBsZ2ZqMUl2dHhFaG5EOWptOG9vYmFacjNBL2ZqNlIxCmoreTZISnFRYnprQUNuM2dI
czdIRXR3ZVZrY3JsN2VzakxSRWZUcWtpOWdIS0l0ZlFMcm9FWkhMR0RPWCtsWGoKQWtFOTJSN3c4
ZlNhNE8rUEk0NTE0R3o5bHZ1K1E5aDNra3VQK3lVTjZPdDZtRHE3Zjljc0tBMndoZFBxKzM2NQov
Q0QwQVZTeVoyRjZzaFNWOE11VmJEMWd0a3hlWEg5dGdxMndreDYvNENDdmxnelpieXZzTGh1YkEz
RHNPQWpyCnRoSm9KbGY0dkFxLzYrK0dVa2J2V2t1akZqUjdpWkxLSFlkcjVYeGZBTGoxSnlvMGVM
bDJJaEF3SXhPRXlDT2wKREx3cUc1SVExdmZRdHpieGE1UkhZblBML1MzZzhNamhFOWtRWGFoUXpt
cFFiS1RtaEhoRlBMVHZDUmlMNGJwWQowNG9EbEZsbjVOb0NWNU9zRTNCcQo9aGtkUQotLS0tLUVO
RCBQR1AgU0lHTkFUVVJFLS0tLS0K

--===============6359621746186202142==--