Skip to content

feat(policies): evaluate policies on attestation push command - #1087

Merged
jiparis merged 78 commits into
chainloop-dev:mainfrom
jiparis:122-push
Jul 17, 2024
Merged

feat(policies): evaluate policies on attestation push command#1087
jiparis merged 78 commits into
chainloop-dev:mainfrom
jiparis:122-push

Conversation

@jiparis

@jiparisjiparis commented Jul 11, 2024

Copy link
Copy Markdown
Member

This PR is a follow up that connects all the pieces to evaluate any policy present in the schema before pushing the attestation. The PR includes:

### features
- [x] Evaluating policies against materials during `att add`
- [x] Evaluating policies against statement (not signed) during `att push`
- [x] validate schema in crafter (during `contract create` and `contract update`)
- [x] Storing evaluation results in the predicate, included in the signed attestation, and sent to the controlplane.

Policies can be specified and linked to any schema. For example, this policy can be used to evaluate any material of type SBOM_SPDX_JSON, and check that was created with Syft.

# File policy-made-with-syft.yaml
apiVersion: workflowcontract.chainloop.dev/v1
kind: Policy
metadata:
name: made-with-syft
spec:
type: SBOM_SPDX_JSON
embedded: |
package main
import future.keywords.in
deny[msg] {
not made_with_syft
msg := "Not made with syft"
}
made_with_syft {
some creator in input.creationInfo.creators
contains(creator, "syft")
}

Policy code can also be detached and referenced from the spec. I'll update the documentation in a separate PR.
Then, it can be linked to any contract:

# my-schema.yaml
schemaVersion: v1
materials:
- name: sbom
type: SBOM_SPDX_JSON
policies:
materials:
- ref: "policy-made-with-syft.yaml" attestation:
- ref: "policy-author.yaml"

Note the policies.materials and policies.attestation section, denoting policies to be applied to all materials or the whole attestation (before signing) accordingly. In the case of materials, since the policy has a type in the specification, it will be applied to all materials that matches that type. There are other options and filters to flexibilize the policy selection criteria, allowing to apply policies to specific materials. For example, it can be filtered by name:

...
policies:
materials:
- ref: "policy-made-with-syft.yaml" selector:
name: "sbom" # only apply the policy to the "sbom" material

If there is no type in the policy spec, it will be applied regardless the material has a compatible type or not. This is useful for custom validations and artifacts.

Closes#122

jiparis added 30 commits July 9, 2024 13:47
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Comment threadapp/cli/cmd/attestation_push.go Outdated
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Comment threadapp/controlplane/api/workflowcontract/v1/crafting_schema.proto Outdated
Comment threadapp/controlplane/api/workflowcontract/v1/crafting_schema.proto Outdated
Comment threadinternal/attestation/crafter/testdata/contracts/with_policy_embedded.yaml Outdated
@@ -0,0 +1,4 @@
schemaVersion: "v1"
policies:
attestation:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

in the future we should also have some examples with material attachments, all of them in this directory seem to be attestation.

Comment threadinternal/attestation/crafter/crafter.go Outdated
Comment threadinternal/attestation/crafter/crafter.go Outdated

@migmartrimigmartri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice!

Approving modulo making sure that we are not embedding the policies on contract create/update

Comment threadinternal/attestation/crafter/crafter.go
Comment threadpkg/policies/policies.go Outdated
Comment threadpkg/policies/policies.go
Comment threadpkg/policies/policies.go Outdated
Comment threadpkg/policies/policies.go
Comment threadpkg/policies/policies.go Outdated
Comment threadpkg/policies/policies.go Outdated
Comment threadpkg/policies/policies.go
Comment threadinternal/attestation/renderer/chainloop/v02.go Outdated
Comment threadinternal/attestation/renderer/renderer.go Outdated
jiparis added 12 commits July 16, 2024 17:48
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
if p.PolicyEvaluations == nil {
p.PolicyEvaluations = make(map[string][]*v1.PolicyEvaluation)
}
p.PolicyEvaluations["ATTESTATION"] = policyResults

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should use the string representation of the MaterialType enum to avoid future inconsistencies

Suggested change
p.PolicyEvaluations["ATTESTATION"] =policyResults
p.PolicyEvaluations[schemaapi.CraftingSchema_Material_ATTESTATION.String()] =policyResults

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Ey, thanks for the suggestion @jotadrilo . But in this case it's not a material, but policies applied to the whole attestation. I know the naming is confusing, we might choose a different key for this particular case. I'll give it a try.

@jotadrilojotadriloJul 17, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

But you are mapping materials and policies, right? In other words, you are grouping policies by their material name.

If this specific grouping has a different semantic, I'd better use a different field in the in-toto predicate (v2), or add a reserved MaterialType name for this kind of grouping.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, we were evaluating those options. We'll revisit this in next iterations, as it might be part of a batch with other improvements. Thanks!

}

_, err = base64.StdEncoding.Decode(rawMaterial, []byte(envelope.Payload))
rawMaterial, err = envelope.DecodeB64Payload()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

told you :)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes! And it also implements URLEncoding which is is supported by DSSE in addition to StdEncoding.

Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
@jiparis
jiparis merged commit 05309bc into chainloop-dev:mainJul 17, 2024
@jiparis
jiparis deleted the 122-push branch July 17, 2024 10:25
Sign up for freeto 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.

Support policies for attestations

4 participants

@jiparis@migmartri@jotadrilo@javirln