ITE-11: Support attribute constraints in layouts

[adityasaky]

This was previously part of #38. I've broken that one down into separate ITEs so it's easier to reason about the changes proposed.

Prototyping here: https://github.com/adityasaky/in-toto-attestation-verifier

[mikhailswift]

CEL seems like a solid choice for this. It's simple enough to reason about easily while still being powerful enough to do the vast majority of the types of policies surrounding predicates that we care about.

Cole and I have been working with some more complex policies that have us thinking about writing policies over multiple predicates. However, I think that idea would need to be fleshed out a lot further before we could really reason about them here and if we can avoid that I think we'd be better off for it.

[TomHennen]

This seems like a big improvement. It seems like combining CEL with in-toto might resolve some of the difficulties of using CEL too.

[TomHennen]

nit: I find the "If this ITE is applied" language a bit confusing (especially given the "If instead" construction below).

The conditionality makes it sound like it's up to the user to decide where to use the ITE, when the proposal is actually to apply the ITE to the entire spec and make all of these formulations possible.

If this just the standard way to write ITEs that's cool.

[adityasaky]

The conditional is meant to apply to the type of layout. It might help to replace the original in-toto layout with references to v1.0. The reason is, this could easily apply to legacy layout schemas without depending on the layout proposed in #49.

[trishankatdatadog]

My biggest feedback for this ITE is that it will be impossible to get consensus on an expression language of choice. We could ofc be strongly opinionated about it, but I think we should repeat what we did with signing envelopes: while we do not mandate any particular one, we strongly recommend DSSE, and support it out-of-the-box. We should be similarly liberal about the choice of policy engine while choosing one by default.

[trishankatdatadog]

As I said earlier in a comment, my biggest feedback for this ITE is that it will be impossible to get consensus on an expression language of choice. We could ofc be strongly opinionated about it, but I think we should repeat what we did with signing envelopes: while we do not mandate any particular one, we strongly recommend DSSE, and support it out-of-the-box. We should be similarly liberal about the choice of policy engine while choosing one by default.

However, I am happy to approve this PR as a working draft so that we can iterate and make progress. Sorry for the delay in review, and thanks for all your hard work!

[adityasaky]

I agree, I think it makes sense to generally call for an expression language with a set of properties, and recommend the use of a single one, possibly / likely CEL.

[trishankatdatadog]

I agree, I think it makes sense to generally call for an expression language with a set of properties, and recommend the use of a single one, possibly / likely CEL.

To properly generalise at the risk of slightly complicating this ITE, there may be need to be an attributesType similar to predicateType, either at the top-level (simpler but more rigid) or predicate (more complicated but more flexible) scope.

We may also want to discuss the security implications (e.g., computational or memory constraints) of choosing any expression language.

[MarkLodato]

A raw list of CEL expressions might be a bit limiting. If something fails, it's difficult to present a clear error message of what went wrong. You might want to have a field for error messages. (Similarly, giving each an optional name might make it more readable.)

For example:

expectedAttributes:
  - name: builderId
    require: "predicate.runDetails.builder.id == 'https://example.com/myBuilder'"
    errorMessage: '"Unexpected builder.id: expected \"https://example.com/myBuilder\", got " + predicate.runDetails.builder.id'
  - name: repository
    require: "predicate.externalParameters.repository == '<expected value>'"
    errorMessage: '...'

[adityasaky]

#55

This is really great! I'm going to work on this and support it in attestation-verifier, but without blocking merging this ITE as draft as it's been an open PR for some time now.

[JustinCappos]

I'm okay to have this merge in as a draft. I would like to see the backwards compatibility part worked on and a little more of the links to the due diligence we did on CEL listed in the document.

[JustinCappos]

Backwards compatible with what? Is this a legacy client and a new layout? A new client and a legacy layout, etc.? I don't think it is always backwards compatible.

[adityasaky]

Clarified this some more to indicate that backwards compatibility isn't a given, we may have situations where a check that ought to have failed actually passes because a legacy client skips attribute checks.

[nmiyake]

Checking for clarification: the in-toto spec for the Steps format specifies these fields as expected_materials and expected_products.

I haven't seen any ITEs or spec updates that change this, but ITE-10 and this ITE-11 (as well as the project at https://github.com/in-toto/attestation-verifier) seem to use this camelCase for these same fields.

Is it the case that:

1. The documentation refers to these fields in camelCase, but that's just for convenience, and actual serialization should remain snake_case
2. The layout schema outlines the fields that should exist, but the actual naming/serialization format for the fields is considered implementation-dependent and is not important to standardize
3. There is an intent to move to camelCase for the standard naming of fields in the layout schema, but the spec just hasn't been updated

We are using our own in-toto implementation for verification and are attempting to adhere to the spec as much as possible/practical, but are having a hard time determining whether there is a correct/canonical serialization format based on these inconsistencies.

[adityasaky]

For me personally, it was mostly 1, though it's something to consider and nail down for compatibility / consistency. Happy to change these back to snake case unless there's interest in using this breaking change to also change the serialization format. I don't feel strongly one way or another.

FWIW, I think we should probably reject 2 if it does come up; it's tempting to treat this as an implementation detail but we should aim for maximum compatibility of layouts and corresponding tooling. @jkjell we should coordinate this, if applicable, wrt witness policies too.