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

[Extensions] Create dynamic configuration section in config.yml for extensions #2615

Closed
Tracked by #2573
cwperks opened this issue Mar 31, 2023 · 8 comments
Closed
Tracked by #2573
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@cwperks
Copy link
Member

cwperks commented Mar 31, 2023

For the initial experimental release of extensions, the security plugin needs to support a couple dynamic configuration options to support the issuance and verification of auth tokens. All configuration options should fall under a new key in the config.dynamic portion of the security plugin config.yml file. Below is an example of the dynamic configuration options.

config:
  dynamic:
    extensions:
      signing_key: <base64_encoded_signing_key>
      encryption_key: <base64_encoded_encryption_key>

The 2 configuration options above represent 2 initial options needed to support the auth token workflows, but this extensions: area of the config.yml file may be extended in the future with more dynamic configuration options.

The 2 settings above represent:

  1. signing_key - This is a base64 encoded secret that will be used to create a JWK to sign the JWTs that are issued - by default the signing is done using symmetric encryption (HMAC SHA512). More configurable keys will be supported in the future, but the default is chosen for the experimental release.
  2. encryption_key - This is another base64 encoded secret, but this will be utilized to encrypt sensitive information in the payload of the JWT.

signing_key and encryption_key are also utilized for an authentication backend to support these tokens issued by the security plugin and used to verify the tokens and decrypt the encrypted sections of the payload.

There are 2 java files around config in this codebase: ConfigV6 and ConfigV7. This change will only support OS >=2 so changes can be only applied into ConfigV7.

@cwperks cwperks added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels Mar 31, 2023
@cwperks cwperks removed the bug Something isn't working label Mar 31, 2023
@cwperks
Copy link
Member Author

cwperks commented Mar 31, 2023

Why not add this in the authc: section of the config since this will be setting up an auth backend if extensions are enabled?

When extensions are enabled, adding an auth backend that can handle the tokens that the security plugin issues for the extensions is not optional. By having a section for extensions in config.dynamic.extensions this sets up a singular place to configure the settings needed for extensions. If the extensions feature is enabled and these settings are not set, that is an invalid setup and the cluster admin should be notified how to set it up. When adding the auth backend to the list in the backend registry, I'm currently unsure if order and challenge matters for the auth backend corresponding to these settings. Should it be the first of the list in auth backends in the BackendRegistry?

@peternied
Copy link
Member

Why does an Admin need to be able to provide there own key? What do you think about the security plugin manage the generation on its own, and supply an API to rotate the keys?

"Extensions needs this feature" doesn't translate to the feature name needs to include the work extensions. To me this is the DelegatingAuthenticationBackend - but I'm happy to consider other names.

@stephen-crawford stephen-crawford added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Apr 3, 2023
@stephen-crawford
Copy link
Contributor

[Triage] This is part of the Extensions project.

@MaciejMierzwa
Copy link
Contributor

MaciejMierzwa commented Apr 5, 2023

Hi,
Are there any more decisions pending for this task? If not my understanding, for now, is that we want to add configuration from the example above and make sure it's loading correctly into ConfigV7 file. Is that correct?
EDIT: Another question @cwperks: File config.yaml get's loaded as configuration at node startup. So for changes to be visible in the cluster admin would need to restart all nodes. This doesn't make the config very dynamic. Is the "dynamic" part only yaml key naming and this configuration is otherwise static?

@cwperks
Copy link
Member Author

cwperks commented Apr 10, 2023

Hi @MaciejMierzwa, sorry I did not reply earlier I was on vacation from Wednesday to Friday last week. The only remaining decision for this task is whether to name the new section of the config extensions or not. I think the section should be called extensions since the config values in this section of the config would be dynamic configuration values needed for extensions.

config.yml gets read at node startup and persisted into the security system index. Updates to this index are propagated to all the nodes in the cluster to refresh the security config.

i.e. If a cluster is using the internal users list, all internal users are stored in this index. If the cluster admin creates a new internal user what happens is that the security index is updated to add a new user into the users area of the configuration and all nodes are instructed to reload their configuration from the index.

Dynamic means settings that are changeable without a reboot opposed to settings defined in opensearch.yml which require a reboot. For instance there is no way to change a cluster name dynamically, that would require a reboot. For security settings, one such setting where its not possible to turn off dynamically is encryption (plugins.security.ssl.http.enabled). There are many others too. Anything on the list here is not dynamic since it gets placed into opensearch.yml: https://github.com/opensearch-project/security/blob/main/tools/install_demo_configuration.sh#L364-L386

@MaciejMierzwa
Copy link
Contributor

I agree on naming @cwperks. So far I have pushed my changes into: https://github.com/MaciejMierzwa/security/tree/extensions_config
After the naming convention is cleared up, I'll create PR to feature/extensions branch. Do we need to ping another team member to weigh in on the topic?

@cwperks
Copy link
Member Author

cwperks commented Apr 11, 2023

Thank you @MaciejMierzwa, I think extensions makes sense for this section of the config since these dynamic settings pertain to how tokens are issued for access tokens given to extensions that allow an extension to act on behalf of the original requesting user (*with additional restrictions).

A couple of names that may be suitable:

  • extensions
  • on_behalf_of
  • obo (short for on-behalf-of)

Can you open up a PR to solicit comments?

@DarshitChanpura
Copy link
Member

Closing this one as related PR has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

No branches or pull requests

5 participants