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

Add optional support for Dkim signing #33

Closed
chaopeng opened this issue Apr 12, 2016 · 17 comments
Closed

Add optional support for Dkim signing #33

chaopeng opened this issue Apr 12, 2016 · 17 comments

Comments

@chaopeng
Copy link

extends Mailer to add sendDKIMMail()

@bbottema
Copy link
Owner

?

@chaopeng
Copy link
Author

I saw a log: Providing access to Session instance for emergency fall-back scenario. Please let us know why you need it. I extends org.codemonkey.simplejavamail.Mailer and add a function to send DKIM mail. it need to convert the Message to DKIMMessage.

public final void sendDKIMMail(final Email email) throws MailException {
    if (validate(email)) {
        try {
            // fill and send wrapped mime message parts
            Message message = produceMimeMessage(email, getSession());
            convertToDKIMMessage(message, ...);
            message.saveChanges(); // some headers and id's will be set for this specific message
            Transport transport = getSession().getTransport();
            try {
                transport.connect();
                transport.sendMessage(message, message.getAllRecipients());
            } finally {
                transport.close();
            }
        } catch (final UnsupportedEncodingException e) {
            logger.error(e.getMessage(), e);
            throw new MailException(String.format("Encoding not accepted: %s", e.getMessage()), e);
        } catch (final MessagingException e) {
            logger.error(e.getMessage(), e);
            throw new MailException(String.format("Generic error: %s", e.getMessage()), e);
        }
    }
}

@bbottema
Copy link
Owner

Ahh, alright. Perhaps Session session should have been protected instead of private, so that you can reference it directly in subclasses instead of through the bean getter method. Then you won't get that warning log.

Btw, what is convertToDKIMMessage? Is that something this library might benefit from?

@chaopeng
Copy link
Author

convertToDKIMMessage is a function to add DKIM signature to mail. I am using https://github.com/markenwerk/java-utils-mail-dkim to help me do this. I think it is good to add this to simple-java-mail.

@bbottema
Copy link
Owner

Will you share your implementation? I'm not really familiar with DKIM.

@bbottema
Copy link
Owner

What about this, @chaopeng:

Email email = new Email();
email.sign(dkimPrivateKeyFileForSender, signingDomain, selector);

Which throws an exception if DkimSigner is missing on the classpath, making the maven dependency on Dkim optional.

@chaopeng
Copy link
Author

Thank you @bbottema . maybe the method also support byte[]/String as key is better.

@bbottema
Copy link
Owner

@chaopeng, what would be the content of that? Would it be the key itself, or a filepath to the key?

@bbottema bbottema changed the title use getSession() Add optional support for Dkim signing Apr 19, 2016
@chaopeng
Copy link
Author

the string/ byte[] will be the key.

@bbottema
Copy link
Owner

bbottema commented May 9, 2016

Found this library as well: https://github.com/usrflo/DKIM-for-JavaMail

/edit: I just noticed that your reference is actually a fork on this one. I'm not sure which would be better, but the fork has been updated more recently.

@chaopeng
Copy link
Author

chaopeng commented May 9, 2016

Yes the one I use is fork on https://github.com/usrflo/DKIM-for-JavaMail . I use https://github.com/markenwerk/java-utils-mail-dkim is because DKIM-for-JavaMail is not in any public maven repo and no activity for many years.

@bbottema
Copy link
Owner

@chaopeng, I made some changes, but I don't know how to test this easily. Do you have a domain with selector and a sample key for me which I can use to test with?

@chaopeng
Copy link
Author

@bbottema do you have domain? If yes you can use this script to generate a key. And selector will be 'test'

#!/usr/bin/env bash

openssl genrsa -out dkim.pem 1024
openssl pkcs8 -topk8 -nocrypt -in dkim.pem -outform der -out dkim.der
openssl rsa -in dkim.pem -pubout

echo "please set DNS TXT record 'test._domainkey.example.com' as 'v=DKIM1;k=rsa;p={publickey};s=email;t=s'"

If you dont I can setup my domain and pm you the key for temporary test.

@bbottema
Copy link
Owner

bbottema commented May 10, 2016

Hi @chaopeng, I committed my changes into the branch "DKIM".

I already verified the initial stage of creating the dkim signer and parsing the private DER key works properly, but I have yet to test the actual sending and verification of the domain record that happens during mailer.sendMail().

If you checkout the branch DKIM, you can try it if you want. See MailerTest for the API in action, it's very simple. Email.signWithDomainKey() accepts a File, Inputsource or byte[].

Right now I'm out of time, but I might try adding a DNS TXT record to my Amazon AWS domain in the weekend and see if everything works. In the meantime, maybe you have the time to try it out.

@bbottema
Copy link
Owner

bbottema commented May 11, 2016

I did some testing with my own domain and it's going quite well, actually, up to the point where the email is being signed (after successfully getting the domain record). When signing the email's content, I get an error:

Caused by: net.markenwerk.utils.mail.dkim.DkimSigningException: Performing RSA cryptography failed.;
nested exception is:
java.io.IOException: javax.crypto.IllegalBlockSizeException: Data must not be longer than 128 bytes
at net.markenwerk.utils.mail.dkim.DomainKey.check(DomainKey.java:270)
at net.markenwerk.utils.mail.dkim.DkimSigner.sign(DkimSigner.java:439)
at net.markenwerk.utils.mail.dkim.DkimMessage.writeTo(DkimMessage.java:141)
at com.sun.mail.smtp.SMTPTransport.sendMessage(SMTPTransport.java:1241)
at org.codemonkey.simplejavamail.Mailer.sendMail(Mailer.java:247)
... 28 more
Caused by: java.io.IOException: javax.crypto.IllegalBlockSizeException: Data must not be longer than 128 bytes
at net.markenwerk.utils.mail.dkim.DkimSigningException.(DkimSigningException.java:77)
... 33 more
Caused by: javax.crypto.IllegalBlockSizeException: Data must not be longer than 128 bytes
at com.sun.crypto.provider.RSACipher.doFinal(RSACipher.java:335)
at com.sun.crypto.provider.RSACipher.engineDoFinal(RSACipher.java:380)
at javax.crypto.Cipher.doFinal(Cipher.java:2121)
at net.markenwerk.utils.mail.dkim.DomainKey.check(DomainKey.java:257)
... 32 more

It might be because I took a random dummy private key from the internet, but I think it was a 2048 bytes encrypted key instead of 1024. I'll run a test with a key from your script.

@bbottema
Copy link
Owner

bbottema commented May 11, 2016

@chaopeng

Ok, cool. It works. This is what gmail received:

X-Google-Original-From: lollypop lol.pop@umbhali.com
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/relaxed; t=1462955593;
s=select; d=umbhali.com; i=lol.pop@umbhali.com;
h=Date:From:To:Message-ID:Subject:MIME-Version:Content-Type;
l=2018; bh=NTpB2kzJrbcBgD27dzkFvL2ulM5fuqqy28iGT6qNOw4=;
b=uxrUbv803HrG0WJDz6G4SvNAowobFwzVyVVr7HTNBWO5g94fiYvB2nyQl1Oto2Uf
ZkYAvf2yajRTLOK0OMhR1KYOjA16SGKkSfWjck+2E2yzdn0kmTpDzv8Sg1A0A+adMNv
0VHUDlfY3FtBEkV2Q1ye1xpXap52WxVZedl4CXug=

So DKIM is fully operational. I'll merge it to the trunk and do a release.

@bbottema
Copy link
Owner

Released in v3.1.0.

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

2 participants