Skip to content

Conversation

@haydentherapper
Copy link
Contributor

This creates a wrapper around the Keypair interface when a SignerVerifier is provided for signing with KMS or any other provided keys. This also retains support for --issue-certificate to request a certificate for a managed key.

Fixes #4327

Summary

Release Note

Documentation

@haydentherapper haydentherapper requested a review from a team as a code owner August 26, 2025 23:01
@codecov
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 22.56637% with 175 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.75%. Comparing base (2ef6022) to head (0c22436).
⚠️ Report is 501 commits behind head on main.

Files with missing lines Patch % Lines
cmd/cosign/cli/attest/attest_blob.go 2.22% 43 Missing and 1 partial ⚠️
cmd/cosign/cli/sign/sign_blob.go 2.22% 43 Missing and 1 partial ⚠️
cmd/cosign/cli/attest_blob.go 0.00% 29 Missing ⚠️
cmd/cosign/cli/attest.go 0.00% 22 Missing ⚠️
internal/key/svkeypair.go 81.03% 8 Missing and 3 partials ⚠️
cmd/cosign/cli/sign/sign.go 20.00% 8 Missing ⚠️
cmd/cosign/cli/attest/attest.go 0.00% 6 Missing ⚠️
cmd/cosign/cli/signblob.go 0.00% 5 Missing ⚠️
cmd/cosign/cli/options/attest.go 0.00% 3 Missing ⚠️
cmd/cosign/cli/options/attest_blob.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4368      +/-   ##
==========================================
- Coverage   40.10%   34.75%   -5.36%     
==========================================
  Files         155      217      +62     
  Lines       10044    15329    +5285     
==========================================
+ Hits         4028     5327    +1299     
- Misses       5530     9311    +3781     
- Partials      486      691     +205     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@haydentherapper
Copy link
Contributor Author

haydentherapper commented Aug 27, 2025

Verified this works with Rekor v2:

curl -LO https://raw.githubusercontent.com/sigstore/root-signing-staging/refs/heads/main/metadata/root_history/1.root.json
cosign initialize --root 1.root.json --mirror="https://tuf-repo-cdn.sigstage.dev" 

cosign generate-key-pair

cosign sign-blob --new-bundle-format --bundle artifact.sigstore.json --yes --use-signing-config --key cosign.key README.md
cosign verify-blob --new-bundle-format --bundle artifact.sigstore.json --key cosign.pub README.md

cosign sign-blob --new-bundle-format --bundle artifact.sigstore.json --yes --use-signing-config --key cosign.key --issue-certificate README.md
cosign verify-blob --new-bundle-format --bundle artifact.sigstore.json --certificate-identity $email --certificate-oidc-issuer https://accounts.google.com --use-signed-timestamps README.md

cosign attest-blob --use-signing-config --statement intoto.txt --bundle sigstore.att.json --key cosign.key
cosign verify-blob-attestation --new-bundle-format --bundle sigstore.att.json --digestAlg sha256 --digest="b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" --key cosign.pub

cosign attest-blob --use-signing-config --statement intoto.txt --bundle sigstore.att.json --key cosign.key --issue-certificate
cosign verify-blob-attestation --new-bundle-format --bundle sigstore.att.json --digestAlg sha256 --digest="b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" --certificate-identity $email --certificate-oidc-issuer https://accounts.google.com --use-signed-timestamps

@haydentherapper haydentherapper force-pushed the support-sv-sc branch 2 times, most recently from 0b84033 to 927fffa Compare August 27, 2025 00:29
@haydentherapper haydentherapper marked this pull request as ready for review August 27, 2025 00:30
@haydentherapper haydentherapper requested a review from steiza August 27, 2025 00:31
Copy link
Member

@steiza steiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM - I don't understand the IssueCertificateForExistingKey part though.

Comment on lines 166 to 169
// Set to false so sigstore-go fetches the Fulcio certificate,
// otherwise SignerFromKeyOpts would through the old signing path
issueCertForKey := c.IssueCertificateForExistingKey
c.IssueCertificateForExistingKey = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this code block. What flag to I pass to cosign attest-blob to specify this behavior? Where does this get used by sigstore-go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we have too much duplication everywhere, I had missed adding --issue-certificate to attest-blob and attest - I just added this.

For context, --issue-certificate was added as part of https://blog.sigstore.dev/adopting-sigstore-incrementally-1b56a69b8c15/, to request a Fulcio certificate even if a key is provided.

This flag isn't used by sigstore-go, it's used by SignerFromKeyOpts, which does a few things - it load a key from KMS/PIV/disk, it generates a key, and if IssueCertificateForExistingKey is set or an ephemeral key was generated, it requests a certificate from Fulcio. This is the root cause of the issue, the method does too much.

sigstore-go will be handling certificate requests, so I need to set IssueCertificateForExistingKey to false to prevent SignerFromKeyOpts from trying to generate a cert.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhhh, okay, now I see. Still, mutating the configuration like this makes me a bit nervous (I've been bitten before in the cosign code base doing similar things!)

The "problem" is that SignerFromKeyOpts does two things:

  • Constructs the SignerVerifier
  • Calls keylessSigner in some cases

There aren't that many places we call SignerFromKeyOpts. Instead of mutating the config like this, would it make sense to have keylessSigner become a public method and call it when needed from the cmd/cosign/cli files? I'm not 100% sure if that's better or not, but I think it would allow us to avoid mutating the configuration like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I'm happy to try this out.
FYI @ret2libc, this would also solve the e2e test failure you were running into.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored this and then searched for method usage across GitHub and found it's being used in some libraries. So one of two options:

  • Call this out in the CHANGELOG - we provide no guarantees for API breakages, and because the method signature has changed, it'll force clients to make an update
  • Check if SigningConfig is set in SignerFromKeyOpts - This is kinda gross

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think a CHANGELOG entry is just fine.

@haydentherapper haydentherapper force-pushed the support-sv-sc branch 3 times, most recently from f6a043d to 6d27d43 Compare August 28, 2025 23:12
steiza
steiza previously approved these changes Aug 29, 2025
Copy link
Member

@steiza steiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

This creates a wrapper around the Keypair interface when a
SignerVerifier is provided for signing with KMS or any other provided
keys. This also retains support for --issue-certificate to request a
certificate for a managed key.

Fixes sigstore#4327

Signed-off-by: Hayden <[email protected]>
This is for uniformity with sign/sign-blob.

Signed-off-by: Hayden <[email protected]>
Now, we can generate a SignerVerifier from a provided key without
mandating that we also request a Fulcio certificate when
"issue-certificate" is provided.

Signed-off-by: Hayden <[email protected]>
@haydentherapper
Copy link
Contributor Author

Re-verified all commands work with after the rebase. Signing with a non-ecdsa-p256 key doesn't work with Rekor v2, but that's a bug in sigstore-go I'm fixing now, no need to block this PR.

@haydentherapper haydentherapper merged commit 59b9930 into sigstore:main Sep 3, 2025
29 checks passed
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Sep 16, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cosign](https://github.com/sigstore/cosign) | minor | `2.5.3` -> `2.6.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>sigstore/cosign (cosign)</summary>

### [`v2.6.0`](https://github.com/sigstore/cosign/blob/HEAD/CHANGELOG.md#v260)

[Compare Source](sigstore/cosign@v2.5.3...v2.6.0)

v2.6.0 introduces a number of new features, including:

- Signing an in-toto statement rather than Cosign constructing one from a predicate, along with verifying a statement's subject using a digest and digest algorithm rather than providing a file reference ([#&#8203;4306](sigstore/cosign#4306))
- Uploading a signature and its verification material (a ["bundle"](https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_bundle.proto)) as an OCI Image 1.1 referring artifact, completing [#&#8203;3927](sigstore/cosign#3927) ([#&#8203;4316](sigstore/cosign#4316))
- Providing service URLs for signing and attesting using a [SigningConfig](https://github.com/sigstore/protobuf-specs/blob/4df5baadcdb582a70c2bc032e042c0a218eb3841/protos/sigstore_trustroot.proto#L185). Note that this is required when using a [Rekor v2](https://github.com/sigstore/rekor-tiles) instance ([#&#8203;4319](sigstore/cosign#4319))

Example generation and verification of a signed in-toto statement:

```
cosign attest-blob --new-bundle-format=true --bundle="digest-key-test.sigstore.json" --key="cosign.key" --statement="../sigstore-go/examples/sigstore-go-signing/intoto.txt"
cosign verify-blob-attestation --bundle="digest-key-test.sigstore.json" --key=cosign.pub --type=unused --digest="b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" --digestAlg="sha256"
```

Example container signing and verification using the new bundle format and referring artifacts:

```
cosign sign --new-bundle-format=true ghcr.io/user/alpine@sha256:a19367999603840546b8612572e338ec076c6d1f2fec61760a9e11410f546733
cosign verify --new-bundle-format=true ghcr.io/user/alpine@sha256:a19367999603840546b8612572e338ec076c6d1f2fec61760a9e11410f546733
```

Example usage of a signing config provided by the public good instance's TUF repository:

```
cosign sign-blob --use-signing-config --bundle sigstore.json README.md
cosign verify-blob --new-bundle-format --bundle sigstore.json --certificate-identity $EMAIL --certificate-oidc-issuer $ISSUER --use-signed-timestamps README.md
```

v2.6.0 leverages sigstore-go's signing and verification APIs gated behind these new flags. In an upcoming major release, we will be
updating Cosign to default to producing and consuming bundles to align with all other Sigstore SDKs.

#### Features

- Add to `attest-blob` the ability to supply a complete in-toto statement, and add to `verify-blob-attestation` the ability to verify with just a digest ([#&#8203;4306](sigstore/cosign#4306))
- Have cosign sign support bundle format ([#&#8203;4316](sigstore/cosign#4316))
- Add support for SigningConfig for sign-blob/attest-blob, support Rekor v2 ([#&#8203;4319](sigstore/cosign#4319))
- Add support for SigningConfig in sign/attest ([#&#8203;4371](sigstore/cosign#4371))
- Support self-managed keys when signing with sigstore-go ([#&#8203;4368](sigstore/cosign#4368))
- Don't require timestamps when verifying with a key ([#&#8203;4337](sigstore/cosign#4337))
- Don't load content from TUF if trusted root path is specified ([#&#8203;4347](sigstore/cosign#4347))
- Add a terminal spinner while signing with sigstore-go ([#&#8203;4402](sigstore/cosign#4402))
- Require exclusively a SigningConfig or service URLs when signing ([#&#8203;4403](sigstore/cosign#4403))
- Remove SHA256 assumption in sign-blob/verify-blob ([#&#8203;4050](sigstore/cosign#4050))
- Bump sigstore-go, support alternative hash algorithms with keys ([#&#8203;4386](sigstore/cosign#4386))

#### Breaking API Changes

- `sign.SignerFromKeyOpts` no longer generates a key. Instead, it returns whether or not the client needs to generate a key, and if so, clients
  should call `sign.KeylessSigner`. This allows clients to more easily manage key generation.

#### Bug Fixes

- Verify subject with bundle only when checking claims ([#&#8203;4320](sigstore/cosign#4320))
- Fixes to cosign sign / verify for the new bundle format ([#&#8203;4346](sigstore/cosign#4346))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMTMuNSIsInVwZGF0ZWRJblZlciI6IjQxLjExMy41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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.

Support self-managed keys with sigstore-go signing

3 participants