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 an attestation for policy verification #295

Closed
wants to merge 2 commits into from
Closed

Conversation

kpk47
Copy link

@kpk47 kpk47 commented Nov 1, 2023

This attestation is based on the SLSA VSA, but modified to support arbitrary security policies.

Fixes #277

@kpk47 kpk47 requested a review from a team as a code owner November 1, 2023 23:13
}],

// Predicate
"predicateType": "https://slsa.dev/verification_summary/v1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Update to https://in-toto.io/attestation/verification_summary/v1 ?

Copy link
Author

Choose a reason for hiding this comment

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

Done

"<String>": <Int>,
...
},
"slsaVersion": "<MAJOR>.<MINOR>",
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is no longer a slsa attestation should slsaVersion be dropped? If necessary (and I'm not sure it is) it could just be included in the names of the various verifiedProperties.

Copy link
Member

Choose a reason for hiding this comment

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

+1, moving this to verifiedProperties makes sense to me.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

We can define how to express SLSA version in the SLSA spec. We're going to need a page on how to use VSAs with SLSA, so we can work out the details there.

}
},
"timeVerified": <TIMESTAMP>,
"resourceUri": <artifact-URI-in-request>,
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: Would this be a good time to consider if we should change this name?

It's a bit confusing that this field name also happens to match this type name.

Would verifiedUri be better??

Copy link
Member

Choose a reason for hiding this comment

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

Do we need this at all? Can't this be included along with the subject?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what I originally thought when we were developing this internally, but @AdamZWu made some compelling arguments for separating it. At least one of those arguments was that it seemed to violate layering.

I think we also recently ran across a case and thought "oh wow, it's a good thing we have this separate field and don't have it in the subject". Maybe something to do with having VSAs that have lots of subjects but really only one resourceUri applies. If it were in the subject it needs to be repeated for each digest even if it's the same thing and that can result in very large attestations. @AdamZWu should more more details.

`verifiedProperties` _array (_string), required_

> Indicates the properties verified for the artifact (and not
> its dependencies), or "FAILED" if policy verification failed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I seem to remember @AdamZWu had some difficulties when implementing this. Adam is it fine to keep this or would it be better to say the array should be empty?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's easier to leave it as-is for now so that semantic changes are kept to a minimum?

Copy link
Member

Choose a reason for hiding this comment

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

I'm also trying to understand how this is populated. Is this based entirely on the producer<->consumer to reconcile?

Copy link
Contributor

Choose a reason for hiding this comment

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

My main reason against having the "FAILED" property is described here: slsa-framework/slsa#968 (comment)

But I am fine for leaving it as a future improvement rather than cram everything is a big change.

Is this based entirely on the producer<->consumer to reconcile?

My understanding is, sort of. In the context of in-toto specs, yes, in-toto simply provides this field for the producers and consumers, there is no motivation to further regulate its uses. However, under the bigger picture of building a network of secure software supply chains, these are needs for standardization -- If organization A produces "apple" in verifiedProperties and organization B expects to see "orange", they cannot effectively exchange the trusted build data.

This, I think, is where SLSA comes in. SLSA defines a set of standardized trusted build properties and levels, which everyone agrees upon and mandates that in the VSA, so that, in addition to "apple", organization A must put "SLSA_..." properties in verifiedProperties; this makes it possible for organization B to consume that VSA -- they don't know what "apple" means, but they do know what "SLSA_..." means. :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: if no one objects lets remove 'FAILED'. If anyone does object we keep it?

Copy link
Author

Choose a reason for hiding this comment

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

I'm happy dropping 'FAILED'. An empty list conveys the same information but feels less error-prone to me.

> Indicates the properties verified for the artifact (and not
> its dependencies), or "FAILED" if policy verification failed.
>
> If including SLSA levels, users MUST NOT include more than one level per SLSA
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this section be removed here and go with the SLSA docs?

Copy link
Author

Choose a reason for hiding this comment

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

Done

> that were verified at the indicated level. Absence of a given value
> MUST be interpreted as reporting _0_ dependencies with that value.
>
> If including SLSA levels, users MUST count each dependency only once per SLSA
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this text can be generalized.

If including 'leveled' properties (e.g. SLSA), users MUST count each dependency once per property, at the highest level verified.

Copy link
Author

Choose a reason for hiding this comment

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

That could work, but I prefer to delete this text (even if we keep the field) and incorporate it into the SLSA spec's description of how to use a VSA for SLSA.

spec/predicates/vsa.md Show resolved Hide resolved
- Added optional `input_attestations` field.
- 0.1: Initial version.

[SLSA Provenance]: /provenance
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this link needs to be updated.

Copy link
Author

Choose a reason for hiding this comment

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

Done

Records a SLSA verification decision about an artifact.
Verification summary attestations communicate that an artifact has been verified
against some policy and details about that verification. Such details may
include, but are not limited to, what SLSA level the artifact and/or its
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: link to SLSA?

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you bring over 'How to verify' too? https://slsa.dev/spec/v1.1/verification_summary#how-to-verify

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Also, I think this will have to discuss a variety of verification options including in-toto layouts (ITE-10/11).

Copy link
Author

Choose a reason for hiding this comment

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

I moved the "How to verify" section from SLSA and updated it to use the new field names. I'm not sure I could write in much detail about how to verify using in-toto layouts. Could that be follow-on work?

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.

Thanks for working on moving this over to in-toto!

@@ -1,11 +1,279 @@
# Predicate type: SLSA Verification Summary (VSA)
# Predicate type: Verification Summary Attestation (VSA)
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts on dropping the "attestation"? I think this is the only one that includes it at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it still be abbreviated VSA or would it be just VS? (or maybe we don't abbreviate at all)

Copy link
Author

Choose a reason for hiding this comment

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

Dropped "attestation" and the abbreviation. I suspect we'll still call them VSAs, but it looks weird to me to have "Verification Summary (VSA)" in the title.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Also, I think this will have to discuss a variety of verification options including in-toto layouts (ITE-10/11).

}
},
"timeVerified": <TIMESTAMP>,
"resourceUri": <artifact-URI-in-request>,
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this at all? Can't this be included along with the subject?

},
"timeVerified": <TIMESTAMP>,
"resourceUri": <artifact-URI-in-request>,
"policy": {
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 this can become a list of resource descriptors. We could, for example, also ship the policy keys that were verified alongside the policy itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is already a ResourceDescriptor (but not a list) according to the doc below. I guess we just need to update this summary?

Copy link
Author

Choose a reason for hiding this comment

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

@adityasaky What do you mean by policy keys, and why should we include them?

@TomHennen AFAICT this example already meets the ResourceDescriptor syntax requirements. What update do we need heer?

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT this example already meets the ResourceDescriptor syntax requirements. What update do we need heer?

Hmm, maybe nothing. When I first saw this I thought that maybe it should look something like

"policy": <ResourceDescriptor>

Copy link
Contributor

Choose a reason for hiding this comment

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

Or "policy": #ResourceDescriptor, to match https://slsa.dev/spec/v1.0/provenance#schema

Copy link
Contributor

Choose a reason for hiding this comment

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

@adityasaky , I agree that it makes sense to support lists of policies. Even in the SLSA use case, there may be specific individual policies for each track. It would be beneficial to be able to aggregate all policies into a single VSA. There could additionally be policies that are owned/maintained by various organizations -- being able to differentiate between them (and their results) separately seems like a good direction for transparency.

Copy link

Choose a reason for hiding this comment

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

A list would be nice, but the same could also be achieved by creating multiple VSAs. We should explore which option makes the most sense.

Copy link
Member

Choose a reason for hiding this comment

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

@kpk47 In the case of Witness policies (and I think in-toto layouts), you would have keys to sign the policy itself, and potential keys for each functionary that performed an operation. The later could be embedded in the policy but, the signing key for the policy itself is an out-of-band communication (needs a separate resource description).

As far as multiple policies go... I think it's going to get complicated fast. What logic is applied between multiple policies? You could defer that decision making process external to the VSA if you're only capturing discrete results per policy but, for me, that's counter to the purpose of capturing a policy decision in a VSA. I think that @lcarva is on the write track with additional VSAs. Each could be inputs to a subsequent policy or future decision.

Copy link
Contributor

Choose a reason for hiding this comment

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

What logic is applied between multiple policies?

That would depend on the policy engine responsible for creating the VSA. If multiple policies are given as input to a decision, the VSA-generating tool would be responsible for reconciling any conflicts/inconsistencies and generating the ultimate pass/fail. Even if a policy engine supports multiple policies as input, it might elect to create multiple VSAs.

You could defer that decision making process external to the VSA if you're only capturing discrete results per policy but, for me, that's counter to the purpose of capturing a policy decision in a VSA.

Depending on the cross-relationship between different policies for the policy engine, this might not be straightforward. Another potential approach could be for an engine to create pseudo policies that are composed of multiple interrelated sub policies and just report that. One potential use case for multiple policies would be if there were policy modifications on-the-fly, i.e. using inline policies as mentioned by @lcarva: #295 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

Apologies for the delay, my notifications have been a mess the last 3 weeks or so.

@jkjell covered my point about policy keys above. I'm actually less convinced a VSA needs to cover multiple actual policies. I think a verification workflow should be scoped per policy artifact (and applicable attestations), so if multiple policies are verified, we should generate multiple VSAs. This, to me, makes sense given the expected use cases for VSAs: record of verification elsewhere in the supply chain or for policy delegations like in-toto's sublayouts.

"digest": { /* DigestSet */ }
}
"inputAttestations": [
{
Copy link
Member

Choose a reason for hiding this comment

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

This could / should also be resource descriptors.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is, but maybe we just need to update the summary?

Copy link
Author

Choose a reason for hiding this comment

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

Same question as above: what update are you requesting?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we wanted to update this it could say

Maybe it should match what's done here? https://slsa.dev/spec/v1.0/provenance#schema

"inputAttestations": [ ...#ResourceDescriptor ],

spec/predicates/vsa.md Show resolved Hide resolved
`verifiedProperties` _array (_string), required_

> Indicates the properties verified for the artifact (and not
> its dependencies), or "FAILED" if policy verification failed.
Copy link
Member

Choose a reason for hiding this comment

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

I'm also trying to understand how this is populated. Is this based entirely on the producer<->consumer to reconcile?

"<String>": <Int>,
...
},
"slsaVersion": "<MAJOR>.<MINOR>",
Copy link
Member

Choose a reason for hiding this comment

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

+1, moving this to verifiedProperties makes sense to me.

> there is no need to include more than one level per track.

<a id="dependencyProperties"></a>
`dependencyProperties` _object, optional_
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be nice to have more guidance for this one too. I think it'll be needed as this generalizes beyond the SLSA-specific schema.

Copy link
Member

Choose a reason for hiding this comment

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

And possibly we'd want to define "dependency" in this spec as well?

> A count of the dependencies verified to have each property.
>
> Map from _string to the number of the artifact's _transitive_ dependencies
> that were verified at the indicated level. Absence of a given value
Copy link
Member

Choose a reason for hiding this comment

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

If the dependencies are verified shouldn't that generate a VSA or some other evidence of its own to reference?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you asking why we need this map when we could just include all the VSAs for the transitive dependencies?

Copy link
Author

Choose a reason for hiding this comment

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

@jkjell You can include VSAs for each transitive dependency in the attestation bundle, but doing so for a nontrivial piece of software will mean creating a large attestation bundle that may be impractical to evaluate. Avoiding that giant bundle is kind of the point of a VSA.

That being said, I'm in favor of removing this field. It isn't clear to me why you'd want dependencies' security properties summarized in the VSA instead of implicitly through the evaluated policy. Alternatively, I could imagine gathering this data on a dashboard if you had a database of VSAs for the subject's transitive dependencies. Such a dashboard could be updated in real time based on whatever data source the VSA producer used to fill these fields. WDYT @TomHennen ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since VSAs are often used to cross organization boundaries I don't think it's always policy to know the actual policy used across all orgs with all dependencies, likewise it may not be possible to have a real-time dashboard that could provide these feedback cross-org.

Certainly in some cases it may work (seems like a good use case for GUAC), but leaving it in VSAs would solve that problem.

We certainly still have this use case internally so we'd need to support it, maybe by adding an org specific field. My preference would be to keep it, perhaps as a SLSA specific field addition, but we can probably work around it (I'd just be sad).

@jkjell You can include VSAs for each transitive dependency in the attestation bundle, but doing so for a nontrivial piece of software will mean creating a large attestation bundle that may be impractical to evaluate. Avoiding that giant bundle is kind of the point of a VSA.

When consuming a piece of software via its VSA you shouldn't need any additional attestations. So just one VSA.

If you're verifying the provenance of some piece of software though, then you may want VSAs for each of the direct dependencies (which will cover their transitive dependencies). You don't need them all. So it shouldn't wind up being that big.

Copy link
Member

Choose a reason for hiding this comment

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

VSAs are often used to cross organization boundaries I don't think it's always policy to know the actual policy used across all orgs with all dependencies

@TomHennen do you mean the policy of the dependency that is verified or the policy of the VSA itself? If policy is unknown for the VSA, then the VSA is nothing more than a signature for external parties. I suppose that might be ok in some situations but, it's a reversion to implicit trust.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kpk47 and I spoke offline, it sounds like a reasonable approach is:

  1. Remove dependencyProperties from this predicate type (to avoid in-toto needing to define the concept of dependencies and other confusing problems)
  2. Have SLSA define something like the dependencyProperties field when using the in-toto VSA following the existing in-toto guidance on extension fields.

Does this sound reasonable to everyone?

Copy link
Contributor

Choose a reason for hiding this comment

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

VSAs are often used to cross organization boundaries I don't think it's always policy to know the actual policy used across all orgs with all dependencies

@TomHennen do you mean the policy of the dependency that is verified or the policy of the VSA itself? If policy is unknown for the VSA, then the VSA is nothing more than a signature for external parties. I suppose that might be ok in some situations but, it's a reversion to implicit trust.

I think maybe removing the thing resolves this, but I don't think it's totally true to say that it's "nothing more than a signature". I agree that it would be ideal if people can see the original evidence and policies that were used throughout the entire supply chain (and the VSA, provenance, and other attestations let people do that), but it won't always be possible (some orgs may not be willing or able to make that data public because of proprietary information).

However, even if they can't make the policy and other attestations public, the VSA still includes the verifiedProperties fields and the verifier.id field, which means that the VSA is better than a plain signature in one important respect, it makes it clear that the verifier (verified.id) is asserting that the software covered by the VSA has some specific but high level properties (verifiedProperties). That can be pretty useful when orgs are exchanging software?

So, the way I see it, plain signature < VSA without public evidence or policy < VSA with public evidence and policy?

But maybe this isn't the place to have that discussion?

Copy link
Member

Choose a reason for hiding this comment

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

I think the verifiedProperties was the piece I was missing. Establishing trust in a verifier AND that it asserts a given set of properties were confirmed by a policy is a great way of abstracting the value of the policy from the policy itself.

>
> Map from _string to the number of the artifact's _transitive_ dependencies
> that were verified at the indicated level. Absence of a given value
> MUST be interpreted as reporting _0_ dependencies with that value.
Copy link
Member

Choose a reason for hiding this comment

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

Is this MUST specific to a particular verifier? Is there an expectations that all dependency properties are known in advance?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to get @AdamZWu's thoughts here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it would be quite hard to require every verifier to know all dependency properties in advance. :P

So yeah "Absence of a given value MUST be interpreted as reporting 0 dependencies..." seems to be too strong of a requirement.

One possible fix is to introduce SLSA's "track" concept here (e.g. SLSA_BUILD_{0,1,2,3}):

  • The presence of any value in a particular track means that the track has been verified;
  • And within a verified track, we can safely have: "Absence of a given value MUST be interpreted as reporting 0 dependencies..."

Copy link
Author

Choose a reason for hiding this comment

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

As I commented above, my proposed solution is to delete the dependencyProperties field.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with removing dependencyProperties. Perhaps it would be worth considering a more generic policy result? We could allow verifiedProperties to be an object which encapsulates verifier specific information. This could also improve trust without knowing the contents of a policy.

The VSA also allows consumers to determine the verified levels of
all of an artifact’s _transitive_ dependencies. The verifier does this by
either a) verifying the provenance of each non-source dependency listed in
the [resolvedDependencies](/provenance/v1#resolvedDependencies) of the artifact
Copy link
Member

Choose a reason for hiding this comment

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

I think this a link to SLSA Build provenance? There may be value for the generic VSA to define an internal version of a resolved dependency for dependencies without a provenance document. I'm currently thinking that along the lines that a dependency could be almost anything that is the subject of an attestation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, that seems like it would be pretty hard to define and reason about.

But it does seem odd for this spec to depend on SLSA build provenance. Maybe this is an example of why VSAs might stay better with SLSA?

Copy link
Author

Choose a reason for hiding this comment

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

I left a longer comment down below, but I'm wondering if we should just remove dependencies from VSAs. They have started to feel like a false start towards recursive SLSA, which I'd prefer to address in a dependency track or external monitoring.

Copy link
Contributor

Choose a reason for hiding this comment

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

Transitive dependency verification can also be achieved by having a policy engine which performs the tree walking. It can be abstracted to be represented by the policy's URI.

<a id="policy"></a>
`policy` _object ([ResourceDescriptor]), required_

> Describes the policy that the `subject` was verified against.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We should refer to the resource of the resourceUri or change resourceUri to subjectUri just to be painfully obvious about the relationship between these two.

> A count of the dependencies verified to have each property.
>
> Map from _string to the number of the artifact's _transitive_ dependencies
> that were verified at the indicated level. Absence of a given value
Copy link
Member

Choose a reason for hiding this comment

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

VSAs are often used to cross organization boundaries I don't think it's always policy to know the actual policy used across all orgs with all dependencies

@TomHennen do you mean the policy of the dependency that is verified or the policy of the VSA itself? If policy is unknown for the VSA, then the VSA is nothing more than a signature for external parties. I suppose that might be ok in some situations but, it's a reversion to implicit trust.

>
> Map from _string to the number of the artifact's _transitive_ dependencies
> that were verified at the indicated level. Absence of a given value
> MUST be interpreted as reporting _0_ dependencies with that value.
Copy link
Member

Choose a reason for hiding this comment

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

I agree with removing dependencyProperties. Perhaps it would be worth considering a more generic policy result? We could allow verifiedProperties to be an object which encapsulates verifier specific information. This could also improve trust without knowing the contents of a policy.

@jkjell
Copy link
Member

jkjell commented Nov 14, 2023

@mikhailswift has also been working on our implementation of VSAs in go-witness


Version: 1.0
Version: 1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

How does versioning work in this repository? I'd be uncomfortable calling this one PR final. Should there be a way to put it in a separate namespace or call it beta or similar, so that people don't rely on it until we're confident it's finalized?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, I would expect the 1.0 SLSA VSA attestation to be deprecated in favor of a 0.1 in-toto VSA since there is a fair amount of incompatible changes between these two.

Copy link
Member

Choose a reason for hiding this comment

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

Since the URI's domain is changing, can we drop the predicate version to v0.x?


For more details, please see https://slsa.dev/spec/v1.0/verification_summary
## Use Cases
Copy link
Contributor

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 should focus on delegation for now. That seems to be confusing everyone.

To me, the more obvious and short-term use case is to make a policy decision in one place in the supply chain and then rely on that decision later in the supply chain. You can think of this as caching a policy decision in a secure way. This is needed for reliability and UX reasons and is the original motivation for VSA. There is no question of trust because the producer and consumer of the VSA are in the same trust domain. Examples:

  • Kubernetes: For reliability, you want to make a policy decision once and then reuse that decision on every pod creation. You don't want your service to roll out half way then start to be blocked, or worse, have a perfectly running service break when a pod randomly restarts.

  • Package loads: A service loads artifacts at runtime and wants a guarantee that everything it loads meets some requirements, e.g. SLSA Build L3. It is too unreliable to do this check directly on every load for the same reason as Kubernetes above. Instead, you can check SLSA Build L3 at package upload time and create a VSA then, and then at load time just check the VSA.

In both cases, this is really like code signing with added metadata. With regular code signing, you would only accept artifacts that are signed by some key. With VSA, you accept artifacts with some key AND have some extra metadata. You could achieve the same thing by signing with a separate key for each property, but that is difficult for key management. It's easier to put that metadata in an attestation and have a single key (or really signing identity).

}
},
"timeVerified": <TIMESTAMP>,
"verifiedUri": <artifact-URI-in-request>,
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation below calls this resourceUri. I personally find verifiedUri confusing, since it's not clear what the direct object of "verified" is. Maybe appliesToUri if you want to change the name?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this different from the subject in the statement above? @jkjell suggests changing to subjectUri below: https://github.com/in-toto/attestation/pull/295/files#r1393385717 -- I was thinking something similar.

},
...
],
"verificationResult": "<PASSED|FAILED>",
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we remove this field. If there is a notion of "pass" or "fail", one can create an entry in verifiedProperties to represent that with more specific semantics.


Version: 1.0
Version: 1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

+1, I would expect the 1.0 SLSA VSA attestation to be deprecated in favor of a 0.1 in-toto VSA since there is a fair amount of incompatible changes between these two.

The VSA also allows consumers to determine the verified levels of
all of an artifact’s _transitive_ dependencies. The verifier does this by
either a) verifying the provenance of each non-source dependency listed in
the [resolvedDependencies](/provenance/v1#resolvedDependencies) of the artifact
Copy link
Contributor

Choose a reason for hiding this comment

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

Transitive dependency verification can also be achieved by having a policy engine which performs the tree walking. It can be abstracted to be represented by the policy's URI.

}
},
"timeVerified": <TIMESTAMP>,
"verifiedUri": <artifact-URI-in-request>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this different from the subject in the statement above? @jkjell suggests changing to subjectUri below: https://github.com/in-toto/attestation/pull/295/files#r1393385717 -- I was thinking something similar.

},
"timeVerified": <TIMESTAMP>,
"resourceUri": <artifact-URI-in-request>,
"policy": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@adityasaky , I agree that it makes sense to support lists of policies. Even in the SLSA use case, there may be specific individual policies for each track. It would be beneficial to be able to aggregate all policies into a single VSA. There could additionally be policies that are owned/maintained by various organizations -- being able to differentiate between them (and their results) separately seems like a good direction for transparency.

Comment on lines +217 to +224
"inputAttestations": [
{
"uri": "https://example.com/provenances/example-1.2.3.tar.gz.intoto.jsonl",
"digest": {"sha256": "abcd..."}
}
],
"verificationResult": "PASSED",
"verifiedProperties": ["SLSA_BUILD_LEVEL_3"],
Copy link
Contributor

Choose a reason for hiding this comment

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

The relationship between the policy and verification results that I find myself expecting is:

  • policy is a list of policies that are considered in the VSA
  • verificationResult is the net result -- whether all policies pass or fail
  • verificationProperties is some indication about the result from each policy taken into account (i.e. whether it is pass or fail)

What might be better, however, is to have keys following the pattern:

  • policies - the list of all policies that have been applied to create this VSA
  • result - the ultimate result of all policies combined
  • policyResults - the individual result of each policy applied. There would have to be easy mapping from each policy to its result, but this could potentially be done by assigning a unique name for each policy so that the full uri/digest don't have to be repeated.

Outside of the SLSA specification, it might be harder to realize value from a field like verifiedProperties.

Copy link
Member

Choose a reason for hiding this comment

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

the ultimate result of all policies combined

It's not clear to me how this should actually be calculated.

For my intended consumption of this, trust in a VSA will be based on a 1:1 mapping of trust in a policy. If I revoke trust in a policy (i.e. just to update the policy). I will revoke trust in all VSA's generated from that policy. Not arguing against the use case, I don't think we would ever use multiple policies in a single VSA. I'm curious to hear from others.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we consider policy+verificationResult vs. verifiedProperties as two sibling structures to convey trust? i.e.:

  • One could decide to delegate trust on a specific policy (if they know its content meets their needs), and just verify that: 1) the policy that they are interested has been verified; and 2) the overall verificationResult is a PASS.

  • Alternatively, one could decide they accept any artifact that meets a certain set of verifiedProperties, e.g. SLSA_BUILD_3 and SLSA_SOURCE_2 (imaginary), and not caring what exact policy triggered the verification of those properties. In that case, they will only be looking up their expected keys in verifiedProperties.

Of course, the latter requires the keys in verifiedProperties to be arbitrated (i.e. having a higher-level protocol like SLSA). Otherwise any verifier can put any random text in it that can be misunderstood by other verifiers.


OTOH, if we remove the verificationResult as @MarkLodato suggests here, then verifiedProperties becomes the only facility to convey trust.

Copy link
Member

Choose a reason for hiding this comment

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

For my intended consumption of this, trust in a VSA will be based on a 1:1 mapping of trust in a policy. If I revoke trust in a policy (i.e. just to update the policy). I will revoke trust in all VSA's generated from that policy. Not arguing against the use case, I don't think we would ever use multiple policies in a single VSA. I'm curious to hear from others.

@jkjell if policy A delegates to policy B, and therefore verification of policy A results in VSA A and VSA B, I don't think we can claim that revoking policy A automatically revokes policy B, at least in the in-toto sublayout context. Policy B may be delegated to by C, D, E, etc as well, so IMO VSA B is fine to continue to trust in isolation. Instead, as your verification starts at policy A (the root policy), you'd not consider VSA B in that instance of the workflow, rather than revoking VSA B.

(`verifier`) verified one or more software artifacts (the `subject` of an
in-toto attestation [Statement]) by evaluating the artifact and a `bundle`
of attestations against some `policy`. Users who trust the `verifier` may
assume that the artifacts satisfied the indicated security polciy without
Copy link

Choose a reason for hiding this comment

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

Minor typo: "polciy"

This might be necessary for legal reasons (keeping a software supplier
confidential) or for security reasons (not revealing that an embargoed patch has
been included).

Copy link

Choose a reason for hiding this comment

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

Another use case is that verification sometimes can be costly and not suitable for certain scenarios.

Something like Enterprise Contract can evaluate policies based on an image's labels or even one of its file's contents. This wouldn't really be possible to perform as a Kubernetes admission web hook.

"timeVerified": <TIMESTAMP>,
"verifiedUri": <artifact-URI-in-request>,
"policy": {
"uri": "<URI>",
Copy link

Choose a reason for hiding this comment

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

I would like the ability to provide an inline policy. This would account for certain scenarios where the policy is adjusted dynamically and may not have a published URI. I think this could be achieved with a data URL. Thoughts? If that's the intention, it would be great to clarify somewhere in this doc.

Copy link
Member

Choose a reason for hiding this comment

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

The resource descriptor allows for that using content: https://github.com/in-toto/attestation/blob/main/spec/v1/resource_descriptor.md.

@marcelamelara marcelamelara requested review from a team and removed request for AdamZWu January 23, 2024 17:15
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.

@kpk47 Thanks for getting this started. As others have noted, since we're looking to define this as a predicate separate from the existing SLSA-defined VSA, I'd recommend this PR introduce a brand new spec doc for the VS predicate that is separate from the VSA, rather than editing on top of the existing spec. So as not to lose all the feedback so far, it may make sense to open a new PR with the new doc.

@marcelamelara
Copy link
Contributor

Closing until we decide next steps for a resolution here. Let's pick up the discussion back in #277.

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.

Attestation for policy verification
9 participants