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 Release Attestation #319

Merged
merged 13 commits into from
Feb 5, 2024
Merged

Conversation

steiza
Copy link
Contributor

@steiza steiza commented Jan 11, 2024

This pull request proposes a Release Attestation, following the process outlined by ITE-9.

The purpose of this attestation is to authoritatively link from a specific release name and version string in a registry, to the artifact names and hashes that make up that release. This allows consumers of that release version to ensure the artifacts they are consuming have not been tampered with.

This is a formalization of what npm does today with what it calls the publish attestation.

Feedback welcome!

Signed-off-by: Zach Steindler <steiza@github.com>
@steiza steiza requested a review from a team as a code owner January 11, 2024 13:37
@kommendorkapten
Copy link

kommendorkapten commented Jan 18, 2024

During another discussion with @phillmv, for certain ecosystems such as Go there may not be an artifact, the release is the source tree. Git commits can have their digest computed, so that may be a good candidate for the digest in the subject. What would a good name be, the name of the release (commonly a tag name)?

After more discussions I withdrawn this comment, I jumped the gun here 😄

spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
@TomHennen
Copy link
Contributor

This is one of the use cases I've imagined for VSAs.

resourceUri would contain the purl, verificationResult would be 'PASSED' and verifiedLevels could be empty.

I've heard feedback that one reason folks might not want to use VSAs for this purpose is that they don't want be in position of evaluating SLSA policy themselves (fair).

The VSA format is also currently probably more complex than it needs to be (I'm working on how it can be significantly simplified).

Is there any other reason you wouldn't use a VSA for this purpose?

spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Show resolved Hide resolved
@steiza
Copy link
Contributor Author

steiza commented Jan 19, 2024

Is there any other reason you wouldn't use a VSA for this purpose?

Yes, so from the proposal we said: "Perhaps surprisingly, this predicate does not depend on SLSA Provenance, but they are better together." We want ecosystems to be able to use release attestations completely independently from if they support SLSA provenance or not.

This isn't because we don't like SLSA provenance (quite the opposite!) Rather, we're looking for security capabilities that are easier for ecosystems to make use of on their way towards SLSA provenance.

Signed-off-by: Zach Steindler <steiza@github.com>
Copy link
Contributor

@MarkLodato MarkLodato left a comment

Choose a reason for hiding this comment

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

Thanks again

spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Show resolved Hide resolved
spec/predicates/release.md Show resolved Hide resolved
…easeId

Signed-off-by: Zach Steindler <steiza@github.com>
@TomHennen
Copy link
Contributor

Is there any other reason you wouldn't use a VSA for this purpose?

Yes, so from the proposal we said: "Perhaps surprisingly, this predicate does not depend on SLSA Provenance, but they are better together." We want ecosystems to be able to use release attestations completely independently from if they support SLSA provenance or not.

This isn't because we don't like SLSA provenance (quite the opposite!) Rather, we're looking for security capabilities that are easier for ecosystems to make use of on their way towards SLSA provenance.

Ah, I think there may be some misunderstanding.

I'm not asking about the SLSA provenance, but rather the verification summary attestation which doesn't have to imply any SLSA evaluation has taken place (there's some discussion of making it more abstract and making it a more generic attestation), and can communicate other details about package.

The thing that I think is most helpful about a VSA is the ability to apply arbitrary tags (via verifiedLevels) to a package to indicate what high level properties the attestation producer believes it to have. To that end, I think this proposal could also achieve those goals if it were to just add a 'properties' field (repeated list of strings). For now, for folks that aren't ready for SLSA (or anything else) they could either leave this properties field empty or (maybe preferably?) some standard 'RELEASED' tag could be defined which would be included.

@steiza
Copy link
Contributor Author

steiza commented Jan 22, 2024

Do note that in the update I pushed today I added an option predicate, releaseId to help prevent confusion when a release is renamed, or when a release is deleted and a new release is created with the same name.

@steiza
Copy link
Contributor Author

steiza commented Jan 22, 2024

I'm not asking about the SLSA provenance, but rather the verification summary attestation which doesn't have to imply any SLSA evaluation has taken place (there's some discussion of making it more abstract and making it a more generic attestation)

Making VSA more abstract is an interesting idea. But today, the top line of https://slsa.dev/spec/v1.0/verification_summary says:

Verification summary attestations communicate that an artifact has been verified at a specific SLSA level and details about that verification.

I'm happy to follow along as the VSA conversation continues, but I don't think we should block this proposal of release attestations as an in-toto predicate on those VSA conversations.

Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

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

This first iteration looks quite good, thanks @steiza !

A couple high level comments. Can you please wrap lines at 80 chars? Also, per #320, once you're ready, can you please also add a proto definition and add this predicate to the in-toto.io namespace?

spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
… text

Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
@steiza
Copy link
Contributor Author

steiza commented Jan 23, 2024

Also, per #320, once you're ready, can you please also add a proto definition

I've added the proto definition (but not the generated code) to this pull request - let me know if you'd like the generated code as well.

and add this predicate to the in-toto.io namespace?

I'm not sure I understand this process, but I took a stab at this with in-toto/in-toto.io#23

@marcelamelara
Copy link
Contributor

I've added the proto definition (but not the generated code) to this pull request - let me know if you'd like the generated code as well.

We have a workflow for generating the language bindings from the protos, so you're all set!

I'm not sure I understand this process, but I took a stab at this with in-toto/in-toto.io#23

This is a new part of the process for us, so we're still finalizing some of the details, but the PR you opened in that repo is definitely in line with how we want this to work. Thanks!!

Copy link
Contributor

@marcelamelara marcelamelara 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 the updates @steiza ! I think we're almost there. Can you please also fix the linter errors?

spec/predicates/release.md Outdated Show resolved Hide resolved
@marcelamelara marcelamelara self-requested a review January 24, 2024 18:32
Signed-off-by: Zach Steindler <steiza@github.com>
Copy link
Contributor

@marcelamelara marcelamelara 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 the updates @steiza ! LGTM. One more (optional) improvement could be to show an example of how one would convert an npm publish attestation into this release attestation, for extra clarity.

Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

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

Thanks again!

spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
Signed-off-by: Zach Steindler <steiza@github.com>
Copy link
Member

@trishankatdatadog trishankatdatadog left a comment

Choose a reason for hiding this comment

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

Let's go 🚀

@marcelamelara marcelamelara requested a review from a team February 2, 2024 17:06
Copy link
Member

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

couple of comments but looks good overall!

spec/predicates/release.md Show resolved Hide resolved
spec/predicates/release.md Outdated Show resolved Hide resolved
spec/predicates/release.md Show resolved Hide resolved
Signed-off-by: Zach Steindler <steiza@github.com>
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

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

Thanks again!

spec/predicates/release.md Outdated Show resolved Hide resolved
@marcelamelara marcelamelara requested a review from pxp928 February 5, 2024 20:01
Copy link
Member

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

Thanks @steiza!

Co-authored-by: Marcela Melara <marcela.melara@intel.com>
Signed-off-by: Zach Steindler <steiza@github.com>
@steiza steiza force-pushed the release-attestation branch from df0caf7 to 1b21574 Compare February 5, 2024 20:10
@marcelamelara marcelamelara merged commit 3e26e49 into in-toto:main Feb 5, 2024
8 checks passed
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.

9 participants