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

[jmurret]

Description

Allows bexpr filters constructed from user input only on data that the user is authorized to read.

Effect on the meaning X-Consul-Results-Filtered-By-ACLs when used with filter

With this change, when using the filter query parameter, the meaning of X-Consul-Results-Filtered-By-ACLs changes in the following way:

• currently it means there are other resources that you do not have access to that match your filter
• the new meaning will be there are other resources that you do not have access to that may or may not match your filter

In essence, the X-Consul-Results-Filtered-By-ACLs header response as though there was no filter.

Let's assume this set up:

• 10 Intentions exist
• 5 of them have a DestinationName starting with web-
• filter query param is set to:

DestinationName+matches+`web-.*`

• the ACL token has read access to 1 intention with a DestinationName of api-2

In the current implementation on the main branch, you would have this logic flow:

• filter param gets 0 of the 10 intentions
• ACL filter then operates on an empty array and leaves X-Consul-Results-Filtered-By-ACLs set to false
• response has zero items and X-Consul-Results-Filtered-By-ACLs is set to false

In changing the logic, the flow would now be:

• ACL filter then removes 5 items and sets the X-Consul-Results-Filtered-By-ACLs to true
• filter param then matches zero results
• response has zero items and X-Consul-Results-Filtered-By-ACLs is set to true
• at this point, X-Consul-Results-Filtered-By-ACLs indicates there are more resources that the token has access to, but there is no guarantee that the filter query parameter would filter them out.

We will need to document this in our docs as well as the change in behavior in our changelog.

Testing & Reproduction steps

List of Effected Code

Based on GitHub

• Catalog.ListNodes()
• Catalog.ListServices()
• Catalog.ServiceNodes()
• Catalog.NodeServices()
• Catalog.NodeServiceList()
• Internal.NodeDump()
• Internal.ServiceDump()
• internal/resource/filter.go - unused reference. Deleted file and test file.
• serverInternalServiceDump.Notify()
• Health.ChecksInState()
• Health.NodeChecks()
• Health.ServiceChecks()
• Health.ServiceNodes()
• ConfigEntry.List() - already filtered ACLs first but needed tests to assert this.
• HttpHandlers.AgentServices()
• HttpHandlers.AgentChecks
• HttpHandlers.AgentMembers()
• Intentions.List()

These references show up in the search for go-expr used in the vulnerability report, but do not create and run filters, they only use an evaluator to validate the syntax is valid and do not expose object data:

• serverHealthBlocking.Notify() - uses structs.CheckServiceNodes which does not have QueryMeta the way that IndexedCheckServiceNodes does.
• AutoConfigAuthorizer.Authorize()
• Binder.Bind()
• Exported Services CLI command
• newFilterEvaluator()
• ServiceResolverConfigEntry.Validate()
• ACL.BindingRuleDelete()
• validateAutoConfigAuthorizer()
• filterSubsetEndpoints()

PR Checklist

• updated test coverage
• external facing docs updated
• appropriate backport labels added
• not a security concern

[jmurret]

random unused code in the file I was modifying.

[sarahalsmiller]

This was a big one to cover, thanks for tackling it and adding tests.

[hc-github-team-consul-core]

📣 Hi @jmurret! a backport is missing for this PR [21950] for versions [1.15,1.18,1.19,1.20] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>