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 the reference predicate #375

Merged
merged 3 commits into from
Oct 2, 2024
Merged

Add the reference predicate #375

merged 3 commits into from
Oct 2, 2024

Conversation

mdeicas
Copy link
Contributor

@mdeicas mdeicas commented Aug 1, 2024

Adds the specification of a predicate to address #179.

One question I had was how to express what content the reference contains. I think the current use cases are met by the MIME type, and adding another field to specify this more explicitly would make the predicate more complex. If needed, it can be added in the future in a new version.

Open to feedback!

Signed-off-by: Marco Deicas <[email protected]>
@mdeicas mdeicas requested a review from a team as a code owner August 1, 2024 13:02
Copy link

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

I got no juice here, but FWIW this looks good to me and enables all the functionality that Isaac and I talked about in https://sched.co/1YeQW

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.

This looks good to me for a first revision Thank You!


`references` array of [ResourceDescriptor](../v1/field_types.md#resource_descriptor) objects, *required*

The referred documents. The `mediaType` and `downloadLocation` of each SHOULD
Copy link
Contributor

Choose a reason for hiding this comment

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

Should downloadLocation be "MUST"? How useful would a reference attestation that doesn't point to something be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense to me, changed to MUST.

Copy link

Choose a reason for hiding this comment

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

i believe ResourceDescriptor defines it such that downloadLocation is only specified if it is different from the uri. Should we defer to those semantics? Otherwise there may be a conflict within the definition in

> The location of the described resource or artifact, if different from the

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 if the the uri is left empty and digest is provided, then wouldn't be a conflict.

@TomHennen
Copy link
Contributor

Thanks for sending this!

Would it be possible to create a protobuf definition

@marcelamelara
Copy link
Contributor

marcelamelara commented Aug 1, 2024

I don't see how SCAI can't cover this use case. In fact, we have a few examples that show how SCAI can be used to transmit SBOM (or any other type of reference to another doc) without having to embed an entire SBOM in a Statement.

Rather than re-inventing the wheel for one specific use case, I'd much rather we do attempt to standardize around specific attribute values (e.g., our example in scai-demos uses the GUAC verbs HasSBOM and HasSLSA), and declare certain fields in the SCAI predicate as required when used in certain ways, not unlike how we do for DigestSets, where the spec does in fact standardize certain field values for supported hash algorithms.

@marcelamelara
Copy link
Contributor

marcelamelara commented Aug 1, 2024

To add, I'm absolutely happy to work with you to explore different fields/values that could be standardized for this common use case (i.e. exchanging SBOM, having it consumed by GUAC). For instance, for this use case, the target field of the SCAI predicate is required if attribute: HasSBOM and mediaType: application/spdx+json. Again, there's nothing in the existing in-toto attestation spec that precludes these sorts of restrictions to existing predicates for specific use cases.

@lumjjb
Copy link

lumjjb commented Aug 2, 2024

(copying my response from the original issue since it may be relevant to discussion)

I think this is slightly different framing from the witness/provenance materials, and resembling more of an intoto link predicate (although the link predicate is a bit generic, so having a defined predicate will help the consumption/adoption by being opinionated on how fields should be used).

i can see where folks are coming from and indeed there is "encodability" overlap with the different predicates, but I agree with sentiment that the intent here is different. The fields required for supporting documents vs source code attestations vs security statements are different, and i believe it is helpful to be intentional with defining each of them. Else, we may see issues around there being too many optional fields (which also have different use based on context), and that can cause confusion. I'll use SPDX as an example (since i am a contributor and thus an offender), where a "Package" can refer to many things (sources, application bundles) and has many optional fields depending on what it represents (which 3.0 does try and solve by providing a bit more specificity in subclassing, but still suffers from legacy convention).

Agreed on the overall goal of having a "predicate dictionary" type construct where we define here are all the predicates or properties and having guidance on how to consume them. It is difficult for me (who is perhaps uninformed) to understand the delineation around predicates and when something is a separate enough usecase for its own predicate. e.g. SCAI vs vulnerability, intoto link vs SLSA.

@mdeicas
Copy link
Contributor Author

mdeicas commented Aug 5, 2024

Thanks for the responses. IMO a new predicate type has a few benefits over SCAI, but I agree that SCAI does work for the current use case at a technical level, and I'm happy to go with the option that the maintainers think is best. Is the SCAI predicate type spec a good place to define the attribute?

The benefits I see for this dedicated predicate type is that it can evolve more easily (e.g. adding new fields) and is simpler to use for consumers (possibly fewer changes to the spec and the data "type" is encoded at the statement layer instead of the predicate layer).

@marcelamelara
Copy link
Contributor

Sorry for my delay.

It is difficult for me (who is perhaps uninformed) to understand the delineation around predicates and when something is a separate enough usecase for its own predicate.

This is totally fair, and the more general feedback for us here is that we should set clearer guidelines for users of in-toto attestations to choose the right predicate(s), or describe more clearly when it really makes sense to create new ones. We have an upcoming maintainer's meeting, this would be a good topic to raise. Thanks!!

Is the SCAI predicate type spec a good place to define the attribute?

Yes, what I'm imagining is adding a new section to the spec titled something like "Supported Assertions" where we would catalogue the GUAC attributes and expected other field requirements/values for those assertions. This would also entail adding the appropriate verification logic to the SCAI APIs, much how we validate supported hash algorithms in resource descriptors.

The benefits I see for this dedicated predicate type is that it can evolve more easily (e.g. adding new fields) and is simpler to use for consumers (possibly fewer changes to the spec and the data "type" is encoded at the statement layer instead of the predicate layer).

I definitely understand this rationale. And the predicates are meant to be living specs that evolve as the community gathers feedback from users/consumers about use cases or usability. This is meant to be a community-driven process, so subsequent versions of the SCAI predicate (or any other) can definitely be amended to add/change fields, and better suit the broad use cases they were designed for.

Could you please explain what you mean by "the type is encoded at the statement layer" in the case of the dedicated reference predicate? If a consumer just sees predicateType: "https://in-toto.io/attestation/reference/v0.1", it still needs to step into the predicate layer and iterate over the list of references in order to understand what types of documents/resources are being referenced. I could just be missing something.

@marcelamelara
Copy link
Contributor

Given that we have the needed approvals for this PR, I won't stand in the way of merging.

@TomHennen TomHennen merged commit e8c8774 into in-toto:main Oct 2, 2024
1 check 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.

5 participants