Support self-managed keys when signing with sigstore-go #4368

haydentherapper · 2025-08-26T23:01:12Z

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

codecov · 2025-08-26T23:03:05Z

Codecov Report

❌ Patch coverage is 22.56637% with 175 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.32%. Comparing base (2ef6022) to head (6d27d43).
⚠️ Report is 491 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.32%   -5.79%     
==========================================
  Files         155      217      +62     
  Lines       10044    15148    +5104     
==========================================
+ Hits         4028     5199    +1171     
- Misses       5530     9280    +3750     
- Partials      486      669     +183

☔ 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 · 2025-08-27T00:22:05Z

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

steiza

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

steiza · 2025-08-27T17:45:00Z

cmd/cosign/cli/attest/attest_blob.go

+		// Set to false so sigstore-go fetches the Fulcio certificate,
+		// otherwise SignerFromKeyOpts would through the old signing path
+		issueCertForKey := c.IssueCertificateForExistingKey
+		c.IssueCertificateForExistingKey = false


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?

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.

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.

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

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

Yeah, I think a CHANGELOG entry is just fine.

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]>

ret2libc · 2025-08-29T06:37:36Z

internal/key/svkeypair.go

+	}
+
+	// TODO: Ideally SignerVerifier would return its configured hash.
+	algo, err := signature.GetDefaultAlgorithmDetails(pubKey)


we should use defaultLoadOptions = cosign.GetDefaultLoadOptions() and pass defaultLoadOptions to this method, to ensure that ed25519ph is used instead of ed25519.

steiza

This looks good to me!

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

haydentherapper force-pushed the support-sv-sc branch from 6fa7574 to 072fbea Compare August 26, 2025 23:18

haydentherapper marked this pull request as draft August 26, 2025 23:26

haydentherapper force-pushed the support-sv-sc branch from 072fbea to 020fda7 Compare August 27, 2025 00:05

haydentherapper force-pushed the support-sv-sc branch 2 times, most recently from 0b84033 to 927fffa Compare August 27, 2025 00:29

haydentherapper marked this pull request as ready for review August 27, 2025 00:30

haydentherapper requested a review from steiza August 27, 2025 00:31

haydentherapper mentioned this pull request Aug 27, 2025

Support SigningConfig for Sign and Attest #4324

Open

haydentherapper force-pushed the support-sv-sc branch from 927fffa to 6c5159e Compare August 27, 2025 16:48

steiza reviewed Aug 27, 2025

View reviewed changes

haydentherapper added 2 commits August 28, 2025 15:15

Add issue-certificate flags to attest and attest-blob

be57b46

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

haydentherapper force-pushed the support-sv-sc branch 2 times, most recently from c56b03d to f6a043d Compare August 28, 2025 22:50

Refactor SignerFromKeyOpts to split Fulcio signer into its own method

6d27d43

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 force-pushed the support-sv-sc branch from f6a043d to 6d27d43 Compare August 28, 2025 23:12

ret2libc reviewed Aug 29, 2025

View reviewed changes

steiza approved these changes Aug 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support self-managed keys when signing with sigstore-go #4368

Support self-managed keys when signing with sigstore-go #4368

Uh oh!

haydentherapper commented Aug 26, 2025

Uh oh!

codecov bot commented Aug 26, 2025 •

edited

Loading

Uh oh!

haydentherapper commented Aug 27, 2025 •

edited

Loading

Uh oh!

steiza left a comment

Uh oh!

steiza Aug 27, 2025

Uh oh!

haydentherapper Aug 27, 2025

Uh oh!

steiza Aug 28, 2025

Uh oh!

haydentherapper Aug 28, 2025

Uh oh!

haydentherapper Aug 28, 2025

Uh oh!

steiza Aug 29, 2025

Uh oh!

ret2libc Aug 29, 2025

Uh oh!

steiza left a comment

Uh oh!

Uh oh!

Support self-managed keys when signing with sigstore-go #4368

Are you sure you want to change the base?

Support self-managed keys when signing with sigstore-go #4368

Uh oh!

Conversation

haydentherapper commented Aug 26, 2025

Summary

Release Note

Documentation

Uh oh!

codecov bot commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

haydentherapper commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steiza left a comment

Choose a reason for hiding this comment

Uh oh!

steiza Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

haydentherapper Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

steiza Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

haydentherapper Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

haydentherapper Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

steiza Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

ret2libc Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

steiza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Aug 26, 2025 •

edited

Loading

haydentherapper commented Aug 27, 2025 •

edited

Loading