go-vela
add proposal for pipeline secret security feature
I lean towards decision 3, SHA in it's own file.
Re decision 2, repurposing the unused trusted flag. I misremember what the original intent for this field was; perhaps to allow privileged mode in the future? There is nothing preventing this from being set to true currently, I believe, so therefore some cleanup / education might be necessary.
I lean towards option 3 since it enables signatures for pipeline files that utilize inline templating- https://go-vela.github.io/docs/templates/#templating-directly-in-velayml.
I lean towards option 3 as well. As far as order of operations, are you thinking it does the checksum against the raw pipeline in the repo vs to once a pipeline is expended with templates etc?
I want to provide a little more context surrounding the proposal. I don't think I was clear enough in the original writing of it, but the structure of the proposal is as follows:
In other words, I would love feedback on each of the individual decisions that are being made within this flow. The decisions here correspond with those mentioned in the proposal. Let me know what you think!
My small comment would be to point slightly back at my original comment in https://github.com/go-vela/community/issues/62#issuecomment-987119653. I don't quite know what "improved security and access control" is aiming at, if it's exposure of secrets to only valid users or limiting the exposure during pull requests or something else altogether.
Another idea if it's really just about locking down secrets to only those who should have access, I feel one of the most direct and simple ways of doing this might be to limit it to only those who have write access in GitHub. I think we've already got that behavior in place for making changes to pipeline settings too. If someone can write or make direct changes to the codebase then it follows that they can expose any secret they want anywhere they want, so allowing ONLY those users to use secrets in PRs makes sense. It might then allow outside contributers to issue PRs and still use secrets while locking down those who don't have access. It would probably also get away from an additional user step of signing a pipeline in order to enable secrets.
After doing a bit of digging and having some conversations about this proposal, I've been able to write up a second option for accomplishing this security task. Thanks everyone who was able to look over what I had written originally. I hope folks can find the time to check out this second option and generate some discussion!
@GregoryDosh I kinda like the idea but I'm a little concerned that's not quite as applicable in this case.
I do like the idea of more advanced security policies for secrets (shared, org, repo). I think some of the things GitHub Actions are doing are interesting; making org secrets more usable across repos but I'm not sure those ideas apply in this particular issue.
I think for signing we wouldn't want things overly complex because of the nature of what it's doing so I would lean into only allowing repo admins that ability.
@kneal I think that is what I'm trying to understand from the original issue.
- improved security and access control
- Improved how? What security issue is of concern here? Is it the following bullet point?
- remove the security risk with secrets in a pull_request build
- This here could be mitigated by allowing only those with admin/write/some-other-level of access the ability to use secrets in a PR event (or any event really). Going to a finer grained ACL model for users and having it align with what source code tools like GitHub/GitLab/etc. already have would potentially be easier for end users.
I'm sure there is something I'm missing, so please help me see what additional benefits does pipeline signing gives. As I see it, this will be an optional thing, so in essence you have to opt-in to security, which I think is the wrong way to go here, and because it's an extra step to enable most won't bother. We should be considering here how we should make the safe & right thing to do the easiest thing to do.
I think this is mainly to address a key issue with pull request secrets and fork workflows exclusively.
- This addresses a very key security issue: pipelines that execute on pull requests can be manipulated to expose secrets on forked repositories. By adding verification, malicious edits to
.vela.ymlwould never get the chance to run.
So, the main thing we are looking to avoid is an outside forked user manipulating the pipeline to like to echo the secret for example. Depending on how the community is using the Vela installation is running this cant be less worry some. The key thing in this workflow is you have a repo with pull requests events enabled and an outside user pull requesting from a fork.
I can think of some Gated Builds ideas that would likely also solve the issue which might be a better path but either way I'm not sure how ACLs help us in cases where an outside repo contributor maliciously attacks the pipeline. They don't even necessarily need to have access to the app (Vela), instead of writing the secret to STDOUT; they could write it to like s3 or an external source and never access Vela to expose that pull request secret value.
I think this is the confusion is in how I described my idea. The way I was thinking about it would solve the " help us in cases where an outside repo contributor maliciously attacks the pipeline" issue.
The scenario today is something of the following: Organization A wants a repository (repo A) to be able to run their CI on pull requests or comment events etc. Perhaps they've got a situation that they build docker images on the initialization of a pull request but they want to be able to also comment "docker rebuild" and it would go out and rebuild that Docker image, push into a stage environment and run all their tests there. This could arguably be done elsewhere and not on a comment event, but follow this example for a moment. That docker step will need to store that artifact somewhere which would include a secret for an S3 bucket etc. This process works, and they like it, so they don't want to change things too much.
Malicious User (MU) notices or assumes that a pipeline is vulnerable to secrets exfiltration. They know they can craft or change the .vela.yml file to include that echo $VAR statement. They manipulate that code in a fork (repo F) and then open the pull request. Because today those secrets are available to pull requests and comments, the MU could exfiltrate that in two ways, either on the initial PR opening or during that comment event on their PR. They can also exfiltrate those secrets either in the CI pipeline itself or in the app code somewhere, perhaps during a testing step?
Now, in my idea if gating secrets on permissions in something like GitHub, those secrets would only be exposed for users who have a write access or admin or whatever source code tooling uses. Vela already uses something like this to manage who can and can't modify settings for a particular repo in the UI. If we follow that for secrets exposure that would make it so that the previous scenario the MU can't actually get any of those secrets if they don't have direct write/admin access to the repo. It wouldn't take a signature file or anything of the sort to gate those secrets. If someone is an admin or has write access to that repo (repo A, not F) then they'd still be able to exfiltrate those secrets and no amount of signing would stop anyhow. Gating the signature to only admins means that MU just needs to push the exfiltration down into the app/test code instead of the CI pipeline. It doesn't solve the exfiltration portion at all.
If secrets are gated on admin/write access then the burden on stopping exfiltration is on those users who are inspecting suspicious outside code contributions and if they comment "docker rebuild" and give away their secrets it was more of a deliberate (but sad) choice on that admin's part and it isn't something that Vela would be enabling by default.
So, essentially what you're proposing is instead of using signing as a method to solve malicious external contributors introduce an ACL system for how secrets are injected into pipelines.
So, when a user triggers a build we take into account that specific user's permissions and then decide if the user should be allowed to have secrets injected into the pipeline based on a secret policy that exists for a given set of secrets.
That's exactly what I was trying to (poorly) articulate. The trouble I can see from my idea would be in articulating to users why secrets weren't available for their particular build. "You're an outside contributor, therefore secrets are disallowed, talk to an admin if you think you should have access and don't."
But otherwise the current runstate for pipelines wouldn't change much. You'd not need to sign an additional file to enable this security feature. It would be toggled on by default and I'd argue shouldn't get turned off for many pipelines.
In other words, I would love feedback on each of the individual decisions that are being made within this flow. The decisions here correspond with those mentioned in the proposal. Let me know what you think!
based on the diagram.. DECISION 3: SHA of expanded pipeline (to address Neal's question); otherwise you can modify a referenced template without affecting the SHA.
DECISION 2: Not sure if Trusted is the appropriate term and it sounds like there maybe was some prior goals for it. I'd vote for a new field at this point.
DECISION 1: I'm kind of a fan of not having an extra file in the repo to worry about. I can see the benefit of it being in version control, but maybe some of that can be translated to additional fields like SignatureModifiedAt SignatureModifiedBy fields?
@GregoryDosh @kneal - would that really need to be a new ACL, or just piggy back off of SCM permissions as we do already - at least initially? I know there was some previous discussion about adding RBAC-like functionality.
@wass3r I'm sure I'm using the terms inappropriately, ACL & RBAC etc. I'm mostly trying to convey that we use the SCM permission already in place like we do for the Settings tab of the repo in the UI. Perhaps it's more in line that a github org has a specific team to allow outside contributions from and allow secrets in. Or use the direct write/admin access from SCM. I don't know how this gets implemented for permissions necessarily, but if I were to rewrite the original title of the issue it would be more like "Prevent exfiltration of secrets by malicious users" and pipeline signing stops one avenue but leaves an entirely other avenue wide open, while also making this prevention an opt-in step rather than opt-out. I think for safety and security it should be opt-out of secure practices.
I vote for option 1 because AFAICT, I don't think option 2 would address the use case this was requested for 👍
IIRC, a Target user requested this feature and needed it to prevent security risk of secrets with the pull_request event.
However, there is a caveat that using an ACL based mechanism wouldn't have worked (at the time they wrote it) 😅
To elaborate, this user managed an org where files on APIs were stored and Vela took actions based off the file contents.
Each API had its own repo but the caveat is the actual maintainers of the APIs weren't allowed direct access to the repo.
To make changes to these files, the API maintainers would need to fork the repo for their API and open a PR from the fork.
I think that pattern is the same for open source maintainers and projects 😃
If I were a maintainer using Vela and my pipeline was using secrets with the pull_request event, it could feel burdensome to fail any build from users who open a PR from a fork. Chances are, most of my open source contributions will come from a fork since users won't have access to my repo and thus won't have access to run builds using secrets with Vela.
I think if we had a gating mechanism like Actions, then this experience could be improved 👍
Using option 1, as an admin/maintainer of my project I could generate my signature and be done with it.
Any user that modifies my pipeline won't have their build run because Vela will tell them the signature doesn't match.
I'm making the assumption that we'll only allow admins of a repo to generate the signature for a repo 😅
Decisions:
- I vote for a separate file (
.vela.sigis fine) that's stored alongside the original Vela pipeline 😃
I have concerns with embedding directly in the pipeline for cases when it is already very large and has a lot going on.
At that point, it feels like it would be adding more chaos to the mix 😞
I have less concerns with making the signature hidden but wondering what the value is?
At a minimum, we'd need more code to denote when a repo is using signatures and probably who generated it?
- I have no opinions on this so I'll roll with the group 👍
Initially, I proposed using trusted to Easton because it was a field that we aren't using and already have available.
But, I'm not against using a different field to control this functionality 😃
- SHA checksum of the pipeline after it has been compiled by the server (or at least templates have been rendered)
I can't be certain, but I believe the compiler's modification service comes into play here as well?
If so, then I think we have to let the server compile the pipeline and generate the checksum based on that.
In a competitive CI landscape adding even one additional step or requirement on end users will probably push them away from your product. Be sure to keep that in mind when you're deciding which way you go on features and how they're implemented. If you're in a situation where you already have tool lock-in internally at your company then it's less of a concern, but you'll be harming your growth opportunities externally as folks look to do competitive analysis and realize other options are safer, securer, and require less manual configuring and headaches.
If the signature option is picked Vela will still need to warn end users about the fact that a misconfigured pipeline can expose pull request secrets to malicious users. That doesn't go away with pipeline signing. Here is a snippet of code (I only modified to obscure some secret naming) that a Vela user wrote. It's naive and probably there are a lot of other things wrong here, but at no point can I see a signed .vela.yml file protect this user from exposing their secrets to a malicious user. I only need to modify their tests to drop those secrets into my API and it's done. This could be automated fairly trivially I bet.
- name: test
image: openjdk:8-jdk
secrets:
- XXX_api_token
- YYY_password
- ZZZ_private_key
ruleset:
event:
- pull_request
- push
- tag
commands:
- SPRING_PROFILES_ACTIVE=local ./gradlew test
If something like ACL/RBAC is picked Vela will still need to warn end users that anyone with a particular set of access (say write for instance) can modify the pipeline and expose secrets. This is actually something that should be in place today regardless of what is picked here. And, will always be true. But at least ACL/RBAC guards against a pipeline misconfiguration error by an end user who doesn't fully grasp the security implications and things that signatures save the day. Still possible in an ACL/RBAC world for users to give the wrong permissions to their repo/org, but that is something that Vela has no hope of stopping. And, if you want to give flexibility in an default-ACL/RBAC enabled repo, just give them a big scary option to disable it, but make it default on and opt-out requires manual intervention and an acknowledgment of the risks. Then as a maintainer of some open source product using Vela I could still have my existing work flow "just work" and new repos coming online could default to something more secure without any additional steps required.
I have no horse in this race, but I hope for the sake of end users you pick something that doesn't give them a false sense of security.
As you can see above, I created a couple draft PRs for the two solutions that were discussed here earlier. Hopefully those can add some additional context for potentially how these ideas could be implemented. While it seems as though the discussion has been a bit more conceptual, perhaps these rough drafts could help us reach a conclusion or notice shortcomings.
@GregoryDosh
In a competitive CI landscape adding even one additional step or requirement on end-users will probably push them away from your product
I think the point you're trying to make is fair but I'm not sure I entirely agree with the statement. I do think added complexity is a negative and we should be striving to reduce when possible but I do think with the amount of churn in pipeline changes I don't think pipeline signing would be as harmful as a solution as is laid out in this statement.
I am fully on board with pausing this proposal for a moment in favor of adding another ACL or gated builds option. I think if those solutions are better or reduce complexity for the product it is a better direction to improve security.
Vela will still need to warn end-users that anyone with a particular set of access (say write for instance) can modify the pipeline and expose secrets.
Highly agree, we should probably add a story for tracking and figure it if the audit page can serve as a place for these kinds of configuration warnings.
But at least ACL/RBAC guards against a pipeline misconfiguration error by an end user who doesn't fully grasp the security implications and things that signatures save the day. Still possible in an ACL/RBAC world for users to give the wrong permissions to their repo/org, but that is something that Vela has no hope of stopping.
IMO we should be cautious with this line of thought, I know there will be users merging or maintaining code they might not understand implications on but the attack vector in a repository is not limited to the .vela.yml configuration that lives in it.
I have spent time a small amount of time dealing with cloud (AWS, GCP, etc) ACL/RBAC policies, and I can tell you the amount of complexity in that space is quite large. That doesn't mean that application-level policies are complex but I've even seen some Vault implementations that also make my head spin. If ACLs are the right answer, I do think trying to leverage an existing technology like OPA would be wiser than writing something from scratch. IMO pipeline signing is a way simpler solution; I would agree ACL/RBAC are more powerful and with the right defaults we might get around some security issues but I think long they are definitely the more complex solution for a consumer to maintain.
I wanted to circle back on this. Some really great points have been made in support of pipeline signatures as well as against the idea (Thank you @GregoryDosh, @jbrockopp, and @kneal). I think, judging by how the discussion has progressed, that an access-level solution seems to be preferred. Please comment if you do not agree with that observation.
In terms of action, do we want to draft up an issue in go-vela/community for introducing some sort of build gatekeeping? Or, should the discussion of implementation be kept here, in this proposal thread? Either way, I want to revitalize this conversation so we can reach some sort of agreeable solution to address the active security issue.