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

[feature] Comparison to new GitHub Artifact Attestations? #3618

Open
agilgur5 opened this issue May 10, 2024 · 10 comments
Open

[feature] Comparison to new GitHub Artifact Attestations? #3618

agilgur5 opened this issue May 10, 2024 · 10 comments
Labels
type:documentation Improvements or additions to documentation type:feature New feature or request

Comments

@agilgur5
Copy link

Is your feature request related to a problem? Please describe.
GitHub recently released Artifact Attestations as a new feature. It seems to overlap with the generators here, although I'm not entirely sure.

Describe the solution you'd like
A comparison of the two would be good to have. I'm not sure if Artifact Attestations would entirely replace the tooling here, but could perhaps be used together with them or as a subset?

Describe alternatives you've considered
n/a

Additional context
Saw the release of this new GH feature and it seemed very simple to use. Was wondering if it would make sense to use to implement SLSA for argoproj/argo-workflows#12099

@agilgur5 agilgur5 added status:triage Issue that has not been triaged type:feature New feature or request labels May 10, 2024
@ramonpetgrave64
Copy link
Collaborator

We have not done a full evaluation yet. But for now, we know that Github's Action is meant to be at L2, while our slsa-framework's Workflows are at L3.

@laurentsimon
Copy link
Collaborator

laurentsimon commented May 10, 2024

I don't remember if we record the ${{ vars.* }}. I recall opening a tracking issue for it a while back but I can't find it. Vars variables are another way to pass parameters to a build

@laurentsimon
Copy link
Collaborator

fwiw, it'd be fairly simple for a maintainer to obfuscate the code used to build their artifact by using a malicious (non-recorded) input combined with GH's script injection features :)

@agilgur5
Copy link
Author

True. We don't use workflow parameters though, so if that's the only missing piece, it sounds like we'd be able to use Artifact Attestations for L3?

@laurentsimon
Copy link
Collaborator

laurentsimon commented May 11, 2024

True. We don't use workflow parameters though, so if that's the only missing piece, it sounds like we'd be able to use Artifact Attestations for L3?

I would say no. The properties need to be project-agnostic, else there's no way for a verifier to trust builds from a builder across all their dependencies without inspecting the build workflow (defeats the purpose of SLSA levels). In SLSA the build level is associated with a builder and that's how we scale trust.

@laurentsimon
Copy link
Collaborator

laurentsimon commented May 11, 2024

fwiw, it'd be fairly simple for a maintainer to obfuscate the code used to build their artifact by using a malicious (non-recorded) input combined with GH's script injection features :)

Other inputs include secrets ${{ secrets.* }}. It's fairly common on GH for users to build and push to registry in the same trust domain (job / VM). Easy to hide malicious inputs for script injections there too. And of course there are the usual GH events that contain user-defined info (in the GH event)

It's also worth distinguishing between generators and builders:

  1. Generators (and the GH action provenance) need to record inputs to the repository workflow (workflow inputs, vars, GH event). The generators in this repo record all of these (need to check for vars @ianlewis do you remember?). Secrets are still a source of inputs but we can't record it since they are secrets. Note that both our generators and GH provenance Action have limitations: they can give assurance about the builder itself, eg it could be using self-hosted runners and could be mis-using caches
  2. Builders need not record all the repo workflow input, secrets or vars. That's because their interface is different: we record the inputs to the reusable workflow itself, so it does not matter how it was triggered by the user: the builder builds the project and the only inputs are those from the reusable workflow. Secrets can be passed but assuming the builder itself is not subject to script injection (or uses the secrets as an input to the build process) then we have a complete record of all inputs. The builder still record the GH event anyway, in case that's useful for troubleshooting

@ianlewis
Copy link
Member

I recall opening a tracking issue for it a while back but I can't find it. Vars variables are another way to pass parameters to a build

#1555

fwiw, it'd be fairly simple for a maintainer to obfuscate the code used to build their artifact by using a malicious (non-recorded) input combined with GH's script injection features :)

"features" 😄 Indeed, it's easy enough for these issues to fly under the radar that GitHub warns about it in the documentation specifically.
https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#example-of-a-script-injection-attack

For the generators, pretty sure we don't record vars yet unfortunately. I believe these are also not in the event payload so we need to add them specifically.

// WorkflowParameters contains parameters given to the workflow invocation.
type WorkflowParameters struct {
// EventInputs is the inputs for the event that triggered the workflow.
EventInputs interface{} `json:"event_inputs,omitempty"`
}

For builders we just have normal inputs and the build itself can't read the vars unless explictly given them so can't affect the build - #1555 (comment)

@ianlewis
Copy link
Member

I would say no. The properties need to be project-agnostic, else there's no way for a verifier to trust builds from a builder across all their dependencies without inspecting the build workflow (defeats the purpose of SLSA levels). In SLSA the build level is associated with a builder and that's how we scale trust.

To be fair though, for generators, we don't do any validation on the inputs so likely likely won't make a difference in our ability to verify.

The only difference is that we can record the inputs in a trustworthy way whereas the GitHub artifact attestations currently cannot. Users of slsa-github-generator generators could implement their own policy checks over the inputs using the --print-provenance flag on slsa-verifier + cue/rego etc.

@ianlewis ianlewis added type:documentation Improvements or additions to documentation and removed status:triage Issue that has not been triaged labels May 13, 2024
@laurentsimon
Copy link
Collaborator

laurentsimon commented May 20, 2024

I would say no. The properties need to be project-agnostic, else there's no way for a verifier to trust builds from a builder across all their dependencies without inspecting the build workflow (defeats the purpose of SLSA levels). In SLSA the build level is associated with a builder and that's how we scale trust.

To be fair though, for generators, we don't do any validation on the inputs so likely likely won't make a difference in our ability to verify.

In practice it's unlikely those inputs would be used during verification, unless they meaningful "high-level" info about a build, like prod vs debug. However they are necessary incident response.

Note that the slsa-verifier supports verifying workflow_dispatch inputs, at least for our v0.2 builders.

@ianlewis
Copy link
Member

ianlewis commented May 21, 2024

In practice it's unlikely those inputs would be used during verification, unless they meaningful "high-level" info about a build, like prod vs debug. However they are necessary incident response.

Note that the slsa-verifier supports verifying workflow_dispatch inputs, at least for our v0.2 builders.

Agreed. I think that it's also likely that users would want to apply their own policy over the inputs. For example, to verify that that the prod/debug flags are set properly for production, or to verify there are no inputs set at all etc.

I did forget that we had the --build-workflow-input flag.
https://github.com/slsa-framework/slsa-verifier/blob/87b5bae6d4230aab069db8488e975e3e9b7c684e/cli/slsa-verifier/verify/options.go#L51-L52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:documentation Improvements or additions to documentation type:feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants