Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] How should extension permissions be defined and where? #2502

Closed
Tracked by #2586
stephen-crawford opened this issue Mar 3, 2023 · 8 comments
Closed
Tracked by #2586
Assignees
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@stephen-crawford
Copy link
Contributor

stephen-crawford commented Mar 3, 2023

Broad Problem Statement

We need to be able to grant extensions permissions.

In order to tackle this task, it is best to divide the problem into smaller sections which can be managed individually. Specifically, there are three sections of issues which compose the larger problem: 1) Permission Syntax & Parsing; 2) Permission Granting & Enforcement; 3) Permission Storage.

How do permissions work now?

Permissions are granted to users via the role system implemented in the Security plugin. The majority of permissions relate to granting or denying privileges for specific actions that may occur in the OpenSearch cluster. Permissions are currently granted from roles. Roles have associated permissions that are resolved during authorization. Despite relating to the operations a user can execute on a cluster, the existing permission system does not directly map permissions to REST actions. Instead, some REST actions may correspond to numerous transport actions. For a user to execute one of the REST requests that map to multiple transport actions, that user must have permissions for all child actions. OpenSearch.org documentation provides the example of _bulk REST calls.

The current formatting of the actions in core follows <action_type>:<action>/<filter> for example indices:data/read/field_caps. A permission can be either index- or cluster-level. An index-level permission can be granted granularly. The index-level permissions allow for an optional resource pattern. For example, indices:admin/aliases can take an optional filter where the target resources are specified after the aliases action. Cluster-level permissions do not allow for this fine-tuning and can only be granted or not.

The current permission system also uses Action Groups as a method of bundling multiple permissions. Action Groups are generally used to aggregate a set of permissions--similar to a role--and then grant those permissions as a group to a user or role.

Extension Requirements

Users need to be able to grant an extension permissions so that they can restrict the access extensions have. Permissions should only be assignable by administrative users who already have those permissions themselves and users should be alerted whenever an extension tries to gain a permission or operate while lacking a permission.

Permission grants will need to be stored somewhere so that they can be preserved on restart. This will most likely be a system index.

Permissions granted to an extension should be consistent for the extension across the nodes in the OpenSearch cluster. This means that permissions grants will have to be confirmed consistent before an extension can operate based on them.

The permissions granted to an extension will need to be query-able and comparable to the existing permission model for users. An extension will only be able to operate using a permission if the user sending the extension a request also has the required permission.

Potentially Beneficial Design Choices

This section explores a few design options that are not required for Extensions, but may be worthwhile options for one reason or another.

Be specific about which permissions are missing: It may be helpful for users to be able to see a list of the permissions that are missing from either the user or extension when authorization fails. For example, consider a user with a set of permissions A, B, C and an extension with permissions C, D, E. In this case, both the user and extension have permission C. Then if the user requests the extension perform an operation that requires permissions, B, C, it may be helpful if we were to inform the user that the extension is missing permission B. Alternatively, if the user were lacking the permission, we could inform them they were lacking the requirement.

Principle of least-privileges: It may be smart to grant extensions as few permissions as possible whenever a grant event occurs. That means, that we restrict the number of implicit permissions an extension has on registration and also do not allow for negative grants. Negative grants allow can lead to a poor user experience since it is not always clear what privileges are being awarded during a negative grant.

Permission change verifier: It may be worthwhile to alert users if they are making a change which would change the privileges of a subject by a significant factor. For example, if an extension could originally read from 100 indices, if a permission change would allow it to read from 1000, we may want to alert the user and ask them to verify the change.

Proposals

Permission Syntax

For more information about permission syntax and parsing visit the dedicated issue.

Permission Granting

For discussion on how permissions will be granted to extensions please see the associated design.

Permission Storage

For information about the options for storing permissions for extensions visit the design.

@stephen-crawford stephen-crawford self-assigned this Mar 3, 2023
@stephen-crawford stephen-crawford converted this from a draft issue Mar 3, 2023
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Mar 3, 2023
@stephen-crawford stephen-crawford moved this from In Progress to Awaiting Review in Security for Extensions Mar 3, 2023
@stephen-crawford stephen-crawford added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Mar 3, 2023
@cwperks
Copy link
Member

cwperks commented Mar 9, 2023

@scrawfor99 I am a little unclear after reading this issue of the full scope of the issue.

I see a couple of problems that need to be solved for extensions around permissions:

  1. How can a cluster admin define the scope of what requests originating from an extension can do. i.e. Cluster admin would like to ensure that an extension is readonly on a handful of indices and any request trying to do otherwise is blocked.

  2. Plugins can currently define roles so cluster admins can assign users to those roles (i.e. anomaly_detection_full_access and anomaly_detection_read_only). How will this work with extensions?

