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 APIs for Statement v1 and SLSA Provenance v1 protos #268

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

marcelamelara
Copy link
Contributor

This PR adds APIs for generating ITE-6 v1 Statements using the protobuf-based language bindings provided by the in-toto Attestation Framework. This PR also adds protobuf-based APIs that replace the now-deprecated SLSA Provenance v1 structs.

Fixes #260, fixes #265.

@adityasaky
Copy link
Member

I like the ideas here, is the plan to take this all the way through signing the statement using dsse as well?

@marcelamelara
Copy link
Contributor Author

is the plan to take this all the way through signing the statement using dsse as well?

@adityasaky Ideally yes. The idea would be to have these sort of generators for each supported predicate type, and switch on the predicate type in a place like runlib before we sign the Statement and generate the DSSE. From the CLI perspective, we may introduce a predicate type flag. Other implementers, like the SLSA provenance generator for GHA might use these APIs directly.

What I don't have a solution for yet is how the the predicate contents themselves will be generated/passed into in-toto, especially in the case of informational ITE-9 predicates.

@marcelamelara
Copy link
Contributor Author

marcelamelara commented Dec 19, 2023

In the interest of keeping PRs small enough and digestible, I'm going to scope this PR at only introducing the new v1 APIs, but not integrating them into the greater in-toto-golang CLI tool. This is meant to work with the Attestor interface introduced in #288 as well, so converting the new Provenance APIs to implement that interface, and adding support for the Link Attestor in runlib will be part of a future PR.

Signed-off-by: Marcela Melara <marcela.melara@intel.com>
Signed-off-by: Marcela Melara <marcela.melara@intel.com>
Signed-off-by: Marcela Melara <marcela.melara@intel.com>
Signed-off-by: Marcela Melara <marcela.melara@intel.com>
@marcelamelara marcelamelara force-pushed the add-ite6-v1-apis branch 3 times, most recently from 7a88a51 to 3835580 Compare December 20, 2023 01:33
Signed-off-by: Marcela Melara <marcela.melara@intel.com>
@marcelamelara marcelamelara marked this pull request as ready for review December 20, 2023 01:34
@marcelamelara marcelamelara changed the title DRAFT: Add APIs for Statement v1 and SLSA Provenance v1 protos Add APIs for Statement v1 and SLSA Provenance v1 protos Dec 20, 2023
Signed-off-by: Marcela Melara <marcela.melara@intel.com>
Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

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

Some nits. Should we discuss this in the context of #288 as well? cc @Forrin

compliant Statement. If there are any marshalling problems,
this function returns an error.
*/
func PredicatePbToStruct(predicatePb proto.Message) (*structpb.Struct, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if want this to be internal/, I don't think we want to accidentally have people use this. Or it could be a private function in attestation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I had set it public since it's called from within runlib, but either moving this to internal/ or even as a private function to be called only within GenerateValidStatement() could work. We would have to change the signature of GenerateValidStatement() to take a protobuf message instead of Struct in the latter case, not sure how I feel about that.


/*
RDListFromRecord converts a map of artifacts as collected
by RecordArtifacts() in runlib.go and converts it into a list
Copy link
Member

Choose a reason for hiding this comment

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

Can this be expanded with the structure of the map? We may retire RecordArtifacts at some point, it'll be helpful to track this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be 100% sure I've understood what you're asking, you're suggesting we include map[string]map[string][string] in this comment?

in_toto/attestation/v1/resource_descriptor.go Outdated Show resolved Hide resolved
in_toto/attestation/v1/statement.go Outdated Show resolved Hide resolved
in_toto/runlib.go Outdated Show resolved Hide resolved
Co-authored-by: Aditya Sirish <8928778+adityasaky@users.noreply.github.com>
Signed-off-by: Marcela Melara <marcela.melara@intel.com>
@Forrin
Copy link
Contributor

Forrin commented Jan 3, 2024

Some nits. Should we discuss this in the context of #288 as well? cc @Forrin

There is for sure some overlap. These can probably be merged fairly easily if I make a few adjustments in my PR. I can remove calling the Attestor and leave that for a future change. Whether it's done in the run function or somewhere else I don't have a strong opinion. However, I do think that the run function is getting a bit bloated and doing too much. Possibly passing it a list of attestors would make the most sense.

But, the run function is technically for a Link style metadata... which I view as an attestor. Open to further conversations.

In the short term, removing how the Attestor code is called and leaving only the interface and LinkAttestor implementation would at least get it merged.

@adityasaky @marcelamelara

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.

Define intoto v1.0 statement type Oversight in the SLAS v1 field invocationID?
3 participants