From 1a51cd1fd118c43995574350932b6621400e5ca4 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 19 Feb 2025 09:55:55 +0100 Subject: [PATCH 1/4] ensure dsse signatures are not encoded twice Signed-off-by: Jose I. Paris --- pkg/attestation/attestations.go | 6 +++++- pkg/attestation/verifier/verifier.go | 21 +++++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/attestation/attestations.go b/pkg/attestation/attestations.go index 4923f5e82..09e007406 100644 --- a/pkg/attestation/attestations.go +++ b/pkg/attestation/attestations.go @@ -64,6 +64,10 @@ func BundleFromDSSEEnvelope(dsseEnvelope *dsse.Envelope) (*protobundle.Bundle, e if err != nil { return nil, fmt.Errorf("decoding: %w", err) } + sig, err := base64.StdEncoding.DecodeString(dsseEnvelope.Signatures[0].Sig) + if err != nil { + return nil, fmt.Errorf("decoding: %w", err) + } return &protobundle.Bundle{ MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.3", Content: &protobundle.Bundle_DsseEnvelope{DsseEnvelope: &sigstoredsse.Envelope{ @@ -71,7 +75,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, }, }, diff --git a/pkg/attestation/verifier/verifier.go b/pkg/attestation/verifier/verifier.go index 17e690c96..2c6b51c29 100644 --- a/pkg/attestation/verifier/verifier.go +++ b/pkg/attestation/verifier/verifier.go @@ -19,6 +19,7 @@ import ( "context" "crypto/sha256" "crypto/x509" + "encoding/base64" "errors" "fmt" @@ -49,6 +50,9 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro return fmt.Errorf("invalid bundle: %w", err) } + // fix for old attestations + fixSignatureInBundle(bundle) + var signingCert *x509.Certificate if bundle.GetVerificationMaterial() == nil || bundle.GetVerificationMaterial().GetCertificate() == nil { // it's a malformed bundle (according to specs) but still verifiable @@ -76,10 +80,7 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro // Use sigstore helpers to validate and extract materials if signingCert == nil { - var sb sigstorebundle.Bundle - if err := sb.UnmarshalJSON(bundleBytes); err != nil { - return fmt.Errorf("invalid bundle: %w", err) - } + sb := &sigstorebundle.Bundle{Bundle: bundle} vc, err := sb.VerificationContent() if err != nil { @@ -110,3 +111,15 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro _, err = dsseVerifier.Verify(ctx, attestation.DSSEEnvelopeFromBundle(bundle)) return err } + +// old attestations have signatures base64 encoded twice +func fixSignatureInBundle(bundle *protobundle.Bundle) { + sig := bundle.GetDsseEnvelope().GetSignatures()[0].GetSig() + dst := make([]byte, base64.StdEncoding.EncodedLen(len(sig))) + i, err := base64.StdEncoding.Decode(dst, sig) + if err == nil { + // it was encoded twice. Use it + sig = dst[:i] + } + bundle.GetDsseEnvelope().GetSignatures()[0].Sig = sig +} From a5b7bf5525048d0c2be3c8b6249b0bdaf96ce01f Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 19 Feb 2025 10:08:53 +0100 Subject: [PATCH 2/4] fix double encoding in DSSE sig Signed-off-by: Jose I. Paris --- pkg/attestation/attestations.go | 2 +- .../{bundle_valid.json => bundle_valid_pre1832.json} | 0 pkg/attestation/verifier/verifier_test.go | 7 ++++++- 3 files changed, 7 insertions(+), 2 deletions(-) rename pkg/attestation/verifier/testdata/{bundle_valid.json => bundle_valid_pre1832.json} (100%) diff --git a/pkg/attestation/attestations.go b/pkg/attestation/attestations.go index 09e007406..499854342 100644 --- a/pkg/attestation/attestations.go +++ b/pkg/attestation/attestations.go @@ -52,7 +52,7 @@ func DSSEEnvelopeFromBundle(bundle *protobundle.Bundle) *dsse.Envelope { Signatures: []dsse.Signature{ { KeyID: sigstoreEnvelope.GetSignatures()[0].GetKeyid(), - Sig: string(sigstoreEnvelope.GetSignatures()[0].GetSig()), + Sig: base64.StdEncoding.EncodeToString(sigstoreEnvelope.GetSignatures()[0].GetSig()), }, }, } diff --git a/pkg/attestation/verifier/testdata/bundle_valid.json b/pkg/attestation/verifier/testdata/bundle_valid_pre1832.json similarity index 100% rename from pkg/attestation/verifier/testdata/bundle_valid.json rename to pkg/attestation/verifier/testdata/bundle_valid_pre1832.json diff --git a/pkg/attestation/verifier/verifier_test.go b/pkg/attestation/verifier/verifier_test.go index 075638dc2..77d1becf0 100644 --- a/pkg/attestation/verifier/verifier_test.go +++ b/pkg/attestation/verifier/verifier_test.go @@ -43,10 +43,15 @@ func TestVerifyBundle(t *testing.T) { expectErr bool }{ { - name: "invalid bundle, but still verifiable", + name: "invalid bundle according to spec, but still verifiable", roots: roots, bundle: "testdata/bundle_wrongversion.json", }, + { + name: "valid bundle, but sig encoded twice", + roots: roots, + bundle: "testdata/bundle_valid_pre1832.json", + }, { name: "valid bundle", roots: roots, From 102f1aa2305460b8e216c53706b603a230acc2c5 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 19 Feb 2025 10:36:23 +0100 Subject: [PATCH 3/4] add missing file Signed-off-by: Jose I. Paris --- pkg/attestation/verifier/testdata/bundle_valid.json | 1 + 1 file changed, 1 insertion(+) create mode 100644 pkg/attestation/verifier/testdata/bundle_valid.json diff --git a/pkg/attestation/verifier/testdata/bundle_valid.json b/pkg/attestation/verifier/testdata/bundle_valid.json new file mode 100644 index 000000000..d833cbe65 --- /dev/null +++ b/pkg/attestation/verifier/testdata/bundle_valid.json @@ -0,0 +1 @@ +{"mediaType":"application/vnd.dev.sigstore.bundle+json;version=0.3", "verificationMaterial":{"certificate":{"rawBytes":"MIIDtzCCAZ+gAwIBAgIUZRFJ5BbRFxDan1H0YXpD6pICmd8wDQYJKoZIhvcNAQELBQAwXjELMAkGA1UEBhMCRVMxEzARBgNVBAgMClNvbWUtU3RhdGUxEjAQBgNVBAoMCUNoYWlubG9vcDESMBAGA1UECwwJY2hhaW5sb29wMRIwEAYDVQQDDAljaGFpbmxvb3AwHhcNMjUwMjE5MDkwNjMzWhcNMjUwMjE5MDkxNjMzWjAvMS0wKwYDVQQKEyQxOGMzZjc4Mi00OTM2LTQ2MzAtYWI4ZC0yMGI1MTEzNjY2OTkwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQHMm9McKqL5dhuPueAhEtmrnz7+9t/pmFLABpm4K6+8+GKFxnYCPyhCJeI/0c7bNYHrWqJYYlwG8iF/g+JxyCYo2cwZTAOBgNVHQ8BAf8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUHAwMwHQYDVR0OBBYEFCxS6OLbOZJXFoKhNKBvH1zgyG1IMB8GA1UdIwQYMBaAFKDk5Tdk8eOOMf/+GDw+62HNH6XKMA0GCSqGSIb3DQEBCwUAA4ICAQAyhA4pdjh9uwQcVPWc5I+DAWaxfgmJfFCM6whUr/UNLqxaSM5Aw3WSsxS52MYCICte64GjjB8yiRgT1FWps6gv02JfibSk8DstzLhsa2yE9Uw3cW5THhEx4RKwXd2UPNAW4/D7lXNfwz+zVYrosNCuh8wSEJ5OTHaC1cYhoG4PLUi0BDY+kBcKyeVjv5+W3UEAOrT4zmso/nd2UQTRofQpLR+jg66SDcqL5nDesR1ZiwoXQJmm1Vt9MjDMXbplH3AB8xeSLImhzRgfnm+6JjTbnACnWqlKdMHs3pxfl6xgoVV9dWYlacZ1Hfde8SKXS0txnEcs00pz3PNRr9uqEGGNcKLT0Xu85zV1hncL7hW39gtGUQ/m3zuEyAVzdcuZBohuKVWdLxzjSucSFsIISI7qrWqjGyNe8AA8+2270h5tDh2TGVehkpZS4c0g3EaLNtE83GRhsBT79C0fuoUuEc9LhgfoJg8QbSy5lJRy5u309E2YMTPXyl0YWUrLrxad7w2X9NoTwaaW6oAXOoyBJL+FHYEKh/tCjJm17rvcp5GABNwbsMOkcV8AJEj2yAVcmrWt6OY9HLqQvJcIkjtCTJ+DxeHyIFSMIm07BtgWkzTJLI3FQUj918jemvudfYnT/wCVVohjhT8BJXyNLl8nvkSJsOKx8JH3fnpKRj8lTF6uZQ=="}}, "dsseEnvelope":{"payload":"eyJfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9TdGF0ZW1lbnQvdjEiLCAic3ViamVjdCI6W3sibmFtZSI6ImNoYWlubG9vcC53b3JrZmxvdy5teXdmIiwgImRpZ2VzdCI6eyJzaGEyNTYiOiJjOTRhODg0NTlhMTg1MWEyNmY5NmVhYmU3ZjRkZTQ1MDQ2ZjU1ODBjYmM1MjVjOWIzYWNlMjA1NzU1YWQ4M2VjIn19LCB7Im5hbWUiOiJnaXQuaGVhZCIsICJkaWdlc3QiOnsic2hhMSI6IjFhNTFjZDFmZDExOGM0Mzk5NTU3NDM1MDkzMmI2NjIxNDAwZTVjYTQifSwgImFubm90YXRpb25zIjp7ImF1dGhvci5lbWFpbCI6ImppcGFyaXNAY2hhaW5sb29wLmRldiIsICJhdXRob3IubmFtZSI6Ikpvc2UgSS4gUGFyaXMiLCAiZGF0ZSI6IjIwMjUtMDItMTlUMDg6NTU6NTVaIiwgIm1lc3NhZ2UiOiJlbnN1cmUgZHNzZSBzaWduYXR1cmVzIGFyZSBub3QgZW5jb2RlZCB0d2ljZVxuXG5TaWduZWQtb2ZmLWJ5OiBKb3NlIEkuIFBhcmlzIDxqaXBhcmlzQGNoYWlubG9vcC5kZXY+XG4iLCAicmVtb3RlcyI6W3sibmFtZSI6Im9yaWdpbiIsICJ1cmwiOiJnaXRAZ2l0aHViLmNvbTpqaXBhcmlzL2NoYWlubG9vcC5naXQifSwgeyJuYW1lIjoidXBzdHJlYW0iLCAidXJsIjoiZ2l0QGdpdGh1Yi5jb206Y2hhaW5sb29wLWRldi9jaGFpbmxvb3AuZ2l0In0sIHsibmFtZSI6Im1pZ3VlIiwgInVybCI6Imh0dHBzOi8vZ2l0aHViLmNvbS9taWdtYXJ0cmkvY2hhaW5sb29wLyJ9LCB7Im5hbWUiOiJqYXZpIiwgInVybCI6Imh0dHBzOi8vZ2l0aHViLmNvbS9qYXZpcmxuL2NoYWlubG9vcCJ9XSwgInNpZ25hdHVyZSI6Ii0tLS0tQkVHSU4gUEdQIFNJR05BVFVSRS0tLS0tXG5cbmlRSXpCQUFCQ0FBZEZpRUVqSDUxSE1Xa0NOWTJkaFp3b0l0eUNrK20zdDRGQW1lMW5LZ0FDZ2tRb0l0eUNrK21cbjN0NkRpZy8rS3Mvb1BTdEwrWmd2ckpoTzFzUG4zSHZGSjMwY1MvTzBLSnNIVy81VlJPVTk0TjJ4MCtJb0hPc0lcbng1cW5oQ3MycnVxNmQyWmxhRTJWamhibHBCZ1k3VFFKOWxTUG1tTU5memswa2pwOHQ0UWxicTZCL0JhZmh5eURcbmVhVDQ2TW5CWnMrMTVXOE4xRjVRVjNXWlptOWY5Nm5DZEVzL2owL1RYby9jb1NHUS96MDdWOUdKdWJOUFMzQjFcblg3WnV4RXBxMzZOaExjaXVVQng2R2kzaGdJVjlKQ3F3Q2RQRzJicmwyK0E0WFgwdGhQUG92ZXlSYjRtcmx6WEpcbmsva1FrSFAySWV1SENodng3VloyM0ZPZS90dHRDazg3SnYvQUxuR0gwWHZISFdOV1FoR25jSlhZcmtwc2xOcFhcbndaaWg2VkEwVW90bVhPVlBVSy9PV1RtTnRKUS9jcmlCQXkyd09FRmxqN1Q3NzgzN05nL2pZWGhJb1ZBeHRjV1ZcbkxrcFRCWDlNOTM5VXJFQU1qYkhmUVB5L3F3Q3MyZG9SZ3RHSGtoNHIwc1o1Y0ZEK3JPdTI3L0s0ZXFMU3czWTdcbmFIQkt4N0IvV1A3b3IvUFF1SUh3SUxuMENZczZWVSt0bkZrTkZvUzBRdkhuaHZnRUJNZTlnUTI3bUcyMmpVajlcbnIzVWZab3RlWVUzMUpwNzNEbjMrdVFlTUxDUlpBeURyYUVUeDdKbjlESHZ2K0Fxd1pBWlFzWDNzdHk1OURMeGJcbi9ubisyaWhpYTdLSUZ3NllnaVRqZGU5L3gzYmlzb09NYndPU0lwbFNuakd6UDM0ZTlGU1g1V3NCQWZsTEVMbk5cbmRHNmNxK3RkejlydE9vM3QrbUVyTk8xTWV3MjgwdmNRRVh5U0tNMGZ4QTJZcWJKeS9scz1cbj1ZWEFoXG4tLS0tLUVORCBQR1AgU0lHTkFUVVJFLS0tLS1cbiJ9fV0sICJwcmVkaWNhdGVUeXBlIjoiY2hhaW5sb29wLmRldi9hdHRlc3RhdGlvbi92MC4yIiwgInByZWRpY2F0ZSI6eyJidWlsZFR5cGUiOiJjaGFpbmxvb3AuZGV2L3dvcmtmbG93cnVuL3YwLjEiLCAiYnVpbGRlciI6eyJpZCI6ImNoYWlubG9vcC5kZXYvY2xpL2RldkBzaGEyNTY6ZDgxOTg4MGIzNTYyYzBmMzI2NzRhNDRmODBmZjFiNDQyZjIyYzA2YWEwY2EyNjA5MTVjYzMwZWQ4YzVhNjVkZiJ9LCAibWV0YWRhdGEiOnsiY29udHJhY3ROYW1lIjoibXlwcm9qZWN0LW15d2YiLCAiY29udHJhY3RWZXJzaW9uIjoiOTUiLCAiZmluaXNoZWRBdCI6IjIwMjUtMDItMTlUMDk6MDY6MzMuNTY0NjA5WiIsICJpbml0aWFsaXplZEF0IjoiMjAyNS0wMi0xOVQwOTowNjoyMS44MzQ1NTNaIiwgIm5hbWUiOiJteXdmIiwgIm9yZ2FuaXphdGlvbiI6Im15LW9yZyIsICJwcm9qZWN0IjoibXlwcm9qZWN0IiwgInByb2plY3RWZXJzaW9uIjoidjAuMTY3LjAiLCAicHJvamVjdFZlcnNpb25QcmVyZWxlYXNlIjp0cnVlLCAidGVhbSI6IiIsICJ3b3JrZmxvd0lEIjoiOTVmMWI1OTctMjBlMS00OThjLWI5MzEtZmM2ZDczMzE4NjBlIiwgIndvcmtmbG93UnVuSUQiOiJmM2YzMzllYS1hNzJhLTQwYzAtYjlmMS0wMzVmNDBlOWNjNGIifSwgInBvbGljeUF0dEJsb2NrZWQiOmZhbHNlLCAicG9saWN5QmxvY2tCeXBhc3NFbmFibGVkIjpmYWxzZSwgInBvbGljeUNoZWNrQmxvY2tpbmdTdHJhdGVneSI6IkFEVklTT1JZIiwgInBvbGljeUV2YWx1YXRpb25zIjp7IkNIQUlOTE9PUC5BVFRFU1RBVElPTiI6W3siZGVzY3JpcHRpb24iOiJWZXJpZmllcyB0aGF0IHRoZSBhdHRlc3RhdGlvbiBleHBsaWNpdGx5IHJlZmVyZW5jZXMgYSBzcGVjaWZpYyBHaXQgY29tbWl0IiwgIm5hbWUiOiJzb3VyY2UtY29tbWl0IiwgInBvbGljeVJlZmVyZW5jZSI6eyJhbm5vdGF0aW9ucyI6eyJuYW1lIjoic291cmNlLWNvbW1pdCIsICJvcmdhbml6YXRpb24iOiIifSwgImRpZ2VzdCI6eyJzaGEyNTYiOiIxNjRiYzMyMmJmMzUwZDZiYjQ1NzY4Yzg2ZGIxODBmNTMxNTA4NWM3OTFhZGViYWYwOTQ5MGY5Y2NlOTk1MjhjIn0sICJuYW1lIjoic291cmNlLWNvbW1pdCIsICJ1cmkiOiJjaGFpbmxvb3A6Ly9sb2NhbGhvc3Q6ODAwMi9zb3VyY2UtY29tbWl0In0sICJyZXF1aXJlbWVudHMiOlsiY2hhaW5sb29wLWJlc3QtcHJhY3RpY2VzL2NvbW1pdC1zaWduZWQiXSwgInNraXBwZWQiOmZhbHNlLCAidHlwZSI6IkFUVEVTVEFUSU9OIiwgIndpdGgiOnsiY2hlY2tfc2lnbmF0dXJlIjoidHJ1ZSJ9fSwgeyJhbm5vdGF0aW9ucyI6eyJjYXRlZ29yeSI6InNib20ifSwgImRlc2NyaXB0aW9uIjoiQ2hlY2tzIHRoYXQgZWl0aGVyIGEgU1BEWCBvciBDeWNsb25lRFggU0JPTSBtYXRlcmlhbCBpcyBwcmVzZW50IGluIHRoZSBhdHRlc3RhdGlvbiIsICJncm91cFJlZmVyZW5jZSI6eyJhbm5vdGF0aW9ucyI6eyJuYW1lIjoic2JvbS1xdWFsaXR5IiwgIm9yZ2FuaXphdGlvbiI6IiJ9LCAiZGlnZXN0Ijp7InNoYTI1NiI6IjcwMzdjZDZlOTRhZjVjYjgwMDJkZmNlZmM3MmViOGQwMzIzZGYwNmM1MjZjNDE4ZDMxNWYwZGFhY2M5ZGNjMjYifSwgIm5hbWUiOiJzYm9tLXF1YWxpdHkiLCAidXJpIjoiY2hhaW5sb29wOi8vbG9jYWxob3N0OjgwMDIvc2JvbS1xdWFsaXR5In0sICJuYW1lIjoic2JvbS1wcmVzZW50IiwgInBvbGljeVJlZmVyZW5jZSI6eyJhbm5vdGF0aW9ucyI6eyJuYW1lIjoic2JvbS1wcmVzZW50IiwgIm9yZ2FuaXphdGlvbiI6IiJ9LCAiZGlnZXN0Ijp7InNoYTI1NiI6ImUyZGEwN2M0ZGFiNzZhNThiNDc0YzY4YTNjNWE1NDMzYmY0YTI4ZWQ4ODFkNGNlYWExNmU5MTcxNjE2ODRlYTIifSwgIm5hbWUiOiJzYm9tLXByZXNlbnQiLCAidXJpIjoiY2hhaW5sb29wOi8vbG9jYWxob3N0OjgwMDIvc2JvbS1wcmVzZW50In0sICJyZXF1aXJlbWVudHMiOlsiY2hhaW5sb29wLWJlc3QtcHJhY3RpY2VzL3Nib20tcXVhbGl0eSJdLCAic2tpcHBlZCI6ZmFsc2UsICJ0eXBlIjoiQVRURVNUQVRJT04iLCAidmlvbGF0aW9ucyI6W3sibWVzc2FnZSI6Im1pc3NpbmcgU0JPTSBtYXRlcmlhbCIsICJzdWJqZWN0Ijoic2JvbS1wcmVzZW50In1dfV19LCAicG9saWN5SGFzVmlvbGF0aW9ucyI6dHJ1ZSwgInJ1bm5lclR5cGUiOiJSVU5ORVJfVFlQRV9VTlNQRUNJRklFRCJ9fQ==", "payloadType":"application/vnd.in-toto+json", "signatures":[{"sig":"MEQCIGUPEQZO3dWg3JLUSEoS99ULmVUtalVB5ekZLAmPPn2DAiB/rnGB/sMqVENkJ0hKD8tCLaTKwsY9L8HxoE9C/ApZkw=="}]}} \ No newline at end of file From 8849f5fa8a35b58898b17aef4e03e9599bfe20a9 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 19 Feb 2025 10:38:44 +0100 Subject: [PATCH 4/4] add comment Signed-off-by: Jose I. Paris --- pkg/attestation/verifier/verifier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/attestation/verifier/verifier.go b/pkg/attestation/verifier/verifier.go index 2c6b51c29..954be3bbd 100644 --- a/pkg/attestation/verifier/verifier.go +++ b/pkg/attestation/verifier/verifier.go @@ -112,7 +112,7 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro return err } -// old attestations have signatures base64 encoded twice +// old attestations have signatures base64 encoded twice, see https://github.com/chainloop-dev/chainloop/issues/1832 func fixSignatureInBundle(bundle *protobundle.Bundle) { sig := bundle.GetDsseEnvelope().GetSignatures()[0].GetSig() dst := make([]byte, base64.StdEncoding.EncodedLen(len(sig)))