-
Notifications
You must be signed in to change notification settings - Fork 358
[RFC] Strengthen System Index Protection in the Plugin Ecosystem #4439
Description
Summary
Today, when a plugin stashes the ThreadContext the plugin is in a "trusted" mode where it can perform any action on a cluster (analogous to sudo). Conventionally, stashing the ThreadContext is used when handling API requests in order for a plugin to read or write to its system indices. The plugin ecosystem relies on good intentions to ensure that plugins do not act rogue and perform malicious actions that threaten the integrity of the system (such as deleting another plugin's system index). There should be a better mechanism for controlling what plugins can do after stashing the ThreadContext, perhaps even limiting the actions that a plugin can perform to only index operations with system indices owned by the plugin.
Details
The goal of this project is to restrict what plugins can do inside of a block where the ThreadContext has been stashed. The proposal is to limit actions a plugin can perform in this block to only operations related to its own system indices. Currently, plugins act as root and can operate unrestricted.
pseudo-code of a plugin’s API Handler:
public class IndexResourceTransportAction extends HandledTransportAction<IndexResourceRequest, IndexResourceResponse> {
private static final Logger LOG = LogManager.getLogger(IndexResourceTransportAction.class);
private final Client client;
private final TransportService transportService;
private final Settings settings;
@Inject
public IndexAnomalyDetectorTransportAction(
TransportService transportService,
ActionFilters actionFilters,
Client client,
Settings settings
) {
super(IndexResourceAction.NAME, transportService, actionFilters, IndexResourceRequest::new);
this.client = client;
this.transportService = transportService;
this.settings = settings;
}
@Override
protected void doExecute(Task task, IndexResourceRequest request, ActionListener<IndexResourceResponse> actionListener) {
// authenticated user context
// Without stashing the thread context, transport actions executed here are within the currently authenticated user context
// Plugins sometimes read user info from the ThreadContext before stashing.
// This is used for API Handlers that create scheduled jobs
// The plugins persist the user read in from the ThreadContext in their own system index
// User user = getUserContext(client);
String resourceName = request.getResourceName();
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
// root context
// Operations on system index here
// Index resource into plugin's system index
} catch (Exception e) {
LOG.error(e);
listener.onFailure(e);
}
}
}
This project proposes to bring plugin awareness to the ThreadContext so that whenever the ThreadContext is stashed, a header must be populated that contains the current plugin’s name. The security plugin can then leverage this header during privilege evaluation to determine if the current plugin is the owner of the (system) indices being acted upon. If not, the security plugin will block the request. No other transport actions will be allowed, only index operations on indices belonging to the plugin.
To accomplish this, the security plugin will also need to become aware of the installed plugins and their system indices. Currently, the security plugin maintains a list of System Indices and requires plugin teams submit Pull Requests to the security repo when adding a new System Index in order for the security plugin to properly protect the system index. The security plugin gives System Indices special protections which means that they cannot be created or deleted by regular users (including admin).
Additional Considerations:
- The job-scheduler plugin needs to be able to read other plugins’ system indices to get schedule information for scheduled jobs
Related Github issue: #4208
Implementation Details
To implement a solution will require a couple of components:
- Introduction of a concept of an ExecutionContext which is associated with the ThreadContext and accepts a single value intended to hold a reference to the plugin that takes over execution. This value could either contain the plugin unique name or the full ClassName to the plugin’s entrypoint. See https://github.com/cwperks/OpenSearch/pull/172/files for proof-of-concept
- Every single instance of core delegating to a plugin should be wrapped with a call to populate the execution context. When control is returned back to core from the plugin, the context should be cleared. If a plugin tries to override the context, it should fail similar to how overriding an existing threadcontext header fails.
- A potential solution to this problem could be the “proxy” pattern: https://stackoverflow.com/a/37702319
- Example implementation that shows how the ActionPlugin interface can be proxied: Create ExecutionContext and show example with ActionPluginProxy cwperks/OpenSearch#173
- Another example of “proxy” pattern, but in this case its the RestHandlers being proxied: Create ExecutionContext and show example with RestHandlerProxy cwperks/OpenSearch#172
- The RestHandlers need to be proxied separately, because once registered the extension points in ActionPlugin.java are no longer called and instead these handlers are called directly from the RestController here.
- Ideally, every instance of core delegating to a plugin should get wrapped, however, API handling is the most important to target for this change initially. Since the Security plugin already wraps every RestHandler, it may be possible to isolate changes to the Security plugin to populate the execution context before delegating to the original handler and clearing upon return. (See https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java#L190)
- One of the problems with this approach is that the security plugin is not aware of which plugin has registered the RestHandler. To solve this, the RestHandler may need to be modified to associate a plugin identity with the RestHandler
- A potential solution to this problem could be the “proxy” pattern: https://stackoverflow.com/a/37702319
- The 3rd implementation detail is for the Security plugin to implement a SystemIndexAccessEvaluator which requires obtaining a map of plugin’s and their system indices ({pluginId} → Set{Index Patterns})
- Formally, plugins that register system indices should be implementing the SystemIndexPlugin extension point. See Security’s implementation for an example: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L2030-L2038
- The systemIndexDescriptorMap is assembled in core here. This map should be shared with the security plugin, but not be shared with other plugins. To do this, a security specific extension point can be added. See the implementation of ActionPlugin.getRestHandlerWrapper for an instance of a security specific endpoint. A node fails to boot if 2 different plugins try to implement the same extension point.
- After obtaining the map of system indices, the security plugin can implement a new access evaluator similar to SecurityIndexAccessEvaluator. This evaluator will read from the thread context to see if
- User info is not populated in the thread context (meaning thread context is stashed) and
- read the execution context to see which plugin is currently executing code. If the action being executed is an operation on a system index owned by the plugin then its permitted, otherwise rejected.
- The logic in the SecurityFilter needs to change not to automatically skip privilege evaluation in blocks where the thread context is stashed. If the execution context is populated and no user info exists in the ThreadContext it should block all transport actions except for operations on a system index owned by the plugin initiating the request.
- The only exception to this rule is allowing job-scheduler to read from other plugins’ job indices. Job-scheduler actively listens to index operations on other plugins system indices that store information about scheduled jobs. For example, if the information about the scheduling of a job changes (i.e. interval every 10 min → interval every hour), job-scheduler listens to changes in documents in other plugins’ system indices to react to the changes.
- The security plugin can stop manually maintaining a list of system indices: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java#L49-L80
Note: This change could be breaking if there are plugins that do other transport actions in a block where the ThreadContext is stashed. This change is intentional to ensure that only index operations on system indices owned by the plugin that’s currently stashed the ThreadContext are permitted. If there is a necessity to run other actions within a block where the ThreadContext is stashed, we can consider implementing policy files for a plugin where a cluster admin gives specific permissions to a plugin to be able to perform. For this, a plugin identity would be analogous to a user with a single role.
Goals:
- No changes in plugins (other than the security plugin) required. Changes are isolated to core + security plugin and other plugins are unaware of this change that makes the entire plugin ecosystem more secure
Mermaid Diagram (For Reference/Regen)
sequenceDiagram
participant User
participant RestController
participant RestHandler
User->>RestController: PUT _plugins/_anomalydetection/detector/create
activate RestController
RestController->>RestController: Get RestHandler from registry
RestController->>RestHandler: Delegate to handleRequest
deactivate RestController
activate RestHandler
RestHandler->>RestHandler: Plugin processes RestRequest
RestHandler->>User: Send API Response
Details
In this diagram, OpenSearch acts as an HTTP server and creates a single thread to handle an API request once a request is received. When handling the request, it goes through the netty pipeline and then eventually into the RestController which is responsible for the actual handling of the request. The RestController has the full registry of RestActions (native + modules + plugins) and based on the requested API Path + Verb, it determines which RestHandler will handle the incoming request. If the RestHandler is from an API that was registered by a plugin, the RestController will call the handleRequest method of the registered RestHandler and the execution is delegated to a plugin at that point in time. Currently, there is nothing tracking where the point of execution is (i.e. the thread is executing code from the core or code from a plugin). It would be useful for the security plugin to have the execution context to differentiate whether a plugin is handling the API request or not.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status