Because Drupal sets the Reply-Path the same as the From header, receiving mail servers may think Drupal-generated email is spam if the domain in the Reply-Path header doesn’t match the domain of the originating email server (which, for Drupal, is usually an smtp process on a webserver.)

Example: when I use the Contact module on the drupal.org website to send messages to anyone, the SPF records on my albin.net domain specify a list of valid originating email servers and can cause the messages sent from drupal.org to be flagged as spam (since drupal.org is not in my SPF record.)

Since this can cause all Drupal-generated to be thrown in the spam/trash bin, I feel this is a critical bug. Please feel free to downgrade it if I’ve overstepped my bounds.

The SPF website has a best practices for web-generated email page, which I followed to make this patch.

(This patch is actually against 5.1, but should apply to 6.x; I fear the bite of hackfest.)

CommentFileSizeAuthor
#21 user.module_68.patch6.53 KBowahab
#17 spf_2.patch2.29 KBJohnAlbin
#14 missing_semi1981.patch698 bytespwolanin
#12 spf_1.patch2.21 KBJohnAlbin
#9 spf_0.patch1.13 KBJohnAlbin
#6 spf.patch1.39 KBZen
#2 drupalmail_0.patch1.28 KBJohnAlbin
drupalmail.patch1.15 KBJohnAlbin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

I think you're right. A lot of my contact-form mail ends up in my spam filter, and I already suspected this to be the case.

Patch looks good, but can you be a bit more verbose in the code comment? What exactly do you mean by "See RFC 2822 Sec 3.6.2 regarding Sender."? The code comment isn't very explanatory. It would be great if we could embed a bit more background information in the code comments (if only to avoid this bug from being re-introduced). We don't have to overdo it -- just enough information for people to understand why we do things this way.

Thanks John.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Needs work » Needs review
FileSize
drupalmail_0.patch1.28 KB

This patch has an explanatory comment.

Zen’s picture

Zen CreditAttribution: Zen commented
Priority: Critical » Normal
Status: Needs review » Needs work

John,

6-dev is stable enough at the moment.

