From 79493391287ffc2554f142549814ecfd2cdd6114 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 18 Apr 2026 12:10:34 +0200 Subject: [PATCH 1/2] fix(attestation): decode DSSE signature at bundle source Move the double-base64 fix into BundleFromDSSEEnvelope so new Sigstore bundles carry a properly decoded signature by construction. FixSignatureInBundle is retained in the verifier for backward compatibility with attestations stored before the fix. Promote the envelope-to-bundle test helper to the shared testhelpers package so other biz integration tests can reuse it. Follow-up to #3055. Signed-off-by: Miguel Martinez Trivino --- app/cli/pkg/action/attestation_push.go | 3 - .../pkg/biz/testhelpers/attestation.go | 43 +++++++++ .../pkg/biz/workflowrun_integration_test.go | 18 +--- pkg/attestation/attestations.go | 15 ++- pkg/attestation/attestations_test.go | 94 +++++++++++++++++++ 5 files changed, 151 insertions(+), 22 deletions(-) create mode 100644 app/controlplane/pkg/biz/testhelpers/attestation.go create mode 100644 pkg/attestation/attestations_test.go diff --git a/app/cli/pkg/action/attestation_push.go b/app/cli/pkg/action/attestation_push.go index 07aab5d66..1609df355 100644 --- a/app/cli/pkg/action/attestation_push.go +++ b/app/cli/pkg/action/attestation_push.go @@ -25,7 +25,6 @@ import ( "time" pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" - "github.com/chainloop-dev/chainloop/pkg/attestation" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter" v1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/renderer" @@ -304,8 +303,6 @@ func (action *AttestationPush) saveBundle(bundle *protobundle.Bundle) error { } func pushToControlPlane(ctx context.Context, conn *grpc.ClientConn, bundle *protobundle.Bundle, workflowRunID string, markVersionAsReleased bool) (string, error) { - // remove additional base64 encoding in signature. See https://github.com/chainloop-dev/chainloop/issues/1832 - attestation.FixSignatureInBundle(bundle) encodedBundle, err := encodeBundle(bundle) if err != nil { return "", fmt.Errorf("encoding attestation: %w", err) diff --git a/app/controlplane/pkg/biz/testhelpers/attestation.go b/app/controlplane/pkg/biz/testhelpers/attestation.go new file mode 100644 index 000000000..52c5a47a6 --- /dev/null +++ b/app/controlplane/pkg/biz/testhelpers/attestation.go @@ -0,0 +1,43 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package testhelpers + +import ( + "encoding/json" + "os" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/attestation" + "github.com/secure-systems-lab/go-securesystemslib/dsse" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/encoding/protojson" +) + +// BundleBytesFromEnvelope reads a DSSE envelope fixture from disk, wraps it into a +// Sigstore bundle, and returns its protojson-encoded bytes. Useful to feed envelope +// test fixtures into APIs that now only accept Sigstore bundles. +func BundleBytesFromEnvelope(t *testing.T, path string) []byte { + t.Helper() + raw, err := os.ReadFile(path) + require.NoError(t, err) + var env dsse.Envelope + require.NoError(t, json.Unmarshal(raw, &env)) + b, err := attestation.BundleFromDSSEEnvelope(&env) + require.NoError(t, err) + out, err := protojson.Marshal(b) + require.NoError(t, err) + return out +} diff --git a/app/controlplane/pkg/biz/workflowrun_integration_test.go b/app/controlplane/pkg/biz/workflowrun_integration_test.go index 9900669c5..daffed588 100644 --- a/app/controlplane/pkg/biz/workflowrun_integration_test.go +++ b/app/controlplane/pkg/biz/workflowrun_integration_test.go @@ -468,18 +468,6 @@ func testBundle(t *testing.T, path string) (*v1.Bundle, []byte) { return &bundle, bundleJSON } -// testBundleBytesFromEnvelope wraps a DSSE envelope file into a Sigstore bundle and returns its protojson bytes. -func testBundleBytesFromEnvelope(t *testing.T, path string) []byte { - _, envBytes := testEnvelope(t, path) - var env dsse.Envelope - require.NoError(t, json.Unmarshal(envBytes, &env)) - b, err := attestation.BundleFromDSSEEnvelope(&env) - require.NoError(t, err) - out, err := protojson.Marshal(b) - require.NoError(t, err) - return out -} - const ( version1 = "v1" version2 = "v2" @@ -519,7 +507,7 @@ func setupWorkflowRunTestData(t *testing.T, suite *testhelpers.TestingUseCases, ProjectVersion: version1, }) assert.NoError(err) - bundleBytes := testBundleBytesFromEnvelope(t, "testdata/attestations/full.json") + bundleBytes := testhelpers.BundleBytesFromEnvelope(t, "testdata/attestations/full.json") d, err := suite.WorkflowRun.SaveAttestation(ctx, s.runOrg1.ID.String(), bundleBytes) assert.NoError(err) s.digestAtt1 = d.String() @@ -530,7 +518,7 @@ func setupWorkflowRunTestData(t *testing.T, suite *testhelpers.TestingUseCases, ProjectVersion: version1, }) assert.NoError(err) - bundleBytes = testBundleBytesFromEnvelope(t, "testdata/attestations/empty.json") + bundleBytes = testhelpers.BundleBytesFromEnvelope(t, "testdata/attestations/empty.json") d, err = suite.WorkflowRun.SaveAttestation(ctx, s.runOrg2.ID.String(), bundleBytes) assert.NoError(err) s.digestAttOrg2 = d.String() @@ -541,7 +529,7 @@ func setupWorkflowRunTestData(t *testing.T, suite *testhelpers.TestingUseCases, ProjectVersion: version2, }) assert.NoError(err) - bundleBytes = testBundleBytesFromEnvelope(t, "testdata/attestations/with-string.json") + bundleBytes = testhelpers.BundleBytesFromEnvelope(t, "testdata/attestations/with-string.json") d, err = suite.WorkflowRun.SaveAttestation(ctx, s.runOrg2Public.ID.String(), bundleBytes) assert.NoError(err) s.digestAttPublic = d.String() diff --git a/pkg/attestation/attestations.go b/pkg/attestation/attestations.go index 2a03fd3ef..e87e26530 100644 --- a/pkg/attestation/attestations.go +++ b/pkg/attestation/attestations.go @@ -70,10 +70,15 @@ func DSSEEnvelopeFromBundle(bundle *protobundle.Bundle) *dsse.Envelope { } func BundleFromDSSEEnvelope(dsseEnvelope *dsse.Envelope) (*protobundle.Bundle, error) { - // DSSE Envelope is already base64 encoded, we need to decode to prevent it from being encoded twice + // DSSE Envelope payload and signature are base64 encoded, we need to decode them so they + // are not encoded twice when stored as raw bytes in the Sigstore bundle. See #1832. payload, err := base64.StdEncoding.DecodeString(dsseEnvelope.Payload) if err != nil { - return nil, fmt.Errorf("decoding: %w", err) + return nil, fmt.Errorf("decoding payload: %w", err) + } + sig, err := base64.StdEncoding.DecodeString(dsseEnvelope.Signatures[0].Sig) + if err != nil { + return nil, fmt.Errorf("decoding signature: %w", err) } return &protobundle.Bundle{ MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.3", @@ -82,7 +87,7 @@ func BundleFromDSSEEnvelope(dsseEnvelope *dsse.Envelope) (*protobundle.Bundle, e PayloadType: dsseEnvelope.PayloadType, Signatures: []*sigstoredsse.Signature{ { - Sig: []byte(dsseEnvelope.Signatures[0].Sig), + Sig: sig, Keyid: dsseEnvelope.Signatures[0].KeyID, }, }, @@ -106,7 +111,9 @@ func DSSEEnvelopeFromBundleBytes(bundle []byte) (*dsse.Envelope, error) { } // FixSignatureInBundle removes any additional base64 encoding from the signature in the bundle. -// Old attestations have signatures base64 encoded twice, see https://github.com/chainloop-dev/chainloop/issues/1832 +// Kept for backward compatibility with attestations stored before the fix for +// https://github.com/chainloop-dev/chainloop/issues/1832, whose signatures are base64-encoded twice. +// New bundles produced by BundleFromDSSEEnvelope already carry a properly decoded signature. func FixSignatureInBundle(bundle *protobundle.Bundle) { sig := bundle.GetDsseEnvelope().GetSignatures()[0].GetSig() dst := make([]byte, base64.StdEncoding.EncodedLen(len(sig))) diff --git a/pkg/attestation/attestations_test.go b/pkg/attestation/attestations_test.go new file mode 100644 index 000000000..37542b141 --- /dev/null +++ b/pkg/attestation/attestations_test.go @@ -0,0 +1,94 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package attestation_test + +import ( + "encoding/base64" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/attestation" + "github.com/secure-systems-lab/go-securesystemslib/dsse" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Guards against the double-base64 bug in https://github.com/chainloop-dev/chainloop/issues/1832. +func TestBundleFromDSSEEnvelopeDecodesSignature(t *testing.T) { + rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} + rawPayload := []byte(`{"_type":"statement"}`) + + env := &dsse.Envelope{ + PayloadType: "application/vnd.in-toto+json", + Payload: base64.StdEncoding.EncodeToString(rawPayload), + Signatures: []dsse.Signature{ + {KeyID: "key-1", Sig: base64.StdEncoding.EncodeToString(rawSig)}, + }, + } + + bundle, err := attestation.BundleFromDSSEEnvelope(env) + require.NoError(t, err) + + gotEnv := bundle.GetDsseEnvelope() + assert.Equal(t, rawPayload, gotEnv.GetPayload()) + require.Len(t, gotEnv.GetSignatures(), 1) + assert.Equal(t, rawSig, gotEnv.GetSignatures()[0].GetSig()) + assert.Equal(t, "key-1", gotEnv.GetSignatures()[0].GetKeyid()) +} + +func TestBundleRoundTripWithFixedSignature(t *testing.T) { + rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} + encodedSig := base64.StdEncoding.EncodeToString(rawSig) + + env := &dsse.Envelope{ + PayloadType: "application/vnd.in-toto+json", + Payload: base64.StdEncoding.EncodeToString([]byte("payload")), + Signatures: []dsse.Signature{ + {KeyID: "key-1", Sig: encodedSig}, + }, + } + + bundle, err := attestation.BundleFromDSSEEnvelope(env) + require.NoError(t, err) + + before := bundle.GetDsseEnvelope().GetSignatures()[0].GetSig() + attestation.FixSignatureInBundle(bundle) + assert.Equal(t, before, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig(), + "FixSignatureInBundle should be a no-op on properly formed bundles") + + gotEnv := attestation.DSSEEnvelopeFromBundle(bundle) + assert.Equal(t, encodedSig, gotEnv.Signatures[0].Sig) +} + +func TestFixSignatureInBundleRepairsLegacyBundles(t *testing.T) { + rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} + encodedSig := base64.StdEncoding.EncodeToString(rawSig) + + env := &dsse.Envelope{ + PayloadType: "application/vnd.in-toto+json", + Payload: base64.StdEncoding.EncodeToString([]byte("payload")), + Signatures: []dsse.Signature{ + {KeyID: "key-1", Sig: encodedSig}, + }, + } + bundle, err := attestation.BundleFromDSSEEnvelope(env) + require.NoError(t, err) + + // Simulate the legacy bug: signature is stored as the ASCII bytes of the base64 string. + bundle.GetDsseEnvelope().GetSignatures()[0].Sig = []byte(encodedSig) + + attestation.FixSignatureInBundle(bundle) + assert.Equal(t, rawSig, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig()) +} From 00a120721d1be3a29bbc9fb81596455ec2a7a857 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 18 Apr 2026 12:14:22 +0200 Subject: [PATCH 2/2] refactor: apply simplify review on signature fix - Guard BundleFromDSSEEnvelope against empty DSSE Signatures slice. - Trim narrative comment on BundleBytesFromEnvelope testhelper. - Deduplicate attestations_test.go with a shared test envelope helper; add explicit no-signatures and no-op cases. Signed-off-by: Miguel Martinez Trivino --- .../pkg/biz/testhelpers/attestation.go | 5 +- pkg/attestation/attestations.go | 3 + pkg/attestation/attestations_test.go | 66 ++++++++----------- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/app/controlplane/pkg/biz/testhelpers/attestation.go b/app/controlplane/pkg/biz/testhelpers/attestation.go index 52c5a47a6..61695a6a2 100644 --- a/app/controlplane/pkg/biz/testhelpers/attestation.go +++ b/app/controlplane/pkg/biz/testhelpers/attestation.go @@ -26,9 +26,8 @@ import ( "google.golang.org/protobuf/encoding/protojson" ) -// BundleBytesFromEnvelope reads a DSSE envelope fixture from disk, wraps it into a -// Sigstore bundle, and returns its protojson-encoded bytes. Useful to feed envelope -// test fixtures into APIs that now only accept Sigstore bundles. +// BundleBytesFromEnvelope reads a DSSE envelope fixture from disk and returns the +// protojson-encoded bytes of an equivalent Sigstore bundle. func BundleBytesFromEnvelope(t *testing.T, path string) []byte { t.Helper() raw, err := os.ReadFile(path) diff --git a/pkg/attestation/attestations.go b/pkg/attestation/attestations.go index e87e26530..7cf0cd095 100644 --- a/pkg/attestation/attestations.go +++ b/pkg/attestation/attestations.go @@ -70,6 +70,9 @@ func DSSEEnvelopeFromBundle(bundle *protobundle.Bundle) *dsse.Envelope { } func BundleFromDSSEEnvelope(dsseEnvelope *dsse.Envelope) (*protobundle.Bundle, error) { + if len(dsseEnvelope.Signatures) == 0 { + return nil, fmt.Errorf("DSSE envelope has no signatures") + } // DSSE Envelope payload and signature are base64 encoded, we need to decode them so they // are not encoded twice when stored as raw bytes in the Sigstore bundle. See #1832. payload, err := base64.StdEncoding.DecodeString(dsseEnvelope.Payload) diff --git a/pkg/attestation/attestations_test.go b/pkg/attestation/attestations_test.go index 37542b141..8f332ae23 100644 --- a/pkg/attestation/attestations_test.go +++ b/pkg/attestation/attestations_test.go @@ -25,70 +25,58 @@ import ( "github.com/stretchr/testify/require" ) -// Guards against the double-base64 bug in https://github.com/chainloop-dev/chainloop/issues/1832. -func TestBundleFromDSSEEnvelopeDecodesSignature(t *testing.T) { - rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} - rawPayload := []byte(`{"_type":"statement"}`) +var ( + testRawSig = []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} + testRawPayload = []byte(`{"_type":"statement"}`) +) - env := &dsse.Envelope{ +func newTestEnvelope(t *testing.T) *dsse.Envelope { + t.Helper() + return &dsse.Envelope{ PayloadType: "application/vnd.in-toto+json", - Payload: base64.StdEncoding.EncodeToString(rawPayload), + Payload: base64.StdEncoding.EncodeToString(testRawPayload), Signatures: []dsse.Signature{ - {KeyID: "key-1", Sig: base64.StdEncoding.EncodeToString(rawSig)}, + {KeyID: "key-1", Sig: base64.StdEncoding.EncodeToString(testRawSig)}, }, } +} - bundle, err := attestation.BundleFromDSSEEnvelope(env) +// Guards against the double-base64 bug in https://github.com/chainloop-dev/chainloop/issues/1832. +func TestBundleFromDSSEEnvelopeDecodesSignature(t *testing.T) { + bundle, err := attestation.BundleFromDSSEEnvelope(newTestEnvelope(t)) require.NoError(t, err) gotEnv := bundle.GetDsseEnvelope() - assert.Equal(t, rawPayload, gotEnv.GetPayload()) + assert.Equal(t, testRawPayload, gotEnv.GetPayload()) require.Len(t, gotEnv.GetSignatures(), 1) - assert.Equal(t, rawSig, gotEnv.GetSignatures()[0].GetSig()) + assert.Equal(t, testRawSig, gotEnv.GetSignatures()[0].GetSig()) assert.Equal(t, "key-1", gotEnv.GetSignatures()[0].GetKeyid()) } -func TestBundleRoundTripWithFixedSignature(t *testing.T) { - rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} - encodedSig := base64.StdEncoding.EncodeToString(rawSig) - - env := &dsse.Envelope{ - PayloadType: "application/vnd.in-toto+json", - Payload: base64.StdEncoding.EncodeToString([]byte("payload")), - Signatures: []dsse.Signature{ - {KeyID: "key-1", Sig: encodedSig}, - }, - } +func TestBundleFromDSSEEnvelopeNoSignatures(t *testing.T) { + env := newTestEnvelope(t) + env.Signatures = nil + _, err := attestation.BundleFromDSSEEnvelope(env) + require.Error(t, err) +} - bundle, err := attestation.BundleFromDSSEEnvelope(env) +func TestFixSignatureInBundleIsNoOpOnFixedBundles(t *testing.T) { + bundle, err := attestation.BundleFromDSSEEnvelope(newTestEnvelope(t)) require.NoError(t, err) before := bundle.GetDsseEnvelope().GetSignatures()[0].GetSig() attestation.FixSignatureInBundle(bundle) - assert.Equal(t, before, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig(), - "FixSignatureInBundle should be a no-op on properly formed bundles") - - gotEnv := attestation.DSSEEnvelopeFromBundle(bundle) - assert.Equal(t, encodedSig, gotEnv.Signatures[0].Sig) + assert.Equal(t, before, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig()) } func TestFixSignatureInBundleRepairsLegacyBundles(t *testing.T) { - rawSig := []byte{0x30, 0x44, 0x02, 0x20, 0xAA, 0xBB, 0xCC, 0xDD} - encodedSig := base64.StdEncoding.EncodeToString(rawSig) - - env := &dsse.Envelope{ - PayloadType: "application/vnd.in-toto+json", - Payload: base64.StdEncoding.EncodeToString([]byte("payload")), - Signatures: []dsse.Signature{ - {KeyID: "key-1", Sig: encodedSig}, - }, - } - bundle, err := attestation.BundleFromDSSEEnvelope(env) + bundle, err := attestation.BundleFromDSSEEnvelope(newTestEnvelope(t)) require.NoError(t, err) // Simulate the legacy bug: signature is stored as the ASCII bytes of the base64 string. + encodedSig := base64.StdEncoding.EncodeToString(testRawSig) bundle.GetDsseEnvelope().GetSignatures()[0].Sig = []byte(encodedSig) attestation.FixSignatureInBundle(bundle) - assert.Equal(t, rawSig, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig()) + assert.Equal(t, testRawSig, bundle.GetDsseEnvelope().GetSignatures()[0].GetSig()) }