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.

Sign up for GitHub

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

[FEATURE] Improve built-in secure transports support #4179

Merged
merged 2 commits into from
Mar 28, 2024
Merged

[FEATURE] Improve built-in secure transports support #4179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e450fc0
[FEATURE] Improve built-in secure transports support
reta Mar 27, 2024
65c6e3c
Address code review comments, added more tests
reta Mar 28, 2024
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
22 changes: 10 additions & 12 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.http.HttpServerTransport;
import org.opensearch.http.HttpServerTransport.Dispatcher;
import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport;
import org.opensearch.identity.Subject;
import org.opensearch.identity.noop.NoopSubject;
import org.opensearch.index.IndexModule;
Expand All @@ -117,6 +118,7 @@
import org.opensearch.plugins.ExtensionAwarePlugin;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.plugins.MapperPlugin;
import org.opensearch.plugins.SecureHttpTransportSettingsProvider;
import org.opensearch.plugins.SecureSettingsFactory;
import org.opensearch.plugins.SecureTransportSettingsProvider;
import org.opensearch.repositories.RepositoriesService;
Expand Down Expand Up @@ -156,7 +158,6 @@
import org.opensearch.security.filter.SecurityFilter;
import org.opensearch.security.filter.SecurityRestFilter;
import org.opensearch.security.http.NonSslHttpServerTransport;
import org.opensearch.security.http.SecureHttpServerTransport;
import org.opensearch.security.http.XFFResolver;
import org.opensearch.security.identity.SecurityTokenManager;
import org.opensearch.security.privileges.PrivilegesEvaluator;
Expand Down Expand Up @@ -239,7 +240,6 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile PrivilegesEvaluator evaluator;
private volatile UserService userService;
private volatile RestLayerPrivilegesEvaluator restLayerEvaluator;
private volatile ThreadPool threadPool;
private volatile ConfigurationRepository cr;
private volatile AdminDNs adminDns;
private volatile ClusterService cs;
Expand Down Expand Up @@ -927,7 +927,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
NetworkService networkService,
Dispatcher dispatcher,
ClusterSettings clusterSettings,
SecureTransportSettingsProvider secureTransportSettingsProvider,
SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
Tracer tracer
) {

Expand All @@ -942,7 +942,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
networkService,
dispatcher,
clusterSettings,
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
tracer
);
}
Expand All @@ -958,7 +958,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
evaluateSslExceptionHandler()
);
// TODO close odshst
final SecureHttpServerTransport odshst = new SecureHttpServerTransport(
final SecureNetty4HttpServerTransport odshst = new SecureNetty4HttpServerTransport(
migrateSettings(settings),
networkService,
bigArrays,
Expand All @@ -967,9 +967,8 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
validatingDispatcher,
clusterSettings,
sharedGroupFactory,
secureTransportSettingsProvider,
tracer,
securityRestHandler
secureHttpTransportSettingsProvider,
tracer
);

return Collections.singletonMap("org.opensearch.security.http.SecurityHttpServerTransport", () -> odshst);
Expand All @@ -985,9 +984,8 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
dispatcher,
clusterSettings,
sharedGroupFactory,
secureTransportSettingsProvider,
tracer,
securityRestHandler
secureHttpTransportSettingsProvider,
tracer
)
);
}
Expand Down Expand Up @@ -2032,7 +2030,7 @@ public SecurityTokenManager getTokenManager() {

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

public static class GuiceHolder implements LifecycleComponent {
Expand Down
20 changes: 10 additions & 10 deletions src/main/java/org/opensearch/security/filter/SecurityRestFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import org.opensearch.security.privileges.RestLayerPrivilegesEvaluator;
import org.opensearch.security.securityconf.impl.AllowlistingSettings;
import org.opensearch.security.securityconf.impl.WhitelistingSettings;
import org.opensearch.security.ssl.http.netty.Netty4HttpRequestHeaderVerifier;
import org.opensearch.security.ssl.transport.PrincipalExtractor;
import org.opensearch.security.ssl.util.ExceptionUtils;
import org.opensearch.security.ssl.util.SSLRequestHelper;
Expand All @@ -69,10 +70,6 @@

import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX;
import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
import static org.opensearch.security.http.SecureHttpServerTransport.CONTEXT_TO_RESTORE;
import static org.opensearch.security.http.SecureHttpServerTransport.EARLY_RESPONSE;
import static org.opensearch.security.http.SecureHttpServerTransport.IS_AUTHENTICATED;
import static org.opensearch.security.http.SecureHttpServerTransport.UNCONSUMED_PARAMS;

public class SecurityRestFilter {

Expand Down Expand Up @@ -128,15 +125,18 @@ public AuthczRestHandler(RestHandler original, AdminDNs adminDNs) {

@Override
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
final Optional<SecurityResponse> maybeSavedResponse = NettyAttribute.popFrom(request, EARLY_RESPONSE);
final Optional<SecurityResponse> maybeSavedResponse = NettyAttribute.popFrom(
request,
Netty4HttpRequestHeaderVerifier.EARLY_RESPONSE
);
if (maybeSavedResponse.isPresent()) {
NettyAttribute.clearAttribute(request, CONTEXT_TO_RESTORE);
NettyAttribute.clearAttribute(request, IS_AUTHENTICATED);
NettyAttribute.clearAttribute(request, Netty4HttpRequestHeaderVerifier.CONTEXT_TO_RESTORE);
NettyAttribute.clearAttribute(request, Netty4HttpRequestHeaderVerifier.IS_AUTHENTICATED);
channel.sendResponse(maybeSavedResponse.get().asRestResponse());
return;
}

NettyAttribute.popFrom(request, CONTEXT_TO_RESTORE).ifPresent(storedContext -> {
NettyAttribute.popFrom(request, Netty4HttpRequestHeaderVerifier.CONTEXT_TO_RESTORE).ifPresent(storedContext -> {
// X_OPAQUE_ID will be overritten on restore - save to apply after restoring the saved context
final String xOpaqueId = threadContext.getHeader(Task.X_OPAQUE_ID);
storedContext.restore();
Expand All @@ -145,7 +145,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
}
});

NettyAttribute.popFrom(request, UNCONSUMED_PARAMS).ifPresent(unconsumedParams -> {
NettyAttribute.popFrom(request, Netty4HttpRequestHeaderVerifier.UNCONSUMED_PARAMS).ifPresent(unconsumedParams -> {
for (String unconsumedParam : unconsumedParams) {
// Consume the parameter on the RestRequest
request.param(unconsumedParam);
Expand All @@ -155,7 +155,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
final SecurityRequestChannel requestChannel = SecurityRequestFactory.from(request, channel);

// Authenticate request
if (!NettyAttribute.popFrom(request, IS_AUTHENTICATED).orElse(false)) {
if (!NettyAttribute.popFrom(request, Netty4HttpRequestHeaderVerifier.IS_AUTHENTICATED).orElse(false)) {
// we aren't authenticated so we should skip this step
checkAndAuthenticateRequest(requestChannel);
}
Expand Down
27 changes: 4 additions & 23 deletions src/main/java/org/opensearch/security/http/NonSslHttpServerTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,15 @@
import org.opensearch.http.HttpHandlingSettings;
import org.opensearch.http.netty4.Netty4HttpServerTransport;
import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport;
import org.opensearch.plugins.SecureTransportSettingsProvider;
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.plugins.SecureHttpTransportSettingsProvider;
import org.opensearch.telemetry.tracing.Tracer;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.SharedGroupFactory;

import io.netty.channel.Channel;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelInboundHandlerAdapter;

public class NonSslHttpServerTransport extends SecureNetty4HttpServerTransport {

private final ChannelInboundHandlerAdapter headerVerifier;

public NonSslHttpServerTransport(
final Settings settings,
final NetworkService networkService,
Expand All @@ -59,9 +52,8 @@ public NonSslHttpServerTransport(
final Dispatcher dispatcher,
final ClusterSettings clusterSettings,
final SharedGroupFactory sharedGroupFactory,
final SecureTransportSettingsProvider secureTransportSettingsProvider,
final Tracer tracer,
final SecurityRestFilter restFilter
final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
final Tracer tracer
) {
super(
settings,
Expand All @@ -72,10 +64,9 @@ public NonSslHttpServerTransport(
dispatcher,
clusterSettings,
sharedGroupFactory,
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
tracer
);
headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings);
}

@Override
Expand All @@ -94,14 +85,4 @@ protected void initChannel(Channel ch) throws Exception {
super.initChannel(ch);
}
}

@Override
protected ChannelInboundHandlerAdapter createHeaderVerifier() {
return headerVerifier;
}

@Override
protected ChannelInboundHandlerAdapter createDecompressor() {
return new Netty4ConditionalDecompressor();
}
}
101 changes: 0 additions & 101 deletions src/main/java/org/opensearch/security/http/SecureHttpServerTransport.java
View file
Open in desktop

This file was deleted.

Loading
Loading