Repository: sigstore/rekor
Pull request #1150 (2022-10-25 14:03:25)

remove double encoding of payload and signature fields for intoto v0.0.2
The intoto v0.0.2 type was defined with a hint to the code generation tool used in `rekor-cli` & `rekor-server` that indicated that these strings were base64 encoded (this caused the code generated to handle encoding and decoding to/from `[]byte` slices). However, `CreateArtifactFromProps` unnecessarily double encoded uploads to Rekor (that originated from `rekor-cli` or any caller of `CreateArtifactFromProps`).

This patch removes that hint to the code generation tool, leaving the field to be handled internally as a `string` rather than `strfmt.Base64`, while still supporting either single or double encoded entries to provide backwards compatibility for released clients. 

Fixes: #1139 

Signed-off-by: Bob Callaway <bcallaway@google.com>


Patch:
diff --git a/go.mod b/go.mod
index 7236e75..b16f7ee 100644
--- a/go.mod
+++ b/go.mod
@@ -31,7 +31,7 @@ require (
 	github.com/sassoftware/relic v0.0.0-20210427151427-dfb082b79b74
 	github.com/secure-systems-lab/go-securesystemslib v0.4.0
 	github.com/sigstore/sigstore v1.4.5
-	github.com/spf13/cobra v1.6.0
+	github.com/spf13/cobra v1.6.1
 	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.13.0
 	github.com/tent/canonical-json-go v0.0.0-20130607151641-96e4ba3a7613
@@ -123,7 +123,7 @@ require (
 	github.com/x448/float16 v0.8.4 // indirect
 	go.mongodb.org/mongo-driver v1.10.0 // indirect
 	go.opencensus.io v0.23.0 // indirect
-	go.step.sm/crypto v0.21.0
+	go.step.sm/crypto v0.22.0
 	go.uber.org/atomic v1.10.0 // indirect
 	go.uber.org/multierr v1.8.0 // indirect
 	golang.org/x/oauth2 v0.1.0 // indirect
diff --git a/go.sum b/go.sum
index e45d52a..268befc 100644
--- a/go.sum
+++ b/go.sum
@@ -742,8 +742,8 @@ github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w=
 github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU=
 github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ=
 github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
-github.com/spf13/cobra v1.6.0 h1:42a0n6jwCot1pUmomAp4T7DeMD+20LFv4Q54pxLf2LI=
-github.com/spf13/cobra v1.6.0/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY=
+github.com/spf13/cobra v1.6.1 h1:o94oiPyS4KD1mPy2fmcYYHHfCxLqYjJOhGsCHFZtEzA=
+github.com/spf13/cobra v1.6.1/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY=
 github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
 github.com/spf13/jwalterweatherman v1.1.0 h1:ue6voC5bR5F8YxI5S67j9i582FU4Qvo2bmqnqMYADFk=
 github.com/spf13/jwalterweatherman v1.1.0/go.mod h1:aNWZUN0dPAAO/Ljvb5BEdw96iTZ0EXowPYD95IqWIGo=
@@ -831,8 +831,8 @@ go.opencensus.io v0.22.5/go.mod h1:5pWMHQbX5EPX2/62yrJeAkowc+lfs/XD7Uxpq3pI6kk=
 go.opencensus.io v0.23.0 h1:gqCw0LfLxScz8irSi8exQc7fyQ0fKQU/qnC/X8+V/1M=
 go.opencensus.io v0.23.0/go.mod h1:XItmlyltB5F7CS4xOC1DcqMoFqwtC6OG2xF7mCv7P7E=
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
-go.step.sm/crypto v0.21.0 h1:Qwxk5JrqG0Q1t8tOfDM3zKTNECG6J5J24qgWZCRM0Ic=
-go.step.sm/crypto v0.21.0/go.mod h1:diT2XWIHQy0397UI3i78qCKeLLLp2wu0/DIJI66u/MU=
+go.step.sm/crypto v0.22.0 h1:aloAQsNYEX87e4xL+jqsd6zCA4mGThPBmz2akMKss8Q=
+go.step.sm/crypto v0.22.0/go.mod h1:7PTDUApWUTmQbDt4x82cbk3nx5k7JwBo2dFG9CGEp9A=
 go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
 go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
diff --git a/pkg/api/entries.go b/pkg/api/entries.go
index 81a00af..f8b207a 100644
--- a/pkg/api/entries.go
+++ b/pkg/api/entries.go
@@ -237,12 +237,12 @@ func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middl
 		go func() {
 			keys, err := entry.IndexKeys()
 			if err != nil {
-				log.ContextLogger(ctx).Error(err)
+				log.ContextLogger(ctx).Error(fmt.Errorf("error generating index keys: %w", err))
 				return
 			}
 			for _, key := range keys {
 				if err := addToIndex(context.Background(), key, entryID); err != nil {
-					log.ContextLogger(ctx).Error(err)
+					log.ContextLogger(ctx).Error(fmt.Errorf("error inserting key/value pairs into index: %w", err))
 				}
 			}
 		}()
diff --git a/pkg/generated/models/intoto_v002_schema.go b/pkg/generated/models/intoto_v002_schema.go
index 86c0b47..bdd7b3b 100644
--- a/pkg/generated/models/intoto_v002_schema.go
+++ b/pkg/generated/models/intoto_v002_schema.go
@@ -313,9 +313,9 @@ func (m *IntotoV002SchemaContent) UnmarshalBinary(b []byte) error {
 // swagger:model IntotoV002SchemaContentEnvelope
 type IntotoV002SchemaContentEnvelope struct {
 
-	// payload of the envelope
-	// Format: byte
-	Payload strfmt.Base64 `json:"payload,omitempty"`
+	// base64 encoded payload of the envelope
+	// Pattern: ^(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=|[A-Za-z0-9+\/]{4})$
+	Payload string `json:"payload,omitempty"`
 
 	// type describing the payload
 	// Required: true
@@ -331,6 +331,10 @@ type IntotoV002SchemaContentEnvelope struct {
 func (m *IntotoV002SchemaContentEnvelope) Validate(formats strfmt.Registry) error {
 	var res []error
 
+	if err := m.validatePayload(formats); err != nil {
+		res = append(res, err)
+	}
+
 	if err := m.validatePayloadType(formats); err != nil {
 		res = append(res, err)
 	}
@@ -345,6 +349,18 @@ func (m *IntotoV002SchemaContentEnvelope) Validate(formats strfmt.Registry) erro
 	return nil
 }
 
+func (m *IntotoV002SchemaContentEnvelope) validatePayload(formats strfmt.Registry) error {
+	if swag.IsZero(m.Payload) { // not required
+		return nil
+	}
+
+	if err := validate.Pattern("content"+"."+"envelope"+"."+"payload", "body", m.Payload, `^(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=|[A-Za-z0-9+\/]{4})$`); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (m *IntotoV002SchemaContentEnvelope) validatePayloadType(formats strfmt.Registry) error {
 
 	if err := validate.Required("content"+"."+"envelope"+"."+"payloadType", "body", m.PayloadType); err != nil {
@@ -452,13 +468,34 @@ type IntotoV002SchemaContentEnvelopeSignaturesItems0 struct {
 	// Format: byte
 	PublicKey strfmt.Base64 `json:"publicKey,omitempty"`
 
-	// signature of the payload
-	// Format: byte
-	Sig strfmt.Base64 `json:"sig,omitempty"`
+	// base64 encoded signature of the payload
+	// Pattern: ^(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=|[A-Za-z0-9+\/]{4})$
+	Sig string `json:"sig,omitempty"`
 }
 
 // Validate validates this intoto v002 schema content envelope signatures items0
 func (m *IntotoV002SchemaContentEnvelopeSignaturesItems0) Validate(formats strfmt.Registry) error {
+	var res []error
+
+	if err := m.validateSig(formats); err != nil {
+		res = append(res, err)
+	}
+
+	if len(res) > 0 {
+		return errors.CompositeValidationError(res...)
+	}
+	return nil
+}
+
+func (m *IntotoV002SchemaContentEnvelopeSignaturesItems0) validateSig(formats strfmt.Registry) error {
+	if swag.IsZero(m.Sig) { // not required
+		return nil
+	}
+
+	if err := validate.Pattern("sig", "body", m.Sig, `^(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=|[A-Za-z0-9+\/]{4})$`); err != nil {
+		return err
+	}
+
 	return nil
 }
 
diff --git a/pkg/generated/restapi/embedded_spec.go b/pkg/generated/restapi/embedded_spec.go
index 697bc22..73e606d 100644
--- a/pkg/generated/restapi/embedded_spec.go
+++ b/pkg/generated/restapi/embedded_spec.go
@@ -1935,9 +1935,9 @@ func init() {
           ],
           "properties": {
             "payload": {
-              "description": "payload of the envelope",
+              "description": "base64 encoded payload of the envelope",
               "type": "string",
-              "format": "byte",
+              "pattern": "^(?:[A-Za-z0-9+\\/]{4})*(?:[A-Za-z0-9+\\/]{2}==|[A-Za-z0-9+\\/]{3}=|[A-Za-z0-9+\\/]{4})$",
               "writeOnly": true
             },
             "payloadType": {
@@ -2009,9 +2009,9 @@ func init() {
       ],
       "properties": {
         "payload": {
-          "description": "payload of the envelope",
+          "description": "base64 encoded payload of the envelope",
           "type": "string",
-          "format": "byte",
+          "pattern": "^(?:[A-Za-z0-9+\\/]{4})*(?:[A-Za-z0-9+\\/]{2}==|[A-Za-z0-9+\\/]{3}=|[A-Za-z0-9+\\/]{4})$",
           "writeOnly": true
         },
         "payloadType": {
@@ -2043,9 +2043,9 @@ func init() {
           "readOnly": true
         },
         "sig": {
-          "description": "signature of the payload",
+          "description": "base64 encoded signature of the payload",
           "type": "string",
-          "format": "byte"
+          "pattern": "^(?:[A-Za-z0-9+\\/]{4})*(?:[A-Za-z0-9+\\/]{2}==|[A-Za-z0-9+\\/]{3}=|[A-Za-z0-9+\\/]{4})$"
         }
       }
     },
@@ -3286,9 +3286,9 @@ func init() {
               ],
               "properties": {
                 "payload": {
-                  "description": "payload of the envelope",
+                  "description": "base64 encoded payload of the envelope",
                   "type": "string",
-                  "format": "byte",
+                  "pattern": "^(?:[A-Za-z0-9+\\/]{4})*(?:[A-Za-z0-9+\\/]{2}==|[A-Za-z0-9+\\/]{3}=|[A-Za-z0-9+\\/]{4})$",
                   "writeOnly": true
                 },
                 "payloadType": {
diff --git a/pkg/types/intoto/v0.0.2/entry.go b/pkg/types/intoto/v0.0.2/entry.go
index 6b7a62a..4e91d0c 100644
--- a/pkg/types/intoto/v0.0.2/entry.go
+++ b/pkg/types/intoto/v0.0.2/entry.go
@@ -84,7 +84,7 @@ func (v V002Entry) IndexKeys() ([]string, error) {
 
 		canonKey, err := keyObj.CanonicalValue()
 		if err != nil {
-			return result, fmt.Errorf("could not canonicize key: %w", err)
+			return result, fmt.Errorf("could not canonicalize key: %w", err)
 		}
 
 		keyHash := sha256.Sum256(canonKey)
@@ -102,7 +102,7 @@ func (v V002Entry) IndexKeys() ([]string, error) {
 	switch *v.IntotoObj.Content.Envelope.PayloadType {
 	case in_toto.PayloadType:
 
-		if v.IntotoObj.Content.Envelope.Payload == nil {
+		if v.IntotoObj.Content.Envelope.Payload == "" {
 			log.Logger.Info("IntotoObj DSSE payload is empty")
 			return result, nil
 		}
@@ -159,7 +159,6 @@ func (v *V002Entry) Unmarshal(pe models.ProposedEntry) error {
 		return errors.New("cannot unmarshal non Intoto v0.0.2 type")
 	}
 
-	var err error
 	if err := types.DecodeEntry(it.Spec, &v.IntotoObj); err != nil {
 		return err
 	}
@@ -173,17 +172,44 @@ func (v *V002Entry) Unmarshal(pe models.ProposedEntry) error {
 		return nil
 	}
 
+	payloadToUse := v.IntotoObj.Content.Envelope.Payload
+	// we need the decoded payload to calculate the correct SHA256 digest
+	decodedPayload, err := base64.StdEncoding.DecodeString(payloadToUse)
+	if err != nil {
+		return fmt.Errorf("envelope payload must be base64 encoded string: %w", err)
+	}
+	// in rekor v0.12 & v1.0.0, rekor-cli would (incorrectly) double encode the payload.
+	// handle that gracefully here
+	doubleDecodedPayload, err := base64.StdEncoding.DecodeString(string(decodedPayload))
+	if err == nil {
+		payloadToUse = string(decodedPayload)
+		v.IntotoObj.Content.Envelope.Payload = payloadToUse
+		decodedPayload = doubleDecodedPayload
+	}
+
 	env := &dsse.Envelope{
-		Payload:     string(v.IntotoObj.Content.Envelope.Payload),
+		Payload:     payloadToUse, // must be base64 encoded string
 		PayloadType: *v.IntotoObj.Content.Envelope.PayloadType,
 	}
 
 	allPubKeyBytes := make([][]byte, 0)
 	for _, sig := range v.IntotoObj.Content.Envelope.Signatures {
+		sigToUse := sig.Sig
+		decodedSig, err := base64.StdEncoding.DecodeString(string(sig.Sig))
+		if err != nil {
+			return fmt.Errorf("envelope signature must be a base64 encoded string: %w", err)
+		}
+		// in rekor v0.12 & v1.0.0, rekor-cli would (incorrectly) double encode the signature.
+		// handle that gracefully here
+		if _, err = base64.StdEncoding.DecodeString(string(decodedSig)); err == nil {
+			sigToUse = string(decodedSig)
+		}
+
 		env.Signatures = append(env.Signatures, dsse.Signature{
 			KeyID: sig.Keyid,
-			Sig:   string(sig.Sig),
+			Sig:   sigToUse, // must be base64 encoded string
 		})
+		sig.Sig = sigToUse // overwrite value in local object to ensure we canonicalize correctly
 
 		allPubKeyBytes = append(allPubKeyBytes, sig.PublicKey)
 	}
@@ -194,11 +220,6 @@ func (v *V002Entry) Unmarshal(pe models.ProposedEntry) error {
 
 	v.env = *env
 
-	decodedPayload, err := base64.StdEncoding.DecodeString(string(v.IntotoObj.Content.Envelope.Payload))
-	if err != nil {
-		return fmt.Errorf("could not decode envelope payload: %w", err)
-	}
-
 	h := sha256.Sum256(decodedPayload)
 	v.IntotoObj.Content.PayloadHash = &models.IntotoV002SchemaContentPayloadHash{
 		Algorithm: swag.String(models.IntotoV002SchemaContentPayloadHashAlgorithmSha256),
@@ -339,8 +360,7 @@ func (v V002Entry) CreateFromArtifactProperties(_ context.Context, props types.A
 		return nil, err
 	}
 
-	b64 := strfmt.Base64([]byte(env.Payload))
-	re.IntotoObj.Content.Envelope.Payload = b64
+	re.IntotoObj.Content.Envelope.Payload = env.Payload
 	re.IntotoObj.Content.Envelope.PayloadType = &env.PayloadType
 
 	for _, sig := range env.Signatures {
@@ -351,13 +371,13 @@ func (v V002Entry) CreateFromArtifactProperties(_ context.Context, props types.A
 
 		canonKey, err := key.CanonicalValue()
 		if err != nil {
-			return nil, fmt.Errorf("could not canonicize key: %w", err)
+			return nil, fmt.Errorf("could not canonicalize key: %w", err)
 		}
 
 		keyBytes := strfmt.Base64(canonKey)
 		re.IntotoObj.Content.Envelope.Signatures = append(re.IntotoObj.Content.Envelope.Signatures, &models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
 			Keyid:     sig.KeyID,
-			Sig:       strfmt.Base64([]byte(sig.Sig)),
+			Sig:       sig.Sig,
 			PublicKey: keyBytes,
 		})
 	}
diff --git a/pkg/types/intoto/v0.0.2/intoto_v0_0_2_schema.json b/pkg/types/intoto/v0.0.2/intoto_v0_0_2_schema.json
index 0008e46..a18f2b5 100644
--- a/pkg/types/intoto/v0.0.2/intoto_v0_0_2_schema.json
+++ b/pkg/types/intoto/v0.0.2/intoto_v0_0_2_schema.json
@@ -13,9 +13,9 @@
                     "type": "object",
                     "properties": {
                         "payload": {
-                            "description": "payload of the envelope",
+                            "description": "base64 encoded payload of the envelope",
                             "type": "string",
-                            "format": "byte",
+                            "pattern": "^(?:[A-Za-z0-9+\\/]{4})*(?:[A-Za-z0-9+\\/]{2}==|[A-Za-z0-9+\\/]{3}=|[A-Za-z0-9+\\/]{4})$",
                             "writeOnly": true
                         },
                         "payloadType": {
@@ -35,9 +35,9 @@
                                         "type": "string"
                                     },
                                     "sig": {
-                                        "description": "signature of the payload",
+                                        "description": "base64 encoded signature of the payload",
                                         "type": "string",
-                                        "format": "byte"
+                                        "pattern": "^(?:[A-Za-z0-9+\\/]{4})*(?:[A-Za-z0-9+\\/]{2}==|[A-Za-z0-9+\\/]{3}=|[A-Za-z0-9+\\/]{4})$"
                                     },
                                     "publicKey": {
                                         "description": "public key that corresponds to this signature",


Test patch:
diff --git a/pkg/types/intoto/v0.0.2/entry_test.go b/pkg/types/intoto/v0.0.2/entry_test.go
index 66bd21a..8270000 100644
--- a/pkg/types/intoto/v0.0.2/entry_test.go
+++ b/pkg/types/intoto/v0.0.2/entry_test.go
@@ -108,14 +108,13 @@ func multiSignEnvelope(t *testing.T, k []*ecdsa.PrivateKey, payload []byte) *dss
 func createRekorEnvelope(dsseEnv *dsse.Envelope, pub [][]byte) *models.IntotoV002SchemaContentEnvelope {
 
 	env := &models.IntotoV002SchemaContentEnvelope{}
-	b64 := strfmt.Base64([]byte(dsseEnv.Payload))
-	env.Payload = b64
+	env.Payload = dsseEnv.Payload
 	env.PayloadType = &dsseEnv.PayloadType
 
 	for i, sig := range dsseEnv.Signatures {
 		env.Signatures = append(env.Signatures, &models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
 			Keyid:     sig.KeyID,
-			Sig:       strfmt.Base64([]byte(sig.Sig)),
+			Sig:       sig.Sig,
 			PublicKey: strfmt.Base64(pub[i]),
 		})
 	}
@@ -173,6 +172,12 @@ func TestV002Entry_Unmarshal(t *testing.T) {
 
 	validPayload := "hellothispayloadisvalid"
 
+	doubleEncodedEnvelope := createRekorEnvelope(envelope(t, key, []byte(validPayload)), [][]byte{pub})
+	doubleEncodedPayload := base64.StdEncoding.EncodeToString([]byte(doubleEncodedEnvelope.Payload))
+	doubleEncodedEnvelope.Payload = doubleEncodedPayload
+	doubleEncodedSig := base64.StdEncoding.EncodeToString([]byte(doubleEncodedEnvelope.Signatures[0].Sig))
+	doubleEncodedEnvelope.Signatures[0].Sig = doubleEncodedSig
+
 	tests := []struct {
 		env     *dsse.Envelope
 		name    string
@@ -265,6 +270,20 @@ func TestV002Entry_Unmarshal(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			env:  envelope(t, key, []byte(validPayload)),
+			name: "double base64 encoded envelope (testing backwards compat with v0.12.x and v1.0.0",
+			it: &models.IntotoV002Schema{
+				Content: &models.IntotoV002SchemaContent{
+					Envelope: doubleEncodedEnvelope,
+					Hash: &models.IntotoV002SchemaContentHash{
+						Algorithm: swag.String(models.IntotoV002SchemaContentHashAlgorithmSha256),
+						Value:     swag.String(envelopeHash(t, envelope(t, priv, []byte(validPayload)))),
+					},
+				},
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {