From fc1234d1c51bc4af44aabb560bed3e24d9a84767 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 30 Jun 2023 14:23:08 -0400 Subject: [PATCH 1/4] Check for transient headers in received Signed-off-by: Craig Perkins --- .../security/transport/SecurityInterceptor.java | 1 - .../security/transport/SecurityRequestHandler.java | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index 45b56f2b41..66f4140d3e 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -132,7 +132,6 @@ public void sendRequestDecorate( TransportRequestOptions options, TransportResponseHandler handler ) { - final Map origHeaders0 = getThreadContext().getHeaders(); final User user0 = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); final String injectedUserString = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER); diff --git a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java index 0fdaa1f33e..fc9c37c59f 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java +++ b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java @@ -95,7 +95,6 @@ protected void messageReceivedDecorate( final TransportChannel transportChannel, Task task ) throws Exception { - String resolvedActionClass = request.getClass().getSimpleName(); if (request instanceof BulkShardRequest) { @@ -142,9 +141,10 @@ protected void messageReceivedDecorate( } // bypass non-netty requests - if (channelType.equals("direct")) { - // for direct channel requests user, injected user and injected roles value are already present as transient headers - // so we don't place them here again + if (getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER) != null + || getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER) != null + || getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES) != null + || getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS) != null) { final String rolesValidation = getThreadContext().getHeader( ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION_HEADER From f114b72a894d47b9a085ae01b67a148b3b05cb42 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 30 Jun 2023 15:30:25 -0400 Subject: [PATCH 2/4] Attempt to make logic simpler by checking for transients or request headers Signed-off-by: Craig Perkins --- .../security/support/ConfigConstants.java | 2 + .../security/support/HeaderHelper.java | 6 ++ .../transport/SecurityInterceptor.java | 4 + .../transport/SecurityRequestHandler.java | 100 ++++++++---------- 4 files changed, 55 insertions(+), 57 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index ee04ff62f3..d83dc56e7e 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -47,6 +47,8 @@ public class ConfigConstants { public static final String OPENDISTRO_SECURITY_ORIGIN = OPENDISTRO_SECURITY_CONFIG_PREFIX + "origin"; public static final String OPENDISTRO_SECURITY_ORIGIN_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "origin_header"; + public static final String OPENDISTRO_SECURITY_SAME_NODE_REQUEST = OPENDISTRO_SECURITY_CONFIG_PREFIX + "is_same_node_request"; + public static final String OPENDISTRO_SECURITY_DLS_QUERY_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "dls_query"; public static final String OPENDISTRO_SECURITY_DLS_FILTER_LEVEL_QUERY_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX diff --git a/src/main/java/org/opensearch/security/support/HeaderHelper.java b/src/main/java/org/opensearch/security/support/HeaderHelper.java index e8d50346a8..c7489b8ff6 100644 --- a/src/main/java/org/opensearch/security/support/HeaderHelper.java +++ b/src/main/java/org/opensearch/security/support/HeaderHelper.java @@ -44,6 +44,12 @@ public static boolean isDirectRequest(final ThreadContext context) { || context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_CHANNEL_TYPE) == null; } + public static boolean isSameNodeRequest(final ThreadContext context) { + + return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST) != null + && (boolean) context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST); + } + // CS-SUPPRESS-SINGLE: RegexpSingleline Java Cryptography Extension is unrelated to OpenSearch extensions public static boolean isExtensionRequest(final ThreadContext context) { return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST) == Boolean.TRUE; diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index 66f4140d3e..858833dea9 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -263,6 +263,10 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADE getThreadContext().putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, origin); } + if (getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST) == null) { + getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST, isSameNodeRequest); + } + if (origin == null && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER) == null) { getThreadContext().putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, Origin.LOCAL.toString()); } diff --git a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java index fc9c37c59f..2e1eeadc2d 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java +++ b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java @@ -165,9 +165,43 @@ protected void messageReceivedDecorate( } putInitialActionClassHeader(initialActionClassValue, resolvedActionClass); + } else { + final String userHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_USER_HEADER); + final String injectedRolesHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_HEADER); + final String injectedUserHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER_HEADER); + + if (Strings.isNullOrEmpty(userHeader)) { + // Keeping role injection with higher priority as plugins under OpenSearch will be using this + // on transport layer + if (!Strings.isNullOrEmpty(injectedRolesHeader)) { + getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES, injectedRolesHeader); + } else if (!Strings.isNullOrEmpty(injectedUserHeader)) { + getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUserHeader); + } + } else { + getThreadContext().putTransient( + ConfigConstants.OPENDISTRO_SECURITY_USER, + Objects.requireNonNull((User) Base64Helper.deserializeObject(userHeader)) + ); + } - super.messageReceivedDecorate(request, handler, transportChannel, task); - return; + String originalRemoteAddress = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS_HEADER); + + if (!Strings.isNullOrEmpty(originalRemoteAddress)) { + getThreadContext().putTransient( + ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS, + new TransportAddress((InetSocketAddress) Base64Helper.deserializeObject(originalRemoteAddress)) + ); + } else { + getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS, request.remoteAddress()); + } + + final String rolesValidation = getThreadContext().getHeader( + ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION_HEADER + ); + if (!Strings.isNullOrEmpty(rolesValidation)) { + getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION, rolesValidation); + } } boolean skipSecurityIfDualMode = getThreadContext().getTransient( @@ -190,8 +224,6 @@ protected void messageReceivedDecorate( ); } - super.messageReceivedDecorate(request, handler, transportChannel, task); - return; } // if the incoming request is an internal:* or a shard request allow only if request was sent by a server node @@ -202,6 +234,7 @@ protected void messageReceivedDecorate( if (!HeaderHelper.isInterClusterRequest(getThreadContext()) && !HeaderHelper.isTrustedClusterRequest(getThreadContext()) && !HeaderHelper.isExtensionRequest(getThreadContext()) + && !HeaderHelper.isDirectRequest(getThreadContext()) && !task.getAction().equals("internal:transport/handshake") && (task.getAction().startsWith("internal:") || task.getAction().contains("["))) { // CS-ENFORCE-SINGLE @@ -224,7 +257,8 @@ protected void messageReceivedDecorate( String principal = null; - if ((principal = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_PRINCIPAL)) == null) { + if ((principal = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_PRINCIPAL)) == null + && !HeaderHelper.isDirectRequest(getThreadContext())) { Exception ex = new OpenSearchSecurityException( "No SSL client certificates found for transport type " + transportChannel.getChannelType() @@ -245,58 +279,11 @@ protected void messageReceivedDecorate( // network intercluster request or cross search cluster request // CS-SUPPRESS-SINGLE: RegexpSingleline Used to allow/disallow TLS connections to extensions - if (HeaderHelper.isInterClusterRequest(getThreadContext()) + if (!(HeaderHelper.isInterClusterRequest(getThreadContext()) || HeaderHelper.isTrustedClusterRequest(getThreadContext()) - || HeaderHelper.isExtensionRequest(getThreadContext())) { + || HeaderHelper.isExtensionRequest(getThreadContext()) + || channelType.equals("direct"))) { // CS-ENFORCE-SINGLE - - final String userHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_USER_HEADER); - final String injectedRolesHeader = getThreadContext().getHeader( - ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_HEADER - ); - final String injectedUserHeader = getThreadContext().getHeader( - ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER_HEADER - ); - - if (Strings.isNullOrEmpty(userHeader)) { - // Keeping role injection with higher priority as plugins under OpenSearch will be using this - // on transport layer - if (!Strings.isNullOrEmpty(injectedRolesHeader)) { - getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES, injectedRolesHeader); - } else if (!Strings.isNullOrEmpty(injectedUserHeader)) { - getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUserHeader); - } - } else { - getThreadContext().putTransient( - ConfigConstants.OPENDISTRO_SECURITY_USER, - Objects.requireNonNull((User) Base64Helper.deserializeObject(userHeader)) - ); - } - - String originalRemoteAddress = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS_HEADER); - - if (!Strings.isNullOrEmpty(originalRemoteAddress)) { - getThreadContext().putTransient( - ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS, - new TransportAddress((InetSocketAddress) Base64Helper.deserializeObject(originalRemoteAddress)) - ); - } else { - getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS, request.remoteAddress()); - } - - final String rolesValidation = getThreadContext().getHeader( - ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION_HEADER - ); - if (!Strings.isNullOrEmpty(rolesValidation)) { - getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION, rolesValidation); - } - - } 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 - - // since OS 2.0 we do not support this any longer because transport client no longer available - final OpenSearchException exception = ExceptionUtils.createTransportClientNoLongerSupportedException(); log.error(exception.toString()); transportChannel.sendResponse(exception); @@ -319,9 +306,8 @@ protected void messageReceivedDecorate( } putInitialActionClassHeader(initialActionClassValue, resolvedActionClass); - - super.messageReceivedDecorate(request, handler, transportChannel, task); } + super.messageReceivedDecorate(request, handler, transportChannel, task); } finally { if (isActionTraceEnabled()) { From 35de56db9623aa2c0d07c68126d19a2eb8a64926 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 30 Jun 2023 15:51:50 -0400 Subject: [PATCH 3/4] Remove add tc header Signed-off-by: Craig Perkins --- .../org/opensearch/security/support/ConfigConstants.java | 2 -- .../java/org/opensearch/security/support/HeaderHelper.java | 6 ------ .../opensearch/security/transport/SecurityInterceptor.java | 4 ---- 3 files changed, 12 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index d83dc56e7e..ee04ff62f3 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -47,8 +47,6 @@ public class ConfigConstants { public static final String OPENDISTRO_SECURITY_ORIGIN = OPENDISTRO_SECURITY_CONFIG_PREFIX + "origin"; public static final String OPENDISTRO_SECURITY_ORIGIN_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "origin_header"; - public static final String OPENDISTRO_SECURITY_SAME_NODE_REQUEST = OPENDISTRO_SECURITY_CONFIG_PREFIX + "is_same_node_request"; - public static final String OPENDISTRO_SECURITY_DLS_QUERY_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "dls_query"; public static final String OPENDISTRO_SECURITY_DLS_FILTER_LEVEL_QUERY_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX diff --git a/src/main/java/org/opensearch/security/support/HeaderHelper.java b/src/main/java/org/opensearch/security/support/HeaderHelper.java index c7489b8ff6..e8d50346a8 100644 --- a/src/main/java/org/opensearch/security/support/HeaderHelper.java +++ b/src/main/java/org/opensearch/security/support/HeaderHelper.java @@ -44,12 +44,6 @@ public static boolean isDirectRequest(final ThreadContext context) { || context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_CHANNEL_TYPE) == null; } - public static boolean isSameNodeRequest(final ThreadContext context) { - - return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST) != null - && (boolean) context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST); - } - // CS-SUPPRESS-SINGLE: RegexpSingleline Java Cryptography Extension is unrelated to OpenSearch extensions public static boolean isExtensionRequest(final ThreadContext context) { return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST) == Boolean.TRUE; diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index 858833dea9..66f4140d3e 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -263,10 +263,6 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADE getThreadContext().putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, origin); } - if (getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST) == null) { - getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_SAME_NODE_REQUEST, isSameNodeRequest); - } - if (origin == null && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER) == null) { getThreadContext().putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, Origin.LOCAL.toString()); } From 219b0b329efe7d736ce5a492df87350bc3932795 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 30 Jun 2023 16:35:19 -0400 Subject: [PATCH 4/4] Handle skipSecurityInDualMode Signed-off-by: Craig Perkins --- .../security/transport/SecurityRequestHandler.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java index 2e1eeadc2d..8ea82c9d9d 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java +++ b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java @@ -204,6 +204,11 @@ protected void messageReceivedDecorate( } } + if (channelType.equals("direct")) { + super.messageReceivedDecorate(request, handler, transportChannel, task); + return; + } + boolean skipSecurityIfDualMode = getThreadContext().getTransient( ConfigConstants.SECURITY_SSL_DUAL_MODE_SKIP_SECURITY ) == Boolean.TRUE; @@ -224,6 +229,8 @@ protected void messageReceivedDecorate( ); } + super.messageReceivedDecorate(request, handler, transportChannel, task); + return; } // if the incoming request is an internal:* or a shard request allow only if request was sent by a server node @@ -234,7 +241,6 @@ protected void messageReceivedDecorate( if (!HeaderHelper.isInterClusterRequest(getThreadContext()) && !HeaderHelper.isTrustedClusterRequest(getThreadContext()) && !HeaderHelper.isExtensionRequest(getThreadContext()) - && !HeaderHelper.isDirectRequest(getThreadContext()) && !task.getAction().equals("internal:transport/handshake") && (task.getAction().startsWith("internal:") || task.getAction().contains("["))) { // CS-ENFORCE-SINGLE @@ -257,8 +263,7 @@ protected void messageReceivedDecorate( String principal = null; - if ((principal = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_PRINCIPAL)) == null - && !HeaderHelper.isDirectRequest(getThreadContext())) { + if ((principal = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_PRINCIPAL)) == null) { Exception ex = new OpenSearchSecurityException( "No SSL client certificates found for transport type " + transportChannel.getChannelType() @@ -281,8 +286,7 @@ protected void messageReceivedDecorate( // CS-SUPPRESS-SINGLE: RegexpSingleline Used to allow/disallow TLS connections to extensions if (!(HeaderHelper.isInterClusterRequest(getThreadContext()) || HeaderHelper.isTrustedClusterRequest(getThreadContext()) - || HeaderHelper.isExtensionRequest(getThreadContext()) - || channelType.equals("direct"))) { + || HeaderHelper.isExtensionRequest(getThreadContext()))) { // CS-ENFORCE-SINGLE final OpenSearchException exception = ExceptionUtils.createTransportClientNoLongerSupportedException(); log.error(exception.toString());