-
Notifications
You must be signed in to change notification settings - Fork 500
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
Adds documentation for permission that provides access to system indexes #4849
Conversation
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
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 putting this PR together @cwillum. Left some comments around the permission name
@@ -65,6 +65,16 @@ Rather than individual permissions, you can often achieve your desired security | |||
{: .tip } | |||
|
|||
|
|||
### System permission | |||
|
|||
The system permission `system:admin/system_index` is unique among other permissions in that it extends some traditional admin-only accessibility to non-admin users. This permission gives normal users the ability to modify system indexes for the cluster. It excludes, however, access to the security system index `.opendistro_security`, which is used to store the configuration YAML files and remains accessible only to admins with an admin certificate. |
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.
This sounds like the permission itself is system:admin/system_index
however system_index
here is actually a placeholder text that should be replaced by the actual name of the system index i.e. system:admin/.opendistro-anomaly-detectors
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 pointing this out. I didn't pick this up from my review of materials.
|
||
The system permission `system:admin/system_index` is unique among other permissions in that it extends some traditional admin-only accessibility to non-admin users. This permission gives normal users the ability to modify system indexes for the cluster. It excludes, however, access to the security system index `.opendistro_security`, which is used to store the configuration YAML files and remains accessible only to admins with an admin certificate. | ||
|
||
Admin users that have the permission `restapi:admin/roles` are able to map the `system:admin/system_index` permission to users just as they would for a cluster or index permission. However, to preserve some control over this permission, the configuration setting `plugins.security.system_indices.additional_control.enabled` allows administrators to disable this permission by setting it to `false`. For more information about this setting, see [Enabling user access to system indexes]({{site.url}}{{site.baseurl}}/security/configuration/yaml/#enabling-user-access-to-system-indexes). |
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.
nit: ... disable this feature ...
. Thoughts?
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.
Agreed.
|
||
Admin users that have the permission `restapi:admin/roles` are able to map the `system:admin/system_index` permission to users just as they would for a cluster or index permission. However, to preserve some control over this permission, the configuration setting `plugins.security.system_indices.additional_control.enabled` allows administrators to disable this permission by setting it to `false`. For more information about this setting, see [Enabling user access to system indexes]({{site.url}}{{site.baseurl}}/security/configuration/yaml/#enabling-user-access-to-system-indexes). | ||
|
||
Keep in mind that an admin user who enables this feature necessarily accepts the risks involved with giving normal users access to system indexes, which may contain sensitive information and configurations essential to a cluster's health. |
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.
We should also mention that users with permission to modify roles restapi:admin/roles
can essentially grant themselves or any other user in cluster access to system indices. However, these permissions will have to be individually granted. They cannot be granted as a *
permission. i.e. system:admin/*
is not a valid permission.
Also, .opendistro_security
index is not permission-able.
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.
Right, covered both these points in current text. Thanks
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
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.
@DarshitChanpura Thanks for the early review. I've addressed your comments. I'll wait to circulate this for documentation team review until Security confirms this is going into 2.10.
@@ -65,6 +65,16 @@ Rather than individual permissions, you can often achieve your desired security | |||
{: .tip } | |||
|
|||
|
|||
### System permission | |||
|
|||
The system permission `system:admin/system_index` is unique among other permissions in that it extends some traditional admin-only accessibility to non-admin users. This permission gives normal users the ability to modify system indexes for the cluster. It excludes, however, access to the security system index `.opendistro_security`, which is used to store the configuration YAML files and remains accessible only to admins with an admin certificate. |
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 pointing this out. I didn't pick this up from my review of materials.
|
||
The system permission `system:admin/system_index` is unique among other permissions in that it extends some traditional admin-only accessibility to non-admin users. This permission gives normal users the ability to modify system indexes for the cluster. It excludes, however, access to the security system index `.opendistro_security`, which is used to store the configuration YAML files and remains accessible only to admins with an admin certificate. | ||
|
||
Admin users that have the permission `restapi:admin/roles` are able to map the `system:admin/system_index` permission to users just as they would for a cluster or index permission. However, to preserve some control over this permission, the configuration setting `plugins.security.system_indices.additional_control.enabled` allows administrators to disable this permission by setting it to `false`. For more information about this setting, see [Enabling user access to system indexes]({{site.url}}{{site.baseurl}}/security/configuration/yaml/#enabling-user-access-to-system-indexes). |
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.
Agreed.
|
||
Admin users that have the permission `restapi:admin/roles` are able to map the `system:admin/system_index` permission to users just as they would for a cluster or index permission. However, to preserve some control over this permission, the configuration setting `plugins.security.system_indices.additional_control.enabled` allows administrators to disable this permission by setting it to `false`. For more information about this setting, see [Enabling user access to system indexes]({{site.url}}{{site.baseurl}}/security/configuration/yaml/#enabling-user-access-to-system-indexes). | ||
|
||
Keep in mind that an admin user who enables this feature necessarily accepts the risks involved with giving normal users access to system indexes, which may contain sensitive information and configurations essential to a cluster's health. |
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.
Right, covered both these points in current text. Thanks
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.
Hey @cwillum I apologize for confusion around system index permission setup. I've added some comments that hopefully clarify the feature.
_security/configuration/yaml.md
Outdated
```yml | ||
plugins.security.system_indices.additional_control.enabled: true | ||
``` | ||
When set to `false`, the permission is disabled and only admins with an admin certificate can make changes to system indexes. By default, the setting is `true` for a new cluster. |
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.
Be default, the setting is false. Refer here:
https://github.com/opensearch-project/security/blob/d77466418d3dfe551ca1ead20fcb4e6fda88515e/src/main/java/org/opensearch/security/support/ConfigConstants.java#L319
_security/configuration/yaml.md
Outdated
``` | ||
When set to `false`, the permission is disabled and only admins with an admin certificate can make changes to system indexes. By default, the setting is `true` for a new cluster. | ||
|
||
To learn more about the `system:admin/<system_index_name>` permission, see [System permission]({{site.url}}{{site.baseurl}}/security/access-control/permissions/#system-permission). |
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.
Thoughts on: To learn more about the System Index Permissions, see ..
|
||
The system permission `system:admin/<system_index_name>` is unique among other permissions in that it extends some traditional admin-only accessibility to non-admin users. This permission gives normal users the ability to modify the system index specified in the permission name. For example, the permission `system:admin/.opendistro-alerting-config` gives a user permission to modify the system index that stores configurations for the Alerting plugin. | ||
|
||
The system permission excludes, however, access to the security system index `.opendistro_security`, which is used to store Security's configuration YAML files and remains accessible only to admins with an admin certificate. |
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.
Thoughts on: "The system permission, however, excludes access to the .opendistro_security
system index, which stores the Security plugin's configuration, and is only accessible to admins with an admin certificate."
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
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.
@DarshitChanpura Thanks for reviewing this part way through. I've addressed the comments. Please have another look when you get a chance. As always, thank you.
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
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.
@peternied Thanks for the review. I've addressed your comments. Will wait to push this along further until I get word from Security that the feature is merged.
|
||
* Specifying the full name of a system index limits access to that index alone: `.opendistro-alerting-config`. | ||
* Specifying the prefix and a partial name for a system index provides access to all system indexes that begin with the name: `.opendistro-anomaly-detector*`. | ||
* Using `.*` is effectively the same as specifying the prefix with wildcard, as described in the previous point. This gives access to all system indexes that begin with a `.`. |
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.
@peternied Thanks for pointing this out. I've reworded the points above to bring guidance in line with this clarification. Is the additional wildcard necessary under allowed_actions
only when you want to specify all system indexes under index_patterns
?
One other nitpick. Are values for this configuration wrapped in single quotes or double quotes? Or doesn't it matter? I've seen both.
* Using `.*` is effectively the same as specifying the prefix with wildcard, as described in the previous point. This gives access to all system indexes that begin with a `.`. | ||
* Entering the wildcard `*` by itself does not give access to any indexes. | ||
|
||
Use extreme caution when using the wildcard to configure access to system indexes. We highly recommend thinking ahead and anticipating the range of access that you will be extending to users before updating your configuration files. |
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.
@peternied Good perspective on that. I removed that caution (seemed redundant, anyhow) and replaced it with a "tip" about using the GET /_cat/indices/<index>
operation to verify your index pattern. Thank you for that.
@cwillum There have been changes in the feature flag name. I've addresses those here: https://github.com/opensearch-project/documentation-website/pull/4953/files. Please review it |
* Changes the feature flag key and updates description of a consideration Signed-off-by: Darshit Chanpura <[email protected]> * Addresses source PR feedback Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]>
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.
all updates have been addressed. LGTM!
Security PR is merged: opensearch-project/security#3325 |
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
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.
@cwillum Please see my comments and changes and let me know when addressed so that I can read again and approve. Thanks!
|
||
Users that have the permission [`restapi:admin/roles`]({{site.url}}{{site.baseurl}}/security/access-control/api/#access-control-for-the-api) are able to map system index permissions to all users in the same way they would for a cluster or index permission in the `roles.yml` file. However, to preserve some control over this permission, the system index permission feature is disabled by default and allows administrators to enable this feature by setting the configuration `plugins.security.system_indices.permissions.enabled` to `true`. For more information about this setting, see [Enabling user access to system indexes]({{site.url}}{{site.baseurl}}/security/configuration/yaml/#enabling-user-access-to-system-indexes). | ||
|
||
Keep in mind that an admin user who enables this feature necessarily accepts the risks involved with giving normal users access to system indexes, which may contain sensitive information and configurations essential to a cluster's health. An admin user should also take precautions when assigning `restapi:admin/roles` to users because this permission gives a user not only the ability to assign the system index permission to another user but also the ability to self-assign access to any system index. |
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.
Keep in mind that an admin user who enables this feature necessarily accepts the risks involved with giving normal users access to system indexes, which may contain sensitive information and configurations essential to a cluster's health. An admin user should also take precautions when assigning `restapi:admin/roles` to users because this permission gives a user not only the ability to assign the system index permission to another user but also the ability to self-assign access to any system index. | |
Keep in mind that an admin user who enables this feature necessarily accepts the risks involved with giving normal users access to system indexes, which may contain sensitive information and configurations essential to a cluster's health. An admin user should also take precautions when assigning the `restapi:admin/roles` permission to users because it provides not only the ability to assign the system index permission to another user but also the ability to self-assign access to any system index. |
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
Signed-off-by: cwillum <[email protected]>
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.
@natebower I've addressed your comments. At the ready to make any last changes if need be. Thanks.
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.
LGTM!
Signed-off-by: cwillum <[email protected]>
…xes (opensearch-project#4849) * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * Fixes some wordings around system index permission access and usage (opensearch-project#4948) * Fixes some wordings around system index permission access and usage Signed-off-by: Darshit Chanpura <[email protected]> * Addresses PR feedback Signed-off-by: Darshit Chanpura <[email protected]> * Addresses more PR feedback Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * Changes the feature flag key and updates some text (opensearch-project#4953) * Changes the feature flag key and updates description of a consideration Signed-off-by: Darshit Chanpura <[email protected]> * Addresses source PR feedback Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> --------- Signed-off-by: cwillum <[email protected]> Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]>
…xes (#4849) * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * Fixes some wordings around system index permission access and usage (#4948) * Fixes some wordings around system index permission access and usage Signed-off-by: Darshit Chanpura <[email protected]> * Addresses PR feedback Signed-off-by: Darshit Chanpura <[email protected]> * Addresses more PR feedback Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * Changes the feature flag key and updates some text (#4953) * Changes the feature flag key and updates description of a consideration Signed-off-by: Darshit Chanpura <[email protected]> * Addresses source PR feedback Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> * fix#4736 system index permission Signed-off-by: cwillum <[email protected]> --------- Signed-off-by: cwillum <[email protected]> Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]>
Description
Document a new permission that allows normal users to modify system indexes.
Issues Resolved
Documents the new permission and the setting that enables or disables its functionality.
Fixes #4736
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.