dupondje / PHP-Push-2

Z-Push Fork With CalDAV/LDAP Support
GNU Affero General Public License v3.0
67 stars 24 forks source link

Return-Path (Envelope From) should always be set to user's valid email #64

Open fortiko opened 11 years ago

fortiko commented 11 years ago

The current logic in backend/imap.php does not guarantee that Return-Path (Envelope From) is always set to the user's valid email address.

Android devices can freely set the "From:" field (iOS only allows the user name!), and can sends values such as "User Name user@acme.org" as "From", which according to the current code would result in a 5th parameter to PHP mail that looks like "-f User Name user@acme.org" which is not accepted. The correct value for a return path would be "-f user@acme.org".

In my case, I use email addresses as user names, so for me the following quick fix works, but somebody should think of how to get a valid email address from the above "From" value. It also requires that IMAP_USE_IMAPMAIL is set to FALSE, for now.

            $envelopefrom = "-f ".$this->username;
fortiko commented 11 years ago

The benefit of this is that email bounces (wrong destination, ...) return to the individual user, NOT to the normal PHP return path which is something generic like www-data@localhost or whatever you set in httpd.conf via

 php_admin_value sendmail_path "/usr/sbin/sendmail -t -i -factivesync@host.tld"
fortiko commented 11 years ago

One other important thing: in theory, we can achieve the same with IMAPMAIL, with two additional lines. The current logic (simply appending "'Return-Path: some@thi.ng" to $headers does not work, because imap_mail has a separate parameter for the return path as per http://php.net/manual/en/function.imap-mail.php !):

$returnpath = $this->username;
[...]
$send =  imap_mail ( $toaddr, $message->headers["subject"], $body, $headers, $ccaddr, $bccaddr, $returnpath);

Unfortunately, this bug https://bugs.php.net/bug.php?id=30688 prevents this for now from working. My conclusion is that it only makes sense not NOT use IMAP_USE_IMAPMAIL (what's the benefit anyway), and as a result get a working, correct return-path/envelope-from.

fmbiete commented 11 years ago

For me, i would deprecate IMAP_USE_IMAPMAIL, and only use mail... But before I will do some testing.

fmbiete commented 11 years ago

I have rewrite the SendMail function, and now it should work better. Also it supports mail (PHP), sendmail (system) and SMTP (PHP). If you are interested in testing, you will need at least this commit: https://github.com/fmbiete/Z-Push-contrib/commit/1fe14969ee6c91e45773f66d089a6a04fb158781

PtY1968 commented 11 years ago

define('IMAP_USE_IMAPMAIL', false); // In config.php

$envelopefrom = pregreplace('/^([^<]+)\s<([^>]+)>\s_$/','-F "$1" -f "$2"',$message->headers["from"]); // In backend/imap.php - this line need for us before '$send = @mail ( $toaddr, $message->headers["subject"], $body, $headers, $envelopefrom );' line, because the username not always an exact email address, but the sender. So, the NDR-s will be posted to the sender address instead of the user.

fortiko commented 10 years ago

Actually, this code does not work as expected:

$envelopefrom = preg_replace('/^([^<]+)\s*<([^>]+)>\s*$/','-F "$1" -f "$2"',$message->headers["from"]); 
$send = @mail ( $toaddr, $message->headers["subject"], $body, $headers, $envelopefrom );

because the variable $envelopefrom needs to look like this: "-f user@acme.com" whereas the above code only generates "user@acme.com": note the missing -f flag, which will in turn cause PHP mail() to ignore the specificed envelopefrom.

So a simple fix is adding the "-f " prefix for the sendmail command in the second line at the end:

$envelopefrom = preg_replace('/^([^<]+)\s*<([^>]+)>\s*$/','-F "$1" -f "$2"',$message->headers["from"]); 
$send = @mail ( $toaddr, $message->headers["subject"], $body, $headers, " -f $envelopefrom" );

I just ran into this issue on a new installation, would it be possible to fix/close this really simple issue, please?

PtY1968 commented 10 years ago

It's interesting, I use the code above in more place, and it's working :) If you check the preg_replace code, you will see, the code (if matches) put the -f before the address.

fortiko commented 10 years ago

Well, for me it did not put the "-f" in front; I printed out $envelopefrom to the log and it was (at least in my case with Sogo ZEG) always an email address /without/ the leading "-f".