Fix authorization bypass in stashContext() (backport #1760 to 2.8)

src/main/java/org/opensearch/securityanalytics/transport/QueryUtils.java

@@ -0,0 +1,142 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.opensearch.securityanalytics.transport;
+
+import org.opensearch.index.query.BoolQueryBuilder;
+import org.opensearch.index.query.BoostingQueryBuilder;
+import org.opensearch.index.query.ConstantScoreQueryBuilder;
+import org.opensearch.index.query.DisMaxQueryBuilder;
+import org.opensearch.index.query.NestedQueryBuilder;
+import org.opensearch.index.query.QueryBuilder;
+import org.opensearch.index.query.TermsQueryBuilder;
+import org.opensearch.index.query.functionscore.FunctionScoreQueryBuilder;
+
+import java.util.List;
+
+/**
+ * Utility methods for inspecting query trees for security-sensitive patterns.
+ * Uses manual instanceof traversal to walk all sub-queries, ensuring complete
+ * coverage of compound query types. Unrecognized query types are denied by default
+ * (treated as potentially containing terms lookup) for forward-compatible safety.
+ */
+public class QueryUtils {
+
+    private QueryUtils() {}
+
+    /**
+     * Checks if a query tree contains any TermsQueryBuilder with a termsLookup
+     * (i.e., a cross-index terms lookup that could be used to probe data in unauthorized indices).
+     * Traverses compound queries manually via instanceof checks.
+     * Unrecognized/opaque query types are treated as unsafe (returns true) to deny by default.
+     */
+    public static boolean containsTermsLookup(QueryBuilder query) {
+        if (query == null) {
+            return false;
+        }
+
+        // Direct check for TermsQueryBuilder with termsLookup
+        if (query instanceof TermsQueryBuilder) {
+            return ((TermsQueryBuilder) query).termsLookup() != null;
+        }
+
+        // Recurse into known compound query types
+        if (query instanceof BoolQueryBuilder) {
+            BoolQueryBuilder boolQuery = (BoolQueryBuilder) query;
+            for (QueryBuilder clause : boolQuery.must()) {
+                if (containsTermsLookup(clause)) return true;
+            }
+            for (QueryBuilder clause : boolQuery.mustNot()) {
+                if (containsTermsLookup(clause)) return true;
+            }
+            for (QueryBuilder clause : boolQuery.should()) {
+                if (containsTermsLookup(clause)) return true;
+            }
+            for (QueryBuilder clause : boolQuery.filter()) {
+                if (containsTermsLookup(clause)) return true;
+            }
+            return false;
+        }
+
+        if (query instanceof ConstantScoreQueryBuilder) {
+            return containsTermsLookup(((ConstantScoreQueryBuilder) query).innerQuery());
+        }
+
+        if (query instanceof BoostingQueryBuilder) {
+            BoostingQueryBuilder boosting = (BoostingQueryBuilder) query;
+            return containsTermsLookup(boosting.positiveQuery()) || containsTermsLookup(boosting.negativeQuery());
+        }
+
+        if (query instanceof DisMaxQueryBuilder) {
+            for (QueryBuilder clause : ((DisMaxQueryBuilder) query).innerQueries()) {
+                if (containsTermsLookup(clause)) return true;
+            }
+            return false;
+        }
+
+        if (query instanceof NestedQueryBuilder) {
+            return containsTermsLookup(((NestedQueryBuilder) query).query());
+        }
+
+        if (query instanceof FunctionScoreQueryBuilder) {
+            return containsTermsLookup(((FunctionScoreQueryBuilder) query).query());
+        }
+
+        // Leaf query types that cannot contain sub-queries or terms lookups are safe.
+        // Known safe leaves: MatchAllQueryBuilder, MatchQueryBuilder, TermQueryBuilder,
+        // RangeQueryBuilder, ExistsQueryBuilder, WildcardQueryBuilder, PrefixQueryBuilder,
+        // RegexpQueryBuilder, FuzzyQueryBuilder, IdsQueryBuilder, MatchPhraseQueryBuilder, etc.
+        // For any unrecognized compound type, deny by default for safety.
+        String queryName = query.getName();
+        if (isKnownSafeLeaf(queryName)) {
+            return false;
+        }
+
+        // Unrecognized query type — deny by default
+        return true;
+    }
+
+    private static boolean isKnownSafeLeaf(String queryName) {
+        switch (queryName) {
+            case "match_all":
+            case "match_none":
+            case "match":
+            case "match_phrase":
+            case "match_phrase_prefix":
+            case "multi_match":
+            case "term":
+            case "range":
+            case "exists":
+            case "wildcard":
+            case "prefix":
+            case "regexp":
+            case "fuzzy":
+            case "ids":
+            case "type":
+            case "query_string":
+            case "simple_query_string":
+            case "span_term":
+            case "span_first":
+            case "span_near":
+            case "span_or":
+            case "span_not":
+            case "span_containing":
+            case "span_within":
+            case "span_multi":
+            case "more_like_this":
+            case "common":
+            case "geo_bounding_box":
+            case "geo_distance":
+            case "geo_polygon":
+            case "geo_shape":
+            case "script":
+            case "percolate":
+            case "wrapper":
+            case "match_bool_prefix":
+                return true;
+            default:
+                return false;
+        }
+    }
+}

src/main/java/org/opensearch/securityanalytics/transport/TransportCreateIndexMappingsAction.java

@@ -5,10 +5,11 @@
 package org.opensearch.securityanalytics.transport;
 
 import org.opensearch.action.ActionListener;
+import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
 import org.opensearch.action.support.master.AcknowledgedResponse;
-import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.securityanalytics.action.CreateIndexMappingsAction;
@@ -18,11 +19,14 @@
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
 
+import java.util.Map;
+
 public class TransportCreateIndexMappingsAction extends HandledTransportAction<CreateIndexMappingsRequest, AcknowledgedResponse> {
     private MapperService mapperService;
     private ClusterService clusterService;
 
     private final ThreadPool threadPool;
+    private final Client client;
 
 
     @Inject
@@ -31,24 +35,35 @@ public TransportCreateIndexMappingsAction(
             ActionFilters actionFilters,
             ThreadPool threadPool,
             MapperService mapperService,
-            ClusterService clusterService
+            ClusterService clusterService,
+            Client client
     ) {
         super(CreateIndexMappingsAction.NAME, transportService, actionFilters, CreateIndexMappingsRequest::new);
         this.clusterService = clusterService;
         this.mapperService = mapperService;
         this.threadPool = threadPool;
+        this.client = client;
     }
 
     @Override
     protected void doExecute(Task task, CreateIndexMappingsRequest request, ActionListener<AcknowledgedResponse> actionListener) {
-        this.threadPool.getThreadContext().stashContext();
-
-        mapperService.createMappingAction(
-                request.getIndexName(),
-                request.getRuleTopic(),
-                request.getAliasMappings(),
-                request.getPartial(),
-                actionListener
-        );
+        // Verify caller has indices:admin/mapping/put on the target index before elevating privileges.
+        // Issues a no-op PutMappingRequest (empty properties) as the caller — the security plugin
+        // checks the permission naturally without stashContext, so unauthorized users get 403.
+        PutMappingRequest putMappingRequest = new PutMappingRequest(request.getIndexName())
+                .source(Map.of("properties", Map.of()));
+        client.admin().indices().putMapping(putMappingRequest, ActionListener.wrap(
+                putMappingResponse -> {
+                    this.threadPool.getThreadContext().stashContext();
+                    mapperService.createMappingAction(
+                            request.getIndexName(),
+                            request.getRuleTopic(),
+                            request.getAliasMappings(),
+                            request.getPartial(),
+                            actionListener
+                    );
+                },
+                actionListener::onFailure
+        ));
     }
-}
+}

