NET-11737 - sec vulnerability - remediate ability to use bexpr to filter results without ACL read on endpoint

pull/21950/head
John Murret 1 week ago
parent 6662e48363
commit 07a618b1fc

@ -380,16 +380,14 @@ func (s *HTTPHandlers) AgentServices(resp http.ResponseWriter, req *http.Request
return nil, err return nil, err
} }
raw, err := filter.Execute(agentSvcs) // Note: we filter the results with ACLs *before* applying the user-supplied
if err != nil { // bexpr filter to ensure that the user can only run expressions on data that
return nil, err // they have access to. This is a security measure to prevent users from
} // running arbitrary expressions on data they don't have access to.
agentSvcs = raw.(map[string]*api.AgentService) // QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// Note: we filter the results with ACLs *after* applying the user-supplied // also allowed to run the bexpr filter on the data, they could potentially
// bexpr filter, to ensure total (and the filter-by-acls header we set below) // infer the specific attributes of data they don't have access to.
// do not include results that would be filtered out even if the user did have
// permission.
total := len(agentSvcs) total := len(agentSvcs)
if err := s.agent.filterServicesWithAuthorizer(authz, agentSvcs); err != nil { if err := s.agent.filterServicesWithAuthorizer(authz, agentSvcs); err != nil {
return nil, err return nil, err
@ -407,6 +405,12 @@ func (s *HTTPHandlers) AgentServices(resp http.ResponseWriter, req *http.Request
setResultsFilteredByACLs(resp, total != len(agentSvcs)) setResultsFilteredByACLs(resp, total != len(agentSvcs))
} }
raw, err := filter.Execute(agentSvcs)
if err != nil {
return nil, err
}
agentSvcs = raw.(map[string]*api.AgentService)
return agentSvcs, nil return agentSvcs, nil
} }
@ -540,16 +544,14 @@ func (s *HTTPHandlers) AgentChecks(resp http.ResponseWriter, req *http.Request)
} }
} }
raw, err := filter.Execute(agentChecks) // Note: we filter the results with ACLs *before* applying the user-supplied
if err != nil { // bexpr filter to ensure that the user can only run expressions on data that
return nil, err // they have access to. This is a security measure to prevent users from
} // running arbitrary expressions on data they don't have access to.
agentChecks = raw.(map[types.CheckID]*structs.HealthCheck) // QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// Note: we filter the results with ACLs *after* applying the user-supplied // also allowed to run the bexpr filter on the data, they could potentially
// bexpr filter, to ensure total (and the filter-by-acls header we set below) // infer the specific attributes of data they don't have access to.
// do not include results that would be filtered out even if the user did have
// permission.
total := len(agentChecks) total := len(agentChecks)
if err := s.agent.filterChecksWithAuthorizer(authz, agentChecks); err != nil { if err := s.agent.filterChecksWithAuthorizer(authz, agentChecks); err != nil {
return nil, err return nil, err
@ -567,6 +569,12 @@ func (s *HTTPHandlers) AgentChecks(resp http.ResponseWriter, req *http.Request)
setResultsFilteredByACLs(resp, total != len(agentChecks)) setResultsFilteredByACLs(resp, total != len(agentChecks))
} }
raw, err := filter.Execute(agentChecks)
if err != nil {
return nil, err
}
agentChecks = raw.(map[types.CheckID]*structs.HealthCheck)
return agentChecks, nil return agentChecks, nil
} }
@ -623,21 +631,14 @@ func (s *HTTPHandlers) AgentMembers(resp http.ResponseWriter, req *http.Request)
} }
} }
// filter the members by parsed filter expression // Note: we filter the results with ACLs *before* applying the user-supplied
var filterExpression string // bexpr filter to ensure that the user can only run expressions on data that
s.parseFilter(req, &filterExpression) // they have access to. This is a security measure to prevent users from
if filterExpression != "" { // running arbitrary expressions on data they don't have access to.
filter, err := bexpr.CreateFilter(filterExpression, nil, members) // QueryMeta.ResultsFilteredByACLs being true already indicates to the user
if err != nil { // that results they don't have access to have been removed. If they were
return nil, err // also allowed to run the bexpr filter on the data, they could potentially
} // infer the specific attributes of data they don't have access to.
raw, err := filter.Execute(members)
if err != nil {
return nil, err
}
members = raw.([]serf.Member)
}
total := len(members) total := len(members)
if err := s.agent.filterMembers(token, &members); err != nil { if err := s.agent.filterMembers(token, &members); err != nil {
return nil, err return nil, err
@ -655,6 +656,21 @@ func (s *HTTPHandlers) AgentMembers(resp http.ResponseWriter, req *http.Request)
setResultsFilteredByACLs(resp, total != len(members)) setResultsFilteredByACLs(resp, total != len(members))
} }
// filter the members by parsed filter expression
var filterExpression string
s.parseFilter(req, &filterExpression)
if filterExpression != "" {
filter, err := bexpr.CreateFilter(filterExpression, nil, members)
if err != nil {
return nil, err
}
raw, err := filter.Execute(members)
if err != nil {
return nil, err
}
members = raw.([]serf.Member)
}
return members, nil return members, nil
} }

@ -433,6 +433,60 @@ func TestAgent_Services_ACLFilter(t *testing.T) {
require.Len(t, val, 2) require.Len(t, val, 2)
require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs")) require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
}) })
// ensure ACL filtering occurs before bexpr filtering.
const bexprMatchingUserTokenPermissions = "Service matches `web.*`"
const bexprNotMatchingUserTokenPermissions = "Service matches `api.*`"
tokenWithWebRead := testCreateToken(t, a, `
service "web" {
policy = "read"
}
`)
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape(bexprMatchingUserTokenPermissions), nil)
req.Header.Add("X-Consul-Token", tokenWithWebRead)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
var val map[string]*api.AgentService
err := dec.Decode(&val)
if err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 1)
require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
req.Header.Add("X-Consul-Token", tokenWithWebRead)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
var val map[string]*api.AgentService
err := dec.Decode(&val)
if err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 0)
require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
var val map[string]*api.AgentService
err := dec.Decode(&val)
if err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 0)
require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
} }
func TestAgent_Service(t *testing.T) { func TestAgent_Service(t *testing.T) {
@ -1432,6 +1486,57 @@ func TestAgent_Checks_ACLFilter(t *testing.T) {
require.Len(t, val, 2) require.Len(t, val, 2)
require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs")) require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
}) })
// ensure ACL filtering occurs before bexpr filtering.
const bexprMatchingUserTokenPermissions = "ServiceName matches `web.*`"
const bexprNotMatchingUserTokenPermissions = "ServiceName matches `api.*`"
tokenWithWebRead := testCreateToken(t, a, `
service "web" {
policy = "read"
}
`)
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/checks?filter="+url.QueryEscape(bexprMatchingUserTokenPermissions), nil)
req.Header.Add("X-Consul-Token", tokenWithWebRead)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
val := make(map[types.CheckID]*structs.HealthCheck)
if err := dec.Decode(&val); err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 1)
require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/checks?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
req.Header.Add("X-Consul-Token", tokenWithWebRead)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
val := make(map[types.CheckID]*structs.HealthCheck)
if err := dec.Decode(&val); err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 0)
require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/checks?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
val := make(map[types.CheckID]*structs.HealthCheck)
if err := dec.Decode(&val); err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 0)
require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
} }
func TestAgent_Self(t *testing.T) { func TestAgent_Self(t *testing.T) {
@ -2110,6 +2215,57 @@ func TestAgent_Members_ACLFilter(t *testing.T) {
require.Len(t, val, 2) require.Len(t, val, 2)
require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs")) require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
}) })
// ensure ACL filtering occurs before bexpr filtering.
bexprMatchingUserTokenPermissions := fmt.Sprintf("Name matches `%s.*`", b.Config.NodeName)
bexprNotMatchingUserTokenPermissions := fmt.Sprintf("Name matches `%s.*`", a.Config.NodeName)
tokenWithReadOnMemberB := testCreateToken(t, a, fmt.Sprintf(`
node "%s" {
policy = "read"
}
`, b.Config.NodeName))
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/members?filter="+url.QueryEscape(bexprMatchingUserTokenPermissions), nil)
req.Header.Add("X-Consul-Token", tokenWithReadOnMemberB)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
val := make([]serf.Member, 0)
if err := dec.Decode(&val); err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 1)
require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/members?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
req.Header.Add("X-Consul-Token", tokenWithReadOnMemberB)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
val := make([]serf.Member, 0)
if err := dec.Decode(&val); err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 0)
require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/agent/members?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
dec := json.NewDecoder(resp.Body)
val := make([]serf.Member, 0)
if err := dec.Decode(&val); err != nil {
t.Fatalf("Err: %v", err)
}
require.Len(t, val, 0)
require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
})
} }
func TestAgent_Join(t *testing.T) { func TestAgent_Join(t *testing.T) {

@ -122,7 +122,7 @@ func TestCatalogDeregister(t *testing.T) {
a := NewTestAgent(t, "") a := NewTestAgent(t, "")
defer a.Shutdown() defer a.Shutdown()
// Register node // Deregister node
args := &structs.DeregisterRequest{Node: "foo"} args := &structs.DeregisterRequest{Node: "foo"}
req, _ := http.NewRequest("PUT", "/v1/catalog/deregister", jsonReader(args)) req, _ := http.NewRequest("PUT", "/v1/catalog/deregister", jsonReader(args))
obj, err := a.srv.CatalogDeregister(nil, req) obj, err := a.srv.CatalogDeregister(nil, req)

@ -533,18 +533,23 @@ func (c *Catalog) ListNodes(args *structs.DCSpecificRequest, reply *structs.Inde
return nil return nil
} }
raw, err := filter.Execute(reply.Nodes) // Note: we filter the results with ACLs *before* applying the user-supplied
if err != nil { // bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := c.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.Nodes = raw.(structs.Nodes)
// Note: we filter the results with ACLs *after* applying the user-supplied raw, err := filter.Execute(reply.Nodes)
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include if err != nil {
// results that would be filtered out even if the user did have permission.
if err := c.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.Nodes = raw.(structs.Nodes)
return c.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes) return c.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes)
}) })
@ -607,14 +612,25 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I
return nil return nil
} }
raw, err := filter.Execute(serviceNodes) // need to temporarily create an IndexedServiceNode so that the ACL filter can be applied
// to the service nodes and then re-use those same node to run the filter expression.
idxServiceNodeReply := &structs.IndexedServiceNodes{
ServiceNodes: serviceNodes,
QueryMeta: reply.QueryMeta,
}
// enforce ACLs
c.srv.filterACLWithAuthorizer(authz, idxServiceNodeReply)
// run the filter expression
raw, err := filter.Execute(idxServiceNodeReply.ServiceNodes)
if err != nil { if err != nil {
return err return err
} }
// convert the result back to the original type
reply.Services = servicesTagsByName(raw.(structs.ServiceNodes)) reply.Services = servicesTagsByName(raw.(structs.ServiceNodes))
reply.QueryMeta = idxServiceNodeReply.QueryMeta
c.srv.filterACLWithAuthorizer(authz, reply)
return nil return nil
}) })
@ -813,6 +829,18 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
reply.ServiceNodes = filtered reply.ServiceNodes = filtered
} }
// Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := c.srv.filterACL(args.Token, reply); err != nil {
return err
}
// This is safe to do even when the filter is nil - its just a no-op then // This is safe to do even when the filter is nil - its just a no-op then
raw, err := filter.Execute(reply.ServiceNodes) raw, err := filter.Execute(reply.ServiceNodes)
if err != nil { if err != nil {
@ -820,13 +848,6 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
} }
reply.ServiceNodes = raw.(structs.ServiceNodes) reply.ServiceNodes = raw.(structs.ServiceNodes)
// 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 := c.srv.filterACL(args.Token, reply); err != nil {
return err
}
return c.srv.sortNodesByDistanceFrom(args.Source, reply.ServiceNodes) return c.srv.sortNodesByDistanceFrom(args.Source, reply.ServiceNodes)
}) })
@ -904,6 +925,18 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
} }
reply.Index, reply.NodeServices = index, services reply.Index, reply.NodeServices = index, services
// Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := c.srv.filterACL(args.Token, reply); err != nil {
return err
}
if reply.NodeServices != nil { if reply.NodeServices != nil {
raw, err := filter.Execute(reply.NodeServices.Services) raw, err := filter.Execute(reply.NodeServices.Services)
if err != nil { if err != nil {
@ -912,13 +945,6 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
reply.NodeServices.Services = raw.(map[string]*structs.NodeService) reply.NodeServices.Services = raw.(map[string]*structs.NodeService)
} }
// 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 := c.srv.filterACL(args.Token, reply); err != nil {
return err
}
return nil return nil
}) })
} }
@ -1009,20 +1035,25 @@ func (c *Catalog) NodeServiceList(args *structs.NodeSpecificRequest, reply *stru
if mergedServices != nil { if mergedServices != nil {
reply.NodeServices = *mergedServices reply.NodeServices = *mergedServices
}
raw, err := filter.Execute(reply.NodeServices.Services) // Note: we filter the results with ACLs *before* applying the user-supplied
if err != nil { // bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := c.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.NodeServices.Services = raw.([]*structs.NodeService)
}
// Note: we filter the results with ACLs *after* applying the user-supplied raw, err := filter.Execute(reply.NodeServices.Services)
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include if err != nil {
// results that would be filtered out even if the user did have permission.
if err := c.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.NodeServices.Services = raw.([]*structs.NodeService)
return nil return nil
}) })

