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 manage roles privilege #110633

Merged
merged 17 commits into from
Aug 27, 2024

Conversation

jfreden
Copy link
Contributor

@jfreden jfreden commented Jul 9, 2024

This PR adds functionality to limit the resources and privileges an Elasticsearch user can grant permissions to when creating a role. This is achieved using a new global (configurable/request aware) cluster privilege , named role, with a sub-key called manage/indices which is an array where each entry is a pair of index patterns and index privileges.

Definition

  • Using a role with this privilege to create, update or delete roles with privileges on indices outside of the indices matched by the index pattern in the indices array, will fail.
  • Using a role with this privilege to try to create, update or delete roles with cluster, run_as, etc. privileges will fail.
  • Using a role with this privilege with restricted indices will fail.
  • Other broader privileges (such as manage_security) will nullify this privilege.

Example

Create test-manage role:

POST _security/role/test-manage
{
    "global": {
        "role": {
            "manage": {
                "indices": [
                    {
                        "names": ["allowed-index-prefix-*"],
                        "privileges":["read"]
                    }
                ]
            }
        }
    }
}

And then a user with that role creates a role:

POST _security/role/a-test-role
{
    "indices": [
        {
            "names": [
                "allowed-index-prefix-some-index"
            ],
            "privileges": [
                "read"
            ]}]
}

But this would fail for:

POST _security/role/a-test-role
{
    "indices": [
        {
            "names": [
                "not-allowed-index-prefix-some-index"
            ],
            "privileges": [
                "read"
            ]}]
}

Backwards compatibility and mixed cluster concerns

  • A new mapping version has been added to the security index to store the new privilege.
  • If the new mapping version is not applied and a role descriptor with the new global privilege is written, the write will fail causing an exception.
  • When sending role descriptors over the transport layer in a mixed cluster, the new global privilege needs to be excluded for older versions. This is hanled with a new transport version.
  • If a role descriptor is serialized for API keys on one node in a mixed cluster and read from another, an older node might not be able to deserialize it, so it needs to be removed before being written in mixed cluster with old nodes. This is handled in the API key service.
  • If a role descriptor containing a global privilege is in a put role request in a mixed cluster where it's not supported on all nodes, fail request to create role.
  • RCS is not applicable here since RCS only considers cluster privileges and index privileges (not global cluster privileges).
  • This doesn't include remote privileges, since the current use case with connectors doesn't need roles to be created on a cluster separate from the cluster where the search data resides.

Follow up work

  • Create a docs PR
  • Error handling for actions that use manage roles. Should configurable cluster privileges that grant restricted usage of actions be listed in error authorization error messages?

@jfreden jfreden force-pushed the add_global_manage_roles branch 2 times, most recently from bdb487a to 51fb87b Compare August 7, 2024 12:42
@jfreden jfreden added the test-full-bwc Trigger full BWC version matrix tests label Aug 8, 2024
@jfreden jfreden force-pushed the add_global_manage_roles branch 4 times, most recently from 049a387 to 8a471dd Compare August 13, 2024 14:19
@jfreden jfreden force-pushed the add_global_manage_roles branch from b8cd20a to 3a9fdea Compare August 14, 2024 14:06
@jfreden jfreden force-pushed the add_global_manage_roles branch from b5d81ce to e7f995d Compare August 15, 2024 07:58
@jfreden jfreden marked this pull request as ready for review August 15, 2024 07:58
@jfreden jfreden requested a review from a team as a code owner August 15, 2024 07:58
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Aug 15, 2024
@jfreden jfreden added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Aug 15, 2024
@elasticsearchmachine elasticsearchmachine added Team:Security Meta label for security team and removed needs:triage Requires assignment of a team area label labels Aug 15, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @jfreden, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

*
* @return a list of tuples of all index and privilege pattern automaton
*/
public List<Tuple<Automaton, Automaton>> indexGroupAutomatons(boolean combine) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests in ManageRolesPrivilegesTests::testPrivilegeIntersectionPutRoleRequest makes this easier to reason about.

Copy link
Contributor

@slobodanadamovic slobodanadamovic left a comment

Choose a reason for hiding this comment

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

