Merge pull request #15709 from Raheela Aslam

* gh-15709:
  Polish "Always fail fast when SSL is enabled without a key store"
  Always fail fast when SSL is enabled without a key store
This commit is contained in:
Andy Wilkinson 2019-01-22 11:53:28 +00:00
commit c488934c7d
9 changed files with 143 additions and 24 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -190,9 +190,9 @@ class SslServerCustomizer implements JettyServerCustomizer {
URL url = ResourceUtils.getURL(ssl.getKeyStore());
factory.setKeyStoreResource(Resource.newResource(url));
}
catch (IOException ex) {
catch (Exception ex) {
throw new WebServerException(
"Could not find key store '" + ssl.getKeyStore() + "'", ex);
"Could not load key store '" + ssl.getKeyStore() + "'", ex);
}
if (ssl.getKeyStoreType() != null) {
factory.setKeyStoreType(ssl.getKeyStoreType());

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -31,6 +31,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.WebServerException;
import org.springframework.util.ResourceUtils;
/**
@ -38,6 +39,7 @@ import org.springframework.util.ResourceUtils;
* instance.
*
* @author Brian Clozel
* @author Raheela Aslam
*/
public class SslServerCustomizer implements NettyServerCustomizer {
@ -135,21 +137,42 @@ public class SslServerCustomizer implements NettyServerCustomizer {
if (sslStoreProvider != null) {
return sslStoreProvider.getTrustStore();
}
return loadKeyStore(ssl.getTrustStoreType(), ssl.getTrustStoreProvider(),
return loadTrustStore(ssl.getTrustStoreType(), ssl.getTrustStoreProvider(),
ssl.getTrustStore(), ssl.getTrustStorePassword());
}
private KeyStore loadKeyStore(String type, String provider, String resource,
String password) throws Exception {
type = (type != null) ? type : "JKS";
return loadStore(type, provider, resource, password);
}
private KeyStore loadTrustStore(String type, String provider, String resource,
String password) throws Exception {
if (resource == null) {
return null;
}
else {
return loadStore(type, provider, resource, password);
}
}
private KeyStore loadStore(String type, String provider, String resource,
String password) throws Exception {
type = (type != null) ? type : "JKS";
KeyStore store = (provider != null) ? KeyStore.getInstance(type, provider)
: KeyStore.getInstance(type);
URL url = ResourceUtils.getURL(resource);
store.load(url.openStream(), (password != null) ? password.toCharArray() : null);
return store;
try {
URL url = ResourceUtils.getURL(resource);
store.load(url.openStream(),
(password != null) ? password.toCharArray() : null);
return store;
}
catch (Exception ex) {
throw new WebServerException("Could not load key store '" + resource + "'",
ex);
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -132,9 +132,9 @@ class SslConnectorCustomizer implements TomcatConnectorCustomizer {
try {
protocol.setKeystoreFile(ResourceUtils.getURL(ssl.getKeyStore()).toString());
}
catch (FileNotFoundException ex) {
throw new WebServerException("Could not load key store: " + ex.getMessage(),
ex);
catch (Exception ex) {
throw new WebServerException(
"Could not load key store '" + ssl.getKeyStore() + "'", ex);
}
if (ssl.getKeyStoreType() != null) {
protocol.setKeystoreType(ssl.getKeyStoreType());

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -41,12 +41,14 @@ import org.xnio.SslClientAuthMode;
import org.springframework.boot.web.server.Ssl;
import org.springframework.boot.web.server.SslStoreProvider;
import org.springframework.boot.web.server.WebServerException;
import org.springframework.util.ResourceUtils;
/**
* {@link UndertowBuilderCustomizer} that configures SSL on the given builder instance.
*
* @author Brian Clozel
* @author Raheela Aslam
*/
class SslBuilderCustomizer implements UndertowBuilderCustomizer {
@ -166,21 +168,40 @@ class SslBuilderCustomizer implements UndertowBuilderCustomizer {
if (sslStoreProvider != null) {
return sslStoreProvider.getTrustStore();
}
return loadKeyStore(ssl.getTrustStoreType(), ssl.getTrustStoreProvider(),
return loadTrustStore(ssl.getTrustStoreType(), ssl.getTrustStoreProvider(),
ssl.getTrustStore(), ssl.getTrustStorePassword());
}
private KeyStore loadKeyStore(String type, String provider, String resource,
String password) throws Exception {
type = (type != null) ? type : "JKS";
return loadStore(type, provider, resource, password);
}
private KeyStore loadTrustStore(String type, String provider, String resource,
String password) throws Exception {
if (resource == null) {
return null;
}
else {
return loadStore(type, provider, resource, password);
}
}
private KeyStore loadStore(String type, String provider, String resource,
String password) throws Exception {
type = (type != null) ? type : "JKS";
KeyStore store = (provider != null) ? KeyStore.getInstance(type, provider)
: KeyStore.getInstance(type);
URL url = ResourceUtils.getURL(resource);
store.load(url.openStream(), (password != null) ? password.toCharArray() : null);
return store;
try {
URL url = ResourceUtils.getURL(resource);
store.load(url.openStream(),
(password != null) ? password.toCharArray() : null);
return store;
}
catch (Exception ex) {
throw new WebServerException("Could not load key store '" + resource + "'",
ex);
}
}
/**

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -26,10 +26,12 @@ import org.eclipse.jetty.server.ConnectionFactory;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.Test;
import org.springframework.boot.web.server.Http2;
import org.springframework.boot.web.server.Ssl;
import org.springframework.boot.web.server.WebServerException;
import static org.assertj.core.api.Assertions.assertThat;
@ -78,6 +80,20 @@ public class SslServerCustomizerTests {
.isNull();
}
@Test
public void configureSslWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException()
throws Exception {
Ssl ssl = new Ssl();
SslServerCustomizer customizer = new SslServerCustomizer(null, ssl, null, null);
try {
customizer.configureSsl(new SslContextFactory(), ssl, null);
}
catch (Exception ex) {
assertThat(ex).isInstanceOf(WebServerException.class);
assertThat(ex).hasMessageContaining("Could not load key store 'null'");
}
}
private Server createCustomizedServer() {
return createCustomizedServer(new Http2());
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -21,6 +21,7 @@ import java.security.NoSuchProviderException;
import org.junit.Test;
import org.springframework.boot.web.server.Ssl;
import org.springframework.boot.web.server.WebServerException;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;
@ -29,6 +30,7 @@ import static org.junit.Assert.fail;
* Tests for {@link SslServerCustomizer}.
*
* @author Andy Wilkinson
* @author Raheela Aslam
*/
public class SslServerCustomizerTests {
@ -68,4 +70,20 @@ public class SslServerCustomizerTests {
}
}
@Test
public void getKeyManagerFactoryWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException()
throws Exception {
Ssl ssl = new Ssl();
SslServerCustomizer customizer = new SslServerCustomizer(ssl, null, null);
try {
customizer.getKeyManagerFactory(ssl, null);
fail();
}
catch (IllegalStateException ex) {
Throwable cause = ex.getCause();
assertThat(cause).isInstanceOf(WebServerException.class);
assertThat(cause).hasMessageContaining("Could not load key store 'null'");
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -37,11 +37,13 @@ import org.junit.Test;
import org.springframework.boot.testsupport.rule.OutputCapture;
import org.springframework.boot.web.server.Ssl;
import org.springframework.boot.web.server.SslStoreProvider;
import org.springframework.boot.web.server.WebServerException;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.Resource;
import org.springframework.test.util.ReflectionTestUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
@ -189,6 +191,19 @@ public class SslConnectorCustomizerTests {
assertThat(this.output.toString()).doesNotContain("Password verification failed");
}
@Test
public void customizeWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException() {
try {
new SslConnectorCustomizer(new Ssl(), null)
.customize(this.tomcat.getConnector());
fail();
}
catch (Exception ex) {
assertThat(ex).isInstanceOf(WebServerException.class);
assertThat(ex).hasMessageContaining("Could not load key store 'null'");
}
}
private KeyStore loadStore() throws KeyStoreException, IOException,
NoSuchAlgorithmException, CertificateException {
KeyStore keyStore = KeyStore.getInstance("JKS");

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -24,6 +24,7 @@ import javax.net.ssl.KeyManager;
import org.junit.Test;
import org.springframework.boot.web.server.Ssl;
import org.springframework.boot.web.server.WebServerException;
import org.springframework.test.util.ReflectionTestUtils;
import static org.assertj.core.api.Assertions.assertThat;
@ -33,6 +34,7 @@ import static org.junit.Assert.fail;
* Tests for {@link SslBuilderCustomizer}
*
* @author Brian Clozel
* @author Raheela Aslam
*/
public class SslBuilderCustomizerTests {
@ -88,4 +90,21 @@ public class SslBuilderCustomizerTests {
}
}
@Test
public void getKeyManagersWhenSslIsEnabledWithNoKeyStoreThrowsWebServerException()
throws Exception {
Ssl ssl = new Ssl();
SslBuilderCustomizer customizer = new SslBuilderCustomizer(8080,
InetAddress.getLocalHost(), ssl, null);
try {
ReflectionTestUtils.invokeMethod(customizer, "getKeyManagers", ssl, null);
fail();
}
catch (IllegalStateException ex) {
Throwable cause = ex.getCause();
assertThat(cause).isInstanceOf(WebServerException.class);
assertThat(cause).hasMessageContaining("Could not load key store 'null'");
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2018 the original author or authors.
* 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.
@ -62,6 +62,7 @@ import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.reactive.function.client.WebClient;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Base for testing classes that extends {@link AbstractReactiveWebServerFactory}.
@ -291,6 +292,12 @@ public abstract class AbstractReactiveWebServerFactoryTests {
assertResponseIsNotCompressed(response);
}
@Test
public void whenSslIsEnabledAndNoKeyStoreIsConfiguredThenServerFailsToStart() {
assertThatThrownBy(() -> testBasicSslWithKeyStore(null, null))
.hasMessageContaining("Could not load key store 'null'");
}
protected WebClient prepareCompressionTest() {
Compression compression = new Compression();
compression.setEnabled(true);