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

Ensure that NodeDump imported nodes are filtered (#15356)

This commit is contained in:
Freddy 2022-11-14 12:35:20 -07:00 committed by GitHub
parent a4c64ab006
commit 0cc3fac6c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 244 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/15356.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:security
Ensure that data imported from peers is filtered by ACLs at the UI Nodes/Services endpoints [CVE-2022-3920](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-3920)
```

7
agent/structs/aclfilter/filter.go
View File

@ -61,7 +61,12 @@ func (f *Filter) Filter(subject any) {
v.QueryMeta.ResultsFilteredByACLs = f.filterIntentions(&v.Intentions)
case *structs.IndexedNodeDump:
v.QueryMeta.ResultsFilteredByACLs = f.filterNodeDump(&v.Dump)
if f.filterNodeDump(&v.Dump) {
v.QueryMeta.ResultsFilteredByACLs = true
}
if f.filterNodeDump(&v.ImportedDump) {
v.QueryMeta.ResultsFilteredByACLs = true
}
case *structs.IndexedServiceDump:
v.QueryMeta.ResultsFilteredByACLs = f.filterServiceDump(&v.Dump)

256
agent/structs/aclfilter/filter_test.go
View File

@ -1444,11 +1444,104 @@ func TestACL_filterNodeDump(t *testing.T) {
},
},
},
ImportedDump: structs.NodeDump{
{
// The node and service names are intentionally the same to ensure that
// local permissions for the same names do not allow reading imports.
Node: "node1",
PeerName: "cluster-02",
Services: []*structs.NodeService{
{
ID: "foo",
Service: "foo",
PeerName: "cluster-02",
},
},
Checks: []*structs.HealthCheck{
{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
PeerName: "cluster-02",
},
},
},
},
}
}
type testCase struct {
authzFn func() acl.Authorizer
expect *structs.IndexedNodeDump
}
t.Run("allowed", func(t *testing.T) {
run := func(t *testing.T, tc testCase) {
authz := tc.authzFn()
list := makeList()
New(authz, logger).Filter(list)
require.Equal(t, tc.expect, list)
}
tt := map[string]testCase{
"denied": {
authzFn: func() acl.Authorizer {
return acl.DenyAll()
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{},
ImportedDump: structs.NodeDump{},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: true},
},
},
"can read local service but not the node": {
authzFn: func() acl.Authorizer {
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(t, err)
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
return authz
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{},
ImportedDump: structs.NodeDump{},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: true},
},
},
"can read the local node but not the service": {
authzFn: func() acl.Authorizer {
policy, err := acl.NewPolicyFromSource(`
node "node1" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(t, err)
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
return authz
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{
{
Node: "node1",
Services: []*structs.NodeService{},
Checks: structs.HealthChecks{},
},
},
ImportedDump: structs.NodeDump{},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: true},
},
},
"can read local data": {
authzFn: func() acl.Authorizer {
policy, err := acl.NewPolicyFromSource(`
service "foo" {
policy = "read"
@ -1462,17 +1555,36 @@ func TestACL_filterNodeDump(t *testing.T) {
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
list := makeList()
New(authz, logger).Filter(list)
require.Len(t, list.Dump, 1)
require.False(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
})
t.Run("allowed to read the service, but not the node", func(t *testing.T) {
return authz
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{
{
Node: "node1",
Services: []*structs.NodeService{
{
ID: "foo",
Service: "foo",
},
},
Checks: []*structs.HealthCheck{
{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
},
},
},
},
ImportedDump: structs.NodeDump{},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: true},
},
},
"can read imported service but not the node": {
authzFn: func() acl.Authorizer {
// Wildcard service read also grants read to imported services.
policy, err := acl.NewPolicyFromSource(`
service "foo" {
service "" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
@ -1481,17 +1593,19 @@ func TestACL_filterNodeDump(t *testing.T) {
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
list := makeList()
New(authz, logger).Filter(list)
require.Empty(t, list.Dump)
require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("allowed to read the node, but not the service", func(t *testing.T) {
return authz
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{},
ImportedDump: structs.NodeDump{},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: true},
},
},
"can read the imported node but not the service": {
authzFn: func() acl.Authorizer {
// Wildcard node read also grants read to imported nodes.
policy, err := acl.NewPolicyFromSource(`
node "node1" {
node "" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
@ -1500,23 +1614,95 @@ func TestACL_filterNodeDump(t *testing.T) {
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
list := makeList()
New(authz, logger).Filter(list)
return authz
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{
{
Node: "node1",
Services: []*structs.NodeService{},
Checks: structs.HealthChecks{},
},
},
ImportedDump: structs.NodeDump{
{
Node: "node1",
PeerName: "cluster-02",
Services: []*structs.NodeService{},
Checks: structs.HealthChecks{},
},
},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: true},
},
},
"can read all data": {
authzFn: func() acl.Authorizer {
policy, err := acl.NewPolicyFromSource(`
service "" {
policy = "read"
}
node "" {
policy = "read"
}
`, acl.SyntaxLegacy, nil, nil)
require.NoError(t, err)
require.Len(t, list.Dump, 1)
require.Empty(t, list.Dump[0].Services)
require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("denied", func(t *testing.T) {
list := makeList()
New(acl.DenyAll(), logger).Filter(list)
require.Empty(t, list.Dump)
require.True(t, list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
return authz
},
expect: &structs.IndexedNodeDump{
Dump: structs.NodeDump{
{
Node: "node1",
Services: []*structs.NodeService{
{
ID: "foo",
Service: "foo",
},
},
Checks: []*structs.HealthCheck{
{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
},
},
},
},
ImportedDump: structs.NodeDump{
{
Node: "node1",
PeerName: "cluster-02",
Services: []*structs.NodeService{
{
ID: "foo",
Service: "foo",
PeerName: "cluster-02",
},
},
Checks: []*structs.HealthCheck{
{
Node: "node1",
CheckID: "check1",
ServiceName: "foo",
PeerName: "cluster-02",
},
},
},
},
QueryMeta: structs.QueryMeta{ResultsFilteredByACLs: false},
},
},
}
for name, tc := range tt {
t.Run(name, func(t *testing.T) {
run(t, tc)
})
}
}
func TestACL_filterNodes(t *testing.T) {
t.Parallel()

4
agent/structs/structs_oss.go
View File

@ -67,7 +67,9 @@ func (n *Node) OverridePartition(_ string) {
func (_ *Coordinate) FillAuthzContext(_ *acl.AuthorizerContext) {}
func (_ *NodeInfo) FillAuthzContext(_ *acl.AuthorizerContext) {}
func (n *NodeInfo) FillAuthzContext(ctx *acl.AuthorizerContext) {
ctx.Peer = n.PeerName
}
// FillAuthzContext stub
func (_ *DirEntry) FillAuthzContext(_ *acl.AuthorizerContext) {}