[BUG] Can't use multiple roles with DLS permissions on the same Index

[sbeginCoveo]

What is the bug?
When a user is bound to two roles using DLS permissions on the same index, only one DLS policy is taken into account.
Each role with DLS permissions works independently as expected, but they won't combine in the typical additive fashion.

How can one reproduce the bug?
Steps to reproduce the behavior:
1: create a "global" role with READ on index pattern * and the following DLS:

{
  "bool": {
    "must_not": {
      "match": {
        "sensitive": true
      }
    }
  }
}

2: create a higher privileged role on any indices (example-*) with some catch-all DLS policy:

{
  "bool": {
    "must": {
      "match_all": {}
    }
  }
}

3: Query that example-* index pattern using the a User bound to both roles. Observe that you will not have any logs that contains "sensitive": true

What is the expected behavior?
The policies to be additive (combine each DLS statement in an OR query)

What is your host/environment?

• Provider: AWS Opensearch
• Version: OpenSearch 2.3
• Plugins: unsure

Do you have any additional context?
Add any other context about the problem.

The codebase seems to indicate selection of a single arbitrary DLS policy for each index:

security/src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java

Line 405 in ca4d752

String dlsEval = SecurityUtils.evalMap(filterLevelQueries, index);

security/src/main/java/org/opensearch/security/support/SecurityUtils.java

Line 98 in ca4d752

.findAny()

Same issue described here

[scrawfor99]

[Triage] Hi @sbeginCoveo, thank you for taking the time to file this issue. As a follow-up did you have any thoughts on how you would handle a negative and positive permission being "added?" What would that look like in the proposed solution. We are happy to review any pull requests to this issue.

[sbeginCoveo]

In theory, there should be no concept of a "negative" permission. As in adding a role to a user should not "remove" access to anything. Permissions should always be additive. What should exist, though, is a negative filter for permissions, such as matching logs that don't have some field.

For example here's a quick pseudo-code boolean "sum":

PermissionA = (! log.sensitive) # negative lookup non-sensitive access to all indicies
PermissionB = (true) # match_all privileged role for single index

Sum = PermissionA || PermissionB

I hope that clears up the confusion!

[scrawfor99]

Hi @sbeginCoveo, thank you for your prompt reply. That does clarify things. I can see how this would be preferable and the expected behavior. I am surprised it is not already haha. Would you be interested in making a contribution towards this effort? If so, I would be happy to help you along the way. If not, I will add a label for our college contributor initiative since it seems like this should be a pretty straightforward change.

[sbeginCoveo]

Thanks @scrawfor99 for the follow-up! I personally don't feel confident enough in Java to make a proper contribution. I also don't have the ide or the tooling in place to make the changes in a timely fashion

[scrawfor99]

Hi @sbeginCoveo, no worries. Thank you for letting me know. I am going to mark this as a good issue for a first time contributor and will leave some notes later today about what changes will be required. After that, the issue should be addressed before too long. If you need this change expedited, please follow-up here and I will try to find the time to fix it for you myself.

[scrawfor99]

Hi @sbeginCoveo, I have been taking a look at this for you but am having issues reproducing the issue. I have created two roles and assigned them to a user DlsTest without any other roles. I then went and created an index my_index1 and populated it with data. When I query the indices using the Query Workbench, I get the same results as an admin or as the new user.

Here are the two roles:

Here are the query results:

This looks to me that the Dls patterns are additive. It seems that the extra permissions from the HigherPriv role is working as expected. Perhaps, I have misunderstood the issue?

[peternied]

@scrawfor99 Is there a test case in our repository that proves this scenario works? I think that might be easier to have a PR with a reproduction of the issue to refine the scenario, what do you think?

[sbeginCoveo]

Since only one DLS statement is taken into account, it could be that the match_all is the one being used when "merging" the two roles. My reproduction procedure may be wrong if let's say the permissions are sorted in a different way. We seemed to have inconsistent results (using the api, and the dashboard discovery) before we decided to read some of the source code

It may be better to have two "non-overlapping" DLS statements to make sure the union of roles works as expected

[scrawfor99]

Hi @peternied, I don't believe we have a test that covers this type of scenario. I think it would be wise to add one however and will take a look.

Hi @sbeginCoveo, thank you for the follow-up. That makes sense and is likely to be the case. I just wanted to confirm with you that I was understanding your reproduction steps for the time being. I will continue to take a look and let you know if anything comes up.