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

[Security Solution] Prevent superuser access PLI gated APIs #176165

Merged
merged 19 commits into from
Feb 15, 2024

Conversation

semd
Copy link
Contributor

@semd semd commented Feb 2, 2024

Summary

This PR solves an issue with superuser (or any *) role and PLI (product level item) control.

Elasticsearch has_privileges API always returns true on any privilege for superuser role, even if the privilege has never been registered (more context here), causing superuser to be able to access product-restricted APIs (e.g. Routes that should only be available on complete tier, are also available on essentials tier).

Solution

We have the registered AppFeatures configuration locally, so we can solve the problem by checking that the action privilege exists and has been registered in the AppFeatures service, before doing any call to ES hasPrivileges API for RBAC.

Changes

  • AppFeatures service now stores a Set with all the (api and ui) actions registered.
  • Endpoint authz checks the actions against AppFeatures before checking RBAC. Only for server-side.
  • Route access: tag control has been extended to check actions against AppFeatures for securitySolution prefixed actions.
  • New securitySolutionAppFeature: route tag control for non-RBAC product feature checks. (This is not being used yet, but it will be needed)

Behavior change

  • UI: no change, everything should keep working the same way.
  • API: routes associated with higher product tier features (such as endpoint or entity analytics) won't be accessible for the superuser/admin role when running on lower product tiers, like security essentials.

@semd semd added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore labels Feb 2, 2024
@semd semd self-assigned this Feb 2, 2024
@azasypkin azasypkin self-requested a review February 2, 2024 16:36
@semd
Copy link
Contributor Author

semd commented Feb 5, 2024

/ci

@semd semd changed the title [Security Solution][POC] Prevent superuser PLI authorization leak [Security Solution] Prevent superuser PLI authorization leak Feb 5, 2024
@semd
Copy link
Contributor Author

semd commented Feb 5, 2024

@elasticmachine merge upstream

@semd
Copy link
Contributor Author

semd commented Feb 5, 2024

/ci

@semd
Copy link
Contributor Author

semd commented Feb 5, 2024

/ci

@semd
Copy link
Contributor Author

semd commented Feb 5, 2024

/ci

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security-wise, the change looks good to me, thanks. I still think that re-using access: tags for feature checks adds additional risk (even though it's not planned, the Security plugin can modify the relationship between this route tag and privilege actions in the future) and a fair amount of complexity that we could potentially avoid if we could rely solely on securitySolutionAppFeature: tags for that purpose. However, if this approach brings significant value to you, I believe it's tolerable.

@semd
Copy link
Contributor Author

semd commented Feb 7, 2024

@elasticmachine merge upstream

@semd
Copy link
Contributor Author

semd commented Feb 7, 2024

/ci

@semd semd requested a review from szwarckonrad February 9, 2024 16:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore)

@semd
Copy link
Contributor Author

semd commented Feb 9, 2024

@elasticmachine merge upstream

@semd semd requested review from a team as code owners February 9, 2024 17:26
@semd semd requested a review from tiansivive February 9, 2024 17:26
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ftr_configs.yml

@semd
Copy link
Contributor Author

semd commented Feb 12, 2024

@elasticmachine merge upstream

@semd semd added the v8.14.0 label Feb 12, 2024
@semd semd changed the title [Security Solution] Prevent superuser PLI authorization leak [Security Solution] Prevent superuser access PLI gated APIs Feb 12, 2024
@semd semd added the Project:Serverless Work as part of the Serverless project for its initial release label Feb 12, 2024
Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all the changes - looks good 👍

@semd
Copy link
Contributor Author

semd commented Feb 13, 2024

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

Copy link
Contributor

@hop-dev hop-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for this, and the thorough entity analytics tests! 🚀

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 11.5MB 11.5MB +288.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @semd

@semd semd merged commit 858347a into elastic:main Feb 15, 2024
35 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 15, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…176165)

## Summary

This PR solves an issue with `superuser` (or any `*`) role and PLI
(product level item) control.

Elasticsearch _has_privileges_ API always returns _true_ on any
privilege for `superuser` role, even if the privilege has never been
registered (more context
[here](elastic/elasticsearch#33928 (comment))),
causing superuser to be able to access product-restricted APIs (e.g.
Routes that should only be available on _complete_ tier, are also
available on _essentials_ tier).

## Solution

We have the registered AppFeatures configuration locally, so we can
solve the problem by checking that the action privilege exists and has
been registered in the AppFeatures service, before doing any call to ES
_hasPrivileges_ API for RBAC.

### Changes

- AppFeatures service now stores a Set with all the (`api` and `ui`)
actions registered.
- Endpoint authz checks the actions against AppFeatures before checking
RBAC. Only for server-side.
- Route `access:` tag control has been extended to check actions against
AppFeatures for _securitySolution_ prefixed actions.
- New `securitySolutionAppFeature:` route tag control for non-RBAC
product feature checks. (This is not being used yet, but it will be
needed)

### Behavior change

- UI: no change, everything should keep working the same way.
- API: routes associated with higher product tier features (such as
endpoint or entity analytics) won't be accessible for the
superuser/admin role when running on lower product tiers, like _security
essentials_.

---------

Co-authored-by: Kibana Machine <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…176165)

## Summary

This PR solves an issue with `superuser` (or any `*`) role and PLI
(product level item) control.

Elasticsearch _has_privileges_ API always returns _true_ on any
privilege for `superuser` role, even if the privilege has never been
registered (more context
[here](elastic/elasticsearch#33928 (comment))),
causing superuser to be able to access product-restricted APIs (e.g.
Routes that should only be available on _complete_ tier, are also
available on _essentials_ tier).

## Solution

We have the registered AppFeatures configuration locally, so we can
solve the problem by checking that the action privilege exists and has
been registered in the AppFeatures service, before doing any call to ES
_hasPrivileges_ API for RBAC.

### Changes

- AppFeatures service now stores a Set with all the (`api` and `ui`)
actions registered.
- Endpoint authz checks the actions against AppFeatures before checking
RBAC. Only for server-side.
- Route `access:` tag control has been extended to check actions against
AppFeatures for _securitySolution_ prefixed actions.
- New `securitySolutionAppFeature:` route tag control for non-RBAC
product feature checks. (This is not being used yet, but it will be
needed)

### Behavior change

- UI: no change, everything should keep working the same way.
- API: routes associated with higher product tier features (such as
endpoint or entity analytics) won't be accessible for the
superuser/admin role when running on lower product tiers, like _security
essentials_.

---------

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Project:Serverless Work as part of the Serverless project for its initial release release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants