v1 VSA: `inputAttestations` should not differentiate between "unset" and "empty"

[MarkLodato]

Right now the spec says:

This field can be absent if the verifier does not support this feature; If present, this field must contain information on all the attestations used to perform verification. An empty collection indicates the decision was made without attestations.

But this is bad practice and goes against our parsing rules: we usually consider empty the same as unset because it's too subtle of a difference. Also protobuf cannot tell the difference between the two. If we do care about this, we need to use a separate field or wrap this in an object.

If we do choose to wrap in an object, we might want to gather policy and inputAttestations into a single inputs object.

/cc @TomHennen @AdamZWu

[AdamZWu]

IIRC the reason we defined the inputAttestations is for the possibility of reproducing the verification result. In that sense, wrapping policy and inputAttestations under something like verification_params makes sense -- reproducing verification is a feature, and if not supported, this key can be unset.

However, policy also plays a different role -- to identify the applicability of the summary. The same artifact can be verified against different policies, and the summary result is only applicable (i.e. reusable) when we know the new verification will be against the same policy.

And since checking applicability is a basic function of VSA, we need a separate "policy identifier" field, which is always present and independent of verification_params.policy.

I think this somewhat mirrors the semantics of "top-level source" in the SLSA provenance, which also has to be redundantly specified in both resolvedDependencies and externalParameters, as they serve difference purposes. WDYT?

[MarkLodato]

Oh, I didn't realize that policy.uri was required. I'll update #754 to fix that (which is tangentially related).

For this particular issue, it sounds like wrapping is more trouble than it's worth. Maybe can consider it in a v2, but for now let's not make such a major last-minute change.

@AdamZWu Do you need to differentiate between "none" and "unknown" for inputAttestations? If not, I think let's live without that option. If so, then let's add another field to represent "none"?

[AdamZWu]

We don't have any active implementation depending on this.

Strictly thinking about the specs and expected behaviors, I think there is really no need to differentiate "none" and "not set" -- for the purpose of reproducing verification, inputAttestations just serve the purpose of allowing sufficient evidences being fetched. So:

• If verification results can be reproduced without any attestation, not setting inputAttestations or using empty inputAttestations doesn't matter;
• If verification results would need attestation, either not setting inputAttestations or using empty inputAttestations would result in re-verification failure (almost immediately), hence the result is not reproduced.

Since unset or empty inputAttestations would not change the outcome of reproducing verification, I think maybe we don't even need to require one or the other... (or add another field)

I guess the only difference may reside in presenting the VSA to humans, i.e, "verification not reproducible" vs. "verification reproducible (without attestations)". But I guess we could live without that?

WDYT @TomHennen ?

[TomHennen]

I think that's good enough for v1! Thanks for the analysis Adam.