Skip to content

Commit

Permalink
ooo
Browse files Browse the repository at this point in the history
  • Loading branch information
@willyborankin
willyborankin committed Jan 12, 2025
1 parent cd1dcbd commit ddc238f
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 77 deletions.
60 changes: 60 additions & 0 deletions src/integrationTest/java/org/opensearch/security/NoSslClusterTests.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package org.opensearch.security;

import java.util.Map;

import com.carrotsearch.randomizedtesting.RandomizedRunner;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.commons.lang3.RandomStringUtils;
import org.awaitility.Awaitility;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;

import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.security.ssl.util.SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED;
import static org.opensearch.security.ssl.util.SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED;
import static org.opensearch.security.support.ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;

@RunWith(RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class NoSslClusterTests {

public static final String ADMIN_PASSWORD = RandomStringUtils.randomAlphabetic(10);

private static final TestSecurityConfig.User USER_ADMIN = new TestSecurityConfig.User("admin").password(ADMIN_PASSWORD)
.roles(ALL_ACCESS);

@ClassRule
public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.users(USER_ADMIN)
.anonymousAuth(false)
.nodeSettings(
Map.of(
SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST,
false,
SECURITY_SSL_HTTP_ENABLED,
false,
SECURITY_SSL_TRANSPORT_ENABLED,
false
)
)
.build();

@Test
public void testNoSslBootUp() throws Exception {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN.getName(), ADMIN_PASSWORD)) {
Awaitility.await()
.alias("Load default configuration")
.until(() -> client.securityHealth().getTextFromJsonBody("/status"), equalTo("UP"));
}
}

}
74 changes: 35 additions & 39 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,11 @@
import org.opensearch.security.transport.SecurityInterceptor;
import org.opensearch.security.user.User;
import org.opensearch.security.user.UserService;
import org.opensearch.tasks.Task;
import org.opensearch.telemetry.tracing.Tracer;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.RemoteClusterService;
import org.opensearch.transport.Transport;
import org.opensearch.transport.Transport.Connection;
import org.opensearch.transport.TransportChannel;
import org.opensearch.transport.TransportInterceptor;
import org.opensearch.transport.TransportRequest;
import org.opensearch.transport.TransportRequestHandler;
Expand Down Expand Up @@ -294,7 +292,7 @@ public void close() throws IOException {
}

private final SslExceptionHandler evaluateSslExceptionHandler() {
if (client || disabled || SSLConfig.isSslOnlyMode()) {
if (client || disabled || sslConfig.isSslOnlyMode()) {
return new SslExceptionHandler() {
};
}
Expand Down Expand Up @@ -354,7 +352,7 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath)
);
}

if (SSLConfig.isSslOnlyMode()) {
if (sslConfig.isSslOnlyMode()) {
this.sslCertReloadEnabled = false;
log.warn("OpenSearch Security plugin run in ssl only mode. No authentication or authorization is performed");
return;
Expand Down Expand Up @@ -408,11 +406,11 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath)
ENDPOINTS_WITH_PERMISSIONS.get(Endpoint.CONFIG).build(SECURITY_CONFIG_UPDATE) + " permission"
);

log.info("Clustername: {}", settings.get("cluster.name", "opensearch"));
log.info("Cluster name: {}", settings.get("cluster.name", "opensearch"));

if (!transportSSLEnabled && !SSLConfig.isSslOnlyMode()) {
throw new IllegalStateException(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED + " must be set to 'true'");
}
// if (!transportSSLEnabled && !SSLConfig.isSslOnlyMode()) {
// throw new IllegalStateException(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED + " must be set to 'true'");
// }

