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

[BUG] do_not_fail_on_forbidden_empty does not work for cat api #1815

Closed
jezsy opened this issue May 2, 2022 · 4 comments · Fixed by #3236
Closed

[BUG] do_not_fail_on_forbidden_empty does not work for cat api #1815

jezsy opened this issue May 2, 2022 · 4 comments · Fixed by #3236
Assignees
Labels
bug Something isn't working help wanted Community contributions are especially encouraged for these issues. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@jezsy
Copy link

jezsy commented May 2, 2022

What is the bug?
Setting do_not_fail_on_forbidden to true does not seem to have any impact on some APIs like cat/_indices and cat/_aliases - the whole operation is rejected even if indices exist for which the user has access to.

_cat/indices/some-index-* would work, but not _cat/indices because it include internal indices which the user does not permission for.

Adding permissions under index_patterns: '*' can make them work, but I think this defeats the purpose of having the do_not_fail_on_forbidden parameter.

How can one reproduce the bug?
Steps to reproduce the behavior:
roles.yml

my_user:
  cluster_permissions:
    - "cluster_monitor"
    - "cluster_manage_index_templates"
  index_permissions:
  - index_patterns:
    - "some-index-*"
    allowed_actions:
      - "read"
      - "write"
      - "create_index"
      - "manage"

config.yml

config:
  dynamic:
    do_not_fail_on_forbidden: true
    do_not_fail_on_forbidden_empty: true
    ...

Error for GET _cat/indices

        "type" : "security_exception",
        "reason" : "no permissions for [indices:monitor/settings/get] and User [name=my_user, backend_roles=[], requestedTenant=null]"

Error for GET _cat/aliases

        "type" : "security_exception",
        "reason" : "no permissions for [indices:admin/aliases/get] and User [name=my_user, backend_roles=[], requestedTenant=null]"

Both of these permissions should be covered under the manage default_action_group, but the operations only work when targeted specifically to the index, so do_not_fail_on_forbidden does not seem to be working as intended.

What is the expected behavior?
do_not_fail_on_forbidden should work similarly for all APIs, where results are filtered based only on the indices that the user has permissions for.

What is your host/environment?

  • Version OpenSearch 1.2.3
  • Plugins OpenSearch Security 1.2.3.0
@jezsy jezsy added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels May 2, 2022
@peternied peternied added help wanted Community contributions are especially encouraged for these issues. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels May 2, 2022
@peternied
Copy link
Member

peternied commented May 2, 2022

Thanks for filing @jezsy, I am having trouble reproducing the issue with /_search. As that data might be sensitive, could you email me directly at petern@amazon.com? Sending the security-auditlog-* log entries and dump of security configuration will be needed to dive into this further.

@jezsy
Copy link
Author

jezsy commented May 12, 2022

Hi @peternied. Apologies for the late response. Upon further tests, it seems my claim about /_search was wrong. kibana_1 was appearing in the results because I had my user mapped to the kibana_user role as well (was doing tests on the Dashboards Dev Tools), which had permissions for the kibana indices. So do_not_fail_on_forbidden works as intended for _search

In that case, the main issue now is just extending usage of parameters do_not_fail_on_forbidden and do_not_fail_on_forbidden_empty to other APIs like _cat (e.g. indices, aliases, shards, maybe more)

@jezsy
Copy link
Author

jezsy commented May 12, 2022

Similarly, I think the two parameters should also apply to GET _alias and GET _aliases. At the moment, the whole operation is rejected instead of just filtering for those indices which the user does have permission for, even if the parameters are configured true

@peternied
Copy link
Member

Thanks for updating us on this issue, we have added this issue into items to be looked at so thank you very much for contributing to this project by filing. Please feel free to open a pull request if you've got recommendations for this or other features.

@davidlago davidlago added the triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. label Oct 10, 2022
@derek-ho derek-ho self-assigned this Aug 24, 2023
RyanL1997 pushed a commit that referenced this issue Aug 29, 2023
…idden setting (#3236)

### Description
This change allows for DNFOF behavior on the _cat/_indices API. It adds
the required index permissions into the DNFOF regex to be picked up in
the DNFOF code path. Previously it was being skipped/returning 403,
since the index permissions were not in the regex.

### Issues Resolved
Fix: #1815

Is this a backport? If so, please add backport PR # and/or commits #

### Testing
[Please provide details of testing done: unit testing, integration
testing and manual testing]

### Check List
- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Derek Ho <dxho@amazon.com>
derek-ho added a commit to derek-ho/security that referenced this issue Aug 29, 2023
…idden setting (opensearch-project#3236)

This change allows for DNFOF behavior on the _cat/_indices API. It adds
the required index permissions into the DNFOF regex to be picked up in
the DNFOF code path. Previously it was being skipped/returning 403,
since the index permissions were not in the regex.

Fix: opensearch-project#1815

Is this a backport? If so, please add backport PR # and/or commits #

[Please provide details of testing done: unit testing, integration
testing and manual testing]

- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Derek Ho <dxho@amazon.com>
(cherry picked from commit 4c095d2)
Signed-off-by: Derek Ho <dxho@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Community contributions are especially encouraged for these issues. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants