RFC: team credential managers

[vito]

Rendered

Status: in need of lots of feedback! There are many open questions.

Please leave comments on individual lines of the proposal document so conversations can be marked as resolved. Top-level comments will be maintained aggressively if necessary so the discussion doesn't grow out of control. Thank you!

[hprotzek]

In our case, we are using vault as the only credentials manager for all teams.

[hprotzek]

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.

[vito]

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!

[hprotzek]

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.

[hprotzek]

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 ?

[itsdalmo]

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

[vito]

Hmm. How about a prefix scheme instead? e.g. ((vault.foo.bar))

We initially avoided explicit prefixes as it directly tied the pipeline config to credential management, vs. being able to supply the vars via -v/-l, but I guess it's kind of unavoidable here, and ends up being more clear anyway, especially as different credential managers may have different look-up schemes. You could also directly put those prefixes in your -v/-l values, so... 🤔 Maybe I'll just change it to that?

[itsdalmo]

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 <provider>.foo.bar only if you are using the non-default provider?

Sidenote; since we want to support nested credential paths like ((foo/bar/baz)), and fields ((foo.bar)) - maybe providers should use a different delimiter for readability, e.g. ((vault://foo/bar.baz)) or ((vault:foo/bar.baz)) instead of ((vault.foo/bar.baz))?

[romankartgh]

Regarding automated scoping:

1. I think it's really important to introduce this as a non-breaking change for existing Concourse users.

   Let's say an existing 4.x or 5.x deployment has 15 teams, 100 pipelines, and the secrets are already arranged as team/foo and team/pipeline/foo in Vault. If scoping behavior changes... then asking end-users to reconfigure all teams after the upgrade, and also changing all pipelines to refer to team/pipeline/foo variables instead of foo variables... would be a big stretch

2. Having automated scoping is actually useful. Right now, when a new pipeline is created, we put quite a few pipeline-specific variables into its own team/pipeline "tree" in Vault. That way you can clearly tell which secret belongs to which pipeline, and secrets don't get mixed up in one big pile. Same way, if you have a pipeline called test, we don't necessarily want to change all its variables in its template to look like test/foo (see previous item too)

3. Also, just to point out, we are sharing a lot of the same secrets between teams. E.g. SMTP host/port/user/pass, etc. With the absence of "global" cred mgr, we'll have to put the same cred mgr config into each team. But we can definitely live with that.

Can scopes be added like this, with team and pipeline resolved at runtime?

credential_managers:
- type: vault
  config:
    url: ...
    ca_cert: ...
    client_token: ...
  prefixes:
   - /concourse
   - /concourse/((team))
   - /concourse/((team))/((pipeline))

[vito]

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 /concourse path would have to be mounted as kv, and then you'd have to use absolute paths to refer to any keys that use a different secret engine. This is the main limitation with today's approach.

Hmm, maybe with your proposal they could just list the same scheme for other engines as other prefixes?:

  prefixes:
  - /kv
  - /kv/((team))
  - /kv/((team))/((pipeline))
  - /aws
  - /aws/((team))
  - /aws/((team))/((pipeline))

[romankartgh]

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

[jclarksnps]

Our use case is that we use vault as the main creds store across all teams (which works fine as it is now). However, we want to use the gcp secrets engine in concourse so we can auto-generate service account credentials for a given pipeline/job run that needs access to google cloud.

[jwntrs]

The idea of per team credential managers feels like a bad idea to me. It actually feels like a good idea, but I don't think it fits with our other multitenancy decisions. When we made the switch to global users we made the decision to go with a soft tenancy model, so that users could be aware of all their teams. We decided that if someone wanted to enforce the hard tenancy model they would need to deploy two different concourse deployments. Making the change to per team credentials managers seems to contradict this decision and blurs our tenancy model.

I do like the idea of supporting multiple credentials managers, and doing so at the global level will help to address at least some of the issues that this RFC outlines.

[jwntrs]

concourse/concourse#3221 solves the problem of sharing across teams, although it doesn't allow for sharing with some teams and not others.

If we allowed for multiple global providers at the top level, we could allow for custom mount paths, which I believe would allow us to use different vault backends?

[vito]

@pivotal-jwinters Could you comment on individual lines on the proposal document instead? I don't want top-level discussions to get out of hand as they can't be threaded.

Also could you explain the contradiction a bit more? 🤔 I'm having a hard time seeing it; which teams a user can see and which credentials a team can access feel like pretty different concerns.

Credential management was introduced before we even had user-centric auth and has never really fit cleanly into the multi-tenant model. Some credential managers don't even have the flexibility for key-path-encoded access control (e.g. Azure KeyVault), which is the only real method we have for multi-tenancy since there's only one auth config across the cluster. Re-inventing our own access control seems to go against the intended use of these credential managers.

The Vault shared-path approach was a stopgap solution in absence of being able to actually leverage Vault's access control, and is a massive security risk - it's basically 'share everything with no checks and balances' which becomes problematic once you start running pull requests or adding teams without thinking about what they're actually going to be able to access. It'll make people decide things like "two teams share this credential, so maybe we should just share it to all 1000 teams". I don't think we should be encouraging such a risky approach to sharing of credentials. We could implement our own cross-team credential sharing model, but now we're getting out of our core competency and tackling problems that the credential managers themselves have been very careful to implement correctly.

So right now I see a whole lot of cons to having global credential managers, and a lot of motivation for moving them down to the team level (outlined in the proposal). There are definitely some "cons" to per-team (e.g. figuring out how Concourse authenticates to all of them could be difficult), but I'd like to explore those and understand all the cons of the proposal (including any contradictions with other aspects of Concourse's design) before bailing on the idea.

[vito]

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.

[Chumper]

This should also be scoped to projects if you implement it.

[Rukenshia]

For us, definitely. Our teams usually use a generic set of keys as /team-name/repo-key to clone from repositories, but for pipelines where they need push access (for just one specific repository), they will store it under ``/team-name/my-pipeline/repo-key` right now. So we would really appreciate having backwards compatibility for this.
git keys are just an example, we also do it for end-to-end test user credentials and others.

[Rukenshia]

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 /concourse/my-team, and they will have to change all of their usages of credentials from ((my-secret)) to ((/concourse/my-team/my-secret))?

[aegershman]

Could we have a name field for these, too? Similar to how you can have multiple of the same Concourse resources but referenced as different names. Then teams could be configured with multiple credentials managers of the same type, right? That way, if you could have prefix schemes which references the provider name, it effectively still allows for "global credentials" alongside team-owned credentials managers.

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)

[vito]

Just a quick update: I'm probably going to close this RFC in favor of project-level credential manager configuration in #32.

I haven't updated the projects RFC yet, but it's been part of the plan for a while - it's briefly mentioned in the v10 slides: https://vito.github.io/slides/v10.html#7,8

Project-level feels more appropriate to me. It puts the credential manager config right next to the pipelines that ultimately use it, lowering the chance of them falling out of sync. Credentials for the credential manager itself would be specified at fly set-project time, which would be very infrequent (probably less frequent even than fly set-team).

This also allows finer granularity of credential access; rather than allowing an entire team's pipelines to unilaterally access credentials, it can be done on a project-by-project basis, which would let you e.g. separate pull request pipelines out into a separate project within your team to ensure they don't access sensitive information. Configuring a whole separate team for this doesn't feel intuitive; it gets a little awkward as teams have a global namespace and are configured by a cluster admin.

Anyhow, once I update #32 I'll close this. I'll keep all the feedback so far in mind, though things might change a bit with this newfound granularity. 🙂

Thanks!

[vito]

Ok, so, concourse/concourse#4600 introduces pipeline-level credential manager configuration, which is kind of a big deal because it addresses the same motivations of this proposal, but configured at a pipeline-level instead of a team-level. I'm going to pivot this RFC to cover that instead and touch on how we might want things to behave with project-level, pipeline-level, and global credential manager configuration.

[vito]

I submitted a new RFC instead since it's a significant enough departure: #39