From ab3bfee37da41e7381619e93283b86f74cc89b53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 7 Jan 2023 00:18:50 +0100 Subject: [PATCH] Split sigstore configuration parsing and API into separate files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to make policy_config*.go a bit smaller, and to allow sigstore logic expansion. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač --- signature/policy_config.go | 101 -------------- signature/policy_config_sigstore.go | 109 +++++++++++++++ signature/policy_config_sigstore_test.go | 167 +++++++++++++++++++++++ signature/policy_config_test.go | 158 --------------------- 4 files changed, 276 insertions(+), 259 deletions(-) create mode 100644 signature/policy_config_sigstore.go create mode 100644 signature/policy_config_sigstore_test.go diff --git a/signature/policy_config.go b/signature/policy_config.go index 97bef2a917..5ca4ad7130 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -518,107 +518,6 @@ func (pr *prSignedBaseLayer) UnmarshalJSON(data []byte) error { return nil } -// newPRSigstoreSigned returns a new prSigstoreSigned if parameters are valid. -func newPRSigstoreSigned(keyPath string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSigstoreSigned, error) { - if len(keyPath) > 0 && len(keyData) > 0 { - return nil, InvalidPolicyFormatError("keyType and keyData cannot be used simultaneously") - } - if signedIdentity == nil { - return nil, InvalidPolicyFormatError("signedIdentity not specified") - } - return &prSigstoreSigned{ - prCommon: prCommon{Type: prTypeSigstoreSigned}, - KeyPath: keyPath, - KeyData: keyData, - SignedIdentity: signedIdentity, - }, nil -} - -// newPRSigstoreSignedKeyPath is NewPRSigstoreSignedKeyPath, except it returns the private type. -func newPRSigstoreSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) (*prSigstoreSigned, error) { - return newPRSigstoreSigned(keyPath, nil, signedIdentity) -} - -// NewPRSigstoreSignedKeyPath returns a new "sigstoreSigned" PolicyRequirement using a KeyPath -func NewPRSigstoreSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { - return newPRSigstoreSignedKeyPath(keyPath, signedIdentity) -} - -// newPRSigstoreSignedKeyData is NewPRSigstoreSignedKeyData, except it returns the private type. -func newPRSigstoreSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) (*prSigstoreSigned, error) { - return newPRSigstoreSigned("", keyData, signedIdentity) -} - -// NewPRSigstoreSignedKeyData returns a new "sigstoreSigned" PolicyRequirement using a KeyData -func NewPRSigstoreSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { - return newPRSigstoreSignedKeyData(keyData, signedIdentity) -} - -// Compile-time check that prSigstoreSigned implements json.Unmarshaler. -var _ json.Unmarshaler = (*prSigstoreSigned)(nil) - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { - *pr = prSigstoreSigned{} - var tmp prSigstoreSigned - var gotKeyPath, gotKeyData = false, false - var signedIdentity json.RawMessage - if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - case "keyPath": - gotKeyPath = true - return &tmp.KeyPath - case "keyData": - gotKeyData = true - return &tmp.KeyData - case "signedIdentity": - return &signedIdentity - default: - return nil - } - }); err != nil { - return err - } - - if tmp.Type != prTypeSigstoreSigned { - return InvalidPolicyFormatError(fmt.Sprintf("Unexpected policy requirement type \"%s\"", tmp.Type)) - } - if signedIdentity == nil { - tmp.SignedIdentity = NewPRMMatchRepoDigestOrExact() - } else { - si, err := newPolicyReferenceMatchFromJSON(signedIdentity) - if err != nil { - return err - } - tmp.SignedIdentity = si - } - - var res *prSigstoreSigned - var err error - switch { - case gotKeyPath && gotKeyData: - return InvalidPolicyFormatError("keyPath and keyData cannot be used simultaneously") - case gotKeyPath && !gotKeyData: - res, err = newPRSigstoreSignedKeyPath(tmp.KeyPath, tmp.SignedIdentity) - case !gotKeyPath && gotKeyData: - res, err = newPRSigstoreSignedKeyData(tmp.KeyData, tmp.SignedIdentity) - case !gotKeyPath && !gotKeyData: - return InvalidPolicyFormatError("At least one of keyPath and keyData must be specified") - default: // Coverage: This should never happen - return fmt.Errorf("Impossible keyPath/keyData presence combination!?") - } - if err != nil { - // Coverage: This cannot currently happen, creating a prSigstoreSigned only fails - // if signedIdentity is nil, which we replace with a default above. - return err - } - *pr = *res - - return nil -} - // newPolicyReferenceMatchFromJSON parses JSON data into a PolicyReferenceMatch implementation. func newPolicyReferenceMatchFromJSON(data []byte) (PolicyReferenceMatch, error) { var typeField prmCommon diff --git a/signature/policy_config_sigstore.go b/signature/policy_config_sigstore.go new file mode 100644 index 0000000000..702e28ca36 --- /dev/null +++ b/signature/policy_config_sigstore.go @@ -0,0 +1,109 @@ +package signature + +import ( + "encoding/json" + "fmt" + + "github.com/containers/image/v5/signature/internal" +) + +// newPRSigstoreSigned returns a new prSigstoreSigned if parameters are valid. +func newPRSigstoreSigned(keyPath string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSigstoreSigned, error) { + if len(keyPath) > 0 && len(keyData) > 0 { + return nil, InvalidPolicyFormatError("keyType and keyData cannot be used simultaneously") + } + if signedIdentity == nil { + return nil, InvalidPolicyFormatError("signedIdentity not specified") + } + return &prSigstoreSigned{ + prCommon: prCommon{Type: prTypeSigstoreSigned}, + KeyPath: keyPath, + KeyData: keyData, + SignedIdentity: signedIdentity, + }, nil +} + +// newPRSigstoreSignedKeyPath is NewPRSigstoreSignedKeyPath, except it returns the private type. +func newPRSigstoreSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) (*prSigstoreSigned, error) { + return newPRSigstoreSigned(keyPath, nil, signedIdentity) +} + +// NewPRSigstoreSignedKeyPath returns a new "sigstoreSigned" PolicyRequirement using a KeyPath +func NewPRSigstoreSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { + return newPRSigstoreSignedKeyPath(keyPath, signedIdentity) +} + +// newPRSigstoreSignedKeyData is NewPRSigstoreSignedKeyData, except it returns the private type. +func newPRSigstoreSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) (*prSigstoreSigned, error) { + return newPRSigstoreSigned("", keyData, signedIdentity) +} + +// NewPRSigstoreSignedKeyData returns a new "sigstoreSigned" PolicyRequirement using a KeyData +func NewPRSigstoreSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { + return newPRSigstoreSignedKeyData(keyData, signedIdentity) +} + +// Compile-time check that prSigstoreSigned implements json.Unmarshaler. +var _ json.Unmarshaler = (*prSigstoreSigned)(nil) + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { + *pr = prSigstoreSigned{} + var tmp prSigstoreSigned + var gotKeyPath, gotKeyData = false, false + var signedIdentity json.RawMessage + if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { + switch key { + case "type": + return &tmp.Type + case "keyPath": + gotKeyPath = true + return &tmp.KeyPath + case "keyData": + gotKeyData = true + return &tmp.KeyData + case "signedIdentity": + return &signedIdentity + default: + return nil + } + }); err != nil { + return err + } + + if tmp.Type != prTypeSigstoreSigned { + return InvalidPolicyFormatError(fmt.Sprintf("Unexpected policy requirement type \"%s\"", tmp.Type)) + } + if signedIdentity == nil { + tmp.SignedIdentity = NewPRMMatchRepoDigestOrExact() + } else { + si, err := newPolicyReferenceMatchFromJSON(signedIdentity) + if err != nil { + return err + } + tmp.SignedIdentity = si + } + + var res *prSigstoreSigned + var err error + switch { + case gotKeyPath && gotKeyData: + return InvalidPolicyFormatError("keyPath and keyData cannot be used simultaneously") + case gotKeyPath && !gotKeyData: + res, err = newPRSigstoreSignedKeyPath(tmp.KeyPath, tmp.SignedIdentity) + case !gotKeyPath && gotKeyData: + res, err = newPRSigstoreSignedKeyData(tmp.KeyData, tmp.SignedIdentity) + case !gotKeyPath && !gotKeyData: + return InvalidPolicyFormatError("At least one of keyPath and keyData must be specified") + default: // Coverage: This should never happen + return fmt.Errorf("Impossible keyPath/keyData presence combination!?") + } + if err != nil { + // Coverage: This cannot currently happen, creating a prSigstoreSigned only fails + // if signedIdentity is nil, which we replace with a default above. + return err + } + *pr = *res + + return nil +} diff --git a/signature/policy_config_sigstore_test.go b/signature/policy_config_sigstore_test.go new file mode 100644 index 0000000000..f28a6b33e1 --- /dev/null +++ b/signature/policy_config_sigstore_test.go @@ -0,0 +1,167 @@ +package signature + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// xNewPRSigstoreSignedKeyPath is like NewPRSigstoreSignedKeyPath, except it must not fail. +func xNewPRSigstoreSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) PolicyRequirement { + pr, err := NewPRSigstoreSignedKeyPath(keyPath, signedIdentity) + if err != nil { + panic("xNewPRSigstoreSignedKeyPath failed") + } + return pr +} + +// xNewPRSigstoreSignedKeyData is like NewPRSigstoreSignedKeyData, except it must not fail. +func xNewPRSigstoreSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) PolicyRequirement { + pr, err := NewPRSigstoreSignedKeyData(keyData, signedIdentity) + if err != nil { + panic("xNewPRSigstoreSignedKeyData failed") + } + return pr +} + +func TestNewPRSigstoreSigned(t *testing.T) { + const testPath = "/foo/bar" + testData := []byte("abc") + testIdentity := NewPRMMatchRepoDigestOrExact() + + // Success + pr, err := newPRSigstoreSigned(testPath, nil, testIdentity) + require.NoError(t, err) + assert.Equal(t, &prSigstoreSigned{ + prCommon: prCommon{prTypeSigstoreSigned}, + KeyPath: testPath, + KeyData: nil, + SignedIdentity: testIdentity, + }, pr) + pr, err = newPRSigstoreSigned("", testData, testIdentity) + require.NoError(t, err) + assert.Equal(t, &prSigstoreSigned{ + prCommon: prCommon{prTypeSigstoreSigned}, + KeyPath: "", + KeyData: testData, + SignedIdentity: testIdentity, + }, pr) + + // Both keyPath and keyData specified + _, err = newPRSigstoreSigned(testPath, testData, testIdentity) + assert.Error(t, err) + + // Invalid signedIdentity + _, err = newPRSigstoreSigned(testPath, nil, nil) + assert.Error(t, err) +} + +func TestNewPRSigstoreSignedKeyPath(t *testing.T) { + const testPath = "/foo/bar" + _pr, err := NewPRSigstoreSignedKeyPath(testPath, NewPRMMatchRepoDigestOrExact()) + require.NoError(t, err) + pr, ok := _pr.(*prSigstoreSigned) + require.True(t, ok) + assert.Equal(t, testPath, pr.KeyPath) + // Failure cases tested in TestNewPRSigstoreSigned. +} + +func TestNewPRSigstoreSignedKeyData(t *testing.T) { + testData := []byte("abc") + _pr, err := NewPRSigstoreSignedKeyData(testData, NewPRMMatchRepoDigestOrExact()) + require.NoError(t, err) + pr, ok := _pr.(*prSigstoreSigned) + require.True(t, ok) + assert.Equal(t, testData, pr.KeyData) + // Failure cases tested in TestNewPRSigstoreSigned. +} + +// Return the result of modifying validJSON with fn and unmarshaling it into *pr +func tryUnmarshalModifiedSigstoreSigned(t *testing.T, pr *prSigstoreSigned, validJSON []byte, modifyFn func(mSI)) error { + var tmp mSI + err := json.Unmarshal(validJSON, &tmp) + require.NoError(t, err) + + modifyFn(tmp) + + *pr = prSigstoreSigned{} + return jsonUnmarshalFromObject(t, tmp, &pr) +} + +func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { + keyDataTests := policyJSONUmarshallerTests{ + newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, + newValidObject: func() (interface{}, error) { + return NewPRSigstoreSignedKeyData([]byte("abc"), NewPRMMatchRepoDigestOrExact()) + }, + otherJSONParser: func(validJSON []byte) (interface{}, error) { + return newPolicyRequirementFromJSON(validJSON) + }, + breakFns: []func(mSI){ + // The "type" field is missing + func(v mSI) { delete(v, "type") }, + // Wrong "type" field + func(v mSI) { v["type"] = 1 }, + func(v mSI) { v["type"] = "this is invalid" }, + // Extra top-level sub-object + func(v mSI) { v["unexpected"] = 1 }, + // Both "keyPath" and "keyData" is missing + func(v mSI) { delete(v, "keyData") }, + // Both "keyPath" and "keyData" is present + func(v mSI) { v["keyPath"] = "/foo/bar" }, + // Invalid "keyPath" field + func(v mSI) { delete(v, "keyData"); v["keyPath"] = 1 }, + func(v mSI) { v["type"] = "this is invalid" }, + // Invalid "keyData" field + func(v mSI) { v["keyData"] = 1 }, + func(v mSI) { v["keyData"] = "this is invalid base64" }, + // Invalid "signedIdentity" field + func(v mSI) { v["signedIdentity"] = "this is invalid" }, + // "signedIdentity" an explicit nil + func(v mSI) { v["signedIdentity"] = nil }, + }, + duplicateFields: []string{"type", "keyData", "signedIdentity"}, + } + keyDataTests.run(t) + // Test the keyPath-specific aspects + policyJSONUmarshallerTests{ + newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, + newValidObject: func() (interface{}, error) { + return NewPRSigstoreSignedKeyPath("/foo/bar", NewPRMMatchRepoDigestOrExact()) + }, + otherJSONParser: func(validJSON []byte) (interface{}, error) { + return newPolicyRequirementFromJSON(validJSON) + }, + duplicateFields: []string{"type", "keyPath", "signedIdentity"}, + }.run(t) + + var pr prSigstoreSigned + + // Start with a valid JSON. + _, validJSON := keyDataTests.validObjectAndJSON(t) + + // Various allowed modifications to the requirement + allowedModificationFns := []func(mSI){ + // Delete the signedIdentity field + func(v mSI) { delete(v, "signedIdentity") }, + } + for _, fn := range allowedModificationFns { + err := tryUnmarshalModifiedSigstoreSigned(t, &pr, validJSON, fn) + require.NoError(t, err) + } + + // Various ways to set signedIdentity to the default value + signedIdentityDefaultFns := []func(mSI){ + // Set signedIdentity to the default explicitly + func(v mSI) { v["signedIdentity"] = NewPRMMatchRepoDigestOrExact() }, + // Delete the signedIdentity field + func(v mSI) { delete(v, "signedIdentity") }, + } + for _, fn := range signedIdentityDefaultFns { + err := tryUnmarshalModifiedSigstoreSigned(t, &pr, validJSON, fn) + require.NoError(t, err) + assert.Equal(t, NewPRMMatchRepoDigestOrExact(), pr.SignedIdentity) + } +} diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 19d6a21547..362c34402b 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -1021,164 +1021,6 @@ func TestPRSignedBaseLayerUnmarshalJSON(t *testing.T) { }.run(t) } -// xNewPRSigstoreSignedKeyPath is like NewPRSigstoreSignedKeyPath, except it must not fail. -func xNewPRSigstoreSignedKeyPath(keyPath string, signedIdentity PolicyReferenceMatch) PolicyRequirement { - pr, err := NewPRSigstoreSignedKeyPath(keyPath, signedIdentity) - if err != nil { - panic("xNewPRSigstoreSignedKeyPath failed") - } - return pr -} - -// xNewPRSigstoreSignedKeyData is like NewPRSigstoreSignedKeyData, except it must not fail. -func xNewPRSigstoreSignedKeyData(keyData []byte, signedIdentity PolicyReferenceMatch) PolicyRequirement { - pr, err := NewPRSigstoreSignedKeyData(keyData, signedIdentity) - if err != nil { - panic("xNewPRSigstoreSignedKeyData failed") - } - return pr -} - -func TestNewPRSigstoreSigned(t *testing.T) { - const testPath = "/foo/bar" - testData := []byte("abc") - testIdentity := NewPRMMatchRepoDigestOrExact() - - // Success - pr, err := newPRSigstoreSigned(testPath, nil, testIdentity) - require.NoError(t, err) - assert.Equal(t, &prSigstoreSigned{ - prCommon: prCommon{prTypeSigstoreSigned}, - KeyPath: testPath, - KeyData: nil, - SignedIdentity: testIdentity, - }, pr) - pr, err = newPRSigstoreSigned("", testData, testIdentity) - require.NoError(t, err) - assert.Equal(t, &prSigstoreSigned{ - prCommon: prCommon{prTypeSigstoreSigned}, - KeyPath: "", - KeyData: testData, - SignedIdentity: testIdentity, - }, pr) - - // Both keyPath and keyData specified - _, err = newPRSigstoreSigned(testPath, testData, testIdentity) - assert.Error(t, err) - - // Invalid signedIdentity - _, err = newPRSigstoreSigned(testPath, nil, nil) - assert.Error(t, err) -} - -func TestNewPRSigstoreSignedKeyPath(t *testing.T) { - const testPath = "/foo/bar" - _pr, err := NewPRSigstoreSignedKeyPath(testPath, NewPRMMatchRepoDigestOrExact()) - require.NoError(t, err) - pr, ok := _pr.(*prSigstoreSigned) - require.True(t, ok) - assert.Equal(t, testPath, pr.KeyPath) - // Failure cases tested in TestNewPRSigstoreSigned. -} - -func TestNewPRSigstoreSignedKeyData(t *testing.T) { - testData := []byte("abc") - _pr, err := NewPRSigstoreSignedKeyData(testData, NewPRMMatchRepoDigestOrExact()) - require.NoError(t, err) - pr, ok := _pr.(*prSigstoreSigned) - require.True(t, ok) - assert.Equal(t, testData, pr.KeyData) - // Failure cases tested in TestNewPRSigstoreSigned. -} - -// Return the result of modifying validJSON with fn and unmarshaling it into *pr -func tryUnmarshalModifiedSigstoreSigned(t *testing.T, pr *prSigstoreSigned, validJSON []byte, modifyFn func(mSI)) error { - var tmp mSI - err := json.Unmarshal(validJSON, &tmp) - require.NoError(t, err) - - modifyFn(tmp) - - *pr = prSigstoreSigned{} - return jsonUnmarshalFromObject(t, tmp, &pr) -} - -func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { - keyDataTests := policyJSONUmarshallerTests{ - newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, - newValidObject: func() (interface{}, error) { - return NewPRSigstoreSignedKeyData([]byte("abc"), NewPRMMatchRepoDigestOrExact()) - }, - otherJSONParser: func(validJSON []byte) (interface{}, error) { - return newPolicyRequirementFromJSON(validJSON) - }, - breakFns: []func(mSI){ - // The "type" field is missing - func(v mSI) { delete(v, "type") }, - // Wrong "type" field - func(v mSI) { v["type"] = 1 }, - func(v mSI) { v["type"] = "this is invalid" }, - // Extra top-level sub-object - func(v mSI) { v["unexpected"] = 1 }, - // Both "keyPath" and "keyData" is missing - func(v mSI) { delete(v, "keyData") }, - // Both "keyPath" and "keyData" is present - func(v mSI) { v["keyPath"] = "/foo/bar" }, - // Invalid "keyPath" field - func(v mSI) { delete(v, "keyData"); v["keyPath"] = 1 }, - func(v mSI) { v["type"] = "this is invalid" }, - // Invalid "keyData" field - func(v mSI) { v["keyData"] = 1 }, - func(v mSI) { v["keyData"] = "this is invalid base64" }, - // Invalid "signedIdentity" field - func(v mSI) { v["signedIdentity"] = "this is invalid" }, - // "signedIdentity" an explicit nil - func(v mSI) { v["signedIdentity"] = nil }, - }, - duplicateFields: []string{"type", "keyData", "signedIdentity"}, - } - keyDataTests.run(t) - // Test the keyPath-specific aspects - policyJSONUmarshallerTests{ - newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, - newValidObject: func() (interface{}, error) { - return NewPRSigstoreSignedKeyPath("/foo/bar", NewPRMMatchRepoDigestOrExact()) - }, - otherJSONParser: func(validJSON []byte) (interface{}, error) { - return newPolicyRequirementFromJSON(validJSON) - }, - duplicateFields: []string{"type", "keyPath", "signedIdentity"}, - }.run(t) - - var pr prSigstoreSigned - - // Start with a valid JSON. - _, validJSON := keyDataTests.validObjectAndJSON(t) - - // Various allowed modifications to the requirement - allowedModificationFns := []func(mSI){ - // Delete the signedIdentity field - func(v mSI) { delete(v, "signedIdentity") }, - } - for _, fn := range allowedModificationFns { - err := tryUnmarshalModifiedSigstoreSigned(t, &pr, validJSON, fn) - require.NoError(t, err) - } - - // Various ways to set signedIdentity to the default value - signedIdentityDefaultFns := []func(mSI){ - // Set signedIdentity to the default explicitly - func(v mSI) { v["signedIdentity"] = NewPRMMatchRepoDigestOrExact() }, - // Delete the signedIdentity field - func(v mSI) { delete(v, "signedIdentity") }, - } - for _, fn := range signedIdentityDefaultFns { - err := tryUnmarshalModifiedSigstoreSigned(t, &pr, validJSON, fn) - require.NoError(t, err) - assert.Equal(t, NewPRMMatchRepoDigestOrExact(), pr.SignedIdentity) - } -} - func TestNewPolicyReferenceMatchFromJSON(t *testing.T) { // Sample success. Others tested in the individual PolicyReferenceMatch.UnmarshalJSON implementations. validPRM := NewPRMMatchRepoDigestOrExact()