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

[DOC] Getting exception while indexing document with user having role with document level security enabled #1273

Closed
swapnilsvaidya opened this issue Jun 19, 2022 · 21 comments · Fixed by #7668
Assignees
Labels
3 - Done Issue is done/complete bug Technical problem with the doc site or broken link security

Comments

@swapnilsvaidya
Copy link

swapnilsvaidya commented Jun 19, 2022

What is the bug?
A clear and concise description of the bug.
I have created a user which is mapped with role having document level security enabled.
If I try to index the document through Bulkrequest using this user I get following exception:

 [ElasticsearchException[Elasticsearch exception [type=security_exception, reason=_opendistro_security_dls_query does not match (SG 900D)]]]

In OpenSearch logs, following is the exception logged.
resolved: Resolved [aliases=[], allIndices=[test_index], types=[*], originalRequested=[test_index], remoteIndices=[]]
mode: ADAPTIVE
[2022-06-19T05:44:05,351][DEBUG][o.o.s.c.DlsQueryParser   ] [node-1] containsTermLookupQuery() returns false
queries: [{"term": { "pminstanceid.keyword": "instance1"}}]
[2022-06-19T05:44:05,352][DEBUG][o.o.s.c.DlsFlsValveImpl  ] [node-1] Doing lucene-level DLS because the query does not contain a TLQ
[2022-06-19T05:44:05,352][DEBUG][o.o.s.f.SecurityFilter   ] [node-1] Failed to apply filter. Task id: 224 (requests[1], index[test_index][0]). Action: indices:data/write/bulk[s]
org.opensearch.OpenSearchSecurityException: _opendistro_security_dls_query does not match (SG 900D)
	at org.opensearch.security.configuration.DlsFlsValveImpl.setDlsHeaders(DlsFlsValveImpl.java:390) ~[opensearch-security-2.0.1.0.jar:2.0.1.0]
	at org.opensearch.security.configuration.DlsFlsValveImpl.invoke(DlsFlsValveImpl.java:157) ~[opensearch-security-2.0.1.0.jar:2.0.1.0]
	at org.opensearch.security.filter.SecurityFilter.apply0(SecurityFilter.java:320) [opensearch-security-2.0.1.0.jar:2.0.1.0]
	at org.opensearch.security.filter.SecurityFilter.apply(SecurityFilter.java:157) [opensearch-security-2.0.1.0.jar:2.0.1.0]
	at org.opensearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:202) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.action.support.TransportAction.execute(TransportAction.java:174) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.action.support.TransportAction.execute(TransportAction.java:102) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.action.bulk.TransportBulkAction$BulkOperation.doRun(TransportBulkAction.java:632) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.action.bulk.TransportBulkAction.executeBulk(TransportBulkAction.java:774) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.action.bulk.TransportBulkAction$1$1.doRun(TransportBulkAction.java:321) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:798) [opensearch-2.0.1.jar:2.0.1]
	at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52) [opensearch-2.0.1.jar:2.0.1]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
	at java.lang.Thread.run(Thread.java:833) [?:?]

After looking into the code, it seems DlsFlsValveImpl is trying to read the dls query related header from the request.
------Code snipper from DlsFlsValveImpl -----

if (threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_DLS_QUERY_HEADER) != null) {
                    Object deserializedDlsQueries = Base64Helper.deserializeObject(threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_DLS_QUERY_HEADER));
                    if (!dlsQueries.equals(deserializedDlsQueries)) {                        
                        throw new OpenSearchSecurityException(ConfigConstants.OPENDISTRO_SECURITY_DLS_QUERY_HEADER + " does not match (SG 900D)");
                    }
                }

However I think this header should not come into picture while indexing the document.
This seems a bug.

How can one reproduce the bug?
Steps to reproduce the behavior:
Create a Role with DLS enabled, associate this role with user. Index the document using bulk request and using this user.
Verify bulkresponse

What is the expected behavior?
While indexing DLS should be ignored

What is your host/environment?

  • OS: [e.g. iOS] All OS
  • Version [e.g. 22]
  • Plugins

Do you have any screenshots?
NA

Do you have any additional context?
NA

@swapnilsvaidya swapnilsvaidya added bug Technical problem with the doc site or broken link untriaged labels Jun 19, 2022
@swapnilsvaidya
Copy link
Author

@peternied Hi Peter, Could you please confirm if you are looking into this issue?

@peternied
Copy link
Member

Hi @swapnilsvaidya I am not currently looking into this issue, and it has been added to our work queue. If you would like to create a pull request the maintainers and I would gladly review it.

@rursprung
Copy link
Contributor

i haven't seen the corresponding comment in the OpenSearch Security documentation, but e.g. Search Guard explicitly declares that roles with DLS enabled mustn't have write access: https://docs.search-guard.com/latest/document-level-security#dls-and-write-access

i presume the same applies for OpenSearch Security and needs to be documented (and maybe enforced by a check in a new major release?)

@cliu123
Copy link
Member

cliu123 commented Aug 29, 2022

@opensearch-project/transfer-request This looks a documentation issue. Could you transfer to documentation repo? Thanks!

@rursprung
Copy link
Contributor

@cliu123: seems that your transfer request hasn't happened yet. would it make sense to instead raise a new, well described, issue in the documentation repo and then close this one here with a reference to the new one?

@peternied peternied transferred this issue from opensearch-project/security Sep 20, 2022
@davidlago
Copy link

In the right repo now.

@Naarcha-AWS Naarcha-AWS added 1 - Backlog Issue: The issue is unassigned or assigned but not started security and removed untriaged labels Dec 15, 2022
@hdhalter
Copy link
Contributor

hdhalter commented Mar 8, 2023

Hi Chris, can you please follow up with the security team to see what needs to be documented here? Thanks.

@peternied
Copy link
Member

@cwillum A little more context, when users have a DLS/FLS rule assigned to them, this is in affect saying "This user XYZ should never see documents with these value(s) or be able to see these fields". If that same user has permissions to add/modify documents in an index with DLS/FLS rules it directly contradicts the cluster security configuration.

@hdhalter
Copy link
Contributor

Hi @peternied , can you please help make the update to the file, and is the appropriate file https://github.com/opensearch-project/documentation-website/blob/main/_security/access-control/document-level-security.md? Thanks!

@hdhalter hdhalter removed the 1 - Backlog Issue: The issue is unassigned or assigned but not started label Dec 19, 2023
@AntonEliatra
Copy link
Contributor

@hdhalter I can look at this one

@hdhalter hdhalter added the 2 - In progress Issue/PR: The issue or PR is in progress. label Jun 6, 2024
@hdhalter hdhalter changed the title [BUG] Getting exception while indexing document with user having role with document level security enabled [DOC] Getting exception while indexing document with user having role with document level security enabled Jun 6, 2024
@AntonEliatra
Copy link
Contributor

@hdhalter not able to reproduce this issue with the OS 2.13, seems this was updated in code, as user is able to index with role which has DLS and upon query only receive relevant results based on the DLS configured

@hdhalter
Copy link
Contributor

Thanks, @AntonEliatra ! I am closing this issue.

@hdhalter hdhalter added 3 - Done Issue is done/complete and removed 2 - In progress Issue/PR: The issue or PR is in progress. labels Jun 26, 2024
@rursprung
Copy link
Contributor

if this behaviour changed (@peternied: was that intentional?) it should nevertheless be documented what needs to be looked out for and what can/cannot be done now.

@AntonEliatra
Copy link
Contributor

Same behaviour is seen in OS 1.3, using DLS and FLS, user able to index any information, but is only able to retrieve the details that are permitted via DLS/FLS.
I can add this to the documentation for additional clarity if necessary

@rursprung
Copy link
Contributor

then i don't think that anything changed and that the rule still is that there must be no DLS if you're indexing data (but this has to be confirmed by the security plugin team).

@hdhalter: please re-open the issue

@AntonEliatra
Copy link
Contributor

