Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access SSL contexts using names instead of Settings #30953

Merged
merged 39 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
8fbe6e1
Refer to SSL contexts by name not settings
tvernum May 7, 2018
222015b
Merge branch 'master' into fix/30344-ssl-context-names
tvernum May 10, 2018
a360d22
[WIP] Use named SSL context in LDAP
tvernum May 14, 2018
29077e2
Merge branch 'master' into fix/30344-ssl-context-names
tvernum May 28, 2018
9ea7a52
Update openldap QA tests for named SSL contexts
tvernum May 28, 2018
997a1ec
Remove sslSocketFactory(Settings) from SSLService
tvernum May 28, 2018
2999649
Deprecate createSSLEngine(Settings)
tvernum May 29, 2018
bcde8a5
Remove createSSLEngine(Settings)
tvernum May 29, 2018
6ce4d32
Reduce use of Settings in SSL
tvernum May 30, 2018
f9cc028
Merge branch 'master' into fix/30344-ssl-context-names
tvernum May 30, 2018
e0561ee
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 12, 2018
bee987c
Improve SSLService tests
tvernum Jun 12, 2018
7c060d7
Small cleanup
tvernum Jun 12, 2018
2282d01
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 13, 2018
1aa0c14
Add additional test for named SSL configurations
tvernum Jun 14, 2018
39e6629
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 14, 2018
ef58ff1
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 14, 2018
d94dd57
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 14, 2018
ee32357
Remove unused imports
tvernum Jun 14, 2018
5a57fd6
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 15, 2018
cf2fdf0
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 18, 2018
42e3aa1
Fix test (feedback)
tvernum Jun 18, 2018
fb0505e
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 22, 2018
37cc028
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jun 26, 2018
817c565
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jul 9, 2018
2eb845a
Fix monitoring to work with dynamic SSL settings
tvernum Jul 9, 2018
2f6c87b
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jul 10, 2018
255a42e
Cleanup test static vars
tvernum Jul 10, 2018
8ea7e35
Address feedback from @jaymode (round 1)
tvernum Jul 10, 2018
e8d8299
Fix import
tvernum Jul 10, 2018
b11d61b
Rename test & context name
tvernum Jul 11, 2018
f4bab91
Remove more uses of sslConfiguration from settings
tvernum Jul 11, 2018
f637dc7
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jul 11, 2018
f774e6d
Remove remaining uses of deprecated methods
tvernum Jul 11, 2018
d1f3147
Don't use JKS keystore in test
tvernum Jul 11, 2018
44c4cf1
Fix broken test
tvernum Jul 11, 2018
9a0c026
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jul 12, 2018
bc3095e
Fix broken tests
tvernum Jul 12, 2018
9f6fa6d
Merge branch 'master' into fix/30344-ssl-context-names
tvernum Jul 13, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.Locale;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Bridges SSLConfiguration into the {@link Settings} framework, using {@link Setting} objects.
Expand Down Expand Up @@ -221,4 +223,10 @@ public static Collection<Setting<?>> getProfileSettings() {
CLIENT_AUTH_SETTING_PROFILES, VERIFICATION_MODE_SETTING_PROFILES);
}

