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 support for SearchTemplateRequest while resolving request #1678

Closed
shikharj05 opened this issue Mar 15, 2022 · 7 comments · Fixed by opensearch-project/OpenSearch#9122
Closed
Assignees
Labels
enhancement New feature or request help wanted Community contributions are especially encouraged for these issues. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@shikharj05
Copy link
Contributor

shikharj05 commented Mar 15, 2022

Security plugin doesn't support SearchTemplateRequest while resolving request/index permissions

This causes permissions to be evaluated against all(*) here- https://github.com/opensearch-project/security/blob/1.1/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java#L310-L312
Hence, a user/role with permissions limited to specific indices sees 403s when a search template request is used.

Currently available workaround is to use the following permissions-

  1. indices:data/read/search/template action for * (all indices)
  2. read action for specific index

Example request-

POST /<index>/_search/template
{<template>}

logs-

[] Resolve aliases, indices and types from SearchTemplateRequest
[] getOrReplaceAllIndices() for class org.opensearch.script.mustache.SearchTemplateRequest
[] class org.opensearch.script.mustache.SearchTemplateRequest not supported (It is likely not a indices related request)
[] Finally resolved for SearchTemplateRequest: Resolved [aliases=[*], allIndices=[*], types=[*], originalRequested=[*], remoteIndices=[]]
[] RequestedResolved : Resolved [aliases=[*], allIndices=[*], types=[*], originalRequested=[*], remoteIndices=[]]
@shikharj05 shikharj05 added the enhancement New feature or request label Mar 15, 2022
@peternied peternied added the help wanted Community contributions are especially encouraged for these issues. label Apr 1, 2022
@anubisg1
Copy link

@anubisg1
Copy link

anubisg1 commented Sep 29, 2022

in the specific my role has the following permissions

indexes: network-* , filebeat-* 
permission: read

do_not_fail_on_forbidden: true is also configured

when loggin in, the api call

api/saved_objects/_find?fields=title&per_page=1&search=*&search_fields=title&type=index-pattern

returns with:

{"statusCode":403,"error":"Forbidden","message":"no permissions for [indices:data/read/search] and User ...

adding the following permissions does nothing.

index: *
permissions: 
    - "indices:data/read/search/template"
    - "indices:data/read/msearch/template"

the only way i can get some progress is by granting:

index: *
permission: indices:data/read/search 

in that case, the previous call succeed

and returns

{
	"page": 1,
	"per_page": 1,
	"total": 4,
	"saved_objects": [{
		"type": "index-pattern",
		"id": "network-*",
		"attributes": {
			"title": "network-*"
		},
		"references": [],
		"migrationVersion": {
			"index-pattern": "7.6.0"
		},
		"updated_at": "2022-09-29T10:44:29.857Z",
		"version": "Wzk3LDdd",
		"namespaces": ["default"],
		"score": 1
	}]
}

unfortunately there are still problems as trying to access discover or dashboards

for the api call api/opensearch-dashboards/settings

the response is

{"statusCode":403,"error":"Forbidden","message":"no permissions for [indices:data/write/update] and User

for the api call /api/saved_objects/_bulk_get

the response is

{"statusCode":403,"error":"Forbidden","message":"no permissions for [] and User

the only way i can properly browse is to grant indices:data/read/search + get .
when the read permission is granted to * i obtain the following answer to the api call

api/saved_objects/_find?fields=title&per_page=1&search=*&search_fields=title&type=index-pattern

{
	"page": 1,
	"per_page": 10000,
	"total": 4,
	"saved_objects": [{
		"type": "index-pattern",
		"id": "network-*",
		"attributes": {
			"title": "network-*"
		},
		"references": [],
		"migrationVersion": {
			"index-pattern": "7.6.0"
		},
		"updated_at": "2022-09-29T10:44:29.857Z",
		"version": "Wzk3LDdd",
		"namespaces": ["default"],
		"score": 0
	}, {
		"type": "index-pattern",
		"id": "filebeat-*",
		"attributes": {
			"title": "filebeat-*"
		},
		"references": [],
		"migrationVersion": {
			"index-pattern": "7.6.0"
		},
		"updated_at": "2022-09-29T10:44:29.857Z",
		"version": "WzEwNCw3XQ==",
		"namespaces": ["default"],
		"score": 0
	}, {
		"type": "index-pattern",
		"id": "snmp-*",
		"attributes": {
			"title": "snmp-*"
		},
		"references": [],
		"migrationVersion": {
			"index-pattern": "7.6.0"
		},
		"updated_at": "2022-09-29T12:30:48.194Z",
		"version": "WzEwNyw3XQ==",
		"namespaces": ["default"],
		"score": 0
	}, {
		"type": "index-pattern",
		"id": "security-auditlog-*",
		"attributes": {
			"title": "security-auditlog-*"
		},
		"references": [],
		"migrationVersion": {
			"index-pattern": "7.6.0"
		},
		"updated_at": "2022-09-29T12:31:07.357Z",
		"version": "WzEwOCw3XQ==",
		"namespaces": ["default"],
		"score": 0
	}]
}

notice how a lot more indexes are returned compared to before

@davidlago davidlago added the triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. label Oct 10, 2022
@stephen-crawford
Copy link
Contributor

stephen-crawford commented Feb 27, 2023

[Triage 2/27/2023] This issue remains relevant and should continue to be a part of the backlog.

@kksaha
Copy link

kksaha commented Apr 17, 2023

Hello All,

Any update on this, I am facing the same issue. Even after adding the permission it's still throwing error
"no permissions for [indices:data/read/search"

@cwperks
Copy link
Member

cwperks commented Jun 27, 2023

I briefly looked at this issue and there's a complication in implementing a fix for this because these actions live in modules/lang-mustache of core instead of directly in :server. The fix for this involves adding a new block in IndexResolverReplacer.getOrReplaceAllIndices like this:

if (request instanceof SearchTemplateRequest) {
    provider.provide(((SearchTemplateRequest) request).getRequest().indices(), request, false);
}

But since SearchTemplateRequest is defined in a module of core rather than directly in server the call for request instanceof SearchTemplateRequest returns false since the request instance was created by a different class loader than the class loader of the security plugin. Security plugin gets an indirect dependency on lang-mustache-client through the high level rest client. See both jars in the screenshot below:

Screenshot 2023-06-27 at 10 53 52 AM

When trying to cast the request to SearchTemplateRequest in the security plugin the following error is thrown:

[2023-06-27T15:01:02,400][ERROR][o.o.s.f.SecurityFilter   ] [opensearch-node1] Unexpected exception java.lang.ClassCastException: class org.opensearch.script.mustache.SearchTemplateRequest cannot be cast to class org.opensearch.script.mustache.SearchTemplateRequest (org.opensearch.script.mustache.SearchTemplateRequest is in unnamed module of loader java.net.FactoryURLClassLoader @42f22995; org.opensearch.script.mustache.SearchTemplateRequest is in unnamed module of loader java.net.FactoryURLClassLoader @6ef1a1b9)

java.lang.ClassCastException: class org.opensearch.script.mustache.SearchTemplateRequest cannot be cast to class org.opensearch.script.mustache.SearchTemplateRequest (org.opensearch.script.mustache.SearchTemplateRequest is in unnamed module of loader java.net.FactoryURLClassLoader @42f22995; org.opensearch.script.mustache.SearchTemplateRequest is in unnamed module of loader java.net.FactoryURLClassLoader @6ef1a1b9)

@cwperks
Copy link
Member

cwperks commented Jun 27, 2023

I think a solution to the issue described above is just to add SearchTemplateAction.NAME inside PrivilegesEvaluator.isClusterPerm.

Inside the Transport Action of SearchTemplateAction it performs a search request where the authorization can be checked there to ensure the user has permissions to search on the requested indices: https://github.com/opensearch-project/OpenSearch/blob/main/modules/lang-mustache/src/main/java/org/opensearch/script/mustache/TransportSearchTemplateAction.java#L83-L111

For SearchTemplateRequest the authorization could be: 1) Can user do a SearchTemplateRequest (cluster permission) and 2) Can the user search on the requested indices?

@peternied
Copy link
Member

Addressing SearchTemplates feels like it will create more complexity around permissions in areas that are already difficult. I feel like this will expose more issues or create unclear user expectations.


Some of my thought process

Who can perform write operations on search templates?

Search templates are cluster wide, there are option to target specific indices. Embedded in the template is index targetting information.

Cluster level permissions are needed to cover the broad nature of these items. While viewing / modifing indices isn't the same kind of permission to the resource its strange to combine the two.

Who can perform get operations on search templates?

Search indices can be found cluster wide or focused on indices. We've got behavoir such as do not fail on forbidden, that could seemly allow returning a subset of responses filtered by matching index - that makes user comprehension hard.

Cluster level permissions, as it would be strange for read operations to have a behavoiral permission declaration that its write counterpart.

Who can search with a template?

Since authoring and reading templates is restricted at the cluster level, it seems that you need those cluster level permissions to ensure the correct - broad scope of what could be within templates.

Cluster level permissions are required.

This seems really restrictive, why so restrictive?

OpenSearch consider divulging document field name secured by FLS a vunerablity. The potential of exposing fields such as this through a search template exists and it seems non-trivial to mitigate.

willyborankin pushed a commit that referenced this issue Aug 16, 2023
…quest Auth (#2921)

Adds integration test to verify change in core, allowing proper
authorization of search template request

related to: #1678 


### Description
[Describe what this change achieves]
* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)
* Why these changes are required?
* What is the old behavior before changes and new behavior after
changes?

### Issues Resolved
[List any issues this PR will resolve]

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 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.

7 participants