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

Support Vault AppRole auth method in provider #1335

Open
michael2m opened this issue Sep 7, 2020 · 14 comments
Open

Support Vault AppRole auth method in provider #1335

michael2m opened this issue Sep 7, 2020 · 14 comments

Comments

@michael2m
Copy link

Feature: AppRole auth method for Vault provider

I would like to see AppRole auth method in Vault provider for Secretless because that offers alternative login to Vault for fetching secrets. Currently the Vault provider is based on the default behavior of the Vault API client, which assumes an environment variable with a token for the token auth method. This is overly restrictive (Vault supports many auth methods).

Solution

Have an additional environment variable to capture the auth method, e.g. VAULT_AUTH_METHOD=approle, whose value indicates the auth method to use. Additional environment variables may be used to feed the required auth details, e.g. VAULT_APPROLE_ROLE_ID= and VAULT_APPROLE_SECRET_ID=. Within the provider factory function it can handle the auth method accordingly (switching on the auth method value).

Alternatives

There does not seem to be an alternative at the moment, due to the way the provider factory function is implemented. It creates a fixed configuration relying on the default behavior of the Vault API client. The solution outlined above could help create a more generic solution, although initially supporting only the AppRole auth method.

In the Vault API client in golang, additional configuration settings have to set (in code). There are no other facilities for obtaining a config, e.g. from environment variables or configuration files like YAML.

Additional context

https://www.vaultproject.io/docs/auth

@izgeri
Copy link
Contributor

izgeri commented Sep 8, 2020

Thanks for filing this @michael2m!

As with the previous issue you filed, I'm not sure what our timeline for making this change would be - but if you're interested in contributing you can find more info in our Secretless Contributing Guide and in our general Conjur Contributing Guide. You can also contact us on Discourse.

I wonder if the Vault go client would be open to an option to configure via env var / file? If not, could Secretless could certainly be configured for its connection to Vault via env var / config file and then pass the config to the Vault client in the provider Go code.

If you'd like to propose an implementation for this, it would be great to share it here for feedback before starting work. I look forward to seeing how this moves forward!

@michael2m
Copy link
Author

Effectively it is a refactor and behavorial extension to the internal/providers/vault/provider.go function ProviderFactory.

  • It should attempt to read the VAULT_AUTH_METHOD env variable. If no such variable, fallback to current behavior and done.
  • Otherwise check against supported auth methods.
  • If auth method equals "approle":
    • Call into the function setting up the Vault client for the provider with AppRole login
    • It reads the VAULT_APPROLE_ROLE_ID env var to use as role ID in AppRole auth. If not existing, it's an error.
    • Next it would read the VAULT_TOKEN env var to use as wrapped token for obtaining the secret ID. If it exists, use Vault client to unwrap the token to obtain the secret ID. If unwrap fails, it's an error.
    • Otherwise read the VAULT_APPROLE_SECRET_ID env var to use as secret ID. If not existing, it's an error.
    • Use the Vault client to write to the /auth/approle/login with the role ID and secret ID as data (effectively performing AppRole login), to obtain the token. If login fails, it's an error.
  • Set the token on the Vault client and done.

Each supported auth method can have its own function(s) to setup the client. This makes for quite a modular design, where the factory function only determines which auth method to use and call into the appropriate auth method handler function to setup the remainder.

BTW the Vault client instance has to be configured through the ProviderFactory function of the Vault provider anyway (currently it does so implicitly). The Vault API for golang has a ReadEnvironment function, but that reads a limited and fixed set of (some deprecated) env vars. It's not useful here.

@sgnn7
Copy link
Contributor

sgnn7 commented Sep 9, 2020

While I'm hesitant to say that env-var config is the right path forward long-term, it could give us a bit more idea on how to start thinking about configuring the providers from within the secretless config file itself.

Regarding your approach proposal, these are my initial thoughts:

  • We must indicate in logs what auth method we are using if it's not the default one.
  • The flow should first check for VAULT_TOKEN and if it exists use the default flow. If the variable is unset/empty, then we should start looking into VAULT_AUTH_METHOD.
  • We should ensure that if VAULT_AUTH_METHOD is not supported that we exit with an error
  • We should verify all env var requirements for an auth method in a block and before instantiating the vault client
  • In the APIs I see that secret_id may not be required so this should be an optional variable. We should however throw a warning when that variable is not specified.
  • This approle variable and specific client instantiation logic should be in its own module path (eg internal/providers/vault/auth/approle/approle.go)

@michael2m
Copy link
Author

I'm not entirely happy with env-var configuration either. Long-term alternative could be to have configuration section per provider, which can be referenced in the from part of configuration. This could also allow e.g. different Vault instances be used (configuring one as provider A and another as B, then use from: A and from: B to refer to them respectively).

However on short-term easiest solution seems sticking to env-vars. I do believe that VAULT_AUTH_METHOD existence should trigger alternative auth methods. Again to remain compatible with current behavior and to have a single env var (i.e. configuration item) determine the auth method used. If a VAULT_AUTH_METHOD is provided, it should be a supported one or error otherwise.

In the AppRole case you may need to unwrap a single-use token to obtain the secret_id, which requires a Vault API client instance. This adds a level of indirection. Hence a different env-var is required to distinguish from the provided secret through env-vars. Ultimately both role_id and secret_id are required for AppRole login. So an error occurs otherwise.

Do you envision the modularity to follow a factory like pattern ?

@sgnn7
Copy link
Contributor

sgnn7 commented Sep 22, 2020

However on short-term easiest solution seems sticking to env-vars.

Yeah I agree

Do you envision the modularity to follow a factory like pattern ?

I think whatever is natural to add to the current architecture is fine. If a big refactor is needed, we may want to separate that from adding in the new functionality. As a small related sidenote, it historically did seem like factory patterns have been the most fitting for modular Golang architectures we worked on so far.

@michael2m
Copy link
Author

If you'd like me to work out a solution in code, please assign this issue to me and label it implementing.

@sgnn7
Copy link
Contributor

sgnn7 commented Sep 25, 2020

@michael2m Let's see what @doodlesbykumbi has on his plate but that sounds good to me!

@doodlesbykumbi
Copy link
Contributor

@michael2m I like the suggestions for how to proceed, envvars for now followed later by provider configuration in secretless.yml. Making providers configurable via secretless.yml opens up some interesting possibilities (I think), i.e. providers could actually get their configuration from other providers.. we'd need to resolve the dependency tree etc. but in certain cases this can can mean absolute no secret zero.

@michael2m
Copy link
Author

michael2m commented Sep 28, 2020

@doodlesbykumbi I think either way could be fine:

  • Implement auth method(s) so they become available to the community sooner, then improve in configuration (independent issue)
  • Or improve configuration before this issue, preventing envvars and backwards compatability issues later

I think you go a little further than I originally did or the related issue #1336. Having named providers, they can have their own configuration and be referred to by their name when used elsewhere.

What more do you get when:

providers could actuallly get their configuration from other providers

?

Could you also share some more thoughts on how:

this can mean absolute no secret zero

?

@doodlesbykumbi
Copy link
Contributor

doodlesbykumbi commented Sep 29, 2020

@michael2m

providers could actually get their configuration from other providers... this can mean absolutely no secret zero

The scenario is one where you have connection details stored in vault that requires authentication details that is stored in another vault. A concrete example is storing credentials for Conjur in the keychain, while Conjur stores the credentials for my service.

Cribbing from your suggestion in #1336, I would imagine something like this. In this example my-provider is a custom configured instance of the vault provider that gets its token from the keychain provider. Note that the keychain provider need not be specified under providers; I'm entertaining the idea of having default instances of providers with (maybe) reserved names.

version: 2

providers:
  my-provider:
    type: vault
    credentials:
      token:
        from: keychain
        get: prod/xyz/vault-token
 ...

services:
  echo_tcp:
    connector: example-tcp-connector
    listenOn: tcp://0.0.0.0:6175
    credentials:
      address:
        from: my-provider
        get: some-secret
...

@michael2m
Copy link
Author

michael2m commented Sep 29, 2020

This is exactly what I had in mind @doodlesbykumbi. And indeed you can get secrets for one provider from another one, which does require a DAG be validated (I think a tree would be too restrictive).

Should improved configuration like above and in #1336 be implemented before continuing this issue ?

@sgnn7
Copy link
Contributor

sgnn7 commented Sep 29, 2020

I just want to put a note here that we need some way to provide secure credentials in the config file since those are most likely variables that need setting and the config file is usually a higher risk vs env vars.

@doodlesbykumbi
Copy link
Contributor

@michael2m

Should improved configuration like above and in #1336 be implemented before continuing this issue ?

Improved configuration is definitely a more ambitious undertaking than we're planning to do for now. Let's proceed with:

Implement auth method(s) so they become available to the community sooner, then improve in configuration (independent issue)

I've gone ahead and assigned you. The implementation of the auth method(s) we have in mind is as stated in your comment about using envvars. Please let me know when there's work ready for review.

I think we can create a separate issue for the improved configuration then continue the discussion and capture the improvements we envisage for config v3 over in that issue.

michael2m pushed a commit to michael2m/secretless-broker that referenced this issue Sep 30, 2020
michael2m pushed a commit to michael2m/secretless-broker that referenced this issue Oct 6, 2020
This link was broken. This change points it to the right header within
the README
michael2m pushed a commit to michael2m/secretless-broker that referenced this issue Oct 14, 2020
michael2m pushed a commit to michael2m/secretless-broker that referenced this issue Oct 15, 2020
@boazmichaely
Copy link
Contributor

Published in CyberArk Aha! idea portal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants