Restrict the index authorization checks in PIT search

Search requests executed against a PIT do not have to set the target indices explicitly. Instead the list of targeted index is extracted from the PIT id directly.
When executed against the search rest layer, the indices of the PIT are copied in the search request in order to fullfill the authorization check when security is enabled.
However, this crucial copy operation is omitted when the search request is internally executed from a node within the cluster.
Consequently, the authorization check ends up evaluating the request against all indices accessible to the user.
To address this performance issue, the proposed change ensures this copy occurs explicitly within the security layer.
This modification restricts the authorization check to indices that are part of the PIT, mitigating the performance bug.
It is essential to note that even in the presence of this bug, the request does not route to all indices; only those present in the PIT are queried.
Furthermore, this issue is not a security vulnerability, as the authorization check continues to limit the list of accessible indices to those permitted for the user.

Closes elastic#99684

server/src/main/java/org/elasticsearch/action/search/SearchContextId.java

@@ -110,6 +110,23 @@ public static SearchContextId decode(NamedWriteableRegistry namedWriteableRegist
         }
     }
 
+    public static String[] decodeIndices(String id) {
+        try (
+            var decodedInputStream = Base64.getUrlDecoder().wrap(new ByteArrayInputStream(id.getBytes(StandardCharsets.ISO_8859_1)));
+            var in = new InputStreamStreamInput(decodedInputStream)
+        ) {
+            final TransportVersion version = TransportVersion.readVersion(in);
+            in.setTransportVersion(version);
+            final Map<ShardId, SearchContextIdForNode> shards = Collections.unmodifiableMap(
+                in.readCollection(Maps::newHashMapWithExpectedSize, SearchContextId::readShardsMapEntry)
+            );
+            return new SearchContextId(shards, Collections.emptyMap()).getActualIndices();
+        } catch (IOException e) {
+            assert false : e;
+            throw new IllegalArgumentException(e);
+        }
+    }
+
     private static void readShardsMapEntry(StreamInput in, Map<ShardId, SearchContextIdForNode> shards) throws IOException {
         shards.put(new ShardId(in), new SearchContextIdForNode(in));
     }

server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java

@@ -142,7 +142,7 @@ public static MultiSearchRequest parseRequest(
             searchRequest.source(new SearchSourceBuilder().parseXContent(parser, false, searchUsageHolder));
             RestSearchAction.validateSearchRequest(restRequest, searchRequest);
             if (searchRequest.pointInTimeBuilder() != null) {
-                RestSearchAction.preparePointInTime(searchRequest, restRequest, namedWriteableRegistry);
+                RestSearchAction.preparePointInTime(searchRequest, restRequest);
             } else {
                 searchRequest.setCcsMinimizeRoundtrips(
                     restRequest.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())

server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

@@ -220,7 +220,7 @@ public static void parseSearchRequest(
         validateSearchRequest(request, searchRequest);
 
         if (searchRequest.pointInTimeBuilder() != null) {
-            preparePointInTime(searchRequest, request, namedWriteableRegistry);
+            preparePointInTime(searchRequest, request);
         } else {
             searchRequest.setCcsMinimizeRoundtrips(
                 request.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())
@@ -373,7 +373,7 @@ static SuggestBuilder parseSuggestUrlParameters(RestRequest request) {
         return null;
     }
 
-    static void preparePointInTime(SearchRequest request, RestRequest restRequest, NamedWriteableRegistry namedWriteableRegistry) {
+    static void preparePointInTime(SearchRequest request, RestRequest restRequest) {
         assert request.pointInTimeBuilder() != null;
         ActionRequestValidationException validationException = null;
         if (request.indices().length > 0) {
@@ -409,8 +409,8 @@ static void preparePointInTime(SearchRequest request, RestRequest restRequest, N
             indicesOptions.ignoreThrottled()
         );
         request.indicesOptions(stricterIndicesOptions);
-        final SearchContextId searchContextId = request.pointInTimeBuilder().getSearchContextId(namedWriteableRegistry);
-        request.indices(searchContextId.getActualIndices());
+        var indices = SearchContextId.decodeIndices(request.pointInTimeBuilder().getEncodedId());
+        request.indices(indices);
     }
 
     /**

server/src/test/java/org/elasticsearch/action/search/SearchContextIdTests.java

@@ -85,5 +85,11 @@ public void testEncode() {
         assertThat(node3.getNode(), equalTo("node_3"));
         assertThat(node3.getSearchContextId().getId(), equalTo(42L));
         assertThat(node3.getSearchContextId().getSessionId(), equalTo("c"));
+
+        final String[] indices = SearchContextId.decodeIndices(id);
+        assertThat(indices.length, equalTo(3));
+        assertThat(indices[0], equalTo("cluster_x:idx"));
+        assertThat(indices[1], equalTo("cluster_y:idy"));
+        assertThat(indices[2], equalTo("idy"));
     }
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

@@ -25,6 +25,8 @@
 import org.elasticsearch.action.datastreams.MigrateToDataStreamAction;
 import org.elasticsearch.action.delete.DeleteAction;
 import org.elasticsearch.action.index.IndexAction;
+import org.elasticsearch.action.search.SearchContextId;
+import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.support.GroupedActionListener;
 import org.elasticsearch.action.support.replication.TransportReplicationAction.ConcreteShardRequest;
 import org.elasticsearch.action.update.UpdateAction;
@@ -84,6 +86,7 @@
 import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
 import org.elasticsearch.xpack.security.operator.OperatorPrivileges.OperatorPrivilegesService;
 
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -469,6 +472,26 @@ private void authorizeAction(
                 l.onResponse(result);
             }));
         } else if (isIndexAction(action)) {
+            if (request instanceof SearchRequest searchRequest && searchRequest.pointInTimeBuilder() != null) {
+                /**
+                 * We override the {@link SearchRequest#indices} with the PIT's indices for the authorization check.
+                 * Although this copy also occurs in the rest layer, we cannot rely on it, as the action might be executed
+                 * directly in the transport client.
+                 * For backward compatibility reasons, we refrain from failing the request if {@link SearchRequest#indices}
+                 * is non-empty. Instead, we verify that the indices match the PIT definition and deny access if they don't.
+                 */
+                var indices = SearchContextId.decodeIndices(searchRequest.pointInTimeBuilder().getEncodedId());
+                if (searchRequest.indices().length > 0 && Arrays.equals(searchRequest.indices(), indices) == false) {
+                    IllegalArgumentException cause = new IllegalArgumentException(
+                        "indices defined in the [pit] don't match the request's indices"
+                    );
+                    auditTrail.accessDenied(requestId, authentication, action, request, authzInfo);
+                    listener.onFailure(actionDenied(authentication, authzInfo, action, request, cause));
+                    return;
+                }
+                searchRequest.indices(indices);
+            }
+            logger.info("request resolved=" + request.getClass().getSimpleName());
             final Metadata metadata = clusterService.state().metadata();
             final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(resolvedIndicesListener -> {
                 final ResolvedIndices resolvedIndices = IndicesAndAliasesResolver.tryResolveWithoutWildcards(action, request);

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

@@ -7,6 +7,7 @@
 package org.elasticsearch.xpack.security.authz;
 
 import org.elasticsearch.ElasticsearchSecurityException;
+import org.elasticsearch.TransportVersion;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.CompositeIndicesRequest;
 import org.elasticsearch.action.DocWriteRequest;
@@ -61,6 +62,7 @@
 import org.elasticsearch.action.search.MultiSearchRequest;
 import org.elasticsearch.action.search.OpenPointInTimeRequest;
 import org.elasticsearch.action.search.ParsedScrollId;
+import org.elasticsearch.action.search.SearchContextId;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchScrollRequest;
 import org.elasticsearch.action.search.SearchTransportService;
@@ -101,6 +103,7 @@
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.bulk.stats.BulkOperationListener;
@@ -110,7 +113,12 @@
 import org.elasticsearch.license.MockLicenseState;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.search.SearchPhaseResult;
+import org.elasticsearch.search.SearchShardTarget;
+import org.elasticsearch.search.builder.PointInTimeBuilder;
+import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.internal.AliasFilter;
+import org.elasticsearch.search.internal.ShardSearchContextId;
 import org.elasticsearch.search.internal.ShardSearchRequest;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.ThreadPool;
@@ -1229,6 +1237,82 @@ public void testSearchAgainstIndex() {
         verifyNoMoreInteractions(auditTrail);
     }
 
+    public void testSearchPITAgainstIndex() {
+        RoleDescriptor role = new RoleDescriptor(
+            "search_index",
+            null,
+            new IndicesPrivileges[] { IndicesPrivileges.builder().indices("index-*").privileges("read").build() },
+            null
+        );
+        roleMap.put(role.getName(), role);
+        final Authentication authentication = createAuthentication(new User("test search user", role.getName()));
+
+        final String requestId = AuditUtil.getOrGenerateRequestId(threadContext);
+        final String indexName = "index-" + randomAlphaOfLengthBetween(1, 5);
+
+        final ClusterState clusterState = mockMetadataWithIndex(indexName);
+        final IndexMetadata indexMetadata = clusterState.metadata().index(indexName);
+
+        PointInTimeBuilder pit = new PointInTimeBuilder(createEncodedPIT(indexMetadata.getIndex()));
+        SearchRequest searchRequest = new SearchRequest().source(new SearchSourceBuilder().pointInTimeBuilder(pit))
+            .allowPartialSearchResults(false);
+        final ShardSearchRequest shardRequest = new ShardSearchRequest(
+            new OriginalIndices(new String[] { indexName }, searchRequest.indicesOptions()),
+            searchRequest,
+            new ShardId(indexMetadata.getIndex(), 0),
+            0,
+            1,
+            AliasFilter.EMPTY,
+            1.0f,
+            System.currentTimeMillis(),
+            null
+        );
+        this.setFakeOriginatingAction = false;
+        authorize(authentication, TransportSearchAction.TYPE.name(), searchRequest, true, () -> {
+            verify(rolesStore).getRoles(Mockito.same(authentication), Mockito.any());
+            IndicesAccessControl iac = threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
+            // Successful search action authorization should set a parent authorization header.
+            assertThat(securityContext.getParentAuthorization().action(), equalTo(TransportSearchAction.TYPE.name()));
+            // Within the action handler, execute a child action (the query phase of search)
+            authorize(authentication, SearchTransportService.QUERY_ACTION_NAME, shardRequest, false, () -> {
+                // This child action triggers a second interaction with the role store (which is cached)
+                verify(rolesStore, times(2)).getRoles(Mockito.same(authentication), Mockito.any());
+                // But it does not create a new IndicesAccessControl
+                assertThat(threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY), sameInstance(iac));
+                // The parent authorization header should only be present for direct child actions
+                // and not be carried over for a child of a child actions.
+                // Meaning, only query phase action should be pre-authorized in this case and potential sub-actions should not.
+                assertThat(securityContext.getParentAuthorization(), nullValue());
+            });
+        });
+        assertArrayEquals(new String[] { indexName }, searchRequest.indices());
+        verify(auditTrail).accessGranted(
+            eq(requestId),
+            eq(authentication),
+            eq(TransportSearchAction.TYPE.name()),
+            eq(searchRequest),
+            authzInfoRoles(new String[] { role.getName() })
+        );
+        verify(auditTrail).accessGranted(
+            eq(requestId),
+            eq(authentication),
+            eq(SearchTransportService.QUERY_ACTION_NAME),
+            eq(shardRequest),
+            authzInfoRoles(new String[] { role.getName() })
+        );
+        verifyNoMoreInteractions(auditTrail);
+
+        // verify invalid search request that tries to override the pit indices
+        SearchRequest invalidSearchRequest = new SearchRequest("another_index").source(new SearchSourceBuilder().pointInTimeBuilder(pit))
+            .allowPartialSearchResults(false);
+        Exception exc = expectThrows(
+            Exception.class,
+            () -> authorize(authentication, TransportSearchAction.TYPE.name(), invalidSearchRequest)
+        );
+        assertThat(exc.getCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(exc.getCause().getMessage(), containsString("indices defined in the [pit] don't match the request's indices"));
+    }
+
     public void testScrollRelatedRequestsAllowed() {
         RoleDescriptor role = new RoleDescriptor(
             "a_all",
@@ -3524,6 +3608,26 @@ static AuthorizationInfo authzInfoRoles(String[] expectedRoles) {
         return ArgumentMatchers.argThat(new RBACAuthorizationInfoRoleMatcher(expectedRoles));
     }
 
+    private static class TestSearchPhaseResult extends SearchPhaseResult {
+        final DiscoveryNode node;
+
+        TestSearchPhaseResult(ShardSearchContextId contextId, DiscoveryNode node) {
+            this.contextId = contextId;
+            this.node = node;
+        }
+    }
+
+    private static String createEncodedPIT(Index index) {
+        DiscoveryNode node1 = DiscoveryNodeUtils.create("node_1");
+        TestSearchPhaseResult testSearchPhaseResult1 = new TestSearchPhaseResult(new ShardSearchContextId("a", 1), node1);
+        testSearchPhaseResult1.setSearchShardTarget(
+            new SearchShardTarget("node_1", new ShardId(index.getName(), index.getUUID(), 0), null)
+        );
+        List<SearchPhaseResult> results = new ArrayList<>();
+        results.add(testSearchPhaseResult1);
+        return SearchContextId.encode(results, Collections.emptyMap(), TransportVersion.current());
+    }
+
     private static class RBACAuthorizationInfoRoleMatcher implements ArgumentMatcher<AuthorizationInfo> {
 
         private final String[] wanted;