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

Limit Service Account permissions #3201

Closed
4 tasks done
Tracked by #2944
peternied opened this issue Aug 16, 2023 · 6 comments · Fixed by #3607
Closed
4 tasks done
Tracked by #2944

Limit Service Account permissions #3201

peternied opened this issue Aug 16, 2023 · 6 comments · Fixed by #3607
Assignees
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@peternied
Copy link
Member

peternied commented Aug 16, 2023

For the initial release permissions for service accounts are going to be locked down so they only can be used with explicitly granted system indexes

Exit Criteria

  • modify authz workflow to filter out all permissions other than index permissions with system index grant
  • add/modify test case that confirms cluster-wide permissions are not accessible
  • add/modify test case where index permissions without system index grant is filtered out
  • add/modify test case where index permissions with system index grant is allowed (happy path)
@peternied peternied mentioned this issue Aug 16, 2023
14 tasks
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Aug 16, 2023
@peternied peternied changed the title Limit Service account permissions or rotate credentials? Limit Service Account permissions Aug 17, 2023
@stephen-crawford
Copy link
Contributor

Looks good to me!

@stephen-crawford
Copy link
Contributor

[Triage] Thank you for filing this issue @peternied. This issue has clear expectations for action items (completing the check list) so going to mark as triaged.

@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 Aug 21, 2023
@cwperks
Copy link
Member

cwperks commented Aug 31, 2023

modify authz workflow to filter out all permissions other than index permissions with system index grant

@peternied What if this did not rely on a role at all and instead extension initialization included a reserved_indices setting that can be used on initialization for extension to reserve indices.

With this setting, the ExtensionsManager will have a mapping from <extension_unique_id> -> reserved set of indices.

Can the service account not be mapped to any roles at all and only carry the identity of the extension?

Within the SystemIndexAccessEvaluator it can lookup if a request is coming from an extension's service account and determine if the extension is making the request on only its set of reserved indices and no other indices or else fail the request.

The implementation of service account tokens I did in opensearch-project/opensearch-sdk-java#892 to show the SDK integrated with security for extensions does this if you'd like to see an example.

@peternied
Copy link
Member Author

This task has already been triaged, pointed, and scheduled in the current sprint, I'm not sure what you are asking?

@cwperks
Copy link
Member

cwperks commented Sep 1, 2023

@peternied I'm concerned that it could be confusing to have a role like

extension_a_svc_account_role:
  reserved: true
  hidden: false
  static: true
  description: "Allow full access to all indices and all cluster APIs"
  cluster_permissions:
    - "*"
  index_permissions:
    - index_patterns:
        - "*"
        - ".opensearch-observability"
      allowed_actions:
        - "*"
        - "system:admin/system_index"
  tenant_permissions:
    - tenant_patterns:
        - "*"
      allowed_actions:
        - "kibana_all_write"

but effectively we introduce logic to "whittle it down" to

extension_a_svc_account_role:
  reserved: true
  hidden: false
  static: true
  description: "Allow full access to all indices and all cluster APIs"
  index_permissions:
    - index_patterns:
        - ".opensearch-observability"
      allowed_actions:
        - "*"
        - "system:admin/system_index"

behind the scenes. (At least that's what I'm understanding this issue to be from the description). Is my understanding incorrect?

If we can modify the SystemIndexAccessEvaluator to lookup the extension's settings based on its principal then we can determine if that principal is associated with the system indices being acted upon. I think its simpler and logic does not need to be introduced to "whittle down" a role.

@peternied
Copy link
Member Author

@cwperks that is the correct observation. It is intentional to be strange, because its easy to implement and easy to revert without breaking deployed services accounts if/when we invest more into this space. Since extensions are not in active development, features such as service accounts are minimally built up to support these scenarios and we can improve on them as provides value.

Since this is already costed and included in the sprint change this design is a signal to remove it from the sprint. Lets move the meta discussion to an internal channel were we figure out how to handle this item.

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
None yet
Development

Successfully merging a pull request may close this issue.

4 participants