fix: add COSIGN_EXPERIMENTAL=1 for verify-bloba #2254

developer-guy · 2022-09-15T15:47:04Z

Signed-off-by: Batuhan Apaydın batuhan.apaydin@trendyol.com

Summary

Release Note

Documentation

haydentherapper · 2022-09-15T15:48:45Z

/hold

Most of these cases do no require the flag.

codecov-commenter · 2022-09-15T15:49:55Z

Codecov Report

Merging #2254 (4e729c3) into main (f43839b) will increase coverage by 0.16%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2254      +/-   ##
==========================================
+ Coverage   28.40%   28.57%   +0.16%     
==========================================
  Files         131      131              
  Lines        7832     7852      +20     
==========================================
+ Hits         2225     2244      +19     
+ Misses       5309     5302       -7     
- Partials      298      306       +8

Impacted Files	Coverage Δ
cmd/cosign/cli/verify.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/verify/verify_blob.go	`45.26% <0.00%> (+0.35%)`	⬆️
cmd/cosign/cli/verify/verify.go	`5.97% <0.00%> (+2.61%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

haydentherapper · 2022-09-15T15:58:42Z

cmd/cosign/cli/verify.go


  # Verify a simple blob and message
-  cosign verify-blob --key cosign.pub --signature sig msg
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key cosign.pub --signature sig msg


no flag required for all examples with a key

+1, including the KMS ones. Anything without --certificate I think

haydentherapper · 2022-09-15T15:59:28Z

cmd/cosign/cli/verify.go


  # Verify a signature against a certificate
-  cosign verify-blob --cert <cert> --signature $sig <blob>
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --cert <cert> --signature $sig <blob>


Can we update to --certificate?

haydentherapper · 2022-09-15T16:01:12Z

cmd/cosign/cli/verify.go


  # Verify a signature against a certificate
-  cosign verify-blob --cert <cert> --signature $sig <blob>
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --cert <cert> --signature $sig <blob>
 `,


@asraa Should we use the list you made of all possible flag combos (cert/key, experimental/no experimental, bundle, expired/unexpired cert) and have an example for each of these with more explanation?

+1

Further, I'd like to see some organization and explanation. Right now, the text is just a laundry list of "if X, then Y"

We should explain unset COSIGN_EXPERIMENTAL and COSIGN_EXPERIMENTAL=1 separately (I don't 100% understand the current behavior so please correct me, indicated uncertainty with "?")

Here's a braindump, maybe should move into Google Doc (feel free to copy it)

no experimental

Check the signature on a blob against the given key or certificate.

Keys

blah

Certificates

--certificate and --certificate-chain example:

The provided certificate (and all of the certificates in the chain(?) must be unexpired right now.

NOTE: this just accepts whatever the last certificate of the chain is as your root. If you want to check that it matches a fixed root certificate, use keyless mode and SIGSTORE_ROOT_FILE (except we don't want people to do that? what do we tell these people?)

--certificate no chain

Checks that the certificate is signed by Fulcio (must be unexpired). Will that ever work?

Bundles

--bundle

If --bundle is provided, use the given Rekor bundle (see ...) to check that the time of the signature was while the certificate (and its ancestors??) were valid (without --bundle, the certificate must be currently valid).

Works for keys and certs both.

What happens if the bundle has a conflicting chain?

--bundle + certs, no chain

If there's a bundle use that to get the chain up to Fulcio?

experimental

Keys

Same as above, but we check that signatures are in Rekor (I think?)

If --bundle provided, check that against Rekor (offline).

Certs

--certificate and --certificate-chain: Like the keyfull version, except instead of automatically trusting the certificate chain, we check that the root matches Fulcio's cert.

If you'd like to BYO-root, set SIGSTORE_ROOT_FILE and we'll compare the root of the chain against that.

We hit Rekor online.

--certificate, no chain: this would only work if signed by the root directly, which probably won't be the case (maybe if you BYO-root?)

Bundles

--keys Same as above, don't hit Rekor.

--certificate and --certificate-chain and --bundle. Same as above, don't hit rekor

TODO: what if bundle has a conflicting cert/chain?

--certificate and --bundle: Same, but use a chain from bundle. Fails if bundle has no chain.

TODO: what happens if the bundle has a conflicting cert?

--bundle: Same, but use cert/chain from bundle. Fails if bundle has no cert/chain.

TODO: if the bundle just provides a key, do we trust it?

znewman01 · 2022-09-15T17:15:53Z

cmd/cosign/cli/verify.go


  # Verify a simple blob with remote signature URL, both http and https schemes are supported
-  cosign verify-blob --key cosign.pub --signature http://host/my.sig
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key cosign.pub --signature http://host/my.sig

  # Verify a signature from an environment variable


let's axe this example? it's just explaining how the environment variables work in a shell, I think

znewman01 · 2022-09-15T17:16:16Z

cmd/cosign/cli/verify.go


  # Verify a signature from an environment variable
-  cosign verify-blob --key cosign.pub --signature $sig msg
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key cosign.pub --signature $sig msg

  # verify a signature with public key provided by URL


Can we combine this with the "signature from URL" example?

znewman01 · 2022-09-15T17:17:05Z

cmd/cosign/cli/verify.go


  # verify a signature with public key provided by URL
-  cosign verify-blob --key https://host.for/<FILE> --signature $sig msg
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key https://host.for/<FILE> --signature $sig msg

  # Verify a signature against a payload from another process using process redirection


IMO, axe this example. Maybe we can move it to the docs/ directory of the Cosign repo. Or even a "useful tricks" section at the bottom of this doc.

znewman01 · 2022-09-15T17:17:32Z

cmd/cosign/cli/verify.go


  # Verify a signature against a payload from another process using process redirection
-  cosign verify-blob --key cosign.pub --signature $sig <(git rev-parse HEAD)
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key cosign.pub --signature $sig <(git rev-parse HEAD)

  # Verify a signature against Azure Key Vault


I would like to combine all the KMS examples. It'd be nice to list the syntax for each, but I don't need it in the full context.

znewman01 · 2022-09-15T17:18:37Z

cmd/cosign/cli/verify.go


  # Verify a simple blob and message
-  cosign verify-blob --key cosign.pub --signature sig msg
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key cosign.pub --signature sig msg


+1, including the KMS ones. Anything without --certificate I think

znewman01 · 2022-09-15T17:19:14Z

cmd/cosign/cli/verify.go


  # verify a signature with public key provided by URL
-  cosign verify-blob --key https://host.for/<FILE> --signature $sig msg
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --key https://host.for/<FILE> --signature $sig msg


s/$sig/<sig>

znewman01 · 2022-09-15T17:42:24Z

cmd/cosign/cli/verify.go


  # Verify a signature against a certificate
-  cosign verify-blob --cert <cert> --signature $sig <blob>
+  COSIGN_EXPERIMENTAL=1 cosign verify-blob --cert <cert> --signature $sig <blob>
 `,


+1

Further, I'd like to see some organization and explanation. Right now, the text is just a laundry list of "if X, then Y"

We should explain unset COSIGN_EXPERIMENTAL and COSIGN_EXPERIMENTAL=1 separately (I don't 100% understand the current behavior so please correct me, indicated uncertainty with "?")

Here's a braindump, maybe should move into Google Doc (feel free to copy it)

no experimental

Check the signature on a blob against the given key or certificate.

Keys

blah

Certificates

--certificate and --certificate-chain example:

The provided certificate (and all of the certificates in the chain(?) must be unexpired right now.

NOTE: this just accepts whatever the last certificate of the chain is as your root. If you want to check that it matches a fixed root certificate, use keyless mode and SIGSTORE_ROOT_FILE (except we don't want people to do that? what do we tell these people?)

--certificate no chain

Checks that the certificate is signed by Fulcio (must be unexpired). Will that ever work?

Bundles

--bundle

If --bundle is provided, use the given Rekor bundle (see ...) to check that the time of the signature was while the certificate (and its ancestors??) were valid (without --bundle, the certificate must be currently valid).

Works for keys and certs both.

What happens if the bundle has a conflicting chain?

--bundle + certs, no chain

If there's a bundle use that to get the chain up to Fulcio?

experimental

Keys

Same as above, but we check that signatures are in Rekor (I think?)

If --bundle provided, check that against Rekor (offline).

Certs

--certificate and --certificate-chain: Like the keyfull version, except instead of automatically trusting the certificate chain, we check that the root matches Fulcio's cert.

If you'd like to BYO-root, set SIGSTORE_ROOT_FILE and we'll compare the root of the chain against that.

We hit Rekor online.

--certificate, no chain: this would only work if signed by the root directly, which probably won't be the case (maybe if you BYO-root?)

Bundles

--keys Same as above, don't hit Rekor.

--certificate and --certificate-chain and --bundle. Same as above, don't hit rekor

TODO: what if bundle has a conflicting cert/chain?

--certificate and --bundle: Same, but use a chain from bundle. Fails if bundle has no chain.

TODO: what happens if the bundle has a conflicting cert?

--bundle: Same, but use cert/chain from bundle. Fails if bundle has no cert/chain.

TODO: if the bundle just provides a key, do we trust it?

developer-guy · 2022-09-16T15:13:55Z

I tried to address some of the reviews. Can you please take a look at @znewman01? Thx

znewman01

I have a couple of minor issues, but otherwise I think this looks good. We may want to totally rework the docs here, but that can happen later.

cmd/cosign/cli/verify.go

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>

developer-guy requested review from asraa and znewman01 September 15, 2022 15:47

haydentherapper reviewed Sep 15, 2022

View reviewed changes

znewman01 mentioned this pull request Sep 15, 2022

fix: fix cert chain validation for verify-blob in non-experimental mode #2256

Merged

znewman01 reviewed Sep 15, 2022

View reviewed changes

developer-guy force-pushed the feature/verify-blob branch 2 times, most recently from 0601020 to 1357e32 Compare September 16, 2022 15:13

developer-guy force-pushed the feature/verify-blob branch from 1357e32 to 8ae1b01 Compare September 16, 2022 15:33

znewman01 reviewed Sep 16, 2022

View reviewed changes

cmd/cosign/cli/verify.go Outdated Show resolved Hide resolved

cmd/cosign/cli/verify.go Outdated Show resolved Hide resolved

cmd/cosign/cli/verify.go Outdated Show resolved Hide resolved

fix: add COSIGN_EXPERIMENTAL=1 for verify-blob

4e729c3

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>

developer-guy force-pushed the feature/verify-blob branch from 8ae1b01 to 4e729c3 Compare September 17, 2022 07:34

developer-guy requested a review from znewman01 September 17, 2022 07:34

developer-guy self-assigned this Sep 17, 2022

dlorenc approved these changes Sep 18, 2022

View reviewed changes

znewman01 approved these changes Sep 18, 2022

View reviewed changes

dlorenc merged commit fde55b6 into sigstore:main Sep 18, 2022

github-actions bot added this to the v1.13.0 milestone Sep 18, 2022

wata727 mentioned this pull request Nov 14, 2022

Add COSIGN_EXPERIMENTAL=1 flag to the verification example terraform-linters/tflint#1588

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add COSIGN_EXPERIMENTAL=1 for verify-bloba #2254

fix: add COSIGN_EXPERIMENTAL=1 for verify-bloba #2254

developer-guy commented Sep 15, 2022

haydentherapper commented Sep 15, 2022 •

edited

codecov-commenter commented Sep 15, 2022 •

edited

haydentherapper Sep 15, 2022

znewman01 Sep 15, 2022

haydentherapper Sep 15, 2022

haydentherapper Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

znewman01 Sep 15, 2022

developer-guy commented Sep 16, 2022

znewman01 left a comment

fix: add COSIGN_EXPERIMENTAL=1 for verify-bloba #2254

fix: add COSIGN_EXPERIMENTAL=1 for verify-bloba #2254

Conversation

developer-guy commented Sep 15, 2022

Summary

Release Note

Documentation

haydentherapper commented Sep 15, 2022 • edited

codecov-commenter commented Sep 15, 2022 • edited

Codecov Report

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

no experimental

Keys

Certificates

Bundles

experimental

Keys

Certs

Bundles

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

no experimental

Keys

Certificates

Bundles

experimental

Keys

Certs

Bundles

developer-guy commented Sep 16, 2022

znewman01 left a comment

Choose a reason for hiding this comment

haydentherapper commented Sep 15, 2022 •

edited

codecov-commenter commented Sep 15, 2022 •

edited