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

Authorization denial errors are not actionable #42166

Closed
tvernum opened this issue May 16, 2019 · 1 comment · Fixed by #69318
Closed

Authorization denial errors are not actionable #42166

tvernum opened this issue May 16, 2019 · 1 comment · Fixed by #69318
Assignees
Labels
>enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team

Comments

@tvernum
Copy link
Contributor

tvernum commented May 16, 2019

The access denied error message is:

action [{}] is unauthorized for user [{}]

This has a few problems:

  1. In the case of an index level action, it doesn't tell you which index was denied.
  2. It doesn't list the user's roles
  3. We discourage security administrators from assigning raw actions to roles, but that is the only information that is provided in the error.

When users run into these errors they aren't being given enough information to be able to solve the problem. We need to be more explicit about exactly what was rejected and the options to resolve it.

One idea was to include a list of the cluster/index privileges that would grant this action (perhaps roughly sorted from least-privilege to most-privileged)

@tvernum tvernum added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels May 16, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@tvernum tvernum self-assigned this Jun 3, 2019
@rjernst rjernst added the Team:Security Meta label for security team label May 4, 2020
tvernum added a commit to tvernum/elasticsearch that referenced this issue Jul 29, 2020
Access denied messages for indices were overly brief and missed two
pieces of useful information:

1. The names of the indices for which access was denied
2. The privileges that could be used to grant that access

This change improves the access denied messages for index based
actions by adding the index and privilege names.
Privilege names are listed in order from least-privilege to
most-privileged so that the first recommended path to resolution is
also the lowest privilege change.

Relates: elastic#42166
tvernum added a commit that referenced this issue Aug 4, 2020
Access denied messages for indices were overly brief and missed two
pieces of useful information:

1. The names of the indices for which access was denied
2. The privileges that could be used to grant that access

This change improves the access denied messages for index based
actions by adding the index and privilege names.
Privilege names are listed in order from least-privilege to
most-privileged so that the first recommended path to resolution is
also the lowest privilege change.

Relates: #42166
tvernum added a commit to tvernum/elasticsearch that referenced this issue Aug 5, 2020
Access denied messages for indices were overly brief and missed two
pieces of useful information:

1. The names of the indices for which access was denied
2. The privileges that could be used to grant that access

This change improves the access denied messages for index based
actions by adding the index and privilege names.
Privilege names are listed in order from least-privilege to
most-privileged so that the first recommended path to resolution is
also the lowest privilege change.

Relates: elastic#42166
Backport of: elastic#60357
tvernum added a commit to tvernum/elasticsearch that referenced this issue Dec 31, 2020
In elastic#60357 we improved the error message when access to perform an
action on an index was denied by including the index name and the
privileges that would grant the action.

This commit extends the second part of that change (the list of
privileges that would resolve the problem) to situations when a
cluster action is denied.

This implementation for cluster privileges is slightly more complex
than that of index privileges because cluster privileges can be
dependent on parameters in the request, not just the action name.
For example, "manage_own_api_key" should be suggested as a matching
privilege when a user attempts to create an API key, or delete their
own API key, but should not be suggested when that same user attempts
to delete another user's API key.

Relates: elastic#42166
tvernum added a commit that referenced this issue Jan 12, 2021
In #60357 we improved the error message when access to perform an
action on an index was denied by including the index name and the
privileges that would grant the action.

This commit extends the second part of that change (the list of
privileges that would resolve the problem) to situations when a
cluster action is denied.

This implementation for cluster privileges is slightly more complex
than that of index privileges because cluster privileges can be
dependent on parameters in the request, not just the action name.
For example, "manage_own_api_key" should be suggested as a matching
privilege when a user attempts to create an API key, or delete their
own API key, but should not be suggested when that same user attempts
to delete another user's API key.

Relates: #42166
tvernum added a commit to tvernum/elasticsearch that referenced this issue Feb 22, 2021
This commit adds a User's list of current role names to access denied
error messages to aid in diagnostics.
This allows an administrator to know whether the correct course of
action is to add another role to the user (e.g. by fixing incorrect
role mappings) or by modifying a role to add more privileges.

Resolves: elastic#42166
tvernum added a commit that referenced this issue Mar 10, 2021
This commit adds a User's list of current role names to access denied
error messages to aid in diagnostics.
This allows an administrator to know whether the correct course of
action is to add another role to the user (e.g. by fixing incorrect
role mappings) or by modifying a role to add more privileges.

Resolves: #42166
tvernum added a commit to tvernum/elasticsearch that referenced this issue Apr 15, 2021
Our authorization engine has a short-circuit check for the intended
action the takes place before resolving index names (wildcards).

That is, a requests like

    GET /_search
    GET /logs-*/_search
    GET /logs-20210414/_search

will fail fast if the user does not have read permission on any
indices, and we will never resolve the list of indices that the
request targets.

Consequently, it is impossible to provide the list of denied indices
in the error message because that list does exist (and, in the case of
wildards would be empty even if we did resolve it).

This change updates the access denied message so that it does not
attempt to include the list of indices if the IndiceAccessControl
object has an empty list of denied indices.

Prior to this, we would generate messages such as

    action [indices:data/read/search] is unauthorized for user [test]
    with roles [test] on indices [],

That "indices []" section is never useful since it does not name any
indices, so it has now been dropped from the message if it is empty.

Relates: elastic#42166, elastic#60357
tvernum added a commit that referenced this issue Apr 19, 2021
Our authorization engine has a short-circuit check for the intended
action the takes place before resolving index names (wildcards).

That is, a requests like

    GET /_search
    GET /logs-*/_search
    GET /logs-20210414/_search

will fail fast if the user does not have read permission on any
indices, and we will never resolve the list of indices that the
request targets.

Consequently, it is impossible to provide the list of denied indices
in the error message because that list does exist (and, in the case of
wildards would be empty even if we did resolve it).

This change updates the access denied message so that it does not
attempt to include the list of indices if the IndicesAccessControl
object has an empty list of denied indices.

Prior to this, we would generate messages such as

    action [indices:data/read/search] is unauthorized for user [test]
    with roles [test] on indices [],

That "indices []" section is never useful since it does not name any
indices, so it has now been dropped from the message if it is empty.

Relates: #42166, #60357
tvernum added a commit to tvernum/elasticsearch that referenced this issue Apr 19, 2021
Our authorization engine has a short-circuit check for the intended
action the takes place before resolving index names (wildcards).

That is, a requests like

    GET /_search
    GET /logs-*/_search
    GET /logs-20210414/_search

will fail fast if the user does not have read permission on any
indices, and we will never resolve the list of indices that the
request targets.

Consequently, it is impossible to provide the list of denied indices
in the error message because that list does not exist (and, in the
case of wildards would be empty even if we did resolve it).

This change updates the access denied message so that it does not
attempt to include the list of indices if the IndicesAccessControl
object has an empty list of denied indices.

Prior to this, we would generate messages such as

    action [indices:data/read/search] is unauthorized for user [test]
    with roles [test] on indices [],

That "indices []" section is never useful since it does not name any
indices, so it has now been dropped from the message if it is empty.

Relates: elastic#42166, elastic#60357
Backport of: elastic#71715
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants