From b461c7f8d8832cb3886e0f39005b6858d93220f5 Mon Sep 17 00:00:00 2001 From: James Carnegie Date: Thu, 2 May 2024 11:36:29 +0100 Subject: [PATCH 1/3] Revert "revert: rego evaluator result" (#15) This reverts commit 0126ba9a0bb91b6532d5f43b45e7c47546792ddf. --- internal/test/test.go | 24 +++++++++++++++++++----- pkg/attest/verify.go | 5 ++++- pkg/attest/verify_test.go | 5 +++-- pkg/policy/evaluator.go | 3 ++- pkg/policy/policy_test.go | 5 +++-- pkg/policy/rego.go | 23 ++++++++--------------- 6 files changed, 39 insertions(+), 26 deletions(-) diff --git a/internal/test/test.go b/internal/test/test.go index 10baf71..220d18b 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -18,6 +18,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/partial" intoto "github.com/in-toto/in-toto-golang/in_toto" + "github.com/open-policy-agent/opa/rego" "github.com/secure-systems-lab/go-securesystemslib/dsse" ) @@ -87,20 +88,33 @@ func GetMockSigner(ctx context.Context) (dsse.SignerVerifier, error) { } type MockPolicyEvaluator struct { - EvaluateFunc func(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) error + EvaluateFunc func(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) } -func (pe *MockPolicyEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) error { +func (pe *MockPolicyEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) { if pe.EvaluateFunc != nil { return pe.EvaluateFunc(ctx, resolver, policy, input) } - return nil + return AllowedResult(), nil } func GetMockPolicy() policy.PolicyEvaluator { return &MockPolicyEvaluator{ - EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) error { - return nil + EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, pfs []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) { + return AllowedResult(), nil + }, + } +} + +func AllowedResult() *rego.ResultSet { + return ®o.ResultSet{ + { + Bindings: rego.Vars{}, + Expressions: []*rego.ExpressionValue{ + { + Value: true, + }, + }, }, } } diff --git a/pkg/attest/verify.go b/pkg/attest/verify.go index 52f05c6..0a963d1 100644 --- a/pkg/attest/verify.go +++ b/pkg/attest/verify.go @@ -31,10 +31,13 @@ func VerifyAttestations(ctx context.Context, resolver oci.AttestationResolver, f if err != nil { return err } - err = evaluator.Evaluate(ctx, resolver, files, input) + rs, err := evaluator.Evaluate(ctx, resolver, files, input) if err != nil { return fmt.Errorf("policy evaluation failed: %w", err) } + if !rs.Allowed() { + return fmt.Errorf("policy evaluation failed: %s", fmt.Sprint(rs)) + } return nil } diff --git a/pkg/attest/verify_test.go b/pkg/attest/verify_test.go index 5751da0..19e55ee 100644 --- a/pkg/attest/verify_test.go +++ b/pkg/attest/verify_test.go @@ -12,6 +12,7 @@ import ( "github.com/docker/attest/pkg/attestation" "github.com/docker/attest/pkg/oci" "github.com/docker/attest/pkg/policy" + "github.com/open-policy-agent/opa/rego" "github.com/stretchr/testify/assert" ) @@ -43,8 +44,8 @@ func TestVerifyAttestations(t *testing.T) { t.Run(tc.name, func(t *testing.T) { mockPE := test.MockPolicyEvaluator{ - EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, pfs []*policy.PolicyFile, input *policy.PolicyInput) error { - return tc.policyEvaluationError + EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, pfs []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) { + return test.AllowedResult(), tc.policyEvaluationError }, } diff --git a/pkg/policy/evaluator.go b/pkg/policy/evaluator.go index 35a34cb..d512e56 100644 --- a/pkg/policy/evaluator.go +++ b/pkg/policy/evaluator.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/docker/attest/pkg/oci" + "github.com/open-policy-agent/opa/rego" ) type policyEvaluatorCtxKeyType struct{} @@ -26,5 +27,5 @@ func GetPolicyEvaluator(ctx context.Context) (PolicyEvaluator, error) { } type PolicyEvaluator interface { - Evaluate(ctx context.Context, resolver oci.AttestationResolver, policy []*PolicyFile, input *PolicyInput) error + Evaluate(ctx context.Context, resolver oci.AttestationResolver, policy []*PolicyFile, input *PolicyInput) (*rego.ResultSet, error) } diff --git a/pkg/policy/policy_test.go b/pkg/policy/policy_test.go index 2665cf7..3740d36 100644 --- a/pkg/policy/policy_test.go +++ b/pkg/policy/policy_test.go @@ -97,11 +97,12 @@ func TestRegoEvaluator_Evaluate(t *testing.T) { policyFiles, err := policy.ResolvePolicy(ctx, tc.resolver, tc.policy) assert.NoErrorf(t, err, "failed to resolve policy") - err = re.Evaluate(ctx, tc.resolver, policyFiles, tc.input) + rs, err := re.Evaluate(ctx, tc.resolver, policyFiles, tc.input) + if tc.expectSuccess { assert.NoErrorf(t, err, "Evaluate failed") } else { - assert.Errorf(t, err, "Evaluate should have failed") + assert.False(t, rs.Allowed(), "Evaluate should have failed") } }) } diff --git a/pkg/policy/rego.go b/pkg/policy/rego.go index 78f3de3..48ca0f3 100644 --- a/pkg/policy/rego.go +++ b/pkg/policy/rego.go @@ -29,10 +29,11 @@ type regoEvaluator struct { func NewRegoEvaluator(debug bool) PolicyEvaluator { return ®oEvaluator{ debug: debug, + query: "data.attestations.allow", } } -func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationResolver, files []*PolicyFile, input *PolicyInput) error { +func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationResolver, files []*PolicyFile, input *PolicyInput) (*rego.ResultSet, error) { var regoOpts []func(*rego.Rego) // Create a new in-memory store @@ -41,7 +42,7 @@ func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationR params.Write = true txn, err := store.NewTransaction(ctx, params) if err != nil { - return err + return nil, err } for _, target := range files { @@ -49,11 +50,11 @@ func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationR if filepath.Ext(target.Path) == ".yaml" { yamlData, err := loadYAML(target.Path, target.Content) if err != nil { - return err + return nil, err } err = store.Write(ctx, txn, storage.AddOp, storage.Path{}, yamlData) if err != nil { - return err + return nil, err } } else { regoOpts = append(regoOpts, rego.Module(target.Path, string(target.Content))) @@ -63,7 +64,7 @@ func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationR err = store.Commit(ctx, txn) if err != nil { store.Abort(ctx, txn) - return err + return nil, err } if re.debug { @@ -75,7 +76,7 @@ func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationR } regoOpts = append(regoOpts, - rego.Query("data.docker.allow"), + rego.Query(re.query), rego.StrictBuiltinErrors(true), rego.Input(input), rego.Store(store), @@ -86,15 +87,7 @@ func (re *regoEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationR r := rego.New(regoOpts...) rs, err := r.Eval(ctx) - if err != nil { - return fmt.Errorf("error from Eval: %w", err) - } - - if !rs.Allowed() { - return fmt.Errorf("policy evaluation failed") - } - - return nil + return &rs, err } var dynamicObj = types.NewObject(nil, types.NewDynamicProperty(types.S, types.A)) From bc7139deaae6d7654f4e2d063709b9f9862b9347 Mon Sep 17 00:00:00 2001 From: James Carnegie Date: Thu, 2 May 2024 14:46:21 +0100 Subject: [PATCH 2/3] Move policy mock for external use (#16) --- internal/test/test.go | 36 +---------------------------------- pkg/attest/verify_test.go | 5 ++--- pkg/policy/mock.go | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 38 deletions(-) create mode 100644 pkg/policy/mock.go diff --git a/internal/test/test.go b/internal/test/test.go index 220d18b..73e26cc 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/docker/attest/pkg/attestation" - "github.com/docker/attest/pkg/oci" "github.com/docker/attest/pkg/policy" "github.com/docker/attest/pkg/signerverifier" "github.com/docker/attest/pkg/tlog" @@ -18,7 +17,6 @@ import ( "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/partial" intoto "github.com/in-toto/in-toto-golang/in_toto" - "github.com/open-policy-agent/opa/rego" "github.com/secure-systems-lab/go-securesystemslib/dsse" ) @@ -59,7 +57,7 @@ func Setup(t *testing.T) (context.Context, dsse.SignerVerifier) { var policyEvaluator policy.PolicyEvaluator if USE_MOCK_POLICY { - policyEvaluator = GetMockPolicy() + policyEvaluator = policy.GetMockPolicy() } else { policyEvaluator = policy.NewRegoEvaluator(true) } @@ -87,38 +85,6 @@ func GetMockSigner(ctx context.Context) (dsse.SignerVerifier, error) { return signerverifier.GenKeyPair() } -type MockPolicyEvaluator struct { - EvaluateFunc func(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) -} - -func (pe *MockPolicyEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationResolver, policy []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) { - if pe.EvaluateFunc != nil { - return pe.EvaluateFunc(ctx, resolver, policy, input) - } - return AllowedResult(), nil -} - -func GetMockPolicy() policy.PolicyEvaluator { - return &MockPolicyEvaluator{ - EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, pfs []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) { - return AllowedResult(), nil - }, - } -} - -func AllowedResult() *rego.ResultSet { - return ®o.ResultSet{ - { - Bindings: rego.Vars{}, - Expressions: []*rego.ExpressionValue{ - { - Value: true, - }, - }, - }, - } -} - type AnnotatedStatement struct { OCIDescriptor *v1.Descriptor InTotoStatement *intoto.Statement diff --git a/pkg/attest/verify_test.go b/pkg/attest/verify_test.go index 19e55ee..538081a 100644 --- a/pkg/attest/verify_test.go +++ b/pkg/attest/verify_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "testing" - "github.com/docker/attest/internal/test" "github.com/docker/attest/pkg/attestation" "github.com/docker/attest/pkg/oci" "github.com/docker/attest/pkg/policy" @@ -43,9 +42,9 @@ func TestVerifyAttestations(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - mockPE := test.MockPolicyEvaluator{ + mockPE := policy.MockPolicyEvaluator{ EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, pfs []*policy.PolicyFile, input *policy.PolicyInput) (*rego.ResultSet, error) { - return test.AllowedResult(), tc.policyEvaluationError + return policy.AllowedResult(), tc.policyEvaluationError }, } diff --git a/pkg/policy/mock.go b/pkg/policy/mock.go new file mode 100644 index 0000000..0f4e891 --- /dev/null +++ b/pkg/policy/mock.go @@ -0,0 +1,40 @@ +package policy + +import ( + "context" + + "github.com/docker/attest/pkg/oci" + "github.com/open-policy-agent/opa/rego" +) + +type MockPolicyEvaluator struct { + EvaluateFunc func(ctx context.Context, resolver oci.AttestationResolver, policy []*PolicyFile, input *PolicyInput) (*rego.ResultSet, error) +} + +func (pe *MockPolicyEvaluator) Evaluate(ctx context.Context, resolver oci.AttestationResolver, policy []*PolicyFile, input *PolicyInput) (*rego.ResultSet, error) { + if pe.EvaluateFunc != nil { + return pe.EvaluateFunc(ctx, resolver, policy, input) + } + return AllowedResult(), nil +} + +func GetMockPolicy() PolicyEvaluator { + return &MockPolicyEvaluator{ + EvaluateFunc: func(ctx context.Context, resolver oci.AttestationResolver, pfs []*PolicyFile, input *PolicyInput) (*rego.ResultSet, error) { + return AllowedResult(), nil + }, + } +} + +func AllowedResult() *rego.ResultSet { + return ®o.ResultSet{ + { + Bindings: rego.Vars{}, + Expressions: []*rego.ExpressionValue{ + { + Value: true, + }, + }, + }, + } +} From 0cadeefe6f9c2eb0d4ec333a72d1d09c648e8c68 Mon Sep 17 00:00:00 2001 From: James Carnegie Date: Thu, 2 May 2024 16:03:59 +0100 Subject: [PATCH 3/3] Fix query and tests (#17) --- pkg/policy/policy_test.go | 1 + pkg/policy/rego.go | 2 +- pkg/policy/testdata/mock-tuf-allow/doi/policy.rego | 2 +- pkg/policy/testdata/mock-tuf-deny/doi/policy.rego | 2 +- pkg/policy/testdata/mock-tuf-verify-sig/doi/policy.rego | 2 +- pkg/policy/testdata/mock-tuf-wrong-key/doi/policy.rego | 2 +- test/testdata/local-policy/doi/policy.rego | 2 +- test/testdata/local-policy/doi/policy_test.rego | 2 +- 8 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/policy/policy_test.go b/pkg/policy/policy_test.go index 3740d36..bf389fb 100644 --- a/pkg/policy/policy_test.go +++ b/pkg/policy/policy_test.go @@ -101,6 +101,7 @@ func TestRegoEvaluator_Evaluate(t *testing.T) { if tc.expectSuccess { assert.NoErrorf(t, err, "Evaluate failed") + assert.True(t, rs.Allowed(), "Evaluate should have succeeded") } else { assert.False(t, rs.Allowed(), "Evaluate should have failed") } diff --git a/pkg/policy/rego.go b/pkg/policy/rego.go index 48ca0f3..c6851b0 100644 --- a/pkg/policy/rego.go +++ b/pkg/policy/rego.go @@ -29,7 +29,7 @@ type regoEvaluator struct { func NewRegoEvaluator(debug bool) PolicyEvaluator { return ®oEvaluator{ debug: debug, - query: "data.attestations.allow", + query: "data.attest.allow", } } diff --git a/pkg/policy/testdata/mock-tuf-allow/doi/policy.rego b/pkg/policy/testdata/mock-tuf-allow/doi/policy.rego index 5e3b8ab..b1b188a 100644 --- a/pkg/policy/testdata/mock-tuf-allow/doi/policy.rego +++ b/pkg/policy/testdata/mock-tuf-allow/doi/policy.rego @@ -1,4 +1,4 @@ -package docker +package attest import rego.v1 diff --git a/pkg/policy/testdata/mock-tuf-deny/doi/policy.rego b/pkg/policy/testdata/mock-tuf-deny/doi/policy.rego index ad73140..5e4ac7e 100644 --- a/pkg/policy/testdata/mock-tuf-deny/doi/policy.rego +++ b/pkg/policy/testdata/mock-tuf-deny/doi/policy.rego @@ -1,4 +1,4 @@ -package docker +package attest import rego.v1 diff --git a/pkg/policy/testdata/mock-tuf-verify-sig/doi/policy.rego b/pkg/policy/testdata/mock-tuf-verify-sig/doi/policy.rego index e2456af..8952fb1 100644 --- a/pkg/policy/testdata/mock-tuf-verify-sig/doi/policy.rego +++ b/pkg/policy/testdata/mock-tuf-verify-sig/doi/policy.rego @@ -1,4 +1,4 @@ -package docker +package attest import rego.v1 diff --git a/pkg/policy/testdata/mock-tuf-wrong-key/doi/policy.rego b/pkg/policy/testdata/mock-tuf-wrong-key/doi/policy.rego index 131129a..aaa78d7 100644 --- a/pkg/policy/testdata/mock-tuf-wrong-key/doi/policy.rego +++ b/pkg/policy/testdata/mock-tuf-wrong-key/doi/policy.rego @@ -1,4 +1,4 @@ -package docker +package attest import rego.v1 diff --git a/test/testdata/local-policy/doi/policy.rego b/test/testdata/local-policy/doi/policy.rego index ed35cbe..d4ec593 100644 --- a/test/testdata/local-policy/doi/policy.rego +++ b/test/testdata/local-policy/doi/policy.rego @@ -1,4 +1,4 @@ -package docker +package attest import rego.v1 diff --git a/test/testdata/local-policy/doi/policy_test.rego b/test/testdata/local-policy/doi/policy_test.rego index fa81aa9..e2bff5c 100644 --- a/test/testdata/local-policy/doi/policy_test.rego +++ b/test/testdata/local-policy/doi/policy_test.rego @@ -1,4 +1,4 @@ -package docker +package attest import rego.v1 config := {"keys": []}