The high level changes are looking good. I'm leaving a few comments just to give you the initial feedback, but I still need more time to complete review of all changes.

@jfreden
Copy link
Contributor Author

jfreden commented Aug 26, 2024

Added an integration test for an API Key with manage_roles to make sure we cover that code path.

@jfreden
Copy link
Contributor Author

jfreden commented Aug 27, 2024

Extended the new API key test to also include a limited-by-role test.

Copy link
Contributor

@slobodanadamovic slobodanadamovic left a comment

Choose a reason for hiding this comment

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

LGTM Thanks for iterations. 👍

@jfreden
Copy link
Contributor Author

jfreden commented Aug 27, 2024

Thanks for the great review!

@jfreden jfreden added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 27, 2024
@elasticsearchmachine elasticsearchmachine merged commit fb32adc into elastic:main Aug 27, 2024
20 checks passed
@jfreden jfreden deleted the add_global_manage_roles branch August 27, 2024 12:10
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this pull request Sep 4, 2024
This PR adds functionality to limit the resources and privileges an
Elasticsearch user can grant permissions to when creating a role. This
is achieved using a new
[global](https://www.elastic.co/guide/en/elasticsearch/reference/current/defining-roles.html)
(configurable/request aware) cluster privilege , named `role`, with a
sub-key called `manage/indices` which is an array where each entry is a
pair of  [index
patterns](https://docs.google.com/document/d/1VN73C2KpmvvOW85-XGUqMmnMwXrfK4aoxRtG8tPqk7Y/edit#heading=h.z74zwo30t0pf)
and [index
privileges](https://www.elastic.co/guide/en/elasticsearch/reference/current/security-privileges.html#privileges-list-indices).

## Definition - Using a role with this privilege to create, update or
delete roles with privileges on indices outside of the indices matched
by the [index
pattern](https://docs.google.com/document/d/1VN73C2KpmvvOW85-XGUqMmnMwXrfK4aoxRtG8tPqk7Y/edit#heading=h.z74zwo30t0pf)
in the indices array, will fail.  - Using a role with this privilege to
try to create, update or delete roles with cluster, run_as, etc.
privileges will fail.  - Using a role with this privilege with
restricted indices will fail. - Other broader privileges (such as
manage_security) will nullify this privilege.

## Example Create `test-manage` role:

```
POST _security/role/test-manage
{
    "global": {
        "role": {
            "manage": {
                "indices": [
                    {
                        "names": ["allowed-index-prefix-*"],
                        "privileges":["read"]
                    }
                ]
            }
        }
    }
}
```

And then a user with that role creates a role:

```
POST _security/role/a-test-role
{
    "indices": [
        {
            "names": [
                "allowed-index-prefix-some-index"
            ],
            "privileges": [
                "read"
            ]}]
}
```

But this would fail for:

```
POST _security/role/a-test-role
{
    "indices": [
        {
            "names": [
                "not-allowed-index-prefix-some-index"
            ],
            "privileges": [
                "read"
            ]}]
}
```

## Backwards compatibility and mixed cluster concerns - A new mapping
version has been added to the security index to store the new privilege.
- If the new mapping version is not applied and a role descriptor with
the new global privilege is written, the write will fail causing an
exception. - When sending role descriptors over the transport layer in a
mixed cluster, the new global privilege needs to be excluded for older
versions. This is hanled with a new transport version.  - If a role
descriptor is serialized for API keys on one node in a mixed cluster and
read from another, an older node might not be able to deserialize it, so
it needs to be removed before being written in mixed cluster with old
nodes. This is handled in the API key service.  - If a role descriptor
containing a global privilege is in a put role request in a mixed
cluster where it's not supported on all nodes, fail request to create
role. - RCS is not applicable here since RCS only considers cluster
privileges and index privileges (not global cluster privileges). - This
doesn't include remote privileges, since the current use case with
connectors doesn't need roles to be created on a cluster separate from
the cluster where the search data resides.

## Follow up work - Create a docs PR  - Error handling for actions that
use manage roles. Should configurable cluster privileges that grant
restricted usage of actions be listed in error authorization error
messages?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team test-full-bwc Trigger full BWC version matrix tests v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants