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

Implement a sign command #91

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

pieterlexis
Copy link
Contributor

@pieterlexis pieterlexis commented Nov 15, 2021

This PR adds support for signing existing provenance files.

You can see the action in action (heh!) here.

This PR is somewhat work in progress to solicit feedback from the maintainers.

TODO:

  • Canonicalize the JSON to be signed
  • Test against in-toto tools
  • Docs
  • End to end tests

@pieterlexis
Copy link
Contributor Author

@Brend-Smits, @marcofranssen I would like to have some feedback on this. Mostly:

  1. Is this something you want in the action?
  2. What is needed to get this merged?
  3. Is this the correct way of doing signing?

@marcofranssen
Copy link
Member

@pieterlexis Thanks a lot for starting these efforts. I wrote down my thoughts in a new issue #92. Feel free to add your ideas there as well. We might be able to leverage some ideas from cosign/notation projects.

@pieterlexis pieterlexis mentioned this pull request Nov 19, 2021
3 tasks
@Brend-Smits
Copy link
Member

Hey @pieterlexis,

How are things going? Can we help you with anything in this pull request?
Please let us know.

@pieterlexis
Copy link
Contributor Author

Hey @pieterlexis,

How are things going? Can we help you with anything in this pull request? Please let us know.

Feedback on how this works, if you like it and perhaps on how I can test this against external tools would be good!

@Brend-Smits
Copy link
Member

Hi @pieterlexis,

First of all, sorry for the delayed reply. We have been working on restructuring the command line interface (see #98).
This pull request was merged this morning. It should make it significantly easier to add new functionality like the sign feature.

With this restructure it is now possible to add sign options for the generate command more easily (see https://github.com/philips-labs/slsa-provenance-action/pull/98/files#diff-801e4ca077c02d198fc44de6c15df5a298e9a262e4ea4b387619867264d3778fR16). The logic of your sign command can then also be reused in the generate command to offer both options.

⚠️ Please note: the CLI changes are not backward compatible (breaking changes). The action itself is backward compatible. ⚠️

We are looking into further refactors of the action itself. We are inspired by GoReleaser and are looking into separating the binary from the action so people can choose to use 'install only mode' or 'direct use' of a specific version of the CLI (see https://github.com/goreleaser/goreleaser-action/blob/master/action.yml).
A first approach to making this action more reusable is found here. The next step will be breaking for the action, and will add the functionality needed to replace the sign action you created.

What do you think about these ideas? Would love to hear your thoughts!

Regarding the other questions, it might be easier to set up a quick call. Are you open to this?

cc @marcofranssen

@pieterlexis
Copy link
Contributor Author

First of all, sorry for the delayed reply. We have been working on restructuring the command line interface (see #98).

Everyone's busy :)

With this restructure it is now possible to add sign options for the generate command more easily (see https://github.com/philips-labs/slsa-provenance-action/pull/98/files#diff-801e4ca077c02d198fc44de6c15df5a298e9a262e4ea4b387619867264d3778fR16). The logic of your sign command can then also be reused in the generate command to offer both options.

I'll rebase this then :).

We are looking into further refactors of the action itself. > We are inspired by GoReleaser and are looking into separating the binary from the action so people can choose to use 'install only mode' or 'direct use' of a specific version of the CLI (see https://github.com/goreleaser/goreleaser-action/blob/master/action.yml).
A first approach to making this action more reusable is found here. The next step will be breaking for the action, and will add the functionality needed to replace the sign action you created.

What do you think about these ideas? Would love to hear your thoughts!

I'll have to have a good look at it.

Regarding the other questions, it might be easier to set up a quick call. Are you open to this?

I am, I already mailed with Marco, but he ghosted me ;)

@marcofranssen
Copy link
Member

@pieterlexis Definitely not my intention to ghost you 😂

I sent you an email as well on the powerdns domain (email used in your commit). I think something went wrong with the email as I don't seem to have any email in my inbox. Did you receive mine?

@pieterlexis pieterlexis force-pushed the sign-command branch 2 times, most recently from 49d227f to ca9ad38 Compare December 9, 2021 13:33
@pieterlexis
Copy link
Contributor Author

Rebased!

I sent you an email as well on the powerdns domain (email used in your commit). I think something went wrong with the email as I don't seem to have any email in my inbox. Did you receive mine?

Yes, I responded within half an hour or so :). I should ask our IT about this. I'll send you an email.

@pieterlexis pieterlexis force-pushed the sign-command branch 2 times, most recently from 57b81cc to 16a38dd Compare December 9, 2021 14:27
@ChaosInTheCRD
Copy link

Hi Everyone! Awesome to see that this integration is coming soon. Would the capability to sign the image within the action provide it with enough "integrity" to regard it as able to meet the following requirement?

Authenticated: The provenance’s authenticity and integrity can be verified by the consumer. This SHOULD be through a digital signature from a private key accessible only to the service generating the provenance.

If so, what else does this action need before it can be regarded as SLSA Level 2 compliant?

@pieterlexis pieterlexis marked this pull request as ready for review December 13, 2021 16:23
@pieterlexis pieterlexis requested a review from a team as a code owner December 13, 2021 16:23
@pieterlexis
Copy link
Contributor Author

Hi Everyone! Awesome to see that this integration is coming soon. Would the capability to sign the image within the action provide it with enough "integrity" to regard it as able to meet the following requirement?

Authenticated: The provenance’s authenticity and integrity can be verified by the consumer. This SHOULD be through a digital signature from a private key accessible only to the service generating the provenance.

If so, what else does this action need before it can be regarded as SLSA Level 2 compliant?

I think so, with the caveat that the key should not be available to other parts of the builds. But someone who fully understands SLSA (like @MarkLodato) should chime in.

Signed-off-by: Pieter Lexis <[email protected]>
Signed-off-by: Pieter Lexis <[email protected]>
According to the SLSA specification, these are [JSON
objects](https://slsa.dev/provenance/v0.1). This commit changes their
type from raw json to the more correct `map[string]interface{}`.

Signed-off-by: Pieter Lexis <[email protected]>
This checks if we can actually verify the signature and if the data in
the payload actually matches what we put in.

Signed-off-by: Pieter Lexis <[email protected]>
Signed-off-by: Pieter Lexis <[email protected]>
@marcofranssen
Copy link
Member

We choose to stick with a single action.

Although we didn't test it we are not entirely sure how GitHub will behave if you publish multiple actions from the same repo to the marketplace.

The v0.5.0 release changed the api slightly to add more flexibility.

For the signing PR we still need to see how we improve this structure to keep it as simple as possible for consumers of the action while keeping the flexibility of different scenarios people would like to use.

@marcofranssen
Copy link
Member

We have introduced signed releases and a bunch of other small improvements. Currently this branch is in conflict with the main branch. We also still need to check if this PR has same results as what we currently do in the workflows using cosign.

https://github.com/philips-labs/slsa-provenance-action/blob/main/.github/workflows/ci.yaml#L180

In this line we are attaching only the predicate part to a Docker image. whereas for binaries we take the full provenance as it was generated. Signing it will cause an additional envelope which still needs some research on how to deal with this accordingly to support both provenance for containers and blob assets.

Sign up for free to 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.

4 participants