Skip to content

Commit

Permalink
Merge pull request from GHSA-frqx-jfcm-6jjr
Browse files Browse the repository at this point in the history
* fix for GHSA-frqx-jfcm-6jjr

Signed-off-by: Bob Callaway <[email protected]>

* add defensive check

Signed-off-by: Bob Callaway <[email protected]>

* add defensive check to dsse type

Signed-off-by: Bob Callaway <[email protected]>

* be stricter for DSSE, add another defensive check

Signed-off-by: Bob Callaway <[email protected]>

---------

Signed-off-by: Bob Callaway <[email protected]>
  • Loading branch information
bobcallaway authored May 26, 2023
1 parent 85bb2bc commit 140c5ad
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 26 deletions.
37 changes: 35 additions & 2 deletions pkg/generated/models/intoto_v002_schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/generated/restapi/embedded_spec.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/types/dsse/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ func (v V001Entry) IndexKeys() ([]string, error) {
var result []string

for _, sig := range v.DSSEObj.Signatures {
if sig == nil || sig.Verifier == nil {
return result, errors.New("missing or malformed public key")
}
keyObj, err := x509.NewPublicKey(bytes.NewReader(*sig.Verifier))
if err != nil {
return result, err
Expand Down Expand Up @@ -202,6 +205,10 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {

allPubKeyBytes := make([][]byte, 0)
for _, publicKey := range dsseObj.ProposedContent.Verifiers {
if publicKey == nil {
return errors.New("an invalid null verifier was provided in ProposedContent")
}

allPubKeyBytes = append(allPubKeyBytes, publicKey)
}

Expand Down
14 changes: 14 additions & 0 deletions pkg/types/dsse/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ func TestV001Entry_Unmarshal(t *testing.T) {
},
}

validEnv := envelope(t, key, []byte("payload"))
validEnvBytes, _ := json.Marshal(validEnv)

validPayload := "hellothispayloadisvalid"

tests := []struct {
Expand Down Expand Up @@ -218,6 +221,17 @@ func TestV001Entry_Unmarshal(t *testing.T) {
},
wantErr: false,
},
{
env: validEnv,
name: "null verifier in array",
it: &models.DSSEV001Schema{
ProposedContent: &models.DSSEV001SchemaProposedContent{
Envelope: swag.String(string(validEnvBytes)),
Verifiers: []strfmt.Base64{pub, nil},
},
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
30 changes: 21 additions & 9 deletions pkg/types/intoto/v0.0.2/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/in-toto/in-toto-golang/in_toto"
"github.com/secure-systems-lab/go-securesystemslib/dsse"
"github.com/spf13/viper"
"golang.org/x/exp/slices"

"github.com/go-openapi/strfmt"
"github.com/go-openapi/swag"
Expand Down Expand Up @@ -78,7 +79,10 @@ func (v V002Entry) IndexKeys() ([]string, error) {
}

for _, sig := range v.IntotoObj.Content.Envelope.Signatures {
keyObj, err := x509.NewPublicKey(bytes.NewReader(sig.PublicKey))
if sig == nil || sig.PublicKey == nil {
return result, errors.New("malformed or missing signature")
}
keyObj, err := x509.NewPublicKey(bytes.NewReader(*sig.PublicKey))
if err != nil {
return result, err
}
Expand Down Expand Up @@ -182,13 +186,17 @@ func (v *V002Entry) Unmarshal(pe models.ProposedEntry) error {
}

allPubKeyBytes := make([][]byte, 0)
for _, sig := range v.IntotoObj.Content.Envelope.Signatures {
for i, sig := range v.IntotoObj.Content.Envelope.Signatures {
if sig == nil {
v.IntotoObj.Content.Envelope.Signatures = slices.Delete(v.IntotoObj.Content.Envelope.Signatures, i, i)
continue
}
env.Signatures = append(env.Signatures, dsse.Signature{
KeyID: sig.Keyid,
Sig: string(sig.Sig),
Sig: string(*sig.Sig),
})

allPubKeyBytes = append(allPubKeyBytes, sig.PublicKey)
allPubKeyBytes = append(allPubKeyBytes, *sig.PublicKey)
}

if _, err := verifyEnvelope(allPubKeyBytes, env); err != nil {
Expand Down Expand Up @@ -381,10 +389,11 @@ func (v V002Entry) CreateFromArtifactProperties(_ context.Context, props types.A
}

keyBytes := strfmt.Base64(canonKey)
sigBytes := strfmt.Base64([]byte(sig.Sig))
re.IntotoObj.Content.Envelope.Signatures = append(re.IntotoObj.Content.Envelope.Signatures, &models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
Keyid: sig.KeyID,
Sig: strfmt.Base64([]byte(sig.Sig)),
PublicKey: keyBytes,
Sig: &sigBytes,
PublicKey: &keyBytes,
})
}

Expand Down Expand Up @@ -458,7 +467,7 @@ func (v V002Entry) Verifier() (pki.PublicKey, error) {
return nil, errors.New("no signatures found on intoto entry")
}

return x509.NewPublicKey(bytes.NewReader(v.IntotoObj.Content.Envelope.Signatures[0].PublicKey))
return x509.NewPublicKey(bytes.NewReader(*v.IntotoObj.Content.Envelope.Signatures[0].PublicKey))
}

func (v V002Entry) Insertable() (bool, error) {
Expand All @@ -480,10 +489,13 @@ func (v V002Entry) Insertable() (bool, error) {
return false, errors.New("missing signatures content")
}
for _, sig := range v.IntotoObj.Content.Envelope.Signatures {
if len(sig.Sig) == 0 {
if sig == nil {
return false, errors.New("missing signature entry")
}
if sig.Sig == nil || len(*sig.Sig) == 0 {
return false, errors.New("missing signature content")
}
if len(sig.PublicKey) == 0 {
if sig.PublicKey == nil || len(*sig.PublicKey) == 0 {
return false, errors.New("missing publicKey content")
}
}
Expand Down
59 changes: 45 additions & 14 deletions pkg/types/intoto/v0.0.2/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,12 @@ func createRekorEnvelope(dsseEnv *dsse.Envelope, pub [][]byte) *models.IntotoV00
env.PayloadType = &dsseEnv.PayloadType

for i, sig := range dsseEnv.Signatures {
keyBytes := strfmt.Base64(pub[i])
sigBytes := strfmt.Base64([]byte(sig.Sig))
env.Signatures = append(env.Signatures, &models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
Keyid: sig.KeyID,
Sig: strfmt.Base64([]byte(sig.Sig)),
PublicKey: strfmt.Base64(pub[i]),
Sig: &sigBytes,
PublicKey: &keyBytes,
})
}

Expand Down Expand Up @@ -172,6 +174,8 @@ func TestV002Entry_Unmarshal(t *testing.T) {
}

validPayload := "hellothispayloadisvalid"
keyBytes := strfmt.Base64("key")
sigBytes := strfmt.Base64("sig")

tests := []struct {
env *dsse.Envelope
Expand Down Expand Up @@ -273,6 +277,31 @@ func TestV002Entry_Unmarshal(t *testing.T) {
wantErr: false,
wantVerifierErr: false,
},
{
env: envelope(t, key, []byte(validPayload)),
name: "null array entry",
it: &models.IntotoV002Schema{
Content: &models.IntotoV002SchemaContent{
Envelope: &models.IntotoV002SchemaContentEnvelope{
Payload: strfmt.Base64("cGF5bG9hZAo="),
PayloadType: swag.String("payloadType"),
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
PublicKey: &keyBytes,
Sig: &sigBytes,
},
nil,
},
},
Hash: &models.IntotoV002SchemaContentHash{
Algorithm: swag.String(models.IntotoV002SchemaContentHashAlgorithmSha256),
Value: swag.String(envelopeHash(t, envelope(t, key, []byte(validPayload)))),
},
},
},
wantErr: true,
wantVerifierErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -295,7 +324,7 @@ func TestV002Entry_Unmarshal(t *testing.T) {

want := []string{}
for _, sig := range v.IntotoObj.Content.Envelope.Signatures {
keyHash := sha256.Sum256(sig.PublicKey)
keyHash := sha256.Sum256(*sig.PublicKey)
want = append(want, "sha256:"+hex.EncodeToString(keyHash[:]))
}
decodedPayload, err := base64.StdEncoding.DecodeString(tt.env.Payload)
Expand Down Expand Up @@ -483,7 +512,7 @@ func TestV002Entry_IndexKeys(t *testing.T) {
}
want := []string{}
for _, sig := range v.IntotoObj.Content.Envelope.Signatures {
keyHash := sha256.Sum256(sig.PublicKey)
keyHash := sha256.Sum256(*sig.PublicKey)
want = append(want, "sha256:"+hex.EncodeToString(keyHash[:]))
}

Expand Down Expand Up @@ -513,6 +542,8 @@ func TestInsertable(t *testing.T) {
}

env := envelope(t, key, []byte("payload"))
keyBytes := strfmt.Base64([]byte("key"))
sigBytes := strfmt.Base64([]byte("sig"))

testCases := []TestCase{
{
Expand All @@ -525,8 +556,8 @@ func TestInsertable(t *testing.T) {
PayloadType: swag.String("payloadType"),
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
PublicKey: strfmt.Base64([]byte("key")),
Sig: strfmt.Base64([]byte("sig")),
PublicKey: &keyBytes,
Sig: &sigBytes,
},
},
},
Expand All @@ -546,8 +577,8 @@ func TestInsertable(t *testing.T) {
PayloadType: swag.String("payloadType"),
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
PublicKey: strfmt.Base64([]byte("key")),
Sig: strfmt.Base64([]byte("sig")),
PublicKey: &keyBytes,
Sig: &sigBytes,
},
},
},
Expand All @@ -567,7 +598,7 @@ func TestInsertable(t *testing.T) {
PayloadType: swag.String("payloadType"),
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
PublicKey: strfmt.Base64([]byte("key")),
PublicKey: &keyBytes,
//Sig: strfmt.Base64([]byte("sig")),
},
},
Expand All @@ -589,7 +620,7 @@ func TestInsertable(t *testing.T) {
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
//PublicKey: strfmt.Base64([]byte("key")),
Sig: strfmt.Base64([]byte("sig")),
Sig: &sigBytes,
},
},
},
Expand Down Expand Up @@ -633,8 +664,8 @@ func TestInsertable(t *testing.T) {
//PayloadType: swag.String("payloadType"),
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
PublicKey: strfmt.Base64([]byte("key")),
Sig: strfmt.Base64([]byte("sig")),
PublicKey: &keyBytes,
Sig: &sigBytes,
},
},
},
Expand All @@ -654,8 +685,8 @@ func TestInsertable(t *testing.T) {
PayloadType: swag.String("payloadType"),
Signatures: []*models.IntotoV002SchemaContentEnvelopeSignaturesItems0{
{
PublicKey: strfmt.Base64([]byte("key")),
Sig: strfmt.Base64([]byte("sig")),
PublicKey: &keyBytes,
Sig: &sigBytes,
},
},
},
Expand Down
3 changes: 2 additions & 1 deletion pkg/types/intoto/v0.0.2/intoto_v0_0_2_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
"type": "string",
"format": "byte"
}
}
},
"required": ["sig", "publicKey"]
}
}
},
Expand Down

0 comments on commit 140c5ad

Please sign in to comment.