Have the starter `docker-publish` action sign digests. #1255

mattmoor · 2021-11-17T23:12:35Z

This change installs sigstore/cosign using the cosign-installer action,
and uses sigstore's "keyless" signing process to sign the resulting image
digest using the action's identity token (see: id-token: write).

Signed-off-by: Matt Moore mattomata@gmail.com

Pre-requisites

Prior to submitting a new workflow, please apply to join the GitHub Technology Partner Program: partner.github.com/apply.

Tasks

For all workflows, the workflow:

Should be contained in a .yml file with the language or platform as its filename, in lower, kebab-cased format (for example, docker-image.yml). Special characters should be removed or replaced with words as appropriate (for example, "dotnet" instead of ".NET").
Should use sentence case for the names of workflows and steps (for example, "Run tests").
Should be named only by the name of the language or platform (for example, "Go", not "Go CI" or "Go Build").
Should include comments in the workflow for any parts that are not obvious or could use clarification.

For CI workflows, the workflow:

Should be preserved under the ci directory.
Should include a matching ci/properties/*.properties.json file (for example, ci/properties/docker-publish.properties.json).
Should run on push to branches: [ $default-branch ] and pull_request to branches: [ $default-branch ].
Packaging workflows should run on release with types: [ created ].
Publishing workflows should have a filename that is the name of the language or platform, in lower case, followed by "-publish" (for example, docker-publish.yml).

For Code Scanning workflows, the workflow:

Should be preserved under the code-scanning directory.
Should include a matching code-scanning/properties/*.properties.json file (for example, code-scanning/properties/codeql.properties.json), with properties set as follows:
- name: Name of the Code Scanning integration.
- organization: Name of the organization producing the Code Scanning integration.
- description: Short description of the Code Scanning integration.
- categories: Array of languages supported by the Code Scanning integration.
- iconName: Name of the SVG logo representing the Code Scanning integration. This SVG logo must be present in the icons directory.
Should run on push to branches: [ $default-branch, $protected-branches ] and pull_request to branches: [ $default-branch ]. We also recommend a schedule trigger of cron: $cron-weekly (for example, codeql.yml).

Some general notes:

This workflow must only use actions that are produced by GitHub, in the actions organization, or
This workflow must only use actions that are produced by the language or ecosystem that the workflow supports. These actions must be published to the GitHub Marketplace. We require that these actions be referenced using the full 40 character hash of the action's commit instead of a tag. Additionally, workflows must include the following comment at the top of the workflow file:
```
# This workflow uses actions that are not certified by GitHub.
# They are provided by a third-party and are governed by
# separate terms of service, privacy policy, and support
# documentation.
```
Automation and CI workflows should not send data to any 3rd party service except for the purposes of installing dependencies.
Automation and CI workflows cannot be dependent on a paid service or product.

dekkagaijin · 2021-11-18T00:48:25Z

ci/docker-publish.yml

+      # https://github.com/sigstore/cosign-installer
+      - name: Install cosign
+        if: github.event_name != 'pull_request'
+        uses: sigstore/cosign-installer@1e95c1de343b5b0c23352d6417ee3e48d5bcd422


Suggested change

uses: sigstore/cosign-installer@1e95c1de343b5b0c23352d6417ee3e48d5bcd422

uses: sigstore/cosign-installer@main

To be consistent with our guidance? Unless the style for these examples requires pinning to a git commit

I was trying to be consistent with the other steps, which do this.

We require that starter-workflows pin to a specific commit when including actions not maintained by GitHub. In addition pinning to a ref like main is not recommended as updates could break customers.

@chrispat since we are a security-focused action, we recommend using @mainfor the action to adopt fixes ASAP
and to optionally pin a specific release version of the tool itself if that degree of stability is desired. I appreciate the desire for stability in these starter workflows, though

As folks involved in supply chain security, we also understand a desire to be very intentional about picking up changes to dependencies 😉

Maybe encoding the @main recommendation in a comment above that step is a reasonable compromise?

You can see what Ruby does: https://github.com/actions/starter-workflows/blob/main/ci/ruby.yml#L29

(I'd encourage you to encourage pinning to a released tag, not @main, that's more Actions-idiomatic.)

jhutchings1 · 2021-11-20T00:10:02Z

@andymckay @chrispat This workflow sends data to a 3rd party service for the purposes of providing a public log of the signing activity. I think this is a good reason to do it, but I'd love your insight on how best to set expectations on this for customers.

jhutchings1 · 2021-11-30T22:35:52Z

ci/docker-publish.yml

+      # Sign the resulting Docker image digest except on PR
+      # https://github.com/sigstore/cosign
+      - name: Sign the published Docker image
+        if: github.event_name != 'pull_request'


Was chatting with @steiza, and he brought up an interesting point here. For private repositories, the transparency log will effectively leak the identity of a private repository. We had the thought that we should add an if to only sign for public repositories. Users could obviously opt-in to signing for private repositories, but this would not leak their private repo information by default.

@mattmoor what do you think?

That works for me if there's a way to express that in actions (I think the best UX here would be for it to show up as skipped like it would for PRs).

I suspect the next best would be for us to add an option to cosign that lets us toggle this off (perhaps with a clear warning) when the image is private (I can discuss with @dlorenc @dekkagaijin et al and try to land something in 1.4).

WDYT?

I staged a fix in cosign for this here: sigstore/cosign#1116, which would let us drop the --force (although I'll probably add a comment indicating that folks comfortable with this can add --force to publish to the transparency log).

Yep, it's very doable. You could add the step below, and then add the (!(env.IS_PRIVATE)) to your existing if statement and I think it would do the job.

- name: Confirm this is a public repository env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} run: | TEMP=$(gh api repos/$GITHUB_REPOSITORY --jq '.private') echo "IS_PRIVATE=$TEMP" >> $GITHUB_ENV - name: Run if public if: (!(env.IS_PRIVATE))

Awesome TIL! I'll try that out and then update this. I realize that this'll end up skipping signing entirely, so we may still want to adopt the cosign change and drop this once we cut 1.4.

ok, I was able to get this working in my little sample repo:

- name: Confirm this is a public repository env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} run: | TEMP=$(gh api repos/$GITHUB_REPOSITORY --jq '.private') echo "IS_PRIVATE=$TEMP" >> $GITHUB_ENV - name: Sign the container image (if public) if: ${{ env.IS_PRIVATE == 'false' }} env: COSIGN_EXPERIMENTAL: "true" run: cosign sign ghcr.io/${{ github.repository }}@${{ steps.push-step.outputs.digest }}

This ran fine (with signing) in the public repo, and from a private repo instantiated from it the signing step was skipped. I'll update the PR with this shortly, thanks for the tip!

Ok, cool this should be updated. When we cut cosign 1.4 we can simplify things, since my PR upstream landed.

Given you have the whole event payload to work with already on disk you don't need to make the api call. Each event has a repository object and that will tell you if it is public or private. Should be something like ${{ github.event.repository.private == 'false' }}

Looks like what @steiza also said. This should be fixed in the latest revision!

steiza · 2021-12-01T14:50:56Z

ci/docker-publish.yml

+      # Sign the resulting Docker image digest except on PRs and private repos
+      # https://github.com/sigstore/cosign
+      - name: Sign the published Docker image
+        if: ${{ github.event_name != 'pull_request' && env.IS_PRIVATE == 'false' }}


I came here to suggest that we only run this on public repos, but you beat me to it!

I think we could simplify the implementation a bit though, by not having the above step and just checking if: ${{ !github.event.repository.private }} here.

It shouldn't be the default, but there might be use-cases where a private repo wants to use keyless signing, and is okay with the public record, so we could have an opt-in env variable (COSIGN_USE_PUBLIC_KEYLESS_SIGNING?) that uses keyless signing regardless. Not required, just a suggestion.

Thanks for contributing this functionality!

@steiza even better, I'll update the PR. The next version of cosign will make it so we can always sign and for private repos they would add --force if they want to publish to the transparency log, which feels like exactly the opt-in we want.

Perfect, this is much tighter, and I just tried it on my public and private demo repos and it works like a charm. ✨

Thanks again @steiza !

jhutchings1 · 2021-12-02T06:12:04Z

Quick update for everyone: I'm working on a communications plan so that we notify users when this change goes in. I'd like to target Monday 12/6 to get everything out, but we might need to move things around depending on internal reviews. Really appreciate all the collaboration here!

mattmoor · 2021-12-02T15:09:31Z

Cool, I'll try to keep things rebased 👍

This change installs `sigstore/cosign` using the `cosign-installer` action, and uses sigstore's "keyless" signing process to sign the resulting image digest using the action's identity token (see: `id-token: write`). Signed-off-by: Matt Moore <mattomata@gmail.com>

jhutchings1

Changes look good to me. Thanks for the collaboration all!

mehran-sayyah · 2021-12-06T17:21:31Z

#1255 دریافت Outlook برای Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Bishal Prasad ***@***.***> Sent: Monday, December 6, 2021 8:36:06 PM To: actions/starter-workflows ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [actions/starter-workflows] Have the starter `docker-publish` action sign digests. (PR #1255) Merged #1255<#1255> into main. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#1255 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ATTU54E4E5QVCECS6NPN4IDUPTUP5ANCNFSM5IIGUMIQ>.

bishal-pdMSFT · 2021-12-06T18:09:42Z

@mattmoor the updated workflow failed for my docker repo.
If failed in signing step:

Error: signing [ghcr.io/bishal-pdMSFT/docker-app@sha256:ec9994d1d8cf472600bbc9fa73d2e6afe865beef000185659f6d0e8f774b47f8]: parsing reference: could not parse reference: ghcr.io/bishal-pdMSFT/docker-app@sha256:ec9994d1d8cf472600bbc9fa73d2e6afe865beef000185659f6d0e8f774b47f8
main.go:46: error during command execution: signing [ghcr.io/bishal-pdMSFT/docker-app@sha256:ec9994d1d8cf472600bbc9fa73d2e6afe865beef000185659f6d0e8f774b47f8]: parsing reference: could not parse reference: ghcr.io/bishal-pdMSFT/docker-app@sha256:ec9994d1d8cf472600bbc9fa73d2e6afe865beef000185659f6d0e8f774b47f8

Can you please check it at priority and let me know if this can be widespread?

mattmoor · 2021-12-06T18:14:20Z

I will check it out now, I was in a meeting when things merged, but I wonder if it is the capitals in your username, which I don't believe are allowed in Docker repository names (not sure how ghcr.io maps this?)

mattmoor · 2021-12-06T18:17:10Z

Looks like that's the case:

# crane ls ghcr.io/bishal-pdMSFT/docker-app
2021/12/06 10:15:51 reading tags for ghcr.io/bishal-pdMSFT/docker-app: parsing repo "ghcr.io/bishal-pdMSFT/docker-app": repository can only contain the runes `abcdefghijklmnopqrstuvwxyz0123456789_-./`: bishal-pdMSFT/docker-app

Depending on how this is handled by ghcr.io we should be able to get this fixed pretty quickly 🤞

mattmoor · 2021-12-06T18:22:55Z

Looks like buildkit is implicitly making the username lowercase: https://github.com/bishal-pdMSFT/docker-app/runs/4434089694?check_suite_focus=true#step:7:187

I assume this is roughly what we want here. Are github usernames guaranteed to be unique regardless of case?

mattmoor · 2021-12-06T18:43:35Z

I confirmed that this works for me here: https://github.com/mattmoor/zero-friction-actions/runs/4434363881?check_suite_focus=true

Let me open an issue here to track the uppercase issue, and discuss options for fixes.

bishal-pdMSFT · 2021-12-06T18:44:07Z

Looks like account name is case insensitive. I tried changing my account name to another name with different case and it was not allowed.

bishal-pdMSFT · 2021-12-06T18:45:12Z

Confirmed internally as well. It is case insensitive.

bishal-pdMSFT · 2021-12-06T18:47:34Z

And for the first time I realize the benefit of choosing a CAPS letter in my account name 😁

mattmoor · 2021-12-06T20:30:34Z

Looks like this is docker/metadata-action lowercasing things, so I posted #1294 which should fix this. Sorry for the breakage @bishal-pdMSFT, and thanks for the pointer @imjasonh

mattmoor requested a review from a team as a code owner November 17, 2021 23:12

dekkagaijin reviewed Nov 18, 2021

View reviewed changes

mattmoor force-pushed the cosign-docker-publish branch from 4225ddf to cc91ee0 Compare November 29, 2021 22:11

jhutchings1 reviewed Nov 30, 2021

View reviewed changes

mattmoor mentioned this pull request Dec 1, 2021

Make the transparency log upload non-fatal. sigstore/cosign#1116

Merged

steiza reviewed Dec 1, 2021

View reviewed changes

mattmoor force-pushed the cosign-docker-publish branch 2 times, most recently from 7c81c83 to 326dc04 Compare December 2, 2021 00:27

mattmoor force-pushed the cosign-docker-publish branch from 326dc04 to b08ab3a Compare December 2, 2021 15:10

Tanapl02 mentioned this pull request Dec 2, 2021

Start longpeng2008/yousan.ai#15

Open

mattmoor and others added 4 commits December 5, 2021 12:13

Fully qualify the digest, add setup-buildx-action as workaround

4d54e29

Drop --force, add public repo check

04a1ab1

Use built-in 'private' bit

4e1ad5a

mattmoor force-pushed the cosign-docker-publish branch from b08ab3a to 4e1ad5a Compare December 5, 2021 20:13

jhutchings1 approved these changes Dec 6, 2021

View reviewed changes

bishal-pdMSFT approved these changes Dec 6, 2021

View reviewed changes

bishal-pdMSFT merged commit 60d206d into actions:main Dec 6, 2021

mattmoor deleted the cosign-docker-publish branch December 6, 2021 18:13

mattmoor mentioned this pull request Dec 6, 2021

docker-publish.yml doesn't work with uppercase usernames/repositories #1293

Closed

haydentherapper mentioned this pull request Dec 7, 2021

Update wording for experimental flag for github starter workflow sigstore/cosign#1156

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Have the starter `docker-publish` action sign digests. #1255

Have the starter `docker-publish` action sign digests. #1255

mattmoor commented Nov 17, 2021 •

edited

Loading

dekkagaijin Nov 18, 2021

mattmoor Nov 18, 2021

chrispat Nov 22, 2021

dekkagaijin Nov 22, 2021 •

edited

Loading

mattmoor Nov 22, 2021

mattmoor Nov 22, 2021

ethomson Nov 29, 2021

jhutchings1 commented Nov 20, 2021

jhutchings1 Nov 30, 2021

mattmoor Nov 30, 2021

mattmoor Dec 1, 2021

jhutchings1 Dec 1, 2021

mattmoor Dec 1, 2021

mattmoor Dec 1, 2021

mattmoor Dec 1, 2021

chrispat Dec 1, 2021 •

edited

Loading

mattmoor Dec 2, 2021

steiza Dec 1, 2021

mattmoor Dec 1, 2021

mattmoor Dec 1, 2021

jhutchings1 commented Dec 2, 2021

mattmoor commented Dec 2, 2021

jhutchings1 left a comment

mehran-sayyah commented Dec 6, 2021 via email

bishal-pdMSFT commented Dec 6, 2021

mattmoor commented Dec 6, 2021

mattmoor commented Dec 6, 2021

mattmoor commented Dec 6, 2021

mattmoor commented Dec 6, 2021

bishal-pdMSFT commented Dec 6, 2021

bishal-pdMSFT commented Dec 6, 2021

bishal-pdMSFT commented Dec 6, 2021

mattmoor commented Dec 6, 2021

	uses: sigstore/cosign-installer@1e95c1de343b5b0c23352d6417ee3e48d5bcd422
	uses: sigstore/cosign-installer@main

Have the starter docker-publish action sign digests. #1255

Have the starter docker-publish action sign digests. #1255

Conversation

mattmoor commented Nov 17, 2021 • edited Loading

Pre-requisites

Tasks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dekkagaijin Nov 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhutchings1 commented Nov 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrispat Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhutchings1 commented Dec 2, 2021

mattmoor commented Dec 2, 2021

jhutchings1 left a comment

Choose a reason for hiding this comment

mehran-sayyah commented Dec 6, 2021 via email

bishal-pdMSFT commented Dec 6, 2021

mattmoor commented Dec 6, 2021

mattmoor commented Dec 6, 2021

mattmoor commented Dec 6, 2021

mattmoor commented Dec 6, 2021

bishal-pdMSFT commented Dec 6, 2021

bishal-pdMSFT commented Dec 6, 2021

bishal-pdMSFT commented Dec 6, 2021

mattmoor commented Dec 6, 2021

Have the starter `docker-publish` action sign digests. #1255

Have the starter `docker-publish` action sign digests. #1255

mattmoor commented Nov 17, 2021 •

edited

Loading

dekkagaijin Nov 22, 2021 •

edited

Loading

chrispat Dec 1, 2021 •

edited

Loading