[Security Solution] Prevent superuser access PLI gated APIs by semd · Pull Request #176165 · elastic/kibana

semd · 2024-02-02T15:57:43Z

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 · 2024-02-05T10:06:44Z

/ci

semd · 2024-02-05T11:56:51Z

@elasticmachine merge upstream

semd · 2024-02-05T11:57:10Z

/ci

x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts

semd · 2024-02-05T15:51:50Z

/ci

…fix'

semd · 2024-02-05T16:31:58Z

/ci

x-pack/plugins/security_solution/server/lib/app_features_service/app_features_service.ts

azasypkin

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 · 2024-02-07T12:43:44Z

@elasticmachine merge upstream

semd · 2024-02-07T12:44:03Z

/ci

elasticmachine · 2024-02-09T16:38:06Z

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

elasticmachine · 2024-02-09T16:38:07Z

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

elasticmachine · 2024-02-09T16:38:09Z

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

semd · 2024-02-09T16:38:16Z

@elasticmachine merge upstream

jbudz

ftr_configs.yml

semd · 2024-02-12T10:39:17Z

@elasticmachine merge upstream

paul-tavares

Thank you for all the changes - looks good 👍

semd · 2024-02-13T11:43:01Z

@elasticmachine merge upstream

kibanamachine · 2024-02-13T11:43:03Z

merge conflict between base and head

hop-dev

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

kibana-ci · 2024-02-14T12:08:39Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: cc20fcf

Failed CI Steps

FTR Configs #81

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

💛 Build #193266 was flaky ee9d7c6
💛 Build #192908 was flaky 67209bc
💛 Build #192867 was flaky c232354
💔 Build #192651 failed 22428e6
💔 Build #191894 failed d5d6f73
💛 Build #191436 was flaky de59226

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

cc @semd

…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 <42973632+kibanamachine@users.noreply.github.com>

check registered appFeatures internally

3958ae7

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 self-assigned this Feb 2, 2024

semd added 2 commits February 2, 2024 17:14

fix endpoint authz check

54b5cb1

removed wrong test

e257d92

azasypkin self-requested a review February 2, 2024 16:36

renaming

c4b076a

update mocks

1b020bb

semd changed the title ~~[Security Solution][POC] Prevent superuser PLI authorization leak~~ [Security Solution] Prevent superuser PLI authorization leak Feb 5, 2024

Merge branch 'main' into poc_security_check_app_features

5329afb

fix ui actions

0d5c361

paul-tavares reviewed Feb 5, 2024

View reviewed changes

x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts Outdated Show resolved Hide resolved

semd added 2 commits February 5, 2024 16:05

add test for registerApiAccessControl

78b153c

throw endpoint context not started error

0bfd1f4

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

de59226

…fix'

azasypkin reviewed Feb 6, 2024

View reviewed changes

x-pack/plugins/security_solution/server/lib/app_features_service/app_features_service.ts Show resolved Hide resolved

azasypkin reviewed Feb 6, 2024

View reviewed changes

x-pack/plugins/security_solution/server/lib/app_features_service/app_features_service.ts Outdated Show resolved Hide resolved

azasypkin reviewed Feb 7, 2024

View reviewed changes

improve api hook implementation

490a334

Merge branch 'main' into poc_security_check_app_features

d5d6f73

semd requested a review from szwarckonrad February 9, 2024 16:38

kibanamachine and others added 2 commits February 9, 2024 11:38

Merge branch 'main' into poc_security_check_app_features

b87b706

solve conflict

22428e6

semd requested review from a team as code owners February 9, 2024 17:26

semd requested a review from tiansivive February 9, 2024 17:26

jbudz approved these changes Feb 9, 2024

View reviewed changes

Merge branch 'main' into poc_security_check_app_features

c232354

semd added the v8.14.0 label Feb 12, 2024

add endpoint api tests for superuser

67209bc

semd changed the title ~~[Security Solution] Prevent superuser PLI authorization leak~~ [Security Solution] Prevent superuser access PLI gated APIs Feb 12, 2024

semd added the Project:Serverless Work as part of the Serverless project for its initial release label Feb 12, 2024

paul-tavares approved these changes Feb 12, 2024

View reviewed changes

conflict solved

ee9d7c6

conflict solved

cc20fcf

hop-dev approved these changes Feb 14, 2024

View reviewed changes

semd merged commit 858347a into elastic:main Feb 15, 2024

kibanamachine added the backport:skip This PR does not require backporting label Feb 15, 2024

Conversation

semd commented Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Solution

Changes

Behavior change

Uh oh!

semd commented Feb 5, 2024

Uh oh!

semd commented Feb 5, 2024

Uh oh!

semd commented Feb 5, 2024

Uh oh!

Uh oh!

semd commented Feb 5, 2024

Uh oh!

semd commented Feb 5, 2024

Uh oh!

Uh oh!

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

semd commented Feb 7, 2024

Uh oh!

semd commented Feb 7, 2024

Uh oh!

elasticmachine commented Feb 9, 2024

Uh oh!

elasticmachine commented Feb 9, 2024

Uh oh!

elasticmachine commented Feb 9, 2024

Uh oh!

semd commented Feb 9, 2024

Uh oh!

jbudz left a comment

Choose a reason for hiding this comment

Uh oh!

semd commented Feb 12, 2024

Uh oh!

paul-tavares left a comment

Choose a reason for hiding this comment

Uh oh!

semd commented Feb 13, 2024

Uh oh!

kibanamachine commented Feb 13, 2024

Uh oh!

hop-dev left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Feb 14, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

semd commented Feb 2, 2024 •

edited

Loading