[META] Remove usages of ThreadContext.stashContext and adopt new mechanism for System Index access

[cwperks]

What is happening?

The OpenSearch Security plugin is working on an effort to strengthen system index protection in the plugin ecosystem. As part of this effort, the Security plugin will start to enforce strong ownership for plugins and their associated System Indices that they formally register through the SystemIndexPlugin.getSystemIndexDescriptors() extension point. See related RFC in the Security plugin for details: opensearch-project/security#4439

Quick note on terminology: For practical purposes, stashing the ThreadContext is used in the plugin ecosystem to "switch" out of the authenticated user context and into the system context.

Stashing, in general, means saving for later and that is what ThreadContext.stashContext does. It stashes the previous context for use at a later time.

Is my plugin affected?

If your plugin has usages of ThreadContext.stashContext() then you will need to adopt these changes.

How do I adopt the changes?

This PR in core is introducing a new extension point called IdentityAwarePlugin which introduces a method called assignSubject(PluginSubject pluginSubject).
The subject that is provided in assignSubject can be used when performing transport actions that interact with any system index that your plugin registers and uses. pluginSubject.runAs(Callable<T> callable) is a direct replacement for try (ThreadContext.StoredContext ctx = threadContext.stashContext) { ... }.

Refactor your plugin to remove calls to ThreadContext.stashContext() and perform any transport actions that access your plugins' system indices within pluginSubject.runAs(() -> { ...code here... })

<TODO> Provide example Security PR

Will this be a breaking change?

There will be no breaking change on the 2.x line. In 3.0.0, some methods in the ThreadContext class will have to be granted explicit permission in order for a plugin to utilize the methods. Usage will be guarded by JSM Permissions. (See this PR for context).

While it would be possible to add an entry into the plugin-security.policy file and surround calls with AccessController.doPrivileged(() -> ...), its not advisable. All plugins that utilize ThreadContext.stashContext should adopt these changes for a more secure ecosystem of plugins.

Can I perform automated testing with the security plugin to ensure plugin behavior is working as expected?

Yes and this is a good practice to do as an CI check that runs on each pull request and during releases. There are a few examples of plugins that have setup Github actions to perform integTests in a cluster with the security plugin installed.

Some plugins that have workflows that test with security include:

• SQL: https://github.com/opensearch-project/sql/blob/main/.github/workflows/integ-tests-with-security.yml
• Anomaly Detection: https://github.com/opensearch-project/anomaly-detection/blob/main/.github/workflows/test_security.yml
• ISM: https://github.com/opensearch-project/index-management/blob/main/.github/workflows/security-test-workflow.yml

Additional considerations

This change will greatly restrict what transport actions plugins can perform. Currently, when a plugin stashes the ThreadContext, they operate in a privileged block where they can perform any Transport Action. After this change, plugins will be limited in the transport actions they can perform. This change will restrict actions to only transport actions involving your particular plugin's system indices that were registered via SystemIndexPlugin.getSystemIndexDescriptors. All other actions will be blocked.

Please comment on this issue if there is a use-case where a plugin needs to perform other sorts of transport actions in a block where the ThreadContext has been stashed. Other sorts of transport actions meaning transport actions that do not pertain to system indices.

So far I have found a single example in the Security plugin:

1. In the Security plugin its possible to specify internal_opensearch as the audit log meaning that Security will store the audit log in an OpenSearch index. In this particular case, the audit log is a regular (not system) index yet Security still needs to be able to write to the index regardless of the authenticated user's permissions. Security needs to stash the context before writing to this index to ensure that the write operation succeeds.

[cwperks]

For some additional technical detail, the PluginSubject that is assigned in IdentityAwarePlugin.assignSubject(PluginSubject pluginSubject) is provided by the IdentityPlugin through the getPluginSubject extension point (practically speaking, the security plugin is the IdentityPlugin). Without an IdentityPlugin installed, it will provide a NoopPluginSubject which simply stashes the threadContext.

With the implementation provided by the Security plugin, it will inject a special pluginUser into the new context to enforce authz checks for the pluginUser.

This user will be restricted to the transport actions they can execute and will only be permitted to perform transport actions that interact with the plugin's registered system indices.

[sohami]

Please comment on this issue if there is a use-case where a plugin needs to perform other sorts of transport actions in a block where the ThreadContext has been stashed. Other sorts of transport actions meaning transport actions that do not pertain to system indices.

Another use case I can think of is if some plugins have "internal" actions which are used to gather some stats but not exposed as public API to the users. These internal actions can be invoked to get those stats on the path of the public API call for some validations, etc and will need mechanism to be able to call these. When I say internal I mean some cloud provider having internal plugins which expose few of the actions that are not available to the end user defining the roles for the end users. So having a way to specify these actions which plugins needs access to, could also be useful.