From e3516059622a09ea2ff439daffaeb87c649d9667 Mon Sep 17 00:00:00 2001 From: cbono Date: Sun, 1 Dec 2019 17:19:05 -0600 Subject: [PATCH 1/2] Verify ssl key alias on server startup See gh-19202 --- .../embedded/jetty/SslServerCustomizer.java | 39 ++++++++- .../embedded/netty/SslServerCustomizer.java | 3 + .../undertow/SslBuilderCustomizer.java | 3 + .../boot/web/server/SslUtils.java | 49 ++++++++++++ .../NettyReactiveWebServerFactoryTests.java | 10 --- .../TomcatReactiveWebServerFactoryTests.java | 18 +++++ .../TomcatServletWebServerFactoryTests.java | 16 ++++ ...AbstractReactiveWebServerFactoryTests.java | 38 +++++++++ .../boot/web/server/SslUtilsTest.java | 79 +++++++++++++++++++ .../AbstractServletWebServerFactoryTests.java | 14 +++- 10 files changed, 256 insertions(+), 13 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java index b1ae3d50386..ef4d5499e72 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java @@ -24,6 +24,7 @@ import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http2.HTTP2Cipher; import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; +import org.eclipse.jetty.server.ConnectionFactory; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; @@ -37,6 +38,7 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.springframework.boot.web.server.Http2; import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.SslStoreProvider; +import org.springframework.boot.web.server.SslUtils; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -105,7 +107,8 @@ class SslServerCustomizer implements JettyServerCustomizer { HttpConnectionFactory connectionFactory = new HttpConnectionFactory(config); SslConnectionFactory sslConnectionFactory = new SslConnectionFactory(sslContextFactory, HttpVersion.HTTP_1_1.asString()); - return new ServerConnector(server, sslConnectionFactory, connectionFactory); + return new SslValidatingServerConnector(server, sslContextFactory, this.ssl.getKeyAlias(), sslConnectionFactory, + connectionFactory); } private boolean isAlpnPresent() { @@ -123,7 +126,8 @@ class SslServerCustomizer implements JettyServerCustomizer { sslContextFactory.setCipherComparator(HTTP2Cipher.COMPARATOR); sslContextFactory.setProvider("Conscrypt"); SslConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, alpn.getProtocol()); - return new ServerConnector(server, ssl, alpn, h2, new HttpConnectionFactory(config)); + return new SslValidatingServerConnector(server, sslContextFactory, this.ssl.getKeyAlias(), ssl, alpn, h2, + new HttpConnectionFactory(config)); } /** @@ -215,4 +219,35 @@ class SslServerCustomizer implements JettyServerCustomizer { } } + /** + * A {@link ServerConnector} that validates the ssl key alias on server startup. + */ + static class SslValidatingServerConnector extends ServerConnector { + + private SslContextFactory sslContextFactory; + + private String keyAlias; + + SslValidatingServerConnector(Server server, SslContextFactory sslContextFactory, String keyAlias, + SslConnectionFactory sslConnectionFactory, HttpConnectionFactory connectionFactory) { + super(server, sslConnectionFactory, connectionFactory); + this.sslContextFactory = sslContextFactory; + this.keyAlias = keyAlias; + } + + SslValidatingServerConnector(Server server, SslContextFactory sslContextFactory, String keyAlias, + ConnectionFactory... factories) { + super(server, factories); + this.sslContextFactory = sslContextFactory; + this.keyAlias = keyAlias; + } + + @Override + protected void doStart() throws Exception { + super.doStart(); + SslUtils.assertStoreContainsAlias(this.sslContextFactory.getKeyStore(), this.keyAlias); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java index f2e32e46325..ac3fd47990b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java @@ -44,6 +44,7 @@ import reactor.netty.tcp.SslProvider; import org.springframework.boot.web.server.Http2; import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.SslStoreProvider; +import org.springframework.boot.web.server.SslUtils; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.ResourceUtils; @@ -106,6 +107,8 @@ public class SslServerCustomizer implements NettyServerCustomizer { protected KeyManagerFactory getKeyManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) { try { KeyStore keyStore = getKeyStore(ssl, sslStoreProvider); + SslUtils.assertStoreContainsAlias(keyStore, ssl.getKeyAlias()); + KeyManagerFactory keyManagerFactory = (ssl.getKeyAlias() == null) ? KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()) : new ConfigurableAliasKeyManagerFactory(ssl.getKeyAlias(), diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java index 95a7e418482..1bde5d121ba 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java @@ -41,6 +41,7 @@ import org.xnio.SslClientAuthMode; import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.SslStoreProvider; +import org.springframework.boot.web.server.SslUtils; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.ResourceUtils; @@ -107,6 +108,8 @@ class SslBuilderCustomizer implements UndertowBuilderCustomizer { private KeyManager[] getKeyManagers(Ssl ssl, SslStoreProvider sslStoreProvider) { try { KeyStore keyStore = getKeyStore(ssl, sslStoreProvider); + SslUtils.assertStoreContainsAlias(keyStore, ssl.getKeyAlias()); + KeyManagerFactory keyManagerFactory = KeyManagerFactory .getInstance(KeyManagerFactory.getDefaultAlgorithm()); char[] keyPassword = (ssl.getKeyPassword() != null) ? ssl.getKeyPassword().toCharArray() : null; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java new file mode 100644 index 00000000000..4f51a3d6b9c --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java @@ -0,0 +1,49 @@ +/* + * Copyright 2012-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.web.server; + +import java.security.KeyStore; +import java.security.KeyStoreException; + +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; + +/** + * Provides utilities around SSL. + * + * @author Chris Bono + * @since 2.1.x + */ +public final class SslUtils { + + private SslUtils() { + } + + public static void assertStoreContainsAlias(KeyStore keyStore, String keyAlias) { + if (!StringUtils.isEmpty(keyAlias)) { + try { + Assert.state(keyStore.containsAlias(keyAlias), + () -> String.format("Keystore does not contain specified alias '%s'", keyAlias)); + } + catch (KeyStoreException ex) { + throw new IllegalStateException( + String.format("Could not determine if keystore contains alias '%s'", keyAlias), ex); + } + } + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java index 2c6743adf3b..e9f35f8c726 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java @@ -19,8 +19,6 @@ package org.springframework.boot.web.embedded.netty; import java.time.Duration; import java.util.Arrays; -import javax.net.ssl.SSLHandshakeException; - import org.junit.Test; import org.mockito.InOrder; import reactor.core.publisher.Mono; @@ -101,14 +99,6 @@ public class NettyReactiveWebServerFactoryTests extends AbstractReactiveWebServe StepVerifier.create(result).expectNext("Hello World").verifyComplete(); } - @Test - public void whenSslIsConfiguredWithAnInvalidAliasTheSslHandshakeFails() { - Mono result = testSslWithAlias("test-alias-bad"); - StepVerifier.setDefaultTimeout(Duration.ofSeconds(30)); - StepVerifier.create(result).expectErrorMatches((throwable) -> throwable instanceof SSLHandshakeException - && throwable.getMessage().contains("HANDSHAKE_FAILURE")).verify(); - } - protected Mono testSslWithAlias(String alias) { String keyStore = "classpath:test.jks"; String keyPassword = "password"; diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactoryTests.java index d8744c4464b..9c9149722fc 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactoryTests.java @@ -31,11 +31,14 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; +import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactory; import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactoryTests; +import org.springframework.boot.web.server.Ssl; import org.springframework.http.server.reactive.HttpHandler; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -166,4 +169,19 @@ public class TomcatReactiveWebServerFactoryTests extends AbstractReactiveWebServ public void compressionOfResponseToPostRequest() { } + @Test + @Override + public void sslWithInvalidAliasFailsDuringStartup() { + String keyStore = "classpath:test.jks"; + String keyPassword = "password"; + AbstractReactiveWebServerFactory factory = getFactory(); + Ssl ssl = new Ssl(); + ssl.setKeyStore(keyStore); + ssl.setKeyPassword(keyPassword); + ssl.setKeyAlias("test-alias-404"); + factory.setSsl(ssl); + assertThatThrownBy(() -> factory.getWebServer(new EchoHandler()).start()) + .isInstanceOf(ConnectorStartFailedException.class); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java index 92a81d99ae4..c8f1ad91214 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java @@ -63,8 +63,11 @@ import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.springframework.boot.testsupport.rule.OutputCapture; +import org.springframework.boot.testsupport.web.servlet.ExampleServlet; +import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.WebServerException; import org.springframework.boot.web.servlet.ServletContextInitializer; +import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory; import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests; import org.springframework.core.io.ByteArrayResource; @@ -81,6 +84,7 @@ import org.springframework.web.client.RestTemplate; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -529,6 +533,18 @@ public class TomcatServletWebServerFactoryTests extends AbstractServletWebServer this.webServer.start(); } + @Test + @Override + public void sslWithInvalidAliasFailsDuringStartup() { + AbstractServletWebServerFactory factory = getFactory(); + Ssl ssl = getSsl(null, "password", "test-alias-404", "src/test/resources/test.jks"); + factory.setSsl(ssl); + ServletRegistrationBean registration = new ServletRegistrationBean<>( + new ExampleServlet(true, false), "/hello"); + assertThatThrownBy(() -> factory.getWebServer(registration).start()) + .isInstanceOf(ConnectorStartFailedException.class); + } + @Override protected JspServlet getJspServlet() throws ServletException { Tomcat tomcat = ((TomcatWebServer) this.webServer).getTomcat(); 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 c00003ef464..2949fd1d124 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 @@ -137,6 +137,44 @@ public abstract class AbstractReactiveWebServerFactoryTests { assertThat(result.block(Duration.ofSeconds(30))).isEqualTo("Hello World"); } + @Test + public void sslWithValidAlias() { + String keyStore = "classpath:test.jks"; + String keyPassword = "password"; + AbstractReactiveWebServerFactory factory = getFactory(); + Ssl ssl = new Ssl(); + ssl.setKeyStore(keyStore); + ssl.setKeyPassword(keyPassword); + ssl.setKeyAlias("test-alias"); + factory.setSsl(ssl); + this.webServer = factory.getWebServer(new EchoHandler()); + this.webServer.start(); + ReactorClientHttpConnector connector = buildTrustAllSslConnector(); + WebClient client = WebClient.builder().baseUrl("https://localhost:" + this.webServer.getPort()) + .clientConnector(connector).build(); + + Mono result = client.post().uri("/test").contentType(MediaType.TEXT_PLAIN) + .body(BodyInserters.fromObject("Hello World")).exchange() + .flatMap((response) -> response.bodyToMono(String.class)); + + StepVerifier.setDefaultTimeout(Duration.ofSeconds(30)); + StepVerifier.create(result).expectNext("Hello World").verifyComplete(); + } + + @Test + public void sslWithInvalidAliasFailsDuringStartup() { + String keyStore = "classpath:test.jks"; + String keyPassword = "password"; + AbstractReactiveWebServerFactory factory = getFactory(); + Ssl ssl = new Ssl(); + ssl.setKeyStore(keyStore); + ssl.setKeyPassword(keyPassword); + ssl.setKeyAlias("test-alias-404"); + factory.setSsl(ssl); + assertThatThrownBy(() -> factory.getWebServer(new EchoHandler()).start()) + .hasStackTraceContaining("Keystore does not contain specified alias 'test-alias-404'"); + } + protected ReactorClientHttpConnector buildTrustAllSslConnector() { SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(SslProvider.JDK) .trustManager(InsecureTrustManagerFactory.INSTANCE); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java new file mode 100644 index 00000000000..e3504dc7e2b --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java @@ -0,0 +1,79 @@ +/* + * Copyright 2012-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.web.server; + +import java.io.File; +import java.io.FileInputStream; +import java.security.KeyStore; +import java.security.KeyStoreException; + +import org.junit.Before; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Tests for {@link SslUtils}. + * + * @author Chris Bono + */ + +public class SslUtilsTest { + + private static final String VALID_ALIAS = "test-alias"; + + private static final String INVALID_ALIAS = "test-alias-5150"; + + private KeyStore keyStore; + + @Before + public void loadKeystore() throws Exception { + this.keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + this.keyStore.load(new FileInputStream(new File("src/test/resources/test.jks")), "secret".toCharArray()); + } + + @Test + public void assertStoreContainsAliasPassesWhenAliasFound() throws KeyStoreException { + SslUtils.assertStoreContainsAlias(this.keyStore, VALID_ALIAS); + } + + @Test + public void assertStoreContainsAliasPassesWhenNullAlias() throws KeyStoreException { + SslUtils.assertStoreContainsAlias(this.keyStore, null); + } + + @Test + public void assertStoreContainsAliasPassesWhenEmptyAlias() throws KeyStoreException { + SslUtils.assertStoreContainsAlias(this.keyStore, ""); + } + + @Test + public void assertStoreContainsAliasFailsWhenAliasNotFound() throws KeyStoreException { + assertThatThrownBy(() -> SslUtils.assertStoreContainsAlias(this.keyStore, INVALID_ALIAS)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Keystore does not contain specified alias '" + INVALID_ALIAS + "'"); + } + + @Test + public void assertStoreContainsAliasFailsWhenKeyStoreThrowsExceptionOnContains() throws KeyStoreException { + KeyStore uninitializedKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + assertThatThrownBy(() -> SslUtils.assertStoreContainsAlias(uninitializedKeyStore, "alias")) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Could not determine if keystore contains alias 'alias'"); + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index 9f5bcd7c8f2..a160da3dd14 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -122,6 +122,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIOException; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; @@ -423,6 +424,17 @@ public abstract class AbstractServletWebServerFactoryTests { assertThat(response).contains("scheme=https"); } + @Test + public void sslWithInvalidAliasFailsDuringStartup() { + AbstractServletWebServerFactory factory = getFactory(); + Ssl ssl = getSsl(null, "password", "test-alias-404", "src/test/resources/test.jks"); + factory.setSsl(ssl); + ServletRegistrationBean registration = new ServletRegistrationBean<>( + new ExampleServlet(true, false), "/hello"); + assertThatThrownBy(() -> factory.getWebServer(registration).start()) + .hasStackTraceContaining("Keystore does not contain specified alias 'test-alias-404'"); + } + @Test public void serverHeaderIsDisabledByDefaultWhenUsingSsl() throws Exception { AbstractServletWebServerFactory factory = getFactory(); @@ -594,7 +606,7 @@ public abstract class AbstractServletWebServerFactoryTests { return getSsl(clientAuth, keyPassword, keyStore, null, null, null); } - private Ssl getSsl(ClientAuth clientAuth, String keyPassword, String keyAlias, String keyStore) { + protected Ssl getSsl(ClientAuth clientAuth, String keyPassword, String keyAlias, String keyStore) { return getSsl(clientAuth, keyPassword, keyAlias, keyStore, null, null, null); } From ac91f14f05ccf7d2198f6e313f03a5c73cc1c296 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 11 Feb 2020 17:03:50 -0800 Subject: [PATCH 2/2] Polish "Verify ssl key alias on server startup" See gh-19202 --- .../embedded/jetty/SslServerCustomizer.java | 7 +++--- .../embedded/netty/SslServerCustomizer.java | 7 +++--- .../undertow/SslBuilderCustomizer.java | 7 +++--- ...ls.java => SslConfigurationValidator.java} | 10 ++++---- .../NettyReactiveWebServerFactoryTests.java | 2 +- .../TomcatServletWebServerFactoryTests.java | 2 +- ...AbstractReactiveWebServerFactoryTests.java | 2 +- ...ava => SslConfigurationValidatorTest.java} | 24 +++++++++---------- .../AbstractServletWebServerFactoryTests.java | 2 +- 9 files changed, 31 insertions(+), 32 deletions(-) rename spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/{SslUtils.java => SslConfigurationValidator.java} (83%) rename spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/{SslUtilsTest.java => SslConfigurationValidatorTest.java} (66%) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java index ef4d5499e72..27cece29657 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.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. @@ -37,8 +37,8 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.springframework.boot.web.server.Http2; import org.springframework.boot.web.server.Ssl; +import org.springframework.boot.web.server.SslConfigurationValidator; import org.springframework.boot.web.server.SslStoreProvider; -import org.springframework.boot.web.server.SslUtils; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -50,6 +50,7 @@ import org.springframework.util.ResourceUtils; * * @author Brian Clozel * @author Olivier Lamy + * @author Chris Bono */ class SslServerCustomizer implements JettyServerCustomizer { @@ -245,7 +246,7 @@ class SslServerCustomizer implements JettyServerCustomizer { @Override protected void doStart() throws Exception { super.doStart(); - SslUtils.assertStoreContainsAlias(this.sslContextFactory.getKeyStore(), this.keyAlias); + SslConfigurationValidator.validateKeyAlias(this.sslContextFactory.getKeyStore(), this.keyAlias); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java index ac3fd47990b..eeab11a5f92 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.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. @@ -43,8 +43,8 @@ import reactor.netty.tcp.SslProvider; import org.springframework.boot.web.server.Http2; import org.springframework.boot.web.server.Ssl; +import org.springframework.boot.web.server.SslConfigurationValidator; import org.springframework.boot.web.server.SslStoreProvider; -import org.springframework.boot.web.server.SslUtils; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.ResourceUtils; @@ -107,8 +107,7 @@ public class SslServerCustomizer implements NettyServerCustomizer { protected KeyManagerFactory getKeyManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) { try { KeyStore keyStore = getKeyStore(ssl, sslStoreProvider); - SslUtils.assertStoreContainsAlias(keyStore, ssl.getKeyAlias()); - + SslConfigurationValidator.validateKeyAlias(keyStore, ssl.getKeyAlias()); KeyManagerFactory keyManagerFactory = (ssl.getKeyAlias() == null) ? KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()) : new ConfigurableAliasKeyManagerFactory(ssl.getKeyAlias(), diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java index 1bde5d121ba..65c948be88b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.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. @@ -40,8 +40,8 @@ import org.xnio.Sequence; import org.xnio.SslClientAuthMode; import org.springframework.boot.web.server.Ssl; +import org.springframework.boot.web.server.SslConfigurationValidator; import org.springframework.boot.web.server.SslStoreProvider; -import org.springframework.boot.web.server.SslUtils; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.ResourceUtils; @@ -108,8 +108,7 @@ class SslBuilderCustomizer implements UndertowBuilderCustomizer { private KeyManager[] getKeyManagers(Ssl ssl, SslStoreProvider sslStoreProvider) { try { KeyStore keyStore = getKeyStore(ssl, sslStoreProvider); - SslUtils.assertStoreContainsAlias(keyStore, ssl.getKeyAlias()); - + SslConfigurationValidator.validateKeyAlias(keyStore, ssl.getKeyAlias()); KeyManagerFactory keyManagerFactory = KeyManagerFactory .getInstance(KeyManagerFactory.getDefaultAlgorithm()); char[] keyPassword = (ssl.getKeyPassword() != null) ? ssl.getKeyPassword().toCharArray() : null; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslConfigurationValidator.java similarity index 83% rename from spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java rename to spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslConfigurationValidator.java index 4f51a3d6b9c..a04027619a9 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslConfigurationValidator.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. @@ -26,14 +26,14 @@ import org.springframework.util.StringUtils; * Provides utilities around SSL. * * @author Chris Bono - * @since 2.1.x + * @since 2.1.13 */ -public final class SslUtils { +public final class SslConfigurationValidator { - private SslUtils() { + private SslConfigurationValidator() { } - public static void assertStoreContainsAlias(KeyStore keyStore, String keyAlias) { + public static void validateKeyAlias(KeyStore keyStore, String keyAlias) { if (!StringUtils.isEmpty(keyAlias)) { try { Assert.state(keyStore.containsAlias(keyAlias), diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java index e9f35f8c726..c455ae901bc 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.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. diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java index c8f1ad91214..59060a67f57 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.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. 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 2949fd1d124..8746030ebf9 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 @@ -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. diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslConfigurationValidatorTest.java similarity index 66% rename from spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java rename to spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslConfigurationValidatorTest.java index e3504dc7e2b..3a48b294338 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslConfigurationValidatorTest.java @@ -27,12 +27,12 @@ import org.junit.Test; import static org.assertj.core.api.Assertions.assertThatThrownBy; /** - * Tests for {@link SslUtils}. + * Tests for {@link SslConfigurationValidator}. * * @author Chris Bono */ -public class SslUtilsTest { +public class SslConfigurationValidatorTest { private static final String VALID_ALIAS = "test-alias"; @@ -47,31 +47,31 @@ public class SslUtilsTest { } @Test - public void assertStoreContainsAliasPassesWhenAliasFound() throws KeyStoreException { - SslUtils.assertStoreContainsAlias(this.keyStore, VALID_ALIAS); + public void validateKeyAliasWhenAliasFoundShouldNotFail() { + SslConfigurationValidator.validateKeyAlias(this.keyStore, VALID_ALIAS); } @Test - public void assertStoreContainsAliasPassesWhenNullAlias() throws KeyStoreException { - SslUtils.assertStoreContainsAlias(this.keyStore, null); + public void validateKeyAliasWhenNullAliasShouldNotFail() { + SslConfigurationValidator.validateKeyAlias(this.keyStore, null); } @Test - public void assertStoreContainsAliasPassesWhenEmptyAlias() throws KeyStoreException { - SslUtils.assertStoreContainsAlias(this.keyStore, ""); + public void validateKeyAliasWhenEmptyAliasShouldNotFail() { + SslConfigurationValidator.validateKeyAlias(this.keyStore, ""); } @Test - public void assertStoreContainsAliasFailsWhenAliasNotFound() throws KeyStoreException { - assertThatThrownBy(() -> SslUtils.assertStoreContainsAlias(this.keyStore, INVALID_ALIAS)) + public void validateKeyAliasWhenAliasNotFoundShouldThrowException() { + assertThatThrownBy(() -> SslConfigurationValidator.validateKeyAlias(this.keyStore, INVALID_ALIAS)) .isInstanceOf(IllegalStateException.class) .hasMessage("Keystore does not contain specified alias '" + INVALID_ALIAS + "'"); } @Test - public void assertStoreContainsAliasFailsWhenKeyStoreThrowsExceptionOnContains() throws KeyStoreException { + public void validateKeyAliasWhenKeyStoreThrowsExceptionOnContains() throws KeyStoreException { KeyStore uninitializedKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - assertThatThrownBy(() -> SslUtils.assertStoreContainsAlias(uninitializedKeyStore, "alias")) + assertThatThrownBy(() -> SslConfigurationValidator.validateKeyAlias(uninitializedKeyStore, "alias")) .isInstanceOf(IllegalStateException.class) .hasMessage("Could not determine if keystore contains alias 'alias'"); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index a160da3dd14..30f79af5c12 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.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.