Skip to content

Commit e351605

Browse files
cbo-indeedmbhave
cbo-indeed
authored and
mbhave
committed
Verify ssl key alias on server startup
See gh-19202
1 parent 747eab0 commit e351605

File tree

10 files changed

+256
-13
lines changed

10 files changed

+256
-13
lines changed

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizer.java

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.eclipse.jetty.http.HttpVersion;
2525
import org.eclipse.jetty.http2.HTTP2Cipher;
2626
import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
27+
import org.eclipse.jetty.server.ConnectionFactory;
2728
import org.eclipse.jetty.server.Connector;
2829
import org.eclipse.jetty.server.HttpConfiguration;
2930
import org.eclipse.jetty.server.HttpConnectionFactory;
@@ -37,6 +38,7 @@
3738
import org.springframework.boot.web.server.Http2;
3839
import org.springframework.boot.web.server.Ssl;
3940
import org.springframework.boot.web.server.SslStoreProvider;
41+
import org.springframework.boot.web.server.SslUtils;
4042
import org.springframework.boot.web.server.WebServerException;
4143
import org.springframework.util.Assert;
4244
import org.springframework.util.ClassUtils;
@@ -105,7 +107,8 @@ private ServerConnector createHttp11ServerConnector(Server server, HttpConfigura
105107
HttpConnectionFactory connectionFactory = new HttpConnectionFactory(config);
106108
SslConnectionFactory sslConnectionFactory = new SslConnectionFactory(sslContextFactory,
107109
HttpVersion.HTTP_1_1.asString());
108-
return new ServerConnector(server, sslConnectionFactory, connectionFactory);
110+
return new SslValidatingServerConnector(server, sslContextFactory, this.ssl.getKeyAlias(), sslConnectionFactory,
111+
connectionFactory);
109112
}
110113

111114
private boolean isAlpnPresent() {
@@ -123,7 +126,8 @@ private ServerConnector createHttp2ServerConnector(Server server, HttpConfigurat
123126
sslContextFactory.setCipherComparator(HTTP2Cipher.COMPARATOR);
124127
sslContextFactory.setProvider("Conscrypt");
125128
SslConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, alpn.getProtocol());
126-
return new ServerConnector(server, ssl, alpn, h2, new HttpConnectionFactory(config));
129+
return new SslValidatingServerConnector(server, sslContextFactory, this.ssl.getKeyAlias(), ssl, alpn, h2,
130+
new HttpConnectionFactory(config));
127131
}
128132

129133
/**
@@ -215,4 +219,35 @@ private void configureSslTrustStore(SslContextFactory factory, Ssl ssl) {
215219
}
216220
}
217221

222+
/**
223+
* A {@link ServerConnector} that validates the ssl key alias on server startup.
224+
*/
225+
static class SslValidatingServerConnector extends ServerConnector {
226+
227+
private SslContextFactory sslContextFactory;
228+
229+
private String keyAlias;
230+
231+
SslValidatingServerConnector(Server server, SslContextFactory sslContextFactory, String keyAlias,
232+
SslConnectionFactory sslConnectionFactory, HttpConnectionFactory connectionFactory) {
233+
super(server, sslConnectionFactory, connectionFactory);
234+
this.sslContextFactory = sslContextFactory;
235+
this.keyAlias = keyAlias;
236+
}
237+
238+
SslValidatingServerConnector(Server server, SslContextFactory sslContextFactory, String keyAlias,
239+
ConnectionFactory... factories) {
240+
super(server, factories);
241+
this.sslContextFactory = sslContextFactory;
242+
this.keyAlias = keyAlias;
243+
}
244+
245+
@Override
246+
protected void doStart() throws Exception {
247+
super.doStart();
248+
SslUtils.assertStoreContainsAlias(this.sslContextFactory.getKeyStore(), this.keyAlias);
249+
}
250+
251+
}
252+
218253
}

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.springframework.boot.web.server.Http2;
4545
import org.springframework.boot.web.server.Ssl;
4646
import org.springframework.boot.web.server.SslStoreProvider;
47+
import org.springframework.boot.web.server.SslUtils;
4748
import org.springframework.boot.web.server.WebServerException;
4849
import org.springframework.util.ResourceUtils;
4950

@@ -106,6 +107,8 @@ else if (this.ssl.getClientAuth() == Ssl.ClientAuth.WANT) {
106107
protected KeyManagerFactory getKeyManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) {
107108
try {
108109
KeyStore keyStore = getKeyStore(ssl, sslStoreProvider);
110+
SslUtils.assertStoreContainsAlias(keyStore, ssl.getKeyAlias());
111+
109112
KeyManagerFactory keyManagerFactory = (ssl.getKeyAlias() == null)
110113
? KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm())
111114
: new ConfigurableAliasKeyManagerFactory(ssl.getKeyAlias(),

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizer.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
import org.springframework.boot.web.server.Ssl;
4343
import org.springframework.boot.web.server.SslStoreProvider;
44+
import org.springframework.boot.web.server.SslUtils;
4445
import org.springframework.boot.web.server.WebServerException;
4546
import org.springframework.util.ResourceUtils;
4647

@@ -107,6 +108,8 @@ private SslClientAuthMode getSslClientAuthMode(Ssl ssl) {
107108
private KeyManager[] getKeyManagers(Ssl ssl, SslStoreProvider sslStoreProvider) {
108109
try {
109110
KeyStore keyStore = getKeyStore(ssl, sslStoreProvider);
111+
SslUtils.assertStoreContainsAlias(keyStore, ssl.getKeyAlias());
112+
110113
KeyManagerFactory keyManagerFactory = KeyManagerFactory
111114
.getInstance(KeyManagerFactory.getDefaultAlgorithm());
112115
char[] keyPassword = (ssl.getKeyPassword() != null) ? ssl.getKeyPassword().toCharArray() : null;

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/SslUtils.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright 2012-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.web.server;
18+
19+
import java.security.KeyStore;
20+
import java.security.KeyStoreException;
21+
22+
import org.springframework.util.Assert;
23+
import org.springframework.util.StringUtils;
24+
25+
/**
26+
* Provides utilities around SSL.
27+
*
28+
* @author Chris Bono
29+
* @since 2.1.x
30+
*/
31+
public final class SslUtils {
32+
33+
private SslUtils() {
34+
}
35+
36+
public static void assertStoreContainsAlias(KeyStore keyStore, String keyAlias) {
37+
if (!StringUtils.isEmpty(keyAlias)) {
38+
try {
39+
Assert.state(keyStore.containsAlias(keyAlias),
40+
() -> String.format("Keystore does not contain specified alias '%s'", keyAlias));
41+
}
42+
catch (KeyStoreException ex) {
43+
throw new IllegalStateException(
44+
String.format("Could not determine if keystore contains alias '%s'", keyAlias), ex);
45+
}
46+
}
47+
}
48+
49+
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import java.time.Duration;
2020
import java.util.Arrays;
2121

22-
import javax.net.ssl.SSLHandshakeException;
23-
2422
import org.junit.Test;
2523
import org.mockito.InOrder;
2624
import reactor.core.publisher.Mono;
@@ -101,14 +99,6 @@ public void whenSslIsConfiguredWithAValidAliasARequestSucceeds() {
10199
StepVerifier.create(result).expectNext("Hello World").verifyComplete();
102100
}
103101

104-
@Test
105-
public void whenSslIsConfiguredWithAnInvalidAliasTheSslHandshakeFails() {
106-
Mono<String> result = testSslWithAlias("test-alias-bad");
107-
StepVerifier.setDefaultTimeout(Duration.ofSeconds(30));
108-
StepVerifier.create(result).expectErrorMatches((throwable) -> throwable instanceof SSLHandshakeException
109-
&& throwable.getMessage().contains("HANDSHAKE_FAILURE")).verify();
110-
}
111-
112102
protected Mono<String> testSslWithAlias(String alias) {
113103
String keyStore = "classpath:test.jks";
114104
String keyPassword = "password";

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactoryTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,14 @@
3131
import org.mockito.ArgumentCaptor;
3232
import org.mockito.InOrder;
3333

34+
import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactory;
3435
import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactoryTests;
36+
import org.springframework.boot.web.server.Ssl;
3537
import org.springframework.http.server.reactive.HttpHandler;
3638

3739
import static org.assertj.core.api.Assertions.assertThat;
3840
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
41+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3942
import static org.mockito.ArgumentMatchers.any;
4043
import static org.mockito.Mockito.inOrder;
4144
import static org.mockito.Mockito.mock;
@@ -166,4 +169,19 @@ public void compressionOfResponseToGetRequest() {
166169
public void compressionOfResponseToPostRequest() {
167170
}
168171

172+
@Test
173+
@Override
174+
public void sslWithInvalidAliasFailsDuringStartup() {
175+
String keyStore = "classpath:test.jks";
176+
String keyPassword = "password";
177+
AbstractReactiveWebServerFactory factory = getFactory();
178+
Ssl ssl = new Ssl();
179+
ssl.setKeyStore(keyStore);
180+
ssl.setKeyPassword(keyPassword);
181+
ssl.setKeyAlias("test-alias-404");
182+
factory.setSsl(ssl);
183+
assertThatThrownBy(() -> factory.getWebServer(new EchoHandler()).start())
184+
.isInstanceOf(ConnectorStartFailedException.class);
185+
}
186+
169187
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,11 @@
6363
import org.mockito.InOrder;
6464

6565
import org.springframework.boot.testsupport.rule.OutputCapture;
66+
import org.springframework.boot.testsupport.web.servlet.ExampleServlet;
67+
import org.springframework.boot.web.server.Ssl;
6668
import org.springframework.boot.web.server.WebServerException;
6769
import org.springframework.boot.web.servlet.ServletContextInitializer;
70+
import org.springframework.boot.web.servlet.ServletRegistrationBean;
6871
import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory;
6972
import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests;
7073
import org.springframework.core.io.ByteArrayResource;
@@ -81,6 +84,7 @@
8184
import static org.assertj.core.api.Assertions.assertThat;
8285
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
8386
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
87+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
8488
import static org.mockito.ArgumentMatchers.any;
8589
import static org.mockito.Mockito.inOrder;
8690
import static org.mockito.Mockito.mock;
@@ -529,6 +533,18 @@ public void onStartup(ServletContext servletContext) throws ServletException {
529533
this.webServer.start();
530534
}
531535

536+
@Test
537+
@Override
538+
public void sslWithInvalidAliasFailsDuringStartup() {
539+
AbstractServletWebServerFactory factory = getFactory();
540+
Ssl ssl = getSsl(null, "password", "test-alias-404", "src/test/resources/test.jks");
541+
factory.setSsl(ssl);
542+
ServletRegistrationBean<ExampleServlet> registration = new ServletRegistrationBean<>(
543+
new ExampleServlet(true, false), "/hello");
544+
assertThatThrownBy(() -> factory.getWebServer(registration).start())
545+
.isInstanceOf(ConnectorStartFailedException.class);
546+
}
547+
532548
@Override
533549
protected JspServlet getJspServlet() throws ServletException {
534550
Tomcat tomcat = ((TomcatWebServer) this.webServer).getTomcat();

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,44 @@ protected final void testBasicSslWithKeyStore(String keyStore, String keyPasswor
137137
assertThat(result.block(Duration.ofSeconds(30))).isEqualTo("Hello World");
138138
}
139139

140+
@Test
141+
public void sslWithValidAlias() {
142+
String keyStore = "classpath:test.jks";
143+
String keyPassword = "password";
144+
AbstractReactiveWebServerFactory factory = getFactory();
145+
Ssl ssl = new Ssl();
146+
ssl.setKeyStore(keyStore);
147+
ssl.setKeyPassword(keyPassword);
148+
ssl.setKeyAlias("test-alias");
149+
factory.setSsl(ssl);
150+
this.webServer = factory.getWebServer(new EchoHandler());
151+
this.webServer.start();
152+
ReactorClientHttpConnector connector = buildTrustAllSslConnector();
153+
WebClient client = WebClient.builder().baseUrl("https://localhost:" + this.webServer.getPort())
154+
.clientConnector(connector).build();
155+
156+
Mono<String> result = client.post().uri("/test").contentType(MediaType.TEXT_PLAIN)
157+
.body(BodyInserters.fromObject("Hello World")).exchange()
158+
.flatMap((response) -> response.bodyToMono(String.class));
159+
160+
StepVerifier.setDefaultTimeout(Duration.ofSeconds(30));
161+
StepVerifier.create(result).expectNext("Hello World").verifyComplete();
162+
}
163+
164+
@Test
165+
public void sslWithInvalidAliasFailsDuringStartup() {
166+
String keyStore = "classpath:test.jks";
167+
String keyPassword = "password";
168+
AbstractReactiveWebServerFactory factory = getFactory();
169+
Ssl ssl = new Ssl();
170+
ssl.setKeyStore(keyStore);
171+
ssl.setKeyPassword(keyPassword);
172+
ssl.setKeyAlias("test-alias-404");
173+
factory.setSsl(ssl);
174+
assertThatThrownBy(() -> factory.getWebServer(new EchoHandler()).start())
175+
.hasStackTraceContaining("Keystore does not contain specified alias 'test-alias-404'");
176+
}
177+
140178
protected ReactorClientHttpConnector buildTrustAllSslConnector() {
141179
SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(SslProvider.JDK)
142180
.trustManager(InsecureTrustManagerFactory.INSTANCE);

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/SslUtilsTest.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Copyright 2012-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.web.server;
18+
19+
import java.io.File;
20+
import java.io.FileInputStream;
21+
import java.security.KeyStore;
22+
import java.security.KeyStoreException;
23+
24+
import org.junit.Before;
25+
import org.junit.Test;
26+
27+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
28+
29+
/**
30+
* Tests for {@link SslUtils}.
31+
*
32+
* @author Chris Bono
33+
*/
34+
35+
public class SslUtilsTest {
36+
37+
private static final String VALID_ALIAS = "test-alias";
38+
39+
private static final String INVALID_ALIAS = "test-alias-5150";
40+
41+
private KeyStore keyStore;
42+
43+
@Before
44+
public void loadKeystore() throws Exception {
45+
this.keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
46+
this.keyStore.load(new FileInputStream(new File("src/test/resources/test.jks")), "secret".toCharArray());
47+
}
48+
49+
@Test
50+
public void assertStoreContainsAliasPassesWhenAliasFound() throws KeyStoreException {
51+
SslUtils.assertStoreContainsAlias(this.keyStore, VALID_ALIAS);
52+
}
53+
54+
@Test
55+
public void assertStoreContainsAliasPassesWhenNullAlias() throws KeyStoreException {
56+
SslUtils.assertStoreContainsAlias(this.keyStore, null);
57+
}
58+
59+
@Test
60+
public void assertStoreContainsAliasPassesWhenEmptyAlias() throws KeyStoreException {
61+
SslUtils.assertStoreContainsAlias(this.keyStore, "");
62+
}
63+
64+
@Test
65+
public void assertStoreContainsAliasFailsWhenAliasNotFound() throws KeyStoreException {
66+
assertThatThrownBy(() -> SslUtils.assertStoreContainsAlias(this.keyStore, INVALID_ALIAS))
67+
.isInstanceOf(IllegalStateException.class)
68+
.hasMessage("Keystore does not contain specified alias '" + INVALID_ALIAS + "'");
69+
}
70+
71+
@Test
72+
public void assertStoreContainsAliasFailsWhenKeyStoreThrowsExceptionOnContains() throws KeyStoreException {
73+
KeyStore uninitializedKeyStore = KeyStore.getInstance(KeyStore.getDefaultType());
74+
assertThatThrownBy(() -> SslUtils.assertStoreContainsAlias(uninitializedKeyStore, "alias"))
75+
.isInstanceOf(IllegalStateException.class)
76+
.hasMessage("Could not determine if keystore contains alias 'alias'");
77+
}
78+
79+
}

0 commit comments

Comments
 (0)