Skip to content

Commit

Permalink
Ensure that dual mode enabled flag from cluster settings can get prop…
Browse files Browse the repository at this point in the history
…agated to core (#4820)

Signed-off-by: Craig Perkins <[email protected]>
(cherry picked from commit 811f26d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] committed Oct 21, 2024
1 parent 24a3ffa commit f4f50fc
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -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;
});
}
}
}
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 @@ -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")
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 @@ 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(

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

0 comments on commit f4f50fc

Please sign in to comment.