src/main/java/org/opensearch/securityanalytics/transport/TransportGetIndexMappingsAction.java

@@ -4,19 +4,17 @@
  */
 package org.opensearch.securityanalytics.transport;
 
-import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.ActionListener;
+import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
-import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
-import org.opensearch.rest.RestStatus;
 import org.opensearch.securityanalytics.action.GetIndexMappingsAction;
 import org.opensearch.securityanalytics.mapper.MapperService;
 import org.opensearch.securityanalytics.action.GetIndexMappingsRequest;
 import org.opensearch.securityanalytics.action.GetIndexMappingsResponse;
-import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
 import org.opensearch.tasks.Task;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
@@ -26,6 +24,7 @@ public class TransportGetIndexMappingsAction extends HandledTransportAction<GetI
     private ClusterService clusterService;
 
     private final ThreadPool threadPool;
+    private final Client client;
 
     @Inject
     public TransportGetIndexMappingsAction(
@@ -34,18 +33,26 @@ public TransportGetIndexMappingsAction(
             GetIndexMappingsAction getIndexMappingsAction,
             MapperService mapperService,
             ClusterService clusterService,
-            ThreadPool threadPool
+            ThreadPool threadPool,
+            Client client
     ) {
         super(getIndexMappingsAction.NAME, transportService, actionFilters, GetIndexMappingsRequest::new);
         this.clusterService = clusterService;
         this.mapperService = mapperService;
         this.threadPool = threadPool;
+        this.client = client;
     }
 
     @Override
     protected void doExecute(Task task, GetIndexMappingsRequest request, ActionListener<GetIndexMappingsResponse> actionListener) {
-        this.threadPool.getThreadContext().stashContext();
-
-        mapperService.getMappingAction(request.getIndexName(), actionListener);
+        // Verify caller has permission on the target index before elevating privileges
+        GetMappingsRequest getMappingsRequest = new GetMappingsRequest().indices(request.getIndexName());
+        client.admin().indices().getMappings(getMappingsRequest, ActionListener.wrap(
+                getMappingsResponse -> {
+                    this.threadPool.getThreadContext().stashContext();
+                    mapperService.getMappingAction(request.getIndexName(), actionListener);
+                },
+                actionListener::onFailure
+        ));
     }
-}
+}

