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

[Enhancement] Expose finer-grained DKIM configuration through the builder api and disable 'l-param' by default #499

Closed
SaeGon-Heo opened this issue Mar 22, 2024 · 13 comments

Comments

@SaeGon-Heo
Copy link

Hello bbottema!
I'm working on DKIM Signing and I have a question about default DKIMSigner config.

https://github.com/bbottema/simple-java-mail/blob/master/modules/dkim-module/src/main/java/org/simplejavamail/internal/dkimsupport/DKIMSigner.java#L47

In DKIMSigner.java it seems set l-param as true always.
But this optional param is not recommended to set in RFC6376.
And mailtrap mail service provider's DKIM document says too.

“l” – length of the message body, and “z”- list of message’s original headers (both not recommended to set).

l-param is not found in google gmail's DKIM header too

DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=~~~;
        h=mime-version:from:date:message-id:subject:to;
        bh=~~~;
        b=~~~
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=~~~; s=~~~;
        h=x-gm-message-state:mime-version:from:date:message-id:subject:to;
        bh=~~~;
        b=~~~
X-Gm-Message-State: ~~~

Is there any reason to use l-param?
If not I think l-param must be set as not use it.

@bbottema
Copy link
Owner

Interesting. To be honest I have no idea and I'm glad you referenced the relevant RFC's. I'll do some research, but know that the DKIM library used under the hood has been passed on to different hands a few times, so it's entirely possible there's no real reason behind it (anymore).

@bbottema bbottema changed the title [Question] Is there any reason to add length-of-body value (l-param) for DKIM? [Enhancement] Expose finer-grained DKIM configuration through the builder api and disable 'l-param' by default Mar 22, 2024
bbottema added a commit that referenced this issue Mar 22, 2024
@bbottema bbottema added this to the 8.8.0 milestone Mar 22, 2024
@bbottema
Copy link
Owner

I've released 8.8.0, which updates the DKIM builder api in a similar fashion as the recent S/MIM update for #498.

You can now configure lengthParam, headerCanonicalization, bodyCanonicalization and signingAlgorithm for DKIM. Here's an example from the DKIM documentation:

currentEmailBuilder.signWithDomainKey(
	DkimConfig.builder()
		.dkimPrivateKeyData(byte[] / File / InputStream)
		.dkimSigningDomain("your_domain.org")
		.dkimSelector("your_selector")
		.useLengthParam(true) // default is false
		.excludedHeadersFromDkimDefaultSigningList("From", "Subject") // default is none
		.headerCanonicalization(DkimConfig.Canonicalization.SIMPLE) // default is RELAXED
		.bodyCanonicalization(DkimConfig.Canonicalization.SIMPLE) // default is RELAXED
		.signingAlgorithm("SHA256_WITH_ED25519") // default is SHA256_WITH_RSA
		.build()
);

bbottema added a commit that referenced this issue Mar 22, 2024
@SaeGon-Heo
Copy link
Author

How fast you done with bunch of codes 😅
Thank you for taking the time to this!
I really like this.

@furkan-atak
Copy link

I wonder how can I extract the dkim signature after build it, to use it as header ? When I reconvert it from EmailBuilder to MimeMessage it broke the dkim and bh fails.

@bbottema
Copy link
Owner

Hi @furkan-atak,

If I understand correctly, you want to pull out the DKIM signature to add it in your email headers directly, but you're hitting a wall with the bh field after switching from EmailBuilder to MimeMessage? Just to clear things up, Simple Java Mail is pretty solid when it comes to converting Email objects to MimeMessages right before sending, since this is the primary use case of using DKIM. This step is meant to keep everything, including DKIM signatures, intact.

It seems like there might be a mix-up or a special case with your issue. Normally, the DKIM signature is part of the email headers, added automatically when you send the email to make sure it's legit and hasn't been tampered with. Messing with the DKIM signature manually isn't typically done because it can easily make the signature invalid, especially if you change parts of the email that the signature covers (like the bh field for the body hash).

If you really need to get at the DKIM signature before sending, I'm curious to know why. Perhaps it's a use case I might want to support. However, I'd advise caution with manual manipulations of DKIM signatures due to the precise nature of email authentication processes.

@furkan-atak
Copy link

