bbottema/simple-java-mail

Enhancement: make DKIM signing more flexible by allowing header exclusions in DKIM signature

ditschedev opened this issue ยท 14 comments

Hey there.
Is it possible to have any influence on the signing process using the DkimSigner? I want to use Amazon SES over SMTP as my MTA but, as they have their own handling for Message-ID and Date Headers I need to exclude them from my signature.

Cheers

I have no idea what all those things mean, but I'll look into it in the future. Right now not a priority unfortunately. Focusing on the Java 8 migration and Jakarta upgrade instead.

I'll happily take pull requests though.

So DKIM signing is managed by the external library java-utils-mail-smime. In it there is a class DkimSigner, which holds a list of all headers that should be signed. Aong others, this includes Message-ID and "Date" and "Resent-Date":

      MANDATORY_HEADERS_TO_SIGN.add("From");

      DEFAULT_HEADERS_TO_SIGN.addAll(MANDATORY_HEADERS_TO_SIGN);
      DEFAULT_HEADERS_TO_SIGN.add("To");
      DEFAULT_HEADERS_TO_SIGN.add("Subject");
      DEFAULT_HEADERS_TO_SIGN.add("Content-Description");
      DEFAULT_HEADERS_TO_SIGN.add("Content-ID");
      DEFAULT_HEADERS_TO_SIGN.add("Content-Type");
      DEFAULT_HEADERS_TO_SIGN.add("Content-Transfer-Encoding");
      DEFAULT_HEADERS_TO_SIGN.add("Cc");
      DEFAULT_HEADERS_TO_SIGN.add("Date");
      DEFAULT_HEADERS_TO_SIGN.add("In-Reply-To");
      DEFAULT_HEADERS_TO_SIGN.add("List-Subscribe");
      DEFAULT_HEADERS_TO_SIGN.add("List-Post");
      DEFAULT_HEADERS_TO_SIGN.add("List-Owner");
      DEFAULT_HEADERS_TO_SIGN.add("List-Id");
      DEFAULT_HEADERS_TO_SIGN.add("List-Archive");
      DEFAULT_HEADERS_TO_SIGN.add("List-Help");
      DEFAULT_HEADERS_TO_SIGN.add("List-Unsubscribe");
      DEFAULT_HEADERS_TO_SIGN.add("MIME-Version");
      DEFAULT_HEADERS_TO_SIGN.add("Message-ID");
      DEFAULT_HEADERS_TO_SIGN.add("Resent-Sender");
      DEFAULT_HEADERS_TO_SIGN.add("Resent-Cc");
      DEFAULT_HEADERS_TO_SIGN.add("Resent-Date");
      DEFAULT_HEADERS_TO_SIGN.add("Resent-To");
      DEFAULT_HEADERS_TO_SIGN.add("Reply-To");
      DEFAULT_HEADERS_TO_SIGN.add("References");
      DEFAULT_HEADERS_TO_SIGN.add("Resent-Message-ID");
      DEFAULT_HEADERS_TO_SIGN.add("Resent-From");
      DEFAULT_HEADERS_TO_SIGN.add("Sender");

I suppose we would have to change the API to make this more flexible. I'll happily take PR's for this.

I will look into this once I got more time to spend.
Currently quite busy but it would be a nice addition.

I finally started some work on this. Just to clarify, you are/were using your own CustomMailer implementation, correct? To delegate the work to Amazon.

Hey there!

Haven't done anything in that regard since last year ๐Ÿ˜…

Firstly: no, the work should not be delegated to Amazon. AWS has a feature called EasyDKIM which essentially allows clients to send unsigned emails to their relay, which then does the signing for you. I now have use cases where I have written custom SMTP servers using similar functionality to AWS SES. Basically you can't sign the following fields: Message-ID, Date, Return-Path, Bounces-To as they are reassigned when the mail is being relayed, which would result in non-matching signatures. I do now have the situation, that my users can generate and rotate custom DKIM Keys with custom domains, which does not allow me the use of EasyDKIM or my own relay.

But my initial plan was to propose an api change by adding another, optional parameter for a config to be passed to the signing method of the populating builder method.

EmailPopulatingBuilder signWithDomainKey(@NotNull final String dkimPrivateKey, @NotNull final String signingDomain, @NotNull final String dkimSelector, DkimConfig signerConfig)

That can then be used to alter the underlying configuration of the used library. And can be used with the defaults just as they are now, not introducing a breaking change.

Thanks for getting back to me!

Perhap a DkimConfig object would be best. Currently I've added a method to the builder api for setting excludedDkimHeaders, which works, but it's better to combine these parameters in a separate object.

So help me understand your use case. You're using Simple Java Mail to send the mail, but I thought SES is a MTA, isn't that what Simple Java Mail is? Or does SES act as an SMTP server. I'm clearly not too familiar with SES.

SES can be used as an MTA, but it's limited.
That's why it also provides an SMTP relay, where you can send your mails to, that then distributes them and handles things like bounce-handling, retries, etc. for you. So I am using the SMTP relay server provided by AWS, but need to do signing and prepping the mail on the client side.

SES is just distributing the mail to the final recipient(s) and handles all the complexity that comes with robustness and deliverability.

I am thinking if an object is actually necessary.
Are there any more use cases where the signing process needs to be configured?

And is exclusion the best approach for this. Meaning it implies that my custom headers are signed too, which may not be what I want. And to enforce that, it is not possible to use the default config of the signer that you posted, as one need to iterate over the headers of the prepared mail object to dynamically generate the signature.

Wouldn't inclusion be a better solution, as it's far more flexible and does not imply behavior? It might as well help to improve clarity on what is being signed.

I am thinking if an object is actually necessary. Are there any more use cases where the signing process needs to be configured?

It's more about internal maintenance at this point. The EmailBuilder is rather lengthy and if I can group four/five fields separately and save an extra method on the builder api, then I think it's a cleaner approach. Also, the method for setting excluded headers only makes sense if the other methods are used, so it makes sense to group them).

The DkimSigner class has a fixed list of headers to sign, those do not include your custom headers. Is there a use case for signing custom headers? Because as far as I know about DKIM, this is not needed for the SPF checks to determine email validity.

Generally you would want DKIM signing to work normally. Now wanting to sign certain headers is fine, but not the common use case. You want full control of what is signed rather than exclude the headers Amazon takes issue with?

DKIM and SPF are different, but both belong to DMARC. SPF is a framework to prevent spoofing, whilst DKIM is used to verify that the contents of the mail have not been tampered with. That also includes the headers. The DKIM signature includes the headers that were signed (From, To and Subject are required) to verify the signature. There are cases, like a X-Support-Ticket header for example, that you might want to sign as well to verify consistency between your systems.

Now AWS for example relays the mail with the signature provided by you and adds another signature, that includes the required fields and the fields that you did not sign (those that are changed by SES).

So what I meant was, either you:

  • use exclusion, but that would imply to me, that my custom headers are signed too. Which would force the implementation to iterate over the headers and call the addHeaderToSign(String header) method for each one that is not excluded.
  • use inclusion and just call the addHeaderToSign(String header) for each header in the inclusion list.

In either case, the required headers need to be included. The problem with exclusion is, that there is no way to add custom headers to the signing process, which is an unnecessary limitation, imo. Everyone that might use this feature should be well aware on what to achieve, hence I would prefer the inclusion approach which just validates the required fields as it achieves more transparency.

Ahh, I see where the confusion is coming from now. I meant excluded headers based on the default included headers, which is a fixed list in DkimSigner and, -having inherited the DKIM related code - I assumed, a default according to DKIM specs. But I admit I haven't double checked this.

Not confusing at all, haha.

(and according to the current DKIM implementation, only From is absolutely mandatory)

Alright, that was my proposal, sorry for the confusion.
Either provide a list of headers to sign, or use the default list in DkimSigner to be downward compatible. ๐Ÿ˜„

Alright, here's what I came up with for excluding certain headers from the default list of headers to be signed with DKIM:

image

Released in 7.9.0.