public List<Setting<SecureString>> getSecureSettingsInUse(Settings settings) {
return Stream.of(this.truststorePassword, this.x509KeyPair.keystorePassword,
this.x509KeyPair.keystoreKeyPassword, this.x509KeyPair.keyPassword)
.filter(s -> s.exists(settings))
.collect(Collectors.toList());
}
}

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public Exporters(Settings settings, Map<String, Exporter.Factory> factories,
this.licenseState = Objects.requireNonNull(licenseState);

clusterService.getClusterSettings().addSettingsUpdateConsumer(this::setExportersSetting, getSettings());
HttpExporter.registerSettingValidators(clusterService);
// this ensures, that logging is happening by adding an empty consumer per affix setting
for (Setting.AffixSetting<?> affixSetting : getSettings()) {
clusterService.getClusterSettings().addAffixUpdateConsumer(affixSetting, (s, o) -> {}, (s, o) -> {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils;
import org.elasticsearch.xpack.core.ssl.SSLConfiguration;
import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings;
import org.elasticsearch.xpack.core.ssl.SSLService;
import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil;
import org.elasticsearch.xpack.monitoring.exporter.Exporter;
Expand All @@ -50,6 +52,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;

/**
* {@code HttpExporter} uses the low-level {@link RestClient} to connect to a user-specified set of nodes for exporting Monitoring
Expand Down Expand Up @@ -252,6 +255,30 @@ public HttpExporter(final Config config, final SSLService sslService, final Thre
listener.setResource(resource);
}

/**
* Adds a validator for the {@link #SSL_SETTING} to prevent dynamic updates when secure settings also exist within that setting
* groups (ssl context).
* Because it is not possible to re-read the secure settings during a dynamic update, we cannot rebuild the {@link SSLIOSessionStrategy}
* (see {@link #configureSecurity(RestClientBuilder, Config, SSLService)} if this exporter has been configured with secure settings
*/
public static void registerSettingValidators(ClusterService clusterService) {
clusterService.getClusterSettings().addAffixUpdateConsumer(SSL_SETTING,
(ignoreKey, ignoreSettings) -> {
// no-op update. We only care about the validator
},
(namespace, settings) -> {
final List<String> secureSettings = SSLConfigurationSettings.withoutPrefix()
.getSecureSettingsInUse(settings)
.stream()
.map(Setting::getKey)
.collect(Collectors.toList());
if (secureSettings.isEmpty() == false) {
throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter [" + namespace
+ "] as it depends on the secure setting(s) [" + Strings.collectionToCommaDelimitedString(secureSettings) + "]");
}
});
}

/**
* Create a {@link RestClientBuilder} from the HTTP Exporter's {@code config}.
*
Expand Down Expand Up @@ -443,8 +470,20 @@ private static void configureHeaders(final RestClientBuilder builder, final Conf
* @throws SettingsException if any setting causes issues
*/
private static void configureSecurity(final RestClientBuilder builder, final Config config, final SSLService sslService) {
final Settings sslSettings = SSL_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings());
final SSLIOSessionStrategy sslStrategy = sslService.sslIOSessionStrategy(sslSettings);
final Setting<Settings> concreteSetting = SSL_SETTING.getConcreteSettingForNamespace(config.name());
final Settings sslSettings = concreteSetting.get(config.settings());
final SSLIOSessionStrategy sslStrategy;
if (SSLConfigurationSettings.withoutPrefix().getSecureSettingsInUse(sslSettings).isEmpty()) {
// This configuration does not use secure settings, so it is possible that is has been dynamically updated.
// We need to load a new SSL strategy in case these settings differ from the ones that the SSL service was configured with.
sslStrategy = sslService.sslIOSessionStrategy(sslSettings);
} else {
// This configuration uses secure settings. We cannot load a new SSL strategy, as the secure settings have already been closed.
// Due to #registerSettingValidators we know that the settings not been dynamically updated, and the pre-configured strategy
// is still the correct configuration for use in this exporter.
final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(concreteSetting.getKey());
sslStrategy = sslService.sslIOSessionStrategy(sslConfiguration);
}
final CredentialsProvider credentialsProvider = createCredentialsProvider(config);
List<String> hostList = HOST_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings());
// sending credentials in plaintext!
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
/*
* 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.xpack.monitoring.exporter.http;

import org.elasticsearch.action.ActionFuture;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESIntegTestCase.Scope;
import org.elasticsearch.test.http.MockWebServer;
import org.elasticsearch.xpack.core.ssl.TestsSSLService;
import org.elasticsearch.xpack.core.ssl.VerificationMode;
import org.elasticsearch.xpack.monitoring.exporter.Exporter;
import org.elasticsearch.xpack.monitoring.exporter.Exporters;
import org.elasticsearch.xpack.monitoring.test.MonitoringIntegTestCase;
import org.hamcrest.CoreMatchers;
import org.junit.AfterClass;
import org.junit.Before;

import javax.net.ssl.SSLContext;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.notNullValue;

@ESIntegTestCase.ClusterScope(scope = Scope.SUITE,
numDataNodes = 1, numClientNodes = 0, transportClientRatio = 0.0, supportsDedicatedMasters = false)
public class HttpSslExporterIT extends MonitoringIntegTestCase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nit, but maybe HttpExporterSslIT so that if someone is looking for HttpExporter test classes, they easily find it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally named it HttpExporterSslIT but the combination of lower-case l followed up uppercase I offended my eyes, so I changed it. I'll switch it back.


private final Settings globalSettings = Settings.builder().put("path.home", createTempDir()).build();
private final Environment environment = TestEnvironment.newEnvironment(globalSettings);

private static MockWebServer webServer;
private static Path keystore;
private MockSecureSettings secureSettings;


@AfterClass
public static void cleanUpStatics() {
if (webServer != null) {
webServer.close();
webServer = null;
}
keystore = null;
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Settings nodeSettings(int nodeOrdinal) {
if (keystore == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkakavas is there something that should be done here for the FIPS work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks for catching this.

@tvernum can you please add the key and certificate files and use those instead of the keystore when setting up the MockWebServer ?
Using it for the truststore is fine, even in a FIPS-140 JVM so no additional requirements here. When I went through our tests, I opted for using the certificate for certificate_authorities instead of the JKS for truststore (when applicable and made sense ) mainly for consistency, but we don't have to .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh FIPS why do you want to make my life complicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to use the truststore (rather than certificate_authorities) because I need to test passwords, and PEM certs don't have passwords.

keystore = getDataPath("/org/elasticsearch/xpack/monitoring/exporter/http/testnode.jks");
assertThat(Files.exists(keystore), CoreMatchers.is(true));
}

if (webServer == null) {
try {
webServer = buildWebServer();
} catch (IOException e) {
throw new RuntimeException(e);
}
}

final String address = "https://" + webServer.getHostName() + ":" + webServer.getPort();
final Settings.Builder builder = Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put("xpack.monitoring.exporters.plaintext.type", "http")
.put("xpack.monitoring.exporters.plaintext.enabled", true)
.put("xpack.monitoring.exporters.plaintext.host", address)
.put("xpack.monitoring.exporters.plaintext.ssl.truststore.path", keystore)
.put("xpack.monitoring.exporters.plaintext.ssl.truststore.password", "testnode")
.put("xpack.monitoring.exporters.secure.type", "http")
.put("xpack.monitoring.exporters.secure.enabled", true)
.put("xpack.monitoring.exporters.secure.host", address)
.put("xpack.monitoring.exporters.secure.ssl.truststore.path", keystore);

secureSettings = new MockSecureSettings();
secureSettings.setString("xpack.monitoring.exporters.secure.ssl.truststore.secure_password", "testnode");
builder.setSecureSettings(secureSettings);

return builder.build();
}

private MockWebServer buildWebServer() throws IOException {
final Settings sslSettings = Settings.builder()
.put("xpack.ssl.keystore.path", keystore)
.put("xpack.ssl.keystore.password", "testnode")
.put(globalSettings)
.build();

TestsSSLService sslService = new TestsSSLService(sslSettings, environment);
final SSLContext sslContext = sslService.sslContext(Settings.EMPTY);
MockWebServer server = new MockWebServer(sslContext, false);
server.start();
return server;
}

@Before
// Force the exporters to be built from closed secure settings (as they would be in a production environment)
public void closeSecureSettings() throws IOException {
if (secureSettings != null) {
secureSettings.close();
}
}

public void testCannotUpdateSslSettingsWithSecureSettings() throws Exception {
// Verify that it was created even though it has a secure setting
assertExporterExists("secure");

// Verify that we cannot modify the SSL settings
final ActionFuture<ClusterUpdateSettingsResponse> future = setVerificationMode("secure", VerificationMode.CERTIFICATE);
final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, future::actionGet);
assertThat(iae.getCause(), instanceOf(IllegalStateException.class));
assertThat(iae.getCause().getMessage(), containsString("secure_password"));
}

public void testCanUpdateSslSettingsWithNoSecureSettings() {
final ActionFuture<ClusterUpdateSettingsResponse> future = setVerificationMode("plaintext", VerificationMode.CERTIFICATE);
final ClusterUpdateSettingsResponse response = future.actionGet();
assertThat(response, notNullValue());
clearTransientSettings("plaintext");
}

public void testCanAddNewExporterWithSsl() {
Path truststore = getDataPath("/org/elasticsearch/xpack/monitoring/exporter/http/testnode.jks");
assertThat(Files.exists(truststore), CoreMatchers.is(true));

final ClusterUpdateSettingsRequest updateSettings = new ClusterUpdateSettingsRequest();
final Settings settings = Settings.builder()
.put("xpack.monitoring.exporters._new.type", "http")
.put("xpack.monitoring.exporters._new.host", "https://" + webServer.getHostName() + ":" + webServer.getPort())
.put("xpack.monitoring.exporters._new.ssl.truststore.path", truststore)
.put("xpack.monitoring.exporters._new.ssl.truststore.password", "testnode")
.put("xpack.monitoring.exporters._new.ssl.verification_mode", VerificationMode.NONE.name())
.build();
updateSettings.transientSettings(settings);
final ActionFuture<ClusterUpdateSettingsResponse> future = client().admin().cluster().updateSettings(updateSettings);
final ClusterUpdateSettingsResponse response = future.actionGet();
assertThat(response, notNullValue());

assertExporterExists("_new");
clearTransientSettings("_new");
}

private void assertExporterExists(String secure) {
final Exporter httpExporter = getExporter(secure);
assertThat(httpExporter, notNullValue());
assertThat(httpExporter, instanceOf(HttpExporter.class));
}

private Exporter getExporter(String name) {
final Exporters exporters = internalCluster().getInstance(Exporters.class);
assertThat(exporters, notNullValue());
return exporters.getExporter(name);
}

private ActionFuture<ClusterUpdateSettingsResponse> setVerificationMode(String name, VerificationMode mode) {
final ClusterUpdateSettingsRequest updateSettings = new ClusterUpdateSettingsRequest();
final Settings settings = Settings.builder()
.put("xpack.monitoring.exporters." + name + ".ssl.verification_mode", mode.name())
.build();
updateSettings.transientSettings(settings);
return client().admin().cluster().updateSettings(updateSettings);
}

private void clearTransientSettings(String... names) {
final ClusterUpdateSettingsRequest updateSettings = new ClusterUpdateSettingsRequest();
final Settings.Builder builder = Settings.builder();
for (String name : names) {
builder.put("xpack.monitoring.exporters." + name + ".*", (String) null);
}
updateSettings.transientSettings(builder.build());
client().admin().cluster().updateSettings(updateSettings).actionGet();
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class PkiRealmBootstrapCheck implements BootstrapCheck {
private List<SSLConfiguration> loadSslConfigurations(Settings settings) {
final List<SSLConfiguration> list = new ArrayList<>();
if (HTTP_SSL_ENABLED.get(settings)) {
list.add(sslService.sslConfiguration(SSLService.getHttpTransportSSLSettings(settings), Settings.EMPTY));
list.add(sslService.getHttpTransportSSLConfiguration());
}

if (XPackSettings.TRANSPORT_SSL_ENABLED.get(settings)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,17 @@
import org.elasticsearch.xpack.core.security.authz.accesscontrol.SecurityIndexSearcherWrapper;
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
import org.elasticsearch.xpack.security.authz.store.FileRolesStore;
import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
import org.elasticsearch.xpack.core.security.index.IndexAuditTrailField;
import org.elasticsearch.xpack.core.security.user.AnonymousUser;
import org.elasticsearch.xpack.core.ssl.SSLConfiguration;
import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings;
import org.elasticsearch.xpack.core.ssl.SSLService;
import org.elasticsearch.xpack.core.ssl.TLSLicenseBootstrapCheck;
import org.elasticsearch.xpack.core.ssl.action.GetCertificateInfoAction;
import org.elasticsearch.xpack.core.ssl.action.TransportGetCertificateInfoAction;
import org.elasticsearch.xpack.core.ssl.rest.RestGetCertificateInfoAction;
import org.elasticsearch.xpack.core.template.TemplateUtils;
import org.elasticsearch.xpack.security.action.filter.SecurityActionFilter;
import org.elasticsearch.xpack.security.action.interceptor.BulkShardRequestInterceptor;
import org.elasticsearch.xpack.security.action.interceptor.IndicesAliasesRequestInterceptor;
Expand Down Expand Up @@ -172,6 +173,7 @@
import org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener;
import org.elasticsearch.xpack.security.authz.accesscontrol.OptOutQueryCache;
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
import org.elasticsearch.xpack.security.authz.store.FileRolesStore;
import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
import org.elasticsearch.xpack.security.ingest.HashProcessor;
import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor;
Expand Down Expand Up @@ -202,7 +204,6 @@
import org.elasticsearch.xpack.security.transport.filter.IPFilter;
import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4HttpServerTransport;
import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4ServerTransport;
import org.elasticsearch.xpack.core.template.TemplateUtils;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

Expand Down Expand Up @@ -231,9 +232,9 @@
import static java.util.Collections.singletonList;
import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_FORMAT_SETTING;
import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.INTERNAL_INDEX_FORMAT;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME;

public class Security extends Plugin implements ActionPlugin, IngestPlugin, NetworkPlugin, ClusterPlugin,
DiscoveryPlugin, MapperPlugin, ExtensiblePlugin {
Expand Down Expand Up @@ -870,10 +871,9 @@ public UnaryOperator<RestHandler> getRestHandlerWrapper(ThreadContext threadCont
return null;
}
final boolean ssl = HTTP_SSL_ENABLED.get(settings);
Settings httpSSLSettings = SSLService.getHttpTransportSSLSettings(settings);
boolean extractClientCertificate = ssl && getSslService().isSSLClientAuthEnabled(httpSSLSettings);
return handler -> new SecurityRestFilter(getLicenseState(), threadContext, authcService.get(), handler,
extractClientCertificate);
final SSLConfiguration httpSSLConfig = getSslService().getHttpTransportSSLConfiguration();
boolean extractClientCertificate = ssl && getSslService().isSSLClientAuthEnabled(httpSSLConfig);
return handler -> new SecurityRestFilter(getLicenseState(), threadContext, authcService.get(), handler, extractClientCertificate);
}

@Override
Expand Down
Loading