[Proposal] Resource Permissions

[DarshitChanpura]

High-Level Design

Status Quo

Currently, there is no mechanism provided by the security plugin for plugins to utilize to provide fine-grained access control to own resources created by each plugin. Some plugins have addressed this by implementing their own custom authorization mechanism, however it is not sustainable for each plugin to handle resource-level FGAC using their own custom implementation. For example, in anomaly detection, granting permissions to delete detectors requires the user to be granted cluster:admin/opendistro/ad/detector/delete, which allows the user to delete all detectors in the cluster. Anomaly Detection has implemented their own authz mechanism. To avoid this the Security plugin should be the only place where authorization happens. To address this, we are introducing a new concept of Resource Permissions. This feature adds an additional step to the authorization evaluation, allowing plugins to define resource-specific permissions.

Feature Implementation

Once plugins define the resource permissions associated with resources, cluster admins can assign the relevant resource permissions to roles, which will then be evaluated within the Security plugin. Resource permissions can be defined in two ways:

1. Starts with resource:<plugin-identifier>/<permission> (proposed)
2. Starts with <plugin-identifier>:<permission>

The main difference is the presence of the standard prefix resource:, which simplifies validation here. The structure would look like:

# Allows users to use all Anomaly Detection functionality on selected resources, i.e., detectors
anomaly_full_access:
  reserved: true
  cluster_permissions:
    - 'cluster_monitor'
    - 'cluster:admin/opendistro/ad/*'
  index_permissions:
    - index_patterns:
        - '*'
      allowed_actions:
        - 'indices_monitor'
        - 'indices:admin/aliases/get'
        - 'indices:admin/mappings/get'
  resource_permissions:
    - identifier_pattern:
        - 'detector1'
        - 'detector2'
      allowed_actions:
        - 'resource:ad/detectors/read'

This allows admins to add a layer of control, restricting access to selected detectors instead of all detectors. Plugins like alerting, ml-commons, and flow-framework can similarly benefit by defining permissions at the resource level.

Evaluation Flow Diagram

• ResourceAccessEvaluator.java

  flowchart TD
  A[Resource Access Evaluator.evaluate]
  A --> B{Does request have resources defined?}
  B -->|No| C[Return response as incomplete]
  B -->|Yes| D{Do roles have resource_permissions defined?}
  D -->|No| E[Return response as denied and mark complete]
  D -->|Yes| F[Evaluate resource_permissions for all roles]
  F --> G{Do resource_permissions match with supplied resource names?}
  G -->|Yes| H[Return response as allowed and mark complete]
  G -->|No| E
  H --> J[Return presponse]
  E --> J
  C --> J

• PrivilegesEvaluator.java’s evaluate() method with resource permissions check:

  flowchart TD
  A[PrivilegesEvaluator.evaluate]
  A --> B[Resolve roles]
  B --> C{Is bulk request for default tenant?}
  C -->|Yes| D[Evaluate and return presponse]
  C -->|No| E[Resolve all indices in the request]
  E --> F[Evaluate snapshot restore request]
  F -->|Complete| G[Return response]
  F -->|Incomplete| H[Evaluate security index access request]
  H -->|Complete| I[Return response]
  H -->|Incomplete| J[Evaluate protected index access request]
  J -->|Complete| K[Return response]
  J -->|Incomplete| L[Evaluate Resource Access request]
  L -->|Complete| M[Return response]
  L -->|Incomplete| N[Evaluate PIT request]
  N -->|Complete| O[Return response]
  N -->|Incomplete| P{Is cluster permission request?}
  P -->|Yes| Q[Evaluate cluster-level permissions and return response]
  P -->|No| R[Evaluater terms aggregation request]
  R -->|Complete| S[Return response]
  R -->|Incomplete| T[Perform DNFOF evaluation if enabled]
  T --> U[Return result]
  style L fill:green,stroke:#333,stroke-width:4px

Low-Level Design

Design Questions

• Are we allowing a mix of roles with and without resource permissions?
  • If yes, what is the default behavior when none of the roles resolved for a user contain a resource permission?
  • Should the user be allowed permission to that resource?
    • If yes, there won’t be backward compatibility issues, but it is not ideal as the feature can be circumvented by not adding any resource permissions.
    • (Proposed) If no, there might be backward compatibility issues, especially in mixed cluster requests, which need thorough testing.
• Should we allow * (match_all) patterns for accessing resources? OR should they be individually listed under the role?
  • Note: `detector_ccan still be allowed but the question here is, should` be prohibited as a resource_name pattern.
• Should ressource_permissions be hidden behind feature flag?
  • No, the primary purpose of introducing this feature is to move the authorization logic, implemented by individual plugins with resources, to Security plugin.

File Changes

1. ActionRequest.java
   1. This base class is extended by every plugin’s implementation of transport request calls. Hence, the proposal is to add a standard class property List<String> resources . This should then be implemented by plugins to populate this property when handling an actionRequest after which SecurityFilter.java will intercept this call to perform privilege evaluation. (It is important to note this change when creating a documentation to onboard plugin developers to this change)
      1. Note: this is done for standardization purposes, however this can be skipped and left on the plugin developers to add this property to each request class. (Not recommended)
