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

Allow the Keystore to be completely pluggable #5832

Closed
ph opened this issue Dec 7, 2017 · 6 comments
Closed

Allow the Keystore to be completely pluggable #5832

ph opened this issue Dec 7, 2017 · 6 comments
Labels
good first issue Indicates a good issue for first-time contributors libbeat Stalled Team:Integrations Label for the Integrations team

Comments

@ph
Copy link
Contributor

ph commented Dec 7, 2017

The current keystore only support a file-based format; it would be nice to be able to plug any keystore and be able to use it.

Possible keystore:

  • vault?
  • kubernetes secrets

Steps required:

  • Split the interface into two new interfaces: Keystore and EditableKeystore, not all keystore can be edited on the CLI.
  • Add configuration options to change the type
  • Link the keystore with the plugin system.
@exekias
Copy link
Contributor

exekias commented Feb 14, 2020

Let's go back to this as I see several efforts wanting to add more Keystore backends:

Here is my proposal based on PH's write up:

  1. Split current Keystore interface in 3: Keystore (read secrets), WritableKeystore (store/update secrets), ListingKeystore (can list existing secrets, this may not be convenient/possible in some cases, for instance, in kubernetes you can restrict listing, which would be a good security practice). The current file-based keystore would implement the 3 interfaces.

  2. We can start adding new backends that implement at least the Keystore interface, with the others being optional. These backends would only be usable in specific places until we implement point 3. But unlocks their use for things like accessing secrets from autodiscover [Metricbeat] Support pulling secrets from Kubernetes/OpenShift for autodiscovery #8847 (autodiscover would instantiate and pass the keystore, it's also in control of ucfg params for autodiscover template parsing).

  3. Come up with a plan for keystore backends configuration at the beat level. Allowing any part of the config to make use of these keystores. This should include configuration and command line.

Open questions

  • Would it make sense to allow more than one configured keystore at the same time?
  • Do we need to namespace keystores? For instance, give them a name and access secrets through ${keystore:name:secret}
  • Will point 3 require a breaking change? If so, we should plan it for 8.0

any thoughts @urso @kvch @kaiyan-sheng @ChrsMark ?

@ChrsMark
Copy link
Member

Thanks for summing up all these @exekias ! Overall your proposal sounds good to me. Commenting inline to some of your open questions.

Open questions

  • Would it make sense to allow more than one configured keystore at the same time?

I think it will be useful to have it. Having in mind the Kubernetes case, I don't think we should restrict a user to only use Kubernetes keystore, and hence k8s secrets, for managing passwords. I could retrieve some low-sensitivity passwords from k8s secrets but would like to retrieve super secure credentials from Vault let's say. In this, this point make sense to me.

  • Do we need to namespace keystores? For instance, give them a name and access secrets through ${keystore:name:secret}

If we include support for different kind of Keystores at the same time we need this I guess.

@ph
Copy link
Contributor Author

ph commented Feb 14, 2020

+1 for multiple Keystore. I think the priority definition of Keystore is a must, named Keystore is a nice to have.

@urso
Copy link

urso commented Feb 14, 2020

Points 1. and 2. sound good. Having an interface for a keystore we can either create a combined keystore, or just use go-ucfg options to make additional keystores available to config unpacking.

Come up with a plan for keystore backends configuration at the beat level. Allowing any part of the config to make use of these keystores. This should include configuration and command line.

Following other configuration patterns we might end up with something like this:

keystore.<type>:
  < settings >

or in case we want to have multiple keystores:

keystores:
- type: < a >
  < settings >
- type: < b >
  < settings >

The first one would indeed be a breacking change. The second one not so much. We could allow keystore and keystores to co-exist, while we deprecate the current keystore setting.

Would it make sense to allow more than one configured keystore at the same time?

I think so. Follow up question: Do we want to allow a keystore configuration to use keys defined in another keystore? e.g. if keystore 1 is on disk, and keystore 2 is an external service that requires credentials, do we want to allow users to store those credentials in keystore 1?

Do we need to namespace keystores? For instance, give them a name and access secrets through ${keystore:name:secret}

This would indeed be a breaking change. As of today a keystore provides a callback like func(path string) (string, ..., error). The callback is free to parse the path. But the current file keystore will just lookup the path as is. Having a name and multiple keystores will require us to define a name per store as well. Maybe we can make it dependend on the keystore type?

Btw, the : symbols is currently reserved by go-ucfg. e.g. if an user write ${secrect_key:mydefaultpassword}, then go-ucfg will set mydefaultpassword if the key does not exist. You will need to find another symbol ;)

@kaiyan-sheng
Copy link
Contributor

  • Do we need to namespace keystores? For instance, give them a name and access secrets through ${keystore:name:secret}

I think namespace is needed here. For example, if we add support for system manager parameter store for AWS, then namespace here would be aws. This can be used for running beats in AWS. If users want to run beats locally, they can have the flexibility to use the current keystore from beats or AWS parameter store.

@botelastic
Copy link

botelastic bot commented Feb 4, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@botelastic botelastic bot added the Stalled label Feb 4, 2021
@botelastic botelastic bot closed this as completed Mar 6, 2021
@zube zube bot removed the [zube]: Done label Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors libbeat Stalled Team:Integrations Label for the Integrations team
Projects
None yet
Development

No branches or pull requests

6 participants