Add a flag to set whether a system index is readable on SystemIndexDescriptor

[cwperks]

Description

I'm opening up this PR to start a discussion on this approach to a problem that I am seeing while trying to change over plugins to use the replacement for ThreadContext.stashContext for System index access.

This PR introduces the notion of a readable system index. readable means that GET and SEARCH operations would be permitted on the index, but write operations would be reserved for the system.

This PR is a start to some much needed code hygiene cleanup to start removing the this request accesses system indices: [{}], but in a future major version, direct access to system indices will be prevented by default which have been present since the fork.

From the core's perspective, the only behavior this PR will change is that the message above will not be logged for search operations that cover readable system indices since that would be officially supported.

When the security plugin is installed, there would be a more noticeable difference in behavior between readable system indices and non-readable ones.

• non-readable system indices - This is the current behavior where security scrubs the results of GET and SEARCH operations on these indices. The reason for this is that the security index can contain secrets and this logic is used to scrub any results
• readable system index - This would be new behavior that would be officially supported where a plugin dev can specify whether a system index is readable when defining system indices using SystemIndexPlugin.getSystemIndexDescriptors. A couple example use-cases:

1. The security auditlog when using an internal opensearch index. This is an index that the security plugin writes to, but users should be permitted to query it
2. Anomaly Detection has a notion of a custom results index where they store results information and users use these indices to make dashboards

This PR is an alternative to #15778

Related Issues

Resolves opensearch-project/security#2487

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.

[cwperks]

@reta I wanted to run this idea by you. Opening this in draft because I haven't written tests yet, but would like to know what you think about this approach as an alternative to #15778

[github-actions]

✅ Gradle check result for 777fe07: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 74.28571% with 9 lines in your changes missing coverage. Please review.

Project coverage is 72.40%. Comparing base (77e4112) to head (00ef4ba).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
.../org/opensearch/indices/SystemIndexDescriptor.java	61.53%	3 Missing and 2 partials ⚠️
.../cluster/metadata/IndexNameExpressionResolver.java	83.33%	2 Missing and 1 partial ⚠️
.../example/resthandler/ExampleRestHandlerPlugin.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #17296      +/-   ##
============================================
- Coverage     72.40%   72.40%   -0.01%     
- Complexity    65554    65583      +29     
============================================
  Files          5292     5292              
  Lines        304493   304548      +55     
  Branches      44218    44229      +11     
============================================
+ Hits         220463   220497      +34     
- Misses        65975    66004      +29     
+ Partials      18055    18047       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

❌ Gradle check result for 9a54a4e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 40f2afc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 00ef4ba: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❕ Gradle check result for 00ef4ba: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[reta]

@reta I wanted to run this idea by you. Opening this in draft because I haven't written tests yet, but would like to know what you think about this approach as an alternative to #15778

Thanks @cwperks , I am not onboard with yet another flag:

• it causes conflicts with Context::isSystemIndexAccessAllowed()
• it does not rely on the permissions model that security plugin has

I think we have touched on this in the past, but we have an opportunity with 3.x release to make hard, breaking decisions regarding sealing system indices, for example:

• forbid all access to system indices through regular indices APIs
• introduce dedicated APIs to access system indices for read / write / ... operation

This will give us a clear security boundaries, introducing additional flags is (to me) not solving the root of the problem. I hope it makes sense, thank you

[cwperks]

I'm a little confused because #15778 was a general attempt to define a new way to supply a list of actions and index patterns that a plugin can perform using its identity - that would be the most general approach to sealing what plugins can perform inside a pluginSubject.runAs(() -> { ... }) block that conforms to the security model.

This will give us a clear security boundaries, introducing additional flags is (to me) not solving the root of the problem.

The new flag would be a flag that the security plugin can use to differentiate system indexes that it should scrub the results from vs. allowing search and get on the index. (This is where the scrubbing logic currently exists in the security plugin).

Introducing this flag would also support all existing plugin use-cases in the default distribution, but is not as generalizable as #15778.

[reta]

Introducing this flag would also support all existing plugin use-cases in the default distribution, but is not as generalizable as #15778.

The #15778 and dedicated API to access system indices are not exclusive - those are complementary. I think making the system indices access explicit through APIs, separate from all other indices and not flag(s) driven, would lead to clear and maintainable solution.

[cwperks]

Maybe I'm just lacking in imagination, but I'm having trouble picture what those APIs may look like.

[dbwiddis]

Maybe I'm just lacking in imagination, but I'm having trouble picture what those APIs may look like.

When I lack in imagination I ask my favorite LLM.

Question: I currently have a bunch of APIs to access indices, like "PUT foo", "GET foo", and "DELETE foo". Some indices are special (they are called System Indices) and I want them to have a different API so that I can take more actions at the REST layer, such as blocking at a gateway, prior to processing them internally. Can you suggest some alternate APIs?

It suggested:

• PUT /system-indices/create/{index_name}
• GET /system-indices/{index_name}
• DELETE /system-indices/{index_name}

[cwperks]

The mental model I have for plugins is similar to that of out-of-process extensions. In the security model for out-of-process extensions, an extension could be assigned a service account token that allows an extension to make privileged requests to OpenSearch, such as storing metadata in reserved (system) indices. I use reserved in this to indicate that an extension reserves index patterns that can be given special protections like system indices that prevent regular users from accessing the indices. By default, extensions can perform any action on their own reserved indices.

Service account tokens were also designed in a way that they could be assigned permissions directly to allow an extension to make REST Requests against a cluster "as a daemon". i.e. an enumerated list of actions an extension is allowed to perform with its identity outside of the context of any authenticated user.

In the current plugin architecture, plugins can perform any action without authz checks. With extensions, that security model was deemed too risky assuming that extensions are 3P. I'd like to introduce a similar notion where its enumerated ahead of time what a plugin will perform and present that to a cluster admin on install. It both let's us create better SDKs for plugin devs with clear guidance on how the security model works w/o having to call low level ThreadContext APIs and improves security.

[reta]

Maybe I'm just lacking in imagination, but I'm having trouble picture what those APIs may look like.

Let's start from the basic blocks we have now: we have an API for plugins to contribute system indices: SystemIndexPlugin / SystemIndexDescriptor / SystemIndexRegistry. Do we have an API for plugins to access system indices? yes and no. The system indices are not first class citizen - although we do have an API to specifically ask for system index creation, we don't have any API to access them, it is all falls down to regular index APIs. Could we separate access to normal and system indices? Fe instead of dealing with tons of flags, could we only allow accessing system indices through SystemIndicesService fe?

[cwperks]

Fe instead of dealing with tons of flags

This may be yet another flag in the IndexNameExpressionResolver, but from a system index perspective its a single flag. The system is readable or unreadable. Unreadable means that the deprecation log is still output on read requests (GET + SEARCH) or that results are scrubbed completely in a cluster running with security.

[cwperks]

For what its worth, I think this introduces a notion that should exist in OpenSearch. An index that's write protected, but readable. The security auditlog using an internal OpenSearch index is a good example of an index that ought to have write protections to only allow writes to it from the system, but allow reads from regular users authorized to read the index.