if (!client) {
final List<Path> filesWithWrongPermissions = AccessController.doPrivileged(new PrivilegedAction<List<Path>>() {
Expand All @@ -432,7 +430,7 @@ public List<Path> run() {
}
});

if (filesWithWrongPermissions != null && filesWithWrongPermissions.size() > 0) {
if (filesWithWrongPermissions != null && !filesWithWrongPermissions.isEmpty()) {
for (final Path p : filesWithWrongPermissions) {
if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) {
log.warn("Directory {} has insecure file permissions (should be 0700)", p);
Expand Down Expand Up @@ -597,7 +595,7 @@ public List<RestHandler> getRestHandlers(
)
);

if (!SSLConfig.isSslOnlyMode()) {
if (!sslConfig.isSslOnlyMode()) {
handlers.add(
new SecurityInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool))
);
Expand Down Expand Up @@ -671,7 +669,7 @@ public List<RestHandler> getRestHandlers(
@Override
public UnaryOperator<RestHandler> getRestHandlerWrapper(final ThreadContext threadContext) {

if (client || disabled || SSLConfig.isSslOnlyMode()) {
if (client || disabled || sslConfig.isSslOnlyMode()) {
return (rh) -> rh;
}

Expand All @@ -681,7 +679,7 @@ public UnaryOperator<RestHandler> getRestHandlerWrapper(final ThreadContext thre
@Override
public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> actions = new ArrayList<>(1);
if (!disabled && !SSLConfig.isSslOnlyMode()) {
if (!disabled && !sslConfig.isSslOnlyMode()) {
actions.add(new ActionHandler<>(ConfigUpdateAction.INSTANCE, TransportConfigUpdateAction.class));
// external storage does not support reload and does not provide SSL certs info
if (!ExternalSecurityKeyStore.hasExternalSslContext(settings)) {
Expand All @@ -696,7 +694,7 @@ public UnaryOperator<RestHandler> getRestHandlerWrapper(final ThreadContext thre
public void onIndexModule(IndexModule indexModule) {
// called for every index!

if (!disabled && !client && !SSLConfig.isSslOnlyMode()) {
if (!disabled && !client && !sslConfig.isSslOnlyMode()) {
log.debug("Handle auditLog {} for onIndexModule() of index {}", auditLog.getClass(), indexModule.getIndex().getName());

final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog);
Expand Down Expand Up @@ -837,7 +835,7 @@ public void onQueryPhase(SearchContext searchContext, long tookInNanos) {
@Override
public List<ActionFilter> getActionFilters() {
List<ActionFilter> filters = new ArrayList<>(1);
if (!client && !disabled && !SSLConfig.isSslOnlyMode()) {
if (!client && !disabled && !sslConfig.isSslOnlyMode()) {
filters.add(Objects.requireNonNull(sf));
}
return filters;
Expand All @@ -847,7 +845,7 @@ public List<ActionFilter> getActionFilters() {
public List<TransportInterceptor> getTransportInterceptors(NamedWriteableRegistry namedWriteableRegistry, ThreadContext threadContext) {
List<TransportInterceptor> interceptors = new ArrayList<TransportInterceptor>(1);

if (!client && !disabled && !SSLConfig.isSslOnlyMode()) {
if (!client && !disabled && !sslConfig.isSslOnlyMode()) {
interceptors.add(new TransportInterceptor() {

@Override
Expand All @@ -858,13 +856,7 @@ public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(
TransportRequestHandler<T> actualHandler
) {

return new TransportRequestHandler<T>() {

@Override
public void messageReceived(T request, TransportChannel channel, Task task) throws Exception {
si.getHandler(action, actualHandler).messageReceived(request, channel, task);
}
};
return (request, channel, task) -> si.getHandler(action, actualHandler).messageReceived(request, channel, task);

}

Expand Down Expand Up @@ -904,7 +896,7 @@ public Map<String, Supplier<Transport>> getSecureTransports(
) {
Map<String, Supplier<Transport>> transports = new HashMap<String, Supplier<Transport>>();

if (SSLConfig.isSslOnlyMode()) {
if (sslConfig.isSslOnlyMode()) {
return super.getSecureTransports(
settings,
threadPool,
Expand All @@ -917,7 +909,7 @@ public Map<String, Supplier<Transport>> getSecureTransports(
);
}

if (transportSSLEnabled) {
if (sslConfig.transportSslEnabled()) {
transports.put(
"org.opensearch.security.ssl.http.netty.SecuritySSLNettyTransport",
() -> new SecureNetty4Transport(
Expand Down Expand Up @@ -952,7 +944,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
Tracer tracer
) {

if (SSLConfig.isSslOnlyMode()) {
if (sslConfig.isSslOnlyMode()) {
return super.getSecureHttpTransports(
settings,
threadPool,
Expand All @@ -969,7 +961,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
}

if (!disabled) {
if (!client && httpSSLEnabled) {
if (!client && sslConfig.httpSslEnabled()) {

final ValidatingDispatcher validatingDispatcher = new ValidatingDispatcher(
threadPool.getThreadContext(),
Expand Down Expand Up @@ -1029,8 +1021,8 @@ public Collection<Object> createComponents(
Supplier<RepositoriesService> repositoriesServiceSupplier
) {

SSLConfig.registerClusterSettingsChangeListener(clusterService.getClusterSettings());
if (SSLConfig.isSslOnlyMode()) {
sslConfig.registerClusterSettingsChangeListener(clusterService.getClusterSettings());
if (sslConfig.isSslOnlyMode()) {
return super.createComponents(
localClient,
clusterService,
Expand Down Expand Up @@ -1080,7 +1072,7 @@ public Collection<Object> createComponents(
final PrivilegesInterceptor privilegesInterceptor;

namedXContentRegistry.set(xContentRegistry);
if (SSLConfig.isSslOnlyMode()) {
if (sslConfig.isSslOnlyMode()) {
auditLog = new NullAuditLog();
privilegesInterceptor = new PrivilegesInterceptor(resolver, clusterService, localClient, threadPool);
} else {
Expand Down Expand Up @@ -1121,7 +1113,7 @@ public Collection<Object> createComponents(

dlsFlsBaseContext = new DlsFlsBaseContext(evaluator, threadPool.getThreadContext(), adminDns);

if (SSLConfig.isSslOnlyMode()) {
if (sslConfig.isSslOnlyMode()) {
dlsFlsValve = new DlsFlsRequestValve.NoopDlsFlsRequestValve();
} else {
dlsFlsValve = new DlsFlsValveImpl(
Expand Down Expand Up @@ -1183,7 +1175,7 @@ public Collection<Object> createComponents(
cs,
Objects.requireNonNull(sslExceptionHandler),
Objects.requireNonNull(cih),
SSLConfig,
sslConfig,
OpenSearchSecurityPlugin::isActionTraceEnabled
);
components.add(principalExtractor);
Expand Down Expand Up @@ -1226,7 +1218,7 @@ public Collection<Object> createComponents(

final var allowDefaultInit = settings.getAsBoolean(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false);
final var useClusterState = useClusterStateToInitSecurityConfig(settings);
if (!SSLConfig.isSslOnlyMode() && !isDisabled(settings) && allowDefaultInit && useClusterState) {
if (!sslConfig.isSslOnlyMode() && !isDisabled(settings) && allowDefaultInit && useClusterState) {
clusterService.addListener(cr);
}
return components;
Expand All @@ -1251,9 +1243,13 @@ public Settings additionalSettings() {

builder.put(super.additionalSettings());

if (!SSLConfig.isSslOnlyMode()) {
builder.put(NetworkModule.TRANSPORT_TYPE_KEY, "org.opensearch.security.ssl.http.netty.SecuritySSLNettyTransport");
builder.put(NetworkModule.HTTP_TYPE_KEY, "org.opensearch.security.http.SecurityHttpServerTransport");
if (!sslConfig.isSslOnlyMode()) {
if (sslConfig.httpSslEnabled()) {
builder.put(NetworkModule.TRANSPORT_TYPE_KEY, "org.opensearch.security.ssl.http.netty.SecuritySSLNettyTransport");
}
if (sslConfig.transportSslEnabled()) {
builder.put(NetworkModule.HTTP_TYPE_KEY, "org.opensearch.security.http.SecurityHttpServerTransport");
}
}
return builder.build();
}
Expand Down Expand Up @@ -1375,7 +1371,7 @@ public List<Setting<?>> getSettings() {
)
);

if (!SSLConfig.isSslOnlyMode()) {
if (!sslConfig.isSslOnlyMode()) {
settings.add(
Setting.listSetting(
ConfigConstants.SECURITY_AUTHCZ_ADMIN_DN,
Expand Down Expand Up @@ -2082,7 +2078,7 @@ public List<String> getSettingsFilter() {
@Override
public void onNodeStarted(DiscoveryNode localNode) {
this.localNode.set(localNode);
if (!SSLConfig.isSslOnlyMode() && !client && !disabled && !useClusterStateToInitSecurityConfig(settings)) {
if (!sslConfig.isSslOnlyMode() && !client && !disabled && !useClusterStateToInitSecurityConfig(settings)) {
cr.initOnNodeStart();
}
final Set<ModuleInfo> securityModules = ReflectionHelper.getModulesLoaded();
Expand All @@ -2096,7 +2092,7 @@ public void onNodeStarted(DiscoveryNode localNode) {
@Override
public Collection<Class<? extends LifecycleComponent>> getGuiceServiceClasses() {

if (client || disabled || SSLConfig.isSslOnlyMode()) {
if (client || disabled || sslConfig.isSslOnlyMode()) {
return Collections.emptyList();
}

Expand Down Expand Up @@ -2157,7 +2153,7 @@ public Optional<SecureSettingsFactory> getSecureSettingFactory(Settings settings
sslSettingsManager,
evaluateSslExceptionHandler(),
securityRestHandler,
SSLConfig
sslConfig
)
);
}
Expand Down
Loading

0 comments on commit ddc238f

Please sign in to comment.