@ -984,26 +984,27 @@ func TestCatalog_RPC_Filter(t *testing.T) {
require.Equal(t, "baz", out.Nodes[0].Node) require.Equal(t, "baz", out.Nodes[0].Node)
}) })
t.Run("ServiceNodes", func(t *testing.T) { t.Run("ListServices", func(t *testing.T) {
args := structs.ServiceSpecificRequest{ args := structs.ServiceSpecificRequest{
Datacenter: "dc1", Datacenter: "dc1",
ServiceName: "redis", ServiceName: "redis",
QueryOptions: structs.QueryOptions{Filter: "ServiceMeta.version == 1"}, QueryOptions: structs.QueryOptions{Filter: "ServiceMeta.version == 1"},
} }
out := new(structs.IndexedServiceNodes) out := new(structs.IndexedServices)
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out)) require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out))
require.Len(t, out.ServiceNodes, 2) require.Len(t, out.Services, 2)
require.Condition(t, func() bool { require.Len(t, out.Services["redis"], 1)
return (out.ServiceNodes[0].Node == "foo" && out.ServiceNodes[1].Node == "bar") || require.Len(t, out.Services["web"], 2)
(out.ServiceNodes[0].Node == "bar" && out.ServiceNodes[1].Node == "foo")
})
args.Filter = "ServiceMeta.version == 2" args.Filter = "ServiceMeta.version == 2"
out = new(structs.IndexedServiceNodes) out = new(structs.IndexedServices)
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out)) require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out))
require.Len(t, out.ServiceNodes, 1) require.Len(t, out.Services, 4)
require.Equal(t, "foo", out.ServiceNodes[0].Node) require.Len(t, out.Services["redis"], 1)
require.Len(t, out.Services["web"], 2)
require.Len(t, out.Services["critical"], 1)
require.Len(t, out.Services["warning"], 1)
}) })
t.Run("NodeServices", func(t *testing.T) { t.Run("NodeServices", func(t *testing.T) {
@ -1022,6 +1023,46 @@ func TestCatalog_RPC_Filter(t *testing.T) {
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &out)) require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &out))
require.Len(t, out.NodeServices.Services, 1) require.Len(t, out.NodeServices.Services, 1)
}) })
t.Run("NodeServiceList", func(t *testing.T) {
args := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: "baz",
QueryOptions: structs.QueryOptions{Filter: "Service == web"},
}
out := new(structs.IndexedNodeServiceList)
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &args, &out))
require.Len(t, out.NodeServices.Services, 2)
args.Filter = "Service == web and Meta.version == 2"
out = new(structs.IndexedNodeServiceList)
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &args, &out))
require.Len(t, out.NodeServices.Services, 1)
})
t.Run("ServiceNodes", func(t *testing.T) {
args := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "redis",
QueryOptions: structs.QueryOptions{Filter: "ServiceMeta.version == 1"},
}
out := new(structs.IndexedServiceNodes)
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out))
require.Len(t, out.ServiceNodes, 2)
require.Condition(t, func() bool {
return (out.ServiceNodes[0].Node == "foo" && out.ServiceNodes[1].Node == "bar") ||
(out.ServiceNodes[0].Node == "bar" && out.ServiceNodes[1].Node == "foo")
})
args.Filter = "ServiceMeta.version == 2"
out = new(structs.IndexedServiceNodes)
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out))
require.Len(t, out.ServiceNodes, 1)
require.Equal(t, "foo", out.ServiceNodes[0].Node)
})
} }
func TestCatalog_ListNodes_StaleRead(t *testing.T) { func TestCatalog_ListNodes_StaleRead(t *testing.T) {
@ -1332,6 +1373,7 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
Datacenter: "dc1", Datacenter: "dc1",
} }
readToken := token("read")
t.Run("deny", func(t *testing.T) { t.Run("deny", func(t *testing.T) {
args.Token = token("deny") args.Token = token("deny")
@ -1348,7 +1390,7 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
}) })
t.Run("allow", func(t *testing.T) { t.Run("allow", func(t *testing.T) {
args.Token = token("read") args.Token = readToken
var reply structs.IndexedNodes var reply structs.IndexedNodes
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil { if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
@ -1361,6 +1403,67 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
t.Fatal("ResultsFilteredByACLs should not true") t.Fatal("ResultsFilteredByACLs should not true")
} }
}) })
// Register additional node
regArgs := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "foo",
Address: "127.0.0.1",
WriteRequest: structs.WriteRequest{
Token: "root",
},
}
var out struct{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", regArgs, &out))
bexprMatchingUserTokenPermissions := fmt.Sprintf("Node matches `%s.*`", s1.config.NodeName)
const bexpNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
var reply structs.IndexedNodes
args = structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: readToken,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodes{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply))
require.Equal(t, 1, len(reply.Nodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
var reply structs.IndexedNodes
args = structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: readToken,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodes{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply))
require.Empty(t, reply.Nodes)
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
var reply structs.IndexedNodes
args = structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodes{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply))
require.Empty(t, reply.Nodes)
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func Benchmark_Catalog_ListNodes(t *testing.B) { func Benchmark_Catalog_ListNodes(t *testing.B) {
@ -2758,6 +2861,14 @@ service "foo" {
node_prefix "" { node_prefix "" {
policy = "read" policy = "read"
} }
node "node-deny" {
policy = "deny"
}
service "service-deny" {
policy = "deny"
}
` `
token = createToken(t, codec, rules) token = createToken(t, codec, rules)
@ -2915,12 +3026,13 @@ func TestCatalog_ListServices_FilterACL(t *testing.T) {
defer codec.Close() defer codec.Close()
testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken("root")) testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken("root"))
opt := structs.DCSpecificRequest{ t.Run("request with user token without filter param sets ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1", Datacenter: "dc1",
QueryOptions: structs.QueryOptions{Token: token}, QueryOptions: structs.QueryOptions{Token: token},
} }
reply := structs.IndexedServices{} reply := structs.IndexedServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &opt, &reply); err != nil { if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
if _, ok := reply.Services["foo"]; !ok { if _, ok := reply.Services["foo"]; !ok {
@ -2932,6 +3044,58 @@ func TestCatalog_ListServices_FilterACL(t *testing.T) {
if !reply.QueryMeta.ResultsFilteredByACLs { if !reply.QueryMeta.ResultsFilteredByACLs {
t.Fatal("ResultsFilteredByACLs should be true") t.Fatal("ResultsFilteredByACLs should be true")
} }
})
const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
const bexpNotMatchingUserTokenPermissions = "ServiceName matches `b.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.Services))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.Services))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.Services))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestCatalog_ServiceNodes_FilterACL(t *testing.T) { func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
@ -2982,11 +3146,80 @@ func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
} }
require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we bexprMatchingUserTokenPermissions := fmt.Sprintf("Node matches `%s.*`", srv.config.NodeName)
// test node filtering down in acl.go for node cases. This also proves const bexpNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
// that we respect the version 8 ACL flag, since the test server sets
// that to false (the regression value of *not* changing this is better // Register a service of the same name on the denied node
// for now until we change the sense of the version 8 ACL flag). regArg := structs.RegisterRequest{
Datacenter: "dc1",
Node: "node-deny",
Address: "127.0.0.1",
Service: &structs.NodeService{
ID: "foo",
Service: "foo",
},
Check: &structs.HealthCheck{
CheckID: "service:foo",
Name: "service:foo",
ServiceID: "foo",
Status: api.HealthPassing,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &regArg, nil); err != nil {
t.Fatalf("err: %s", err)
}
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt = structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedServiceNodes{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.ServiceNodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt = structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedServiceNodes{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.ServiceNodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
opt = structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedServiceNodes{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.ServiceNodes))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestCatalog_NodeServices_ACL(t *testing.T) { func TestCatalog_NodeServices_ACL(t *testing.T) {
@ -3075,6 +3308,139 @@ func TestCatalog_NodeServices_FilterACL(t *testing.T) {
svc, ok := reply.NodeServices.Services["foo"] svc, ok := reply.NodeServices.Services["foo"]
require.True(t, ok) require.True(t, ok)
require.Equal(t, "foo", svc.ID) require.Equal(t, "foo", svc.ID)
const bexprMatchingUserTokenPermissions = "Service matches `f.*`"
const bexpNotMatchingUserTokenPermissions = "Service matches `b.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.NodeServices.Services))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.NodeServices.Services))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeServices{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Nil(t, reply.NodeServices)
require.False(t, reply.ResultsFilteredByACLs)
})
}
func TestCatalog_NodeServicesList_FilterACL(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
dir, token, srv, codec := testACLFilterServer(t)
defer os.RemoveAll(dir)
defer srv.Shutdown()
defer codec.Close()
testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken("root"))
opt := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{Token: token},
}
var reply structs.IndexedNodeServiceList
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &opt, &reply))
require.NotNil(t, reply.NodeServices)
require.Len(t, reply.NodeServices.Services, 1)
const bexprMatchingUserTokenPermissions = "Service matches `f.*`"
const bexpNotMatchingUserTokenPermissions = "Service matches `b.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeServiceList{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.NodeServices.Services))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeServiceList{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.NodeServices.Services))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeServiceList{}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Empty(t, reply.NodeServices.Services)
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestCatalog_GatewayServices_TerminatingGateway(t *testing.T) { func TestCatalog_GatewayServices_TerminatingGateway(t *testing.T) {

@ -9,7 +9,6 @@ import (
"time" "time"
metrics "github.com/armon/go-metrics" metrics "github.com/armon/go-metrics"
"github.com/armon/go-metrics/prometheus"
hashstructure_v2 "github.com/mitchellh/hashstructure/v2" hashstructure_v2 "github.com/mitchellh/hashstructure/v2"
"github.com/hashicorp/go-bexpr" "github.com/hashicorp/go-bexpr"
@ -22,33 +21,6 @@ import (
"github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/structs"
) )
var ConfigSummaries = []prometheus.SummaryDefinition{
{
Name: []string{"config_entry", "apply"},
Help: "",
},
{
Name: []string{"config_entry", "get"},
Help: "",
},
{
Name: []string{"config_entry", "list"},
Help: "",
},
{
Name: []string{"config_entry", "listAll"},
Help: "",
},
{
Name: []string{"config_entry", "delete"},
Help: "",
},
{
Name: []string{"config_entry", "resolve_service_config"},
Help: "",
},
}
// The ConfigEntry endpoint is used to query centralized config information // The ConfigEntry endpoint is used to query centralized config information
type ConfigEntry struct { type ConfigEntry struct {
srv *Server srv *Server
@ -280,7 +252,14 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe
return err return err
} }
// Filter the entries returned by ACL permissions. // Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
filteredEntries := make([]structs.ConfigEntry, 0, len(entries)) filteredEntries := make([]structs.ConfigEntry, 0, len(entries))
for _, entry := range entries { for _, entry := range entries {
if err := entry.CanRead(authz); err != nil { if err := entry.CanRead(authz); err != nil {

@ -783,7 +783,7 @@ service "foo" {
} }
operator = "read" operator = "read"
` `
id := createToken(t, codec, rules) token := createToken(t, codec, rules)
// Create some dummy service/proxy configs to be looked up. // Create some dummy service/proxy configs to be looked up.
state := s1.fsm.State() state := s1.fsm.State()
@ -804,7 +804,7 @@ operator = "read"
args := structs.ConfigEntryQuery{ args := structs.ConfigEntryQuery{
Kind: structs.ServiceDefaults, Kind: structs.ServiceDefaults,
Datacenter: s1.config.Datacenter, Datacenter: s1.config.Datacenter,
QueryOptions: structs.QueryOptions{Token: id}, QueryOptions: structs.QueryOptions{Token: token},
} }
var out structs.IndexedConfigEntries var out structs.IndexedConfigEntries
err := msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &out) err := msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &out)
@ -828,6 +828,58 @@ operator = "read"
require.Equal(t, structs.ProxyConfigGlobal, proxyConf.Name) require.Equal(t, structs.ProxyConfigGlobal, proxyConf.Name)
require.Equal(t, structs.ProxyDefaults, proxyConf.Kind) require.Equal(t, structs.ProxyDefaults, proxyConf.Kind)
require.False(t, out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false") require.False(t, out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
// ensure ACL filtering occurs before bexpr filtering.
const bexprMatchingUserTokenPermissions = "Name matches `f.*`"
const bexprNotMatchingUserTokenPermissions = "Name matches `db.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
args = structs.ConfigEntryQuery{
Kind: structs.ServiceDefaults,
Datacenter: s1.config.Datacenter,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
var reply structs.IndexedConfigEntries
err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &reply)
require.NoError(t, err)
require.Equal(t, 1, len(reply.Entries))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
args = structs.ConfigEntryQuery{
Kind: structs.ServiceDefaults,
Datacenter: s1.config.Datacenter,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprNotMatchingUserTokenPermissions,
},
}
var reply structs.IndexedConfigEntries
err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &reply)
require.NoError(t, err)
require.Zero(t, len(reply.Entries))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
args = structs.ConfigEntryQuery{
Kind: structs.ServiceDefaults,
Datacenter: s1.config.Datacenter,
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprNotMatchingUserTokenPermissions,
},
}
var reply structs.IndexedConfigEntries
err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &reply)
require.NoError(t, err)
require.Zero(t, len(reply.Entries))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestConfigEntry_ListAll_ACLDeny(t *testing.T) { func TestConfigEntry_ListAll_ACLDeny(t *testing.T) {

@ -63,18 +63,23 @@ func (h *Health) ChecksInState(args *structs.ChecksInStateRequest,
} }
reply.Index, reply.HealthChecks = index, checks reply.Index, reply.HealthChecks = index, checks
raw, err := filter.Execute(reply.HealthChecks) // Note: we filter the results with ACLs *before* applying the user-supplied
if err != nil { // bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.HealthChecks = raw.(structs.HealthChecks)
// Note: we filter the results with ACLs *after* applying the user-supplied raw, err := filter.Execute(reply.HealthChecks)
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include if err != nil {
// 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 err
} }
reply.HealthChecks = raw.(structs.HealthChecks)
return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks) return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks)
}) })
@ -111,18 +116,23 @@ func (h *Health) NodeChecks(args *structs.NodeSpecificRequest,
} }
reply.Index, reply.HealthChecks = index, checks reply.Index, reply.HealthChecks = index, checks
raw, err := filter.Execute(reply.HealthChecks) // Note: we filter the results with ACLs *before* applying the user-supplied
if err != nil { // bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := h.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.HealthChecks = raw.(structs.HealthChecks)
// Note: we filter the results with ACLs *after* applying the user-supplied raw, err := filter.Execute(reply.HealthChecks)
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include if err != nil {
// 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 err
} }
reply.HealthChecks = raw.(structs.HealthChecks)
return nil return nil
}) })
@ -303,6 +313,18 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
thisReply.Nodes = nodeMetaFilter(arg.NodeMetaFilters, thisReply.Nodes) thisReply.Nodes = nodeMetaFilter(arg.NodeMetaFilters, thisReply.Nodes)
} }
// Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := h.srv.filterACL(arg.Token, &thisReply); err != nil {
return err
}
raw, err := filter.Execute(thisReply.Nodes) raw, err := filter.Execute(thisReply.Nodes)
if err != nil { if err != nil {
return err return err
@ -310,13 +332,6 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
filteredNodes := raw.(structs.CheckServiceNodes) filteredNodes := raw.(structs.CheckServiceNodes)
thisReply.Nodes = filteredNodes.Filter(structs.CheckServiceNodeFilterOptions{FilterType: arg.HealthFilterType}) thisReply.Nodes = filteredNodes.Filter(structs.CheckServiceNodeFilterOptions{FilterType: arg.HealthFilterType})
// 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(arg.Token, &thisReply); err != nil {
return err
}
if err := h.srv.sortNodesByDistanceFrom(arg.Source, thisReply.Nodes); err != nil { if err := h.srv.sortNodesByDistanceFrom(arg.Source, thisReply.Nodes); err != nil {
return err return err
} }

@ -1527,11 +1527,62 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
require.True(t, found, "bad: %#v", reply.HealthChecks) require.True(t, found, "bad: %#v", reply.HealthChecks)
require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
// test node filtering down in acl.go for node cases. This also proves const bexprNotMatchingUserTokenPermissions = "ServiceName matches `b.*`"
// that we respect the version 8 ACL flag, since the test server sets
// that to false (the regression value of *not* changing this is better t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
// for now until we change the sense of the version 8 ACL flag). opt := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.HealthChecks))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.HealthChecks))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
opt := structs.NodeSpecificRequest{
Datacenter: "dc1",
Node: srv.config.NodeName,
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.HealthChecks))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestHealth_ServiceChecks_FilterACL(t *testing.T) { func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
@ -1571,11 +1622,77 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
require.Empty(t, reply.HealthChecks) require.Empty(t, reply.HealthChecks)
require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we // Register a service of the same name on the denied node
// test node filtering down in acl.go for node cases. This also proves regArg := structs.RegisterRequest{
// that we respect the version 8 ACL flag, since the test server sets Datacenter: "dc1",
// that to false (the regression value of *not* changing this is better Node: "node-deny",
// for now until we change the sense of the version 8 ACL flag). Address: "127.0.0.1",
Service: &structs.NodeService{
ID: "foo",
Service: "foo",
},
Check: &structs.HealthCheck{
CheckID: "service:foo",
Name: "service:foo",
ServiceID: "foo",
Status: api.HealthPassing,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &regArg, nil); err != nil {
t.Fatalf("err: %s", err)
}
const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
const bexprNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
require.NoError(t, err)
require.Equal(t, 1, len(reply.HealthChecks))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
require.NoError(t, err)
require.Zero(t, len(reply.HealthChecks))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
opt := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
require.NoError(t, err)
require.Zero(t, len(reply.HealthChecks))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestHealth_ServiceNodes_FilterACL(t *testing.T) { func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
@ -1607,11 +1724,77 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
require.Empty(t, reply.Nodes) require.Empty(t, reply.Nodes)
require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we // Register a service of the same name on the denied node
// test node filtering down in acl.go for node cases. This also proves regArg := structs.RegisterRequest{
// that we respect the version 8 ACL flag, since the test server sets Datacenter: "dc1",
// that to false (the regression value of *not* changing this is better Node: "node-deny",
// for now until we change the sense of the version 8 ACL flag). Address: "127.0.0.1",
Service: &structs.NodeService{
ID: "foo",
Service: "foo",
},
Check: &structs.HealthCheck{
CheckID: "service:foo",
Name: "service:foo",
ServiceID: "foo",
Status: api.HealthPassing,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &regArg, nil); err != nil {
t.Fatalf("err: %s", err)
}
const bexprMatchingUserTokenPermissions = "Service.Service matches `f.*`"
const bexprNotMatchingUserTokenPermissions = "Node.Node matches `node-deny.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedCheckServiceNodes{}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
require.NoError(t, err)
require.Equal(t, 1, len(reply.Nodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
opt := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedCheckServiceNodes{}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
require.NoError(t, err)
require.Zero(t, len(reply.Nodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
opt := structs.ServiceSpecificRequest{
Datacenter: "dc1",
ServiceName: "foo",
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedCheckServiceNodes{}
err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
require.NoError(t, err)
require.Zero(t, len(reply.Nodes))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestHealth_ChecksInState_FilterACL(t *testing.T) { func TestHealth_ChecksInState_FilterACL(t *testing.T) {
@ -1647,11 +1830,59 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
require.True(t, found, "missing service 'foo': %#v", reply.HealthChecks) require.True(t, found, "missing service 'foo': %#v", reply.HealthChecks)
require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
// We've already proven that we call the ACL filtering function so we const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
// test node filtering down in acl.go for node cases. This also proves const bexprNotMatchingUserTokenPermissions = "ServiceName matches `b.*`"
// that we respect the version 8 ACL flag, since the test server sets
// that to false (the regression value of *not* changing this is better t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
// for now until we change the sense of the version 8 ACL flag). req := structs.ChecksInStateRequest{
Datacenter: "dc1",
State: api.HealthPassing,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.HealthChecks))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.ChecksInStateRequest{
Datacenter: "dc1",
State: api.HealthPassing,
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.HealthChecks))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only service without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := structs.ChecksInStateRequest{
Datacenter: "dc1",
State: api.HealthPassing,
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexprNotMatchingUserTokenPermissions,
},
}
reply := structs.IndexedHealthChecks{}
if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.HealthChecks))
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestHealth_RPC_Filter(t *testing.T) { func TestHealth_RPC_Filter(t *testing.T) {

@ -550,18 +550,24 @@ func (s *Intention) List(args *structs.IntentionListRequest, reply *structs.Inde
} else { } else {
reply.DataOrigin = structs.IntentionDataOriginLegacy reply.DataOrigin = structs.IntentionDataOriginLegacy
} }
raw, err := filter.Execute(reply.Intentions)
if err != nil { // Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := s.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.Intentions = raw.(structs.Intentions)
// Note: we filter the results with ACLs *after* applying the user-supplied raw, err := filter.Execute(reply.Intentions)
// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include if err != nil {
// results that would be filtered out even if the user did have permission.
if err := s.srv.filterACL(args.Token, reply); err != nil {
return err return err
} }
reply.Intentions = raw.(structs.Intentions)
return nil return nil
}, },

@ -1639,6 +1639,11 @@ func TestIntentionList_acl(t *testing.T) {
token, err := upsertTestTokenWithPolicyRules(codec, TestDefaultInitialManagementToken, "dc1", `service_prefix "foo" { policy = "write" }`) token, err := upsertTestTokenWithPolicyRules(codec, TestDefaultInitialManagementToken, "dc1", `service_prefix "foo" { policy = "write" }`)
require.NoError(t, err) require.NoError(t, err)
const (
bexprMatch = "DestinationName matches `f.*`"
bexprNoMatch = "DestinationName matches `nomatch.*`"
)
// Create a few records // Create a few records
for _, name := range []string{"foobar", "bar", "baz"} { for _, name := range []string{"foobar", "bar", "baz"} {
ixn := structs.IntentionRequest{ ixn := structs.IntentionRequest{
@ -1691,12 +1696,29 @@ func TestIntentionList_acl(t *testing.T) {
require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
}) })
t.Run("filtered", func(t *testing.T) { // maskResultsFilteredByACLs() in rpc.go sets ResultsFilteredByACLs to false if the token is an empty string
// after resp.QueryMeta.ResultsFilteredByACLs has been determined to be true from filterACLs().
t.Run("filtered with no token should return no results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := &structs.IntentionListRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Filter: bexprMatch,
},
}
var resp structs.IndexedIntentions
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
require.Len(t, resp.Intentions, 0)
require.False(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
})
// has access to everything
t.Run("filtered with initial management token should return 1 and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := &structs.IntentionListRequest{ req := &structs.IntentionListRequest{
Datacenter: "dc1", Datacenter: "dc1",
QueryOptions: structs.QueryOptions{ QueryOptions: structs.QueryOptions{
Token: TestDefaultInitialManagementToken, Token: TestDefaultInitialManagementToken,
Filter: "DestinationName == foobar", Filter: bexprMatch,
}, },
} }
@ -1705,6 +1727,54 @@ func TestIntentionList_acl(t *testing.T) {
require.Len(t, resp.Intentions, 1) require.Len(t, resp.Intentions, 1)
require.False(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false") require.False(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
}) })
// ResultsFilteredByACLs should reflect user does not have access to read all intentions but has access to some.
t.Run("filtered with user token whose permissions match filter should return 1 and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := &structs.IntentionListRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token.SecretID,
Filter: bexprMatch,
},
}
var resp structs.IndexedIntentions
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
require.Len(t, resp.Intentions, 1)
require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
// ResultsFilteredByACLs need to act as though no filter was applied.
t.Run("filtered with user token whose permissions do match filter should return 0 and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := &structs.IntentionListRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token.SecretID,
Filter: bexprNoMatch,
},
}
var resp structs.IndexedIntentions
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
require.Len(t, resp.Intentions, 0)
require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
// ResultsFilteredByACLs should reflect user does not have access to read any intentions
t.Run("filtered with anonymous token should return 0 and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := &structs.IntentionListRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: "anonymous",
Filter: bexprMatch,
},
}
var resp structs.IndexedIntentions
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
require.Len(t, resp.Intentions, 0)
require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
} }
// Test basic matching. We don't need to exhaustively test inputs since this // Test basic matching. We don't need to exhaustively test inputs since this

@ -117,6 +117,18 @@ func (m *Internal) NodeDump(args *structs.DCSpecificRequest,
} }
reply.Index = maxIndex reply.Index = maxIndex
// Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := m.srv.filterACL(args.Token, reply); err != nil {
return err
}
raw, err := filter.Execute(reply.Dump) raw, err := filter.Execute(reply.Dump)
if err != nil { if err != nil {
return fmt.Errorf("could not filter local node dump: %w", err) return fmt.Errorf("could not filter local node dump: %w", err)
@ -129,13 +141,6 @@ func (m *Internal) NodeDump(args *structs.DCSpecificRequest,
} }
reply.ImportedDump = importedRaw.(structs.NodeDump) reply.ImportedDump = importedRaw.(structs.NodeDump)
// 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 := m.srv.filterACL(args.Token, reply); err != nil {
return err
}
return nil return nil
}) })
} }
@ -235,12 +240,25 @@ func (m *Internal) ServiceDump(args *structs.ServiceDumpRequest, reply *structs.
} }
} }
reply.Index = maxIndex reply.Index = maxIndex
}
// Note: we filter the results with ACLs *before* applying the user-supplied
// bexpr filter to ensure that the user can only run expressions on data that
// they have access to. This is a security measure to prevent users from
// running arbitrary expressions on data they don't have access to.
// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
// that results they don't have access to have been removed. If they were
// also allowed to run the bexpr filter on the data, they could potentially
// infer the specific attributes of data they don't have access to.
if err := m.srv.filterACL(args.Token, reply); err != nil {
return err
}
raw, err := filter.Execute(reply.Nodes) raw, err := filter.Execute(reply.Nodes)
if err != nil { if err != nil {
return fmt.Errorf("could not filter local service dump: %w", err) return fmt.Errorf("could not filter local service dump: %w", err)
} }
reply.Nodes = raw.(structs.CheckServiceNodes) reply.Nodes = raw.(structs.CheckServiceNodes)
}
if !args.NodesOnly { if !args.NodesOnly {
importedRaw, err := filter.Execute(reply.ImportedNodes) importedRaw, err := filter.Execute(reply.ImportedNodes)
@ -249,12 +267,6 @@ func (m *Internal) ServiceDump(args *structs.ServiceDumpRequest, reply *structs.
} }
reply.ImportedNodes = importedRaw.(structs.CheckServiceNodes) reply.ImportedNodes = importedRaw.(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 := m.srv.filterACL(args.Token, reply); err != nil {
return err
}
return nil return nil
}) })

@ -656,11 +656,73 @@ func TestInternal_NodeDump_FilterACL(t *testing.T) {
t.Fatal("ResultsFilteredByACLs should be true") t.Fatal("ResultsFilteredByACLs should be true")
} }
// We've already proven that we call the ACL filtering function so we // need to ensure that ACLs are filtered prior to bexprFiltering
// test node filtering down in acl.go for node cases. This also proves // Register additional node
// that we respect the version 8 ACL flag, since the test server sets regArgs := &structs.RegisterRequest{
// that to false (the regression value of *not* changing this is better Datacenter: "dc1",
// for now until we change the sense of the version 8 ACL flag). Node: "foo",
Address: "127.0.0.1",
WriteRequest: structs.WriteRequest{
Token: "root",
},
}
var out struct{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", regArgs, &out))
bexprMatchingUserTokenPermissions := fmt.Sprintf("Node matches `%s.*`", srv.config.NodeName)
const bexpNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeDump{}
if err := msgpackrpc.CallWithCodec(codec, "Internal.NodeDump", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.Dump))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeDump{}
if err := msgpackrpc.CallWithCodec(codec, "Internal.NodeDump", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.Dump))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only record without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: "",
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodeDump{}
if err := msgpackrpc.CallWithCodec(codec, "Internal.NodeDump", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Empty(t, reply.Dump)
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestInternal_EventFire_Token(t *testing.T) { func TestInternal_EventFire_Token(t *testing.T) {
@ -1064,6 +1126,113 @@ func TestInternal_ServiceDump_ACL(t *testing.T) {
require.Empty(t, out.Gateways) require.Empty(t, out.Gateways)
require.True(t, out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true") require.True(t, out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
}) })
// need to ensure that ACLs are filtered prior to bexprFiltering
// Register additional node
regArgs := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "node-deny",
ID: types.NodeID("e0155642-135d-4739-9853-b1ee6c9f945b"),
Address: "192.18.1.2",
Service: &structs.NodeService{
Kind: structs.ServiceKindTypical,
ID: "memcached",
Service: "memcached",
Port: 5678,
},
Check: &structs.HealthCheck{
Name: "memcached check",
Status: api.HealthPassing,
ServiceID: "memcached",
},
WriteRequest: structs.WriteRequest{
Token: "root",
},
}
var out struct{}
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", regArgs, &out))
const (
bexprMatchingUserTokenPermissions = "Service.Service matches `redis.*`"
bexpNotMatchingUserTokenPermissions = "Node.Node matches `node-deny.*`"
)
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
token := tokenWithRules(t, `
node "node-deny" {
policy = "deny"
}
node "node1" {
policy = "read"
}
service "redis" {
policy = "read"
}
`)
var reply structs.IndexedNodesWithGateways
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexprMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodesWithGateways{}
if err := msgpackrpc.CallWithCodec(codec, "Internal.ServiceDump", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Equal(t, 1, len(reply.Nodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
token := tokenWithRules(t, `
node "node-deny" {
policy = "deny"
}
node "node1" {
policy = "read"
}
service "redis" {
policy = "read"
}
`)
var reply structs.IndexedNodesWithGateways
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: token,
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodesWithGateways{}
if err := msgpackrpc.CallWithCodec(codec, "Internal.ServiceDump", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Zero(t, len(reply.Nodes))
require.True(t, reply.ResultsFilteredByACLs)
})
t.Run("request with filter that would match only record without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
var reply structs.IndexedNodesWithGateways
req := structs.DCSpecificRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{
Token: "", // no token
Filter: bexpNotMatchingUserTokenPermissions,
},
}
reply = structs.IndexedNodesWithGateways{}
if err := msgpackrpc.CallWithCodec(codec, "Internal.ServiceDump", &req, &reply); err != nil {
t.Fatalf("err: %s", err)
}
require.Empty(t, reply.Nodes)
require.False(t, reply.ResultsFilteredByACLs)
})
} }
func TestInternal_GatewayServiceDump_Terminating(t *testing.T) { func TestInternal_GatewayServiceDump_Terminating(t *testing.T) {

@ -81,19 +81,21 @@ func (s *serverInternalServiceDump) Notify(ctx context.Context, req *structs.Ser
return 0, nil, err return 0, nil, err
} }
totalNodeLength := len(nodes)
aclfilter.New(authz, s.deps.Logger).Filter(&nodes)
raw, err := filter.Execute(nodes) raw, err := filter.Execute(nodes)
if err != nil { if err != nil {
return 0, nil, fmt.Errorf("could not filter local service dump: %w", err) return 0, nil, fmt.Errorf("could not filter local service dump: %w", err)
} }
nodes = raw.(structs.CheckServiceNodes) nodes = raw.(structs.CheckServiceNodes)
aclfilter.New(authz, s.deps.Logger).Filter(&nodes)
return idx, &structs.IndexedCheckServiceNodes{ return idx, &structs.IndexedCheckServiceNodes{
Nodes: nodes, Nodes: nodes,
QueryMeta: structs.QueryMeta{ QueryMeta: structs.QueryMeta{
Index: idx, Index: idx,
Backend: structs.QueryBackendBlocking, Backend: structs.QueryBackendBlocking,
ResultsFilteredByACLs: totalNodeLength != len(nodes),
}, },
}, nil }, nil
}, },

@ -55,6 +55,10 @@ func TestServerInternalServiceDump(t *testing.T) {
Service: "web", Service: "web",
Kind: structs.ServiceKindTypical, Kind: structs.ServiceKindTypical,
}, },
{
Service: "web-deny",
Kind: structs.ServiceKindTypical,
},
{ {
Service: "db", Service: "db",
Kind: structs.ServiceKindTypical, Kind: structs.ServiceKindTypical,
@ -67,14 +71,14 @@ func TestServerInternalServiceDump(t *testing.T) {
})) }))
} }
authz := newStaticResolver( policyAuth := policyAuthorizer(t, `
policyAuthorizer(t, `
service "mgw" { policy = "read" } service "mgw" { policy = "read" }
service "web" { policy = "read" } service "web" { policy = "read" }
service "web-deny" { policy = "deny" }
service "db" { policy = "read" } service "db" { policy = "read" }
node_prefix "node-" { policy = "read" } node_prefix "node-" { policy = "read" }
`), `)
) authz := newStaticResolver(policyAuth)
dataSource := ServerInternalServiceDump(ServerDataSourceDeps{ dataSource := ServerInternalServiceDump(ServerDataSourceDeps{
GetStore: func() Store { return store }, GetStore: func() Store { return store },
@ -121,6 +125,42 @@ func TestServerInternalServiceDump(t *testing.T) {
result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh) result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh)
require.Empty(t, result.Nodes) require.Empty(t, result.Nodes)
}) })
const (
bexprMatchingUserTokenPermissions = "Service.Service matches `web.*`"
bexpNotMatchingUserTokenPermissions = "Service.Service matches `mgw.*`"
)
authz.SwapAuthorizer(policyAuthorizer(t, `
service "mgw" { policy = "deny" }
service "web" { policy = "read" }
service "web-deny" { policy = "deny" }
service "db" { policy = "read" }
node_prefix "node-" { policy = "read" }
`))
t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
eventCh := make(chan proxycfg.UpdateEvent)
require.NoError(t, dataSource.Notify(ctx, &structs.ServiceDumpRequest{
QueryOptions: structs.QueryOptions{Filter: bexprMatchingUserTokenPermissions},
}, "", eventCh))
result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh)
require.Len(t, result.Nodes, 1)
require.Equal(t, "web", result.Nodes[0].Service.Service)
require.True(t, result.ResultsFilteredByACLs)
})
t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
eventCh := make(chan proxycfg.UpdateEvent)
require.NoError(t, dataSource.Notify(ctx, &structs.ServiceDumpRequest{
QueryOptions: structs.QueryOptions{Filter: bexpNotMatchingUserTokenPermissions},
}, "", eventCh))
result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh)
require.Len(t, result.Nodes, 0)
require.True(t, result.ResultsFilteredByACLs)
})
}) })
} }

@ -1,109 +0,0 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package resource
import (
"fmt"
"github.com/hashicorp/go-bexpr"
"github.com/hashicorp/consul/proto-public/pbresource"
)
type MetadataFilterableResources interface {
GetMetadata() map[string]string
}
// FilterResourcesByMetadata will use the provided go-bexpr based filter to
// retain matching items from the provided slice.
//
// The only variables usable in the expressions are the metadata keys prefixed
// by "metadata."
//
// If no filter is provided, then this does nothing and returns the input.
func FilterResourcesByMetadata[T MetadataFilterableResources](resources []T, filter string) ([]T, error) {
if filter == "" || len(resources) == 0 {
return resources, nil
}
eval, err := createMetadataFilterEvaluator(filter)
if err != nil {
return nil, err
}
filtered := make([]T, 0, len(resources))
for _, res := range resources {
vars := &metadataFilterFieldDetails{
Meta: res.GetMetadata(),
}
match, err := eval.Evaluate(vars)
if err != nil {
return nil, err
}
if match {
filtered = append(filtered, res)
}
}
if len(filtered) == 0 {
return nil, nil
}
return filtered, nil
}
// FilterMatchesResourceMetadata will use the provided go-bexpr based filter to
// determine if the provided resource matches.
//
// The only variables usable in the expressions are the metadata keys prefixed
// by "metadata."
//
// If no filter is provided, then this returns true.
func FilterMatchesResourceMetadata(res *pbresource.Resource, filter string) (bool, error) {
if res == nil {
return false, nil
} else if filter == "" {
return true, nil
}
eval, err := createMetadataFilterEvaluator(filter)
if err != nil {
return false, err
}
vars := &metadataFilterFieldDetails{
Meta: res.Metadata,
}
match, err := eval.Evaluate(vars)
if err != nil {
return false, err
}
return match, nil
}
// ValidateMetadataFilter will validate that the provided filter is going to be
// a valid input to the FilterResourcesByMetadata function.
//
// This is best called from a Validate hook.
func ValidateMetadataFilter(filter string) error {
if filter == "" {
return nil
}
_, err := createMetadataFilterEvaluator(filter)
return err
}
func createMetadataFilterEvaluator(filter string) (*bexpr.Evaluator, error) {
sampleVars := &metadataFilterFieldDetails{
Meta: make(map[string]string),
}
eval, err := bexpr.CreateEvaluatorForType(filter, nil, sampleVars)
if err != nil {
return nil, fmt.Errorf("filter %q is invalid: %w", filter, err)
}
return eval, nil
}
type metadataFilterFieldDetails struct {
Meta map[string]string `bexpr:"metadata"`
}

