-
Notifications
You must be signed in to change notification settings - Fork 37
RFC: team credential managers #21
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| # Summary | ||
|
|
||
| Move credential management configuration to the team configuration level, and | ||
| allow configuring multiple credential managers for each team. | ||
|
|
||
|
|
||
| # Motivation | ||
|
|
||
| Configuring a single credential manager is very limiting. Because there is only | ||
| one point of authentication configured, each credential manager has to support | ||
| some form of multi-tenancy. The current strategy is to encode team and pipeline | ||
| names in the paths for the keys that are looked up, but this has many | ||
| downsides: | ||
|
|
||
| * This makes it impossible to share credentials between teams. Instead the | ||
| credential has to be duplicated under each team's path. | ||
|
|
||
| By moving credential manager config to each team we can instead leverage the | ||
| credential manager's access control to determine how credentials are shared | ||
| across teams (e.g. Vault policies). | ||
|
|
||
| * With Vault, this makes it impossible to use any backend except `kv`, because | ||
| all keys live under the same path scheme, and different backends can't be | ||
| mounted under paths managed by other backends. This removes a lot of the | ||
| value of using Vault in the first place. | ||
|
|
||
| By eliminating the path enforcement you can now refer to different secret | ||
| backend mount points. | ||
|
|
||
| * Some credential managers, e.g. Azure KeyVault, have very strict requirements | ||
| for key names (`[a-z\-]+`), effectively making scoping conventions impossible | ||
| to enforce. | ||
|
|
||
| By configuring at the team level, each team can point to their own KeyVault | ||
| or configure their own access control. | ||
|
|
||
| * It would be nice to be able to leverage a specialized credential manager like | ||
| IAM/STS for some things (like the `s3` resource) and use Vault for everything | ||
| else. Right now you can only configure one credential manager, so this is | ||
| impossible. | ||
|
|
||
| By allowing teams to configure multiple credential managers, all credential | ||
| managers can be tried in order when looking up a given credential. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be vary about trying credential managers in order; if that means naively looking up secrets. It's very easy to hit rate limits on e.g. AWS SSM Parameter store, and AWS Secrets Manager is charged per request - related issue: concourse/concourse#2924
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. How about a prefix scheme instead? e.g. We initially avoided explicit prefixes as it directly tied the pipeline config to credential management, vs. being able to supply the vars via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a little less convenient, but I think it makes sense to avoid rate limits and unnecessary costs 🤷♂️ Could maybe be helped a bit if was possible to set a default credentials provider, so you only have to specify Sidenote; since we want to support nested credential paths like |
||
|
|
||
|
|
||
| # Proposal | ||
|
|
||
| The first step is to extend the team config file set by `fly set-team --config` | ||
| to support configuring credential managers. Something like this: | ||
|
|
||
| ```yaml | ||
| roles: # ... | ||
|
|
||
| credential_managers: | ||
| - type: iam | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we have a That way, Concourse admins could configure all teams with a "default/global" provider, and optionally allow teams to bring their own credhub (or vault, other providers, etc.) to the table. And you wouldn't have to put repeated "global shared" credentials (like a corporate business unit's shared newrelic license key or something) in every individual provider. Thoughts? (anyone please point out problems/nuances/discomfort with this that I might not be seeing) |
||
| config: | ||
| access_key: blahblah | ||
| secret_key: blahblah | ||
| - type: vault | ||
| config: | ||
| url: https://vault.example.com:8200 | ||
| ca_cert: | | ||
| -----BEGIN CERTIFICATE----- | ||
| ... | ||
| client_token: blahblahbla | ||
| ``` | ||
|
|
||
| Then, any time we're resolving a `((var))` the `web` node would resolve the var | ||
| using each configured credential manager, in order. Distinct fields can be | ||
| accessed like `((foo.bar))`, and nested credential paths can be accessed like | ||
| `((foo/bar/baz))`. | ||
|
|
||
| In this case, team's Vault auth config would be associated to a policy which | ||
| determines which credentials the team can access. This way shared credentials | ||
| can be shared without duplicating the credential, and private credentials can | ||
| be kept private. | ||
|
|
||
| All credential managers would be modified to remove the automatic team/pipeline | ||
| variable name scoping. They would instead be looked up starting from the root | ||
| level. | ||
|
|
||
|
|
||
| # Open Questions | ||
|
|
||
| * Is there a need for globally-configured and team-configured credential | ||
| managers to coexist, or can we switch to entirely team-configured (as is the | ||
| initial goal)? | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to have the possibility to create with the new credentials managers a similar setup as with the current ones. So have a globally-configured would be good.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a pretty important decision to make as supporting both global and per-team configuration could end up being kinda confusing, so I'd like to know a bit more about your situation. 🙂 How many teams do you have? If there are many of them, could the pain of managing approle auth/etc. be automated away as part of team management? I could be convinced to keep the existing global config and just have this RFC be an additive change to support per-team config. It certainly would be pretty backwards-compatible to remove the global configuration and the path conventions. Since this proposal already includes support for multiple credential managers, it wouldn't be too big of a leap to support per-team + global and just define the precedence carefully. Thanks for the feedback! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are having right now 68 teams and we are already using terraform to provision vault and concourse, so yes we could create the approles per team with it. My initial idea, was to be able to create new teams without involving to run terraform. Just add the team in concourse and concourse is able to read from a shared vault path. But after thinking about it, I guess you are right. Having only per-team configuration makes the config much simpler and straight forward and we can control the access over the already existing vault policies. We then relying on terraform to create teams, but I hope with terraform 0.12 and the new templating it will be much easier to render the team config yml. |
||
| * Concourse will now be responsible for safely storing access to each and every | ||
| credential manager, which increases risk. Is it enough to mitigate this by | ||
| requiring that database encryption be configured? | ||
|
|
||
| * Will anyone miss the automatic credential scoping assumptions? Is there value | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding automated scoping:
Can scopes be added like this, with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, those concerns make a lot of sense! I think I'll incorporate this back in some form, then - either as it is today or as something like your example. 👍 Conceptually it makes a lot of sense, plus you could always use absolute-path syntax to explicitly refer to a common key. We might want to avoid a nested path scheme, though - Vault doesn't allow different secret engines to have overlapping mount paths, so the whole Hmm, maybe with your proposal they could just list the same scheme for other engines as other prefixes?: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just one other thing to point out. when I was refactoring cred mgr code for caching, I noticed that Kubernetes cred mgr also needs a namespace in addition to the path -- it's dynamic and team name is a part of it. Not sure why it was done that way.. but we need to think how it would fit here |
||
| in automatically looking under `/(team)/(pipeline)` for `((foo))`? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For us, definitely. Our teams usually use a generic set of keys as |
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the automatic credential scoping made be configurable? If we don't have them anymore, we would need to create approles per team. Maybe this should be a parameter for the credentials manager? So we can configure a global manager with /(team)/(pipeline) scoping and another with /common ? |
||
| * Would a default key prefix make sense? e.g. `/concourse`? (Maybe this is just | ||
| up to the discretion of the credential manager?) | ||
|
|
||
| * Supporting IAM/STS token acquisition is one of the motivators for this | ||
| proposal, but I think we need a concrete example implementation to really | ||
| understand if this proposal is a good fit. The above example configures an | ||
| access key and secret manually instead of using EC2 IAM role. | ||
|
|
||
| * When and how often do we authenticate with each credential manager? If you're | ||
| using Vault with a periodic token, something will have to continuously renew | ||
| the token. | ||
|
|
||
| Will the `web` node have to maintain long-lived clients for accessing each | ||
| configured credential manager across all teams? Is that going to be a | ||
| scalability concern? Is that going to be a security concern? (Can it be | ||
| avoided?) | ||
|
|
||
| Should it detect situations where this is required? | ||
|
|
||
| Should we just not support periodic tokens? | ||
|
|
||
| * Should we work credential caching into this proposal? | ||
|
|
||
| * How should credential manager authentication errors be surfaced? | ||
|
|
||
| * Is there a need for configuring a path prefix (e.g. the default `/concourse` | ||
| for Vault)? I've left that out for now assuming we can just get rid of it. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would my option be if I did not have a path prefix? Would I have to scope the policy for each team so that they can still only access |
||
|
|
||
|
|
||
| # Answered Questions | ||
|
|
||
|
|
||
| # New Implications | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chatting with @pivotal-jwinters led to potentially thinking of this as something that's per-pipeline, not just per-team. I haven't put any technical thinking into it yet, but I do really like it from the viewpoint that it allows for even safer scoping of credential access (i.e. we don't have to worry about our "prs" pipeline having access to prod credentials), and also makes pipelines more self-contained, which is the single most important thing about pipelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also be scoped to projects if you implement it.