Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change DKIM header fields #1965

Open
KlaasBonnema opened this issue Feb 14, 2020 · 17 comments
Open

Change DKIM header fields #1965

KlaasBonnema opened this issue Feb 14, 2020 · 17 comments

Comments

@KlaasBonnema
Copy link

PHPMailer (6.1.4) includes a default set of header fields in a DKIM key. There currently is no configuration parameter to make changes to this list, that is remove individual fields from the standard list of header fields.
There are situations that warrant changes to the list. A primary reason is that certain fields could be changed or replaced by mail servers down the line. Mail servers should not do that but you can rarely control their behaviour where you should be able to control the DKIM record if you (PHPMailer) is creating it.

@XL-2000
Copy link

XL-2000 commented Feb 15, 2020

You can set $DKIM_extraHeaders yourself to add headers

changed or replaced by mail servers down the line

If this is the case, how can or would you distinguish malicious and "trusted" intrusions or injections? In other words, the whole point of DKIM is to make sure no illegal alterations were made between generating the message and reading it. If changes are being made, how would you be able to state anything about the authenticity of the message?

$autoSignHeaders = [ 'From', 'To', 'CC', 'Date', 'Subject', 'Reply-To', 'Message-ID', 'Content-Type', 'Mime-Version', 'X-Mailer', ];
Should never be allowed to be changed down the line, right?
Only specific "trusted" fields like the return path should be changeable down the line, and are (therefore) not part of the auto sign list

@KlaasBonnema
Copy link
Author

Well, the standard list defined by PHPMailer is arbitrary and open for discussion to what fields should be included or not. My point is that there is no configurable way to exclude fields from the standard list, you can add fields however with DKIM_extraHeaders.

Commercial mail providers and large companies generally have less fields that they include in the DKIM signature. They do that for a reason. A major issue with DKIM is that mail servers tend to modify certain header fields. Mail servers that you as a sender do not control. Then the only alternative may be to not sign a field that is known to give problems or accept that your mails will not arrive. I have seen this happening to Message-ID where mail servers add their own Message-ID.
I do not suggest to change the default list of fields, only that I can remove a field when it causes delivery problems.

@Synchro
Copy link
Member

Synchro commented Feb 15, 2020

Well, the standard list defined by PHPMailer is arbitrary and open for discussion to what fields should be included or not.

It's not arbitrary – It's the recommended set straight from RFC6376. I would expect that changing a message ID is itself an RFC contravention, as it will break lots of things, especially threading.

Synchro added a commit that referenced this issue Feb 15, 2020
@XL-2000
Copy link

XL-2000 commented Feb 15, 2020

What Synchro states is correct. I understand what you say Klaas, but IMO you are trying to solve the "problem" in the wrong place. If servers are changing the headers, you should raise an issue there. The auto sign fields should not be modified in any case

@KlaasBonnema
Copy link
Author

"It's not arbitrary – It's the recommended set straight from RFC6376."
Did you recently read the RFC? It has one required field - From and then has a general statement about choosing fields that protect the "core" of the message and it lists serveral fields as examples, more than the standard list of PHPMailer and it does not list X_Mailer which is in the PHPMailer list.
So my point here remains that the standard list is open for discussion (as we are having here now).

However I do not want to drop or even change the standard list. It is a usefull starting point. I just want to be able to take one or two standard fields out.

"If servers are changing the headers, you should raise an issue there." This is a nice point but how do you beat several 10000 mail servers and their maintainers? They are all to chicken to change something in case it will break their system and I cannot even blame them (too much).
What is happening is that the DKIM system fails due to several reasons where either a header or body is changed by a mail server with probably good intentions. With DMARC turned on this means that one pillar is failing. You only need to forward the message to also loose SPF and your message is bounced. Now you have a choice if it is important that your message is never compromised or if it is important that your message arrives in the first place. I personally would either throw out DKIM and PHPMailer with it or modify the PHPMailer source code to get a troublesome header out of the DKIM signature.

Synchro added a commit that referenced this issue Feb 17, 2020
* DKIM tweaks, see #1962, #1964, #1965

* Don't use the `l` tag in DKIM signature, fixes #1964

* CS

* CS

* Fix order of operations, see #1962

* Remove trailing line break from output of `DKIM_Add()`, see #1962
@andris9
Copy link

andris9 commented Oct 14, 2020

In my experience some sending services (eg SES) override Message-ID header, so if you have signed the message with your Message-ID value, then signature fails. There are two options:

  1. do not include the Message-ID header in the signature
  2. do not sign the message at all and instead configure your sending service to manage signing

@Synchro
Copy link
Member

Synchro commented Oct 14, 2020

