bbottema/simple-java-mail

Add config support for 'verifyingServerIdentity' with SMTP, also: since Angus 1.1.0 (8.6.0) server identity checks are on by default and can be countered by `mailerBuilder.verifyingServerIdentity(false)`

narpetri opened this issue · 12 comments

Starting from version 8.6.0 everything stopped working.

My config (Kotlin):

    private fun connectMailBuilder() {
        mailBuilder = MailerBuilder
            .withSMTPServer(
                System.getenv("POSTFIX_HOST"),
                25,
                System.getenv("POSTFIX_USER"),
                System.getenv("POSTFIX_PASSWORD")
            )
            .withProperty("mail.smtp.ssl.enable", false)
            .withDebugLogging(false)
            .async()

        pooledMailer = mailBuilder
            .withConnectionPoolCoreSize(1)
            .withConnectionPoolMaxSize(2)
            .withConnectionPoolClaimTimeoutMillis(1)
            .withConnectionPoolExpireAfterMillis(59000)
            .withTransportModeLoggingOnly(false)
            .buildMailer()

        connect.set(true)
    }

When trying to connect I started getting this error:

Exception in thread "pool-1-thread-1" org.simplejavamail.smtpconnectionpool.TransportHandlingException: Error when trying to open connection to the server, session:
	{mail.smtp.user=utp1-postfix, mail.smtp.ssl.trust=*, mail.smtp.port=25, mail.smtp.auth=true, mail.smtp.starttls.enable=true, mail.smtp.host=00.000.000.000, simplejavamail.transportstrategy=SMTP, mail.smtp.ssl.enable=false, mail.smtp.connectiontimeout=60000, mail.transport.protocol=smtp, mail.smtp.writetimeout=60000, mail.smtp.starttls.required=false, mail.smtp.timeout=60000, SESSION_BASED_EMAIL_TO_MIME_MESSAGE_CONVERTER_KEY=SessionBasedEmailToMimeMessageConverter(session=jakarta.mail.Session@592a514a, operationalConfig=OperationalConfigImpl(async=true, properties={mail.smtp.ssl.enable=false}, sessionTimeout=60000, threadPoolSize=10, threadPoolKeepAliveTime=1, clusterKey=70f8eadc-ebaa-4bee-a059-c7c69049f7d9, connectionPoolCoreSize=1, connectionPoolMaxSize=2, connectionPoolClaimTimeoutMillis=1, connectionPoolExpireAfterMillis=59000, connectionPoolLoadBalancingStrategy=ROUND_ROBIN, transportModeLoggingOnly=false, debugLogging=false, disableAllClientValidation=false, sslHostsToTrust=[], trustAllSSLHost=true, verifyingServerIdentity=true, executorService=org.simplejavamail.internal.batchsupport.concurrent.NonJvmBlockingThreadPoolExecutor@5c29bb95[Running, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0], executorServiceIsUserProvided=false, customMailer=null), emailGovernance=EmailGovernanceImpl(emailValidator=EmailValidator[validationRuleCount=3], emailDefaults=Email{
	id=null
	sentDate=null
	fromRecipient=null,
	replyToRecipients=[],
	bounceToRecipient=null,
	text='null',
	textHTML='null',
	textCalendar='null (method: null)',
	contentTransferEncoding='quoted-printable',
	subject='null',
	recipients=[]
}, emailOverrides=Email{
	id=null
	sentDate=null
	fromRecipient=null,
	replyToRecipients=[],
	bounceToRecipient=null,
	text='null',
	textHTML='null',
	textCalendar='null (method: null)',
	contentTransferEncoding='quoted-printable',
	subject='null',
	recipients=[]
}, maximumEmailSize=null))}
	at org.simplejavamail.smtpconnectionpool.TransportAllocator.connectTransport(TransportAllocator.java:73)
	at org.simplejavamail.smtpconnectionpool.TransportAllocator.allocate(TransportAllocator.java:45)
	at org.simplejavamail.smtpconnectionpool.TransportAllocator.allocate(TransportAllocator.java:28)
	at org.bbottema.genericobjectpool.GenericObjectPool$AutoAllocatorDeallocator.allocatedCorePoolAndDeallocateOneOrPlanDeallocations(GenericObjectPool.java:266)
	at org.bbottema.genericobjectpool.GenericObjectPool$AutoAllocatorDeallocator.run(GenericObjectPool.java:256)
	at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: jakarta.mail.MessagingException: Could not convert socket to TLS;
  nested exception is:
	java.io.IOException: Can't verify identity of server: 00.000.000.000
	at org.eclipse.angus.mail.smtp.SMTPTransport.startTLS(SMTPTransport.java:2173)
	at org.eclipse.angus.mail.smtp.SMTPTransport.protocolConnect(SMTPTransport.java:741)
	at jakarta.mail.Service.connect(Service.java:367)
	at jakarta.mail.Service.connect(Service.java:225)
	at jakarta.mail.Service.connect(Service.java:174)
	at org.simplejavamail.smtpconnectionpool.TransportAllocator.connectTransport(TransportAllocator.java:70)
	... 5 more
Caused by: java.io.IOException: Can't verify identity of server: 00.000.000.000
	at org.eclipse.angus.mail.util.SocketFetcher.checkServerIdentity(SocketFetcher.java:699)
	at org.eclipse.angus.mail.util.SocketFetcher.configureSSLSocket(SocketFetcher.java:636)
	at org.eclipse.angus.mail.util.SocketFetcher.startTLS(SocketFetcher.java:555)
	at org.eclipse.angus.mail.smtp.SMTPTransport.startTLS(SMTPTransport.java:2168)
	... 10 more

Before this everything worked without errors. I deliberately replaced the IP address with zeros - that’s my real address.

What has changed with the 8.6.0 release, and what configurations need to be changed to continue to benefit from the latest updates and improvements?

Thank you!

This is unexpected. 8.6 is the switch to Angus Mail, which is the successor to Jakarta Mail. Angus splits up the interface from the implementation, effectively providing a reference implementation. However, supposedly, they haven't changed anything.

I'll have to dig through the Angus changes to see what they did.

Just for my sake, can you please, right after it failed, try with a 8.5 version you think should be working and see if that is the case? I just want to rule out a server side issue.

Of course, I would not write something without being sure of it. I switch to version 8.5.1 (or 8.4) - everything works right away. I return 8.6.0 or 8.6.3 - everything immediately crashes with the error that I published.

If you need any more information, I will be happy to help (as far as my competence is sufficient).

What happens if you ditch the line .withProperty("mail.smtp.ssl.enable", false) and replace if with .withTransportStrategy() and try with each of the strategies? Also try with TransportStrategy.SMTP, first disabling opportunistic TLS like so:

TransportStrategy.SMTP.setOpportunisticTLS(false);

I tried dropping the .withProperty("mail.smtp.ssl.enable", false) line and ran 5 tests:

  1. Without indicating anything at all;
  2. .withTransportStrategy(TransportStrategy.SMTP_OAUTH2)
  3. .withTransportStrategy(TransportStrategy.SMTP_TLS)
  4. .withTransportStrategy(TransportStrategy.SMTPS)
  5. .withTransportStrategy(TransportStrategy.SMTP)

None of the options gave a positive result.

Then I tried disabling opportunistic TLS and setting the SMTP strategy - it worked. Working option:

    private fun connectMailBuilder() {
        TransportStrategy.SMTP.setOpportunisticTLS(false)

        mailBuilder = MailerBuilder
            .withSMTPServer(
                System.getenv("POSTFIX_HOST"),
                25,
                System.getenv("POSTFIX_USER"),
                System.getenv("POSTFIX_PASSWORD")
            )
            .withTransportStrategy(TransportStrategy.SMTP)
            .withDebugLogging(false)
            .async()

        pooledMailer = mailBuilder
            .withConnectionPoolCoreSize(1)
            .withConnectionPoolMaxSize(2)
            .withConnectionPoolClaimTimeoutMillis(1)
            .withConnectionPoolExpireAfterMillis(59000)
            .withTransportModeLoggingOnly(false)
            .buildMailer()

        connect.set(true)
    }

