opensearch-project / security

🔐 Secure your cluster with TLS, numerous authentication backends, data masking, audit logging as well as role-based access control on indices, documents, and fields
https://opensearch.org/docs/latest/security-plugin/index/
Apache License 2.0
191 stars 272 forks source link

[BUG] `dfm_empty_overrides_all` removes DLS also if other role has no explicit data/read permission #3963

Open rursprung opened 8 months ago

rursprung commented 8 months ago

What is the bug? With issue #1572 the dfm_empty_overrides_all config option had been introduced in opensearch-security (merged in 2.0.0.0) to allow a user having an admin role to have full access even if he also is assigned another role with less access.

The presumption from our side (and also based on the documentation from Search Guard; note that there’s none for OpenSearch, I’ve raised a ticket for that) was that this applies to roles which grant read access to the data (e.g. which have the indices:data/read/search index permission).

However, we now noticed that this also seems to apply to other roles (where you obviously never specify a DLS).

How can one reproduce the bug? Namely we have an example with two roles granted to a normal user: One which grants read access to an index (with a DLS):

{
  "index_permissions": [
    {
      "index_patterns": [
        "example-index"
      ],
      "dls": "[.. some DLS here ..]",
      "allowed_actions": [
        "indices:data/read/search",
      ]
    }
  ]
}

And one which grants only access to search templates:

{
  "index_permissions" : [
    {
      "index_patterns" : [ "*" ],
      "allowed_actions" : [ "indices:data/read/search/template" ]
    }
  ]
}

What is the expected behavior? The expectation was that this would grant the users the right to use a search template and execute queries against example-index, respecting their DLS. However, it turns out that if dfm_empty_overrides_all is enabled, the DLS is removed for this user and he sees all data in this index.

At the very least this is unintuitive (and should be clearly documented as such), however I consider this a bug and think it should be changed so that the empty DLS/FLS only counts if it is on a role which grants the read access (open question: should this be separate for indices:data/read/search and indices:data/read/get and possibly other such rights?).

What is your host/environment? I’ve tested this on OpenSearch 2.11.0.

Do you have any screenshots? n/a

Do you have any additional context? same behaviour on Search Guard (classic)

rursprung commented 8 months ago

note that there's a workaround proposed by floragunn: the second role must have a DLS which never matches, e.g.:

{
  "index_permissions" : [
    {
      "index_patterns" : [ "*" ],
      "dls": "{\"match_none\":{}}",
      "allowed_actions" : [ "indices:data/read/search/template" ]
    }
  ]
}
stephen-crawford commented 8 months ago

[Triage] Hi @rursprung thanks for filing this issue. The documentation will be addressed shortly.

For this overall use case there are some efforts around a new feature "Views" which may be of interest to you. You can find some more information on this new feature over on the OpenSearch core repo under its RFC. This issue can be closed when the documentation change is complete.

peternied commented 8 months ago

@rursprung Thanks for filing - I think the DLS/FLS feature in generally has usability problems. I think that Views [1] feature might offer a better alternatives, curious what you think or what you'd like to see there,

rursprung commented 8 months ago

@peternied: how would the views work with parameter substitution of user attributes (coming from the JWT)?

if that's supported it might be an option. then we could create the views with the equivalent of the DLS and grant read access to that to normal users and additionally grant full access to the underlying index to the admin user which will then just ignore the views and work directly with the indices.

peternied commented 8 months ago

how would the views work with parameter substitution of user attributes (coming from the JWT)?

@rursprung Offhand, if all the information about how the view is constructed from information in the caller's identity token and part of the url/parameters I think it would work well. If the use cases for parameters were documented out in an RFC that might be a good place to discuss edge cases such as handling the absence of a parameter.