[Backport 2.x] Ensure that dual mode enabled flag from cluster settin…

…gs can get propagated to core (opensearch-project#4830) Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
cwperks · Oct 21, 2024 · a343a9b · a343a9b
1 parent 9241a4a
commit a343a9b
Show file tree

Hide file tree

Showing 7 changed files with 156 additions and 8 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/EncryptionInTransitMigrationTests.java 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
@@ -85,7 +85,9 @@ public class LocalCluster extends ExternalResource implements AutoCloseable, Ope
     private final List<Class<? extends Plugin>> plugins;
     private final ClusterManager clusterManager;
     private final TestSecurityConfig testSecurityConfig;
+    private Map<Integer, Settings> 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<Integer, Settings> nodeSpecificOverride,
         Settings nodeOverride,
         ClusterManager clusterManager,
         List<Class<? extends Plugin>> plugins,
@@ -108,13 +111,15 @@ private LocalCluster(
         Map<String, LocalCluster> remotes,
         List<TestIndex> 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,14 +238,16 @@ private void start() {
         try {
             NodeSettingsSupplier nodeSettingsSupplier = minimumOpenSearchSettingsSupplierFactory.minimumOpenSearchSettings(
                 sslOnly,
+                nodeSpecificOverride,
                 nodeOverride
             );
             localOpenSearchCluster = new LocalOpenSearchCluster(
                 clusterName,
                 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<Integer, Settings.Builder> nodeSpecificOverrideSettingsBuilder = new HashMap<>();
 
         private boolean sslOnly = false;
+        private Integer expectedNodeStartupCount;
         private final List<Class<? extends Plugin>> plugins = new ArrayList<>();
         private Map<String, LocalCluster> remoteClusters = new HashMap<>();
         private List<LocalCluster> 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<String, Object> settings) {
             settings.forEach((key, value) -> {
                 if (value instanceof List) {
@@ -378,6 +393,25 @@ public Builder nodeSettings(Map<String, Object> settings) {
             return this;
         }
 
+        public Builder nodeSpecificSettings(int nodeNumber, Map<String, Object> 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<String> 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<Integer, Settings> nodeSpecificSettings = new HashMap<>();
+                for (Map.Entry<Integer, Settings.Builder> 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
@@ -97,6 +97,7 @@ public class LocalOpenSearchCluster {
     private final List<Class<? extends Plugin>> additionalPlugins;
     private final List<Node> nodes = new ArrayList<>();
     private final TestCertificates testCertificates;
+    private final Integer expectedNodeStartupCount;
 
     private File clusterHomeDir;
     private List<String> seedHosts;
@@ -112,13 +113,15 @@ public LocalOpenSearchCluster(
         ClusterManager clusterManager,
         NodeSettingsSupplier nodeSettingsSupplier,
         List<Class<? extends Plugin>> 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/...st/java/org/opensearch/test/framework/cluster/MinimumSecuritySettingsSupplierFactory.java b/...st/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<Integer, Settings> 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
@@ -2164,7 +2164,9 @@ public PluginSubject getPluginSubject(Plugin plugin) {
 
     @Override
     public Optional<SecureSettingsFactory> 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
@@ -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<SecureTransportParameters> parameters(Settings settings) {
+                return Optional.of(new SecureTransportParameters() {
+                    @Override
+                    public boolean dualModeEnabled() {
+                        return sslConfig.isDualModeEnabled();
+                    }
+                });
+            }
+
             @Override
             public Optional<SSLEngine> 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
@@ -674,7 +674,9 @@ public List<String> getSettingsFilter() {
 
     @Override
     public Optional<SecureSettingsFactory> 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) {