@ -1,195 +0,0 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package resource
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/hashicorp/consul/proto/private/prototest"
"github.com/hashicorp/consul/sdk/testutil"
)
func TestFilterResourcesByMetadata(t *testing.T) {
type testcase struct {
in []*pbresource.Resource
filter string
expect []*pbresource.Resource
expectErr string
}
create := func(name string, kvs ...string) *pbresource.Resource {
require.True(t, len(kvs)%2 == 0)
meta := make(map[string]string)
for i := 0; i < len(kvs); i += 2 {
meta[kvs[i]] = kvs[i+1]
}
return &pbresource.Resource{
Id: &pbresource.ID{
Name: name,
},
Metadata: meta,
}
}
run := func(t *testing.T, tc testcase) {
got, err := FilterResourcesByMetadata(tc.in, tc.filter)
if tc.expectErr != "" {
require.Error(t, err)
testutil.RequireErrorContains(t, err, tc.expectErr)
} else {
require.NoError(t, err)
prototest.AssertDeepEqual(t, tc.expect, got)
}
}
cases := map[string]testcase{
"nil input": {},
"no filter": {
in: []*pbresource.Resource{
create("one"),
create("two"),
create("three"),
create("four"),
},
filter: "",
expect: []*pbresource.Resource{
create("one"),
create("two"),
create("three"),
create("four"),
},
},
"bad filter": {
in: []*pbresource.Resource{
create("one"),
create("two"),
create("three"),
create("four"),
},
filter: "garbage.value == zzz",
expectErr: `Selector "garbage" is not valid`,
},
"filter everything out": {
in: []*pbresource.Resource{
create("one"),
create("two"),
create("three"),
create("four"),
},
filter: "metadata.foo == bar",
},
"filter simply": {
in: []*pbresource.Resource{
create("one", "foo", "bar"),
create("two", "foo", "baz"),
create("three", "zim", "gir"),
create("four", "zim", "gaz", "foo", "bar"),
},
filter: "metadata.foo == bar",
expect: []*pbresource.Resource{
create("one", "foo", "bar"),
create("four", "zim", "gaz", "foo", "bar"),
},
},
"filter prefix": {
in: []*pbresource.Resource{
create("one", "foo", "bar"),
create("two", "foo", "baz"),
create("three", "zim", "gir"),
create("four", "zim", "gaz", "foo", "bar"),
create("four", "zim", "zzz"),
},
filter: "(zim in metadata) and (metadata.zim matches `^g.`)",
expect: []*pbresource.Resource{
create("three", "zim", "gir"),
create("four", "zim", "gaz", "foo", "bar"),
},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
run(t, tc)
})
}
}
func TestFilterMatchesResourceMetadata(t *testing.T) {
type testcase struct {
res *pbresource.Resource
filter string
expect bool
expectErr string
}
create := func(name string, kvs ...string) *pbresource.Resource {
require.True(t, len(kvs)%2 == 0)
meta := make(map[string]string)
for i := 0; i < len(kvs); i += 2 {
meta[kvs[i]] = kvs[i+1]
}
return &pbresource.Resource{
Id: &pbresource.ID{
Name: name,
},
Metadata: meta,
}
}
run := func(t *testing.T, tc testcase) {
got, err := FilterMatchesResourceMetadata(tc.res, tc.filter)
if tc.expectErr != "" {
require.Error(t, err)
testutil.RequireErrorContains(t, err, tc.expectErr)
} else {
require.NoError(t, err)
require.Equal(t, tc.expect, got)
}
}
cases := map[string]testcase{
"nil input": {},
"no filter": {
res: create("one"),
filter: "",
expect: true,
},
"bad filter": {
res: create("one"),
filter: "garbage.value == zzz",
expectErr: `Selector "garbage" is not valid`,
},
"no match": {
res: create("one"),
filter: "metadata.foo == bar",
},
"match simply": {
res: create("one", "foo", "bar"),
filter: "metadata.foo == bar",
expect: true,
},
"match via prefix": {
res: create("four", "zim", "gaz", "foo", "bar"),
filter: "(zim in metadata) and (metadata.zim matches `^g.`)",
expect: true,
},
"no match via prefix": {
res: create("four", "zim", "zzz", "foo", "bar"),
filter: "(zim in metadata) and (metadata.zim matches `^g.`)",
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
run(t, tc)
})
}
}
Loading…
Cancel
Save