It's because I am trying to use dkim in milter (mail filter). So it's actually does not send a mail to anywhere but it communicates with postfix (if you do not know much about postfix and milter protocol stages: https://www.postfix.org/MILTER_README.html) so because of that instead of directly signing and sending an email, I need the dkim header to add the mail headers in the related milter stage which is eom(end of message).

@SaeGon-Heo
Copy link
Author

SaeGon-Heo commented Mar 27, 2024 via email

@bbottema
Copy link
Owner

It's because I am trying to use dkim in milter (mail filter). So it's actually does not send a mail to anywhere but it communicates with postfix (if you do not know much about postfix and milter protocol stages: https://www.postfix.org/MILTER_README.html) so because of that instead of directly signing and sending an email, I need the dkim header to add the mail headers in the related milter stage which is eom(end of message).

Hhm, you could try creating your own CustomMailer implementation. This gives you access to the converted MimeMessage without sending it. Would that be of help?

@SaeGon-Heo
Copy link
Author

SaeGon-Heo commented Mar 27, 2024 via email

@furkan-atak
Copy link

@bbottema My issue is not about sending stuff or related about the main purpose of the library. I mainly use the library for content replacement and dkim sign for the mail so my problem is about that. However, even if a did not change the body and just convert byte[] to EmailPopulatingBuilder than Email than MimeMessage than have a dkim header and adding that to the header did not work well.

And this is what I do:
MimeMessage mimeMessage = EmailConverter.emailToMimeMessage(email);

MimeMessage dkimMimeMessage = genereateDkimMimeMessage(email); /* returns
new DkimMessageIdFixingMimeMessage(messageToSign, dkimSigner, email.getId()); with default config*/

String dkimEmailContent = EmailConverter.mimeMessageToEML(dkimMimeMessage);

String dkimHeader = FormatUtils.findFirstWithRegex(DKIM_HEADER_VALUE_FINDER_PATTERN, dkimEmailContent);

@SaeGon-Heo thanks for the idea. However, it's a protocol level config not related to the application layer I am implementing and before that, I have my own test cases. And in that tests bh (body hash) fails in dkim.

@bbottema
Copy link
Owner

bbottema commented Mar 27, 2024

@furkan-atak Apologies, but I still don't understand what you're doing or trying to accomplish.

You want to send an email to postfix. Signed or unsigned? I'm assuming unsigned, because if signed, the header would already be there. So you first sign the email just to get the header and then send the unsigned email plus header separately to postfix? And how does it break DKIM, where? At which stage?

@SaeGon-Heo
Copy link
Author

SaeGon-Heo commented Mar 27, 2024

@furkan-atak
okay...
I misunderstood because of this sentence

I need the dkim header to add the mail headers in the related milter stage which is eom(end of message).

likely you want to add DKIM header in milter's EOM(end-of-data) stage.
But you want to add DKIM header to original MIMEMessage before send it to email server associated with milter, am I right?

And this is what I do:
MimeMessage mimeMessage = EmailConverter.emailToMimeMessage(email);

MimeMessage dkimMimeMessage = genereateDkimMimeMessage(email);

String dkimEmailContent = EmailConverter.mimeMessageToEML(dkimMimeMessage);

String dkimHeader = FormatUtils.findFirstWithRegex(DKIM_HEADER_VALUE_FINDER_PATTERN, dkimEmailContent);

And add dkimHeader to mimeMessage, is it?

Then I wonder why you need this process...
Because, as I know, DKIM header is stuff for identity email sender's domain at server which receiving emails using Asymmetric Cryptography with mail header and body.

And I guess here, as you saw in your test, bh tag check may fail because of DKIM's Canonicalization.

In short, when make DKIM signature, mail header and body is canonicalized to prevent DKIM signature verification failure when transit it to destination via relays like milter.

Message transit from author to recipient is through
relays that typically make no substantive change to the message
content and thus preserve the DKIM signature.

Your DKIM header's bh is calculated after relaxed/relaxed canonicalization by default config.
And original MIMEMessage not canonicalization in any way.
It can lead to verification failure because original mail header and body is not canonicalized but DKIM header made using canonicalized header/body.

You may solve this by choose simple canonicalization for header/body.
But simple body canonicalization also modify body in some case.
And most case use relaxed/relaxed canonicalization nowadays.

If you want to simply check DKIM header is added correctly before send it, how about use DKIMVerifier in org.apache.james.jdkim:apache-jdkim:0.3?
But I think check DKIM when receiving email is most important point than any other stage.
Note that public key dns/txt check is performed when make DKIM header, too.

@furkan-atak
Copy link

Hey Guys, after a while I found the problem but forget obviously writing down here, the problem is that when you convert an eml or byte[] into a MimeMessage the MimeMessage produce a new Content-Type with new boundary part id's in it. So when you get MimeMessage with dkim signed you have to use that Content-Type header with the original mail too because these boundary ids also exist in body parts and so it effects the body hash too. To overcome it, after my last conversion to MimeMessage I also change Content-Type header of the mail with the last MimeMessage's Content-Type. (Not: This is an SMTP-Milter level manipulation. Here's more information about milter change header: https://web.mit.edu/freebsd/head/contrib/sendmail/libmilter/docs/smfi_chgheader.html)

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

No branches or pull requests

3 participants