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

Add get-user-privileges API #33928

Merged
merged 20 commits into from
Oct 18, 2018
Merged

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Sep 21, 2018

The get-user-privileges API (_xpack/security/user/_privileges) returns
the current user's effective privileges in a role-like structure.

Because the effective privileges are the result of merging potentially
multiple roles, the resulting structure has a few differences to the
standard role JSON - some fields that are single values in roles, are
multivariate (arrays) in this API.

It is only possible to return the current user's privileges, but the
es-security-runas-user header can be used to load another user as the
current effective user.

Resolves: #32777

This API is intended as a companion to the _has_privileges API.
It returns the list of privileges that are held by the current user.
This information is difficult to reason about, and consumers should
avoid making direct security decisions based solely on this data.
For example, each of the following index privileges (as well as many
more) would grant a user access to index a new document into the
"metrics-2018-08-30" index, but clients should not try and deduce
that information from this API.
- "all" on "*"
- "all" on "metrics-*"
- "write" on "metrics-2018-*"
- "write" on "metrics-2018-08-30"

Rather, if a client wished to know if a user had "index" access to
_any_ index, it would be possible to use this API to determine whether
the user has any index privileges, and on which index patterns, and
then feed those index patterns into _has_privileges in order to
determine whether the "index" privilege had been granted.

The result JSON is modelled on the Role API, with a few small changes
to reflect how privileges are modelled when multiple roles are merged
together (multiple DLS queries, multiple FLS grants, multiple global
conditions, etc).
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@tvernum
Copy link
Contributor Author

tvernum commented Sep 21, 2018

FYI: @kobelb

@tvernum
Copy link
Contributor Author

tvernum commented Sep 21, 2018

To be handled in separate PRs:

  • Docs
  • High Level Rest Client (in progress)

@tvernum
Copy link
Contributor Author

tvernum commented Sep 21, 2018

@kobelb
As part of this I found an issue that I thought I dealt with in 6.4 (but obviously didn't) regarding undefined privileges.

In 6.4, this happens:

POST _xpack/security/privilege
{ "kibana" : { "read" : { "actions": [ "data:read/*" ] } } }

{"kibana":{"read":{"created":true}}}

GET _xpack/security/privilege

{"kibana":{"read":{"application":"kibana","name":"read","actions":["data:read/*"],"metadata":{}}}}

PUT _xpack/security/role/test1
{ "applications": [ { "application": "kibana" , "privileges": ["read"], "resources":["*"] } ] }

{ "role":{"created":true}}

PUT _xpack/security/role/test2
{ "applications": [ { "application": "kibana" , "privileges": ["action:*"], "resources":["*"] } ] }

{ "role":{"created":true}}

PUT _xpack/security/role/test3
{ "applications": [ { "application": "kibana" , "privileges": ["write"], "resources":["*"] } ] }

{ "role":{"created":true}}

PUT _xpack/security/user/test1
{ "password" : "magic123", "roles" : [ "test1"] }

{"created":true}

PUT _xpack/security/user/test2
{ "password" : "magic123", "roles" : [ "test2"] }

{"created":true}

PUT _xpack/security/user/test3
{ "password" : "magic123", "roles" : [ "test3"] }

{"created":true}
for N in 1 2 3
do
  printf "\nUser: test%d :: " $N 
  curl -u test$N:magic123 \
   -XGET http://localhost:9200/_xpack/security/user/_has_privileges \
   -d '{ "application": [ { "application": "kibana", "privileges": [ "read", "write", "nothing" ], "resources": [ "*" ] } ] }' \
   -H 'Content-Type: application/json'
done

User: test1 :: {"username":"test1","has_all_requested":true,"cluster":{},"index":{},"application":{"kibana":{"*":{"read":true,"nothing":true,"write":true}}}}
User: test2 :: {"username":"test2","has_all_requested":false,"cluster":{},"index":{},"application":{"kibana":{"*":{"read":false,"nothing":true,"write":true}}}}
User: test3 :: {"username":"test3","has_all_requested":false,"cluster":{},"index":{},"application":{"kibana":{"*":{"read":false,"nothing":false,"write":false}}}}

The privileges "write" and "nothing" don't exist but:

  • user 1 has them, because it has read which does exist, so it has something and something grants nothing.
  • user 2 has them, for a similar reason because it has action:*, which is something.
  • user 3 does not have those privilege, because it has write which doesn't exist, and nothing does not grant nothing.

The "read" privilege does exist, so only user 1 has it.

I've fixed that in this PR, so that on this branch we get

User: test1 :: {"username":"test1","has_all_requested":false,"cluster":{},"index":{},"application":{"kibana":{"*":{"read":true,"nothing":false,"write":false}}}}
User: test2 :: {"username":"test2","has_all_requested":false,"cluster":{},"index":{},"application":{"kibana":{"*":{"read":false,"nothing":false,"write":false}}}}
User: test3 :: {"username":"test3","has_all_requested":false,"cluster":{},"index":{},"application":{"kibana":{"*":{"read":false,"nothing":false,"write":true}}}}

which is what you expect.

  1. no one has privileges you wouldn't expect
  2. user 3 has write even though it isn't defined yet, because it has the same name.

I don't think that will cause you any issues, but I want to highlight it in advance.

What I've just discovered (and will fix) is that this means that superuser will return false for privileges that don't exist.
I think that's wrong, so there will be a special case that the "*" privilege will always return true, even for privileges that don't exist (since, by definition it matches everything).

@kobelb
Copy link
Contributor

kobelb commented Sep 21, 2018

@tvernum in 6.4, we're only checking whether the user has specific actions and we aren't checking whether they have specific privileges, is it safe to assume that this same behavior that you've noticed doesn't exist in those scenarios?

@tvernum
Copy link
Contributor Author

tvernum commented Sep 26, 2018

@kobelb That's correct. Checking for raw actions is not affected by this change.

@kobelb
Copy link
Contributor

kobelb commented Oct 1, 2018

Are we still on track for this to land in 6.5? We'd ideally like to switch to this within 6.5 or else we'll be relying on a rather unperformant alternative.

@tvernum
Copy link
Contributor Author

tvernum commented Oct 4, 2018

Ping @jaymode @albertzaharovits -- Any chance of a review?

XPackLicenseState licenseState) {
super(settings, licenseState);
this.securityContext = securityContext;
controller.registerHandler(GET, "/_xpack/security/user/{username}/_privileges", this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this handler necessary? Isn't the /_xpack/security/user/_privileges enough given this is a SAME_USER_PRIVILEGE type of action?

Copy link
Member

Choose a reason for hiding this comment

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

👍

.filter(e -> e.matchesPrivilege(privilege))
.map(e -> e.resourceNames)
.flatMap(Set::stream)
.collect(Collectors.toSet());
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks to me that you went to great lengths to display app privileges in nicely formatted JSON.
What do you think if you also do resource-wildcard-compression here? I mean, to go through
the flatted array and eliminate resources that are a subset of another, eg user/kimchy is a subset of user/* so do not print it.
Again, I would not suggest this if you hadn't already reached up to here! Kibana will be thrilled!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is trickier than it sounds.
The Automatons are (in some cases) already unioned, so calculating subsets is tricky.

In the example from this javadoc:

"read", [ "user/*" ]
"all", [ "user/kimchy", "config/*" ]

the two resources for all are already merged into a single automaton, so there's nothing that is a pure subset of the read resource.

We can ignore the union by recompiling each of the resource names, but that would introduce Automaton compilation that we don't otherwise need.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, it would require compiling the Automaton for every element of the Set.
We can leave it as a homework for when somebody else trips on this and can't do without it.

@rjernst rjernst removed the review label Oct 10, 2018
Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

I left some minor comments. Otherwise LGTM


import static org.hamcrest.Matchers.equalTo;

public class GetUserPrivilegesResponseTests extends ESTestCase {
Copy link
Member

Choose a reason for hiding this comment

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

since this response implements equals and hashcode, can you add tests for this?

@tvernum tvernum merged commit 9200e15 into elastic:master Oct 18, 2018
tvernum added a commit that referenced this pull request Oct 22, 2018
This API is intended as a companion to the _has_privileges API.
It returns the list of privileges that are held by the current user.

This information is difficult to reason about, and consumers should
avoid making direct security decisions based solely on this data.
For example, each of the following index privileges (as well as many
more) would grant a user access to index a new document into the
"metrics-2018-08-30" index, but clients should not try and deduce
that information from this API.
- "all" on "*"
- "all" on "metrics-*"
- "write" on "metrics-2018-*"
- "write" on "metrics-2018-08-30"

Rather, if a client wished to know if a user had "index" access to
_any_ index, it would be possible to use this API to determine whether
the user has any index privileges, and on which index patterns, and
then feed those index patterns into _has_privileges in order to
determine whether the "index" privilege had been granted.

The result JSON is modelled on the Role API, with a few small changes
to reflect how privileges are modelled when multiple roles are merged
together (multiple DLS queries, multiple FLS grants, multiple global
conditions, etc).

Backport of: 9200e15, 2283a33, aeb3cda
kcm pushed a commit that referenced this pull request Oct 30, 2018
This API is intended as a companion to the _has_privileges API.
It returns the list of privileges that are held by the current user.

This information is difficult to reason about, and consumers should
avoid making direct security decisions based solely on this data.
For example, each of the following index privileges (as well as many
more) would grant a user access to index a new document into the
"metrics-2018-08-30" index, but clients should not try and deduce
that information from this API.
- "all" on "*"
- "all" on "metrics-*"
- "write" on "metrics-2018-*"
- "write" on "metrics-2018-08-30"

Rather, if a client wished to know if a user had "index" access to
_any_ index, it would be possible to use this API to determine whether
the user has any index privileges, and on which index patterns, and
then feed those index patterns into _has_privileges in order to
determine whether the "index" privilege had been granted.

The result JSON is modelled on the Role API, with a few small changes
to reflect how privileges are modelled when multiple roles are merged
together (multiple DLS queries, multiple FLS grants, multiple global
conditions, etc).
semd added a commit to elastic/kibana that referenced this pull request Feb 15, 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](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>
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 <42973632+kibanamachine@users.noreply.github.com>
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 <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants