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

API bugfix: server identity verification should not be tied to host trusting #221

Closed
floragunn opened this issue Aug 16, 2019 · 7 comments
Closed

Comments

@floragunn
Copy link

Simple Java Mail Version: 5.2.0

If I connect to a SMTP server on localhost with a self signed certificate and setting MailerBuilder.trustingAllHosts(true) and/or MailerBuilder.trustingSSLHosts(new String[]{"*"}) get an "Can't verify identity of server: 127.0.0.1" exception:

org.simplejavamail.mailer.internal.mailsender.MailSenderException: Third party error
	at org.simplejavamail.mailer.internal.mailsender.MailSender.sendMailClosure(MailSender.java:283)
	at org.simplejavamail.mailer.internal.mailsender.MailSender.send(MailSender.java:211)
	at org.simplejavamail.mailer.Mailer.sendMail(Mailer.java:240)
	at org.simplejavamail.mailer.Mailer.sendMail(Mailer.java:231)
	at com.floragunn.signals.watch.actions.smtp.SmtpMailer.sendMail(SmtpMailer.java:53)
	at com.floragunn.signals.watch.actions.smtp.SmtpAction.execute(SmtpAction.java:81)
	at com.floragunn.signals.ActionTest.testSmtpAction(ActionTest.java:203)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)
Caused by: javax.mail.MessagingException: Could not connect to SMTP host: 127.0.0.1, port: 3000;
  nested exception is:
	java.io.IOException: Can't verify identity of server: 127.0.0.1
	at com.sun.mail.smtp.SMTPTransport.openServer(SMTPTransport.java:2187)
	at com.sun.mail.smtp.SMTPTransport.protocolConnect(SMTPTransport.java:716)
	at javax.mail.Service.connect(Service.java:342)
	at javax.mail.Service.connect(Service.java:222)
	at javax.mail.Service.connect(Service.java:171)
	at org.simplejavamail.mailer.internal.mailsender.MailSender.sendMailClosure(MailSender.java:265)
	... 32 more
Caused by: java.io.IOException: Can't verify identity of server: 127.0.0.1
	at com.sun.mail.util.SocketFetcher.checkServerIdentity(SocketFetcher.java:673)
	at com.sun.mail.util.SocketFetcher.configureSSLSocket(SocketFetcher.java:610)
	at com.sun.mail.util.SocketFetcher.createSocket(SocketFetcher.java:376)
	at com.sun.mail.util.SocketFetcher.getSocket(SocketFetcher.java:214)
	at com.sun.mail.smtp.SMTPTransport.openServer(SMTPTransport.java:2151)
	... 37 more

I can fix it with MailerBuilder..withProperty("mail.smtps.ssl.checkserveridentity", "false") but for me it looks that MailerBuilder.trustingAllHosts(true) and/or MailerBuilder.trustingSSLHosts(new String[]{"*"}) should imply this?

@bbottema
Copy link
Owner

bbottema commented Aug 16, 2019

What you are saying sounds reasonable, but unfortunately I'm not an expert on the topic of SSL and this particular issue seems to stem from the SSL support in the underlying JavaMail library. Maybe we can page @cbarcenas and see if he has better understanding, he's been a great help in the past on similar topics.

@floragunn floragunn reopened this Aug 16, 2019
@floragunn
Copy link
Author

(Sorry, pressed wrong button)

@bbottema @cbarcenas the docs for

  • trustingSSLHosts() say "Configures the new session to only accept server certificates issued to one of the provided hostnames, and disables certificate issue validation"

  • trustingAllHosts() say: "Configures the current session to trust all hosts and don't validate any SSL keys."

So in both cases "no validation" should happen which IMHO should imply mail.smtps.ssl.checkserveridentity=false

@bbottema bbottema removed this from the 5.3.0 milestone Aug 16, 2019
@bbottema
Copy link
Owner

bbottema commented Aug 18, 2019

trustingSSLHosts (mail.smtp(s).ssl.trust) indicates not trusting all host and "mail.smtps.ssl.checkserveridentity" is not selective so that's not a good match.

After conducting a source code review of JavaMail, it seems these properties serve different purposes and its exposure in Simple Java Mail misaligned (and mail.smtps.ssl.checkserveridentity is not implied by trusting a host!). Trusting a host means that after verifying its identity, you say you don't distrust it. If you leave it empty, you trust it by default. It's like an optional whitelist, but is not related to verifying the server's identity using SSL keys.

Knowing this, I'm thinking what is the best way to expose this API better. Would it make sense to expose both features separately, or roll it all into one method (like the current trustingHosts()).

@bbottema
Copy link
Owner

I think I will split it up in two methods: one to enable/disable server identity checks using SSL key and one that enables the whitelist for hosts (not related to SSL).

@bbottema bbottema added this to the 5.4.0 milestone Aug 18, 2019
@bbottema
Copy link
Owner

bbottema commented Aug 18, 2019

@floragunncom, I've released 5.4.0-SNAPSHOT with the API split up, that fixes your issue (you'll need to add the snapshot repo).

You can now do:

mailerBuilder
   .verifyingServerIdentity(false)
   .buildMailer();

Probably you don't need to touch trustingHosts or trustingAllHosts, but setting your server as trusted host would be prudent.

Can you verify this solved your issue, @floragunncom?

@bbottema bbottema changed the title Question on trusting server identity API bugfix: server identity verification should not be tied to host trusting Aug 18, 2019
@floragunn
Copy link
Author

+1, thx

@bbottema
Copy link
Owner

5.4.0 released.

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