diff --git a/src/main/java/org/opensearch/securityanalytics/transport/QueryUtils.java b/src/main/java/org/opensearch/securityanalytics/transport/QueryUtils.java new file mode 100644 index 000000000..9c69d85a8 --- /dev/null +++ b/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; + } + } +} diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportCreateIndexMappingsAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportCreateIndexMappingsAction.java index 162c45c16..0828fd8ca 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportCreateIndexMappingsAction.java +++ b/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 { 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 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 + )); } -} \ No newline at end of file +} diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportGetIndexMappingsAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportGetIndexMappingsAction.java index b8b9110d8..d01294a4c 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportGetIndexMappingsAction.java +++ b/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 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 + )); } -} \ No newline at end of file +} diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportGetMappingsViewAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportGetMappingsViewAction.java index 319bc5bca..3649d62a0 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportGetMappingsViewAction.java +++ b/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 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 + )); } -} \ No newline at end of file +} diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchCorrelationRuleAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchCorrelationRuleAction.java index 4affd451f..31cd3120a 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchCorrelationRuleAction.java +++ b/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 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( diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchDetectorAction.java index a46952cc6..ab794da5f 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchDetectorAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchDetectorAction.java @@ -6,44 +6,27 @@ 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.SearchResponseSections; -import org.opensearch.action.search.ShardSearchFailure; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; import org.opensearch.commons.authuser.User; import org.opensearch.client.Client; import org.opensearch.common.inject.Inject; -import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.settings.Settings; import org.opensearch.cluster.service.ClusterService; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.rest.RestStatus; -import org.opensearch.search.SearchHit; -import org.opensearch.search.SearchHits; -import org.opensearch.search.aggregations.InternalAggregations; -import org.opensearch.search.internal.InternalSearchResponse; -import org.opensearch.search.profile.SearchProfileShardResults; -import org.opensearch.search.suggest.Suggest; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.securityanalytics.action.SearchDetectorAction; import org.opensearch.securityanalytics.action.SearchDetectorRequest; import org.opensearch.securityanalytics.settings.SecurityAnalyticsSettings; import org.opensearch.securityanalytics.util.DetectorIndices; import org.opensearch.threadpool.ThreadPool; - import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; - -import java.util.Collections; -import java.util.Locale; - -import static org.opensearch.rest.RestStatus.OK; import static org.opensearch.securityanalytics.util.DetectorUtils.getEmptySearchResponse; public class TransportSearchDetectorAction extends HandledTransportAction implements SecureTransportAction { @@ -91,6 +74,14 @@ protected void doExecute(Task task, SearchDetectorRequest searchDetectorRequest, addFilter(user, searchDetectorRequest.searchRequest().source(), "detector.user.backend_roles.keyword"); } + // Reject queries containing terms lookups that reference external indices + SearchSourceBuilder source = searchDetectorRequest.searchRequest().source(); + if (source != null && source.query() != null && QueryUtils.containsTermsLookup(source.query())) { + actionListener.onFailure(new OpenSearchStatusException( + "Terms lookup queries referencing external indices are not permitted in detector search", RestStatus.FORBIDDEN)); + return; + } + this.threadPool.getThreadContext().stashContext(); if (!detectorIndices.detectorIndexExists()) { actionListener.onResponse(getEmptySearchResponse()); @@ -113,4 +104,4 @@ private void setFilterByEnabled(boolean filterByEnabled) { this.filterByEnabled = filterByEnabled; } -} \ No newline at end of file +} diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchRuleAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchRuleAction.java index 4f5f4c4d1..04107c781 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchRuleAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportSearchRuleAction.java @@ -25,6 +25,7 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.index.reindex.BulkByScrollResponse; import org.opensearch.rest.RestStatus; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.internal.InternalSearchResponse; import org.opensearch.securityanalytics.action.SearchRuleAction; import org.opensearch.securityanalytics.action.SearchRuleRequest; @@ -88,6 +89,13 @@ class AsyncSearchRulesAction { } void start() { + 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 rule search", RestStatus.FORBIDDEN)); + return; + } + TransportSearchRuleAction.this.threadPool.getThreadContext().stashContext(); if (request.isPrepackaged()) { ruleIndices.initPrepackagedRulesIndex( @@ -243,4 +251,4 @@ private void finishHim(SearchResponse response, Exception t) { })); } } -} \ No newline at end of file +} diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportUpdateIndexMappingsAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportUpdateIndexMappingsAction.java index 3717e24b2..7924840ac 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportUpdateIndexMappingsAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportUpdateIndexMappingsAction.java @@ -6,12 +6,15 @@ import org.opensearch.OpenSearchStatusException; 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.client.Client; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; +import org.opensearch.index.IndexNotFoundException; import org.opensearch.rest.RestStatus; import org.opensearch.securityanalytics.action.UpdateIndexMappingsAction; import org.opensearch.securityanalytics.mapper.MapperService; @@ -22,6 +25,7 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.util.Map; public class TransportUpdateIndexMappingsAction extends HandledTransportAction { @@ -29,6 +33,7 @@ public class TransportUpdateIndexMappingsAction extends HandledTransportAction actionListener) { - this.threadPool.getThreadContext().stashContext(); - try { - IndexMetadata index = clusterService.state().metadata().index(request.getIndexName()); - if (index == null) { - actionListener.onFailure( - SecurityAnalyticsException.wrap( - new OpenSearchStatusException( - "Could not find index [" + request.getIndexName() + "]", RestStatus.NOT_FOUND + // 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(); + try { + IndexMetadata index = clusterService.state().metadata().index(request.getIndexName()); + if (index == null) { + actionListener.onFailure( + SecurityAnalyticsException.wrap( + new OpenSearchStatusException( + "Could not find index [" + request.getIndexName() + "]", RestStatus.NOT_FOUND + ) + ) + ); + return; + } + mapperService.updateMappingAction( + request.getIndexName(), + request.getAlias(), + buildAliasJson(request.getField()), + actionListener) + ; + } catch (IOException e) { + actionListener.onFailure(e); + } + }, + e -> { + if (isIndexNotFoundException(e)) { + actionListener.onFailure( + SecurityAnalyticsException.wrap( + new OpenSearchStatusException( + "Could not find index [" + request.getIndexName() + "]", RestStatus.NOT_FOUND + ) ) - ) - ); - return; + ); + } else { + actionListener.onFailure(e); + } + } + )); + } + + private boolean isIndexNotFoundException(Exception e) { + if (e instanceof IndexNotFoundException) { + return true; + } + Throwable cause = e.getCause(); + while (cause != null) { + if (cause instanceof IndexNotFoundException) { + return true; } - mapperService.updateMappingAction( - request.getIndexName(), - request.getAlias(), - buildAliasJson(request.getField()), - actionListener) - ; - } catch (IOException e) { - actionListener.onFailure(e); + cause = cause.getCause(); } + return false; } private String buildAliasJson(String fieldName) throws IOException { return "type=alias,path=" + fieldName; } -} \ No newline at end of file +} diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/SecureMappingsAndSearchRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/SecureMappingsAndSearchRestApiIT.java new file mode 100644 index 000000000..d05514547 --- /dev/null +++ b/src/test/java/org/opensearch/securityanalytics/resthandler/SecureMappingsAndSearchRestApiIT.java @@ -0,0 +1,273 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +package org.opensearch.securityanalytics.resthandler; + +import org.apache.http.HttpEntity; +import org.apache.http.HttpHost; +import org.apache.http.HttpStatus; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.junit.After; +import org.junit.Before; +import org.opensearch.client.Request; +import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; +import org.opensearch.client.RestClient; +import org.opensearch.common.settings.Settings; +import org.opensearch.commons.rest.SecureRestClientBuilder; +import org.opensearch.rest.RestStatus; +import org.opensearch.securityanalytics.SecurityAnalyticsPlugin; +import org.opensearch.securityanalytics.SecurityAnalyticsRestTestCase; + +import java.io.IOException; +import java.util.Collections; + +/** + * Tests that verify the authorization bypass fixes in the security-analytics plugin. + * These tests confirm that: + * 1. Users without index permissions cannot read/write arbitrary indices via plugin endpoints + * 2. Terms lookup queries referencing external indices are blocked + * 3. Normal operations for authorized users still work + */ +public class SecureMappingsAndSearchRestApiIT extends SecurityAnalyticsRestTestCase { + + static String SECURITY_ANALYTICS_READ_ACCESS_ROLE = "security_analytics_read_access"; + + private static final String SENSITIVE_INDEX = "hr-salaries-test"; + private static final String SENSITIVE_INDEX_MAPPING = + "\"properties\": {" + + " \"employee_name\": { \"type\": \"text\" }," + + " \"employee_national_id\": { \"type\": \"keyword\" }," + + " \"base_salary_usd\": { \"type\": \"integer\" }" + + "}"; + + private RestClient readOnlyClient; + private final String readOnlyUser = "analyst_read_only"; + + @Before + public void setup() throws IOException { + if (!securityEnabled()) return; + + createTestIndex(client(), SENSITIVE_INDEX, SENSITIVE_INDEX_MAPPING, Settings.EMPTY); + indexDoc(client(), SENSITIVE_INDEX, "1", + "{\"employee_name\": \"Alice\", \"employee_national_id\": \"123-45-6789\", \"base_salary_usd\": 150000}", + true); + + String[] backendRoles = {"ANALYST"}; + createUserWithData(readOnlyUser, readOnlyUser, SECURITY_ANALYTICS_READ_ACCESS_ROLE, backendRoles); + readOnlyClient = new SecureRestClientBuilder( + getClusterHosts().toArray(new HttpHost[]{}), isHttps(), readOnlyUser, readOnlyUser + ).setSocketTimeout(60000).build(); + } + + @After + public void cleanup() throws IOException { + if (!securityEnabled()) return; + if (readOnlyClient != null) readOnlyClient.close(); + deleteUser(readOnlyUser); + client().performRequest(new Request("DELETE", "/" + SENSITIVE_INDEX)); + } + + public void testMappingsViewRejectsUnauthorizedIndex() throws IOException { + if (!securityEnabled()) return; + + try { + readOnlyClient.performRequest(new Request("GET", + SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI + + "?index_name=" + SENSITIVE_INDEX + "&rule_topic=windows")); + fail("Expected 403 for unauthorized index access via mappings/view"); + } catch (ResponseException e) { + assertEquals(RestStatus.FORBIDDEN.getStatus(), e.getResponse().getStatusLine().getStatusCode()); + } + } + + public void testGetMappingsRejectsUnauthorizedIndex() throws IOException { + if (!securityEnabled()) return; + + try { + readOnlyClient.performRequest(new Request("GET", + SecurityAnalyticsPlugin.MAPPER_BASE_URI + "?index_name=" + SENSITIVE_INDEX)); + fail("Expected 403 for unauthorized index access via GET mappings"); + } catch (ResponseException e) { + assertEquals(RestStatus.FORBIDDEN.getStatus(), e.getResponse().getStatusLine().getStatusCode()); + } + } + + public void testDetectorSearchBlocksTermsLookup() throws IOException { + if (!securityEnabled()) return; + + String termsLookupQuery = "{" + + "\"size\": 0," + + "\"query\": {" + + " \"terms\": {" + + " \"detector.name\": {" + + " \"index\": \"" + SENSITIVE_INDEX + "\"," + + " \"id\": \"1\"," + + " \"path\": \"employee_national_id\"" + + " }" + + " }" + + "}" + + "}"; + + HttpEntity entity = new StringEntity(termsLookupQuery, ContentType.APPLICATION_JSON); + try { + makeRequest(readOnlyClient, "POST", + SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/_search", + Collections.emptyMap(), entity); + fail("Expected 403 for terms lookup referencing external index"); + } catch (ResponseException e) { + assertEquals(RestStatus.FORBIDDEN.getStatus(), e.getResponse().getStatusLine().getStatusCode()); + } + } + + public void testDetectorSearchAllowsInlineTerms() throws IOException { + if (!securityEnabled()) return; + + String inlineTermsQuery = "{" + + "\"size\": 0," + + "\"query\": {" + + " \"terms\": {" + + " \"detector.name\": [\"test-1\", \"test-2\"]" + + " }" + + "}" + + "}"; + + HttpEntity entity = new StringEntity(inlineTermsQuery, ContentType.APPLICATION_JSON); + Response response = makeRequest(readOnlyClient, "POST", + SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/_search", + Collections.emptyMap(), entity); + assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + } + + public void testDetectorSearchAllowsMatchAll() throws IOException { + if (!securityEnabled()) return; + + String query = "{\"size\": 0, \"query\": {\"match_all\": {}}}"; + HttpEntity entity = new StringEntity(query, ContentType.APPLICATION_JSON); + Response response = makeRequest(readOnlyClient, "POST", + SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/_search", + Collections.emptyMap(), entity); + assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + } + + public void testRuleSearchBlocksTermsLookup() throws IOException { + if (!securityEnabled()) return; + + String termsLookupQuery = "{" + + "\"size\": 0," + + "\"query\": {" + + " \"terms\": {" + + " \"rule.title\": {" + + " \"index\": \"" + SENSITIVE_INDEX + "\"," + + " \"id\": \"1\"," + + " \"path\": \"employee_national_id\"" + + " }" + + " }" + + "}" + + "}"; + + HttpEntity entity = new StringEntity(termsLookupQuery, ContentType.APPLICATION_JSON); + try { + makeRequest(readOnlyClient, "POST", + SecurityAnalyticsPlugin.RULE_BASE_URI + "/_search?pre_packaged=true", + Collections.emptyMap(), entity); + fail("Expected 403 for terms lookup in rule search"); + } catch (ResponseException e) { + assertEquals(RestStatus.FORBIDDEN.getStatus(), e.getResponse().getStatusLine().getStatusCode()); + } + } + + public void testCreateMappingsRejectsUnauthorizedIndex() throws IOException { + if (!securityEnabled()) return; + + // Create a custom role with only cluster permission, no index permissions + createCustomRole("mapping_cluster_only", + "cluster:admin/opensearch/securityanalytics/mapping/*"); + String customUser = "mapping_cluster_user"; + String[] backendRoles = {"ANALYST"}; + createUser(customUser, customUser, backendRoles); + createUserRolesMapping("mapping_cluster_only", new String[]{customUser}); + + RestClient customClient = new SecureRestClientBuilder( + getClusterHosts().toArray(new HttpHost[]{}), isHttps(), customUser, customUser + ).setSocketTimeout(60000).build(); + + try { + Request request = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI); + request.setJsonEntity("{\"index_name\": \"" + SENSITIVE_INDEX + "\", " + + "\"rule_topic\": \"windows\", \"partial\": true}"); + customClient.performRequest(request); + fail("Expected 403 for POST mappings without index permissions"); + } catch (ResponseException e) { + assertEquals(RestStatus.FORBIDDEN.getStatus(), e.getResponse().getStatusLine().getStatusCode()); + } finally { + customClient.close(); + deleteUser(customUser); + tryDeletingRole("mapping_cluster_only"); + } + } + + public void testUpdateMappingsRejectsUnauthorizedIndex() throws IOException { + if (!securityEnabled()) return; + + // First create mappings as admin + Request adminRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI); + adminRequest.setJsonEntity("{\"index_name\": \"" + SENSITIVE_INDEX + "\", " + + "\"rule_topic\": \"windows\", \"partial\": true}"); + client().performRequest(adminRequest); + + // Create a custom role with only cluster permission + createCustomRole("mapping_cluster_only2", + "cluster:admin/opensearch/securityanalytics/mapping/*"); + String customUser = "mapping_cluster_user2"; + String[] backendRoles = {"ANALYST"}; + createUser(customUser, customUser, backendRoles); + createUserRolesMapping("mapping_cluster_only2", new String[]{customUser}); + + RestClient customClient = new SecureRestClientBuilder( + getClusterHosts().toArray(new HttpHost[]{}), isHttps(), customUser, customUser + ).setSocketTimeout(60000).build(); + + try { + Request request = new Request("PUT", SecurityAnalyticsPlugin.MAPPER_BASE_URI); + request.setJsonEntity("{\"index_name\": \"" + SENSITIVE_INDEX + "\", " + + "\"field\": \"employee_name\", \"alias\": \"winlog.event_data.SubjectUserName\"}"); + customClient.performRequest(request); + fail("Expected 403 for PUT mappings without index permissions"); + } catch (ResponseException e) { + assertEquals(RestStatus.FORBIDDEN.getStatus(), e.getResponse().getStatusLine().getStatusCode()); + } finally { + customClient.close(); + deleteUser(customUser); + tryDeletingRole("mapping_cluster_only2"); + } + } + + public void testMappingsViewWorksForAuthorizedUser() throws IOException { + if (!securityEnabled()) return; + + // Admin should be able to use mappings/view + Request request = new Request("GET", + SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI + + "?index_name=" + SENSITIVE_INDEX + "&rule_topic=windows"); + Response response = client().performRequest(request); + assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + } + + public void testGetMappingsWorksForAuthorizedUser() throws IOException { + if (!securityEnabled()) return; + + Request request = new Request("GET", + SecurityAnalyticsPlugin.MAPPER_BASE_URI + "?index_name=" + SENSITIVE_INDEX); + Response response = client().performRequest(request); + assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + } + + private Response indexDoc(RestClient client, String index, String id, String doc, boolean refresh) throws IOException { + Request request = new Request("PUT", "/" + index + "/_doc/" + id + (refresh ? "?refresh=true" : "")); + request.setJsonEntity(doc); + return client.performRequest(request); + } +} diff --git a/src/test/java/org/opensearch/securityanalytics/transport/QueryUtilsTests.java b/src/test/java/org/opensearch/securityanalytics/transport/QueryUtilsTests.java new file mode 100644 index 000000000..3fcf46313 --- /dev/null +++ b/src/test/java/org/opensearch/securityanalytics/transport/QueryUtilsTests.java @@ -0,0 +1,153 @@ +/* + * 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.MatchAllQueryBuilder; +import org.opensearch.index.query.MatchQueryBuilder; +import org.opensearch.index.query.NestedQueryBuilder; +import org.opensearch.index.query.TermsQueryBuilder; +import org.opensearch.indices.TermsLookup; +import org.opensearch.test.OpenSearchTestCase; + +public class QueryUtilsTests extends OpenSearchTestCase { + + public void testNullQuery() { + assertFalse(QueryUtils.containsTermsLookup(null)); + } + + public void testMatchAllQuery() { + assertFalse(QueryUtils.containsTermsLookup(new MatchAllQueryBuilder())); + } + + public void testMatchQuery() { + assertFalse(QueryUtils.containsTermsLookup(new MatchQueryBuilder("field", "value"))); + } + + public void testInlineTermsQuery() { + TermsQueryBuilder terms = new TermsQueryBuilder("field", "val1", "val2"); + assertFalse(QueryUtils.containsTermsLookup(terms)); + } + + public void testTermsLookupQuery() { + TermsQueryBuilder terms = new TermsQueryBuilder("field", new TermsLookup("other-index", "doc1", "path")); + assertTrue(QueryUtils.containsTermsLookup(terms)); + } + + public void testBoolWithTermsLookupInMust() { + BoolQueryBuilder bool = new BoolQueryBuilder() + .must(new MatchAllQueryBuilder()) + .must(new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + assertTrue(QueryUtils.containsTermsLookup(bool)); + } + + public void testBoolWithTermsLookupInShould() { + BoolQueryBuilder bool = new BoolQueryBuilder() + .must(new MatchAllQueryBuilder()) + .should(new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + assertTrue(QueryUtils.containsTermsLookup(bool)); + } + + public void testBoolWithTermsLookupInFilter() { + BoolQueryBuilder bool = new BoolQueryBuilder() + .must(new MatchAllQueryBuilder()) + .filter(new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + assertTrue(QueryUtils.containsTermsLookup(bool)); + } + + public void testBoolWithTermsLookupInMustNot() { + BoolQueryBuilder bool = new BoolQueryBuilder() + .must(new MatchAllQueryBuilder()) + .mustNot(new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + assertTrue(QueryUtils.containsTermsLookup(bool)); + } + + public void testBoolWithoutTermsLookup() { + BoolQueryBuilder bool = new BoolQueryBuilder() + .must(new MatchAllQueryBuilder()) + .must(new TermsQueryBuilder("field", "val1", "val2")) + .filter(new MatchQueryBuilder("status", "active")); + assertFalse(QueryUtils.containsTermsLookup(bool)); + } + + public void testNestedBoolWithTermsLookup() { + BoolQueryBuilder inner = new BoolQueryBuilder() + .must(new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + BoolQueryBuilder outer = new BoolQueryBuilder() + .must(new MatchAllQueryBuilder()) + .filter(inner); + assertTrue(QueryUtils.containsTermsLookup(outer)); + } + + public void testConstantScoreWithTermsLookup() { + ConstantScoreQueryBuilder constantScore = new ConstantScoreQueryBuilder( + new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + assertTrue(QueryUtils.containsTermsLookup(constantScore)); + } + + public void testConstantScoreWithoutTermsLookup() { + ConstantScoreQueryBuilder constantScore = new ConstantScoreQueryBuilder( + new MatchAllQueryBuilder()); + assertFalse(QueryUtils.containsTermsLookup(constantScore)); + } + + public void testBoostingWithTermsLookupInPositive() { + BoostingQueryBuilder boosting = new BoostingQueryBuilder( + new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data")), + new MatchAllQueryBuilder() + ).negativeBoost(0.5f); + assertTrue(QueryUtils.containsTermsLookup(boosting)); + } + + public void testBoostingWithTermsLookupInNegative() { + BoostingQueryBuilder boosting = new BoostingQueryBuilder( + new MatchAllQueryBuilder(), + new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data")) + ).negativeBoost(0.5f); + assertTrue(QueryUtils.containsTermsLookup(boosting)); + } + + public void testDisMaxWithTermsLookup() { + DisMaxQueryBuilder disMax = new DisMaxQueryBuilder() + .add(new MatchAllQueryBuilder()) + .add(new TermsQueryBuilder("field", new TermsLookup("secret-index", "id1", "data"))); + assertTrue(QueryUtils.containsTermsLookup(disMax)); + } + + public void testDisMaxWithoutTermsLookup() { + DisMaxQueryBuilder disMax = new DisMaxQueryBuilder() + .add(new MatchAllQueryBuilder()) + .add(new MatchQueryBuilder("field", "value")); + assertFalse(QueryUtils.containsTermsLookup(disMax)); + } + + public void testNestedQueryWithTermsLookup() { + NestedQueryBuilder nested = new NestedQueryBuilder("path", + new TermsQueryBuilder("path.field", new TermsLookup("secret-index", "id1", "data")), + org.apache.lucene.search.join.ScoreMode.Avg); + assertTrue(QueryUtils.containsTermsLookup(nested)); + } + + public void testNestedQueryWithoutTermsLookup() { + NestedQueryBuilder nested = new NestedQueryBuilder("path", + new MatchQueryBuilder("path.field", "value"), + org.apache.lucene.search.join.ScoreMode.Avg); + assertFalse(QueryUtils.containsTermsLookup(nested)); + } + + public void testDeeplyNestedTermsLookup() { + TermsQueryBuilder termsLookup = new TermsQueryBuilder("field", + new TermsLookup("secret-index", "id1", "data")); + + BoolQueryBuilder level3 = new BoolQueryBuilder().must(termsLookup); + ConstantScoreQueryBuilder level2 = new ConstantScoreQueryBuilder(level3); + BoolQueryBuilder level1 = new BoolQueryBuilder().filter(level2); + + assertTrue(QueryUtils.containsTermsLookup(level1)); + } +}