2. TransportService.java
   1. Add resource: as a valid transport action prefix here.
3. RoleV7.java
   1. This class will now define a new type of class: Resource (Use the class Index as reference)
   2. Add this newly defined class as a property to the parent
4. ResourceAccessEvaluator.java (new file)
   1. A new class that defines the evaluator method to verify the resource permissions.
   2. This method should assume the resource names are passed in the request (the string list defined above).
   3. If no resources are present in the list, it should assume that the plugin has not opted into this feature and so should bypass this authorization check to return a success response.
5. PrivilegeEvaluator.java
   1. Modify evaluate() method to add a check that calls the evaluator defined above.

Test Plan

1. Add local tests in the security plugin.
2. Add mixed cluster tests. (To evaluate requests coming in during rolling-upgrade scenario)
3. Add tests in anomaly detection or other plugins that could leverage this feature.

Will the feature require a security review?

This feature will require a security review to confirm that it correctly evaluates the scope of the requested resource.

Documentation

The documentation should be added for this feature detailing how users can utilize this feature.

--

Next Steps

• [ ] Open a META issue that tracks this feature
  • [ ] Add a task to update TransportService, in core repo, to accept a new action type prefix
  • [ ] Open a task to add this feature in Security Plugin (preferred if done via feature branch)
  • [ ] Open a follow-up issue to add tests for this feature
  • [ ] Open a PoC PR in anomaly detector plugin to demonstrate how this feature allows admins to allocate permissions to individual resources.

[shikharj05]

Thanks for the proposal @DarshitChanpura ! This looks interesting! High-level comments-

From what I understand, this proposal is targeting current roles and adding new resource_permissions to the existing RBAC system. What are your thoughts about treating resources as entities and allowing admins/users to tie up ACLs with a resource? For example, treating detector as a resource and access control will be defined on this resource (i.e. close to the entity being evaluated).

This proposal talks in depth about plugins, can you add an example if such a resource is added by core. This could also pave the way for treating indices as resources and tying down an ACL with them.

As this is introducing a new field/permission model, should this be a new config_version ? Also, there's another RFC to retire V6 https://github.com/opensearch-project/security/issues/4493 , I think we can use this opportunity to define how new permissions models/config_version changes should be added and older ones removed. Thoughts @DarshitChanpura @cwperks ?

[shikharj05]

Adding few more comments on each design question-

Are we allowing a mix of roles with and without resource permissions? If yes, what is the default behavior when none of the roles resolved for a user contain a resource permission? Should the user be allowed permission to that resource? If yes, there won’t be backward compatibility issues, but it is not ideal as the feature can be circumvented by not adding any resource permissions. (Proposed) If no, there might be backward compatibility issues, especially in mixed cluster requests, which need thorough testing.

For backwards compatibility, I think this should be introduced with default behavior of treating * as a resource on a role that doesn't have any explicit definitions of resource_permissions. I agree that this might not be the best security behavior, however, it allows for smoother upgrades and easier migration path for customers. To negate circumventing of the feature, for any new roles being created, resource_permissions must be explicitly added (maybe, we can add similar requirement for updates to existing roles). Thoughts?

For example, an existing role without resource permissions specified would be treated as equal to the following-

# Allow users to read Anomaly Detection detectors and results
anomaly_read_access:
  reserved: true
  cluster_permissions:
    - 'cluster:admin/opendistro/ad/detector/info'
    - 'cluster:admin/opendistro/ad/detector/search'
    - 'cluster:admin/opendistro/ad/detector/validate'
    - 'cluster:admin/opendistro/ad/detectors/get'
    - 'cluster:admin/opendistro/ad/result/search'
    - 'cluster:admin/opendistro/ad/result/topAnomalies'
    - 'cluster:admin/opendistro/ad/tasks/search'
  resource_permissions:
    - '*'

This would allow users upgrading to newer versions of OpenSearch where these permissions are introduced while letting users/admins define more granular access control methods.

[shikharj05]

Should we allow (match_all) patterns for accessing resources? OR should they be individually listed under the role? Note: detector_c can still be allowed but the question here is, should * be prohibited as a resource_name pattern.

For the reasons mentioned in previous comment, I think * needs to be allowed.

[shikharj05]

Should ressource_permissions be hidden behind feature flag? No, the primary purpose of introducing this feature is to move the authorization logic, implemented by individual plugins with resources, to Security plugin.

I agree with the requirement, however, this could also break existing customers. If we can cover migrations/upgrades with enough tests, it should be okay to use a feature flag to enable it. However, I would prefer a flag to turn this functionality on/off smoothly for initial release as this could be a breaking change.

[shikharj05]

File Changes

For file change, kindly consider the changes being brought in by https://github.com/opensearch-project/security/issues/3870 and ensure changes are compatible with the RFC.

RoleV7.java

Since this introduces a new class in existing model, do you know if contents of this class are sent over transport channel with requests? If so, this could cause issues during ugprades in a mixed-version cluster.