From 811f26de2444b6db41309d8d1c5bcc5debe6ec0e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 21 Oct 2024 13:51:02 -0400 Subject: [PATCH] Ensure that dual mode enabled flag from cluster settings can get propagated to core (#4820) Signed-off-by: Craig Perkins --- .../EncryptionInTransitMigrationTests.java | 70 +++++++++++++++++++ .../test/framework/cluster/LocalCluster.java | 46 +++++++++++- .../cluster/LocalOpenSearchCluster.java | 12 +++- ...inimumSecuritySettingsSupplierFactory.java | 12 ++++ .../security/OpenSearchSecurityPlugin.java | 4 +- .../ssl/OpenSearchSecureSettingsFactory.java | 16 ++++- .../ssl/OpenSearchSecuritySSLPlugin.java | 4 +- 7 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/EncryptionInTransitMigrationTests.java diff --git a/src/integrationTest/java/org/opensearch/security/EncryptionInTransitMigrationTests.java b/src/integrationTest/java/org/opensearch/security/EncryptionInTransitMigrationTests.java new file mode 100644 index 0000000000..58eb7218e6 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/EncryptionInTransitMigrationTests.java @@ -0,0 +1,70 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + */ +package org.opensearch.security; + +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.awaitility.Awaitility.await; +import static org.junit.Assert.assertEquals; + +/** + * Test related to SSL-only mode of security plugin. In this mode, the security plugin is responsible only for TLS/SSL encryption. + * Therefore, the plugin does not perform authentication and authorization. Moreover, the REST resources (e.g. /_plugins/_security/whoami, + * /_plugins/_security/authinfo, etc.) provided by the plugin are not available. + */ +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class EncryptionInTransitMigrationTests { + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.DEFAULT) + .anonymousAuth(false) + .loadConfigurationIntoIndex(false) + .nodeSettings(Map.of(ConfigConstants.SECURITY_SSL_ONLY, true)) + .sslOnly(true) + .nodeSpecificSettings(0, Map.of(ConfigConstants.SECURITY_CONFIG_SSL_DUAL_MODE_ENABLED, true)) + .nodeSpecificSettings(1, Map.of(ConfigConstants.SECURITY_CONFIG_SSL_DUAL_MODE_ENABLED, true)) + .extectedNodeStartupCount(2) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .build(); + + @Test + public void shouldOnlyConnectWithThirdNodeAfterDynamicDualModeChange() { + try (TestRestClient client = cluster.getRestClient()) { + TestRestClient.HttpResponse response = client.get("_cat/nodes"); + response.assertStatusCode(200); + + String[] lines = response.getBody().split("\n"); + assertEquals("Expected 2 nodes in the initial response", 2, lines.length); + + String settingsJson = "{\"persistent\": {\"plugins.security_config.ssl_dual_mode_enabled\": false}}"; + TestRestClient.HttpResponse settingsResponse = client.putJson("_cluster/settings", settingsJson); + settingsResponse.assertStatusCode(200); + + await().atMost(10, SECONDS).pollInterval(1, SECONDS).until(() -> { + TestRestClient.HttpResponse secondResponse = client.get("_cat/nodes"); + String[] secondLines = secondResponse.getBody().split("\n"); + return secondLines.length == 3; + }); + } + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java index 894bb5baa9..d2c53c1de7 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java @@ -85,7 +85,9 @@ public class LocalCluster extends ExternalResource implements AutoCloseable, Ope private final List> plugins; private final ClusterManager clusterManager; private final TestSecurityConfig testSecurityConfig; + private Map nodeSpecificOverride; private Settings nodeOverride; + private Integer expectedNodeStartupCount; private final String clusterName; private final MinimumSecuritySettingsSupplierFactory minimumOpenSearchSettingsSupplierFactory; private final TestCertificates testCertificates; @@ -100,6 +102,7 @@ private LocalCluster( String clusterName, TestSecurityConfig testSgConfig, boolean sslOnly, + Map nodeSpecificOverride, Settings nodeOverride, ClusterManager clusterManager, List> plugins, @@ -108,13 +111,15 @@ private LocalCluster( Map remotes, List testIndices, boolean loadConfigurationIntoIndex, - String defaultConfigurationInitDirectory + String defaultConfigurationInitDirectory, + Integer expectedNodeStartupCount ) { this.plugins = plugins; this.testCertificates = testCertificates; this.clusterManager = clusterManager; this.testSecurityConfig = testSgConfig; this.sslOnly = sslOnly; + this.nodeSpecificOverride = nodeSpecificOverride; this.nodeOverride = nodeOverride; this.clusterName = clusterName; this.minimumOpenSearchSettingsSupplierFactory = new MinimumSecuritySettingsSupplierFactory(testCertificates); @@ -125,6 +130,7 @@ private LocalCluster( if (StringUtils.isNoneBlank(defaultConfigurationInitDirectory)) { System.setProperty(INIT_CONFIGURATION_DIR, defaultConfigurationInitDirectory); } + this.expectedNodeStartupCount = expectedNodeStartupCount; } public String getSnapshotDirPath() { @@ -232,6 +238,7 @@ private void start() { try { NodeSettingsSupplier nodeSettingsSupplier = minimumOpenSearchSettingsSupplierFactory.minimumOpenSearchSettings( sslOnly, + nodeSpecificOverride, nodeOverride ); localOpenSearchCluster = new LocalOpenSearchCluster( @@ -239,7 +246,8 @@ private void start() { clusterManager, nodeSettingsSupplier, plugins, - testCertificates + testCertificates, + expectedNodeStartupCount ); localOpenSearchCluster.start(); @@ -312,8 +320,10 @@ public CertificateData getAdminCertificate() { public static class Builder { private final Settings.Builder nodeOverrideSettingsBuilder = Settings.builder(); + private final Map nodeSpecificOverrideSettingsBuilder = new HashMap<>(); private boolean sslOnly = false; + private Integer expectedNodeStartupCount; private final List> plugins = new ArrayList<>(); private Map remoteClusters = new HashMap<>(); private List clusterDependencies = new ArrayList<>(); @@ -365,6 +375,11 @@ public Builder sslOnly(boolean sslOnly) { return this; } + public Builder extectedNodeStartupCount(int expectedNodeStartupCount) { + this.expectedNodeStartupCount = expectedNodeStartupCount; + return this; + } + public Builder nodeSettings(Map settings) { settings.forEach((key, value) -> { if (value instanceof List) { @@ -378,6 +393,25 @@ public Builder nodeSettings(Map settings) { return this; } + public Builder nodeSpecificSettings(int nodeNumber, Map settings) { + if (!nodeSpecificOverrideSettingsBuilder.containsKey(nodeNumber)) { + Settings.Builder builderCopy = Settings.builder(); + builderCopy.put(nodeOverrideSettingsBuilder.build()); + nodeSpecificOverrideSettingsBuilder.put(nodeNumber, builderCopy); + } + Settings.Builder nodeSettingsBuilder = nodeSpecificOverrideSettingsBuilder.get(nodeNumber); + settings.forEach((key, value) -> { + if (value instanceof List) { + List values = ((List) value).stream().map(String::valueOf).collect(Collectors.toList()); + nodeSettingsBuilder.putList(key, values); + } else { + nodeSettingsBuilder.put(key, String.valueOf(value)); + } + }); + + return this; + } + /** * Adds additional plugins to the cluster */ @@ -512,10 +546,15 @@ public LocalCluster build() { } clusterName += "_" + num.incrementAndGet(); Settings settings = nodeOverrideSettingsBuilder.build(); + Map nodeSpecificSettings = new HashMap<>(); + for (Map.Entry entry : nodeSpecificOverrideSettingsBuilder.entrySet()) { + nodeSpecificSettings.put(entry.getKey(), entry.getValue().build()); + } return new LocalCluster( clusterName, testSecurityConfig, sslOnly, + nodeSpecificSettings, settings, clusterManager, plugins, @@ -524,7 +563,8 @@ public LocalCluster build() { remoteClusters, testIndices, loadConfigurationIntoIndex, - defaultConfigurationInitDirectory + defaultConfigurationInitDirectory, + expectedNodeStartupCount ); } catch (Exception e) { log.error("Failed to build LocalCluster", e); diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java index 96da63d9fb..8570c3d398 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java @@ -97,6 +97,7 @@ public class LocalOpenSearchCluster { private final List> additionalPlugins; private final List nodes = new ArrayList<>(); private final TestCertificates testCertificates; + private final Integer expectedNodeStartupCount; private File clusterHomeDir; private List seedHosts; @@ -112,13 +113,15 @@ public LocalOpenSearchCluster( ClusterManager clusterManager, NodeSettingsSupplier nodeSettingsSupplier, List> additionalPlugins, - TestCertificates testCertificates + TestCertificates testCertificates, + Integer expectedNodeStartCount ) { this.clusterName = clusterName; this.clusterManager = clusterManager; this.nodeSettingsSupplier = nodeSettingsSupplier; this.additionalPlugins = additionalPlugins; this.testCertificates = testCertificates; + this.expectedNodeStartupCount = expectedNodeStartCount; try { createClusterDirectory(clusterName); } catch (IOException e) { @@ -198,7 +201,12 @@ public void start() throws Exception { log.info("Startup finished. Waiting for GREEN"); - waitForCluster(ClusterHealthStatus.GREEN, TimeValue.timeValueSeconds(10), nodes.size()); + int expectedCount = nodes.size(); + if (expectedNodeStartupCount != null) { + expectedCount = expectedNodeStartupCount; + } + + waitForCluster(ClusterHealthStatus.GREEN, TimeValue.timeValueSeconds(10), expectedCount); log.info("Started: {}", this); } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/MinimumSecuritySettingsSupplierFactory.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/MinimumSecuritySettingsSupplierFactory.java index 4ad5f8420e..34a105ea39 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/MinimumSecuritySettingsSupplierFactory.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/MinimumSecuritySettingsSupplierFactory.java @@ -28,6 +28,8 @@ package org.opensearch.test.framework.cluster; +import java.util.Map; + import org.opensearch.common.settings.Settings; import org.opensearch.security.support.ConfigConstants; import org.opensearch.test.framework.certificate.TestCertificates; @@ -51,6 +53,16 @@ public NodeSettingsSupplier minimumOpenSearchSettings(boolean sslOnly, Settings return i -> minimumOpenSearchSettingsBuilder(i, sslOnly).put(other).build(); } + public NodeSettingsSupplier minimumOpenSearchSettings(boolean sslOnly, Map nodeOverride, Settings other) { + return i -> { + Settings override = nodeOverride.get(i); + if (override != null) { + return minimumOpenSearchSettingsBuilder(i, sslOnly).put(other).put(override).build(); + } + return minimumOpenSearchSettingsBuilder(i, sslOnly).put(other).build(); + }; + } + private Settings.Builder minimumOpenSearchSettingsBuilder(int node, boolean sslOnly) { Settings.Builder builder = Settings.builder(); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 594af15f03..15d5e4c286 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -2166,7 +2166,9 @@ public PluginSubject getPluginSubject(Plugin plugin) { @Override public Optional getSecureSettingFactory(Settings settings) { - return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, evaluateSslExceptionHandler(), securityRestHandler)); + return Optional.of( + new OpenSearchSecureSettingsFactory(threadPool, sks, evaluateSslExceptionHandler(), securityRestHandler, SSLConfig) + ); } @SuppressWarnings("removal") diff --git a/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java b/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java index 5351eea57e..9d482b18a8 100644 --- a/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java +++ b/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java @@ -27,6 +27,7 @@ import org.opensearch.security.filter.SecurityRestFilter; import org.opensearch.security.ssl.http.netty.Netty4ConditionalDecompressor; import org.opensearch.security.ssl.http.netty.Netty4HttpRequestHeaderVerifier; +import org.opensearch.security.ssl.transport.SSLConfig; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.Transport; import org.opensearch.transport.TransportAdapterProvider; @@ -38,17 +39,20 @@ public class OpenSearchSecureSettingsFactory implements SecureSettingsFactory { private final SecurityKeyStore sks; private final SslExceptionHandler sslExceptionHandler; private final SecurityRestFilter restFilter; + private final SSLConfig sslConfig; public OpenSearchSecureSettingsFactory( ThreadPool threadPool, SecurityKeyStore sks, SslExceptionHandler sslExceptionHandler, - SecurityRestFilter restFilter + SecurityRestFilter restFilter, + SSLConfig sslConfig ) { this.threadPool = threadPool; this.sks = sks; this.sslExceptionHandler = sslExceptionHandler; this.restFilter = restFilter; + this.sslConfig = sslConfig; } @Override @@ -64,6 +68,16 @@ public void onError(Throwable t) { }); } + @Override + public Optional parameters(Settings settings) { + return Optional.of(new SecureTransportParameters() { + @Override + public boolean dualModeEnabled() { + return sslConfig.isDualModeEnabled(); + } + }); + } + @Override public Optional buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException { return Optional.of(sks.createServerTransportSSLEngine()); diff --git a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java index e6a1b47888..c16706c870 100644 --- a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java +++ b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java @@ -674,7 +674,9 @@ public List getSettingsFilter() { @Override public Optional getSecureSettingFactory(Settings settings) { - return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, NOOP_SSL_EXCEPTION_HANDLER, securityRestHandler)); + return Optional.of( + new OpenSearchSecureSettingsFactory(threadPool, sks, NOOP_SSL_EXCEPTION_HANDLER, securityRestHandler, SSLConfig) + ); } protected Settings migrateSettings(Settings settings) {