Skip to content

Filter frontend provenance attestations#3416

Merged
tonistiigi merged 7 commits intomoby:masterfrom
jedevc:filter-frontend-provenance
Jan 4, 2023
Merged

Filter frontend provenance attestations#3416
tonistiigi merged 7 commits intomoby:masterfrom
jedevc:filter-frontend-provenance

Conversation

@jedevc
Copy link
Copy Markdown
Member

@jedevc jedevc commented Dec 15, 2022

The frontend isn't allowed to create provenance attestations - only buildkit is allowed to do this.

We do the check in two stages:

  • Verify in the solver that the frontend set a reason for generating the attestation, and then that reason wasn't for provenance
  • Validate in the exporter (after unbundling) that the attestation predicate was valid for the specified reason. For now, we allow any SLSA provenance attestation for provenance and only allow SPDX for SBOMs.

@jedevc jedevc requested a review from tonistiigi December 15, 2022 17:26
@jedevc
Copy link
Copy Markdown
Member Author

jedevc commented Dec 15, 2022

@tonistiigi in changing the tests to use SBOMs, I hit an issue with what I think is the history API?

=== RUN   TestIntegration/TestExportAttestations/worker=containerd/image
    client_test.go:6122: 
        	Error Trace:	/src/client/client_test.go:6122
        	            				/src/client/client_test.go:7252
        	Error:      	Should be true
        	Test:       	TestIntegration/TestExportAttestations/worker=containerd/image
=== RUN   TestIntegration/TestExportAttestations/worker=containerd/local
=== RUN   TestIntegration/TestExportAttestations/worker=containerd/tar
=== CONT  TestIntegration/TestExportAttestations/worker=containerd

Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand these many commits changing the "attestation reason" handling. Iiuc we just need to check the predicate type does not match SLSA, unless attestation is made internally with ContentCallback. A simple for loop would do.

This isn't anywhere at the moment, but we should be consistent here to
help avoid any future logic issues.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc force-pushed the filter-frontend-provenance branch from 6583ddb to 85060b0 Compare December 16, 2022 12:12
@jedevc
Copy link
Copy Markdown
Member Author

jedevc commented Dec 16, 2022

Removed the reason handling, we just check against the SLSA predicate type now, in the same place where we do checking of Paths, etc.

The rest of the commits are minor fixups and checks that I found while writing the patch, that we should take as well - happy to split those up into a separate PR if neccessary.

@jedevc jedevc requested a review from tonistiigi December 16, 2022 12:13
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc force-pushed the filter-frontend-provenance branch 2 times, most recently from af1db32 to b587aeb Compare December 16, 2022 13:28
eg.Go(func() error {
switch att.Kind {
case gatewaypb.AttestationKindInToto:
if strings.HasPrefix(att.InToto.PredicateType, "https://slsa.dev/provenance/") {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to check .. in the URL path?

I assume not needed, but posting this for confirmation

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think we do - my understanding is that end-users should treat as essentially opaque strings and not attempt to resolve them: https://github.com/in-toto/attestation/blob/main/spec/field_types.md#TypeURI.

The only reason we do the strings.HasPrefix here instead of comparing directly against the strings in the in-toto library is to match against different versions (including unreleased ones).

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc force-pushed the filter-frontend-provenance branch from b587aeb to 13a24b7 Compare December 16, 2022 13:36
@tonistiigi tonistiigi merged commit 617b78c into moby:master Jan 4, 2023
@tonistiigi tonistiigi mentioned this pull request Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants