-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[Security] Add entity store and asset criticality index privileges to built in roles #129662
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
[Security] Add entity store and asset criticality index privileges to built in roles #129662
Conversation
|
Pinging @elastic/es-security (Team:Security) |
|
Hi @opauloh, I've created a changelog YAML for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a little more context on this?
This appears to grant access to all security entities and asset criticality indices across all spaces, is this definitely what we want?
Does this need to go into 9.1? The feature freeze is today
...core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java
Outdated
Show resolved
Hide resolved
| // Security - Entity Store is view only | ||
| RoleDescriptor.IndicesPrivileges.builder() | ||
| .indices(ReservedRolesStore.ENTITY_STORE_V1_LATEST_INDEX) | ||
| .privileges("read", "view_index_metadata") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double check - Serverless Editor appears to have write access to these indices - is this difference intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bringing it to my attention!
I will tag @hop-dev and @jaredburgettelastic to help on that. Mark / Jared, from what I could see, users don't really need to write directly into the Entity Store index, but do you happen to know a use case where they would need? Like running the API to clean the Entity Store or something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I think this was a mistake in the original PR apologies!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for confirming @hop-dev, so @richard-dennehy, the changes on this PR are correct, we will follow up on reducing the extra permissions on Serverless editor role to keep it consistent!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@opauloh sorry I have just thought, because the transform runs as the user who enabled the entity store, I believe we do need write permissions? Have I got that wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially thought the same, but it turns out the Transform API doesn't require write privilege.
Required authorization
Index privileges: create_index,read,index,view_index_metadata
Cluster privileges: manage_transform
I guess the rationale behind this is that users shouldn't be writing to the transform destination indices directly anyway?
That's also accurate with the Entity Store permission page. (No write access required to the .entities.v1.latest.* index)
Also, by design, Editors wouldn't have the "manage" privilege, so Editors users won't likely be the ones to enable Entity Store / Asset Inventory, unless extra permissions are granted:
I.e screenshot from the permissions screen when accessing the Entity Store with an Editor user:
However, editors should be able to read entity store data and update asset criticality once it's enabled.
The only use case I can imagine writing directly would be handy is, if we had to update data directly on the destination index using the user permission to update data without waiting for the transforms to run, however, I didn't find anything performing that operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed investigation, that's interesting about the transform write permissions! 👍
Yes, the Entity Store and Asset Inventory features are cross-space functionality, and built-in roles should have access to them as they have the Kibana "All spaces" permission by default.
Not necessarily, the main reason to include it now was to grant users less friction access to the new "Asset Inventory" feature in Kibana that's releasing on 9.1, which relies on Asset Criticality and Entity Store that were released in previous versions, however, since Inventory will be in Technical Preview for 9.1 is not mandatory to include the permissions, but it will be for 9.2 when the "Asset Inventory" features goes GA. |
Updates the entity store index pattern to ensure it matches the minimum necessary index name and narrow it down to the correct use case
Modifies the reserved role descriptor to allow all privileges on the entities index. Adds a test to verify that the entities index has all access allowed.
Corrects a typo in a test case by replacing `Array` with `Arrays`, ensuring the test functions as intended.
|
Heya @opauloh ! I see you're changing the Just a couple questions:
|
|
Hi @kc13greiner, thanks for those questions
Sure, I will use the user Entity store to exemplify. User entity store source it's data from multiple indices, which are processed by a pivot transform where outputs the aggregated data into the final index
In this scenario, Editor users of the Entity Store experience wants to have the ability to change the Criticality and Risk scoring, and for this it's needed write permission to the Why add 'write' permission to the `.entities.v1.latest.security index to the Kibana System and not to Editor / Viewer?* We started to improve the UX for the users, users visualize data directly from
Good point! After reviewing it, all we need for the mentioned improvement is to add "write" permission on top of the "read" permission, I will update it accordingly, the idea of granting "all" was to prepare for future use cases that are in discussion like migration and backfilling of Entity Store data, this means we will surely need extra permissions when the time comes, but unlike the use case above we don't know exactly when, so I agree it's a safer choice to add more permissions as needed and only the exact required permissions. |
Reduces the privileges granted to the Kibana owned reserved role for the .entities indices to only read and write. This change restricts the role from having "all" privileges, enhancing security.
|
cc @kc13greiner, forgot I had auto-merged enabled, so it merged as soon as I updated the permission. Let me know if everything is clear |
… built in Editor and Viewer roles (elastic#129662) * Adding asset criticality and entity store permissions to built in roles * Update docs/changelog/129662.yaml * [CI] Auto commit changes from spotless * Corrects entity store index pattern Updates the entity store index pattern to ensure it matches the minimum necessary index name and narrow it down to the correct use case --------- Co-authored-by: elasticsearchmachine <[email protected]>
|
Thank you for the excellent overview! I agree that it makes sense in this case to add And thanks for dialing back LGTM! |
… built in Editor and Viewer roles (elastic#129662) * Adding asset criticality and entity store permissions to built in roles * Update docs/changelog/129662.yaml * [CI] Auto commit changes from spotless * Corrects entity store index pattern Updates the entity store index pattern to ensure it matches the minimum necessary index name and narrow it down to the correct use case --------- Co-authored-by: elasticsearchmachine <[email protected]>

Summary
Currently, the editor and viewer built-in roles do not contain the appropriate Entity Store and Asset Criticality index privileges for Security users.
This PR updates the index privileges to include the
.entities.v1.latest.security*and.asset-criticality.asset-criticality-*indices with the appropriate permission:Viewer
.entities.v1.latest.security*.asset-criticality.asset-criticality-*Editor
.entities.v1.latest.security*(Users don't write directly at entity store indices).asset-criticality.asset-criticality-*Kibana system