Skip to content

Commit

Permalink
Allow accepting multiple GPG keyrings via signedBy.keyPaths
Browse files Browse the repository at this point in the history 
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
@mtrmac
mtrmac committed Jul 13, 2022
1 parent 4cb7f87 commit d218ff3
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 32 deletions.
3 changes: 2 additions & 1 deletion docs/containers-policy.json.5.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,14 @@ This requirement requires an image to be signed using “simple signing” with
"type": "signedBy",
"keyType": "GPGKeys", /* The only currently supported value */
"keyPath": "/path/to/local/keyring/file",
"keyPaths": ["/path/to/local/keyring/file1","/path/to/local/keyring/file2"…],
"keyData": "base64-encoded-keyring-data",
"signedIdentity": identity_requirement
}
```
<!-- Later: other keyType values -->

Exactly one of `keyPath` and `keyData` must be present, containing a GPG keyring of one or more public keys. Only signatures made by these keys are accepted.
Exactly one of `keyPath`, `keyPaths` and `keyData` must be present, containing a GPG keyring of one or more public keys. Only signatures made by these keys are accepted.

The `signedIdentity` field, a JSON object, specifies what image identity the signature claims about the image.
One of the following alternatives are supported:
Expand Down
7 changes: 7 additions & 0 deletions signature/fixtures/policy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@
}
}
],
"registry.redhat.io/beta": [
{
"type": "signedBy",
"keyType": "GPGKeys",
"keyPaths": ["/keys/RH-production-signing-key-gpg-keyring", "/keys/RH-beta-signing-key-gpg-keyring"]
}
],
"private-mirror:5000/vendor-mirror": [
{
"type": "signedBy",
Expand Down
39 changes: 29 additions & 10 deletions signature/policy_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,19 +316,22 @@ func (pr *prReject) UnmarshalJSON(data []byte) error {
}

// newPRSignedBy returns a new prSignedBy if parameters are valid.
func newPRSignedBy(keyType sbKeyType, keyPath string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) {
func newPRSignedBy(keyType sbKeyType, keyPath string, keyPaths []string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) {
if !keyType.IsValid() {
return nil, InvalidPolicyFormatError(fmt.Sprintf("invalid keyType \"%s\"", keyType))
}
keySources := 0
if keyPath != "" {
keySources++
}
if keyPaths != nil {
keySources++
}
if keyData != nil {
keySources++
}
if keySources != 1 {
return nil, InvalidPolicyFormatError("exactly one of keyPath and keyData must be specified")
return nil, InvalidPolicyFormatError("exactly one of keyPath, keyPaths and keyData must be specified")
}
if signedIdentity == nil {
return nil, InvalidPolicyFormatError("signedIdentity not specified")
Expand All @@ -337,24 +340,35 @@ func newPRSignedBy(keyType sbKeyType, keyPath string, keyData []byte, signedIden
prCommon: prCommon{Type: prTypeSignedBy},
KeyType: keyType,
KeyPath: keyPath,
KeyPaths: keyPaths,
KeyData: keyData,
SignedIdentity: signedIdentity,
}, nil
}

// newPRSignedByKeyPath is NewPRSignedByKeyPath, except it returns the private type.
func newPRSignedByKeyPath(keyType sbKeyType, keyPath string, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) {
return newPRSignedBy(keyType, keyPath, nil, signedIdentity)
return newPRSignedBy(keyType, keyPath, nil, nil, signedIdentity)
}

// NewPRSignedByKeyPath returns a new "signedBy" PolicyRequirement using a KeyPath
func NewPRSignedByKeyPath(keyType sbKeyType, keyPath string, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) {
return newPRSignedByKeyPath(keyType, keyPath, signedIdentity)
}

// newPRSignedByKeyPaths is NewPRSignedByKeyPaths, except it returns the private type.
func newPRSignedByKeyPaths(keyType sbKeyType, keyPaths []string, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) {
return newPRSignedBy(keyType, "", keyPaths, nil, signedIdentity)
}

// NewPRSignedByKeyPaths returns a new "signedBy" PolicyRequirement using KeyPaths
func NewPRSignedByKeyPaths(keyType sbKeyType, keyPaths []string, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) {
return newPRSignedByKeyPaths(keyType, keyPaths, signedIdentity)
}

// newPRSignedByKeyData is NewPRSignedByKeyData, except it returns the private type.
func newPRSignedByKeyData(keyType sbKeyType, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) {
return newPRSignedBy(keyType, "", keyData, signedIdentity)
return newPRSignedBy(keyType, "", nil, keyData, signedIdentity)
}

// NewPRSignedByKeyData returns a new "signedBy" PolicyRequirement using a KeyData
Expand All @@ -369,7 +383,7 @@ var _ json.Unmarshaler = (*prSignedBy)(nil)
func (pr *prSignedBy) UnmarshalJSON(data []byte) error {
*pr = prSignedBy{}
var tmp prSignedBy
var gotKeyPath, gotKeyData = false, false
var gotKeyPath, gotKeyPaths, gotKeyData = false, false, false
var signedIdentity json.RawMessage
if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} {
switch key {
Expand All @@ -380,6 +394,9 @@ func (pr *prSignedBy) UnmarshalJSON(data []byte) error {
case "keyPath":
gotKeyPath = true
return &tmp.KeyPath
case "keyPaths":
gotKeyPaths = true
return &tmp.KeyPaths
case "keyData":
gotKeyData = true
return &tmp.KeyData
Expand Down Expand Up @@ -408,14 +425,16 @@ func (pr *prSignedBy) UnmarshalJSON(data []byte) error {
var res *prSignedBy
var err error
switch {
case gotKeyPath && !gotKeyData:
case gotKeyPath && !gotKeyPaths && !gotKeyData:
res, err = newPRSignedByKeyPath(tmp.KeyType, tmp.KeyPath, tmp.SignedIdentity)
case !gotKeyPath && gotKeyData:
case !gotKeyPath && gotKeyPaths && !gotKeyData:
res, err = newPRSignedByKeyPaths(tmp.KeyType, tmp.KeyPaths, tmp.SignedIdentity)
case !gotKeyPath && !gotKeyPaths && gotKeyData:
res, err = newPRSignedByKeyData(tmp.KeyType, tmp.KeyData, tmp.SignedIdentity)
case !gotKeyPath && !gotKeyData:
return InvalidPolicyFormatError("At least one of keyPath and keyData must be specified")
case !gotKeyPath && !gotKeyPaths && !gotKeyData:
return InvalidPolicyFormatError("Exactly one of keyPath, keyPaths and keyData must be specified, none of them present")
default:
return fmt.Errorf("Exactly one of keyPath and keyData must be specified, more than one present")
return fmt.Errorf("Exactly one of keyPath, keyPaths and keyData must be specified, more than one present")
}
if err != nil {
return err
Expand Down
81 changes: 72 additions & 9 deletions signature/policy_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ var policyFixtureContents = &Policy{
"/keys/RH-key-signing-key-gpg-keyring",
NewPRMMatchRepoDigestOrExact()),
},
"registry.redhat.io/beta": {
xNewPRSignedByKeyPaths(SBKeyTypeGPGKeys,
[]string{"/keys/RH-production-signing-key-gpg-keyring", "/keys/RH-beta-signing-key-gpg-keyring"},
newPRMMatchRepoDigestOrExact()),
},
"private-mirror:5000/vendor-mirror": {
xNewPRSignedByKeyPath(SBKeyTypeGPGKeys,
"/keys/vendor-gpg-keyring",
Expand Down Expand Up @@ -379,6 +384,15 @@ func xNewPRSignedByKeyPath(keyType sbKeyType, keyPath string, signedIdentity Pol
return pr
}

// xNewPRSignedByKeyPaths is like NewPRSignedByKeyPaths, except it must not fail.
func xNewPRSignedByKeyPaths(keyType sbKeyType, keyPaths []string, signedIdentity PolicyReferenceMatch) PolicyRequirement {
pr, err := NewPRSignedByKeyPaths(keyType, keyPaths, signedIdentity)
if err != nil {
panic("xNewPRSignedByKeyPaths failed")
}
return pr
}

// xNewPRSignedByKeyData is like NewPRSignedByKeyData, except it must not fail.
func xNewPRSignedByKeyData(keyType sbKeyType, keyData []byte, signedIdentity PolicyReferenceMatch) PolicyRequirement {
pr, err := NewPRSignedByKeyData(keyType, keyData, signedIdentity)
Expand Down Expand Up @@ -708,41 +722,62 @@ func TestPRRejectUnmarshalJSON(t *testing.T) {

func TestNewPRSignedBy(t *testing.T) {
const testPath = "/foo/bar"
testPaths := []string{"/path/1", "/path/2"}
testData := []byte("abc")
testIdentity := NewPRMMatchRepoDigestOrExact()

// Success
pr, err := newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, testIdentity)
pr, err := newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, nil, testIdentity)
require.NoError(t, err)
assert.Equal(t, &prSignedBy{
prCommon: prCommon{prTypeSignedBy},
KeyType: SBKeyTypeGPGKeys,
KeyPath: testPath,
KeyPaths: nil,
KeyData: nil,
SignedIdentity: testIdentity,
}, pr)
pr, err = newPRSignedBy(SBKeyTypeGPGKeys, "", testPaths, nil, testIdentity)
require.NoError(t, err)
assert.Equal(t, &prSignedBy{
prCommon: prCommon{prTypeSignedBy},
KeyType: SBKeyTypeGPGKeys,
KeyPath: "",
KeyPaths: testPaths,
KeyData: nil,
SignedIdentity: testIdentity,
}, pr)
pr, err = newPRSignedBy(SBKeyTypeGPGKeys, "", testData, testIdentity)
pr, err = newPRSignedBy(SBKeyTypeGPGKeys, "", nil, testData, testIdentity)
require.NoError(t, err)
assert.Equal(t, &prSignedBy{
prCommon: prCommon{prTypeSignedBy},
KeyType: SBKeyTypeGPGKeys,
KeyPath: "",
KeyPaths: nil,
KeyData: testData,
SignedIdentity: testIdentity,
}, pr)

// Invalid keyType
_, err = newPRSignedBy(sbKeyType(""), testPath, nil, testIdentity)
_, err = newPRSignedBy(sbKeyType(""), testPath, nil, nil, testIdentity)
assert.Error(t, err)
_, err = newPRSignedBy(sbKeyType("this is invalid"), testPath, nil, testIdentity)
_, err = newPRSignedBy(sbKeyType("this is invalid"), testPath, nil, nil, testIdentity)
assert.Error(t, err)

// Both keyPath and keyData specified
_, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, testData, testIdentity)
// Invalid keyPath/keyPaths/keyData combinations
_, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, testPaths, testData, testIdentity)
assert.Error(t, err)
_, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, testPaths, nil, testIdentity)
assert.Error(t, err)
_, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, testData, testIdentity)
assert.Error(t, err)
_, err = newPRSignedBy(SBKeyTypeGPGKeys, "", testPaths, testData, testIdentity)
assert.Error(t, err)
_, err = newPRSignedBy(SBKeyTypeGPGKeys, "", nil, nil, testIdentity)
assert.Error(t, err)

// Invalid signedIdentity
_, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, nil)
_, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, nil, nil)
assert.Error(t, err)
}

Expand All @@ -756,6 +791,16 @@ func TestNewPRSignedByKeyPath(t *testing.T) {
// Failure cases tested in TestNewPRSignedBy.
}

func TestNewPRSignedByKeyPaths(t *testing.T) {
testPaths := []string{"/path/1", "/path/2"}
_pr, err := NewPRSignedByKeyPaths(SBKeyTypeGPGKeys, testPaths, NewPRMMatchRepoDigestOrExact())
require.NoError(t, err)
pr, ok := _pr.(*prSignedBy)
require.True(t, ok)
assert.Equal(t, testPaths, pr.KeyPaths)
// Failure cases tested in TestNewPRSignedBy.
}

func TestNewPRSignedByKeyData(t *testing.T) {
testData := []byte("abc")
_pr, err := NewPRSignedByKeyData(SBKeyTypeGPGKeys, testData, NewPRMMatchRepoDigestOrExact())
Expand Down Expand Up @@ -799,12 +844,19 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) {
func(v mSI) { delete(v, "keyType") },
// Invalid "keyType" field
func(v mSI) { v["keyType"] = "this is invalid" },
// Both "keyPath" and "keyData" is missing
// All three of "keyPath", "keyPaths" and "keyData" are missing
func(v mSI) { delete(v, "keyData") },
// Both "keyPath" and "keyData" is present
// All three of "keyPath", "keyPaths" and "keyData" are present
func(v mSI) { v["keyPath"] = "/foo/bar"; v["keyPaths"] = []string{"/1", "/2"} },
// Two of "keyPath", "keyPaths" and "keyData" are present
func(v mSI) { v["keyPath"] = "/foo/bar"; v["keyPaths"] = []string{"/1", "/2"}; delete(v, "keyData") },
func(v mSI) { v["keyPath"] = "/foo/bar" },
func(v mSI) { v["keyPaths"] = []string{"/1", "/2"} },
// Invalid "keyPath" field
func(v mSI) { delete(v, "keyData"); v["keyPath"] = 1 },
// Invalid "keyPaths" field
func(v mSI) { delete(v, "keyData"); v["keyPaths"] = 1 },
func(v mSI) { delete(v, "keyData"); v["keyPaths"] = []int{1} },
// Invalid "keyData" field
func(v mSI) { v["keyData"] = 1 },
func(v mSI) { v["keyData"] = "this is invalid base64" },
Expand All @@ -827,6 +879,17 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) {
},
duplicateFields: []string{"type", "keyType", "keyPath", "signedIdentity"},
}.run(t)
// Test the keyPaths-specific aspects
policyJSONUmarshallerTests{
newDest: func() json.Unmarshaler { return &prSignedBy{} },
newValidObject: func() (interface{}, error) {
return NewPRSignedByKeyPaths(SBKeyTypeGPGKeys, []string{"/1", "/2"}, NewPRMMatchRepoDigestOrExact())
},
otherJSONParser: func(validJSON []byte) (interface{}, error) {
return newPolicyRequirementFromJSON(validJSON)
},
duplicateFields: []string{"type", "keyType", "keyPaths", "signedIdentity"},
}.run(t)

var pr prSignedBy

Expand Down
21 changes: 16 additions & 5 deletions signature/policy_eval_signedby.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,37 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva
}

// FIXME: move this to per-context initialization
var data []byte
var data [][]byte
keySources := 0
if pr.KeyPath != "" {
keySources++
d, err := os.ReadFile(pr.KeyPath)
if err != nil {
return sarRejected, nil, err
}
data = d
data = [][]byte{d}
}
if pr.KeyPaths != nil {
keySources++
data = [][]byte{}
for _, path := range pr.KeyPaths {
d, err := os.ReadFile(path)
if err != nil {
return sarRejected, nil, err
}
data = append(data, d)
}
}
if pr.KeyData != nil {
keySources++
data = pr.KeyData
data = [][]byte{pr.KeyData}
}
if keySources != 1 {
return sarRejected, nil, errors.New(`Internal inconsistency: not exactly one of "keyPath" and "keyData" specified`)
return sarRejected, nil, errors.New(`Internal inconsistency: not exactly one of "keyPath", "keyPaths" and "keyData" specified`)
}

// FIXME: move this to per-context initialization
mech, trustedIdentities, err := NewEphemeralGPGSigningMechanism(data)
mech, trustedIdentities, err := newEphemeralGPGSigningMechanism(data)
if err != nil {
return sarRejected, nil, err
}
Expand Down
30 changes: 26 additions & 4 deletions signature/policy_eval_signedby_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,18 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
keyData, err := os.ReadFile("fixtures/public-key.gpg")
require.NoError(t, err)

// Successful validation, with KeyData and KeyPath
// Successful validation, with KeyPath, KeyPaths and KeyData.
for _, fn := range []func() (PolicyRequirement, error){
func() (PolicyRequirement, error) {
return NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
},
// Test the files in both orders, to make sure the correct public keys accepted in either position.
func() (PolicyRequirement, error) {
return NewPRSignedByKeyPaths(ktGPG, []string{"fixtures/public-key-1.gpg", "fixtures/public-key-1.gpg"}, prm)
},
func() (PolicyRequirement, error) {
return NewPRSignedByKeyPaths(ktGPG, []string{"fixtures/public-key-2.gpg", "fixtures/public-key-1.gpg"}, prm)
},
func() (PolicyRequirement, error) {
return NewPRSignedByKeyData(ktGPG, keyData, prm)
},
Expand Down Expand Up @@ -95,17 +102,32 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {

// Invalid KeyPath/KeyPaths/KeyData combinations.
for _, fn := range []func() (PolicyRequirement, error){
// Both KeyPath and KeyData set. Do not use NewPRSignedBy*, because it would reject this.
// Two or more of KeyPath, KeyPaths and KeyData set. Do not use NewPRSignedBy*, because it would reject this.
func() (PolicyRequirement, error) {
return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyPaths: []string{"fixtures/public-key-1.gpg", "fixtures/public-key-2.gpg"}, KeyData: keyData, SignedIdentity: prm}, nil
},
func() (PolicyRequirement, error) {
return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyData: []byte("abc"), SignedIdentity: prm}, nil
return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyPaths: []string{"fixtures/public-key-1.gpg", "fixtures/public-key-2.gpg"}, SignedIdentity: prm}, nil
},
// Neither KeyPath nor KeyData set. Do not use NewPRSignedBy*, because it would reject this.
func() (PolicyRequirement, error) {
return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyData: keyData, SignedIdentity: prm}, nil
},
func() (PolicyRequirement, error) {
return &prSignedBy{KeyType: ktGPG, KeyPaths: []string{"fixtures/public-key-1.gpg", "fixtures/public-key-2.gpg"}, KeyData: keyData, SignedIdentity: prm}, nil
},
// None of KeyPath, KeyPaths and KeyData set. Do not use NewPRSignedBy*, because it would reject this.
func() (PolicyRequirement, error) {
return &prSignedBy{KeyType: ktGPG, SignedIdentity: prm}, nil
},
func() (PolicyRequirement, error) { // Invalid KeyPath
return NewPRSignedByKeyPath(ktGPG, "/this/does/not/exist", prm)
},
func() (PolicyRequirement, error) { // Invalid KeyPaths
return NewPRSignedByKeyPaths(ktGPG, []string{"/this/does/not/exist"}, prm)
},
func() (PolicyRequirement, error) { // One of the KeyPaths is invalid
return NewPRSignedByKeyPaths(ktGPG, []string{"fixtures/public-key.gpg", "/this/does/not/exist"}, prm)
},
} {
pr, err := fn()
require.NoError(t, err)
Expand Down
Loading

0 comments on commit d218ff3

Please sign in to comment.