Skip to content

Commit

Permalink
mesh,catalog: add missing ACL hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
ishustava committed Oct 12, 2023
1 parent 7ab6bd9 commit c79f2a8
Show file tree
Hide file tree
Showing 43 changed files with 959 additions and 167 deletions.
8 changes: 1 addition & 7 deletions internal/auth/internal/types/computed_traffic_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func RegisterComputedTrafficPermission(r resource.Registry) {
ACLs: &resource.ACLHooks{
Read: aclReadHookComputedTrafficPermissions,
Write: aclWriteHookComputedTrafficPermissions,
List: aclListHookComputedTrafficPermissions,
List: resource.NoOpACLListHook,
},
Validate: ValidateComputedTrafficPermissions,
Scope: resource.ScopeNamespace,
Expand Down Expand Up @@ -71,9 +71,3 @@ func aclReadHookComputedTrafficPermissions(authorizer acl.Authorizer, authzConte
func aclWriteHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error {
return authorizer.ToAllowAuthorizer().TrafficPermissionsWriteAllowed(res.Id.Name, authzContext)
}

func aclListHookComputedTrafficPermissions(_ acl.Authorizer, _ *acl.AuthorizerContext) error {
// No-op List permission as we want to default to filtering resources
// from the list using the Read enforcement
return nil
}
8 changes: 1 addition & 7 deletions internal/auth/internal/types/traffic_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func RegisterTrafficPermissions(r resource.Registry) {
ACLs: &resource.ACLHooks{
Read: aclReadHookTrafficPermissions,
Write: aclWriteHookTrafficPermissions,
List: aclListHookTrafficPermissions,
List: resource.NoOpACLListHook,
},
Validate: ValidateTrafficPermissions,
Mutate: MutateTrafficPermissions,
Expand Down Expand Up @@ -286,12 +286,6 @@ func aclWriteHookTrafficPermissions(authorizer acl.Authorizer, authzContext *acl
})
}

func aclListHookTrafficPermissions(_ acl.Authorizer, _ *acl.AuthorizerContext) error {
// No-op List permission as we want to default to filtering resources
// from the list using the Read enforcement
return nil
}

func authorizeDestination(res *pbresource.Resource, intentionAllowed func(string) error) error {
tp, err := resource.Decode[*pbauth.TrafficPermissions](res)
if err != nil {
Expand Down
8 changes: 1 addition & 7 deletions internal/auth/internal/types/workload_identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func RegisterWorkloadIdentity(r resource.Registry) {
ACLs: &resource.ACLHooks{
Read: aclReadHookWorkloadIdentity,
Write: aclWriteHookWorkloadIdentity,
List: aclListHookWorkloadIdentity,
List: resource.NoOpACLListHook,
},
Validate: nil,
})
Expand Down Expand Up @@ -48,9 +48,3 @@ func aclWriteHookWorkloadIdentity(
}
return authorizer.ToAllowAuthorizer().IdentityWriteAllowed(res.Id.Name, authzCtx)
}

func aclListHookWorkloadIdentity(authorizer acl.Authorizer, context *acl.AuthorizerContext) error {
// No-op List permission as we want to default to filtering resources
// from the list using the Read enforcement
return nil
}
21 changes: 21 additions & 0 deletions internal/catalog/catalogtest/helpers/acl_hooks_test_helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package helpers

import (
"testing"

"github.com/hashicorp/consul/internal/catalog"
"github.com/hashicorp/consul/internal/catalog/internal/testhelpers"
"github.com/hashicorp/consul/internal/resource"
pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1"
"github.com/hashicorp/consul/proto-public/pbresource"
)

func RunWorkloadSelectingTypeACLsTests[T catalog.WorkloadSelecting](t *testing.T, typ *pbresource.Type,
getData func(selector *pbcatalog.WorkloadSelector) T,
registerFunc func(registry resource.Registry),
) {
testhelpers.RunWorkloadSelectingTypeACLsTests[T](t, typ, getData, registerFunc)
}
6 changes: 6 additions & 0 deletions internal/catalog/exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ var (
FailoverStatusConditionAcceptedUsingMeshDestinationPortReason = failover.UsingMeshDestinationPortReason
)

type WorkloadSelecting = types.WorkloadSelecting

func ACLHooksForWorkloadSelectingType[T WorkloadSelecting]() *resource.ACLHooks {
return types.ACLHooksForWorkloadSelectingType[T]()
}

// RegisterTypes adds all resource types within the "catalog" API group
// to the given type registry
func RegisterTypes(r resource.Registry) {
Expand Down
103 changes: 103 additions & 0 deletions internal/catalog/internal/testhelpers/acl_hooks_test_helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package testhelpers

import (
"testing"

"google.golang.org/protobuf/proto"

"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/resourcetest"
pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1"
"github.com/hashicorp/consul/proto-public/pbresource"
)

// WorkloadSelecting denotes a resource type that uses workload selectors.
type WorkloadSelecting interface {
proto.Message
GetWorkloads() *pbcatalog.WorkloadSelector
}

func RunWorkloadSelectingTypeACLsTests[T WorkloadSelecting](t *testing.T, typ *pbresource.Type,
getData func(selector *pbcatalog.WorkloadSelector) T,
registerFunc func(registry resource.Registry),
) {
// Wire up a registry to generically invoke hooks
registry := resource.NewRegistry()
registerFunc(registry)

cases := map[string]resourcetest.ACLTestCase{
"no rules": {
Rules: ``,
Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}),
Typ: typ,
ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"service test read": {
Rules: `service "test" { policy = "read" }`,
Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"service test write with named selectors and insufficient policy": {
Rules: `service "test" { policy = "write" }`,
Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"service test write with prefixed selectors and insufficient policy": {
Rules: `service "test" { policy = "write" }`,
Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"service test write with named selectors": {
Rules: `service "test" { policy = "write" } service "workload" { policy = "read" }`,
Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
"service test write with prefixed selectors": {
Rules: `service "test" { policy = "write" } service_prefix "workload-" { policy = "read" }`,
Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload-"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
"service test write with prefixed selectors and a policy with more specific than the selector": {
Rules: `service "test" { policy = "write" } service_prefix "workload-" { policy = "read" }`,
Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"wor"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"service test write with prefixed selectors and a policy with less specific than the selector": {
Rules: `service "test" { policy = "write" } service_prefix "wor" { policy = "read" }`,
Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload-"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
resourcetest.RunACLTestCase(t, tc, registry)
})
}
}
52 changes: 52 additions & 0 deletions internal/catalog/internal/types/acl_hooks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package types

import (
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/proto-public/pbresource"
)

func aclReadHookResourceWithWorkloadSelector(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error {
return authorizer.ToAllowAuthorizer().ServiceReadAllowed(id.GetName(), authzContext)
}

func aclWriteHookResourceWithWorkloadSelector[T WorkloadSelecting](authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error {
decodedService, err := resource.Decode[T](res)
if err != nil {
return resource.ErrNeedData
}

// First check service:write on the name.
err = authorizer.ToAllowAuthorizer().ServiceWriteAllowed(res.GetId().GetName(), authzContext)
if err != nil {
return err
}

// Then also check whether we're allowed to select a service.
for _, name := range decodedService.GetData().GetWorkloads().GetNames() {
err = authorizer.ToAllowAuthorizer().ServiceReadAllowed(name, authzContext)
if err != nil {
return err
}
}

for _, prefix := range decodedService.GetData().GetWorkloads().GetPrefixes() {
err = authorizer.ToAllowAuthorizer().ServiceReadAllowed(prefix, authzContext)
if err != nil {
return err
}
}

return nil
}

func ACLHooksForWorkloadSelectingType[T WorkloadSelecting]() *resource.ACLHooks {
return &resource.ACLHooks{
Read: aclReadHookResourceWithWorkloadSelector,
Write: aclWriteHookResourceWithWorkloadSelector[T],
List: resource.NoOpACLListHook,
}
}
1 change: 1 addition & 0 deletions internal/catalog/internal/types/dns_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func RegisterDNSPolicy(r resource.Registry) {
Proto: &pbcatalog.DNSPolicy{},
Scope: resource.ScopeNamespace,
Validate: ValidateDNSPolicy,
ACLs: ACLHooksForWorkloadSelectingType[*pbcatalog.DNSPolicy](),
})
}

Expand Down
18 changes: 18 additions & 0 deletions internal/catalog/internal/types/dns_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/known/anypb"

"github.com/hashicorp/consul/internal/catalog/internal/testhelpers"
"github.com/hashicorp/consul/internal/resource"
pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1"
"github.com/hashicorp/consul/proto-public/pbresource"
Expand Down Expand Up @@ -161,3 +162,20 @@ func TestValidateDNSPolicy_EmptySelector(t *testing.T) {
require.ErrorAs(t, err, &actual)
require.Equal(t, expected, actual)
}

func TestDNSPolicyACLs(t *testing.T) {
// Wire up a registry to generically invoke hooks
registry := resource.NewRegistry()
RegisterDNSPolicy(registry)

testhelpers.RunWorkloadSelectingTypeACLsTests[*pbcatalog.DNSPolicy](t, pbcatalog.DNSPolicyType,
func(selector *pbcatalog.WorkloadSelector) *pbcatalog.DNSPolicy {
return &pbcatalog.DNSPolicy{
Workloads: selector,
Weights: &pbcatalog.Weights{Passing: 1, Warning: 0},
}
},
func(registry resource.Registry) {
RegisterDNSPolicy(registry)
})
}
8 changes: 1 addition & 7 deletions internal/catalog/internal/types/failover_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func RegisterFailoverPolicy(r resource.Registry) {
ACLs: &resource.ACLHooks{
Read: aclReadHookFailoverPolicy,
Write: aclWriteHookFailoverPolicy,
List: aclListHookFailoverPolicy,
List: resource.NoOpACLListHook,
},
})
}
Expand Down Expand Up @@ -371,9 +371,3 @@ func aclWriteHookFailoverPolicy(authorizer acl.Authorizer, authzContext *acl.Aut
return nil

}

func aclListHookFailoverPolicy(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext) error {
// No-op List permission as we want to default to filtering resources
// from the list using the Read enforcement.
return nil
}
Loading

0 comments on commit c79f2a8

Please sign in to comment.