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

Add checksum of on-cluster resources to Provenance.RefSource #6928

Closed
chitrangpatel opened this issue Jul 14, 2023 · 14 comments
Closed

Add checksum of on-cluster resources to Provenance.RefSource #6928

chitrangpatel opened this issue Jul 14, 2023 · 14 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@chitrangpatel
Copy link
Contributor

Feature request

In the SLSA v1.0 predicate, we capture the uri and digest of remote tasks and pipelines. However, if the task/pipeline was defined locally and submitted to the cluster then the SLSA provenance does not have a way to take a note of it in the provenance. In the Tekton Chains WG meeting, we reached an agreement that we should store the checksum of the spec of the on-cluster resource in the provenance.

Currently, we already produce and store the sha256 checksum in Provenance.RefSource if the reference task/pipeline used a Cluster resolver. However, we don't do this if the task/pipeline was referenced using just a Name, even though they are both effectively doing the same thing.

This feature request attempts to compute the sha256 checksum of the on-cluster resources that are referenced by Name and add it to Provenance.RefSource.

Use case

The checksum of the referenced on-cluster pipeline/task will be available in the provenance. If the users re-run the pipeline/task, using the same resources then the checksum should match with what was inserted in the provenance. This will allow verification of resources used for the build.

@chitrangpatel chitrangpatel added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 14, 2023
@chitrangpatel
Copy link
Contributor Author

/assign

@chitrangpatel
Copy link
Contributor Author

cc @wlynch @lcarva @chuangw6

@chuangw6
Copy link
Member

chuangw6 commented Jul 14, 2023

SGTM. This was missing previously since on-cluster resources referenced by name instead of by resolver syntax are not resolved by remote cluster resolver (via resolution request). As you mentioned, "they are both effectively doing the same thing.". So adding them for consistency makes sense.

So IIUC it would be just returning the checksum here right? (Same for task level)

// GetPipeline will resolve a Pipeline from the local cluster using a versioned Tekton client. It will
// return an error if it can't find an appropriate Pipeline for any reason.
// TODO: if we want to set RefSource for in-cluster pipeline, set it here.
// https://github.com/tektoncd/pipeline/issues/5522
// TODO(#6666): Support local resources verification
func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) (*v1.Pipeline, *v1.RefSource, *trustedresources.VerificationResult, error) {
// If we are going to resolve this reference locally, we need a namespace scope.
if l.Namespace == "" {
return nil, nil, nil, fmt.Errorf("Must specify namespace to resolve reference to pipeline %s", name)
}
pipeline, err := l.Tektonclient.TektonV1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{})
if err != nil {
return nil, nil, nil, err
}
return pipeline, nil, nil, nil
}

@chitrangpatel
Copy link
Contributor Author

SGTM. This was missing previously since on-cluster resources referenced by name instead of by resolver syntax are not resolved by remote cluster resolver (via resolution request). As you mentioned, "they are both effectively doing the same thing.". So adding them for consistency makes sense.

So IIUC it would be just returning the checksum here right? (Same for task level)

// GetPipeline will resolve a Pipeline from the local cluster using a versioned Tekton client. It will
// return an error if it can't find an appropriate Pipeline for any reason.
// TODO: if we want to set RefSource for in-cluster pipeline, set it here.
// https://github.com/tektoncd/pipeline/issues/5522
// TODO(#6666): Support local resources verification
func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) (*v1.Pipeline, *v1.RefSource, *trustedresources.VerificationResult, error) {
// If we are going to resolve this reference locally, we need a namespace scope.
if l.Namespace == "" {
return nil, nil, nil, fmt.Errorf("Must specify namespace to resolve reference to pipeline %s", name)
}
pipeline, err := l.Tektonclient.TektonV1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{})
if err != nil {
return nil, nil, nil, err
}
return pipeline, nil, nil, nil
}

Yes, that's correct.

@wlynch
Copy link
Member

wlynch commented Jul 14, 2023

I'd sync with @Yongxuanzhang here - this should be very similar, if not identical to the checksum used for trusted resources.

@chitrangpatel
Copy link
Contributor Author

chitrangpatel commented Jul 14, 2023

I'd sync with @Yongxuanzhang here - this should be very similar, if not identical to the checksum used for trusted resources.

This is what the Cluster Resolver does. I was planning to replicate this ability. @Yongxuanzhang Is this the checksum we also perform with trusted resources?

@Yongxuanzhang
Copy link
Member

Yongxuanzhang commented Jul 14, 2023

I think what @chitrangpatel proposed is to record this for the resources ref via local resolver, the link in @chuangw6's comment, it should be a simple replication of cluster resolver. No direct relationship with trusted resources.

@chitrangpatel
Copy link
Contributor Author

I'd sync with @Yongxuanzhang here - this should be very similar, if not identical to the checksum used for trusted resources.

In trusted resources, we are also producing a sha256 checksum of the task/pipeline. See

func verifyInterface(obj interface{}, verifier signature.Verifier, signature []byte) error {
ts, err := json.Marshal(obj)
if err != nil {
return fmt.Errorf("failed to marshal the object: %w", err)
}
h := sha256.New()
h.Write(ts)

I will try to use approach this throughout.

@chuangw6
Copy link
Member

I'd sync with @Yongxuanzhang here - this should be very similar, if not identical to the checksum used for trusted resources.

+1 to yongxuan. It seems like the checksum we computed here has nothing to do with trusted resources b/c trusted resource checks the signature (computed by author) against the checksum it computes internally and see if it matches. Only thing in provenance.refSource that matters to trusted resources is the uri.

@wlynch I barely remember in #5687 there is a specific reason why we only do the checksum of the spec part instead of the whole object for in-cluster resources. Do you remember why? Is it for downstream Chains verification in future or something else?

@chitrangpatel
Copy link
Contributor Author

Following discussion on slack, I've opened a new issue to synchronize the checksum computation between trusted resources and cluster resolver. Issue #6958

See details in there.

@Yongxuanzhang
Copy link
Member

Thanks! I'm not opposed this, as long as it is the correct way for provenance!

@tekton-robot
Copy link
Collaborator

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 Oct 19, 2023
@vdemeester
Copy link
Member

/remove-lifecycle stale
@chitrangpatel what's the status on this issue ? I see #6958 is closed, so wondering about that 😛

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 23, 2023
@chitrangpatel
Copy link
Contributor Author

Can be closed! Thanks for the ping :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

6 participants