Trusted Artifact chain policy

[zregvart]

Given the change in[1], and the new data available in the attestation, we can create a chain of PipelineRun tasks that contributed to the image artifact being built.

Namely, the *_ARTIFACT parameters of a task should reference the *_ARTIFACT results of a task whose artifact(s) it depends on.

Given such a chain, we can examine if all the Tasks comprising that chain are acceptable.

This assumes that any acceptable Task conforms to using only the trusted artifacts when performing the its undertaking.

reference: https://issues.redhat.com/browse/EC-252

[1] konflux-ci/build-definitions#712

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2f7d9bf) to head (1f73321).

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #868    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          105       107     +2     
  Lines         4952      5052   +100     
==========================================
+ Hits          4952      5052   +100     

Files	Coverage Δ
policy/lib/tekton/task.rego	100.00% <100.00%> (ø)
policy/release/trusted_artifacts.rego	100.00% <100.00%> (ø)
policy/release/trusted_artifacts_test.rego	100.00% <100.00%> (ø)
policy/release/trusted_task.rego	100.00% <ø> (ø)

[chmeliik]

I can think of two ways to get around the check in its current state:

1. use a result name that doesn't end with _ARTIFACT

params:
- name: SOURCE_ARTIFACT
  value: $(tasks.my-task.results.SOURCE_TARBALL)

2. Don't use a result at all, pass a hardcoded value

params:
- name: SOURCE_ARTIFACT
  value: 'file:sha256-{digest_known_ahead_of_time}'

[lcarva]

I can think of two ways to get around the check in its current state:

1. use a result name that doesn't end with _ARTIFACT

params:
- name: SOURCE_ARTIFACT
  value: $(tasks.my-task.results.SOURCE_TARBALL)

2. Don't use a result at all, pass a hardcoded value

params:
- name: SOURCE_ARTIFACT
  value: 'file:sha256-{digest_known_ahead_of_time}'

EC sees the resolved value, e.g. file:sha256-abc... So it shouldn't matter how the value is populated. If you can predict what the digest of the artifact will be, hard coding that value is fine. It also fine if you are getting the digest from a random Task - if it's the digest of an artifact produced by a, another, trusted task, great. Otherwise, EC should pick this up as a violation.

A successful check bypass would involve tricking EC into thinking that a certain digest came from a trusted task when it didn't.

It's important to note that the Tasks in this repo are mostly blindly trusted. If someone were to update the buildah task definition, for example, to not use the expected param name, or to even use the artifact without checking its digest, it would totally fool EC. We have a spike to see what we can do about this.

[chmeliik]

If I understand this PR correctly, the check:

1. constructs a dependency graph from the build task to the tasks that pass results to it
2. checks that all the tasks reachable from build-container (prefetch-dependencies, git-clone) are trusted

            build-container
            /            \
           /              \
    SOURCE_ARTIFACT   CACHI2_ARTIFACT
           \              /
            \            /
         prefetch-dependencies
                 /
                /
         SOURCE_ARTIFACT
              /
             /
         git-clone

IIUC the tree is constructed by matching the result values of each task to the param values of each task. But

1. if the param value is hardcoded, no task will have such a result, no task will be added to the dep tree and the check will pass
2. it filters out the results and params whose names don't end with _ARTIFACT (easy to fix)

            build-container
                  \
                   \
        CACHI2_ARTIFACT = hardcoded 'file:sha256:...'
                     .
                      .
                <dangling param>



            build-container
                  \
                   \
            param_name = SOURCE_ARTIFACT
                     .
                      .
   <broken chain because the result name doesn't end with _ARTIFACT>
                        .
                         .
                result_name = SOURCE_TARBALL
                            \
                             \
                         my-custom-task
                               \
                                \
                            SOURCE_ARTIFACT
                                  \
                                   \
                                git-clone

[lcarva]

@chmeliik, you are correct. We should have a policy rule that ensures that any used *_ARTIFACT parameter is found in the result of another trusted Task. Would that address this issue?

[chmeliik]

@chmeliik, you are correct. We should have a policy rule that ensures that any used *_ARTIFACT parameter is found in the result of another trusted Task. Would that address this issue?

I think that covers the hardcoded params problem 👍 (assuming that the trusted-artifact params of trusted tasks will always be called *_ARTIFACT)

For the "result name doesn't end with _ARTIFACT" problem, the fix is probably to just not filter results by name

[zregvart]

We raised the issue of all trusted artifact inputs being produced on the pipeline on one of the first team demos of this feature.

The threat modeling for such a case goes something like this: a malicious actor introduces an untrusted Task on the pipeline that modifies or produces a trusted artifact directly into the artifact store, the same malicious actor also needs to modify the pipeline so that the input artifact (Task parameter) is not set to a fixed value. The newly added rule in 62c9c5d#diff-91d65f0192b90a30de0005fc38d3f727ef11a0dfb68e5b5f34d6ce988917279cR79-R114 should check that all input artifacts have been produced on the pipeline. The malicious actor could still produce the artifact on the pipeline, i.e. set the value of the Task result to circumvent that new rule, though such attempt should be caught by the existing rule making sure that all Tasks on the chain are acceptable.

The issue of result/parameter names not being named *_ARTIFACT should be negated via the Task definitions of acceptable Tasks, as long as they have their results and parameters named with *_ARTIFACT convention any tampering should be detected by the newly added rule as well. We rely on the acceptable Tasks to maintain this convention, we could also enforce this via a Task definition rule. Created #876 for a followup on that.

[lcarva]

Let's revive this.

[zregvart]

I've rebased and added one more test case for when there are not TA parameters/results on the pipeline. What we also need is a change in policy.release.trusted_task package that will allow non-trusted Tasks on the pipeline if the built artifact was produced in the TA chain of trusted Tasks. One approach could be to merge these two packages into one and then branch on the length of the TA chain?

[lcarva]

I've rebased and added one more test case for when there are not TA parameters/results on the pipeline. What we also need is a change in policy.release.trusted_task package that will allow non-trusted Tasks on the pipeline if the built artifact was produced in the TA chain of trusted Tasks. One approach could be to merge these two packages into one and then branch on the length of the TA chain?

Merging the packages probably makes sense. I think we need to take into account two scenarios:

1. If using TA, then it's ok to have non-trusted Tasks. (Plus all the validation around TA.)
2. Otherwise, every Task in the Pipeline must be trusted.

I'd like to focus on the usage aspect of this. The same policy config should work with either scenario above. This should simplify migration.

[lcarva]

We probably also want to verify non-builder Tasks. For example, the sast-snyk-check-oci-ta Task does not produce an image, but it should consume the SOURCE_ARTIFACT produced from a trusted Task. So maybe the convention here is look at any Task that does take a *_ARTIFACT parameter?

[lcarva]

Changes were merged as part of #1007. I'll close this out and have a follow up PR/stories to address some of the comments.

[lcarva]

We probably also want to verify non-builder Tasks. For example, the sast-snyk-check-oci-ta Task does not produce an image, but it should consume the SOURCE_ARTIFACT produced from a trusted Task. So maybe the convention here is look at any Task that does take a *_ARTIFACT parameter?

Filed https://issues.redhat.com/browse/EC-652 to follow up on this.