Skip to content

Commit ad2b130

Browse files
mhalbrittermhalbritter
committed
Merge pull request spring-projects#32179 from cdanger
* 32179: Polish "Fix bug in webserver start when loading PKCS#11 KeyStore" Fix bug in webserver start when loading PKCS#11 KeyStore Closes spring-projectsgh-32179
2 parents 5b2b122 + 1656909 commit ad2b130

File tree

17 files changed

+732
-34
lines changed

17 files changed

+732
-34
lines changed

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
* @author Brian Clozel
5252
* @author Olivier Lamy
5353
* @author Chris Bono
54+
* @author Cyril Dangerville
5455
*/
5556
class SslServerCustomizer implements JettyServerCustomizer {
5657

@@ -220,16 +221,24 @@ private void configureSslPasswords(SslContextFactory.Server factory, Ssl ssl) {
220221
}
221222

222223
private void configureSslKeyStore(SslContextFactory.Server factory, Ssl ssl) {
223-
try {
224-
URL url = ResourceUtils.getURL(ssl.getKeyStore());
225-
factory.setKeyStoreResource(Resource.newResource(url));
226-
}
227-
catch (Exception ex) {
228-
throw new WebServerException("Could not load key store '" + ssl.getKeyStore() + "'", ex);
224+
String keystoreType = (ssl.getKeyStoreType() != null) ? ssl.getKeyStoreType() : "JKS";
225+
String keystoreLocation = ssl.getKeyStore();
226+
if (keystoreType.equalsIgnoreCase("PKCS11")) {
227+
if (keystoreLocation != null && !keystoreLocation.isEmpty()) {
228+
throw new IllegalArgumentException("Input keystore location is not valid for keystore type 'PKCS11': '"
229+
+ keystoreLocation + "'. Must be undefined / null.");
230+
}
229231
}
230-
if (ssl.getKeyStoreType() != null) {
231-
factory.setKeyStoreType(ssl.getKeyStoreType());
232+
else {
233+
try {
234+
URL url = ResourceUtils.getURL(keystoreLocation);
235+
factory.setKeyStoreResource(Resource.newResource(url));
236+
}
237+
catch (Exception ex) {
238+
throw new WebServerException("Could not load key store '" + keystoreLocation + "'", ex);
239+
}
232240
}
241+
factory.setKeyStoreType(keystoreType);
233242
if (ssl.getKeyStoreProvider() != null) {
234243
factory.setKeyStoreProvider(ssl.getKeyStoreProvider());
235244
}

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
* @author Brian Clozel
5858
* @author Raheela Aslam
5959
* @author Chris Bono
60+
* @author Cyril Dangerville
6061
* @since 2.0.0
6162
* @deprecated this class is meant for Spring Boot internal use only.
6263
*/
@@ -171,17 +172,25 @@ private KeyStore loadTrustStore(String type, String provider, String resource, S
171172
private KeyStore loadStore(String type, String provider, String resource, String password) throws Exception {
172173
type = (type != null) ? type : "JKS";
173174
KeyStore store = (provider != null) ? KeyStore.getInstance(type, provider) : KeyStore.getInstance(type);
174-
try {
175-
URL url = ResourceUtils.getURL(resource);
176-
try (InputStream stream = url.openStream()) {
177-
store.load(stream, (password != null) ? password.toCharArray() : null);
175+
if (type.equalsIgnoreCase("PKCS11")) {
176+
if (resource != null && !resource.isEmpty()) {
177+
throw new IllegalArgumentException("Input keystore location is not valid for keystore type 'PKCS11': '"
178+
+ resource + "'. Must be undefined / null.");
178179
}
179-
return store;
180+
store.load(null, (password != null) ? password.toCharArray() : null);
180181
}
181-
catch (Exception ex) {
182-
throw new WebServerException("Could not load key store '" + resource + "'", ex);
182+
else {
183+
try {
184+
URL url = ResourceUtils.getURL(resource);
185+
try (InputStream stream = url.openStream()) {
186+
store.load(stream, (password != null) ? password.toCharArray() : null);
187+
}
188+
}
189+
catch (Exception ex) {
190+
throw new WebServerException("Could not load key store '" + resource + "'", ex);
191+
}
183192
}
184-
193+
return store;
185194
}
186195

187196
/**

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
* @author Brian Clozel
4040
* @author Andy Wilkinson
4141
* @author Scott Frederick
42+
* @author Cyril Dangerville
4243
*/
4344
class SslConnectorCustomizer implements TomcatConnectorCustomizer {
4445

@@ -139,15 +140,23 @@ protected void configureSslStoreProvider(AbstractHttp11JsseProtocol<?> protocol,
139140
}
140141

141142
private void configureSslKeyStore(SSLHostConfigCertificate certificate, Ssl ssl) {
142-
try {
143-
certificate.setCertificateKeystoreFile(ResourceUtils.getURL(ssl.getKeyStore()).toString());
144-
}
145-
catch (Exception ex) {
146-
throw new WebServerException("Could not load key store '" + ssl.getKeyStore() + "'", ex);
143+
String keystoreType = (ssl.getKeyStoreType() != null) ? ssl.getKeyStoreType() : "JKS";
144+
String keystoreLocation = ssl.getKeyStore();
145+
if (keystoreType.equalsIgnoreCase("PKCS11")) {
146+
if (keystoreLocation != null && !keystoreLocation.isEmpty()) {
147+
throw new IllegalArgumentException("Input keystore location is not valid for keystore type 'PKCS11': '"
148+
+ keystoreLocation + "'. Must be undefined / null.");
149+
}
147150
}
148-
if (ssl.getKeyStoreType() != null) {
149-
certificate.setCertificateKeystoreType(ssl.getKeyStoreType());
151+
else {
152+
try {
153+
certificate.setCertificateKeystoreFile(ResourceUtils.getURL(keystoreLocation).toString());
154+
}
155+
catch (Exception ex) {
156+
throw new WebServerException("Could not load key store '" + keystoreLocation + "'", ex);
157+
}
150158
}
159+
certificate.setCertificateKeystoreType(keystoreType);
151160
if (ssl.getKeyStoreProvider() != null) {
152161
certificate.setCertificateKeystoreProvider(ssl.getKeyStoreProvider());
153162
}

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
*
5252
* @author Brian Clozel
5353
* @author Raheela Aslam
54+
* @author Cyril Dangerville
5455
*/
5556
class SslBuilderCustomizer implements UndertowBuilderCustomizer {
5657

@@ -180,16 +181,26 @@ private KeyStore loadTrustStore(String type, String provider, String resource, S
180181
private KeyStore loadStore(String type, String provider, String resource, String password) throws Exception {
181182
type = (type != null) ? type : "JKS";
182183
KeyStore store = (provider != null) ? KeyStore.getInstance(type, provider) : KeyStore.getInstance(type);
183-
try {
184-
URL url = ResourceUtils.getURL(resource);
185-
try (InputStream stream = url.openStream()) {
186-
store.load(stream, (password != null) ? password.toCharArray() : null);
184+
if (type.equalsIgnoreCase("PKCS11")) {
185+
if (resource != null && !resource.isEmpty()) {
186+
throw new IllegalArgumentException("Input keystore location is not valid for keystore type 'PKCS11': '"
187+
+ resource + "'. Must be undefined / null.");
187188
}
188-
return store;
189+
store.load(null, (password != null) ? password.toCharArray() : null);
189190
}
190-
catch (Exception ex) {
191-
throw new WebServerException("Could not load key store '" + resource + "'", ex);
191+
else {
192+
try {
193+
URL url = ResourceUtils.getURL(resource);
194+
try (InputStream stream = url.openStream()) {
195+
store.load(stream, (password != null) ? password.toCharArray() : null);
196+
}
197+
}
198+
catch (Exception ex) {
199+
throw new WebServerException("Could not load key store '" + resource + "'", ex);
200+
}
192201
}
202+
203+
return store;
193204
}
194205

195206
/**

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

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package org.springframework.boot.web.embedded.jetty;
1818

1919
import java.net.InetSocketAddress;
20+
import java.security.Provider;
21+
import java.security.Security;
2022
import java.util.ArrayList;
2123
import java.util.List;
2224

@@ -27,24 +29,47 @@
2729
import org.eclipse.jetty.server.Server;
2830
import org.eclipse.jetty.server.SslConnectionFactory;
2931
import org.eclipse.jetty.util.ssl.SslContextFactory;
32+
import org.junit.jupiter.api.AfterAll;
33+
import org.junit.jupiter.api.BeforeAll;
3034
import org.junit.jupiter.api.Test;
3135
import org.junit.jupiter.api.condition.OS;
3236

3337
import org.springframework.boot.testsupport.junit.DisabledOnOs;
38+
import org.springframework.boot.web.embedded.netty.MockPkcs11SecurityProvider;
3439
import org.springframework.boot.web.server.Http2;
3540
import org.springframework.boot.web.server.Ssl;
3641
import org.springframework.boot.web.server.WebServerException;
3742

3843
import static org.assertj.core.api.Assertions.assertThat;
3944
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
45+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
46+
import static org.assertj.core.api.Assertions.assertThatNoException;
4047

4148
/**
4249
* Tests for {@link SslServerCustomizer}.
4350
*
4451
* @author Andy Wilkinson
52+
* @author Cyril Dangerville
4553
*/
4654
class SslServerCustomizerTests {
4755

56+
private static final Provider PKCS11_PROVIDER = new MockPkcs11SecurityProvider();
57+
58+
@BeforeAll
59+
static void beforeAllTests() {
60+
/*
61+
* Add the mock Java security provider for PKCS#11-related unit tests.
62+
*
63+
*/
64+
Security.addProvider(PKCS11_PROVIDER);
65+
}
66+
67+
@AfterAll
68+
static void afterAllTests() {
69+
// Remove the provider previously added in setup()
70+
Security.removeProvider(PKCS11_PROVIDER.getName());
71+
}
72+
4873
@Test
4974
@SuppressWarnings("rawtypes")
5075
void whenHttp2IsNotEnabledServerConnectorHasSslAndHttpConnectionFactories() {
@@ -82,8 +107,11 @@ void alpnConnectionFactoryHasNullDefaultProtocolToAllowNegotiationToHttp11() {
82107
assertThat(((ALPNServerConnectionFactory) factories.get(1)).getDefaultProtocol()).isNull();
83108
}
84109

110+
/**
111+
* Null/undefined keystore is invalid unless keystore type is PKCS11.
112+
*/
85113
@Test
86-
void configureSslWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException() {
114+
void configureSslWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsWebServerException() {
87115
Ssl ssl = new Ssl();
88116
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
89117
assertThatExceptionOfType(Exception.class)
@@ -94,6 +122,33 @@ void configureSslWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException() {
94122
});
95123
}
96124

125+
/**
126+
* No keystore path should be defined if keystore type is PKCS#11.
127+
*/
128+
@Test
129+
void configureSslWhenSslIsEnabledWithPkcs11AndKeyStoreThrowsIllegalArgumentException() {
130+
Ssl ssl = new Ssl();
131+
ssl.setKeyStoreType("PKCS11");
132+
ssl.setKeyStoreProvider(PKCS11_PROVIDER.getName());
133+
ssl.setKeyStore("src/test/resources/test.jks");
134+
ssl.setKeyPassword("password");
135+
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
136+
assertThatIllegalArgumentException()
137+
.isThrownBy(() -> customizer.configureSsl(new SslContextFactory.Server(), ssl, null))
138+
.withMessageContaining("Input keystore location is not valid for keystore type 'PKCS11'");
139+
}
140+
141+
@Test
142+
void customizeWhenSslIsEnabledWithPkcs11AndKeyStoreProvider() {
143+
Ssl ssl = new Ssl();
144+
ssl.setKeyStoreType("PKCS11");
145+
ssl.setKeyStoreProvider(PKCS11_PROVIDER.getName());
146+
ssl.setKeyStorePassword("1234");
147+
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
148+
// Loading the KeyManagerFactory should be successful
149+
assertThatNoException().isThrownBy(() -> customizer.configureSsl(new SslContextFactory.Server(), ssl, null));
150+
}
151+
97152
private Server createCustomizedServer() {
98153
return createCustomizedServer(new Http2());
99154
}

0 commit comments

Comments
 (0)