Skip to content

Commit f9897c5

Browse files
Fix signing and verification with ed25519 keys with bundles and Rekor
With the recent changes we made to use sigstore-go rather than Cosign for signing and verification, ed25519 managed key support broke, because we were incorrectly specifying ed25519ph for dsse Rekor entries and not specifying ed25519ph for hashedrekord entries. This PR correctly sets load options for when signing and verifying a blob (using the prehash variant) and when signing/verifying attestations (using the pure variant). This also fixes a bug where the SignerVerifier Keypair didn't handle crypto.Hash(0) for ed25519, which specifies no hash when signing. This has been tested with sign/verify, sign-blob/verify-blob, attest/verify-attestation, and attest-blob/verify-blob-attestation. Signed-off-by: Hayden <[email protected]>
1 parent 0a1de88 commit f9897c5

File tree

12 files changed

+84
-27
lines changed

12 files changed

+84
-27
lines changed

cmd/cosign/cli/attest/attest.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"github.com/sigstore/rekor/pkg/generated/client"
4747
"github.com/sigstore/rekor/pkg/generated/models"
4848
"github.com/sigstore/sigstore-go/pkg/sign"
49+
"github.com/sigstore/sigstore/pkg/signature"
4950
"github.com/sigstore/sigstore/pkg/signature/dsse"
5051
signatureoptions "github.com/sigstore/sigstore/pkg/signature/options"
5152
)
@@ -164,11 +165,14 @@ func (c *AttestCommand) Exec(ctx context.Context, imageRef string) error {
164165
var err error
165166

166167
if c.Sk || c.Slot != "" || c.KeyRef != "" || c.CertPath != "" {
168+
// Set no load options so that Ed25519 is preferred over Ed25519ph, required for signing DSSEs
169+
var signOpts []signature.LoadOption
170+
c.KeyOpts.DefaultLoadOptions = &signOpts
167171
sv, _, err = cosign_sign.SignerFromKeyOpts(ctx, c.CertPath, c.CertChainPath, c.KeyOpts)
168172
if err != nil {
169173
return fmt.Errorf("getting signer: %w", err)
170174
}
171-
keypair, err = key.NewSignerVerifierKeypair(sv, c.DefaultLoadOptions)
175+
keypair, err = key.NewSignerVerifierKeypair(sv, &signOpts)
172176
if err != nil {
173177
return fmt.Errorf("creating signerverifier keypair: %w", err)
174178
}

cmd/cosign/cli/attest/attest_blob.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,14 @@ func (c *AttestBlobCommand) Exec(ctx context.Context, artifactPath string) error
165165
var err error
166166

167167
if c.Sk || c.Slot != "" || c.KeyRef != "" || c.CertPath != "" {
168+
// Set no load options so that Ed25519 is preferred over Ed25519ph, required for signing DSSEs
169+
var signOpts []signature.LoadOption
170+
c.KeyOpts.DefaultLoadOptions = &signOpts
168171
sv, _, err = cosign_sign.SignerFromKeyOpts(ctx, c.CertPath, c.CertChainPath, c.KeyOpts)
169172
if err != nil {
170173
return fmt.Errorf("getting signer: %w", err)
171174
}
172-
keypair, err = key.NewSignerVerifierKeypair(sv, c.DefaultLoadOptions)
175+
keypair, err = key.NewSignerVerifierKeypair(sv, &signOpts)
173176
if err != nil {
174177
return fmt.Errorf("creating signerverifier keypair: %w", err)
175178
}

cmd/cosign/cli/sign/sign.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,14 @@ func signDigestBundle(ctx context.Context, digest name.Digest, ko options.KeyOpt
256256
var err error
257257

258258
if ko.Sk || ko.Slot != "" || ko.KeyRef != "" || signOpts.Cert != "" {
259+
// Set no load options so that Ed25519 is preferred over Ed25519ph, required for signing DSSEs
260+
var signLoadOpts []signature.LoadOption
261+
ko.DefaultLoadOptions = &signLoadOpts
259262
sv, _, err = SignerFromKeyOpts(ctx, signOpts.Cert, signOpts.CertChain, ko)
260263
if err != nil {
261264
return fmt.Errorf("getting signer: %w", err)
262265
}
263-
keypair, err = key.NewSignerVerifierKeypair(sv, ko.DefaultLoadOptions)
266+
keypair, err = key.NewSignerVerifierKeypair(sv, &signLoadOpts)
264267
if err != nil {
265268
return fmt.Errorf("creating signerverifier keypair: %w", err)
266269
}

cmd/cosign/cli/sign/sign_blob.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ func SignBlobCmd(ro *options.RootOptions, ko options.KeyOpts, payloadPath string
8787
var err error
8888

8989
if ko.Sk || ko.Slot != "" || ko.KeyRef != "" {
90+
// Default load options prefer Ed25519ph over Ed25519, required for blobs with hashedrekord
9091
sv, _, err = SignerFromKeyOpts(ctx, "", "", ko)
9192
if err != nil {
9293
return nil, fmt.Errorf("getting signer: %w", err)

cmd/cosign/cli/verify/verify.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,9 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) {
240240
var pubKey signature.Verifier
241241
switch {
242242
case keyRef != "":
243-
pubKey, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, keyRef, c.HashAlgorithm)
243+
// Set no load options so that Ed25519 is preferred over Ed25519ph, required for verifying DSSEs
244+
var signOpts []signature.LoadOption
245+
pubKey, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, keyRef, c.HashAlgorithm, &signOpts)
244246
if err != nil {
245247
return fmt.Errorf("loading public key: %w", err)
246248
}

cmd/cosign/cli/verify/verify_attestation.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"github.com/sigstore/cosign/v2/pkg/policy"
4141
sigs "github.com/sigstore/cosign/v2/pkg/signature"
4242
"github.com/sigstore/sigstore-go/pkg/root"
43+
"github.com/sigstore/sigstore/pkg/signature"
4344
)
4445

4546
// VerifyAttestationCommand verifies a signature on a supplied container image
@@ -210,7 +211,9 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
210211
// Keys are optional!
211212
switch {
212213
case keyRef != "":
213-
co.SigVerifier, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, keyRef, c.HashAlgorithm)
214+
// Set no load options so that Ed25519 is preferred over Ed25519ph, required for verifying DSSEs
215+
var signOpts []signature.LoadOption
216+
co.SigVerifier, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, keyRef, c.HashAlgorithm, &signOpts)
214217
if err != nil {
215218
return fmt.Errorf("loading public key: %w", err)
216219
}

cmd/cosign/cli/verify/verify_blob.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (c *VerifyBlobCmd) Exec(ctx context.Context, blobRef string) error {
122122
opts := make([]static.Option, 0)
123123
switch {
124124
case c.KeyRef != "":
125-
co.SigVerifier, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, c.KeyRef, c.HashAlgorithm)
125+
co.SigVerifier, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, c.KeyRef, c.HashAlgorithm, c.DefaultLoadOptions)
126126
if err != nil {
127127
return fmt.Errorf("loading public key: %w", err)
128128
}
@@ -265,7 +265,7 @@ func (c *VerifyBlobCmd) Exec(ctx context.Context, blobRef string) error {
265265
bundleCert, err := loadCertFromPEM(certBytes)
266266
if err != nil {
267267
// check if cert is actually a public key
268-
co.SigVerifier, err = sigs.LoadPublicKeyRaw(certBytes, crypto.SHA256)
268+
co.SigVerifier, err = sigs.LoadPublicKeyRaw(certBytes, crypto.SHA256, c.DefaultLoadOptions)
269269
if err != nil {
270270
return fmt.Errorf("loading verifier from bundle: %w", err)
271271
}

cmd/cosign/cli/verify/verify_blob_attestation.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"github.com/sigstore/sigstore-go/pkg/root"
4848
sgverify "github.com/sigstore/sigstore-go/pkg/verify"
4949
"github.com/sigstore/sigstore/pkg/cryptoutils"
50+
"github.com/sigstore/sigstore/pkg/signature"
5051
)
5152

5253
// VerifyBlobAttestationCommand verifies an attestation on a supplied blob
@@ -127,12 +128,16 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st
127128
NewBundleFormat: c.NewBundleFormat && checkNewBundle(c.BundlePath),
128129
}
129130

131+
// Set no load options so that Ed25519 is preferred over Ed25519ph, required for verifying DSSEs
132+
var signOpts []signature.LoadOption
133+
c.DefaultLoadOptions = &signOpts
134+
130135
// Keys are optional!
131136
var cert *x509.Certificate
132137
opts := make([]static.Option, 0)
133138
switch {
134139
case c.KeyRef != "":
135-
co.SigVerifier, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, c.KeyRef, c.HashAlgorithm)
140+
co.SigVerifier, err = sigs.PublicKeyFromKeyRefWithHashAlgo(ctx, c.KeyRef, c.HashAlgorithm, c.DefaultLoadOptions)
136141
if err != nil {
137142
return fmt.Errorf("loading public key: %w", err)
138143
}
@@ -329,7 +334,7 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st
329334
bundleCert, err := loadCertFromPEM(certBytes)
330335
if err != nil {
331336
// check if cert is actually a public key
332-
co.SigVerifier, err = sigs.LoadPublicKeyRaw(certBytes, crypto.SHA256)
337+
co.SigVerifier, err = sigs.LoadPublicKeyRaw(certBytes, crypto.SHA256, c.DefaultLoadOptions)
333338
if err != nil {
334339
return fmt.Errorf("loading verifier from bundle: %w", err)
335340
}

internal/key/svkeypair.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,20 @@ func (k *SignerVerifierKeypair) GetPublicKeyPem() (string, error) {
125125

126126
// SignData signs the given data with the SignerVerifier.
127127
func (k *SignerVerifierKeypair) SignData(ctx context.Context, data []byte) ([]byte, []byte, error) {
128-
h := k.sigAlg.GetHashType().New()
129-
h.Write(data)
130-
digest := h.Sum(nil)
131-
sOpts := []signature.SignOption{signatureoptions.WithContext(ctx), signatureoptions.WithDigest(digest)}
128+
sOpts := []signature.SignOption{}
129+
130+
hf := k.sigAlg.GetHashType()
131+
dataToSign := data
132+
// RSA, ECDSA, and Ed25519ph sign a digest, while pure Ed25519's interface takes data and hashes during signing
133+
if hf != crypto.Hash(0) {
134+
hasher := hf.New()
135+
hasher.Write(data)
136+
dataToSign = hasher.Sum(nil)
137+
sOpts = append(sOpts, signatureoptions.WithDigest(dataToSign))
138+
}
132139
sig, err := k.sv.SignMessage(bytes.NewReader(data), sOpts...)
133140
if err != nil {
134141
return nil, nil, err
135142
}
136-
return sig, digest, nil
143+
return sig, dataToSign, nil
137144
}

internal/key/svkeypair_test.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1"
3535
"github.com/sigstore/sigstore/pkg/cryptoutils"
3636
"github.com/sigstore/sigstore/pkg/signature"
37+
"github.com/sigstore/sigstore/pkg/signature/options"
3738
)
3839

3940
// mockSignerVerifier is a mock implementation of signature.SignerVerifier for testing.
@@ -61,7 +62,7 @@ func (m *mockSignerVerifier) VerifySignature(_, _ io.Reader, _ ...signature.Veri
6162
return errors.New("not implemented")
6263
}
6364

64-
func TestNewKMSKeypair(t *testing.T) {
65+
func TestNewSignerVerifierKeypairKeypair(t *testing.T) {
6566
ecdsaPriv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
6667
if err != nil {
6768
t.Fatalf("failed to generate ecdsa key: %v", err)
@@ -78,6 +79,7 @@ func TestNewKMSKeypair(t *testing.T) {
7879
testCases := []struct {
7980
name string
8081
sv signature.SignerVerifier
82+
prehash bool
8183
expectErr bool
8284
errMsg string
8385
}{
@@ -102,6 +104,14 @@ func TestNewKMSKeypair(t *testing.T) {
102104
},
103105
expectErr: false,
104106
},
107+
{
108+
name: "ED25519ph key",
109+
sv: &mockSignerVerifier{
110+
pubKey: ed25519Priv.Public(),
111+
},
112+
prehash: true,
113+
expectErr: false,
114+
},
105115
{
106116
name: "Unsupported key type",
107117
sv: &mockSignerVerifier{
@@ -122,7 +132,11 @@ func TestNewKMSKeypair(t *testing.T) {
122132

123133
for _, tc := range testCases {
124134
t.Run(tc.name, func(t *testing.T) {
125-
kp, err := NewSignerVerifierKeypair(tc.sv, nil)
135+
var loadOpts *[]signature.LoadOption
136+
if tc.prehash {
137+
loadOpts = &[]signature.LoadOption{options.WithED25519ph()}
138+
}
139+
kp, err := NewSignerVerifierKeypair(tc.sv, loadOpts)
126140
if tc.expectErr {
127141
if err == nil {
128142
t.Errorf("expected an error, but got none")
@@ -137,6 +151,11 @@ func TestNewKMSKeypair(t *testing.T) {
137151
t.Error("expected a keypair, but got nil")
138152
}
139153
}
154+
if !tc.expectErr {
155+
if _, _, err := kp.SignData(context.Background(), []byte("data")); err != nil {
156+
t.Errorf("unexpected error: %v", err)
157+
}
158+
}
140159
})
141160
}
142161
}

0 commit comments

Comments
 (0)