security icon indicating copy to clipboard operation
security copied to clipboard

[RFC] Retire support for config option `config.dynamic.multi_rolespan_enabled` in `config.yml`

Open nibix opened this issue 1 year ago • 5 comments

Background

I am asking for opinions and comments on the following topic because it influences the work on #3870.

Current situation

The OpenSearch security config file config.yml supports an option called config.dynamic.multi_rolespan_enabled.

This config option is undocumented. Doing a search for it only returns hits which includes dumps of config.yml where the option with default value is just coincidentally contained in the config file:

  • https://www.google.com/search?q=site%253Aopensearch.org+%22multi_rolespan_enabled%22

The only very brief mention of this option can be found in the docs of a related project:

  • https://docs.search-guard.com/7.x-53/changelog-searchguard-7-x-35_0_0#breaking-default-changed-for-permissions-evaluation-across-different-roles-multi-rolespan

Functionality

If this option is set to true (which is the default if a config.yml file in version 2/7 is in use), privileges for a single OpenSearch operation can be collected from several roles. If this option is set to false, privileges for a single operation must be located in exactly one role.

This can pertain to actions or indices.

Several indices

If I issue a search request on indices a and b, I need the privilege indices:data/read/search for both indices.

With multi_rolespan_enabled = true, it is possible that the user has two different roles which provide the different privileges:

role_1:
  index_permissions:
    index_patterns:
    - a
    allowed_actions:  
    - indices:data/read/search
role_2:
  index_permissions:
    index_patterns:
    - b
    allowed_actions:  
    - indices:data/read/search

With multi_rolespan_enabled = false, however, these roles would be not sufficient for a single search operation on both indices. Instead, a user would need to have a single role with both privileges:

role_1:
  index_permissions:
    index_patterns:
    - a
    - b
    allowed_actions:  
    - indices:data/read/search

Several actions

For example, in order to issue a bulk index insert request, I need two privileges:

  • index privilege indices:data/write/bulk
  • index privilege indices:data/write/index

With multi_rolespan_enabled = true, it is possible that the user has two different roles which provide the different privileges:

role_1:
  index_permissions:
    index_patterns:
    - foo
    allowed_actions:  
    - indices:data/write/bulk
role_2:
  index_permissions:
    index_patterns:
    - foo
    allowed_actions:  
    - indices:data/write/index

With multi_rolespan_enabled = false, however, these roles would be not sufficient for a bulk index operation. Instead, a user would need to have a single role with both privileges:

role_1:
  index_permissions:
    index_patterns:
    - foo
    allowed_actions:  
    - indices:data/write/bulk
    - indices:data/write/index

The set of operations which require two privileges at once is very limited, all can be quickly found by checking the code at:

https://github.com/opensearch-project/security/blob/6dedfb455766601b1fdee8e5334cd0905bfe2ed2/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java#L601

This shows that the following operations need two privileges:

  • _bulk, indices:data/write/bulk: requires indices:data/write/bulk and depending on the bulk item type indices:data/write/index, indices:data/write/delete or indices:data/write/update
  • Cross cluster search backing action indices:admin/shards/search_shards requires additionally indices:data/read/search
  • _aliases, indices:admin/aliases with an item type remove needs additionally indices:admin/delete
  • Create index, indices:admin/create with a specified alias needs additionally indices:admin/aliases
  • Restore snapshot, cluster:admin/snapshot/restore needs additionally indices:admin/create, indices:data/write/index -- but only if the config option plugins.security.check_snapshot_restore_write_privileges is set to true

Default value

On OpenSearch clusters where the config.yml configuration with _meta.config_version=2 is in use, this defaults to true.

On OpenSearch clusters where a config.yml configuration from ODFE 0.x in version 1 is in use, this defaults to false.

Reasons for using the non-default value multi_rolespan_enabled = false

Seriously, I do not know any reasons for using the non-default value multi_rolespan_enabled = false.

Having the option set to false allows a more granular control over privileges.

However, I would argue that this added granularity does not have any value:

  • It does not make sense to allow me searching in index a in one request and additionally in index b in another request, but denying my to search a and b at the same time.
  • It does not make sense to deny me deleting indices using the POST _aliases API while allowing it me using the normal DELETE index API.

Current work

In the context of #3870, we need to re-write significant parts of the privilege evaluation code. Supporting multi_rolespan_enabled would significantly increase the complexity of the code - and thus also increase the necessary work and later maintenance effort.

Proposal: Just drop it

In my opinion, it would be not economical to put any effort into an option that offers hardly any value.

Thus, I am proposing to drop support for that option. We should always behave like its value would be true -- the default value.

As it is an undocumented option, I would argue that such a breaking change would be even possible in a minor version upgrade.

nibix avatar Jun 27 '24 07:06 nibix

@scrawfor99 @cwperks @peternied @DarshitChanpura Another proposal for a simplification :) Would be curious on your opinion on this as well.

nibix avatar Jun 27 '24 07:06 nibix

Hi @nibix, I agree. Let's drop the false option. It serves no real value.

stephen-crawford avatar Jun 27 '24 14:06 stephen-crawford

[Triage] Hi @nibix, thanks for filing this issue. Going to mark as triaged.

stephen-crawford avatar Jul 01 '24 15:07 stephen-crawford

Breaking changes like removing config options are generally done on major releases. I'd be in favor of deprecating in 2.x and dropping it in 3.0.0, but I don't think it should be dropped in 2.x or else it could break some clusters.

Edit: On further review, this feature is undocumented and does not provide clear benefit. I think the benefits from Optimized Privilege Evaluation outweigh support of this undocumented setting.

cwperks avatar Jul 01 '24 17:07 cwperks

Taking a fresh look at this RFC. This configuration option doesn't make any sense to me because there is a clear workaround where a cluster admin could always create a role that has all necessary permissions which would render config.dynamic.multi_rolespan_enabled not very useful.

When thinking about tradeoffs, I think the performance improvements from Optimized Privilege Evaluation outweigh the support for config.dynamic.multi_rolespan_enabled

cwperks avatar Sep 06 '24 19:09 cwperks

@nibix Can this be closed now?

cwperks avatar Feb 05 '25 20:02 cwperks

Yes, I think so.

nibix avatar Feb 06 '25 12:02 nibix