I'm strongly in favour of 2.

I'm glad you're here @andris9!

Since both you and @KlaasBonnema seem to know what you're talking about when it comes to DKIM, I could use some help with my DKIM Validator. I've been comparing it with Scott Kitterman's python validator, but that code is really hard to read.

@andris9
Copy link

andris9 commented Oct 14, 2020

IDK if it's any help but I've been recently building a similar validator/signer for Node.js. You can find the source here. Not sure if it is any easier to read than the Python code as there are no comments and it's still alpha (even though it should work fine). The library processes messages as streams, so the dkim body hash calculation is a bit more complex then it would be when processing the entire message at once (eg. the stream processor needs to keep track of whitespaces to know if it needs to normalise these or not).

@Synchro
Copy link
Member

Synchro commented Oct 14, 2020

I saw that too! My main problem is around headers, specifically canonicalisation. There is some ambiguity about how to deal with DKIM-Signature headers; you need to remove the b value of the signature that you are currently validating, but any others should stay in place, though the signature headers are not listed in the h tag, and the headers are meant to be canonicalised in the order listed in the h tag (I think?). This then leads to confusion - in what order and where do you add back the DKIM signatures when canonicalising? I've seen one reference that says they should be added at the top, but I'm really not sure. Anyway, net result is that my header signatures don't match.

What would be really great is to be able to see how your library canonicalises some examples so that I can compare what I'm doing, but my JS isn't up to figuring that out!

@andris9
Copy link

andris9 commented Oct 14, 2020

In general verifying is exactly the same as signing except that you have to use the algorithms specified in the DKIM-Signature header instead of choosing these yourself. And obviously, instead of signing the final header data, you verify it against the signature found from the DKIM-Signature header.

The process is following

  1. you remove the DKIM-Signature header field from header list that you are verifying (but keep the other DKIM-Signatures in place as these might be part of the signature (probably are not but you never know))
  2. calculate body hash and verify it against the bh= value. No point doing anything else if this step fails
  3. take signed header key listing order from h=
  4. filter the message header lines based on key listing but from bottom to top (important if the multiple fields with the same key are signed, see example below)
  5. order the resulting header lines based on key listing
  6. add the DKIM-Signature to the top of the header data, strip out the b= value
  7. normalize header lines based on the canonicalization algorithm
  8. take the resulting data and verify it against the public key from DNS and the signature data from b=

Example

Headers:

DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=out.srv.dev; q=dns/txt;
 s=smtp; t=1602669123; h=Message-ID: Content-Type: MIME-Version: To:
 Subject: Subject: Date: From: Sender;
 bh=qZQWTbOhUwtjFbiSuywcDjgZkYuAeZ+65mtqLMgU/lc=; b=p4WDZf90n0KWIaYO9482lAHT2pspp48i+r17NuAppeWbaBrJ3+3b3QzPJHz6TUOVJ97woW9g
 njh6djfXaHy1OluKfpUjRCEzux36Vt2oF+Ta3soT09m4V6M/VnMhzG/naVg/eJruH89hvplH
 RlutQbvXmec+U3dkrrqM5f0e8W4=
X-Mailgun-Sending-Ip: 141.193.32.19
X-Mailgun-Sid: WyJhN2FlZSIsICJhbmRyaXNAZWtpcmkuZWUiLCAiMTgxZCJd
Received: from [127.0.0.1] (134-247-159-217.sta.estpak.ee [217.159.247.134])
 by smtp-out-n01.prod.eu-central-1.postgun.com with SMTP id
 5f86ca43c20cc98d9eabfed2 (version=TLS1.3, cipher=TLS_AES_128_GCM_SHA256);
 Wed, 14 Oct 2020 09:52:03 GMT
Sender: info=srv.dev@out.srv.dev
From: <info@srv.dev>
Date: Thu, 14 Oct 2020 03:40:36 EDT
Subject: FW: Earn money
Subject: FW2: Earn money
To: <andris@kreata.ee>
MIME-Version: 1.0
Content-Type: text/plain
Message-ID: <test1@srv.dev>

This is the list of header keys (notice that Subject is listed twice)

h=Message-ID: Content-Type: MIME-Version: To: Subject: Subject: Date: From: Sender;

So the header fields to check for after filtering and reordering are the following (notice the ordering of Subject fields):

DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=out.srv.dev; q=dns/txt;
 s=smtp; t=1602669123; h=Message-ID: Content-Type: MIME-Version: To:
 Subject: Subject: Date: From: Sender;
 bh=qZQWTbOhUwtjFbiSuywcDjgZkYuAeZ+65mtqLMgU/lc=; b=
Message-ID: <test1@srv.dev>
Content-Type: text/plain
MIME-Version: 1.0
To: <andris@kreata.ee>
Subject: FW2: Earn money
Subject: FW: Earn money
Date: Thu, 14 Oct 2020 03:40:36 EDT
From: <info@srv.dev>
Sender: info=srv.dev@out.srv.dev

next validate it against the key from smtp._domainkey.out.srv.dev and signature value p4WDZf90n0KWIaYO9482lAHT2pspp48i+r17NuAppeWbaBrJ3+3b3QzPJHz6TUOVJ97woW9gnjh6djfXaHy1OluKfpUjRCEzux36Vt2oF+Ta3soT09m4V6M/VnMhzG/naVg/eJruH89hvplHRlutQbvXmec+U3dkrrqM5f0e8W4=

...and it should match

@Synchro
Copy link
Member

Synchro commented Oct 14, 2020

Thank you, that's very useful. Could you show the canonicalised version of that example?
There's another thing I'm unclear on: Canonicalisation of the headers involves unfolding, and this process may add spaces where there were none in the original, which is ok, but DKIM rules strip out spaces from the signature header before processing it; I'm not sure at what point that should happen. Example, starting with this header:

DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=out.srv.dev; q=dns/txt;
 s=smtp; t=1602669123; h=Message-ID: Content-Type: MIME-Version: To:
 Subject: Subject: Date: From: Sender;
 bh=qZQWTbOhUwtjFbiSuywcDjgZkYuAeZ+65mtqLMgU/lc=; b=p4WDZf90n0KWIaYO9482lAHT2pspp48i+r17NuAppeWbaBrJ3+3b3QzPJHz6TUOVJ97woW9g
 njh6djfXaHy1OluKfpUjRCEzux36Vt2oF+Ta3soT09m4V6M/VnMhzG/naVg/eJruH89hvplH
 RlutQbvXmec+U3dkrrqM5f0e8W4=

We apply normal DKIM canonicalisation: unfolding, reducing whitespace, remove leading space, lowercase field name, remove b value, giving:

dkim-signature:a=rsa-sha256; v=1; c=relaxed/relaxed; d=out.srv.dev; q=dns/txt; s=smtp; t=1602669123; h=Message-ID: Content-Type: MIME-Version: To: Subject: Subject: Date: From: Sender; bh=qZQWTbOhUwtjFbiSuywcDjgZkYuAeZ+65mtqLMgU/lc=; b=

Should the canonicalisation apply DKIM's internal requirement to strip spaces within the value as part of the process? Giving:

dkim-signature:a=rsa-sha256;v=1;c=relaxed/relaxed;d=out.srv.dev;q=dns/txt;s=smtp;t=1602669123;h=Message-ID:Content-Type:MIME-Version:To:Subject:Subject:Date:From:Sender;bh=qZQWTbOhUwtjFbiSuywcDjgZkYuAeZ+65mtqLMgU/lc=;b=

Or is it left in its canonical form for hashing, but then the spaces are stripped before parsing the DKIM tags within the value?

@andris9
Copy link

andris9 commented Oct 14, 2020

Oh sure, forgot that. When using relaxed algo then you unfold the folded header line (in this case only the DKIM-Signature as other fields are not folded). For simple algo you keep the headers as is (except ordering). I'll add the example signature data later when I'm back at my computer (on phone right now).

@KlaasBonnema
Copy link
Author

A good validation reference is check-auth@verifier.port25.com. Send the email that you want to verify to it and a detailed answer with an analysis will be returned. They also show the canonicallized data.
Also a good idea is the DKIM validator plugin for Thunderbird. The auther really knows about DKIM.
You can use these two references as a second opinion for your DKIM validators.
The DKIM RFC is the definitive reference but is can be hard to interprete.

As for the your mail service adding a Message-ID after you have added it before and used it to sign the message:

Your mail service should NEVER add a Message-ID when there already is a Message-ID present. If they do this complain about it. (If they react is another thing...)
The reason mail services do add a Message-ID is if the message does not have one when it reaches the mail service. The RFC says that mail messages SHOULD have a Message-ID. It is known that some android phone mail apps don't add a Message-ID.

When you know that your mail service is adding a Message-ID anyway then a fallback can be to not add it yourself. However you must then rely on the mail service to always do it. Can you be sure your mails always pass through this service? The drawback is that the Message-ID is not signed. (unless the mail server also signs the message...). Also the underlying purpose is that the Message-ID is used to follow the mail through the system. If you give up the right to add your own then you are missing out on the chance to set your own message-ID stamp on the mail

If you can guarantee that your mail service signs every message then this can be an alternative. You can then still sign the message yourself because messages can have more than one signature and only one is needed to pass.

