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

TEP-0091: Verified Remote Resources #537

Closed
wants to merge 13 commits into from
Closed

Conversation

squee1945
Copy link

No description provided.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 11, 2021

CLA Signed

The committers are authorized under a signed CLA.

@tekton-robot tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 11, 2021
@bobcatfish
Copy link
Contributor

/assign

@bobcatfish bobcatfish added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Oct 11, 2021
@squee1945 squee1945 changed the title Initial draft for Task Bundle Verification. TEP-0091: Verified Task Bundles Oct 11, 2021
Copy link
Contributor

@bobcatfish bobcatfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together @squee1945 ! I've focused first on the description of the problem and haven't commented yet on the design itself.

My biggest pieces of feedback are both kinda similar, basically there are a couple of considerations that the proposal is explicitly declaring out of scope that I think we should include - at least to think through how this proposal could be extended to them before moving forward:

  • Applying this approach outside of OCI bundles; esp. given SLSA requirements we know that we need to be able to support Tasks and Pipelines in version control (and even bundles need to have originated from version control to satisfy SLSA) - and we know that for some of what we're planning, e.g. with Workflows, that we have some plans to support pulling Tasks and Pipelines directly from version control
  • Using this approach for Pipelines as well as Tasks

- A Pipeline Author can indicate that a Task Bundle must be verified in order
for Chains to create a provenance attestation.
- A Pipeline Author can configure a build to fail if a Task Bundle cannot be
verified.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we can (and i think we could make it work) id like to see us include a requirement that it be possible to apply this approach to Tasks outside of bundles as well, specifically I'm thinking of Tasks in version control (via the remote resolution work @sbwsg has been doing - which would mean also being able to generalize to potentially other sources of Tasks as well)

(potentially Tasks in cluster as well though we might need to be a bit more creative for that)

and on the topic of version control, this is slightly out of scope but i think worth thinking about: if we want ppl to be able to use tekton to achieve SLSA level 3, they're going to need to be able to meet the "build as code" requirement, which will be interesting for bundles - and for bundles it's going to mean ultimately being able to trace back the source of a bundle to a version controlled repo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(i wonder if eventually we're going to want to redefine "bundle" such that there are multiple ways of backing it - e.g. OCI registry, version control, bucket... but with provenance information as well)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"for bundles it's going to mean ultimately being able to trace back the source of a bundle to a version controlled repo"

Interesting. By extension this would also seem to implicate builder images too? Meaning, the builder images themselves will have to have some sort of signed source provenance attestation linking them back to version controlled repo?

for Chains to create a provenance attestation.
- A Pipeline Author can configure a build to fail if a Task Bundle cannot be
verified.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also i think worth including as a requirement that whatever approach we take here be generalizable to Pipelines as well as Tasks (i think Tekton Bundles can also contain Pipelines?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was explicitly avoiding Pipelines because I think Chains operates over TaskRun results?

If Chains operates exclusively over TaskRun results, then then a PipelineRun (either inline, or via a Pipeline Ref) will cause TaskRuns to be specified - those TaskRuns will either be inline, or a reference to a cluster Task, or a reference to a Task Bundle. If the latter, it could be verified according to this TEP.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was explicitly avoiding Pipelines because I think Chains operates over TaskRun results?

yup, chains operate over taskRun results and a pipelineRun will cause taskRuns to be specified.

The taskRun either part of a pipeline or not triggers chains if it produces IMAGES as task result (or have image pipeline resource as an output). The high level goal here is to design the solution which can be extended to a pipeline and also complements TEP-0084. The proposal in this PR looks pretty robust and extensible. The motivation behind verifying a pipeline (in YAML or bundles) is to make sure the tasks are executed in the order they were specified and the ones which were part of the pipeline.

teps/0091-verified-task-bundles.md Outdated Show resolved Hide resolved
teps/0091-verified-task-bundles.md Outdated Show resolved Hide resolved
-->

This proposal is explicitly focussed on Task *Bundles* because they are
packaged as an OCI image and work well with Cosign. This does not preclude
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think before we move forward with the proposal we should at least try to work through what this would look like for other situations, specifically when Tasks exist in version control, and also how we could handle non-cosign requirements (as per our flexibility principles to be careful about when we're opinionated)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbwsg can you point me to any more information about using a task ref from version control? For Git, we might be able to use signer: gpg for example.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing - here are the implementation options we're considering as part of the remote resolution TEP.

One of the goals in that TEP is to offer / integrate a mechanism for verification of remote tasks (and pipelines) before they are executed by Pipelines' reconcilers. Happy to update that TEP to accommodate any changes needed to support this one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbwsg TEP-0060 looks great and I think this TEP could easily be modified to be built on top of it (e.g., introduce optional verification configuration). I'm new to K8S development, but the TektonResourceRequest CRD seems to be most aligned to the overall model - presuming people can plug in custom resolvers - but no need to litigate that here as I think most of the alternatives presented in TEP-0060 could form the basis for this TEP.

So... now we have a bit of a 🐔 and 🥚 issue. How can I help?

- A Pipeline Author can configure a build to fail if a Task Bundle cannot be
verified.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few potential requirements that we could include (if possible i think it would be really useful to express the requirements in terms such that a) we only mention design choices such as cosign in the proposal and b) we can be as clear as possible about what 'trusted' and 'safe' mean in this context:

  • There is a process by which a Task or Pipeline can be marked as "trusted" (haha I'm not defining trusted BUT what I'm trying to say is that trust = owners of the Task or Pipeline explicitly sign off on the contents in some way)
  • Verification can ensure the contents of the Task or Pipeline have not been modified since being marked as "trusted"
  • It is not possible to modify the Task or Pipeline in such a way that it has been modified and the verification will still pass (i.e. the Task or Pipelines does not contain the means of verification)
  • Verification can be performed by Tekton Pipelines; Tekton Chains can see whether or not the verification occurred and know that it was signed off on by Tekton Pipelines (this might be a bit too far into the implementation - however im a bit worried that relying on annotations might not work here - i.e. i think we need to know the annotation wasn't just added by someone with permission to edit the TaskRun/PipelineRun)
  • When evaluating a new Task for use, it must be clear to a user how to provide a user with all the data they need to verify the task (e.g. the correct public key)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added suggested requirements.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"relying on annotations might not work here - i.e. i think we need to know the annotation wasn't just added by someone with permission to edit the TaskRun/PipelineRun"

I hadn't contemplated this. Are there other types of annotations in Tekton that are subject to abuse if these annotations can be edited like this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other types of annotations in Tekton that are subject to abuse if these annotations can be edited like this?

i think you could make things not work correctly, e.g. by changing labels or even changing owner references, but im not sure what the abuse vector would be (general disruption and mischief?)

I believe tekton chains signs the annotations it adds ( @priyawadhwa correct me if i'm wrong ) - maybe the same approach will work for pipelines adding annotations we need to trust?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bobcatfish yah so the annotations could be edited to say pretty much anything. If you want to prevent that then this is where spire could come in -- with spire, the pipelines controller could add some annotation and also a signature over that annotation, which chains could then verify. you can configure spire to only sign requests that come from the tekton controller, so something else running the cluster wouldn't be able to do it.

to follow the current chains format for signing you would have the user specify a path to a private key (this could be in a k8s secret or in kms). you'd just have to make sure that nothing else in the cluster could sign with it!

@bobcatfish
Copy link
Contributor

hey @pritidesai looking at https://github.com/tap8stry/tapestry-pipelines I think this TEP might be very relevant to your interests? :D

@pritidesai
Copy link
Member

/assign

@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2021
@nadgowdas
Copy link
Contributor

@squee1945 there is some work I've been doing that could be relevant to your proposal here, please let us know your thoughts on this https://github.com/OpenSecureSupplyChain/tkn-admcontroller

@pritidesai
Copy link
Member

Thank you @squee1945 for the proposal, looks great 👍 very exciting 🎉

Couple of initial comments:

I think the proposal here can also be useful for the use cases where a task is not producing an image i.e. chains not creating any attestations, for example, the compliance pipelines or the clearance pipelines which not necessarily build an image would need such level of trust. And can rely on this proposal.

We have a multiple sources of truth while executing a task (1) the task definition can be inlined taskSpec (2) the task definition can be part of a bundle (3) a task definition can be part of a catalog (4) the Task CRD on the cluster. I guess my question is not related to this TEP but in general, should we cover all of these sources in long run to establish some kind of trust?

@squee1945
Copy link
Author

@pritidesai I've initially focussed on Task Bundles here in the spirit of MVP.

Re: catalog (which I take to mean source control?). I think it's very possible to expand this to support Tasks in source control, though frankly, I think we need to wait for some recent developments on signing Git commits to shake out a bit.

taskSpec. I don't think there's a meaningful way to sign a taskSpec since it is included in the build definition directly. The primary reason for signature validation is to trust the parts outside of the build definition that are being used. Please let me know if you have use cases for this.

taskRef. This is an interesting case. So the idea is that perhaps there are two personas within an org: one that can submit TaskRuns and one that can apply Task definitions. The signature verification would prevent bad acting on the part of the persona that applies Task definitions. To support this, I think we'd need to add an annotation to the Task that would hold the signature. Out-of-band, the persona that submits TaskRun would need to know the good public key and provide that in their TaskRun, something like the following:

apiVersion: tekton.dev/v1beta1
kind: TaskRun

spec:

  taskRef:
    name: local-task
    verification:
      signer: [some-signature-verifier-built-into-Tekton]
      key: [some-public-key]
      onError: [ stopAndFail | continue ]

Tekton would likely produce a CLI tool that would create the signature to annotate a given Task, or possibly bake it into the Task controller and annotate automatically when the Task is applied - provided the private key could be adequately guarded from the persona applying the Task.

--

I see a fairly clear line in supporting these other cases, though the final case definitely has a fair amount of scope. Does a TEP describe a single piece of deliverable work? If so, I'd like to keep the scope down. I could add a discussion about supporting these other cases to the TEP as future work though - how does that sound?

Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 2, 2022
This commit is an initial step of work for Trust Task.(Forked from tektoncd/community#537)

Prior this commit we cannot verify if the taskrun is manipulated or not before applying to the cluster.

This commit leverages cosign to do verification on taskspec. Admission webhook and verification on taskrun.spec will be introduced in following PRs. With these work we can deploy a standalone webhook for verification without changing current Tekton Pipeline APIs.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 2, 2022
This commit is an initial step of work for Trust Task.(Forked from tektoncd/community#537)

Prior this commit we cannot verify if the taskrun is manipulated or not before applying to the cluster.

This commit leverages cosign to do verification on taskspec. Admission webhook and verification on taskrun.spec will be introduced in following PRs. With these work we can deploy a standalone webhook for verification without changing current Tekton Pipeline APIs.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 2, 2022
This commit is an initial step of work for Trust Task.(Forked from tektoncd/community#537)

Prior this commit we cannot verify if the taskrun is manipulated or not before applying to the cluster.

This commit leverages cosign to do verification on taskspec. Admission webhook and verification on taskrun.spec will be introduced in following PRs. With these work we can deploy a standalone webhook for verification without changing current Tekton Pipeline APIs.
tekton-robot pushed a commit to tektoncd/experimental that referenced this pull request Mar 2, 2022
This commit is an initial step of work for Trust Task.(Forked from tektoncd/community#537)

Prior this commit we cannot verify if the taskrun is manipulated or not before applying to the cluster.

This commit leverages cosign to do verification on taskspec. Admission webhook and verification on taskrun.spec will be introduced in following PRs. With these work we can deploy a standalone webhook for verification without changing current Tekton Pipeline APIs.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 2, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 3, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 3, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
Yongxuanzhang added a commit to Yongxuanzhang/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
tekton-robot pushed a commit to tektoncd/experimental that referenced this pull request Mar 7, 2022
This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.
@tekton-robot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 23, 2022
@tekton-robot
Copy link
Contributor

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 22, 2022
@dibyom
Copy link
Member

dibyom commented Jun 27, 2022

/close

Closing in favor of #739

@tekton-robot
Copy link
Contributor

@dibyom: Closed this PR.

In response to this:

/close

Closing in favor of #739

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/s3c Issues or PRs that are related to Secure Software Supply Chain (S3C) kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
Status: NEW
Status: UnAssigned
Status: Opened
Development

Successfully merging this pull request may close these issues.

10 participants