encrypt values to create the github action secret #1990

cpanato · 2022-06-13T15:35:40Z

Summary

Update go-github to v45
fail fast if the GitHub token is not set
encrypt values to create the github action secret

Running this patch in a repo

https://github.com/cpanato/testing-ci-providers/runs/6864530948?check_suite_focus=true

$ cosign verify --key cosign.pub ghcr.io/cpanato/testing-ci-providers/test:latest

Verification for ghcr.io/cpanato/testing-ci-providers/test:latest --
The following checks were performed on each of these signatures:
  - The cosign claims were validated
  - The signatures were verified against the specified public key

[{"critical":{"identity":{"docker-reference":"ghcr.io/cpanato/testing-ci-providers/test"},"image":{"docker-manifest-digest":"sha256:1954e3d3c624d07f1c743f9d987b9a04c0454f325fa24402993e79a1b00f0998"},"type":"cosign container image signature"},"optional":null}]

Ticket Link

Fixes #1431

cpanato · 2022-06-13T15:36:38Z

pkg/cosign/git/github/github.go

+	var peersPubKey [32]byte
+	copy(peersPubKey[:], decodedPubKey[0:32])
+
+	var rand io.Reader


not sure if just this is enough or we should create something else

Usually you want crypto/rand.Reader https://pkg.go.dev/crypto/rand#pkg-variables

codecov-commenter · 2022-06-13T15:38:11Z

Codecov Report

Merging #1990 (ac0e556) into main (655a681) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1990   +/-   ##
=======================================
  Coverage   26.16%   26.16%           
=======================================
  Files         127      127           
  Lines        7422     7422           
=======================================
  Hits         1942     1942           
  Misses       5225     5225           
  Partials      255      255

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 655a681...ac0e556. Read the comment docs.

znewman01

Looking good! Can I ask for you to test with env:

        COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
        COSIGN_PASSWORD: ${{ secrets.COSIGN_PASSWORD }}

in the CI test (instead of dumping to cosign.key)?

znewman01 · 2022-06-13T16:18:40Z

pkg/cosign/git/github/github.go

+	var peersPubKey [32]byte
+	copy(peersPubKey[:], decodedPubKey[0:32])
+
+	var rand io.Reader


Usually you want crypto/rand.Reader https://pkg.go.dev/crypto/rand#pkg-variables

znewman01 · 2022-06-13T16:19:40Z

pkg/cosign/git/github/github.go

+	if err != nil {
+		return nil, fmt.Errorf("failed to decode public key: %w", err)
+	}
+	var peersPubKey [32]byte


What's the advantage of this extra variable? Maybe a comment explaining why we can't just use decodedPubKey.

i need to copy the 32 first and then pass that as a reference to the function, cannot use that straight

cpanato · 2022-06-14T07:50:20Z

Looking good! Can I ask for you to test with env:
        COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
        COSIGN_PASSWORD: ${{ secrets.COSIGN_PASSWORD }}
in the CI test (instead of dumping to cosign.key)?

thanks for the review and feedback

the job is not dumping the key, it actually using it, see https://github.com/cpanato/testing-ci-providers/blob/main/.github/workflows/cosign-key.yml

developer-guy

LGTM, thanks @cpanato for taking the responsibility 👍

znewman01 · 2022-06-15T13:57:54Z

Looking good! Can I ask for you to test with env:
        COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
        COSIGN_PASSWORD: ${{ secrets.COSIGN_PASSWORD }}
in the CI test (instead of dumping to cosign.key)?
thanks for the review and feedback

the job is not dumping the key, it actually using it, see https://github.com/cpanato/testing-ci-providers/blob/main/.github/workflows/cosign-key.yml

Sorry, I was unclear. The motivation for pinging this bug was a workflow involving the $COSIGN_PRIVATE_KEY and $COSIGN_PASSWORD env vars. I was just wondering if we should test those as well, rather than a workflow that wrote $COSIGN_PRIVATE_KEY to disk and passed it through the --key flag. This is probably the workflow we'll recommend because it's 1 less step.

cpanato · 2022-06-15T16:01:25Z

ah, I see now

I think that does not work now, but i can test and if that does not work, propose a fix

UPDATE:
remember that we have the env:// prefix that can be used for that, then you can do the following

      - name: Build and sign a container image
        env:
          COSIGN_PASSWORD: ${{ secrets.COSIGN_PASSWORD }}
          COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
        run: |
          cosign sign --key env://COSIGN_PRIVATE_KEY ghcr.io/cpanato/testing-ci-providers/test:latest

Signed-off-by: cpanato <ctadeu@gmail.com>

cpanato · 2022-06-16T09:17:22Z

Looking good! Can I ask for you to test with env:
        COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
        COSIGN_PASSWORD: ${{ secrets.COSIGN_PASSWORD }}
in the CI test (instead of dumping to cosign.key)?
thanks for the review and feedback
the job is not dumping the key, it actually using it, see https://github.com/cpanato/testing-ci-providers/blob/main/.github/workflows/cosign-key.yml
Sorry, I was unclear. The motivation for pinging this bug was a workflow involving the $COSIGN_PRIVATE_KEY and $COSIGN_PASSWORD env vars. I was just wondering if we should test those as well, rather than a workflow that wrote $COSIGN_PRIVATE_KEY to disk and passed it through the --key flag. This is probably the workflow we'll recommend because it's 1 less step.

@znewman01 to get back to you
today we cannot use the key as a environment variable, it expects a file, but i can propose a PR to add this change and get feedback

cpanato · 2022-06-16T09:17:35Z

gently ping to review @developer-guy @dlorenc

znewman01 · 2022-06-16T13:28:29Z

Looking good! Can I ask for you to test with env:
        COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
        COSIGN_PASSWORD: ${{ secrets.COSIGN_PASSWORD }}
in the CI test (instead of dumping to cosign.key)?
thanks for the review and feedback
the job is not dumping the key, it actually using it, see https://github.com/cpanato/testing-ci-providers/blob/main/.github/workflows/cosign-key.yml
Sorry, I was unclear. The motivation for pinging this bug was a workflow involving the $COSIGN_PRIVATE_KEY and $COSIGN_PASSWORD env vars. I was just wondering if we should test those as well, rather than a workflow that wrote $COSIGN_PRIVATE_KEY to disk and passed it through the --key flag. This is probably the workflow we'll recommend because it's 1 less step.
@znewman01 to get back to you today we cannot use the key as a environment variable, it expects a file, but i can propose a PR to add this change and get feedback

Whoops, I guess that was never expected to work! Sorry for the confusion.

And yes, such a PR would be great.

znewman01 · 2022-06-16T13:30:57Z

Oh, actually @cpanato : the --key flag takes an env://ENV_VAR syntax. So this should work with env://COSIGN_PRIVATE_KEY. No need for a PR

cpanato · 2022-06-16T13:32:15Z

Oh, actually @cpanato : the --key flag takes an env://ENV_VAR syntax. So this should work with env://COSIGN_PRIVATE_KEY. No need for a PR

yep, i've made a comment in the PR for this exactly same thing :)

dlorenc · 2022-06-17T02:14:24Z

Is this one ready to go?

cpanato requested a review from dlorenc June 13, 2022 15:35

cpanato commented Jun 13, 2022

View reviewed changes

znewman01 reviewed Jun 13, 2022

View reviewed changes

cpanato requested a review from developer-guy June 14, 2022 08:11

developer-guy previously approved these changes Jun 14, 2022

View reviewed changes

znewman01 previously approved these changes Jun 15, 2022

View reviewed changes

cpanato added 4 commits June 15, 2022 18:02

Update go-github to v45

187eaeb

Signed-off-by: cpanato <ctadeu@gmail.com>

fail fast if github token is not set

e4fce9f

Signed-off-by: cpanato <ctadeu@gmail.com>

encrypt values to create the github action secret

beb3b3d

Signed-off-by: cpanato <ctadeu@gmail.com>

update codegen

ac0e556

Signed-off-by: cpanato <ctadeu@gmail.com>

cpanato dismissed stale reviews from znewman01 and developer-guy via ac0e556 June 15, 2022 16:03

cpanato force-pushed the GH-1431 branch from 9a6b8ac to ac0e556 Compare June 15, 2022 16:03

cpanato requested a review from developer-guy June 16, 2022 07:55

dlorenc approved these changes Jun 16, 2022

View reviewed changes

developer-guy approved these changes Jun 17, 2022

View reviewed changes

cpanato merged commit 5e261dc into sigstore:main Jun 17, 2022

cpanato deleted the GH-1431 branch June 17, 2022 08:11

github-actions bot added this to the v1.10.0 milestone Jun 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

encrypt values to create the github action secret #1990

encrypt values to create the github action secret #1990

cpanato commented Jun 13, 2022

cpanato Jun 13, 2022 •

edited

Loading

znewman01 Jun 13, 2022

codecov-commenter commented Jun 13, 2022 •

edited

Loading

znewman01 left a comment

znewman01 Jun 13, 2022

znewman01 Jun 13, 2022

cpanato Jun 14, 2022

cpanato commented Jun 14, 2022

developer-guy left a comment

znewman01 commented Jun 15, 2022

cpanato commented Jun 15, 2022 •

edited

Loading

cpanato commented Jun 16, 2022

cpanato commented Jun 16, 2022

znewman01 commented Jun 16, 2022

znewman01 commented Jun 16, 2022

cpanato commented Jun 16, 2022

dlorenc commented Jun 17, 2022

encrypt values to create the github action secret #1990

encrypt values to create the github action secret #1990

Conversation

cpanato commented Jun 13, 2022

Summary

Ticket Link

cpanato Jun 13, 2022 • edited Loading

Choose a reason for hiding this comment

znewman01 Jun 13, 2022

Choose a reason for hiding this comment

codecov-commenter commented Jun 13, 2022 • edited Loading

Codecov Report

znewman01 left a comment

Choose a reason for hiding this comment

znewman01 Jun 13, 2022

Choose a reason for hiding this comment

znewman01 Jun 13, 2022

Choose a reason for hiding this comment

cpanato Jun 14, 2022

Choose a reason for hiding this comment

cpanato commented Jun 14, 2022

developer-guy left a comment

Choose a reason for hiding this comment

znewman01 commented Jun 15, 2022

cpanato commented Jun 15, 2022 • edited Loading

cpanato commented Jun 16, 2022

cpanato commented Jun 16, 2022

znewman01 commented Jun 16, 2022

znewman01 commented Jun 16, 2022

cpanato commented Jun 16, 2022

dlorenc commented Jun 17, 2022

cpanato Jun 13, 2022 •

edited

Loading

codecov-commenter commented Jun 13, 2022 •

edited

Loading

cpanato commented Jun 15, 2022 •

edited

Loading