From ff2a8f4ea297a3f6bd2a76be692ac2d8042bef89 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:11:51 -0700 Subject: [PATCH] fix(sign): pass digest hash algorithm to underlying signer in SignData Fixes #5018 Motivation Signing with a HashiCorp Vault Transit KMS ECDSA P-521 key fails with "signing digest: signing bundle: error signing bundle: unexpected length of digest for hash function specified". This is a regression in the bundle-signing path introduced when SignerVerifierKeypair was added: it correctly computes the digest using the key's own default hash algorithm (SHA-512 for P-521), but never told the underlying signer.SignMessage which hash algorithm that digest was produced with. KMS-backed SignerVerifiers created via pkg/signature/keys.go:SignerVerifierFromKeyRef are constructed with a hardcoded crypto.SHA256 default (unrelated to the actual key's curve), so sigstore-go's ComputeDigestForSigning fell back to that default, saw a 64-byte digest against an expected 32-byte SHA-256 digest, and rejected it with the mismatch error above. Approach internal/key/svkeypair.go: SignerVerifierKeypair.SignData now also passes signatureoptions.WithCryptoSignerOpts(hashType) alongside WithDigest(digest), so the signer is told exactly which hash algorithm produced the digest, regardless of what hash it was constructed with by default. This affects every SignerVerifier-backed signing path (any KMS provider, not just Vault) whenever the key's natural hash isn't SHA-256 (e.g. P-384/P-521 ECDSA, larger RSA keys). For keys whose natural hash is already SHA-256 (e.g. default P-256), this is a no-op: the newly-passed option matches the signer's existing default. Validation go build ./... go test ./internal/key/... ./pkg/signature/... ./pkg/cosign/bundle/... -count=1 ok github.com/sigstore/cosign/v3/internal/key ok github.com/sigstore/cosign/v3/pkg/signature ok github.com/sigstore/cosign/v3/pkg/cosign/bundle Added TestKMSKeypair_ECDSA_P521_SignData, which reproduces the digest length/hash mismatch reported in the issue (verified it fails without this fix with "expected underlying signer to be given crypto.SHA512 opts matching the digest, got ", and passes with it). This was verified with a unit test against a mock SignerVerifier reproducing the KMS signer's option-handling behavior; it was not validated against a live HashiCorp Vault instance. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- internal/key/svkeypair.go | 2 +- internal/key/svkeypair_test.go | 46 +++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/internal/key/svkeypair.go b/internal/key/svkeypair.go index fb373ae4..3f061101 100644 --- a/internal/key/svkeypair.go +++ b/internal/key/svkeypair.go @@ -133,7 +133,7 @@ func (k *SignerVerifierKeypair) SignData(ctx context.Context, data []byte) ([]by h := hashType.New() h.Write(data) digest = h.Sum(nil) - sOpts = append(sOpts, signatureoptions.WithDigest(digest)) + sOpts = append(sOpts, signatureoptions.WithDigest(digest), signatureoptions.WithCryptoSignerOpts(hashType)) } sig, err := k.sv.SignMessage(bytes.NewReader(data), sOpts...) diff --git a/internal/key/svkeypair_test.go b/internal/key/svkeypair_test.go index aba195c2..6ac5a259 100644 --- a/internal/key/svkeypair_test.go +++ b/internal/key/svkeypair_test.go @@ -38,9 +38,10 @@ import ( // mockSignerVerifier is a mock implementation of signature.SignerVerifier for testing. type mockSignerVerifier struct { - pubKey crypto.PublicKey - pubKeyErr error - signErr error + pubKey crypto.PublicKey + pubKeyErr error + signErr error + signerOpts crypto.SignerOpts } func (m *mockSignerVerifier) PublicKey(_ ...signature.PublicKeyOption) (crypto.PublicKey, error) { @@ -50,10 +51,15 @@ func (m *mockSignerVerifier) PublicKey(_ ...signature.PublicKeyOption) (crypto.P return m.pubKey, nil } -func (m *mockSignerVerifier) SignMessage(_ io.Reader, _ ...signature.SignOption) ([]byte, error) { +func (m *mockSignerVerifier) SignMessage(_ io.Reader, opts ...signature.SignOption) ([]byte, error) { if m.signErr != nil { return nil, m.signErr } + var signerOpts crypto.SignerOpts + for _, opt := range opts { + opt.ApplyCryptoSignerOpts(&signerOpts) + } + m.signerOpts = signerOpts return []byte("mock-signature"), nil } @@ -220,6 +226,9 @@ func TestKMSKeypair_ECDSA_Methods(t *testing.T) { if !bytes.Equal(digest, expectedDigest) { t.Errorf("expected digest %x, got %x", expectedDigest, digest) } + if sv.signerOpts == nil || sv.signerOpts.HashFunc() != crypto.SHA256 { + t.Errorf("expected underlying signer to be given crypto.SHA256 opts, got %v", sv.signerOpts) + } }) t.Run("SignData with error", func(t *testing.T) { @@ -241,6 +250,35 @@ func TestKMSKeypair_ECDSA_Methods(t *testing.T) { }) } +// TestKMSKeypair_ECDSA_P521_SignData exercises the P-521 case reported in +// https://github.com/sigstore/cosign/issues/5018, where a SHA-512 digest was +// signed against a signer configured for SHA-256, producing a digest/hash +// length mismatch for KMS providers (e.g. HashiCorp Vault) that validate the +// crypto.SignerOpts passed alongside the digest. +func TestKMSKeypair_ECDSA_P521_SignData(t *testing.T) { + ecdsaPriv, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate ecdsa key: %v", err) + } + sv := &mockSignerVerifier{pubKey: &ecdsaPriv.PublicKey} + kp, err := NewSignerVerifierKeypair(sv, nil) + if err != nil { + t.Fatalf("failed to create KMSKeypair: %v", err) + } + + data := []byte("some data to sign") + _, digest, err := kp.SignData(context.Background(), data) + if err != nil { + t.Fatalf("SignData returned an error: %v", err) + } + if len(digest) != crypto.SHA512.Size() { + t.Errorf("expected a SHA-512 digest of length %d, got %d", crypto.SHA512.Size(), len(digest)) + } + if sv.signerOpts == nil || sv.signerOpts.HashFunc() != crypto.SHA512 { + t.Errorf("expected underlying signer to be given crypto.SHA512 opts matching the digest, got %v", sv.signerOpts) + } +} + func TestKMSKeypair_ED25519_Methods(t *testing.T) { _, ed25519Priv, err := ed25519.GenerateKey(rand.Reader) if err != nil {