Message info
 
To:aur-dev@archlinux.org From:Florian Pritz Subject:Re: [aur-dev] [PATCH] validate email and fully check existence of it Date:Tue, 20 Mar 2012 12:15:11 +0100
 

On 20.03.2012 10:36, BlackEagle wrote:
> - check if the format is valid
> - go and connect to the smtp server of the given domain and verify if
> the given email exists there

Good idea, but I've seen mail servers accepting all recipients during
the SMTP session, but later sending a DSN because the user didn't exist.
That means we should still implement verification emails. Having this
check is still good though.

> ---
> web/lib/aur.inc.php | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 74 insertions(+), 1 deletions(-)
>
> diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php
> index c662b80..3fc0a14 100644
> --- a/web/lib/aur.inc.php
> +++ b/web/lib/aur.inc.php
> @@ -80,7 +80,80 @@ function check_sid($dbh=NULL) {
> # verify that an email address looks like it is legitimate
> #
> function valid_email($addy) {
> - return (filter_var($addy, FILTER_VALIDATE_EMAIL) !== false);
> + // check against RFC 3696
> + if(filter_var($addy, FILTER_VALIDATE_EMAIL) === false) {
> + return false;
> + }
> +
> + // check dns for mx, a, aaaa records
> + list($local, $domain) = explode('@', $addy);
> + if(! (checkdnsrr($domain, 'MX') || checkdnsrr($domain, 'A') || checkdnsrr($domain, 'AAAA'))) {

Please display an error message if this fails.

> + return false;
> + }
> +
> + // get mx records and check full email address
> + $mxlist = array();
> + $mxweight = array();
> + getmxrr($domain, $mxlist, $mxweight);

http://php.net/getmxrr#refsect1-function.getmxrr-notes states:
"This function should not be used for the purposes of address
verification. Only the mailexchangers found in DNS are returned ..."

> + $mx = array_combine($mxweight, $mxlist);
> + ksort($mx);
> +
> + //smtp_test_email($addy, current($mx));
> + foreach($mx as $prio => $mxsrv) {
> + if(smtp_test_email($addy, $mxsrv) === true) {
> + return true;
> + }
> + }

Since this will eventually return false negatives, please display all
SMTP sessions if you get here. This will help users understand what went
wrong so they can fix it more easily.

> +
> + return false;
> +}
> +
> +# verify that an email address exists on the smtp server
> +#
> +function smtp_test_email($addy, $mxsrv) {
> + if(($smtp = fsockopen($mxsrv, 25)) === false) {
> + return false;
> + }
> +
> + if(intval(preg_replace('/^\([0-9]{3}\).*/', '\1', fgets($smtp))) !== 220) {
> + smtp_close($smtp);
> + return false;
> + }
> +
> + fwrite($smtp, "HELO $mxsrv\r\n");

HELO should send the client name, not the server name.

> + if(intval(preg_replace('/^\([0-9]{3}\).*/', '\1', fgets($smtp))) !== 250) {
> + smtp_close($smtp);
> + return false;
> + }
> +
> + fwrite($smtp, "MAIL FROM: <mailtest@archlinux.org>\r\n");
> + if(intval(preg_replace('/^\([0-9]{3}\).*/', '\1', fgets($smtp))) !== 250) {
> + smtp_close($smtp);
> + return false;
> + }
> +
> + fwrite($smtp, "RCPT TO: <$addy>\r\n");
> + $code = intval(preg_replace('/^\([0-9]{3}\).*/', '\1', fgets($smtp)));
> + /**
> + * 250 = success
> + * 451 or 452 = address got greylisted but another error occured

s/got/not/?

> + * so assume ok
> + */
> + if($code !== 250 && $code !== 451 && $code !== 452) {

I'm not sure if rejecting greylisted adresses is a good idea because
that means the users have to resubmit the form after 5-30 minutes. You
should ask them to confirm that their address is correct (typo catching)
and then ignore all temporary errors (all 4xx codes). If you get a 5xx
it's fine to abort.

If you want to make sure the address really exists, you have to
implement verification mails. Just checking return codes for RCPT TO is
not enough.

> + smtp_close($smtp);
> + return false;
> + }
> +
> + smtp_close($smtp);
> + return true;
> +}
> +
> +# close smtp conneciton
> +#
> +function smtp_close(&$smtp) {
> + fwrite($smtp, "RSET\r\n");
> + fwrite($smtp, "QUIT\r\n");
> + fclose($smtp);
> }
>
> # a new seed value for mt_srand()


--
Florian Pritz


Attachment:

"signature.asc"