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

Ensure that dual mode enabled flag from cluster settings can get propagated to core #4820

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
@@ -0,0 +1,73 @@
/*
* 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 org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

/**
* 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);

Thread.sleep(2000);
cwperks marked this conversation as resolved.
Show resolved Hide resolved

TestRestClient.HttpResponse secondResponse = client.get("_cat/nodes");
secondResponse.assertStatusCode(200);

String[] secondLines = secondResponse.getBody().split("\n");
assertEquals("Expected 3 nodes after disabling SSL dual mode", 3, secondLines.length);
} catch (InterruptedException e) {
fail("Test failed due to exception: " + e.getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -125,6 +130,7 @@ private LocalCluster(
if (StringUtils.isNoneBlank(defaultConfigurationInitDirectory)) {
System.setProperty(INIT_CONFIGURATION_DIR, defaultConfigurationInitDirectory);
}
this.expectedNodeStartupCount = expectedNodeStartupCount;
}

public String getSnapshotDirPath() {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<>();
Expand Down Expand Up @@ -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) {
Expand All @@ -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
*/
Expand Down Expand Up @@ -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,
Expand All @@ -524,7 +563,8 @@ public LocalCluster build() {
remoteClusters,
testIndices,
loadConfigurationIntoIndex,
defaultConfigurationInitDirectory
defaultConfigurationInitDirectory,
expectedNodeStartupCount
);
} catch (Exception e) {
log.error("Failed to build LocalCluster", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2166,7 +2166,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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,9 @@

@Override
public Optional<SecureSettingsFactory> getSecureSettingFactory(Settings settings) {
return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, NOOP_SSL_EXCEPTION_HANDLER, securityRestHandler));
return Optional.of(

Check warning on line 677 in src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java#L677

Added line #L677 was not covered by tests
new OpenSearchSecureSettingsFactory(threadPool, sks, NOOP_SSL_EXCEPTION_HANDLER, securityRestHandler, SSLConfig)
);
}

protected Settings migrateSettings(Settings settings) {
Expand Down
Loading