Error message

You need to log in or create an account to access this page.

Problem/Motivation

DKIM fails because the mail body gets altered, in more detail:

  • During the conversion of HTML to Plain Text (drupal_html_to_text), the h3 and h4 tags are prefixed by ".... " and ".. " respectively.
  • SMTP uses dot-stuffing (see https://tools.ietf.org/html/rfc5321#section-4.5.2 for details) which alters the prefixes of the h3 and h4 plain text conversions.
  • By altering these prefixes, the mail body gets altered and consequently, DKIM verification fails.

I haven't tested it, but I expect the same issue exists in Drupal 8 with the MailFormatHelper in Drupal\Core\Mail.

Context:

SMTP uses a single period (dot) at the beginning of a separate line to denote the end of a mail message. Because users are not expected to be aware of this, SMTP adds an additional period (dot) before each dot on a separate line when it occurs in the message body, before sending the message.
Then DKIM signature is constructed on the mail body, including the additional period added by SMTP.
When the message is received, the receivers SMTP removes the added dots to reconstruct the original message. It thereby alters the mail body which causes the DKIM verification to fail with the error: Mail body has been altered.

Steps to reproduce

Proposed resolution

Use some other prefix instead of the series of dots.

Remaining tasks

  1. Create MR
  2. Review
  3. Commit

User interface changes

API changes

Data model changes

Issue fork drupal-3021619

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

INAScon created an issue. See original summary.

inascon’s picture

StatusFileSize
new523 bytes

Attached is a patch to solve the issue.

inascon’s picture

StatusFileSize
new1.53 KB

Patch failed the mail test. That's fixed now.

inascon’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: 3021619-dkim-fails-on-html-to-text.patch, failed testing. View results

inascon’s picture

StatusFileSize
new1.76 KB
inascon’s picture

Status: Needs work » Needs review

Version: 7.61 » 7.x-dev

Core issues are now filed against the dev versions where changes will be made. Document the specific release you are using in your issue comment. More information about choosing a version.

poker10’s picture

Version: 7.x-dev » 11.x-dev
Status: Needs review » Needs work

Thanks for reporting and working on this.

It seems like the D7 code is the same as the D10 code regarding the indentation (see MailFormatHelper::htmlToText() vs drupal_html_to_text()), so I think this needs to be evaluated/fixed in D10 first. Moving it there. We would need to create an MR for 11.x-dev (probably based on the D7 patch). Thanks!

immaculatexavier’s picture

Status: Needs work » Needs review
StatusFileSize
new1.91 KB
new2.8 KB

As per #9, Fixes for D10.

needs-review-queue-bot’s picture

Status: Needs review » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

sakthi_dev made their first commit to this issue’s fork.

sakthi_dev’s picture

Status: Needs work » Needs review

Converted the latest patch #10 to MR.

smustgrave’s picture

Priority: Major » Normal
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Issue summary should be updated to use the standard issue template

keshav patel’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

Added the issue summary according to the standard issue template.

smustgrave’s picture

Hiding patch files.

Test-only https://git.drupalcode.org/issue/drupal-3021619/-/jobs/1312508 didn't give a failure like expected so leaving in review.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Was able to get a test-only feature https://git.drupalcode.org/issue/drupal-3021619/-/jobs/1378481

Seems like a small enough change.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

This seems fine. I think we should issue a change record for this. Can be set back to RTBC once it exists.