-
Notifications
You must be signed in to change notification settings - Fork 63
Description
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:
- 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.