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

Cannot add a void DKIM header field #1966

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

Cannot add a void DKIM header field #1966

KlaasBonnema opened this issue Feb 14, 2020 · 8 comments

Comments

@KlaasBonnema
Copy link

PHPMailer 6.1.4 - If one of the default DKIM header fields of any added header field does not exist in the mail header then the field name is not included in the DKIM record.
A void header field is however allowed and it does have a function. A void field is a header field that has no value, for example X-Mailer:
A void field specifies that the header is not present and adding the header later would invalidate the DKIM key.
PHPMailer should therefore add all the requested header fields to the DKIM record, even if no actual matching header field is found.

@XL-2000
Copy link

XL-2000 commented Feb 15, 2020

Please note that the DKIM RFC treats empty and omitted fields differently.

First of all, If you want to add empty values, you should make sure the header field is there with an empty value. This is equal to your "void" as in it will invalidate the signature once the header would be added and have an actual value.

Using non-existing headers while building the signature is problematic, because the verifier on the receiving end probably will not include the missing headers on Verification. Verifiers are likely to verify with "skepticism", ignoring the missing headers altogether.
To prevent collision between the Signer and the Verifier, it is not advisable to sign using headers which are not there

@KlaasBonnema
Copy link
Author

If I add header field X-Header: then this will be an empty field. I can also add X-Header to the list of DKIM fields and the it will be added to the signature and validated on receipt.
If I currently add X-Header to the DKIM fields without adding a header field X-Header: (with or without value) then PHPMailer will not add X-Header to the signature.
This means that someone could maliciously add a header X-Header: to the mail without being detected. PHPMailer should instead add X-Header to the signature regardless if a header X-Header: exists. If there is no header X-Header: then DKIM will still verify ok. If a malicious header X-Header: is added then the DKIM check will fail.
If a verifier does not handle a case where the signature specifies X-Header but there is no actual header X-Header: then the verifier does not adhere to the RFC where this situation is explicitly mentioned.

@XL-2000
Copy link

XL-2000 commented Feb 15, 2020

I do not agree! If you explicitly configure PHPMailer to use X-header in the signature, you should also make sure it is there! Your monkey, your circus, or in other words; your responsibility.
Also, if the verifier does not adhere to the RFC, it is not something to be tackled by PHPmailer. In the end, you want PHPMailer to be as robust as it can be, so anything which can cause conflict and can be avoided, while adhering to the RFC, is preferred IMO.
Still, it is not advisable to sign using ommitted headers, and I think it is in general not a good idea to design code against advice.

Next to that, if someone maliciously would add the X-header, which was not part of the signing, there is no problem yet. Headers are added down the line (X- ones moreover) which have no effect on the integrity of the email itself. If it has effect on the integrity, my previous statement holds, you should make sure it is added yourself in the first place....

@Synchro
Copy link
Member

Synchro commented Feb 15, 2020

I think I'm with @XL-2000 on this one. The most obvious example of unsigned headers is Received; if you signed your message with no received header (because doing so is also an RFC contravention) but included it in the DKIM sig, it would be impossible to deliver the message. You can't control what headers are added (e.g. gmail adds loads of them), so you should only try to control what you can control. Sure, this isn't optimal, but DKIM is a necessarily imperfect approach because it has to work within the confines of the rest of email.

That said, this quote:

Tags that have an empty value are not the same as omitted tags. An
omitted tag is treated as having the default value; a tag with an
empty value explicitly designates the empty string as the value.

applies to tags within the DKIM sig header itself, not headers in general, so I don't think it has any impact on what headers are signed. What does have an impact is this, in the definition of the h= tag:

The field MAY contain names of header
fields that do not exist when signed; nonexistent header fields do
not contribute to the signature computation (that is, they are
treated as the null input, including the header field name, the
separating colon, the header field value, and any CRLF
terminator).

So while you may include headers that don't exist in the signature, they do not contribute to the signature including the name of the field. This means that adding a non-existent header to the signature will not prevent its addition, because an actor adding such a header can also remove its name from the h= list without impacting the signature computation.

@KlaasBonnema
Copy link
Author

"If you explicitly configure PHPMailer to use X-header in the signature, you should also make sure it is there!".
You miss the point here. I want to be able to add X-Header to the DKIM signature to prevent that someone malicious add's an X-Header to the email headers later. because the X-Header is in the DKIM h= DKIM field but not in the mail headers, it will be part of the DKIM signature. If an X-Header field is added later to the mail with or without a value after the : then the DKIM verification will fail because the h= field says that it should be used in the validation (if present).
"nonexistent header fields do not contribute to the signature computation". This means that the b= signature is calculated using the X-Header value in the h= field but it does not use an actual X-Header: value from the email headers.

An example to show where this is vulnerable: I compose an email but I do not specify a Reply-To: address. A response would go to my From: address. Now a hacker somehow modifies the email and inserts a fake Reply-To: header. Since PHPMailer did not include Reply-To in the DKIM h= field even when Reply-To is in the standard set of fields this will go undetected and the DKIM record will validate. A response will now go to the faked Reply-To address.

@Synchro
Copy link
Member

Synchro commented Feb 15, 2020

I want to be able to add X-Header to the DKIM signature to prevent that someone malicious add's an X-Header to the email headers later

That can't work because of the reason I posted above. Anyone that can add a previously nonexistent header can also remove it from the h tag without affecting the DKIM signature. The only defence against that particular scenario is to include a reply-to header and sign it. I agree it's not ideal, but it's what we're stuck with.

To elaborate in case it's unclear: if we have this header (what you're proposing):

Subject: hello
From: user <user@example.com>
DKIM-Signature: ... h=subject:from:reply-to

an attacker can modify it to this undetected:

Subject: hello
From: user <user@example.com>
Reply-to: badguy <badguy@example.com>
DKIM-Signature: ... h=subject:from

and there's nothing we can do about it except make our initial message:

Subject: hello
From: user <user@example.com>
Reply-To: user <user@example.com>
DKIM-Signature: ... h=subject:from:reply-to

@KlaasBonnema
Copy link
Author

"Anyone that can add a previously nonexistent header can also remove it from the h tag without affecting the DKIM signature."
You are making a conceptual mistake here. Look at what consitutes the DKIM signature:

  1. The body text is signed and results in the bh= value
  2. Then the DKIM fields preceded by the header fields are signed as one unit:

Subject: hello
From: user user@example.com
DKIM-Signature: ... h=subject:from:reply-to
bh=xxxxx
b=

Now if anyone modifies the DKIM signature including the h= field or any of the header fields or adds a Reply-To: field then the verification will fail.
So you cannot take the reply-to out of the h= field and still verify ok.

Try it out if you need further convincing.

@Synchro
Copy link
Member

Synchro commented Feb 15, 2020

I think this is unclear in the definition of the h tag, however something I missed is the notes that follow it, which clarify it quite unambiguously:

INFORMATIVE EXPLANATION: By "signing" header fields that do not
actually exist, a Signer can allow a Verifier to detect
insertion of those header fields after signing. However, since
a Signer cannot possibly know what header fields might be
defined in the future, this mechanism cannot be used to prevent
the addition of any possible unknown header fields.

INFORMATIVE NOTE: "Signing" fields that are not present at the time of signing not only prevents fields and values from being added but also prevents adding fields with no values.

So you're quite right.

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

3 participants