honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Cleanup: Fix SSLService design problems 

In order to fix various issues with the SSL service, the following cleanups have been done

* Removed SSLServiceProvider and all the lazy loading magic
* Do not try to create an SSLContext in the constructor. According to Guice docs the constructor
  should only be used ever for variable declarations but not business logic. This also fixes a nasty
  OOM, in case an exception was thrown in the constructor, because Guice tried to recreate that class
  in an endless loop
* Get responsibilities right (which resulted in this nasty lazy loading provider design).
  The SSLService allows to create a SSLEngine at the time you need it, but you need to supply
  specific configuration if you want it to instead of using the default configuration
  and creating a SSLContext on startup like we did before.

All changes are internal.

Closes elastic/elasticsearch#454
Closes elastic/elasticsearch#453

Original commit: elastic/x-pack-elasticsearch@7ca49f781c
This commit is contained in:
Alexander Reelsen 2014-12-10 12:36:43 +01:00
parent b55180f3f8
commit cf0987a4a0
11 changed files with 97 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/main/java/org/elasticsearch/shield/authc/support/ldap/LdapSslSocketFactory.java
View File

@ -8,12 +8,10 @@ package org.elasticsearch.shield.authc.support.ldap;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.inject.Provider;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.shield.ShieldSettingsException;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import javax.net.SocketFactory;
import java.io.IOException;
@ -44,14 +42,14 @@ public class LdapSslSocketFactory extends SocketFactory {
private static LdapSslSocketFactory instance;
private static Provider<SSLService> sslServiceProvider;
private static SSLService sslService;
/**
* This should only be invoked once to establish a static instance that will be used for each constructor.
*/
@Inject
public static void init(SSLServiceProvider sslServiceProvider) {
LdapSslSocketFactory.sslServiceProvider = sslServiceProvider;
public static void init(SSLService sslService) {
LdapSslSocketFactory.sslService = sslService;
}
/**
@ -74,7 +72,7 @@ public class LdapSslSocketFactory extends SocketFactory {
*/
public static synchronized SocketFactory getDefault() {
if (instance == null) {
instance = new LdapSslSocketFactory(sslServiceProvider.get().getSSLSocketFactory());
instance = new LdapSslSocketFactory(sslService.getSSLSocketFactory());
}
return instance;
}

2
src/main/java/org/elasticsearch/shield/ssl/SSLModule.java
View File

@ -19,6 +19,6 @@ public class SSLModule extends AbstractShieldModule {
@Override
protected void configure(boolean clientMode) {
bind(SSLServiceProvider.class).asEagerSingleton();
bind(SSLService.class).asEagerSingleton();
}
}

101
src/main/java/org/elasticsearch/shield/ssl/SSLService.java
View File

@ -6,9 +6,10 @@
package org.elasticsearch.shield.ssl;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.collect.Maps;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.inject.Provider;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.shield.ShieldSettingsException;
@ -16,6 +17,7 @@ import javax.net.ssl.*;
import java.io.FileInputStream;
import java.security.KeyStore;
import java.util.Arrays;
import java.util.Map;
/**
* This service houses the private key and trust managers needed for SSL/TLS negotiation. It is the central place to
@ -25,22 +27,41 @@ public class SSLService extends AbstractComponent {
static final String[] DEFAULT_CIPHERS = new String[]{ "TLS_RSA_WITH_AES_128_CBC_SHA256", "TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_DHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA" };
private final TrustManagerFactory trustFactory;
private final KeyManagerFactory keyManagerFactory;
private final String sslProtocol;
private final SSLContext sslContext;
private final String[] ciphers;
private Map<String, SSLContext> sslContexts = Maps.newHashMapWithExpectedSize(3);
SSLService(Settings settings) {
@Inject
public SSLService(Settings settings) {
super(settings);
}
String keyStorePath = componentSettings.get("keystore.path", System.getProperty("javax.net.ssl.keyStore"));
String keyStorePassword = componentSettings.get("keystore.password", System.getProperty("javax.net.ssl.keyStorePassword"));
String keyStoreAlgorithm = componentSettings.get("keystore.algorithm", System.getProperty("ssl.KeyManagerFactory.algorithm", KeyManagerFactory.getDefaultAlgorithm()));
/**
* @return An SSLSocketFactory (for client-side SSL handshaking)
*/
public SSLSocketFactory getSSLSocketFactory() {
return getSslContext(ImmutableSettings.EMPTY).getSocketFactory();
}
String trustStorePath = componentSettings.get("truststore.path", System.getProperty("javax.net.ssl.trustStore"));
String trustStorePassword = componentSettings.get("truststore.password", System.getProperty("javax.net.ssl.trustStorePassword"));
String trustStoreAlgorithm = componentSettings.get("truststore.algorithm", System.getProperty("ssl.TrustManagerFactory.algorithm", TrustManagerFactory.getDefaultAlgorithm()));
public SSLEngine createSSLEngine() {
return createSSLEngine(ImmutableSettings.EMPTY);
}
public SSLEngine createSSLEngine(Settings settings) {
String[] ciphers = settings.getAsArray("ciphers", componentSettings.getAsArray("ciphers", DEFAULT_CIPHERS));
return createSSLEngine(getSslContext(settings), ciphers);
}
public SSLContext getSslContext() {
return getSslContext(ImmutableSettings.EMPTY);
}
private SSLContext getSslContext(Settings settings) {
String keyStorePath = settings.get("keystore.path", componentSettings.get("keystore.path", System.getProperty("javax.net.ssl.keyStore")));
String keyStorePassword = settings.get("keystore.password", componentSettings.get("keystore.password", System.getProperty("javax.net.ssl.keyStorePassword")));
String keyStoreAlgorithm = settings.get("keystore.algorithm", componentSettings.get("keystore.algorithm", System.getProperty("ssl.KeyManagerFactory.algorithm", KeyManagerFactory.getDefaultAlgorithm())));
String trustStorePath = settings.get("truststore.path", componentSettings.get("truststore.path", System.getProperty("javax.net.ssl.trustStore")));
String trustStorePassword = settings.get("truststore.password", componentSettings.get("truststore.password", System.getProperty("javax.net.ssl.trustStorePassword")));
String trustStoreAlgorithm = settings.get("truststore.algorithm", componentSettings.get("truststore.algorithm", System.getProperty("ssl.TrustManagerFactory.algorithm", TrustManagerFactory.getDefaultAlgorithm())));
if (trustStorePath == null) {
//the keystore will also be the truststore
@ -55,49 +76,29 @@ public class SSLService extends AbstractComponent {
throw new ShieldSettingsException("No keystore password configured");
}
this.ciphers = componentSettings.getAsArray("ciphers", DEFAULT_CIPHERS);
//protocols supported: https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#SSLContext
this.sslProtocol = componentSettings.get("protocol", "TLS");
String sslProtocol = componentSettings.get("protocol", "TLS");
logger.debug("using keyStore[{}], keyAlgorithm[{}], trustStore[{}], truststoreAlgorithm[{}], ciphersuites[{}], TLS protocol[{}]",
keyStorePath, keyStoreAlgorithm, trustStorePath, trustStoreAlgorithm, ciphers, sslProtocol);
// no need for a complex key, same path + protocol define about reusability of a SSLContext
// also no need for pwd verification. If it worked before, it will work again
String key = keyStorePath + trustStorePath + sslProtocol;
SSLContext sslContext = sslContexts.get(key);
if (sslContext == null) {
logger.debug("using keyStore[{}], keyAlgorithm[{}], trustStore[{}], truststoreAlgorithm[{}], TLS protocol[{}]",
keyStorePath, keyStoreAlgorithm, trustStorePath, trustStoreAlgorithm, sslProtocol);
this.trustFactory = getTrustFactory(trustStorePath, trustStorePassword, trustStoreAlgorithm);
this.keyManagerFactory = createKeyManagerFactory(keyStorePath, keyStorePassword, keyStoreAlgorithm);
this.sslContext = createSslContext(trustFactory);
}
TrustManagerFactory trustFactory = getTrustFactory(trustStorePath, trustStorePassword, trustStoreAlgorithm);
KeyManagerFactory keyManagerFactory = createKeyManagerFactory(keyStorePath, keyStorePassword, keyStoreAlgorithm);
sslContext = createSslContext(keyManagerFactory, trustFactory, sslProtocol);
sslContexts.put(key, sslContext);
} else {
logger.trace("Found keystore[{}], trustStore[{}], TLS protocol[{}] in SSL context cache, reusing", keyStorePath, trustStorePath, sslProtocol);
}
/**
* @return An SSLSocketFactory (for client-side SSL handshaking)
*/
public SSLSocketFactory getSSLSocketFactory() {
return sslContext.getSocketFactory();
}
public SSLEngine createSSLEngine() {
return createSSLEngine(this.sslContext);
}
public SSLContext getSslContext() {
return sslContext;
}
public SSLEngine createSSLEngineWithTruststore(Settings settings) {
String trustStore = settings.get("truststore.path", System.getProperty("javax.net.ssl.trustStore"));
String trustStorePassword = settings.get("truststore.password", System.getProperty("javax.net.ssl.trustStorePassword"));
String trustStoreAlgorithm = settings.get("truststore.algorithm", System.getProperty("ssl.TrustManagerFactory.algorithm", TrustManagerFactory.getDefaultAlgorithm()));
// no truststore or password, return regular ssl engine
if (trustStore == null || trustStorePassword == null) {
return createSSLEngine();
}
TrustManagerFactory trustFactory = getTrustFactory(trustStore, trustStorePassword, trustStoreAlgorithm);
SSLContext customTruststoreSSLContext = createSslContext(trustFactory);
return createSSLEngine(customTruststoreSSLContext);
}
private SSLEngine createSSLEngine(SSLContext sslContext) {
private SSLEngine createSSLEngine(SSLContext sslContext, String[] ciphers) {
SSLEngine sslEngine = sslContext.createSSLEngine();
try {
sslEngine.setEnabledCipherSuites(ciphers);
@ -122,7 +123,7 @@ public class SSLService extends AbstractComponent {
}
}
private SSLContext createSslContext(TrustManagerFactory trustFactory) {
private SSLContext createSslContext(KeyManagerFactory keyManagerFactory, TrustManagerFactory trustFactory, String sslProtocol) {
// Initialize sslContext
try {
SSLContext sslContext = SSLContext.getInstance(sslProtocol);

33
src/main/java/org/elasticsearch/shield/ssl/SSLServiceProvider.java
View File

@ -1,33 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.shield.ssl;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.inject.Provider;
import org.elasticsearch.common.settings.Settings;
/**
*
*/
public class SSLServiceProvider implements Provider<SSLService> {
private final Settings settings;
private SSLService service;
@Inject
public SSLServiceProvider(Settings settings) {
this.settings = settings;
}
@Override
public synchronized SSLService get() {
if (service == null) {
service = new SSLService(settings);
}
return service;
}
}

12
src/main/java/org/elasticsearch/shield/transport/netty/NettySecuredHttpServerTransport.java
View File

@ -6,7 +6,6 @@
package org.elasticsearch.shield.transport.netty;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.inject.internal.Nullable;
import org.elasticsearch.common.netty.channel.ChannelPipeline;
import org.elasticsearch.common.netty.channel.ChannelPipelineFactory;
import org.elasticsearch.common.netty.handler.ssl.SslHandler;
@ -15,7 +14,6 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.http.netty.NettyHttpServerTransport;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import org.elasticsearch.shield.transport.filter.IPFilter;
import javax.net.ssl.SSLEngine;
@ -26,14 +24,16 @@ import javax.net.ssl.SSLEngine;
public class NettySecuredHttpServerTransport extends NettyHttpServerTransport {
private final IPFilter ipFilter;
private final @Nullable SSLService sslService;
private final SSLService sslService;
private final boolean ssl;
@Inject
public NettySecuredHttpServerTransport(Settings settings, NetworkService networkService, BigArrays bigArrays,
IPFilter ipFilter, SSLServiceProvider sslServiceProvider) {
IPFilter ipFilter, SSLService sslService) {
super(settings, networkService, bigArrays);
this.ipFilter = ipFilter;
this.sslService = settings.getAsBoolean("shield.http.ssl", false) ? sslServiceProvider.get() : null;
this.ssl = settings.getAsBoolean("shield.http.ssl", false);
this.sslService = sslService;
}
@Override
@ -50,7 +50,7 @@ public class NettySecuredHttpServerTransport extends NettyHttpServerTransport {
@Override
public ChannelPipeline getPipeline() throws Exception {
ChannelPipeline pipeline = super.getPipeline();
if (sslService != null) {
if (ssl) {
SSLEngine engine = sslService.createSSLEngine();
engine.setUseClientMode(false);
engine.setNeedClientAuth(settings.getAsBoolean("shield.http.ssl.client.auth", false));

19
src/main/java/org/elasticsearch/shield/transport/netty/NettySecuredTransport.java
View File

@ -15,7 +15,6 @@ import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import org.elasticsearch.shield.transport.filter.IPFilter;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.netty.NettyTransport;
@ -27,17 +26,17 @@ import javax.net.ssl.SSLEngine;
*/
public class NettySecuredTransport extends NettyTransport {
private final @Nullable SSLService sslService;
private final @Nullable
IPFilter authenticator;
private final SSLService sslService;
private final @Nullable IPFilter authenticator;
private final boolean ssl;
@Inject
public NettySecuredTransport(Settings settings, ThreadPool threadPool, NetworkService networkService, BigArrays bigArrays, Version version,
@Nullable IPFilter authenticator, SSLServiceProvider sslServiceProvider) {
@Nullable IPFilter authenticator, SSLService sslService) {
super(settings, threadPool, networkService, bigArrays, version);
this.authenticator = authenticator;
boolean ssl = settings.getAsBoolean("shield.transport.ssl", false);
this.sslService = ssl ? sslServiceProvider.get() : null;
this.ssl = settings.getAsBoolean("shield.transport.ssl", false);
this.sslService = sslService;
}
@Override
@ -62,10 +61,10 @@ public class NettySecuredTransport extends NettyTransport {
@Override
public ChannelPipeline getPipeline() throws Exception {
ChannelPipeline pipeline = super.getPipeline();
if (sslService != null) {
if (ssl) {
SSLEngine serverEngine;
if (profileSettings.get("shield.truststore.path") != null) {
serverEngine = sslService.createSSLEngineWithTruststore(profileSettings.getByPrefix("shield."));
serverEngine = sslService.createSSLEngine(profileSettings.getByPrefix("shield."));
} else {
serverEngine = sslService.createSSLEngine();
}
@ -91,7 +90,7 @@ public class NettySecuredTransport extends NettyTransport {
@Override
public ChannelPipeline getPipeline() throws Exception {
ChannelPipeline pipeline = super.getPipeline();
if (sslService != null) {
if (ssl) {
SSLEngine clientEngine = sslService.createSSLEngine();
clientEngine.setUseClientMode(true);

4
src/test/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryFactoryTests.java
View File

@ -14,7 +14,7 @@ import org.elasticsearch.shield.authc.support.SecuredStringTests;
import org.elasticsearch.shield.authc.support.ldap.AbstractLdapConnection;
import org.elasticsearch.shield.authc.support.ldap.LdapSslSocketFactory;
import org.elasticsearch.shield.authc.support.ldap.LdapTest;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.test.junit.annotations.Network;
import org.hamcrest.Matchers;
@ -38,7 +38,7 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase {
@BeforeClass
public static void setTrustStore() throws URISyntaxException {
File filename = new File(LdapConnectionTests.class.getResource("../support/ldap/ldaptrust.jks").toURI()).getAbsoluteFile();
LdapSslSocketFactory.init(new SSLServiceProvider(ImmutableSettings.builder()
LdapSslSocketFactory.init(new SSLService(ImmutableSettings.builder()
.put("shield.ssl.keystore.path", filename)
.put("shield.ssl.keystore.password", "changeit")
.build()));

4
src/test/java/org/elasticsearch/shield/authc/ldap/OpenLdapTests.java
View File

@ -8,7 +8,7 @@ package org.elasticsearch.shield.authc.ldap;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.shield.authc.support.SecuredStringTests;
import org.elasticsearch.shield.authc.support.ldap.LdapSslSocketFactory;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.test.junit.annotations.Network;
import org.junit.AfterClass;
@ -30,7 +30,7 @@ public class OpenLdapTests extends ElasticsearchTestCase {
@BeforeClass
public static void setTrustStore() throws URISyntaxException {
File filename = new File(LdapConnectionTests.class.getResource("../support/ldap/ldaptrust.jks").toURI()).getAbsoluteFile();
LdapSslSocketFactory.init(new SSLServiceProvider(ImmutableSettings.builder()
LdapSslSocketFactory.init(new SSLService(ImmutableSettings.builder()
.put("shield.ssl.keystore.path", filename)
.put("shield.ssl.keystore.password", "changeit")
.build()));

4
src/test/java/org/elasticsearch/shield/authc/support/ldap/LdapSslSocketFactoryTests.java
View File

@ -9,7 +9,7 @@ import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.shield.ShieldSettingsException;
import org.elasticsearch.shield.authc.ldap.LdapConnectionTests;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.hamcrest.Matchers;
import org.junit.AfterClass;
@ -27,7 +27,7 @@ public class LdapSslSocketFactoryTests extends ElasticsearchTestCase {
@BeforeClass
public static void setTrustStore() throws URISyntaxException {
File filename = new File(LdapConnectionTests.class.getResource("../support/ldap/ldaptrust.jks").toURI()).getAbsoluteFile();
LdapSslSocketFactory.init(new SSLServiceProvider(ImmutableSettings.builder()
LdapSslSocketFactory.init(new SSLService(ImmutableSettings.builder()
.put("shield.ssl.keystore.path", filename)
.put("shield.ssl.keystore.password", "changeit")
.build()));

34
src/test/java/org/elasticsearch/shield/ssl/SSLServiceTests.java
View File

@ -8,9 +8,9 @@ package org.elasticsearch.shield.ssl;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import java.io.File;
@ -34,19 +34,7 @@ public class SSLServiceTests extends ElasticsearchTestCase {
.put("shield.ssl.keystore.password", "testnode")
.put("shield.ssl.truststore.path", testnodeStore.getPath())
.put("shield.ssl.truststore.password", "testnode")
.build());
}
@Test @Ignore //TODO it appears that setting a specific protocol doesn't make a difference
public void testSpecificProtocol() {
SSLService ssl = new SSLService(settingsBuilder()
.put("shield.ssl.protocol", "TLSv1.2")
.put("shield.ssl.keystore.path", testnodeStore.getPath())
.put("shield.ssl.keystore.password", "testnode")
.put("shield.ssl.truststore.path", testnodeStore.getPath())
.put("shield.ssl.truststore.password", "testnode")
.build());
assertThat(ssl.createSSLEngine().getSSLParameters().getProtocols(), arrayContaining("TLSv1.2"));
.build()).createSSLEngine();
}
@Test
@ -62,7 +50,23 @@ public class SSLServiceTests extends ElasticsearchTestCase {
.put("truststore.path", testClientStore.getPath())
.put("truststore.password", "testclient");
SSLEngine sslEngineWithTruststore = sslService.createSSLEngineWithTruststore(settingsBuilder.build());
SSLEngine sslEngineWithTruststore = sslService.createSSLEngine(settingsBuilder.build());
assertThat(sslEngineWithTruststore, is(not(nullValue())));
SSLEngine sslEngine = sslService.createSSLEngine();
assertThat(sslEngineWithTruststore, is(not(sameInstance(sslEngine))));
}
@Test
public void testThatSslContextCachingWorks() throws Exception {
SSLService sslService = new SSLService(settingsBuilder()
.put("shield.ssl.keystore.path", testnodeStore.getPath())
.put("shield.ssl.keystore.password", "testnode")
.build());
SSLContext sslContext = sslService.getSslContext();
SSLContext cachedSslContext = sslService.getSslContext();
assertThat(sslContext, is(sameInstance(cachedSslContext)));
}
}

3
src/test/java/org/elasticsearch/shield/transport/ssl/SslClientAuthTests.java
View File

@ -16,7 +16,6 @@ import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.node.internal.InternalNode;
import org.elasticsearch.shield.ssl.SSLService;
import org.elasticsearch.shield.ssl.SSLServiceProvider;
import org.elasticsearch.test.ShieldIntegrationTest;
import org.elasticsearch.test.ShieldSettingsSource;
import org.elasticsearch.test.rest.client.http.HttpRequestBuilder;
@ -74,7 +73,7 @@ public class SslClientAuthTests extends ShieldIntegrationTest {
@Test
public void testThatHttpWorksWithSslClientAuth() throws IOException {
Settings settings = settingsBuilder().put(ShieldSettingsSource.getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/testclient.jks", "testclient")).build();
SSLService sslService = new SSLServiceProvider(settings).get();
SSLService sslService = new SSLService(settings);
SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
sslService.getSslContext(),