It seems this is the appropriate way of connecting to the server, then. Classically secured, I would say. Probably privately managed? Or within demilitarized zone? Or do you disagree with this connection configuration?

I generally discourage using custom properties, since that is what causes these kind of issues when a library version changes. Better depend on one of the transport strategies instead.

This solved my problem and this option suits me. Thanks for your work!

@bbottema Please check this: Angus Mail changelog.
Angus Mail check server identity by default after 1.1.0 version, Jakarta Mail never.
Code difference in there source:

// com.sun.mail.util.SocketFetcher#configureSSLSocket
boolean idCheck = PropUtil.getBooleanProperty(props, prefix + ".ssl.checkserveridentity", false);
// org.eclipse.angus.mail.util.SocketFetcher#configureSSLSocket
boolean idCheck = PropUtil.getBooleanProperty(props, prefix + ".ssl.checkserveridentity", true);

The difference cause Simple Java Mail can not connect to server with invalid certificates even with TransportStrategy.SMTP set.

Angus 1.1.0 was released in December 30, 2022. We switched to Angus on Jan 17, 2024 with the release of 8.6.0.

The difference cause Simple Java Mail can not connect to server with invalid certificates even with TransportStrategy.SMTP set.

Correction: only when TransportStrategy.SMTP is set. I just checked the code and we always explicitly configure *.ssl.checkserveridentity for all transport strategies, except SMTP.

// MailerImpl.java

	static private void configureServerIdentityVerification(@NotNull final Session session, @NotNull final OperationalConfig operationalConfig, @Nullable final TransportStrategy transportStrategy) {
		if (transportStrategy != null && transportStrategy != TransportStrategy.SMTP) {
			session.getProperties().setProperty(transportStrategy.propertyNameCheckServerIdentity(),
					Boolean.toString(operationalConfig.isVerifyingServerIdentity()));
		}
	}

As for why we don't set this for SMTP, I'm not sure:

// TransportStrategy.SMTP

		/**
		 * Always throws an exception, as this property is not relevant for plain SMTP.
		 */
		@Override
		public String propertyNameCheckServerIdentity() {
			throw new IllegalStateException("This property is not relevant for plain SMTP");
		}

I'm not entirely sure yet, but I think it's because of the opportunisticTLS which tries to upgrade the connection, in which case "*.ssl.checkserveridentity" becomes relevant again and that's why it fails for @narpetri unless opportunisticTLS is disabled manyally... I think. @ztyzbb

So, @narpetri, you can already revert the workaround of TransportStrategy.SMTP.setOpportunisticTLS(false) and use one of the other transport strategies as long as you also set mailerBuilder.verifyingServerIdentity(false). I've now release a fix in 8.8.2, so that doing this also works for the SMTP transport strategy (previously you would get an error saying it makes no sense).

And @ztyzbb, thanks you so much for bringing this to my attention!

@bbottema From web site and javadoc, when use TransportStrategy.SMTP, server certificates are not validated in any way. But after Angus 1.1.0's change, server identity checks are on by default, so we can't connect to server with invalid certificates unless set mailerBuilder.verifyingServerIdentity(false) explicitly. We should set it internally when use TransportStrategy.SMTP to match the doc.

To be clear: Simple Java Mail already has had this feature turned on for all transport strategies except SMTP. Due to historical reasons, for SMTP this feature was not enabled because it was not relevant until opportunistic TLS was implemented for #105. Now with the switch to Angus, for SMTP too, this feature is now enabled by default.

@ztyzbb, I understand this breaks backwards compatibility, but I think it's the right choice. If you want degraded security on the server, that's totally up to you and you can do so with mailerBuilder.verifyingServerIdentity(false), but the default mode for the client should be to want to verify the server identity.

I already documented this in the RELEASE notes, but I realize I didn't update the documentation on the website. I will also update the JavaDoc. Thank you for reminding me. (edit: I have now updated documentation on these issues).

Yeah, you are right, verify the server identity by default is better.