Add support for SBOM attestations #730

lcarva · 2023-03-01T01:42:23Z

Changes

The idea is a TaskRun produces an unsigned SBOM and pushes it to the OCI registry. The reference to the SBOM is then added to the result IMAGE_SBOM_URL. When Chains sees this result, it downloads the SBOM from the OCI registry, signs it, then adds to the image attestations with an SBOM-specific predicate.

~~This is also possible when using the IMAGES result, and the corresponding SBOMS result. Both must have the exact number of items.~~

~~It's a draft because it's still missing important things like unit tests and documentation. Some of the TODOs also need to be addressed. Looking for early feedback on the overall approach.~~

This commit teaches Chains to create SBOM attestations based on type hinting results from TaskRuns.

The general idea is that a Task generates the SBOM, then uploads it as a blob to an OCI registry. Chains then downloads the blob and uses it as the payload when creating the SBOM in-toto attestation.

Multiple SBOMs per image are allowed. They may be of different formats.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

* Added support for signing SBOMs

tekton-robot · 2023-03-01T01:42:24Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tekton-robot · 2023-04-19T20:40:20Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/artifacts/signable.go	74.1%	74.7%	0.6
pkg/chains/formats/sbom/sbom.go	Do not exist	70.3%
pkg/chains/formats/slsa/v1/intotoite6.go	88.9%	80.0%	-8.9
pkg/chains/objects/objects.go	40.5%	51.0%	10.5

imjasonh · 2023-04-19T21:17:14Z

(Mostly a drive-by comment, didn't review the code) I love it! If you're interested in getting ko to integrate with this I'd be happy to help in any way I can. ko currently only produces unsigned SBOMs but it'd be great if they got signed by Chains along the way. 👍

pkg/chains/formats/slsa/v2/slsav2.go

pkg/chains/formats/sbom/sbom.go

tekton-robot · 2023-08-03T17:16:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from chitrangpatel after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lcarva · 2023-08-11T16:13:30Z

It's been a while. I've re-worked this PR to address the comments and to focus more on SBOM attestations (instead of SBOM attachments). I updated the description to reflect these changes. It is now out of draft.

It also now includes a tutorial, docs/tutorials/signed-sbom.md, which will, obviously, be useful to users, but may also help reviewers.

I think this PR is pretty complete right now, but let me know if there's something missing!

(Apologies for the huge PR. I could not find a reasonable way to break it up.)

tekton-robot · 2023-08-11T16:14:47Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/artifacts/sbom.go	Do not exist	77.2%
pkg/artifacts/signable.go	73.8%	70.2%	-3.6
pkg/artifacts/structured.go	Do not exist	100.0%
pkg/chains/formats/sbom/sbom.go	Do not exist	71.9%
pkg/chains/formats/slsa/v1/intotoite6.go	88.9%	72.7%	-16.2
pkg/chains/objects/objects.go	66.0%	67.2%	1.2
pkg/chains/signing.go	73.7%	73.8%	0.0
pkg/config/config.go	91.8%	91.5%	-0.3
pkg/reconciler/pipelinerun/controller.go	95.0%	95.5%	0.5
pkg/reconciler/taskrun/controller.go	94.1%	94.7%	0.6

This commit teaches Chains to create SBOM attestations based on type hinting results from TaskRuns. The general idea is that a Task generates the SBOM, then uploads it as a blob to an OCI registry. Chains then downloads the blob and uses it as the payload when creating the SBOM in-toto attestation. Multiple SBOMs per image are allowed. They may be of different formats. Signed-off-by: Luiz Carvalho <lucarval@redhat.com>

tekton-robot · 2023-08-11T16:50:23Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/artifacts/sbom.go	Do not exist	77.2%
pkg/artifacts/signable.go	73.8%	70.2%	-3.6
pkg/artifacts/structured.go	Do not exist	100.0%
pkg/chains/formats/sbom/sbom.go	Do not exist	71.9%
pkg/chains/formats/slsa/v1/intotoite6.go	88.9%	72.7%	-16.2
pkg/chains/objects/objects.go	66.0%	67.2%	1.2
pkg/chains/signing.go	73.7%	73.8%	0.0
pkg/config/config.go	91.8%	91.5%	-0.3
pkg/reconciler/pipelinerun/controller.go	95.0%	95.5%	0.5
pkg/reconciler/taskrun/controller.go	94.1%	94.7%	0.6

chitrangpatel · 2023-08-29T16:04:26Z

Thanks for adding the examples and tutorials. I'm wondering if we might benefit from an e2e test here (our e2e tests are already taking ~45 mins 😅).

lcarva · 2023-08-29T17:04:35Z

Thanks for adding the examples and tutorials. I'm wondering if we might benefit from an e2e test here (our e2e tests are already taking ~45 mins 😅).

Yeah, for sure! Is this something that should be part of this PR or a follow up? I'm happy with either way. Just trying to not make this PR even harder to review.

chitrangpatel · 2023-08-29T17:09:12Z

Thanks for adding the examples and tutorials. I'm wondering if we might benefit from an e2e test here (our e2e tests are already taking ~45 mins 😅).

Yeah, for sure! Is this something that should be part of this PR or a follow up? I'm happy with either way. Just trying to not make this PR even harder to review.

I think it can definitely be in a follow up PR.

chitrangpatel · 2023-08-29T18:36:25Z

docs/config.md

@@ -33,9 +33,19 @@ The list of images can be separated by commas or by newlines.
  value: img1@sha256:digest1, img2@sha256:digest2
 ```

-When processing a `TaskRun`, Chains will parse through the list, then sign and attest each image.
+Chains also suports signing SBOMs. If using the `IMAGE_URL` and `IMAGE_DIGEST` combination, the
+`TaskRun` can also emit the `IMAGE_SBOM_URL` and the `IMAGE_SBOM_FORMAT` results. The first contains


It can be in a separate PR but we should probably also add support for Object Results, like we have for Artifact Inputs.

xxxARTIFACTS_SBOM: url:... format:...

pkg/artifacts/sbom.go

pkg/artifacts/signable.go

pkg/chains/formats/slsa/v1/intotoite6_test.go

wlynch · 2023-08-29T21:20:50Z

pkg/chains/signing.go

@@ -44,6 +45,15 @@ type ObjectSigner struct {
 	Backends          map[string]storage.Backend
 	SecretPath        string
 	Pipelineclientset versioned.Interface
+	appContext        context.Context


I don't think we want to attach contexts to the signer like this - this would prevent us from reusing the signer for multiple requests if there's a deadline attached to it. We already plumb through a context with Sign - we should reuse that if possible.

The context here has a different purpose (which is probably a code smell).

The SBOM formater needs access to the OCI registry so it can read the SBOM (consider a private repo). To do this, we need a kube client to initialize the k8schain.

One way to get a kube client is to get it from the context, e.g. kubeclient.Get(ctx). This, however, does not work with the context given to a reconcile iteration - the one ObjectSigner.Sign gets. It only works for the "application" context - the one NewController gets.

Instead of storing the application context, we could store the kube client directly in the ObjectSigner. That might make things more obvious.

@wlynch, wdyt?

pkg/chains/formats/sbom/sbom.go

wlynch · 2023-08-29T21:31:03Z

pkg/chains/objects/objects.go

@@ -65,6 +68,7 @@ type TektonObject interface {
 	SupportsTaskRunArtifact() bool
 	SupportsPipelineRunArtifact() bool
 	SupportsOCIArtifact() bool
+	SupportsSBOMArtifact() bool


I know this is following the pattern of existing funcs, but does this make sense to add to the Object interface? 🤔

I don't know if "whether SBOM artifacts are supported" is a property of an Object we want to enforce on all implementations. Should this be broken out into its own interface?

chitrangpatel · 2023-08-30T14:03:13Z

pkg/artifacts/sbom.go

+				if _, err := name.NewDigest(trimmed); err != nil {
+					logger.Errorf("error getting digest for SBOM image %s: %v", trimmed, err)
+					continue
+				}


Everything until here is completely the same as the above case. Wrap it into a func?

The IMAGES type-hinting result allows a TaskRun to provide a dynamically sized list of images to be processed by Chains. In the case of SLSA Provenance, one statement is created which contains in its list of subjects all the images from the result. This makes sense as these images do share provenance. The SBOMS type-hinting result was meant to add support for SBOM when the IMAGES result is used. However, unlike SLSA Provenance, an SBOM is usually descriptive of a single image. This means that the order of the items in the SBOMS result must match the order of the images in the IMAGES result. This can be problematic. This commit drops support for the SBOMS type-hinting result, and the corresponding SBOMS_FORMAT, to avoid confusion. If we want to re-introduce it, let's make sure that Chains can provide some sort of assurance regarding the order of the SBOMs matching the order of the images. Signed-off-by: Luiz Carvalho <lucarval@redhat.com>

tekton-robot · 2023-09-05T13:30:53Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/artifacts/sbom.go	Do not exist	60.9%
pkg/artifacts/signable.go	73.8%	70.2%	-3.6
pkg/artifacts/structured.go	Do not exist	100.0%
pkg/chains/formats/sbom/sbom.go	Do not exist	71.9%
pkg/chains/formats/slsa/v1/intotoite6.go	88.9%	72.7%	-16.2
pkg/chains/objects/objects.go	66.0%	67.2%	1.2
pkg/chains/signing.go	73.7%	73.8%	0.0
pkg/config/config.go	91.8%	91.5%	-0.3
pkg/reconciler/pipelinerun/controller.go	95.0%	95.5%	0.5
pkg/reconciler/taskrun/controller.go	94.1%	94.7%	0.6

Signed-off-by: Luiz Carvalho <lucarval@redhat.com>

chuangw6 · 2023-09-11T15:03:51Z

docs/config.md

@@ -33,9 +33,18 @@ The list of images can be separated by commas or by newlines.
  value: img1@sha256:digest1, img2@sha256:digest2
 ```

-When processing a `TaskRun`, Chains will parse through the list, then sign and attest each image.
+Chains also suports signing SBOMs. If using the `IMAGE_URL` and `IMAGE_DIGEST` combination, the


I am trying to consolidating type hinting docs in #913. Not sure which PR will be merged first. But this section may be better to live in the new "how to chain with pipeline" doc?

chuangw6 · 2023-09-11T15:11:02Z

docs/config.md

+| Key | Description | Supported Values | Default |
+| :--- | :--- | :--- | :--- |
+| `artifacts.sbom.format` | The statement format to store `SBOM` payloads in. | `in-toto` | `in-toto` |
+| `artifacts.sbom.storage` | The storage backend to store `SBOM` payloads in. Multiple backends can be specified with comma-separated list ("oci,tekton"). To disable the `OCI` artifact input an empty string ("").| `tekton`, `oci`, `gcs`, `docdb`, `grafeas` | `oci` |


Do we support all the storage backend for sbom currently? If not, should we limit this list to what we currently support?

chuangw6 · 2023-09-11T15:16:34Z

examples/sbom/sbom-task.yaml

+  - name: IMAGE_URL
+    description: Reference to the image the SBOM will be generated for.
+  - name: IMAGE_DIGEST
+    description: Digest of the image the SBOM will be generated for.
+  - name: IMAGE_SBOM_URL
+    description: Reference, including digest, to the SBOM blob.
+  - name: IMAGE_SBOM_FORMAT
+    description: The SBOM format.
+    type: string


If Chains takes this approach, I am curious if we should work with catalog team to make sure the syft task can produce right results so that chains can sign? https://github.com/tektoncd/catalog/tree/main/task/syft/0.1

chuangw6 · 2023-09-11T15:19:16Z

docs/config.md

@@ -33,9 +33,18 @@ The list of images can be separated by commas or by newlines.
  value: img1@sha256:digest1, img2@sha256:digest2
 ```

-When processing a `TaskRun`, Chains will parse through the list, then sign and attest each image.
+Chains also suports signing SBOMs. If using the `IMAGE_URL` and `IMAGE_DIGEST` combination, the


Chains supports multiple images type hinting by just appending a prefix before IMAGE_URL and IMAGE_DIGEST. Does SBOM generation support this? If so, can we mention this in the doc as well?

chuangw6 · 2023-09-13T15:08:33Z

pkg/chains/formats/sbom/sbom.go

+	"k8s.io/client-go/kubernetes"
+)
+
+func GenerateAttestation(ctx context.Context, kc kubernetes.Interface, builderID string, maxBytes int64, sbom *objects.SBOMObject) (interface{}, error) {


shouldn't this sbom formatter implement the Payloader interface?

chuangw6 · 2023-09-13T18:58:45Z

pkg/artifacts/structured.go

+func (b *structuredSignableExtractor) extract(ctx context.Context, obj objects.TektonObject) []StructuredSignable {
+	logger := logging.FromContext(ctx)


Great refactor. Thanks Luiz. 2 things

Could we possibly do the refactor in another PR instead of overloading this sbom PR?

Could we extend this refactor to object param/results also? related to this comment Add support for SBOM attestations #730 (comment)

Extracted the StructuredSignable piece to #924. Hopefully, this will make it easier to also do this for the object param/results.

tekton-robot · 2023-09-15T20:53:43Z

@lcarva: PR needs rebase.

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.

tekton-robot · 2023-09-15T20:54:05Z

@lcarva: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-tekton-chains-integration-tests	`c6069f5`	link	true	`/test pull-tekton-chains-integration-tests`
pull-tekton-chains-go-coverage	`c6069f5`	link	false	`/test pull-tekton-chains-go-coverage`
pull-tekton-chains-unit-tests	`c6069f5`	link	true	`/test pull-tekton-chains-unit-tests`
pull-tekton-chains-build-tests	`c6069f5`	link	true	`/test pull-tekton-chains-build-tests`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

lcarva · 2023-10-04T18:07:54Z

The changes in this PR means that Chains would sign any SBOM mentioned in the results of a TaskRun.

In a system where users are able to run their own Tasks, this can be easily abused. It turns out that that's the use case I was trying to address. Thus, I have been struggling in accepting this is a good approach.

After some further reflection, I'm going to abandon this PR.

I don't plan on removing the branch nor my fork, so if you want to take a shot at this and address the security concerns, you are welcome to do so.

If you're curious, for my particular case, I'm going with an approach that relies on the information from the SLSA Provenance since that's already signed (and generated!) by Chains. A task that produces the sbom will use something like oras to push the SBOM to the registry as an OCI blob and record the digest reference as a Task result. On the verification side, we can assert that the task that produced the SBOM is trusted, fetch the SBOM given the reference, and process it as needed.

tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 1, 2023

tekton-robot requested review from priyawadhwa and wlynch March 1, 2023 01:42

lcarva force-pushed the sbom branch 2 times, most recently from d58b3e6 to 2924f10 Compare March 7, 2023 18:12

lcarva force-pushed the sbom branch 2 times, most recently from 38d972f to 61d022a Compare March 8, 2023 18:50

lcarva force-pushed the sbom branch 5 times, most recently from 401193c to c8ab778 Compare March 29, 2023 13:58

lcarva force-pushed the sbom branch from e8dab56 to ab53674 Compare April 5, 2023 15:08

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2023

lcarva force-pushed the sbom branch from ab53674 to a52b8b3 Compare April 19, 2023 20:07

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 19, 2023

lcarva force-pushed the sbom branch from a52b8b3 to 2166414 Compare April 19, 2023 20:29

lcarva marked this pull request as ready for review April 19, 2023 20:35

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 19, 2023

tekton-robot requested a review from chitrangpatel April 19, 2023 20:35

chitrangpatel reviewed Apr 27, 2023

View reviewed changes

pkg/chains/formats/slsa/v2/slsav2.go Outdated Show resolved Hide resolved

chitrangpatel reviewed Apr 27, 2023

View reviewed changes

pkg/chains/formats/sbom/sbom.go Show resolved Hide resolved

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 25, 2023

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 3, 2023

lcarva force-pushed the sbom branch 2 times, most recently from e9676f4 to 59f6a5c Compare August 11, 2023 15:31

lcarva changed the title ~~Add support for signing SBOMs~~ Add support for SBOM attestations Aug 11, 2023

lcarva force-pushed the sbom branch from 59f6a5c to db23aa6 Compare August 11, 2023 16:03

lcarva marked this pull request as ready for review August 11, 2023 16:08

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 11, 2023

tekton-robot requested a review from wlynch August 11, 2023 16:08

lcarva force-pushed the sbom branch from db23aa6 to 4c99afa Compare August 11, 2023 16:45

chitrangpatel reviewed Aug 29, 2023

View reviewed changes

wlynch reviewed Aug 29, 2023

View reviewed changes

chitrangpatel reviewed Aug 30, 2023

View reviewed changes

Remove appContext

c6069f5

Signed-off-by: Luiz Carvalho <lucarval@redhat.com>

chuangw6 reviewed Sep 13, 2023

View reviewed changes

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2023

lcarva closed this Oct 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for SBOM attestations #730

Add support for SBOM attestations #730

lcarva commented Mar 1, 2023 •

edited

Loading

tekton-robot commented Mar 1, 2023

tekton-robot commented Apr 19, 2023

imjasonh commented Apr 19, 2023

tekton-robot commented Aug 3, 2023

lcarva commented Aug 11, 2023

tekton-robot commented Aug 11, 2023

tekton-robot commented Aug 11, 2023

chitrangpatel commented Aug 29, 2023

lcarva commented Aug 29, 2023

chitrangpatel commented Aug 29, 2023

chitrangpatel Aug 29, 2023 •

edited

Loading

wlynch Aug 29, 2023

lcarva Sep 5, 2023

wlynch Aug 29, 2023

chitrangpatel Aug 30, 2023

tekton-robot commented Sep 5, 2023

chuangw6 Sep 11, 2023

chuangw6 Sep 11, 2023

chuangw6 Sep 11, 2023

chuangw6 Sep 11, 2023

chuangw6 Sep 13, 2023

chuangw6 Sep 13, 2023

lcarva Sep 15, 2023

tekton-robot commented Sep 15, 2023

tekton-robot commented Sep 15, 2023

lcarva commented Oct 4, 2023

		func (b *structuredSignableExtractor) extract(ctx context.Context, obj objects.TektonObject) []StructuredSignable {
		logger := logging.FromContext(ctx)

Add support for SBOM attestations #730

Add support for SBOM attestations #730

Conversation

lcarva commented Mar 1, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Mar 1, 2023

tekton-robot commented Apr 19, 2023

imjasonh commented Apr 19, 2023

tekton-robot commented Aug 3, 2023

lcarva commented Aug 11, 2023

tekton-robot commented Aug 11, 2023

tekton-robot commented Aug 11, 2023

chitrangpatel commented Aug 29, 2023

lcarva commented Aug 29, 2023

chitrangpatel commented Aug 29, 2023

chitrangpatel Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Sep 15, 2023

tekton-robot commented Sep 15, 2023

lcarva commented Oct 4, 2023

lcarva commented Mar 1, 2023 •

edited

Loading

chitrangpatel Aug 29, 2023 •

edited

Loading