Simplify SSL server configuratin validation (elastic/x-pack-elasticsearch#1826)

Today we have some hidden complexity related to default configurations
might specify NO_KEY which is in some cases valid for server configuration.
This change removes the leniencey paramenters on the validation methods and removes
obsolet asserts.

Original commit: elastic/x-pack-elasticsearch@17ed4b1d20
This commit is contained in:
Simon Willnauer 2017-06-22 21:48:51 +02:00 committed by GitHub
parent 12eec0e911
commit 2fef2c72eb
4 changed files with 13 additions and 25 deletions

View File

@ -45,11 +45,11 @@ public class SecurityNetty4HttpServerTransport extends Netty4HttpServerTransport
this.sslSettings = SSLService.getHttpTransportSSLSettings(settings);
this.sslService = sslService;
if (ssl) {
if (sslService.isConfigurationValidForServerUsage(sslSettings, false) == false) {
this.sslConfiguration = sslService.sslConfiguration(sslSettings, Settings.EMPTY);
if (sslService.isConfigurationValidForServerUsage(sslConfiguration) == false) {
throw new IllegalArgumentException("a key must be provided to run as a server. the key should be configured using the " +
"[xpack.security.http.ssl.key] or [xpack.security.http.ssl.keystore.path] setting");
}
this.sslConfiguration = sslService.sslConfiguration(sslSettings, Settings.EMPTY);
} else {
this.sslConfiguration = null;
}

View File

@ -21,7 +21,6 @@ import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportSettings;
import org.elasticsearch.transport.netty4.Netty4Transport;
import org.elasticsearch.xpack.security.transport.ServerTransportFilter;
import org.elasticsearch.xpack.ssl.SSLConfiguration;
import org.elasticsearch.xpack.ssl.SSLService;
import org.elasticsearch.xpack.security.transport.filter.IPFilter;
@ -64,13 +63,11 @@ public class SecurityNetty4Transport extends Netty4Transport {
final Settings profileSslSettings = profileSslSettings(profileSettings);
SSLConfiguration configuration = sslService.sslConfiguration(profileSslSettings, transportSSLSettings);
profileConfiguration.put(entry.getKey(), configuration);
assert sslService.isConfigurationValidForServerUsage(profileSslSettings, true) :
"the ssl configuration is not valid for server use but we are running as a server. this should have been caught by" +
" the key config validation";
}
if (profileConfiguration.containsKey(TransportSettings.DEFAULT_PROFILE) == false) {
profileConfiguration.put(TransportSettings.DEFAULT_PROFILE, sslConfiguration);
}
this.profileConfiguration = Collections.unmodifiableMap(profileConfiguration);

View File

@ -250,15 +250,10 @@ public class SSLService extends AbstractComponent {
/**
* Returns whether the provided settings results in a valid configuration that can be used for server connections
* @param settings the settings used to identify the ssl configuration, typically under a *.ssl. prefix
* @param useTransportFallback if {@code true} this will use the transport configuration for fallback, otherwise the global
* configuration will be used
* @param sslConfiguration the configuration to check
*/
public boolean isConfigurationValidForServerUsage(Settings settings, boolean useTransportFallback) {
SSLConfiguration fallback = useTransportFallback ? transportSSLConfiguration.get() : globalSSLConfiguration;
SSLConfiguration sslConfiguration = new SSLConfiguration(settings, fallback);
return sslConfiguration.keyConfig() != KeyConfig.NONE
|| (useTransportFallback && transportSSLConfiguration.get().keyConfig() == KeyConfig.NONE);
public boolean isConfigurationValidForServerUsage(SSLConfiguration sslConfiguration) {
return sslConfiguration.keyConfig() != KeyConfig.NONE;
}
/**

View File

@ -157,10 +157,6 @@ public class SSLServiceTests extends ESTestCase {
assertThat(sslEngine, notNullValue());
}
public void testCreateWithoutAnySettingsValidForServer() throws Exception {
SSLService sslService = new SSLService(Settings.EMPTY, env);
assertTrue(sslService.isConfigurationValidForServerUsage(Settings.EMPTY, true));
}
public void testCreateWithKeystoreIsValidForServer() throws Exception {
Settings settings = Settings.builder()
@ -168,7 +164,8 @@ public class SSLServiceTests extends ESTestCase {
.put("xpack.ssl.keystore.password", "testnode")
.build();
SSLService sslService = new SSLService(settings, env);
assertTrue(sslService.isConfigurationValidForServerUsage(Settings.EMPTY, false));
assertTrue(sslService.isConfigurationValidForServerUsage(sslService.sslConfiguration(Settings.EMPTY)));
}
public void testValidForServerWithFallback() throws Exception {
@ -177,8 +174,7 @@ public class SSLServiceTests extends ESTestCase {
.put("xpack.ssl.truststore.password", "testnode")
.build();
SSLService sslService = new SSLService(settings, env);
// transport is valid in default config
assertTrue(sslService.isConfigurationValidForServerUsage(Settings.EMPTY, true));
assertFalse(sslService.isConfigurationValidForServerUsage(sslService.sslConfiguration(Settings.EMPTY)));
settings = Settings.builder()
.put("xpack.ssl.truststore.path", testnodeStore)
@ -187,10 +183,10 @@ public class SSLServiceTests extends ESTestCase {
.put("xpack.security.transport.ssl.keystore.password", "testnode")
.build();
sslService = new SSLService(settings, env);
assertFalse(sslService.isConfigurationValidForServerUsage(Settings.EMPTY, false));
assertTrue(sslService.isConfigurationValidForServerUsage(
settings.getByPrefix("xpack.security.transport.ssl."), false));
assertTrue(sslService.isConfigurationValidForServerUsage(Settings.EMPTY, true));
assertFalse(sslService.isConfigurationValidForServerUsage(sslService.sslConfiguration(Settings.EMPTY)));
assertTrue(sslService.isConfigurationValidForServerUsage(sslService.sslConfiguration(
settings.getByPrefix("xpack.security.transport.ssl."))));
assertFalse(sslService.isConfigurationValidForServerUsage(sslService.sslConfiguration(Settings.EMPTY)));
}
public void testGetVerificationMode() throws Exception {