@andris9
Copy link

andris9 commented Oct 15, 2020

I made a mistake with signature header ordering. DKIM-Signature goes to the bottom, not to the top. Here's the actual data that is verified against the public key and signature using relaxed (unfolded, trimmed, lowercase header keys) headers canonicalisation.

message-id:<test1@srv.dev>
content-type:text/plain
mime-version:1.0
to:<andris@kreata.ee>
subject:FW2: Earn money
subject:FW: Earn money
date:Thu, 14 Oct 2020 03:40:36 EDT
from:<info@srv.dev>
sender:info=srv.dev@out.srv.dev
dkim-signature:a=rsa-sha256; v=1; c=relaxed/relaxed; d=out.srv.dev; q=dns/txt; s=smtp; t=1602669123; h=Message-ID: Content-Type: MIME-Version: To: Subject: Subject: Date: From: Sender; bh=qZQWTbOhUwtjFbiSuywcDjgZkYuAeZ+65mtqLMgU/lc=; b=

Each line ends with <CR><LF> and there is no newline after b=

yet again - notice the ordering of subject fields.

@Synchro
Copy link
Member

Synchro commented Oct 15, 2020

Perfect, thanks

@andris9
Copy link

andris9 commented Nov 28, 2020

@Synchro If it's any help then I added a cli option for my DKIM/ARC/DMARC/SPF validator module. You can pipe an email message to a cli command, provide mock DNS responses (so you could sign messages with domains like "example.com" when testing) and get back a JSON formatted report for all signatures in the message. You can find the documentation for available arguments here and an example with input files (both a signed email and mock DNS responses) here.

Report output is really large, so jq is used here to filter out only DKIM signature verification results.

# assuming you have jq and node (at least v14) installed in your system

$ npm install -g mailauth
$ mailauth report -d dns.json < ./test.eml | jq '.dkim.results'

[
  {
    "signingDomain": "ekiri.ee",
    "selector": "default",
    "signature": "TXuCNlsqy6o362UqYRAM+DCPjfYVJw1COP1hLfcDKGMN9YGaQjOsZ8hzCqiGd9qWJVZ3R0rGeUwpaMBHffwTwbZO6DAcdvMwqks7Qfr61znGB3w9QmQKGwTJ2ouJpuzn1RkLHvwIGrwuw8C7mTLcHrdPD3hRaDxcD8T+OhaylGI=",
    "algo": "rsa-sha256",
    "format": "relaxed/simple",
    "bodyHash": "Av6TMj5HbJPuKnFnvPvelZMBt+Ktj6y1ftx8i5fqYJo=",
    "bodyHashExpecting": "Av6TMj5HbJPuKnFnvPvelZMBt+Ktj6y1ftx8i5fqYJo=",
    "status": {
      "result": "pass",
      "comment": false,
      "header": {
        "i": "@ekiri.ee",
        "s": "default",
        "a": "rsa-sha256",
        "b": "TXuCNlsq"
      },
      "policy": {},
      "aligned": "ekiri.ee"
    },
    "publicKey": "-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDweRHLL9VRaEYrU6BeueH5aUWkSYYQA02MzcXfYyZGR2WWvMYOlJYPo2JgazbQnIV0S5xt/3e9rbnR8GMJNJxWqyAYAS0bNF4UatJ9f9C334Rn26308pmYYCZ+YaKFxtlKMndSjH8xgfnKMk5/Lsq/+E/8gl3PXvO9vo15CfcgEQIDAQAB\n-----END PUBLIC KEY-----",
    "rr": "v=DKIM1;k=rsa;p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDweRHLL9VRaEYrU6BeueH5aUWkSYYQA02MzcXfYyZGR2WWvMYOlJYPo2JgazbQnIV0S5xt/3e9rbnR8GMJNJxWqyAYAS0bNF4UatJ9f9C334Rn26308pmYYCZ+YaKFxtlKMndSjH8xgfnKMk5/Lsq/+E/8gl3PXvO9vo15CfcgEQIDAQAB",
    "info": "dkim=pass header.i=@ekiri.ee header.s=default header.a=rsa-sha256 header.b=TXuCNlsq"
  }
]

@Synchro
Copy link
Member

Synchro commented Dec 2, 2020

Thanks @andris9 – I already have something similar in my validator to allow output in HTML, JSON, or plain text. My main motivation for writing it is to allow it to be used in unit tests for PHPMailer, so machine-readable command-line feedback is important. At the moment it's quite qualitative (listing which tests have passed or failed, along with overall verdicts), but your approach of including technical breakdowns is something I should do too. Hey, I could even make it compatible!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants