From 2425dcd2003e619458ca4bd54ac06bb442f9ad59 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 5 Nov 2020 10:25:58 +0000 Subject: [PATCH] Do not set Tomcat's key store and key pass when null Fixes gh-24041 --- .../tomcat/SslConnectorCustomizer.java | 10 ++++++--- .../tomcat/SslConnectorCustomizerTests.java | 21 +++++++++++++++++++ ...AbstractReactiveWebServerFactoryTests.java | 6 ++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizer.java index feca8afedf..bc4c50b3d9 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,8 +69,12 @@ class SslConnectorCustomizer implements TomcatConnectorCustomizer { protocol.setSSLEnabled(true); protocol.setSslProtocol(ssl.getProtocol()); configureSslClientAuth(protocol, ssl); - protocol.setKeystorePass(ssl.getKeyStorePassword()); - protocol.setKeyPass(ssl.getKeyPassword()); + if (ssl.getKeyStorePassword() != null) { + protocol.setKeystorePass(ssl.getKeyStorePassword()); + } + if (ssl.getKeyPassword() != null) { + protocol.setKeyPass(ssl.getKeyPassword()); + } protocol.setKeyAlias(ssl.getKeyAlias()); String ciphers = StringUtils.arrayToCommaDelimitedString(ssl.getCiphers()); if (StringUtils.hasText(ciphers)) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizerTests.java index 90eef99ab4..3814f91bee 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizerTests.java @@ -28,6 +28,7 @@ import org.apache.catalina.LifecycleState; import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.webresources.TomcatURLStreamHandlerFactory; +import org.apache.coyote.http11.Http11NioProtocol; import org.apache.tomcat.util.net.SSLHostConfig; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -185,6 +186,26 @@ class SslConnectorCustomizerTests { .withMessageContaining("Could not load key store 'null'"); } + @Test + void keyStorePasswordIsNotSetWhenNull() { + Http11NioProtocol protocol = (Http11NioProtocol) this.tomcat.getConnector().getProtocolHandler(); + protocol.setKeystorePass("password"); + Ssl ssl = new Ssl(); + ssl.setKeyStore("src/test/resources/test.jks"); + new SslConnectorCustomizer(ssl, null).customize(this.tomcat.getConnector()); + assertThat(protocol.getKeystorePass()).isEqualTo("password"); + } + + @Test + void keyPasswordIsNotSetWhenNull() { + Http11NioProtocol protocol = (Http11NioProtocol) this.tomcat.getConnector().getProtocolHandler(); + protocol.setKeyPass("password"); + Ssl ssl = new Ssl(); + ssl.setKeyStore("src/test/resources/test.jks"); + new SslConnectorCustomizer(ssl, null).customize(this.tomcat.getConnector()); + assertThat(protocol.getKeyPass()).isEqualTo("password"); + } + private KeyStore loadStore() throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { KeyStore keyStore = KeyStore.getInstance("JKS"); Resource resource = new ClassPathResource("test.jks"); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java index e9ec3e2f05..a6e6422559 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java @@ -123,6 +123,7 @@ public abstract class AbstractReactiveWebServerFactoryTests { Ssl ssl = new Ssl(); ssl.setKeyStore(keyStore); ssl.setKeyPassword(keyPassword); + ssl.setKeyStorePassword("secret"); factory.setSsl(ssl); this.webServer = factory.getWebServer(new EchoHandler()); this.webServer.start(); @@ -142,6 +143,7 @@ public abstract class AbstractReactiveWebServerFactoryTests { AbstractReactiveWebServerFactory factory = getFactory(); Ssl ssl = new Ssl(); ssl.setKeyStore(keyStore); + ssl.setKeyStorePassword("secret"); ssl.setKeyPassword(keyPassword); ssl.setKeyAlias("test-alias"); factory.setSsl(ssl); @@ -187,6 +189,7 @@ public abstract class AbstractReactiveWebServerFactoryTests { ssl.setClientAuth(Ssl.ClientAuth.WANT); ssl.setKeyStore("classpath:test.jks"); ssl.setKeyPassword("password"); + ssl.setKeyStorePassword("secret"); ssl.setTrustStore("classpath:test.jks"); testClientAuthSuccess(ssl, buildTrustAllSslWithClientKeyConnector()); } @@ -198,6 +201,7 @@ public abstract class AbstractReactiveWebServerFactoryTests { ssl.setKeyStore("classpath:test.jks"); ssl.setKeyPassword("password"); ssl.setTrustStore("classpath:test.jks"); + ssl.setKeyStorePassword("secret"); testClientAuthSuccess(ssl, buildTrustAllSslConnector()); } @@ -232,6 +236,7 @@ public abstract class AbstractReactiveWebServerFactoryTests { Ssl ssl = new Ssl(); ssl.setClientAuth(Ssl.ClientAuth.NEED); ssl.setKeyStore("classpath:test.jks"); + ssl.setKeyStorePassword("secret"); ssl.setKeyPassword("password"); ssl.setTrustStore("classpath:test.jks"); testClientAuthSuccess(ssl, buildTrustAllSslWithClientKeyConnector()); @@ -242,6 +247,7 @@ public abstract class AbstractReactiveWebServerFactoryTests { Ssl ssl = new Ssl(); ssl.setClientAuth(Ssl.ClientAuth.NEED); ssl.setKeyStore("classpath:test.jks"); + ssl.setKeyStorePassword("secret"); ssl.setKeyPassword("password"); ssl.setTrustStore("classpath:test.jks"); testClientAuthFailure(ssl, buildTrustAllSslConnector());