Do you think it makes sense to break this into multiple issues for 1) how to define how requests originating from an extension can interact with the cluster and 2) how can users be permitted to perform different actions registered by the extension?


For 1) would it make sense to tie an extension to a service account (a dedicated account for the extension) and this account can be associated with one role that contains its policy for how it interacts with the cluster.

i.e.

service account `ext1` is mapped to `ext1_role`

ext1_role:
  cluster_permissions:
    - "cluster:*"
  index_permissions:
    - index_patterns:
        - 'logs-'
      allowed_actions:
        - 'indices_all'

For 2) the biggest problem I see is how to map an Extension REST Handler to a name similar to the action name currently used for authorization.

@peternied
Copy link
Member

We need to be able to grant extensions permissions.

This statement is pretty vague, lets focus towards more specific 'problem' scenarios. Here are some kinds of problems I see around extensions as we look to migration existing plugins.

  • Plugins have existing permissions such as cluster:admin/opensearch/ad/detector/delete will that permission still work for an extension. Where will work need to be done the scenario works as users expect?

  • Plugins previously have a concept of [predefined roles] that make configuration easier. Can an extension define these roles without editing the security codebase?

  • There is a onboarding process for the security plugin for plugins [link], how will that change for an extension? (This intersects with some of the above areas)

Do you think it makes sense to break this into multiple issues

I like this idea from Craig to help focus in on different areas.

Proposal

I only see one proposal - I think there are many viable approaches, could you list some out with pros/cons for them as points of comparison?

@RyanL1997
Copy link
Collaborator

RyanL1997 commented Mar 13, 2023

Hi @scrawfor99, Thanks for this write up. Attach to Peter's question:

Plugins have existing permissions such as cluster:admin/opensearch/ad/detector/delete will that permission still work for an extension. Where will work need to be done the scenario works as users expect?

I do have a general question about the compatibility with current design: if we choose to change the permission syntax for extensions, should we also redesign the syntax pattern for these types of permissions for plugins?

Another suggestion is that maybe we can separate the proposal into with 3 sub title for the permission syntax. Something like : "Option 1 : xxx, Option 2 xxx, ...". By doing that we can also list out the pros and cons.

@stephen-crawford
Copy link
Contributor Author

stephen-crawford commented Mar 13, 2023

Hi @peternied,

Thank you for your feedback and suggestions. I tried to address each of them below and let you know how I plan to follow-up.

We need to be able to grant extensions permissions.

This statement is pretty vague, lets focus towards more specific 'problem' scenarios. Here are some kinds of problems I see around extensions as we look to migration existing plugins.

The problem statement is intentionally broad to reflect the scale of the issue question. I think following your and Craig's suggestion to make smaller example scenarios seems like a good way to create more refined headings and sub-problems.

Plugins have existing permissions such as `cluster:admin/opensearch/ad/detector/delete` will that permission still work for an extension. Where will work need to be done the scenario works as users expect?

Plugins previously have a concept of [predefined roles] that make configuration easier. Can an extension define these roles without editing the security codebase?

There is a onboarding process for the security plugin for plugins [link], how will that change for an extension? (This intersects with some of the above areas)

Do you think it makes sense to break this into multiple issues

I like this idea from Craig to help focus in on different areas.

I am fine creating multiple issues but this will complicate the project board a great deal when we end up with 3 issues for this; 2 issues for another task; 4 for another; etc. If we want to break things down to very fine details we may want to look into some other options for organizing things. For now, I will create the issues and link them back to this issue in the appropriate location on the document without adding them directly to the project board.

Proposal

I only see one proposal - I think there are many viable approaches, could you list some out with pros/cons for them as points of comparison?

There are three options listed under the "Proposal" section for how we may go about defining permissions as an object. I will make sub-headers like @RyanL1997 suggested to make it easier to see.

@stephen-crawford
Copy link
Contributor Author

Hi @RyanL1997,

Thank you for reviewing this document and following-up. I have done my best to reply to your comments below.

Hi @scrawfor99, Thanks for this write up. Attach to Peter's question:

Plugins have existing permissions such as cluster:admin/opensearch/ad/detector/delete will that permission still work for an extension. Where will work need to be done the scenario works as users expect?

I do have a general question about the compatibility with current design: if we choose to change the permission syntax for extensions, should we also redesign the syntax pattern for these types of permissions for plugins?

I would like all permissions to be the same syntax for extensions and plugins. So if we use a new syntax for extensions, I think it would be best to transition the syntax for plugins as well. That being said, because we don't want to make a breaking change and also would like to support easy migration for users, I intend to make the legacy syntax supported as well. For this, I would plan on transitioning all permissions to the new syntactical model and then resolving the legacy permissions to the new model internally. I would also want us to put the legacy syntax on a deprecation path though we may decide to never fully remove support for them.

