Update ComputedTrafficPermissions ACL hooks (#20622)

This commit is contained in:
Chris S. Kim 2024-02-13 15:16:54 -05:00 committed by GitHub
parent 64cd172f30
commit 248969c2a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 129 additions and 204 deletions

View File

@ -63,9 +63,15 @@ func validateComputedTrafficPermissions(res *DecodedComputedTrafficPermissions)
} }
func aclReadHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error { func aclReadHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error {
return authorizer.ToAllowAuthorizer().TrafficPermissionsReadAllowed(id.Name, authzContext) err := authorizer.ToAllowAuthorizer().TrafficPermissionsReadAllowed(id.Name, authzContext)
if err != nil {
// fallback to operator read
err = authorizer.ToAllowAuthorizer().OperatorReadAllowed(authzContext)
}
return err
} }
func aclWriteHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { func aclWriteHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error {
return authorizer.ToAllowAuthorizer().TrafficPermissionsWriteAllowed(res.Id.Name, authzContext) // Users should not be writing computed resources.
return authorizer.ToAllowAuthorizer().OperatorWriteAllowed(authzContext)
} }

View File

@ -8,12 +8,9 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/resourcetest" "github.com/hashicorp/consul/internal/resource/resourcetest"
pbauth "github.com/hashicorp/consul/proto-public/pbauth/v2beta1" pbauth "github.com/hashicorp/consul/proto-public/pbauth/v2beta1"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil"
) )
@ -50,127 +47,101 @@ func TestValidateComputedTrafficPermissions_Permissions(t *testing.T) {
} }
func TestComputedTrafficPermissionsACLs(t *testing.T) { func TestComputedTrafficPermissionsACLs(t *testing.T) {
// Wire up a registry to generically invoke hooks
registry := resource.NewRegistry() registry := resource.NewRegistry()
Register(registry) Register(registry)
type testcase struct { ctpData := &pbauth.ComputedTrafficPermissions{
rules string AllowPermissions: []*pbauth.Permission{},
check func(t *testing.T, authz acl.Authorizer, res *pbresource.Resource) DenyPermissions: []*pbauth.Permission{},
readOK string IsDefault: true,
writeOK string
listOK string
} }
const ( cases := map[string]resourcetest.ACLTestCase{
DENY = "deny"
ALLOW = "allow"
DEFAULT = "default"
)
checkF := func(t *testing.T, expect string, got error) {
switch expect {
case ALLOW:
if acl.IsErrPermissionDenied(got) {
t.Fatal("should be allowed")
}
case DENY:
if !acl.IsErrPermissionDenied(got) {
t.Fatal("should be denied")
}
case DEFAULT:
require.Nil(t, got, "expected fallthrough decision")
default:
t.Fatalf("unexpected expectation: %q", expect)
}
}
reg, ok := registry.Resolve(pbauth.ComputedTrafficPermissionsType)
require.True(t, ok)
run := func(t *testing.T, tc testcase) {
ctpData := &pbauth.ComputedTrafficPermissions{}
res := resourcetest.Resource(pbauth.ComputedTrafficPermissionsType, "wi1").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, ctpData).
Build()
resourcetest.ValidateAndNormalize(t, registry, res)
config := acl.Config{
WildcardName: structs.WildcardSpecifier,
}
authz, err := acl.NewAuthorizerFromRules(tc.rules, &config, nil)
require.NoError(t, err)
authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()})
t.Run("read", func(t *testing.T) {
err := reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, res)
checkF(t, tc.readOK, err)
})
t.Run("write", func(t *testing.T) {
err := reg.ACLs.Write(authz, &acl.AuthorizerContext{}, res)
checkF(t, tc.writeOK, err)
})
t.Run("list", func(t *testing.T) {
err := reg.ACLs.List(authz, &acl.AuthorizerContext{})
checkF(t, tc.listOK, err)
})
}
cases := map[string]testcase{
"no rules": { "no rules": {
rules: ``, Rules: ``,
readOK: DENY, Data: ctpData,
writeOK: DENY, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 read, no intentions": { "workload identity w1 read, no intentions": {
rules: `identity "wi1" { policy = "read" }`, Rules: `identity "test" { policy = "read" }`,
readOK: ALLOW, Data: ctpData,
writeOK: DENY, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 read, deny intentions": { "workload identity w1 read, deny intentions": {
rules: `identity "wi1" { policy = "read", intentions = "deny" }`, Rules: `identity "test" { policy = "read", intentions = "deny" }`,
readOK: DENY, Data: ctpData,
writeOK: DENY, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 read, intentions read": { "workload identity w1 read, intentions read": {
rules: `identity "wi1" { policy = "read", intentions = "read" }`, Rules: `identity "test" { policy = "read", intentions = "read" }`,
readOK: ALLOW, Data: ctpData,
writeOK: DENY, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 write, write intentions": { "workload identity w1 write, write intentions": {
rules: `identity "wi1" { policy = "read", intentions = "write" }`, Rules: `identity "test" { policy = "read", intentions = "write" }`,
readOK: ALLOW, Data: ctpData,
writeOK: ALLOW, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY, // users should not write computed resources
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 write, deny intentions": { "workload identity w1 write, deny intentions": {
rules: `identity "wi1" { policy = "write", intentions = "deny" }`, Rules: `identity "test" { policy = "write", intentions = "deny" }`,
readOK: DENY, Data: ctpData,
writeOK: DENY, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 write, intentions read": { "workload identity w1 write, intentions read": {
rules: `identity "wi1" { policy = "write", intentions = "read" }`, Rules: `identity "test" { policy = "write", intentions = "read" }`,
readOK: ALLOW, Data: ctpData,
writeOK: DENY, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity w1 write, intentions write": { "workload identity w1 write, intentions write": {
rules: `identity "wi1" { policy = "write", intentions = "write" }`, Rules: `identity "test" { policy = "write", intentions = "write" }`,
readOK: ALLOW, Data: ctpData,
writeOK: ALLOW, Typ: pbauth.ComputedTrafficPermissionsType,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY, // users should not write computed resources
ListOK: resourcetest.DEFAULT,
},
"operator read": {
Rules: `operator = "read"`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY, // users should not write computed resources
ListOK: resourcetest.DEFAULT,
},
"operator write": {
Rules: `operator = "write"`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
}, },
} }
for name, tc := range cases { for name, tc := range cases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
run(t, tc) resourcetest.RunACLTestCase(t, tc, registry)
}) })
} }
} }

View File

@ -8,139 +8,87 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/resourcetest" "github.com/hashicorp/consul/internal/resource/resourcetest"
pbauth "github.com/hashicorp/consul/proto-public/pbauth/v2beta1" pbauth "github.com/hashicorp/consul/proto-public/pbauth/v2beta1"
"github.com/hashicorp/consul/proto-public/pbresource"
) )
func TestWorkloadIdentityACLs(t *testing.T) { func TestWorkloadIdentityACLs(t *testing.T) {
const (
DENY = "deny"
ALLOW = "allow"
DEFAULT = "default"
)
registry := resource.NewRegistry() registry := resource.NewRegistry()
Register(registry) Register(registry)
reg, ok := registry.Resolve(pbauth.WorkloadIdentityType)
require.True(t, ok)
type testcase struct {
rules string
check func(t *testing.T, authz acl.Authorizer, res *pbresource.Resource)
readOK string
writeOK string
listOK string
}
checkF := func(t *testing.T, expect string, got error) {
switch expect {
case ALLOW:
if acl.IsErrPermissionDenied(got) {
t.Fatal("should be allowed")
}
case DENY:
if !acl.IsErrPermissionDenied(got) {
t.Fatal("should be denied")
}
case DEFAULT:
require.Nil(t, got, "expected fallthrough decision")
default:
t.Fatalf("unexpected expectation: %q", expect)
}
}
run := func(t *testing.T, tc testcase) {
wid := &pbauth.WorkloadIdentity{} wid := &pbauth.WorkloadIdentity{}
res := resourcetest.Resource(pbauth.WorkloadIdentityType, "wi1").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, wid).
Build()
resourcetest.ValidateAndNormalize(t, registry, res)
config := acl.Config{ cases := map[string]resourcetest.ACLTestCase{
WildcardName: structs.WildcardSpecifier,
}
authz, err := acl.NewAuthorizerFromRules(tc.rules, &config, nil)
require.NoError(t, err)
authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()})
t.Run("read", func(t *testing.T) {
err := reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, res)
checkF(t, tc.readOK, err)
})
t.Run("write", func(t *testing.T) {
err := reg.ACLs.Write(authz, &acl.AuthorizerContext{}, res)
checkF(t, tc.writeOK, err)
})
t.Run("list", func(t *testing.T) {
err := reg.ACLs.List(authz, &acl.AuthorizerContext{})
checkF(t, tc.listOK, err)
})
t.Run("errors", func(t *testing.T) {
require.ErrorIs(t, reg.ACLs.Read(authz, &acl.AuthorizerContext{}, nil, nil), resource.ErrNeedResource)
require.ErrorIs(t, reg.ACLs.Write(authz, &acl.AuthorizerContext{}, nil), resource.ErrNeedResource)
})
}
cases := map[string]testcase{
"no rules": { "no rules": {
rules: ``, Rules: ``,
readOK: DENY, Typ: pbauth.WorkloadIdentityType,
writeOK: DENY, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 read, no intentions": { "workload identity wi1 read, no intentions": {
rules: `identity "wi1" { policy = "read" }`, Rules: `identity "test" { policy = "read" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: DENY, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 read, deny intentions has no effect": { "workload identity wi1 read, deny intentions has no effect": {
rules: `identity "wi1" { policy = "read", intentions = "deny" }`, Rules: `identity "test" { policy = "read", intentions = "deny" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: DENY, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 read, intentions read has no effect": { "workload identity wi1 read, intentions read has no effect": {
rules: `identity "wi1" { policy = "read", intentions = "read" }`, Rules: `identity "test" { policy = "read", intentions = "read" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: DENY, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 write, write intentions has no effect": { "workload identity wi1 write, write intentions has no effect": {
rules: `identity "wi1" { policy = "read", intentions = "write" }`, Rules: `identity "test" { policy = "read", intentions = "write" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: DENY, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 write, deny intentions has no effect": { "workload identity wi1 write, deny intentions has no effect": {
rules: `identity "wi1" { policy = "write", intentions = "deny" }`, Rules: `identity "test" { policy = "write", intentions = "deny" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: ALLOW, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 write, intentions read has no effect": { "workload identity wi1 write, intentions read has no effect": {
rules: `identity "wi1" { policy = "write", intentions = "read" }`, Rules: `identity "test" { policy = "write", intentions = "read" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: ALLOW, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
}, },
"workload identity wi1 write, intentions write": { "workload identity wi1 write, intentions write": {
rules: `identity "wi1" { policy = "write", intentions = "write" }`, Rules: `identity "test" { policy = "write", intentions = "write" }`,
readOK: ALLOW, Typ: pbauth.WorkloadIdentityType,
writeOK: ALLOW, Data: wid,
listOK: DEFAULT, ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
}, },
} }
for name, tc := range cases { for name, tc := range cases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
run(t, tc) resourcetest.RunACLTestCase(t, tc, registry)
}) })
} }
} }