search

louisameline / php-mail-signature

Sign your e-mails with DKIM and Domain Keys in PHP
GNU Lesser General Public License v2.1
59 stars 19 forks source link

Support multiple instances of a same header type #12

Open fdumortier opened 7 years ago

fdumortier commented 7 years ago

Hello, When I send a signed e-mail whose header contains a "To:..." line, the receiver detects a wrong signature. I checked my outgoing e-mail with http://www.appmaildev.com/fr/dkim and also with https://www.mail-tester.com/ and the DKIM seems to be wrong. When I remove this "To:" line from the header, everything works fine. Any idea why? Kind regards, François

louisameline commented 7 years ago

Hi, I'd have to check but I suspect that php removes this To: line and appends its own at the start or end of the headers. Do you see that in the headers of your received email? Any noticeable alteration of the headers?

fdumortier commented 7 years ago

Hi, I checked the header on https://www.mail-tester.com/ and it looks unchanged...

fdumortier commented 7 years ago

Hi, I also checked the header on http://www.appmaildev.com/fr/dkim and it also looks unchanged...

fdumortier commented 7 years ago

Hi, There's something even more weird : it is only the signature itself that is wrong, not the body hash that is ok. It looks like if the issue relies in the body hash crypting itself and not in any message alteration during the transfer. I hope you'll find a solution soon! Kind regards, François

fdumortier commented 7 years ago

Hi, To help you identify the issue and search for a solution, you can access this PHP page: https://www.dumortier.eu/mail/test2.php and using checkboxes, you can add a DKIM signature, a "To:" line to the headers, and select a multipart/alternative version an finally type an e-mail address to send it. Kind regards, François

louisameline commented 7 years ago

Before I spend time on this, I was just wondering... Why would you have two To: headers? Not sure why you would do that to begin with.

louisameline commented 7 years ago

The signature is made on the To: given to get_signed_headers() and the other is ignored, that's probably just it. The receiver maybe uses the other To: to check the signature, so it fails.

louisameline commented 7 years ago

The RFC talks about this, I see that this library does not indeed work as it should. If there are let's say three headers of the same type, you should be able to sign either:

Could you test something for me please? Replace these lines (l.480):

if(!empty($to)) $headers .= 'To: '.$to."\r\n";
if(!empty($subject)) $headers .= 'Subject: '.$subject."\r\n";

with

if(!empty($subject)) $headers = 'Subject: '.$subject."\r\n".$headers;
if(!empty($to)) $headers = 'To: '.$to."\r\n".$headers;

I think it should solve your issue, the last To will be used instead of the first one.

However, what needs to be done ultimately is to allow this in the options:

'signed_headers' => array(
        'message-Id',
        'Content-type',
        'To',
        'To',
        'subject'
    );

If someone asks for this, I could find a moment to do it.

The RFC:

Signers choosing to sign an existing header field that occurs more than once in the message (such as Received) MUST sign the physically last instance of that header field in the header block. Signers wishing to sign multiple instances of such a header field MUST include the header field name multiple times in the h= tag of the DKIM-Signature header field, and MUST sign such header fields in order from the bottom of the header field block to the top. The signer MAY include more instances of a header field name in h= than there are actual corresponding header fields to indicate that additional header fields of that name SHOULD NOT be added.

fdumortier commented 7 years ago

Thanks for your time. I don't need a second "To:" line and now I realize that the first PHP mail() parameter is added to the headers. I thought it was just meant to be the receiver e-mail address without display name. I'll change my script and keep you informed.

fdumortier commented 7 years ago

Hi, I just checked removing the second "To:" from the headers and... It's Ok ! Thanks a lot for taking care of it.

louisameline commented 7 years ago

Glad you didn't need two headers of a same type, but this is still an issue though since it's in the RFC. I would appreciate if you could test what I suggested, since it would be a step in the right direction :)

fdumortier commented 7 years ago

I'll do that. But my main issue is to find a solution for signing multi-part bodies. Please have a look on my second issue.

fdumortier commented 7 years ago

Hi, as promised, I tried your proposal and... it's working correctly with a second 'To:" ! Unfortunately, it didn't solve my second issue regarding multipart bodies... Could you help me? Here is my test script: https://www.dumortier.eu/mail/test2.php Kind regards, François