[Sandbox] Add local index authorization check to analytics engine before planning phase

[finnegancarroll]

Description

The analytics engine's query execution path (PPL, SQL) bypasses the security plugin's index-level privilege evaluation. The FragmentExecutionRequest on the data node is executed with dispatchFragmentStreaming and never evaluated for index permissions by the security plugin SecurityFilter (wraps the ActionFilter).

SQL / PPL queries with analytics engine

In a typical SQL / PPL query authorization is handled at the node-to-node transport layer on the coordinator as the request is being dispatched to shards. The security plugin provides an ActionFilter which every TransportAction child holds a reference to, and executes on new requests.

The overall flow for a regular DSL search query is then:

NodeClient.execute(SearchAction.INSTANCE, searchRequest)
    → NodeClient.executeLocally(action, request, listener)
      → transportAction(action).execute(request, listener)
        → TransportAction.execute(task, request, listener)
          → RequestFilterChain.proceed()

The path for an analytics engine SQL / PPL query avoids the RequestFilterChain entirely. The ShardTaskRunner is responsible for dispatching a FragmentExecutionRequest on the transport layer.

    @Override
    public void run(ShardStageTask task, ActionListener<Void> listener) {
        ShardExecutionTarget target = (ShardExecutionTarget) task.target();
        FragmentExecutionRequest request = requestBuilder.apply(target);
        PendingExecutions pending = pendingFor(target);
        transport.dispatchFragmentStreaming(request, target.node(), stage.responseListenerFor(listener), config.parentTask(), pending);
    }

The FragmentExecutionRequest fetches a new connection from the transport service, and directly executes the request itself. Skipping the TransportAction framework which typically holds and applies the ActionFilter.

        pending.tryRun(() -> {
            try {
                Transport.Connection connection = getConnection(null, targetNode.getId());
                transportService.sendChildRequest(connection, FragmentExecutionAction.NAME, request, parentTask, options, handler);
            } catch (Exception e) {
                try {
                    listener.onFailure(e);
                } finally {
                    pending.finishAndRunNext();
                }
            }
        });

DSL queries with analytics engine

DSL permissions are actually evaluated correctly already. This comes from how DSL is routed into the analytics plugin. DSL queries construct a regular SearchRequest which is handled by the TransportSearchAction and are executed on the transport layer with the local node client as usual.

Handling of DSL queries is done by a new SearchActionFilter which is also injected into the RequestFilterChain and redirects these SearchRequests to the DslExecuteAction.INSTANCE handler instead of executing them as normal search requests. Since this SearchActionFilter executes AFTER the security plugin ActionFilter (which has highest priority), privileges are evaluated correctly.

Path	Unauthorized Index	Status
DSL (POST /{index}/_search)	✅ Denied	Secure
PPL (POST /_plugins/_ppl)	❌ Allowed	Gap
SQL (POST /_plugins/_sql)	❌ Allowed	Gap

Proposed fix

These changes somewhat mimic the functionality of DSL queries for SQL and PPL by adding a pre-planning authorization check which passes a no-op AnalyticsAuthRequest through the ActionFilter to determine if permissions are valid for the equivalent set of FragmentExecutionRequests.

No actual request is executed on the transport layer, the AnalyticsAuthRequest is passed through the full ActionFilter chain to invoke security plugin privilege evaluation, and proceeds with the analytics engine planning + query only if this probing AnalyticsAuthRequest is authorized for the same indices.

Testing

Testing this feature requires an analytics engine enabled OpenSearch node, with SQL plugin installed, and security plugin installed. Currently SQL plugin supports this environment with distribution level security enabled tests. This seems like the most ideal place to house ITs validating index level security over the analytics plugin.

Test implementation is pending. Currently changes are validated with local single node testing.

Related Issues

N/A

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit ae21cbe.

Path	Line	Severity	Description
sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/DefaultPlanExecutor.java	122	medium	Authorization is silently skipped when extractIndices returns an empty array. Any logical plan that produces no TableScan nodes (e.g., constant-folded queries, function-only plans, or plan shapes Calcite optimizes away from TableScan) bypasses the index-level auth check entirely and proceeds directly to doExecuteOnSearchPool.
sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/DefaultPlanExecutor.java	155	medium	collectIndices only inspects org.apache.calcite.rel.core.TableScan nodes. Calcite plans can reference data through other node types (e.g., JdbcTableScan, custom RelNode subclasses, or views resolved to non-TableScan nodes). Indices accessed via those paths are invisible to the auth check, so a crafted plan could touch unauthorized indices without triggering the AnalyticsAuthRequest.
sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/action/TransportAnalyticsAuthAction.java	33	low	The no-op doExecute unconditionally calls listener.onResponse(new AnalyticsAuthResponse()), granting authorization to any request that reaches it. The security model depends entirely on an external action-filter chain (e.g., OpenSearch Security plugin) intercepting the request before doExecute is reached. If the security plugin is absent or misconfigured, all authorization checks pass silently.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 0 | Medium: 2 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[Bukhtawar]

