Skip to content

Identify extension Transport requests and permit handshake and extension registration actions #2599

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

Merged
cwperks merged 20 commits into from
May 3, 2023
Merged

Identify extension Transport requests and permit handshake and extension registration actions #2599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e2b750d
WIP on rest layer authz
cwperks Mar 27, 2023
478523a
WIP on rest-layer authz
cwperks Mar 28, 2023
3444295
Extension handshake
cwperks Mar 28, 2023
eb03f38
Extension TLS
cwperks Mar 28, 2023
85dba00
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
3419692
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
9ef4e0f
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
b7686dd
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
298aa27
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
5a64ef7
Remove local check
cwperks Apr 2, 2023
62a076c
Use extensions feature flag
cwperks Apr 3, 2023
87c421b
Merge branch 'main' into extension-tls
cwperks Apr 10, 2023
40b9c98
Merge branch 'main' into extension-tls
cwperks Apr 22, 2023
b1f7369
Switch to values.iterator
cwperks Apr 22, 2023
a740ea4
Merge branch 'main' into extension-tls
cwperks Apr 27, 2023
6cdda66
Remove ImmutableOpenMap
cwperks Apr 27, 2023
bec4270
Merge branch 'main' into extension-tls
cwperks Apr 28, 2023
bca8b36
Check in extensions registry
cwperks May 1, 2023
e3e45ef
Remove Feature Flag check
cwperks May 2, 2023
67a541e
Remove unused import
cwperks May 2, 2023
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
8 changes: 7 additions & 1 deletion 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 @@ -94,6 +94,7 @@
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.env.Environment;
import org.opensearch.env.NodeEnvironment;
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.http.HttpServerTransport;
import org.opensearch.http.HttpServerTransport.Dispatcher;
import org.opensearch.index.Index;
Expand Down Expand Up @@ -1187,13 +1188,16 @@ public static class GuiceHolder implements LifecycleComponent {
private static IndicesService indicesService;
private static PitService pitService;

private static ExtensionsManager extensionsManager;

@Inject
public GuiceHolder(final RepositoriesService repositoriesService,
final TransportService remoteClusterService, IndicesService indicesService, PitService pitService) {
final TransportService remoteClusterService, IndicesService indicesService, PitService pitService, ExtensionsManager extensionsManager) {
GuiceHolder.repositoriesService = repositoriesService;
GuiceHolder.remoteClusterService = remoteClusterService.getRemoteClusterService();
GuiceHolder.indicesService = indicesService;
GuiceHolder.pitService = pitService;
GuiceHolder.extensionsManager = extensionsManager;
}

public static RepositoriesService getRepositoriesService() {
Expand All @@ -1210,6 +1214,8 @@ public static IndicesService getIndicesService() {

public static PitService getPitService() { return pitService; }

public static ExtensionsManager getExtensionsManager() { return extensionsManager; }


@Override
public void close() {
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/opensearch/security/support/ConfigConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ public class ConfigConstants {

public static final String OPENDISTRO_SECURITY_SSL_TRANSPORT_TRUSTED_CLUSTER_REQUEST = OPENDISTRO_SECURITY_CONFIG_PREFIX+"ssl_transport_trustedcluster_request";

public static final String OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST = OPENDISTRO_SECURITY_CONFIG_PREFIX+"ssl_transport_extension_request";


/**
* Set by the SSL plugin, this is the peer node certificate on the transport layer
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/opensearch/security/support/HeaderHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ public static boolean isDirectRequest(final ThreadContext context) {
return "direct".equals(context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_CHANNEL_TYPE))
|| context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_CHANNEL_TYPE) == null;
}

public static boolean isExtensionRequest(final ThreadContext context) {
return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST) == Boolean.TRUE;
}


public static String getSafeFromHeader(final ThreadContext context, final String headerName) {
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.transport.TransportAddress;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.search.internal.ShardSearchRequest;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.auditlog.AuditLog.Origin;
import org.opensearch.security.ssl.SslExceptionHandler;
Expand Down Expand Up @@ -195,6 +197,7 @@ else if(!Strings.isNullOrEmpty(injectedUserHeader)) {
//also allow when issued from a remote cluster for cross cluster search
if ( !HeaderHelper.isInterClusterRequest(getThreadContext())
&& !HeaderHelper.isTrustedClusterRequest(getThreadContext())
&& !HeaderHelper.isExtensionRequest(getThreadContext())
&& !task.getAction().equals("internal:transport/handshake")
&& (task.getAction().startsWith("internal:") || task.getAction().contains("["))) {

Expand All @@ -216,14 +219,14 @@ else if(!Strings.isNullOrEmpty(injectedUserHeader)) {
transportChannel.sendResponse(ex);
return;
} else {

if(getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN) == null) {
getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN, Origin.TRANSPORT.toString());
}

//network intercluster request or cross search cluster request
if(HeaderHelper.isInterClusterRequest(getThreadContext())
|| HeaderHelper.isTrustedClusterRequest(getThreadContext())) {
|| HeaderHelper.isTrustedClusterRequest(getThreadContext())
|| HeaderHelper.isExtensionRequest(getThreadContext())) {

final String userHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_USER_HEADER);
final String injectedRolesHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_HEADER);
Expand Down Expand Up @@ -256,7 +259,6 @@ else if(!Strings.isNullOrEmpty(injectedUserHeader)) {
}

} else {

//this is a netty request from a non-server node (maybe also be internal: or a shard request)
//and therefore issued by a transport client

Expand Down Expand Up @@ -326,6 +328,14 @@ protected void addAdditionalContextValues(final String action, final TransportRe
}
}

String extensionUniqueId = getThreadContext().getHeader("extension_unique_id");
Copy link
Member

@peternied peternied May 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For follow up PR, can we put this in a constant in core and use that constant here?

Copy link
Member Author

@cwperks cwperks May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peternied yes that's a good idea to import the constant from core. After this PR, I plan to expand on the TLS logic to introduce extension_dns similar to node_dns that make this check stronger by verifying that the principal extracted from the extension certificate is present in a list of known principals and will include the change to make this a constant.

For the FeatureFlags, I do think it makes sense to include a reference to the feature flag since this feature is directly related to extensions though its not necessarily needed.

if (extensionUniqueId != null) {
ExtensionsManager extManager = OpenSearchSecurityPlugin.GuiceHolder.getExtensionsManager();
if (extManager.getExtensionIdMap().containsKey(extensionUniqueId)) {
getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST, Boolean.TRUE);
}
}

super.addAdditionalContextValues(action, request, localCerts, peerCerts, principal);
}
}