src/main/java/org/opensearch/securityanalytics/transport/TransportGetMappingsViewAction.java

@@ -4,22 +4,17 @@
  */
 package org.opensearch.securityanalytics.transport;
 
-import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.ActionListener;
+import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
-import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
-import org.opensearch.rest.RestStatus;
-import org.opensearch.securityanalytics.action.GetIndexMappingsAction;
-import org.opensearch.securityanalytics.action.GetIndexMappingsRequest;
-import org.opensearch.securityanalytics.action.GetIndexMappingsResponse;
 import org.opensearch.securityanalytics.action.GetMappingsViewAction;
 import org.opensearch.securityanalytics.action.GetMappingsViewRequest;
 import org.opensearch.securityanalytics.action.GetMappingsViewResponse;
 import org.opensearch.securityanalytics.mapper.MapperService;
-import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
 import org.opensearch.tasks.Task;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
@@ -28,6 +23,7 @@ public class TransportGetMappingsViewAction extends HandledTransportAction<GetMa
     private MapperService mapperService;
     private ClusterService clusterService;
     private final ThreadPool threadPool;
+    private final Client client;
 
     @Inject
     public TransportGetMappingsViewAction(
@@ -36,17 +32,26 @@ public TransportGetMappingsViewAction(
             GetMappingsViewAction getMappingsViewAction,
             MapperService mapperService,
             ClusterService clusterService,
-            ThreadPool threadPool
+            ThreadPool threadPool,
+            Client client
     ) {
         super(getMappingsViewAction.NAME, transportService, actionFilters, GetMappingsViewRequest::new);
         this.clusterService = clusterService;
         this.mapperService = mapperService;
         this.threadPool = threadPool;
+        this.client = client;
     }
 
     @Override
     protected void doExecute(Task task, GetMappingsViewRequest request, ActionListener<GetMappingsViewResponse> actionListener) {
-        this.threadPool.getThreadContext().stashContext();
-        this.mapperService.getMappingsViewAction(request.getIndexName(), request.getRuleTopic(), actionListener);
+        // Verify caller has permission on the target index before elevating privileges
+        GetMappingsRequest getMappingsRequest = new GetMappingsRequest().indices(request.getIndexName());
+        client.admin().indices().getMappings(getMappingsRequest, ActionListener.wrap(
+                getMappingsResponse -> {
+                    this.threadPool.getThreadContext().stashContext();
+                    this.mapperService.getMappingsViewAction(request.getIndexName(), request.getRuleTopic(), actionListener);
+                },
+                actionListener::onFailure
+        ));
     }
-}
+}

src/main/java/org/opensearch/securityanalytics/transport/TransportSearchCorrelationRuleAction.java

@@ -11,6 +11,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.search.TotalHits;
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.search.SearchResponse;
 import org.opensearch.action.search.ShardSearchFailure;
@@ -19,10 +20,11 @@
 import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
-import org.opensearch.commons.notifications.action.SendNotificationRequest;
 import org.opensearch.index.IndexNotFoundException;
+import org.opensearch.rest.RestStatus;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.SearchHits;
+import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.search.internal.InternalSearchResponse;
 import org.opensearch.securityanalytics.action.SearchCorrelationRuleAction;
 import org.opensearch.securityanalytics.action.SearchCorrelationRuleRequest;
@@ -82,6 +84,13 @@ public TransportSearchCorrelationRuleAction(
 
     @Override
     protected void doExecute(Task task, SearchCorrelationRuleRequest request, ActionListener<SearchResponse> listener) {
+        SearchSourceBuilder source = request.getSearchRequest().source();
+        if (source != null && source.query() != null && QueryUtils.containsTermsLookup(source.query())) {
+            listener.onFailure(new OpenSearchStatusException(
+                    "Terms lookup queries referencing external indices are not permitted in correlation rule search", RestStatus.FORBIDDEN));
+            return;
+        }
+
         this.threadPool.getThreadContext().stashContext();
 
         client.search(