Skip to content

Commit a172e14

Browse files
wilkinsonawilkinsona
committed
Polish "Always fail fast when SSL is enabled without a key store"
Closes gh-15709
1 parent 62c8ac6 commit a172e14

File tree

9 files changed

+71
-33
lines changed

9 files changed

+71
-33
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -190,9 +190,9 @@ private void configureSslKeyStore(SslContextFactory factory, Ssl ssl) {
190190
URL url = ResourceUtils.getURL(ssl.getKeyStore());
191191
factory.setKeyStoreResource(Resource.newResource(url));
192192
}
193-
catch (IOException ex) {
193+
catch (Exception ex) {
194194
throw new WebServerException(
195-
"Could not find key store '" + ssl.getKeyStore() + "'", ex);
195+
"Could not load key store '" + ssl.getKeyStore() + "'", ex);
196196
}
197197
if (ssl.getKeyStoreType() != null) {
198198
factory.setKeyStoreType(ssl.getKeyStoreType());

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.boot.web.embedded.netty;
1818

19-
import java.io.FileNotFoundException;
2019
import java.net.URL;
2120
import java.security.KeyStore;
2221
import java.util.Arrays;
@@ -169,9 +168,11 @@ private KeyStore loadStore(String type, String provider, String resource,
169168
(password != null) ? password.toCharArray() : null);
170169
return store;
171170
}
172-
catch (FileNotFoundException ex) {
173-
throw new WebServerException("Could not load store: " + ex.getMessage(), ex);
171+
catch (Exception ex) {
172+
throw new WebServerException("Could not load key store '" + resource + "'",
173+
ex);
174174
}
175+
175176
}
176177

177178
}

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/SslConnectorCustomizer.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -132,9 +132,9 @@ private void configureSslKeyStore(AbstractHttp11JsseProtocol<?> protocol, Ssl ss
132132
try {
133133
protocol.setKeystoreFile(ResourceUtils.getURL(ssl.getKeyStore()).toString());
134134
}
135-
catch (FileNotFoundException ex) {
136-
throw new WebServerException("Could not load key store: " + ex.getMessage(),
137-
ex);
135+
catch (Exception ex) {
136+
throw new WebServerException(
137+
"Could not load key store '" + ssl.getKeyStore() + "'", ex);
138138
}
139139
if (ssl.getKeyStoreType() != null) {
140140
protocol.setKeystoreType(ssl.getKeyStoreType());

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.boot.web.embedded.undertow;
1818

19-
import java.io.FileNotFoundException;
2019
import java.net.InetAddress;
2120
import java.net.Socket;
2221
import java.net.URL;
@@ -199,8 +198,9 @@ private KeyStore loadStore(String type, String provider, String resource,
199198
(password != null) ? password.toCharArray() : null);
200199
return store;
201200
}
202-
catch (FileNotFoundException ex) {
203-
throw new WebServerException("Could not load store: " + ex.getMessage(), ex);
201+
catch (Exception ex) {
202+
throw new WebServerException("Could not load key store '" + resource + "'",
203+
ex);
204204
}
205205
}
206206

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/SslServerCustomizerTests.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,10 +26,12 @@
2626
import org.eclipse.jetty.server.HttpConnectionFactory;
2727
import org.eclipse.jetty.server.Server;
2828
import org.eclipse.jetty.server.SslConnectionFactory;
29+
import org.eclipse.jetty.util.ssl.SslContextFactory;
2930
import org.junit.Test;
3031

3132
import org.springframework.boot.web.server.Http2;
3233
import org.springframework.boot.web.server.Ssl;
34+
import org.springframework.boot.web.server.WebServerException;
3335

3436
import static org.assertj.core.api.Assertions.assertThat;
3537

@@ -78,6 +80,20 @@ public void alpnConnectionFactoryHasNullDefaultProtocolToAllowNegotiationToHttp1
7880
.isNull();
7981
}
8082

83+
@Test
84+
public void configureSslWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException()
85+
throws Exception {
86+
Ssl ssl = new Ssl();
87+
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
88+
try {
89+
customizer.configureSsl(new SslContextFactory(), ssl, null);
90+
}
91+
catch (Exception ex) {
92+
assertThat(ex).isInstanceOf(WebServerException.class);
93+
assertThat(ex).hasMessageContaining("Could not load key store 'null'");
94+
}
95+
}
96+
8197
private Server createCustomizedServer() {
8298
return createCustomizedServer(new Http2());
8399
}

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@
2121
import org.junit.Test;
2222

2323
import org.springframework.boot.web.server.Ssl;
24+
import org.springframework.boot.web.server.WebServerException;
2425

2526
import static org.assertj.core.api.Assertions.assertThat;
2627
import static org.junit.Assert.fail;
@@ -70,18 +71,18 @@ public void trustStoreProviderIsUsedWhenCreatingTrustStore() throws Exception {
7071
}
7172

7273
@Test
73-
public void keyStoreProviderIsUsedWhenKeyStoreNotContaining() throws Exception {
74+
public void getKeyManagerFactoryWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException()
75+
throws Exception {
7476
Ssl ssl = new Ssl();
75-
ssl.setKeyPassword("password");
7677
SslServerCustomizer customizer = new SslServerCustomizer(ssl, null, null);
7778
try {
7879
customizer.getKeyManagerFactory(ssl, null);
7980
fail();
8081
}
8182
catch (IllegalStateException ex) {
8283
Throwable cause = ex.getCause();
83-
assertThat(cause).isInstanceOf(IllegalArgumentException.class);
84-
assertThat(cause).hasMessageContaining("Resource location must not be null");
84+
assertThat(cause).isInstanceOf(WebServerException.class);
85+
assertThat(cause).hasMessageContaining("Could not load key store 'null'");
8586
}
8687
}
8788

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -37,11 +37,13 @@
3737
import org.springframework.boot.testsupport.rule.OutputCapture;
3838
import org.springframework.boot.web.server.Ssl;
3939
import org.springframework.boot.web.server.SslStoreProvider;
40+
import org.springframework.boot.web.server.WebServerException;
4041
import org.springframework.core.io.ClassPathResource;
4142
import org.springframework.core.io.Resource;
4243
import org.springframework.test.util.ReflectionTestUtils;
4344

4445
import static org.assertj.core.api.Assertions.assertThat;
46+
import static org.junit.Assert.fail;
4547
import static org.mockito.BDDMockito.given;
4648
import static org.mockito.Mockito.mock;
4749

@@ -189,6 +191,19 @@ public void customizeWhenSslStoreProviderPresentShouldIgnorePasswordFromSsl()
189191
assertThat(this.output.toString()).doesNotContain("Password verification failed");
190192
}
191193

194+
@Test
195+
public void customizeWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException() {
196+
try {
197+
new SslConnectorCustomizer(new Ssl(), null)
198+
.customize(this.tomcat.getConnector());
199+
fail();
200+
}
201+
catch (Exception ex) {
202+
assertThat(ex).isInstanceOf(WebServerException.class);
203+
assertThat(ex).hasMessageContaining("Could not load key store 'null'");
204+
}
205+
}
206+
192207
private KeyStore loadStore() throws KeyStoreException, IOException,
193208
NoSuchAlgorithmException, CertificateException {
194209
KeyStore keyStore = KeyStore.getInstance("JKS");

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/undertow/SslBuilderCustomizerTests.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424
import org.junit.Test;
2525

2626
import org.springframework.boot.web.server.Ssl;
27+
import org.springframework.boot.web.server.WebServerException;
2728
import org.springframework.test.util.ReflectionTestUtils;
2829

2930
import static org.assertj.core.api.Assertions.assertThat;
@@ -90,22 +91,19 @@ public void trustStoreProviderIsUsedWhenCreatingTrustStore() throws Exception {
9091
}
9192

9293
@Test
93-
public void getKeyManagersWhenKeyStoreIsNotProvided() throws Exception {
94+
public void getKeyManagersWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException()
95+
throws Exception {
9496
Ssl ssl = new Ssl();
95-
ssl.setKeyPassword("password");
9697
SslBuilderCustomizer customizer = new SslBuilderCustomizer(8080,
9798
InetAddress.getLocalHost(), ssl, null);
9899
try {
99-
KeyManager[] keyManagers = ReflectionTestUtils.invokeMethod(customizer,
100-
"getKeyManagers", ssl, null);
101-
Class<?> name = Class.forName("org.springframework.boot.web.embedded.undertow"
102-
+ ".SslBuilderCustomizer$ConfigurableAliasKeyManager");
103-
assertThat(keyManagers[0]).isNotInstanceOf(name);
100+
ReflectionTestUtils.invokeMethod(customizer, "getKeyManagers", ssl, null);
101+
fail();
104102
}
105103
catch (IllegalStateException ex) {
106104
Throwable cause = ex.getCause();
107-
assertThat(cause).isInstanceOf(IllegalArgumentException.class);
108-
assertThat(cause).hasMessageContaining("Resource location must not be null");
105+
assertThat(cause).isInstanceOf(WebServerException.class);
106+
assertThat(cause).hasMessageContaining("Could not load key store 'null'");
109107
}
110108
}
111109

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2018 the original author or authors.
2+
* Copyright 2012-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -62,6 +62,7 @@
6262
import org.springframework.web.reactive.function.client.WebClient;
6363

6464
import static org.assertj.core.api.Assertions.assertThat;
65+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
6566

6667
/**
6768
* Base for testing classes that extends {@link AbstractReactiveWebServerFactory}.
@@ -291,6 +292,12 @@ public void noCompressionForUserAgent() {
291292
assertResponseIsNotCompressed(response);
292293
}
293294

295+
@Test
296+
public void whenSslIsEnabledAndNoKeyStoreIsConfiguredThenServerFailsToStart() {
297+
assertThatThrownBy(() -> testBasicSslWithKeyStore(null, null))
298+
.hasMessageContaining("Could not load key store 'null'");
299+
}
300+
294301
protected WebClient prepareCompressionTest() {
295302
Compression compression = new Compression();
296303
compression.setEnabled(true);

0 commit comments

Comments
 (0)