luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

health: support `ResultsFilteredByACLs` flag/header (#11602)

This commit is contained in:
Dan Upton 2021-12-03 17:31:32 +00:00 committed by GitHub
parent b07edb9fe6
commit 3ad8540d23
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 250 additions and 203 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
agent/consul/acl.go
View File

@ -1219,10 +1219,12 @@ func (f *aclFilter) allowSession(node string, ent *acl.AuthorizerContext) bool {
}
// filterHealthChecks is used to filter a set of health checks down based on
// the configured ACL rules for a token.
func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) {
// the configured ACL rules for a token. Returns true if any elements were
// removed.
func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) bool {
hc := *checks
var authzContext acl.AuthorizerContext
var removed bool
for i := 0; i < len(hc); i++ {
check := hc[i]
@ -1232,10 +1234,12 @@ func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) {
}
f.logger.Debug("dropping check from result due to ACLs", "check", check.CheckID)
removed = true
hc = append(hc[:i], hc[i+1:]...)
i--
}
*checks = hc
return removed
}
// filterServices is used to filter a set of services based on ACLs.
@ -1332,10 +1336,12 @@ func (f *aclFilter) filterNodeServiceList(services **structs.NodeServiceList) {
}
}
// filterCheckServiceNodes is used to filter nodes based on ACL rules.
func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) {
// filterCheckServiceNodes is used to filter nodes based on ACL rules. Returns
// true if any elements were removed.
func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) bool {
csn := *nodes
var authzContext acl.AuthorizerContext
var removed bool
for i := 0; i < len(csn); i++ {
node := csn[i]
@ -1344,22 +1350,20 @@ func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) {
continue
}
f.logger.Debug("dropping node from result due to ACLs", "node", structs.NodeNameString(node.Node.Node, node.Node.GetEnterpriseMeta()))
removed = true
csn = append(csn[:i], csn[i+1:]...)
i--
}
*nodes = csn
return removed
}
// filterServiceTopology is used to filter upstreams/downstreams based on ACL rules.
// this filter is unlike others in that it also returns whether the result was filtered by ACLs
func (f *aclFilter) filterServiceTopology(topology *structs.ServiceTopology) bool {
numUp := len(topology.Upstreams)
numDown := len(topology.Downstreams)
f.filterCheckServiceNodes(&topology.Upstreams)
f.filterCheckServiceNodes(&topology.Downstreams)
return numUp != len(topology.Upstreams) || numDown != len(topology.Downstreams)
filteredUpstreams := f.filterCheckServiceNodes(&topology.Upstreams)
filteredDownstreams := f.filterCheckServiceNodes(&topology.Downstreams)
return filteredUpstreams || filteredDownstreams
}
// filterDatacenterCheckServiceNodes is used to filter nodes based on ACL rules.
@ -1801,7 +1805,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
filt.filterCheckServiceNodes(v)
case *structs.IndexedCheckServiceNodes:
filt.filterCheckServiceNodes(&v.Nodes)
v.QueryMeta.ResultsFilteredByACLs = filt.filterCheckServiceNodes(&v.Nodes)
case *structs.IndexedServiceTopology:
filtered := filt.filterServiceTopology(v.ServiceTopology)
@ -1817,7 +1821,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
filt.filterCoordinates(&v.Coordinates)
case *structs.IndexedHealthChecks:
filt.filterHealthChecks(&v.HealthChecks)
v.QueryMeta.ResultsFilteredByACLs = filt.filterHealthChecks(&v.HealthChecks)
case *structs.IndexedIntentions:
filt.filterIntentions(&v.Intentions)

308
agent/consul/acl_test.go
View File

@ -9,6 +9,7 @@ import (
"testing"
"time"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-uuid"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
"github.com/mitchellh/copystructure"
@ -2151,72 +2152,93 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega
func TestACL_filterHealthChecks(t *testing.T) {
t.Parallel()
// Create some health checks.
fill := func() structs.HealthChecks {
return structs.HealthChecks{
&structs.HealthCheck{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
logger := hclog.NewNullLogger()
makeList := func() *structs.IndexedHealthChecks {
return &structs.IndexedHealthChecks{
HealthChecks: structs.HealthChecks{
{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
},
},
}
}
{
hc := fill()
filt := newACLFilter(acl.DenyAll(), nil)
filt.filterHealthChecks(&hc)
if len(hc) != 0 {
t.Fatalf("bad: %#v", hc)
}
}
t.Run("allowed", func(t *testing.T) {
require := require.New(t)
// Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(err)
{
hc := fill()
filt := newACLFilter(perms, nil)
filt.filterHealthChecks(&hc)
if len(hc) != 0 {
t.Fatalf("bad: %#v", hc)
}
}
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(err)
// Chain on access to the node.
policy, err = acl.NewPolicyFromSource(`
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
list := makeList()
filterACLWithAuthorizer(logger, authz, list)
// Now it should go through.
{
hc := fill()
filt := newACLFilter(perms, nil)
filt.filterHealthChecks(&hc)
if len(hc) != 1 {
t.Fatalf("bad: %#v", hc)
}
}
require.Len(list.HealthChecks, 1)
require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
})
t.Run("allowed to read the service, but not the node", func(t *testing.T) {
require := require.New(t)
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(err)
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(err)
list := makeList()
filterACLWithAuthorizer(logger, authz, list)
require.Empty(list.HealthChecks)
require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("allowed to read the node, but not the service", func(t *testing.T) {
require := require.New(t)
policy, err := acl.NewPolicyFromSource(`
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(err)
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(err)
list := makeList()
filterACLWithAuthorizer(logger, authz, list)
require.Empty(list.HealthChecks)
require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("denied", func(t *testing.T) {
require := require.New(t)
list := makeList()
filterACLWithAuthorizer(logger, acl.DenyAll(), list)
require.Empty(list.HealthChecks)
require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
}
func TestACL_filterIntentions(t *testing.T) {
@ -2474,100 +2496,104 @@ node "node1" {
func TestACL_filterCheckServiceNodes(t *testing.T) {
t.Parallel()
// Create some nodes.
fill := func() structs.CheckServiceNodes {
return structs.CheckServiceNodes{
structs.CheckServiceNode{
Node: &structs.Node{
Node: "node1",
},
Service: &structs.NodeService{
ID: "foo",
Service: "foo",
},
Checks: structs.HealthChecks{
&structs.HealthCheck{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
logger := hclog.NewNullLogger()
makeList := func() *structs.IndexedCheckServiceNodes {
return &structs.IndexedCheckServiceNodes{
Nodes: structs.CheckServiceNodes{
{
Node: &structs.Node{
Node: "node1",
},
Service: &structs.NodeService{
ID: "foo",
Service: "foo",
},
Checks: structs.HealthChecks{
{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
},
},
},
},
}
}
// Try permissive filtering.
{
nodes := fill()
filt := newACLFilter(acl.AllowAll(), nil)
filt.filterCheckServiceNodes(&nodes)
if len(nodes) != 1 {
t.Fatalf("bad: %#v", nodes)
}
if len(nodes[0].Checks) != 1 {
t.Fatalf("bad: %#v", nodes[0].Checks)
}
}
t.Run("allowed", func(t *testing.T) {
require := require.New(t)
// Try restrictive filtering.
{
nodes := fill()
filt := newACLFilter(acl.DenyAll(), nil)
filt.filterCheckServiceNodes(&nodes)
if len(nodes) != 0 {
t.Fatalf("bad: %#v", nodes)
}
}
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(err)
// Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(err)
{
nodes := fill()
filt := newACLFilter(perms, nil)
filt.filterCheckServiceNodes(&nodes)
if len(nodes) != 0 {
t.Fatalf("bad: %#v", nodes)
}
}
list := makeList()
filterACLWithAuthorizer(logger, authz, list)
// Chain on access to the node.
policy, err = acl.NewPolicyFromSource(`
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
if err != nil {
t.Fatalf("err %v", err)
}
perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
if err != nil {
t.Fatalf("err: %v", err)
}
require.Len(list.Nodes, 1)
require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
})
// Now it should go through.
{
nodes := fill()
filt := newACLFilter(perms, nil)
filt.filterCheckServiceNodes(&nodes)
if len(nodes) != 1 {
t.Fatalf("bad: %#v", nodes)
}
if len(nodes[0].Checks) != 1 {
t.Fatalf("bad: %#v", nodes[0].Checks)
}
}
t.Run("allowed to read the service, but not the node", func(t *testing.T) {
require := require.New(t)
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(err)
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(err)
list := makeList()
filterACLWithAuthorizer(logger, authz, list)
require.Empty(list.Nodes)
require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("allowed to read the node, but not the service", func(t *testing.T) {
require := require.New(t)
policy, err := acl.NewPolicyFromSource(`
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(err)
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(err)
list := makeList()
filterACLWithAuthorizer(logger, authz, list)
require.Empty(list.Nodes)
require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("denied", func(t *testing.T) {
require := require.New(t)
list := makeList()
filterACLWithAuthorizer(logger, acl.DenyAll(), list)
require.Empty(list.Nodes)
require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
}
func TestACL_filterServiceTopology(t *testing.T) {

1
agent/consul/catalog_endpoint_test.go
View File

@ -2742,6 +2742,7 @@ node_prefix "" {
CheckID: "service:bar",
Name: "service:bar",
ServiceID: "bar",
Status: api.HealthPassing,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}

42
agent/consul/health_endpoint.go
View File

@ -55,9 +55,6 @@ func (h *Health) ChecksInState(args *structs.ChecksInStateRequest,
return err
}
reply.Index, reply.HealthChecks = index, checks
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
raw, err := filter.Execute(reply.HealthChecks)
if err != nil {
@ -65,6 +62,13 @@ func (h *Health) ChecksInState(args *structs.ChecksInStateRequest,
}
reply.HealthChecks = raw.(structs.HealthChecks)
// Note: we filter the results with ACLs *after* applying the user-supplied
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
// results that would be filtered out even if the user did have permission.
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks)
})
}
@ -99,15 +103,20 @@ func (h *Health) NodeChecks(args *structs.NodeSpecificRequest,
return err
}
reply.Index, reply.HealthChecks = index, checks
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
raw, err := filter.Execute(reply.HealthChecks)
if err != nil {
return err
}
reply.HealthChecks = raw.(structs.HealthChecks)
// Note: we filter the results with ACLs *after* applying the user-supplied
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
// results that would be filtered out even if the user did have permission.
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
return nil
})
}
@ -156,9 +165,6 @@ func (h *Health) ServiceChecks(args *structs.ServiceSpecificRequest,
return err
}
reply.Index, reply.HealthChecks = index, checks
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
raw, err := filter.Execute(reply.HealthChecks)
if err != nil {
@ -166,6 +172,13 @@ func (h *Health) ServiceChecks(args *structs.ServiceSpecificRequest,
}
reply.HealthChecks = raw.(structs.HealthChecks)
// Note: we filter the results with ACLs *after* applying the user-supplied
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
// results that would be filtered out even if the user did have permission.
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks)
})
}
@ -232,16 +245,19 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
reply.Nodes = nodeMetaFilter(args.NodeMetaFilters, reply.Nodes)
}
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
raw, err := filter.Execute(reply.Nodes)
if err != nil {
return err
}
reply.Nodes = raw.(structs.CheckServiceNodes)
// Note: we filter the results with ACLs *after* applying the user-supplied
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
// results that would be filtered out even if the user did have permission.
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err
}
return h.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes)
})

72
agent/consul/health_endpoint_test.go
View File

@ -1431,6 +1431,9 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
}
t.Parallel()
require := require.New(t)
dir, token, srv, codec := testACLFilterServer(t)
defer os.RemoveAll(dir)
defer srv.Shutdown()
@ -1442,9 +1445,9 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
QueryOptions: structs.QueryOptions{Token: token},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply)
require.NoError(err)
found := false
for _, chk := range reply.HealthChecks {
switch chk.ServiceName {
@ -1454,9 +1457,8 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
t.Fatalf("bad: %#v", reply.HealthChecks)
}
}
if !found {
t.Fatalf("bad: %#v", reply.HealthChecks)
}
require.True(found, "bad: %#v", reply.HealthChecks)
require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we
// test node filtering down in acl.go for node cases. This also proves
@ -1471,6 +1473,9 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
}
t.Parallel()
require := require.New(t)
dir, token, srv, codec := testACLFilterServer(t)
defer os.RemoveAll(dir)
defer srv.Shutdown()
@ -1482,9 +1487,9 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
QueryOptions: structs.QueryOptions{Token: token},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
require.NoError(err)
found := false
for _, chk := range reply.HealthChecks {
if chk.ServiceName == "foo" {
@ -1492,18 +1497,14 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
break
}
}
if !found {
t.Fatalf("bad: %#v", reply.HealthChecks)
}
require.True(found, "bad: %#v", reply.HealthChecks)
opt.ServiceName = "bar"
reply = structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
if len(reply.HealthChecks) != 0 {
t.Fatalf("bad: %#v", reply.HealthChecks)
}
err = msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
require.NoError(err)
require.Empty(reply.HealthChecks)
require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we
// test node filtering down in acl.go for node cases. This also proves
@ -1518,6 +1519,9 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
}
t.Parallel()
require := require.New(t)
dir, token, srv, codec := testACLFilterServer(t)
defer os.RemoveAll(dir)
defer srv.Shutdown()
@ -1529,21 +1533,16 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
QueryOptions: structs.QueryOptions{Token: token},
}
reply := structs.IndexedCheckServiceNodes{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
if len(reply.Nodes) != 1 {
t.Fatalf("bad: %#v", reply.Nodes)
}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
require.NoError(err)
require.Len(reply.Nodes, 1)
opt.ServiceName = "bar"
reply = structs.IndexedCheckServiceNodes{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
if len(reply.Nodes) != 0 {
t.Fatalf("bad: %#v", reply.Nodes)
}
err = msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
require.NoError(err)
require.Empty(reply.Nodes)
require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we
// test node filtering down in acl.go for node cases. This also proves
@ -1558,6 +1557,9 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
}
t.Parallel()
require := require.New(t)
dir, token, srv, codec := testACLFilterServer(t)
defer os.RemoveAll(dir)
defer srv.Shutdown()
@ -1569,9 +1571,8 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
QueryOptions: structs.QueryOptions{Token: token},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &opt, &reply)
require.NoError(err)
found := false
for _, chk := range reply.HealthChecks {
@ -1582,9 +1583,8 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
t.Fatalf("bad service 'bar': %#v", reply.HealthChecks)
}
}
if !found {
t.Fatalf("missing service 'foo': %#v", reply.HealthChecks)
}
require.True(found, "missing service 'foo': %#v", reply.HealthChecks)
require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we
// test node filtering down in acl.go for node cases. This also proves