Another suggestion is that maybe we can separate the proposal into with 3 sub title for the permission syntax. Something like : "Option 1 : xxx, Option 2 xxx, ...". By doing that we can also list out the pros and cons.

This sounds like a good idea and I will go ahead and make this change above.

@stephen-crawford
Copy link
Contributor Author

stephen-crawford commented Mar 14, 2023

Hi @cwperks,

Thank you for your detailed and thoughtful reply. I have done my best to reply to each of your comments below.

@scrawfor99 I am a little unclear after reading this issue of the full scope of the issue.

I heard this from @peternied as well. I think that it is a challenging problem to define the scope for. When originally discussing the plans for extensions, the question we posed was "How should extension permissions be defined and where?" This question led me to the broad definition of the problem statement. Perhaps we should have been more explicit or granular. I certainly understand your confusion.

I see a couple of problems that need to be solved for extensions around permissions:

How can a cluster admin define the scope of what requests originating from an extension can do. i.e. Cluster admin would like to ensure that an extension is readonly on a handful of indices and any request trying to do otherwise is blocked.

My current intention is to treat extensions like a user with a set of permissions. Then extensions will granted a set of permissions by a cluster administrator. When a request is sent by a user, they will have to have the required permissions that the extension would need to perform the request. Then when it resolves after reaching the extension, the request will only fully process if the extension also has the required permissions. I will attach a diagram to clarify this flow.

Plugins can currently define roles so cluster admins can assign users to those roles (i.e. anomaly_detection_full_access and anomaly_detection_read_only). How will this work with extensions?

It is not clear to me if we are supporting roles with extensions. Assuming we plan to, I think that the adoption strategy is relatively straightforward. When an extension is installed, we will need to grab various configurations from it to allow it to operate with the cluster as a whole. Part of these configurations can be default roles which the extension author has defined in advance. These roles could then be assignable by the cluster administrator to different users by resolving the roles to their constituent permissions and assigning users those permissions. Alternatively, the roles could only be resolved when absolutely necessary and the permissions they contain could be treated as distinct from the general permissions granted to a user. These two options have their own pros and cons which I will address in a T-table in an associated issue.

Do you think it makes sense to break this into multiple issues for 1) how to define how requests originating from an extension can interact with the cluster and 2) how can users be permitted to perform different actions registered by the extension?

I have concerns about making too many individual issues related to extensions. If we make too many individual issues, I think it is going to be difficult to get consistent feedback from each other and will also cause the project board to become a mess. That being said, I realize that this issue is very broad and may be difficult for me to work with and others to review. To fix this, I will make new issues and link them to this document directly without adding them to the project board.

For 1) would it make sense to tie an extension to a service account (a dedicated account for the extension) and this account can be associated with one role that contains its policy for how it interacts with the cluster.

i.e.

service account `ext1` is mapped to `ext1_role`

ext1_role:
  cluster_permissions:
    - "cluster:*"
  index_permissions:
    - index_patterns:
        - 'logs-'
      allowed_actions:
        - 'indices_all'

This is effectively what I had in mind for storing the permissions associated with each extension. I am not sure that an entire object class will be required since we would likely only be storing permissions in the class--meaning a data table should be a fine alternative without needing to create another object to track. I will compare the options of creating an object class and not.

For 2) the biggest problem I see is how to map an Extension REST Handler to a name similar to the action name currently used for authorization.

For registering entirely new actions, we will need to have a dedicated discussion. I do not know how we should go about this as we are going to struggle to enforce naming conventions coming form extensions. If an extension wants to rename an existing action that should not be a problem since we can just resolve things behind the scenes. The problem I see is when an extension invents an entirely new action. More than likely, we will need to include the actions that should be registered as part of the extension installation process. We can have extension writers list the actions they want to register and inject them into the appropriate code classes as part of the extension being installed. I will make this a separate linked issue.

@peternied
Copy link
Member

Extension scopes are covered in #2587 are there other concepts from this issue that should persist or should we close it out?

@peternied peternied reopened this Apr 3, 2023
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Security for Extensions Apr 3, 2023
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Apr 3, 2023
@stephen-crawford stephen-crawford removed the untriaged Require the attention of the repository maintainers and may need to be prioritized label Apr 3, 2023
@davidlago davidlago moved this from Done to In Progress in Security for Extensions May 10, 2023
@davidlago
Copy link

Closing as we're dealing with this in the scopes track like @peternied said above.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Security for Extensions May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
Status: Done
Development

No branches or pull requests

5 participants