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.

Sign up for GitHub

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

Jump to bottom

Remove system-index write-access from superuser role #81400

Merged
merged 57 commits into from
Jan 17, 2022
Merged

Remove system-index write-access from superuser role #81400

merged 57 commits into from
Jan 17, 2022

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Dec 7, 2021 •
edited
Loading

This commit changes the superuser role so that it no longer has any sort of write access to restricted indices (system indices).
This improves the safety ad security of the cluster, as it means that there are no out-of-the-box users or roles that can write to, delete or close the security index.

Superusers can still read from (and monitor) system indices.

Done

  • Remove write access from superuser role descriptor (46614de)
  • Change XPackSecurity internal user to have its own private role and not rely on superuser (7da9999)
  • Fix QA tests that use test_superuser to manipulate system indices
  • Remove short circuit case for superuser if the user has other roles as well.

@tvernum tvernum added >breaking WIP :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Dec 7, 2021
@tvernum tvernum requested a review from ywangd December 7, 2021 04:02
@tvernum
Copy link
Contributor Author

tvernum commented Dec 7, 2021

@jkakavas @ywangd FYI. It's still draft for now, but I'll be looking for reviews later.

@tvernum
Copy link
Contributor Author

tvernum commented Dec 7, 2021

It is not yet decided whether we want to go down this path at this point in time. This PR is to explore what it would look like if we did make that decision.

@tylersmalley tylersmalley mentioned this pull request Dec 9, 2021
Closed
@ashokaditya ashokaditya mentioned this pull request Dec 20, 2021
Merged
2 tasks
@jportner jportner mentioned this pull request Jan 17, 2022
Closed
@mark-vieira mark-vieira mentioned this pull request Jan 18, 2022
Merged
This was referenced Jan 19, 2022
Closed
Merged
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Feb 8, 2022
@ywangd
Clean up for superuser role name references
e32c273 
The _xpack_security user no longer has the superuser role since elastic#81400
@ywangd ywangd mentioned this pull request Feb 8, 2022
Merged
ywangd added a commit that referenced this pull request Feb 14, 2022
@ywangd
Clean up for superuser role name references (#83627)
a9cdbf4 
The _xpack_security user no longer has the superuser role since #81400
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Feb 14, 2022
@ywangd
Clean up for superuser role name references (elastic#83627)
b35c670 
The _xpack_security user no longer has the superuser role since elastic#81400
@ywangd ywangd mentioned this pull request Feb 14, 2022
Merged
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Feb 14, 2022
@ywangd
Clean up for superuser role name references (elastic#83627)
b78628d 
The _xpack_security user no longer has the superuser role since elastic#81400
@ywangd ywangd mentioned this pull request Feb 14, 2022
Merged
elasticsearchmachine pushed a commit that referenced this pull request Feb 14, 2022
@ywangd
Clean up for superuser role name references (#83627) (#83881)
1d1379e 
The _xpack_security user no longer has the superuser role since #81400
elasticsearchmachine pushed a commit that referenced this pull request Feb 14, 2022
@ywangd
Clean up for superuser role name references (#83627) (#83882)
11c26af 
The _xpack_security user no longer has the superuser role since #81400
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Mar 31, 2022
@tvernum
Ignore app priv failures when resolving superuser
13946cd 
In elastic#81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve
those other roles would not block superuser access - when a user has
superuser role, any failures in role resolution will be effectively
ignored, and the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only if other roles
or application privileges are unavailable
@tvernum tvernum mentioned this pull request Mar 31, 2022
Merged
elasticsearchmachine pushed a commit that referenced this pull request Apr 1, 2022
@tvernum
Ignore app priv failures when resolving superuser (#85519)
c61f562 
In #81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve those
other roles would not block superuser access - when a user has superuser
role, any failures in role resolution will be effectively ignored, and
the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only, whenever other
roles or application privileges are unavailable

Relates: #85312
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 1, 2022
@tvernum
Ignore app priv failures when resolving superuser (elastic#85519)
5269c4c 
In elastic#81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve those
other roles would not block superuser access - when a user has superuser
role, any failures in role resolution will be effectively ignored, and
the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only, whenever other
roles or application privileges are unavailable

Relates: elastic#85312
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 1, 2022
@tvernum
Ignore app priv failures when resolving superuser (elastic#85519)
37d36a0 
In elastic#81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve those
other roles would not block superuser access - when a user has superuser
role, any failures in role resolution will be effectively ignored, and
the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only, whenever other
roles or application privileges are unavailable

Relates: elastic#85312
elasticsearchmachine pushed a commit that referenced this pull request Apr 1, 2022
@tvernum
Ignore app priv failures when resolving superuser (#85519) (#85588)
b690154 
In #81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve those
other roles would not block superuser access - when a user has superuser
role, any failures in role resolution will be effectively ignored, and
the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only, whenever other
roles or application privileges are unavailable

Relates: #85312
elasticsearchmachine pushed a commit that referenced this pull request Apr 1, 2022
@tvernum
Ignore app priv failures when resolving superuser (#85519) (#85587)
ccfc925 
In #81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve those
other roles would not block superuser access - when a user has superuser
role, any failures in role resolution will be effectively ignored, and
the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only, whenever other
roles or application privileges are unavailable

Relates: #85312
@williamrandolph williamrandolph mentioned this pull request May 25, 2022
Open
robb3rt pushed a commit that referenced this pull request Jul 11, 2022
added warning for required privileges
fd8e1fe 
Adjusted the document to ensure alignment and correctness after following changes were made: #81400
and adjusted for proposed solution mentioned here: #81451 (comment)
@robb3rt robb3rt mentioned this pull request Jul 11, 2022
Open
@Cpt-Falcon
Copy link

Cpt-Falcon commented Jul 12, 2022 •
edited
Loading

a

@greibrokka greibrokka mentioned this pull request Jul 20, 2022
Closed
@RobsonSutton RobsonSutton mentioned this pull request Jul 23, 2022
Open
@makelicious makelicious mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ywangd ywangd ywangd approved these changes

Assignees
No one assigned
Labels
>breaking :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.0.0-rc2 v8.1.0 WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@tvernum @elasticmachine @joshdover @jportner @Cpt-Falcon @ywangd @pugnascotia @elasticsearchmachine