diff --git a/signature/fixtures/dir-img-manifest-digest-error/manifest.json b/signature/fixtures/dir-img-manifest-digest-error/manifest.json new file mode 120000 index 00000000..3dee14b4 --- /dev/null +++ b/signature/fixtures/dir-img-manifest-digest-error/manifest.json @@ -0,0 +1 @@ +../v2s1-invalid-signatures.manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-manifest-digest-error/signature-1 b/signature/fixtures/dir-img-manifest-digest-error/signature-1 new file mode 120000 index 00000000..f010fd4c --- /dev/null +++ b/signature/fixtures/dir-img-manifest-digest-error/signature-1 @@ -0,0 +1 @@ +../dir-img-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-mixed/manifest.json b/signature/fixtures/dir-img-mixed/manifest.json new file mode 120000 index 00000000..ff7d2ffa --- /dev/null +++ b/signature/fixtures/dir-img-mixed/manifest.json @@ -0,0 +1 @@ +../dir-img-valid/manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-mixed/signature-1 b/signature/fixtures/dir-img-mixed/signature-1 new file mode 120000 index 00000000..b27cdc45 --- /dev/null +++ b/signature/fixtures/dir-img-mixed/signature-1 @@ -0,0 +1 @@ +../invalid-blob.signature \ No newline at end of file diff --git a/signature/fixtures/dir-img-mixed/signature-2 b/signature/fixtures/dir-img-mixed/signature-2 new file mode 120000 index 00000000..f010fd4c --- /dev/null +++ b/signature/fixtures/dir-img-mixed/signature-2 @@ -0,0 +1 @@ +../dir-img-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-modified-manifest/manifest.json b/signature/fixtures/dir-img-modified-manifest/manifest.json new file mode 100644 index 00000000..82fde381 --- /dev/null +++ b/signature/fixtures/dir-img-modified-manifest/manifest.json @@ -0,0 +1,27 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "config": { + "mediaType": "application/vnd.docker.container.image.v1+json", + "size": 7023, + "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7" + }, + "layers": [ + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 32654, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f" + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 16724, + "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b" + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736" + } + ], + "extra": "this manifest has been modified" +} diff --git a/signature/fixtures/dir-img-modified-manifest/signature-1 b/signature/fixtures/dir-img-modified-manifest/signature-1 new file mode 120000 index 00000000..f010fd4c --- /dev/null +++ b/signature/fixtures/dir-img-modified-manifest/signature-1 @@ -0,0 +1 @@ +../dir-img-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-no-manifest/signature-1 b/signature/fixtures/dir-img-no-manifest/signature-1 new file mode 120000 index 00000000..f010fd4c --- /dev/null +++ b/signature/fixtures/dir-img-no-manifest/signature-1 @@ -0,0 +1 @@ +../dir-img-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-unsigned/manifest.json b/signature/fixtures/dir-img-unsigned/manifest.json new file mode 120000 index 00000000..ff7d2ffa --- /dev/null +++ b/signature/fixtures/dir-img-unsigned/manifest.json @@ -0,0 +1 @@ +../dir-img-valid/manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-valid-2/manifest.json b/signature/fixtures/dir-img-valid-2/manifest.json new file mode 120000 index 00000000..ff7d2ffa --- /dev/null +++ b/signature/fixtures/dir-img-valid-2/manifest.json @@ -0,0 +1 @@ +../dir-img-valid/manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-valid-2/signature-1 b/signature/fixtures/dir-img-valid-2/signature-1 new file mode 120000 index 00000000..f010fd4c --- /dev/null +++ b/signature/fixtures/dir-img-valid-2/signature-1 @@ -0,0 +1 @@ +../dir-img-valid/signature-1 \ No newline at end of file diff --git a/signature/fixtures/dir-img-valid-2/signature-2 b/signature/fixtures/dir-img-valid-2/signature-2 new file mode 100644 index 00000000..dbba8f42 Binary files /dev/null and b/signature/fixtures/dir-img-valid-2/signature-2 differ diff --git a/signature/fixtures/dir-img-valid/manifest.json b/signature/fixtures/dir-img-valid/manifest.json new file mode 120000 index 00000000..c5bd2543 --- /dev/null +++ b/signature/fixtures/dir-img-valid/manifest.json @@ -0,0 +1 @@ +../image.manifest.json \ No newline at end of file diff --git a/signature/fixtures/dir-img-valid/signature-1 b/signature/fixtures/dir-img-valid/signature-1 new file mode 100644 index 00000000..d0e18720 Binary files /dev/null and b/signature/fixtures/dir-img-valid/signature-1 differ diff --git a/signature/policy_eval.go b/signature/policy_eval.go index ec9d5c0e..a6a863a3 100644 --- a/signature/policy_eval.go +++ b/signature/policy_eval.go @@ -2,6 +2,56 @@ package signature import "github.com/projectatomic/skopeo/types" +// PolicyRequirementError is an explanatory text for rejecting a signature or an image. +type PolicyRequirementError string + +func (err PolicyRequirementError) Error() string { + return string(err) +} + +// signatureAcceptanceResult is the principal value returned by isSignatureAuthorAccepted. +type signatureAcceptanceResult string + +const ( + sarAccepted signatureAcceptanceResult = "sarAccepted" + sarRejected signatureAcceptanceResult = "sarRejected" + sarUnknown signatureAcceptanceResult = "sarUnknown" +) + +// PolicyRequirement is a rule which must be satisfied by at least one of the signatures of an image. +// The type is public, but its definition is private. +type PolicyRequirement interface { + // FIXME: For speed, we should support creating per-context state (not stored in the PolicyRequirement), to cache + // costly initialization like creating temporary GPG home directories and reading files. + // Setup() (someState, error) + // Then, the operations below would be done on the someState object, not directly on a PolicyRequirement. + + // isSignatureAuthorAccepted, given an image and a signature blob, returns: + // - sarAccepted if the signature has been verified against the appropriate public key + // (where "appropriate public key" may depend on the contents of the signature); + // in that case a parsed Signature should be returned. + // - sarRejected if the signature has not been verified; + // in that case error must be non-nil, and should be an PolicyRequirementError if evaluation + // succeeded but the result was rejection. + // - sarUnknown if if this PolicyRequirement does not deal with signatures. + // NOTE: sarUnknown should not be returned if this PolicyRequirement should make a decision but something failed. + // Returning sarUnknown and a non-nil error value is invalid. + // WARNING: This makes the signature contents acceptable for futher processing, + // but it does not necessarily mean that the contents of the signature are + // consistent with local policy. + // For example: + // - Do not use a true value to determine whether to run + // a container based on this image; use IsRunningImageAllowed instead. + // - Just because a signature is accepted does not automatically mean the contents of the + // signature are authorized to run code as root, or to affect system or cluster configuration. + isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) + + // isRunningImageAllowed returns true if the requirement allows running an image. + // If it returns false, err must be non-nil, and should be an PolicyRequirementError if evaluation + // succeeded but the result was rejection. + isRunningImageAllowed(image types.Image) (bool, error) +} + // PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement. // The type is public, but its implementation is private. type PolicyReferenceMatch interface { diff --git a/signature/policy_eval_baselayer.go b/signature/policy_eval_baselayer.go new file mode 100644 index 00000000..61b62e08 --- /dev/null +++ b/signature/policy_eval_baselayer.go @@ -0,0 +1,18 @@ +// Policy evaluation for prSignedBaseLayer. + +package signature + +import ( + "github.com/Sirupsen/logrus" + "github.com/projectatomic/skopeo/types" +) + +func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { + return sarUnknown, nil, nil +} + +func (pr *prSignedBaseLayer) isRunningImageAllowed(image types.Image) (bool, error) { + // FIXME? Reject this at policy parsing time already? + logrus.Errorf("signedBaseLayer not implemented yet!") + return false, PolicyRequirementError("signedBaseLayer not implemented yet!") +} diff --git a/signature/policy_eval_baselayer_test.go b/signature/policy_eval_baselayer_test.go new file mode 100644 index 00000000..937cb928 --- /dev/null +++ b/signature/policy_eval_baselayer_test.go @@ -0,0 +1,24 @@ +package signature + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPRSignedBaseLayerIsSignatureAuthorAccepted(t *testing.T) { + pr, err := NewPRSignedBaseLayer(NewPRMMatchRepository()) + require.NoError(t, err) + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil) + assertSARUnknown(t, sar, parsedSig, err) +} + +func TestPRSignedBaseLayerIsRunningImageAllowed(t *testing.T) { + // This will obviously need to change after signedBaseLayer is implemented. + pr, err := NewPRSignedBaseLayer(NewPRMMatchRepository()) + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + res, err := pr.isRunningImageAllowed(nil) + assertRunningRejectedPolicyRequirement(t, res, err) +} diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go new file mode 100644 index 00000000..c8018e8a --- /dev/null +++ b/signature/policy_eval_signedby.go @@ -0,0 +1,137 @@ +// Policy evaluation for prSignedBy. + +package signature + +import ( + "errors" + "fmt" + "io/ioutil" + "os" + "strings" + + "github.com/projectatomic/skopeo/docker/utils" + "github.com/projectatomic/skopeo/types" +) + +func (pr *prSignedBy) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { + switch pr.KeyType { + case SBKeyTypeGPGKeys: + case SBKeyTypeSignedByGPGKeys, SBKeyTypeX509Certificates, SBKeyTypeSignedByX509CAs: + // FIXME? Reject this at policy parsing time already? + return sarRejected, nil, fmt.Errorf(`"Unimplemented "keyType" value "%s"`, string(pr.KeyType)) + default: + // This should never happen, newPRSignedBy ensures KeyType.IsValid() + return sarRejected, nil, fmt.Errorf(`"Unknown "keyType" value "%s"`, string(pr.KeyType)) + } + + if pr.KeyPath != "" && pr.KeyData != nil { + return sarRejected, nil, errors.New(`Internal inconsistency: both "keyPath" and "keyData" specified`) + } + // FIXME: move this to per-context initialization + var data []byte + if pr.KeyData != nil { + data = pr.KeyData + } else { + d, err := ioutil.ReadFile(pr.KeyPath) + if err != nil { + return sarRejected, nil, err + } + data = d + } + + // FIXME: move this to per-context initialization + dir, err := ioutil.TempDir("", "skopeo-signedBy-") + if err != nil { + return sarRejected, nil, err + } + defer os.RemoveAll(dir) + mech, err := newGPGSigningMechanismInDirectory(dir) + if err != nil { + return sarRejected, nil, err + } + + trustedIdentities, err := mech.ImportKeysFromBytes(data) + if err != nil { + return sarRejected, nil, err + } + if len(trustedIdentities) == 0 { + return sarRejected, nil, PolicyRequirementError("No public keys imported") + } + + signature, err := verifyAndExtractSignature(mech, sig, signatureAcceptanceRules{ + validateKeyIdentity: func(keyIdentity string) error { + for _, trustedIdentity := range trustedIdentities { + if keyIdentity == trustedIdentity { + return nil + } + } + // Coverage: We use a private GPG home directory and only import trusted keys, so this should + // not be reachable. + return PolicyRequirementError(fmt.Sprintf("Signature by key %s is not accepted", keyIdentity)) + }, + validateSignedDockerReference: func(ref string) error { + if !pr.SignedIdentity.matchesDockerReference(image, ref) { + return PolicyRequirementError(fmt.Sprintf("Signature for identity %s is not accepted", ref)) + } + return nil + }, + validateSignedDockerManifestDigest: func(digest string) error { + manifest, err := image.Manifest() + if err != nil { + return err + } + digestMatches, err := utils.ManifestMatchesDigest(manifest, digest) + if err != nil { + return err + } + if !digestMatches { + return PolicyRequirementError(fmt.Sprintf("Signature for digest %s does not match", digest)) + } + return nil + }, + }) + if err != nil { + return sarRejected, nil, err + } + + return sarAccepted, signature, nil +} + +func (pr *prSignedBy) isRunningImageAllowed(image types.Image) (bool, error) { + sigs, err := image.Signatures() + if err != nil { + return false, err + } + var rejections []error + for _, s := range sigs { + var reason error + switch res, _, err := pr.isSignatureAuthorAccepted(image, s); res { + case sarAccepted: + // One accepted signature is enough. + return true, nil + case sarRejected: + reason = err + case sarUnknown: + // Huh?! This should not happen at all; treat it as any other invalid value. + fallthrough + default: + reason = fmt.Errorf(`Internal error: Unexpected signature verification result "%s"`, string(res)) + } + rejections = append(rejections, reason) + } + var summary error + switch len(rejections) { + case 0: + summary = PolicyRequirementError("A signature was required, but no signature exists") + case 1: + summary = rejections[0] + default: + var msgs []string + for _, e := range rejections { + msgs = append(msgs, e.Error()) + } + summary = PolicyRequirementError(fmt.Sprintf("None of the signatures were accepted, reasons: %s", + strings.Join(msgs, "; "))) + } + return false, summary +} diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go new file mode 100644 index 00000000..b3cd026f --- /dev/null +++ b/signature/policy_eval_signedby_test.go @@ -0,0 +1,239 @@ +package signature + +import ( + "io/ioutil" + "os" + "path" + "testing" + + "github.com/projectatomic/skopeo/directory" + "github.com/projectatomic/skopeo/docker" + "github.com/projectatomic/skopeo/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// dirImageMock returns a types.Image for a directory, claiming a specified intendedDockerReference. +func dirImageMock(dir, intendedDockerReference string) types.Image { + return docker.GenericImageFromSource(&dirImageSourceMock{ + ImageSource: directory.NewDirImageSource(dir), + intendedDockerReference: intendedDockerReference, + }) +} + +// dirImageSourceMock inherits dirImageSource, but overrides its IntendedDockerReference method. +type dirImageSourceMock struct { + types.ImageSource + intendedDockerReference string +} + +func (d *dirImageSourceMock) IntendedDockerReference() string { + return d.intendedDockerReference +} + +func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { + ktGPG := SBKeyTypeGPGKeys + prm := NewPRMMatchExact() + testImage := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest") + testImageSig, err := ioutil.ReadFile("fixtures/dir-img-valid/signature-1") + require.NoError(t, err) + + // Successful validation, with KeyData and KeyPath + pr, err := NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + sar, parsedSig, err := pr.isSignatureAuthorAccepted(testImage, testImageSig) + assertSARAccepted(t, sar, parsedSig, err, Signature{ + DockerManifestDigest: TestImageManifestDigest, + DockerReference: "testing/manifest:latest", + }) + + keyData, err := ioutil.ReadFile("fixtures/public-key.gpg") + require.NoError(t, err) + pr, err = NewPRSignedByKeyData(ktGPG, keyData, prm) + require.NoError(t, err) + sar, parsedSig, err = pr.isSignatureAuthorAccepted(testImage, testImageSig) + assertSARAccepted(t, sar, parsedSig, err, Signature{ + DockerManifestDigest: TestImageManifestDigest, + DockerReference: "testing/manifest:latest", + }) + + // Unimplemented and invalid KeyType values + for _, keyType := range []sbKeyType{SBKeyTypeSignedByGPGKeys, + SBKeyTypeX509Certificates, + SBKeyTypeSignedByX509CAs, + sbKeyType("This is invalid"), + } { + // Do not use NewPRSignedByKeyData, because it would reject invalid values. + pr := &prSignedBy{ + KeyType: keyType, + KeyData: []byte("abc"), + SignedIdentity: prm, + } + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil) + assertSARRejected(t, sar, parsedSig, err) + } + + // Both KeyPath and KeyData set. Do not use NewPRSignedBy*, because it would reject this. + prSB := &prSignedBy{ + KeyType: ktGPG, + KeyPath: "/foo/bar", + KeyData: []byte("abc"), + SignedIdentity: prm, + } + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err = prSB.isSignatureAuthorAccepted(nil, nil) + assertSARRejected(t, sar, parsedSig, err) + + // Invalid KeyPath + pr, err = NewPRSignedByKeyPath(ktGPG, "/this/does/not/exist", prm) + require.NoError(t, err) + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, nil) + assertSARRejected(t, sar, parsedSig, err) + + // Errors initializing the temporary GPG directory and mechanism are not obviously easy to reach. + + // KeyData has no public keys. + pr, err = NewPRSignedByKeyData(ktGPG, []byte{}, prm) + require.NoError(t, err) + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, nil) + assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) + + // A signature which does not GPG verify + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image parmater.. + sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, []byte("invalid signature")) + assertSARRejected(t, sar, parsedSig, err) + + // A valid signature using an unknown key. + // (This is (currently?) rejected through the "mech.Verify fails" path, not the "!identityFound" path, + // because we use a temporary directory and only import the trusted keys.) + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + sig, err := ioutil.ReadFile("fixtures/unknown-key.signature") + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image parmater.. + sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, sig) + assertSARRejected(t, sar, parsedSig, err) + + // A valid signature of an invalid JSON. + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + sig, err = ioutil.ReadFile("fixtures/invalid-blob.signature") + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image parmater.. + sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, sig) + assertSARRejected(t, sar, parsedSig, err) + assert.IsType(t, InvalidSignatureError{}, err) + + // A valid signature with a rejected identity. + nonmatchingPRM, err := NewPRMExactReference("this/doesnt:match") + require.NoError(t, err) + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", nonmatchingPRM) + require.NoError(t, err) + sar, parsedSig, err = pr.isSignatureAuthorAccepted(testImage, testImageSig) + assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) + + // Error reading image manifest + image := dirImageMock("fixtures/dir-img-no-manifest", "testing/manifest:latest") + sig, err = ioutil.ReadFile("fixtures/dir-img-no-manifest/signature-1") + require.NoError(t, err) + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + sar, parsedSig, err = pr.isSignatureAuthorAccepted(image, sig) + assertSARRejected(t, sar, parsedSig, err) + + // Error computing manifest digest + image = dirImageMock("fixtures/dir-img-manifest-digest-error", "testing/manifest:latest") + sig, err = ioutil.ReadFile("fixtures/dir-img-manifest-digest-error/signature-1") + require.NoError(t, err) + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + sar, parsedSig, err = pr.isSignatureAuthorAccepted(image, sig) + assertSARRejected(t, sar, parsedSig, err) + + // A valid signature with a non-matching manifest + image = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest") + sig, err = ioutil.ReadFile("fixtures/dir-img-modified-manifest/signature-1") + require.NoError(t, err) + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + sar, parsedSig, err = pr.isSignatureAuthorAccepted(image, sig) + assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) +} + +// createInvalidSigDir creates a directory suitable for dirImageMock, in which image.Signatures() +// fails. +// The caller should eventually call os.RemoveAll on the returned path. +func createInvalidSigDir(t *testing.T) string { + dir, err := ioutil.TempDir("", "skopeo-test-unreadable-signature") + require.NoError(t, err) + err = ioutil.WriteFile(path.Join(dir, "manifest.json"), []byte("{}"), 0644) + require.NoError(t, err) + // Creating a 000-permissions file would work for unprivileged accounts, but root (in particular, + // in the Docker container we use for testing) would still have access. So, create a symlink + // pointing to itself, to cause an ELOOP. (Note that a symlink pointing to a nonexistent file would be treated + // just like a nonexistent signature file, and not an error.) + err = os.Symlink("signature-1", path.Join(dir, "signature-1")) + require.NoError(t, err) + return dir +} + +func TestPRSignedByIsRunningImageAllowed(t *testing.T) { + ktGPG := SBKeyTypeGPGKeys + prm := NewPRMMatchExact() + + // A simple success case: single valid signature. + image := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest") + pr, err := NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err := pr.isRunningImageAllowed(image) + assertRunningAllowed(t, allowed, err) + + // Error reading signatures + invalidSigDir := createInvalidSigDir(t) + defer os.RemoveAll(invalidSigDir) + image = dirImageMock(invalidSigDir, "testing/manifest:latest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(image) + assertRunningRejected(t, allowed, err) + + // No signatures + image = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(image) + assertRunningRejectedPolicyRequirement(t, allowed, err) + + // 1 invalid signature: use dir-img-valid, but a non-matching Docker reference + image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:notlatest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(image) + assertRunningRejectedPolicyRequirement(t, allowed, err) + + // 2 valid signatures + image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(image) + assertRunningAllowed(t, allowed, err) + + // One invalid, one valid signature (in this order) + image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(image) + assertRunningAllowed(t, allowed, err) + + // 2 invalid signatures: use dir-img-valid-2, but a non-matching Docker reference + image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:notlatest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(image) + assertRunningRejectedPolicyRequirement(t, allowed, err) +} diff --git a/signature/policy_eval_simple.go b/signature/policy_eval_simple.go new file mode 100644 index 00000000..53530353 --- /dev/null +++ b/signature/policy_eval_simple.go @@ -0,0 +1,25 @@ +// Policy evaluation for the various simple PolicyRequirement types. + +package signature + +import "github.com/projectatomic/skopeo/types" + +func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { + // prInsecureAcceptAnything semantics: Every image is allowed to run, + // but this does not consider the signature as verified. + return sarUnknown, nil, nil +} + +func (pr *prInsecureAcceptAnything) isRunningImageAllowed(image types.Image) (bool, error) { + return true, nil +} + +func (pr *prReject) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { + // FIXME? Name the image, or better the matched scope in Policy.Specific. + return sarRejected, nil, PolicyRequirementError("Any signatures for these images are rejected by policy.") +} + +func (pr *prReject) isRunningImageAllowed(image types.Image) (bool, error) { + // FIXME? Name the image, or better the matched scope in Policy.Specific. + return false, PolicyRequirementError("Running these images is rejected by policy.") +} diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go new file mode 100644 index 00000000..c4434b10 --- /dev/null +++ b/signature/policy_eval_simple_test.go @@ -0,0 +1,32 @@ +package signature + +import "testing" + +func TestPRInsecureAcceptAnythingIsSignatureAuthorAccepted(t *testing.T) { + pr := NewPRInsecureAcceptAnything() + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil) + assertSARUnknown(t, sar, parsedSig, err) +} + +func TestPRInsecureAcceptAnythingIsRunningImageAllowed(t *testing.T) { + pr := NewPRInsecureAcceptAnything() + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + res, err := pr.isRunningImageAllowed(nil) + assertRunningAllowed(t, res, err) +} + +func TestPRRejectIsSignatureAuthorAccepted(t *testing.T) { + pr := NewPRReject() + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil) + assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) +} + +func TestPRRejectIsRunningImageAllowed(t *testing.T) { + // This will obviously need to change after this is implemented. + pr := NewPRReject() + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + res, err := pr.isRunningImageAllowed(nil) + assertRunningRejectedPolicyRequirement(t, res, err) +} diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go new file mode 100644 index 00000000..4e1e0ca2 --- /dev/null +++ b/signature/policy_eval_test.go @@ -0,0 +1,59 @@ +package signature + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Helpers for validating PolicyRequirement.isSignatureAuthorAccepted results: + +// assertSARRejected verifies that isSignatureAuthorAccepted returns a consistent sarRejected result +// with the expected signature. +func assertSARAccepted(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error, expectedSig Signature) { + assert.Equal(t, sarAccepted, sar) + assert.Equal(t, &expectedSig, parsedSig) + assert.NoError(t, err) +} + +// assertSARRejected verifies that isSignatureAuthorAccepted returns a consistent sarRejected result. +func assertSARRejected(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error) { + assert.Equal(t, sarRejected, sar) + assert.Nil(t, parsedSig) + assert.Error(t, err) +} + +// assertSARRejectedPolicyRequiremnt verifies that isSignatureAuthorAccepted returns a consistent sarRejected resul, +// and that the returned error is a PolicyRequirementError.. +func assertSARRejectedPolicyRequirement(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error) { + assertSARRejected(t, sar, parsedSig, err) + assert.IsType(t, PolicyRequirementError(""), err) +} + +// assertSARRejected verifies that isSignatureAuthorAccepted returns a consistent sarUnknown result. +func assertSARUnknown(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error) { + assert.Equal(t, sarUnknown, sar) + assert.Nil(t, parsedSig) + assert.NoError(t, err) +} + +// Helpers for validating PolicyRequirement.isRunningImageAllowed results: + +// assertRunningAllowed verifies that isRunningImageAllowed returns a consistent true result +func assertRunningAllowed(t *testing.T, allowed bool, err error) { + assert.Equal(t, true, allowed) + assert.NoError(t, err) +} + +// assertRunningRejected verifies that isRunningImageAllowed returns a consistent false result +func assertRunningRejected(t *testing.T, allowed bool, err error) { + assert.Equal(t, false, allowed) + assert.Error(t, err) +} + +// assertRunningRejectedPolicyRequirement verifies that isRunningImageAllowed returns a consistent false result +// and that the returned error is a PolicyRequirementError. +func assertRunningRejectedPolicyRequirement(t *testing.T, allowed bool, err error) { + assertRunningRejected(t, allowed, err) + assert.IsType(t, PolicyRequirementError(""), err) +} diff --git a/signature/policy_types.go b/signature/policy_types.go index 3a231669..8e614cce 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -26,7 +26,6 @@ type PolicyRequirements []PolicyRequirement // PolicyRequirement is a rule which must be satisfied by at least one of the signatures of an image. // The type is public, but its definition is private. -type PolicyRequirement interface{} // Will be expanded and moved elsewhere later. // prCommon is the common type field in a JSON encoding of PolicyRequirement. type prCommon struct {