Skip to content

Commit bc6fc33

Browse files
philwebbphilwebb
committed
Polish
1 parent 1939d23 commit bc6fc33

File tree

12 files changed

+178
-219
lines changed

12 files changed

+178
-219
lines changed

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.springframework.util.ClassUtils;
4545
import org.springframework.util.ObjectUtils;
4646
import org.springframework.util.ResourceUtils;
47+
import org.springframework.util.StringUtils;
4748

4849
/**
4950
* {@link JettyServerCustomizer} that configures SSL on the given Jetty server instance.
@@ -224,10 +225,8 @@ private void configureSslKeyStore(SslContextFactory.Server factory, Ssl ssl) {
224225
String keystoreType = (ssl.getKeyStoreType() != null) ? ssl.getKeyStoreType() : "JKS";
225226
String keystoreLocation = ssl.getKeyStore();
226227
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-
}
228+
Assert.state(!StringUtils.hasText(keystoreLocation),
229+
() -> "Keystore location '" + keystoreLocation + "' must be empty or null for PKCS11 key stores");
231230
}
232231
else {
233232
try {
@@ -240,7 +239,7 @@ private void configureSslKeyStore(SslContextFactory.Server factory, Ssl ssl) {
240239
}
241240
factory.setKeyStoreType(keystoreType);
242241
if (ssl.getKeyStoreProvider() != null) {
243-
factory.setKeyStoreProvider(ssl.getKeyStoreProvider());
242+
factory.setKeyStoreProvider(this.ssl.getKeyStoreProvider());
244243
}
245244
}
246245

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

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@
4848
import org.springframework.boot.web.server.SslConfigurationValidator;
4949
import org.springframework.boot.web.server.SslStoreProvider;
5050
import org.springframework.boot.web.server.WebServerException;
51+
import org.springframework.util.Assert;
5152
import org.springframework.util.ResourceUtils;
53+
import org.springframework.util.StringUtils;
5254

5355
/**
5456
* {@link NettyServerCustomizer} that configures SSL for the given Reactor Netty server
@@ -169,25 +171,26 @@ private KeyStore loadTrustStore(String type, String provider, String resource, S
169171
return loadStore(type, provider, resource, password);
170172
}
171173

172-
private KeyStore loadStore(String type, String provider, String resource, String password) throws Exception {
173-
type = (type != null) ? type : "JKS";
174-
KeyStore store = (provider != null) ? KeyStore.getInstance(type, provider) : KeyStore.getInstance(type);
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.");
179-
}
180-
store.load(null, (password != null) ? password.toCharArray() : null);
174+
private KeyStore loadStore(String keystoreType, String provider, String keystoreLocation, String password)
175+
throws Exception {
176+
keystoreType = (keystoreType != null) ? keystoreType : "JKS";
177+
char[] passwordChars = (password != null) ? password.toCharArray() : null;
178+
KeyStore store = (provider != null) ? KeyStore.getInstance(keystoreType, provider)
179+
: KeyStore.getInstance(keystoreType);
180+
if (keystoreType.equalsIgnoreCase("PKCS11")) {
181+
Assert.state(!StringUtils.hasText(keystoreLocation),
182+
() -> "Keystore location '" + keystoreLocation + "' must be empty or null for PKCS11 key stores");
183+
store.load(null, passwordChars);
181184
}
182185
else {
183186
try {
184-
URL url = ResourceUtils.getURL(resource);
187+
URL url = ResourceUtils.getURL(keystoreLocation);
185188
try (InputStream stream = url.openStream()) {
186-
store.load(stream, (password != null) ? password.toCharArray() : null);
189+
store.load(stream, passwordChars);
187190
}
188191
}
189192
catch (Exception ex) {
190-
throw new WebServerException("Could not load key store '" + resource + "'", ex);
193+
throw new WebServerException("Could not load key store '" + keystoreLocation + "'", ex);
191194
}
192195
}
193196
return store;

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,8 @@ private void configureSslKeyStore(SSLHostConfigCertificate certificate, Ssl ssl)
143143
String keystoreType = (ssl.getKeyStoreType() != null) ? ssl.getKeyStoreType() : "JKS";
144144
String keystoreLocation = ssl.getKeyStore();
145145
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-
}
146+
Assert.state(!StringUtils.hasText(keystoreLocation),
147+
() -> "Keystore location '" + keystoreLocation + "' must be empty or null for PKCS11 key stores");
150148
}
151149
else {
152150
try {

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

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

1919
import java.net.InetSocketAddress;
20-
import java.security.Provider;
21-
import java.security.Security;
2220
import java.util.ArrayList;
2321
import java.util.List;
2422

@@ -29,20 +27,19 @@
2927
import org.eclipse.jetty.server.Server;
3028
import org.eclipse.jetty.server.SslConnectionFactory;
3129
import org.eclipse.jetty.util.ssl.SslContextFactory;
32-
import org.junit.jupiter.api.AfterAll;
33-
import org.junit.jupiter.api.BeforeAll;
3430
import org.junit.jupiter.api.Test;
3531
import org.junit.jupiter.api.condition.OS;
3632

3733
import org.springframework.boot.testsupport.junit.DisabledOnOs;
38-
import org.springframework.boot.web.embedded.netty.MockPkcs11SecurityProvider;
34+
import org.springframework.boot.web.embedded.test.MockPkcs11Security;
35+
import org.springframework.boot.web.embedded.test.MockPkcs11SecurityProvider;
3936
import org.springframework.boot.web.server.Http2;
4037
import org.springframework.boot.web.server.Ssl;
4138
import org.springframework.boot.web.server.WebServerException;
4239

4340
import static org.assertj.core.api.Assertions.assertThat;
4441
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
45-
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
42+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
4643
import static org.assertj.core.api.Assertions.assertThatNoException;
4744

4845
/**
@@ -51,25 +48,9 @@
5148
* @author Andy Wilkinson
5249
* @author Cyril Dangerville
5350
*/
51+
@MockPkcs11Security
5452
class SslServerCustomizerTests {
5553

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-
7354
@Test
7455
@SuppressWarnings("rawtypes")
7556
void whenHttp2IsNotEnabledServerConnectorHasSslAndHttpConnectionFactories() {
@@ -107,11 +88,8 @@ void alpnConnectionFactoryHasNullDefaultProtocolToAllowNegotiationToHttp11() {
10788
assertThat(((ALPNServerConnectionFactory) factories.get(1)).getDefaultProtocol()).isNull();
10889
}
10990

110-
/**
111-
* Null/undefined keystore is invalid unless keystore type is PKCS11.
112-
*/
11391
@Test
114-
void configureSslWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsWebServerException() {
92+
void configureSslWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsException() {
11593
Ssl ssl = new Ssl();
11694
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
11795
assertThatExceptionOfType(Exception.class)
@@ -122,30 +100,26 @@ void configureSslWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsWebServerExcept
122100
});
123101
}
124102

125-
/**
126-
* No keystore path should be defined if keystore type is PKCS#11.
127-
*/
128103
@Test
129-
void configureSslWhenSslIsEnabledWithPkcs11AndKeyStoreThrowsIllegalArgumentException() {
104+
void configureSslWhenSslIsEnabledWithPkcs11AndKeyStoreThrowsException() {
130105
Ssl ssl = new Ssl();
131106
ssl.setKeyStoreType("PKCS11");
132-
ssl.setKeyStoreProvider(PKCS11_PROVIDER.getName());
107+
ssl.setKeyStoreProvider(MockPkcs11SecurityProvider.NAME);
133108
ssl.setKeyStore("src/test/resources/test.jks");
134109
ssl.setKeyPassword("password");
135110
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
136-
assertThatIllegalArgumentException()
111+
assertThatIllegalStateException()
137112
.isThrownBy(() -> customizer.configureSsl(new SslContextFactory.Server(), ssl, null))
138-
.withMessageContaining("Input keystore location is not valid for keystore type 'PKCS11'");
113+
.withMessageContaining("must be empty or null for PKCS11 key stores");
139114
}
140115

141116
@Test
142117
void customizeWhenSslIsEnabledWithPkcs11AndKeyStoreProvider() {
143118
Ssl ssl = new Ssl();
144119
ssl.setKeyStoreType("PKCS11");
145-
ssl.setKeyStoreProvider(PKCS11_PROVIDER.getName());
120+
ssl.setKeyStoreProvider(MockPkcs11SecurityProvider.NAME);
146121
ssl.setKeyStorePassword("1234");
147122
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
148-
// Loading the KeyManagerFactory should be successful
149123
assertThatNoException().isThrownBy(() -> customizer.configureSsl(new SslContextFactory.Server(), ssl, null));
150124
}
151125

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

Lines changed: 0 additions & 48 deletions
This file was deleted.

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

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@
1717
package org.springframework.boot.web.embedded.netty;
1818

1919
import java.security.NoSuchProviderException;
20-
import java.security.Provider;
21-
import java.security.Security;
2220

23-
import org.junit.jupiter.api.AfterAll;
24-
import org.junit.jupiter.api.BeforeAll;
2521
import org.junit.jupiter.api.Test;
2622

23+
import org.springframework.boot.web.embedded.test.MockPkcs11Security;
24+
import org.springframework.boot.web.embedded.test.MockPkcs11SecurityProvider;
2725
import org.springframework.boot.web.server.Ssl;
2826
import org.springframework.boot.web.server.WebServerException;
2927

@@ -38,29 +36,9 @@
3836
* @author Cyril Dangerville
3937
*/
4038
@SuppressWarnings("deprecation")
39+
@MockPkcs11Security
4140
class SslServerCustomizerTests {
4241

43-
private static final Provider PKCS11_PROVIDER = new MockPkcs11SecurityProvider();
44-
45-
@BeforeAll
46-
static void setup() {
47-
/*
48-
* Add the mock Java security provider for PKCS#11-related unit tests.
49-
*
50-
* For an integration test with an actual PKCS#11 library - SoftHSM - properly
51-
* installed and configured on the system (inside a container), used via Java
52-
* built-in SunPKCS11 provider, see the 'spring-boot-smoke-test-webflux-ssl'
53-
* project in 'spring-boot-tests/spring-boot-smoke-tests' folder.
54-
*/
55-
Security.addProvider(PKCS11_PROVIDER);
56-
}
57-
58-
@AfterAll
59-
static void shutdown() {
60-
// Remove the provider previously added in setup()
61-
Security.removeProvider(PKCS11_PROVIDER.getName());
62-
}
63-
6442
@Test
6543
void keyStoreProviderIsUsedWhenCreatingKeyStore() {
6644
Ssl ssl = new Ssl();
@@ -85,41 +63,34 @@ void trustStoreProviderIsUsedWhenCreatingTrustStore() {
8563
.withMessageContaining("com.example.TrustStoreProvider");
8664
}
8765

88-
/**
89-
* Null/undefined keystore is not valid unless keystore type is PKCS11.
90-
*/
9166
@Test
92-
void getKeyManagerFactoryWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsWebServerException() {
67+
void getKeyManagerFactoryWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsException() {
9368
Ssl ssl = new Ssl();
9469
SslServerCustomizer customizer = new SslServerCustomizer(ssl, null, null);
9570
assertThatIllegalStateException().isThrownBy(() -> customizer.getKeyManagerFactory(ssl, null))
9671
.withCauseInstanceOf(WebServerException.class).withMessageContaining("Could not load key store 'null'");
9772
}
9873

99-
/**
100-
* No keystore path should be defined if keystore type is PKCS#11.
101-
*/
10274
@Test
103-
void getKeyManagerFactoryWhenSslIsEnabledWithPkcs11AndKeyStoreThrowsIllegalArgumentException() {
75+
void getKeyManagerFactoryWhenSslIsEnabledWithPkcs11AndKeyStoreThrowsException() {
10476
Ssl ssl = new Ssl();
10577
ssl.setKeyStoreType("PKCS11");
106-
ssl.setKeyStoreProvider(PKCS11_PROVIDER.getName());
78+
ssl.setKeyStoreProvider(MockPkcs11SecurityProvider.NAME);
10779
ssl.setKeyStore("src/test/resources/test.jks");
10880
ssl.setKeyPassword("password");
10981
SslServerCustomizer customizer = new SslServerCustomizer(ssl, null, null);
11082
assertThatIllegalStateException().isThrownBy(() -> customizer.getKeyManagerFactory(ssl, null))
111-
.withCauseInstanceOf(IllegalArgumentException.class)
112-
.withMessageContaining("Input keystore location is not valid for keystore type 'PKCS11'");
83+
.withCauseInstanceOf(IllegalStateException.class)
84+
.withMessageContaining("must be empty or null for PKCS11 key stores");
11385
}
11486

11587
@Test
11688
void getKeyManagerFactoryWhenSslIsEnabledWithPkcs11AndKeyStoreProvider() {
11789
Ssl ssl = new Ssl();
11890
ssl.setKeyStoreType("PKCS11");
119-
ssl.setKeyStoreProvider(PKCS11_PROVIDER.getName());
91+
ssl.setKeyStoreProvider(MockPkcs11SecurityProvider.NAME);
12092
ssl.setKeyStorePassword("1234");
12193
SslServerCustomizer customizer = new SslServerCustomizer(ssl, null, null);
122-
// Loading the KeyManagerFactory should be successful
12394
assertThatNoException().isThrownBy(() -> customizer.getKeyManagerFactory(ssl, null));
12495
}
12596

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/MockKeyStoreSpi.java‎ renamed to ‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/test/MockKeyStoreSpi.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
package org.springframework.boot.web.embedded.netty;
17+
package org.springframework.boot.web.embedded.test;
1818

1919
import java.io.InputStream;
2020
import java.io.OutputStream;
@@ -31,7 +31,7 @@
3131
import java.util.Map;
3232

3333
/**
34-
* Mock Security Provider for testing purposes only (e.g. SslServerCustomizerTests class)
34+
* Mock Security Provider for testing purposes.
3535
*
3636
* @author Cyril Dangerville
3737
*/
@@ -45,7 +45,7 @@ public class MockKeyStoreSpi extends KeyStoreSpi {
4545
KEYGEN.initialize(2048);
4646
}
4747
catch (NoSuchAlgorithmException ex) {
48-
throw new RuntimeException(ex);
48+
throw new IllegalStateException(ex);
4949
}
5050
}
5151

@@ -99,8 +99,6 @@ public Enumeration<String> engineAliases() {
9999

100100
@Override
101101
public boolean engineContainsAlias(String alias) {
102-
// contains any required alias, for testing purposes
103-
// Add alias to aliases list on the fly
104102
this.aliases.put(alias, KEYGEN.generateKeyPair());
105103
return true;
106104
}
@@ -112,7 +110,6 @@ public int engineSize() {
112110

113111
@Override
114112
public boolean engineIsKeyEntry(String alias) {
115-
// Handle all keystore entries as key entries
116113
return this.aliases.containsKey(alias);
117114
}
118115

@@ -133,7 +130,6 @@ public void engineStore(OutputStream stream, char[] password) {
133130

134131
@Override
135132
public void engineLoad(InputStream stream, char[] password) {
136-
// Nothing to do, this is a mock keystore implementation, for testing only.
137133
}
138134

139135
}

0 commit comments

Comments
 (0)