Skip to content

Conversation

icha024
Copy link
Contributor

@icha024 icha024 commented May 7, 2018

For Tomcat, if an SslStoreProvider is configured,
SslStoreProviderUrlStreamHandlerFactory stores the trust-store with an
empty password. Previously, if a password was supplied using the
ssl.trust-store-password property, that would be the password used to
load the trust-store and the connector would warn with "Password
verification failed" message.

Fixes gh-12688

For Tomcat, if an SslStoreProvider is configured,
`SslStoreProviderUrlStreamHandlerFactory` stores the trust-store with an
empty password. Previously, if a password was supplied using the
ssl.trust-store-password property, that would be the password used to
load the trust-store and the connector would warn with "Password
verification failed" message.

Fixes spring-projectsgh-12688
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 7, 2018
@@ -118,6 +118,7 @@ protected void configureSslStoreProvider(AbstractHttp11JsseProtocol<?> protocol,
SslStoreProviderUrlStreamHandlerFactory.KEY_STORE_URL);
}
if (sslStoreProvider.getTrustStore() != null) {
protocol.setTruststorePass("");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see how this change will fix the issue. protocol.setTruststorePass is only called if sslStoreProvider != null. Setting it to empty here doesn't change anything. This can be seen from the fact that the test doesn't fail even if this line is commented out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not the best test for it.

This change uses the same mechanism as your previous patch for setting empty keystore password, and yes it does change it - the same way that previous patch did :)

The only difference is the incorrect truststore password is treated as a warning instead of an exception in starting up the server (hence the test doesn't really test the side-effect properly, my bad)

Use the setup described in #12688 then run mvn -Djavax.net.ssl.trustStorePassword=password spring-boot:run to test it with/without this patch.

Without this patch:

2018-05-11 23:43:41.079  WARN 10368 --- [           main] o.apache.tomcat.util.net.SSLHostConfig   : The provided trust store password could not be used to unlock and/or validate the trust store. Retrying to access the trust store with a null password which will skip validation.

java.security.UnrecoverableKeyException: Password verification failed
	at sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:778) ~[na:1.8.0_171]
	at sun.security.provider.JavaKeyStore$JKS.engineLoad(JavaKeyStore.java:56) ~[na:1.8.0_171]
	at sun.security.provider.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:224) ~[na:1.8.0_171]
	at sun.security.provider.JavaKeyStore$DualFormatJKS.engineLoad(JavaKeyStore.java:70) ~[na:1.8.0_171]
	at java.security.KeyStore.load(KeyStore.java:1445) ~[na:1.8.0_171]
	at org.apache.tomcat.util.net.SSLUtilBase.getStore(SSLUtilBase.java:139) ~[tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.tomcat.util.net.SSLHostConfig.getTruststore(SSLHostConfig.java:714) ~[tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.tomcat.util.net.jsse.JSSEUtil.getTrustManagers(JSSEUtil.java:304) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.tomcat.util.net.AbstractJsseEndpoint.createSSLContext(AbstractJsseEndpoint.java:114) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.tomcat.util.net.AbstractJsseEndpoint.initialiseSsl(AbstractJsseEndpoint.java:87) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.tomcat.util.net.NioEndpoint.bind(NioEndpoint.java:225) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.tomcat.util.net.AbstractEndpoint.start(AbstractEndpoint.java:1150) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.coyote.AbstractProtocol.start(AbstractProtocol.java:591) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.catalina.connector.Connector.startInternal(Connector.java:1018) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.apache.catalina.core.StandardService.addConnector(StandardService.java:225) [tomcat-embed-core-8.5.29.jar:8.5.29]
	at org.springframework.boot.web.embedded.tomcat.TomcatWebServer.addPreviouslyRemovedConnectors(TomcatWebServer.java:256) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.web.embedded.tomcat.TomcatWebServer.start(TomcatWebServer.java:198) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.startWebServer(ServletWebServerApplicationContext.java:300) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.finishRefresh(ServletWebServerApplicationContext.java:162) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:553) [spring-context-5.0.5.RELEASE.jar:5.0.5.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:140) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:759) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:395) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:327) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1255) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1243) [spring-boot-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at com.clianz.vault.ssl.demo.DemoApplication.main(DemoApplication.java:37) [classes/:na]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_171]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_171]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_171]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_171]
	at org.springframework.boot.maven.AbstractRunMojo$LaunchRunner.run(AbstractRunMojo.java:496) [spring-boot-maven-plugin-2.0.1.RELEASE.jar:2.0.1.RELEASE]
	at java.lang.Thread.run(Thread.java:748) [na:1.8.0_171]

@mbhave mbhave added type: enhancement A general enhancement for: merge-with-amendments Needs some changes when we merge and removed status: waiting-for-triage An issue we've not yet triaged labels May 29, 2018
@mbhave mbhave added this to the 2.0.x milestone May 29, 2018
mbhave added a commit that referenced this pull request May 29, 2018
* gh-13088:
  Fix test in "Truststore password if SSLstoreprovider present"
  Use empty trust-store password if SSL store provider present
@mbhave
Copy link
Contributor

mbhave commented May 29, 2018

Thanks for the pull request. It is now on master along with this additional commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JVM truststore/keystore config conflict with EmbeddedServletContainerCustomizer.setSslStoreProvider(...)
3 participants