@rursprung but the user is able to index with DLS/FLS enabled, I don't follow

@hdhalter hdhalter reopened this Jun 27, 2024
@hdhalter
Copy link
Contributor

@peternied , @scrawfor99

@AntonEliatra
Copy link
Contributor

By design, there should be nothing preventing the user from indexing anything, the DLS/FLS is only applied during search/retrieval of data

@stephen-crawford
Copy link
Contributor

Based on the documentation, I think @AntonEliatra is correct. If someone wants to share the configuration they were able to reproduce the issue with then I can try to help them debug the issue. Otherwise, I think things are correct as is and suspect this was a configuration issue at the time of its filing.

@rursprung
Copy link
Contributor

note that we never had an exception (it wasn't us raising the issue) but we were very clearly told by floragunn (CC @nibix) that we should never use a user with a DLS role to index data (and it's still in their Search Guard documentation and since SG & OS-security are closely related the reasoning & behaviour should be the same?).

@nibix
Copy link

nibix commented Jul 1, 2024

@rursprung

but we were very clearly told by floragunn (CC @nibix) that we should never use a user with a DLS role to index data (and it's still in their Search Guard documentation and since SG & OS-security are closely related the reasoning & behaviour should be the same?).

So, IMHO, the situation is like this:

  • There's indeed no code which would prevent you from creating a user with DLS restrictions on an index, which is at the same time able to write into that index. Neither in OpenSearch security, nor in Search Guard.
  • If one creates such a user, that user will be able to write new documents which they won't be able to retrieve (due to the DLS rule). Additionally, if they know the IDs of existing documents, they also can overwrite documents they are not allowed to see (disclaimer: untested)
  • This is at least a contradiction to common security intuitions
  • To keep things simple, the Search Guard docs recommend not to give write privileges to users with DLS restrictions on the same index: https://docs.search-guard.com/latest/document-level-security#dls-and-write-access
  • I don't want to say that there is no use-case at all for using the combination of DLS with write privileges. Yet, I have yet to see one. I would also guess that this will be a 0.01% niche case - thus, in order to keep the docs simple for the 99.99% of users, I do believe that this plain recommendation is sufficient.
  • Additionally, as this is such a niche case, the test coverage for this combination is likely extremely low. Thus, it is likely to encounter bugs like the one above.
  • BTW, just my 5ct to the originally mentioned exception: I think that's a race condition - thus it will be difficult to reproduce.

AntonEliatra added a commit to AntonEliatra/documentation-website that referenced this issue Jul 10, 2024
Naarcha-AWS added a commit that referenced this issue Jul 11, 2024
* Adding DLS with write permission recommendation #1273

Signed-off-by: AntonEliatra <[email protected]>

* Update _security/access-control/document-level-security.md

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

---------

Signed-off-by: AntonEliatra <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this issue Jul 11, 2024
* Adding DLS with write permission recommendation #1273

Signed-off-by: AntonEliatra <[email protected]>

* Update _security/access-control/document-level-security.md

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

---------

Signed-off-by: AntonEliatra <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
(cherry picked from commit 8fffcbc)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
leanneeliatra pushed a commit to leanneeliatra/opensearch-documentation-website-forl that referenced this issue Jul 24, 2024
… (opensearch-project#7668)

* Adding DLS with write permission recommendation opensearch-project#1273

Signed-off-by: AntonEliatra <[email protected]>

* Update _security/access-control/document-level-security.md

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

---------

Signed-off-by: AntonEliatra <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: [email protected] <[email protected]>
sandervandegeijn pushed a commit to sandervandegeijn/documentation-website that referenced this issue Jul 30, 2024
… (opensearch-project#7668)

* Adding DLS with write permission recommendation opensearch-project#1273

Signed-off-by: AntonEliatra <[email protected]>

* Update _security/access-control/document-level-security.md

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

---------

Signed-off-by: AntonEliatra <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: Sander van de Geijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done Issue is done/complete bug Technical problem with the doc site or broken link security
Projects
None yet
Development

Successfully merging a pull request may close this issue.