I was wondering if we add a transport interceptor that can also validate if the request was indeed authorised.

public class AnalyticsAuthInterceptor implements TransportInterceptor {
      @Override
      public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(
          String action, ..., TransportRequestHandler<T> actualHandler, ...) {

          if (action.equals(FragmentExecutionAction.NAME)) {
              return (request, channel, task) -> {
                  String token = threadContext.getHeader("auth_token");
                  if (token == null || !verify(token)) {
                      throw new SecurityException("Analytics auth not performed");
                  }
                  actualHandler.messageReceived(request, channel, task);
              };
          }
          return actualHandler;
      }
  }

[finnegancarroll]

I was wondering if we add a transport interceptor that can also validate if the request was indeed authorised.

Thanks for taking a look @Bukhtawar, I spent a small bit of time trying to implement this and it seems like we would need to work this authorization token down to the thread context from the QueryPlanExecutor with some additional plumbing.

I can look at implementing this but i'm wondering if we are guarding against anything additional here since the QueryPlanExecutor is the single entry point for analytics engine.

[cwperks]

@finnegancarroll another problem to think about with regards to security is how to extract indices from a request pertinent to indices.

There currently are 2 ways:

1. Inside the IndexResolverReplacer of the security plugin
2. The modern way is to implement TransportIndicesResolvingAction so that security puts that onus on the transport action to be able to resolve the indices.

[cwperks]

The FragmentExecutionRequest on the data node is executed with dispatchFragmentStreaming and never evaluated for index permissions by the security plugin SecurityFilter (wraps the ActionFilter).

Agreed on putting the call to dispatchFragmentStreaming within a transport action. @finnegancarroll should we get the security plugin in a state where we can test with the sandbox from core?

[cwperks]

Quickly came up with cwperks/security#97 which adds test that would be expected to fail until a fix like this lands.

[cwperks]

@finnegancarroll I'm also seeing some action names that I think we should rename: cwperks#362

^ I haven't been tracking sandbox too closely so lmk what you think and if there are other actions similar.

[expani]

Thanks for getting this started @finnegancarroll

[expani]

RelNodeUtils#findNode(RelNode node, Class<T> type)

We need to write a variation of this method that also takes a parameter of depth as a guard and returns empty if Scan is not found.

The depth parameter can be HardCoded to 10/15 as a constant.

[expani]

If we are extracting indices here, it means Index patterns will also be resolved here.

So, to avoid duplication the OpenSearchTableScanRule needs to be able to read the indices extracted here. Maybe via the PlannerContext or some other common place.

[finnegancarroll]

I think PrivilegesEvaluator in security plugin holds onto a IndexNameExpressionResolver and should handle this for us. So we should only need to provide the literal index names here.

[DarshitChanpura]

For my understanding, why are we implementing security check outside Security plugin? we are trying to move away from that model, by making security aware of the related actions.

Adding a transport action should allow using the authz layer of security plugin. (I didn't read through the conversations, maybe this is already discuseed)

[finnegancarroll]

For my understanding, why are we implementing security check outside Security plugin? we are trying to move away from that model, by making security aware of the related actions.

Adding a transport action should allow using the authz layer of security plugin. (I didn't read through the conversations, maybe this is already discuseed)

+1

Initially I thought there was some design limitation here but reading through the flow for FragmentExecutionRequest it's not clear what stops us from executing these normally as transport actions. Giving this refactor a try.

[finnegancarroll]

Agreed on putting the call to dispatchFragmentStreaming within a transport action. @finnegancarroll should we get the security plugin in a state where we can test with the sandbox from core?

Copying offline discussion here. One issue testing security for this feature is we need all three of SQL plugin, Security plugin, OpenSearch core. Just the security plugin and opensearch core alone do not provide us a front end to test analytics engine with, and using the test-ppl-frontend is insufficient because it does not integrate with security plugin authentication handlers.

[finnegancarroll]

Hi @DarshitChanpura @finnegancarroll @expani @Bukhtawar .
Already discussed with a couple of you offline but migrating to a cleaner approach where DefaultPlanExecutor executes an analytics engine query locally through a transport action.

Please let me know your thoughts. Adding integration tests to SQL plugin.

#21789