Review:

  • if (variable_get('site_mail', ini_get('sendmail_from'))) { : This looks dodgy as an if clause. Moreover, this would cause errors when the site e-mail address is not specified (it is not, at present, a required field. But, it probably should be). The if clause can very likely just be discarded and the defaults merged with the array declaration above.
  • Related issue: The use of ini_get('sendmail_from'). which I know is used elsewhere in Drupal, itself is incorrect. sendmail_from is a Win32 only directive as php.ini on a Linux box will indicate. The default could be something like webmaster@example.com, with example.com retrieved from the URL.

I will update the patch later tonight if I'm not beaten to it.

-K

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

The if clause may be dodgy, but I was specifically trying to re-use code already in Drupal. This is a tricky issue because the only way to get the correct values is to ask for them from the user. But the user may put their own email address and not know what the correct values are because they are dependent on the webserver’s SMTP service (which may not even be the same domain as the website.)

Perhaps we should have two email fields: one for website contact email (which is what the current field is mostly used for) and one for web server email.

What we really need is a wizard for “web server email.” Something like:

  1. have Drupal send an email to yourself (via the website contact email address)
  2. have the user copy and paste the received email headers into a form
  3. have Drupal parse the headers to see what the email address of the web sever actually is

There’s a bit of work with that idea, of course.

sepeck’s picture

sepeck CreditAttribution: sepeck commented

Just to mention, rfc2821 supersedes rfc821.
http://www.faqs.org/rfcs/rfc2821.html

Section 4.4 covers some return-path usage and contains a note regarding rfc821's coverage. Not entirely sure of it would affect anything just thought I would point it out. I will try to do some more checking on it later.

Zen’s picture

Zen CreditAttribution: Zen commented
Status: Needs work » Needs review
FileSize
spf.patch1.39 KB

I've forked the default e-mail address issue to http://drupal.org/node/135320

I've re-rolled this patch assuming that the default address exists and is valid.

-K

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Needs review » Needs work

sepeck is correct. This patch needs to be rewritten taking RFC 2821 into account.

Zen’s picture

Zen CreditAttribution: Zen commented
Status: Needs work » Needs review

Isn't that rather arbitrary, John? Please either show that something is wrong or post a patch fixing an issue where something will fail.

-K

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Needs review » Needs work
FileSize
spf_0.patch1.13 KB

Sorry, Karthik. I could have been more specific. The patch specifically mentions RFC 821, but it has been obsoleted by RFC 2821.

Some points to note:

  1. RFC 2821 says, “A message-originating SMTP system SHOULD NOT send a message that already contains a Return-path header” because “when the delivery SMTP server makes the "final delivery" of a message, it inserts a return-path line at the beginning of the mail data.” (4.4)
  2. And, “The return address in the MAIL command SHOULD, if possible, be derived from the system's identity for the submitting (local) user, and the "From:" header field otherwise.” (app. B)
  3. And, “If there is a system identity available, it SHOULD also be copied to the Sender header field if it is different from the address in the From header field. (Any Sender field that was already there SHOULD be removed.)” (app. B)
  4. The site_mail field is designed (and described as) a valid email for the website, i.e. the default From address. However, that email address is NOT the email that should be used for the Sender header. The Sender header should be the ACTUAL sender, i.e. the email address of the web/php server (often www@example.com.)
  5. The Errors-To header is non-standard and discouraged by many.

So the solution to the problem would involve setting the Sender</code, <code>Errors-To, and Return-Path headers with the email address of the web/php process (and NOT with the $from value.)

Since there is no easy way to determine the “email address of the web/php process,” this new patch uses site_mail.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Needs work » Needs review

Accidently set wrong Status message.

Dries’s picture

Dries CreditAttribution: Dries commented

I think this patch looks good.

If your site (and mailserver) is example.com and if you set your e-mail address to be root@foo.com -- then you might get into trouble with spam filters? If so, I really think we need to add some _accessible_ guideline to the form description of the 'site e-mail' description. See http://drupal.org/node/135320. It seems like even with this patch, there still is a bit of a documentation work to be done, otherwise users might still be doing the wrong thing? Or am I confused now.

Either way, feel free to mark this RTBC.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Needs review » Reviewed & tested by the community
FileSize
spf_1.patch2.21 KB

Merged this patch with the one at http://drupal.org/node/135320

The only change is an expanded description of the site_mail setting.

Dries’s picture

Dries CreditAttribution: Dries commented
Version: 6.x-dev » 5.x-dev

I've committed this to CVS HEAD. We'll want to backport this to DRUPAL-5 so I'm changing the version number.

Thanks for your persistence, John! Good job.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Version: 5.x-dev » 6.x-dev
Priority: Normal » Critical
FileSize
missing_semi1981.patch698 bytes

There was bad code in the commit:

parse error in includes/common.inc on line 1981

A ";" is missing (see patch).

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

Oh, geez. Sorry guys. Thanks for catching that, Peter!

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to CVS HEAD. It might be helpful to roll a new patch for DRUPAL-5.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Version: 6.x-dev » 5.x-dev
FileSize
spf_2.patch2.29 KB

New patch re-rolled for DRUPAL-5. Without the parse error.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Fixed

Committed to 5.

There is a small code style issue- the double spaces between sentences should be single.

Dries’s picture

Dries CreditAttribution: Dries commented

I've ugraded my site to use this patch but the problem persists. All my contact form messages are marked as spam by Gmail. Odd.

sepeck’s picture

sepeck CreditAttribution: sepeck commented

Dries, If you could forward me the message header information from 2-3 messages I could look to see if there might be any other factors involved.

owahab’s picture

owahab CreditAttribution: owahab commented
Version: 4.7.6 » 5.x-dev
Status: Closed (fixed) » Fixed
FileSize
user.module_68.patch6.53 KB

Porting to 4.7

owahab’s picture

owahab CreditAttribution: owahab commented
Version: 5.x-dev » 4.7.6
Status: Fixed » Reviewed & tested by the community
Gerhard Killesreiter’s picture

Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented
Status: Reviewed & tested by the community » Fixed

committed.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)
killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented

I had to revert the patch on the 4.7 branch
http://drupal.org/node/165732

mnur’s picture

mnur CreditAttribution: mnur commented
Title: Drupal-generated email can look like spam (to SPF) » Drupal-generated email can look like spam
Version: 5.x-dev » 6.13
Assigned: Unassigned » mnur
Status: Fixed » Active

I am new to Drupal. I have problem with invite module. The email generated by my website goes directly to spam folder. I found out this went one of my friend said that my email from Drupal site goes to their spam folder. This case happened only went I send the email to Yahoo, however for Gmail, it is not categorize as a spam mail. Now I am using invite module 6.x-2.0-alpha1 and latest version of Drupal 6.13. What should do? I have read the solution for invite module version 5, however I am not sure where should I paste the patch if it valid for invite module 6 or Drupal 6.13. Thank you for your help?

Daniel Norton’s picture

Daniel Norton CreditAttribution: Daniel Norton commented

Site mail can also get a higher spam score if the "From" field does not have a display name. This patch for D7 addresses that issue:

  • #834202: Provide for RFC 2822 display-name in email sent from default site email address
    • dddave’s picture

    dddave CreditAttribution: dddave commented
    Version: 6.13 » 6.x-dev
    Component: base system » mail system
    Assigned: mnur » Unassigned
    Priority: Critical » Normal
    Status: Active » Postponed

    This issue won't move until any solution is backported from the issue Daniel linked to (or from the issue which caused Daniel's to be classified as duplicate).

    Damien Tournoud’s picture

    Damien Tournoud CreditAttribution: Damien Tournoud commented
    Version: 6.x-dev » 5.x-dev
    Status: Postponed » Closed (fixed)

    Please do not reopen old issues. This one is about adding technical headers to emails, and is long fixed.

    sivahari’s picture

    sivahari CreditAttribution: sivahari commented

    I am using webform module. The Email sent from the module semms SPAM. How can i resolve the problem.

    --Sivahari