Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACL Rules should be more expressive #5221

Open
drawks opened this issue Jan 14, 2019 · 16 comments
Open

ACL Rules should be more expressive #5221

drawks opened this issue Jan 14, 2019 · 16 comments
Labels
theme/acls ACL and token generation type/enhancement Proposed improvement or new feature type/umbrella-☂️ Makes issue the "source of truth" for multiple requests relating to the same topic

Comments

@drawks
Copy link
Contributor

drawks commented Jan 14, 2019

Current ACL matches are strictly exact match or prefix, looking through existing issues there are tickets where people have expressed interest in:

Feature Description

It seems there is plenty of demand for more expressive ACL matches, yet the exact change has been spread across multiple different issues (some closed) which makes the overall demand seem less by division in specificity.

Use Case(s)

  1. Variables expansion in ACL rules for things like $node_self, $self_registered_services, $self_node_tags, $self_metadata_kv
    This would allow for more easily spinning token per node with a small set of policies which could be used to very easily limit access based on the requesting node's own identity and properties

  2. regex/globs/wildcards/suffix in ACL rules seems pretty self explanatory. Simple prefix matches enables for one single opinionated organizational structure and access model. However it really handcuffs everyone to that structure without allowing for the decades of the demonstrable flexibility of regex or glob path matching available in all typical shell/unix tooling.

  3. negation for any of the match types would allow for easily describing policies which explicitly disallow access on match. This may be desirable in generally permissive environments and would make the default policy action of "allow" to be useful as more than a permissive mode while debugging freshly bootstrapped environments.

@pearkes pearkes added type/enhancement Proposed improvement or new feature theme/acls ACL and token generation labels Jan 15, 2019
@pearkes
Copy link
Contributor

pearkes commented Jan 15, 2019

Thanks for posting the feedback here. I think you did a good job summarizing the desire for a more expressive ACL system. Similar to my comment (#5110 (comment)) on, we want to leave this open to understand the needs here in more detail and to collect feedback for future ACL system improvements.

I'm actually going to close #5110 in favor of this issue, hopefully we can make this the central issue for those seeking expressiveness improvements in the ACL system.

@scalp42
Copy link
Contributor

scalp42 commented May 3, 2019

One example is the agent policy which need access to its own name for example:

node "mynode-1" {
  policy = "write"
}

The issue is that if you have 200 nodes, you still end up with 200 policies which are exactly the same besides the node name changing.

$node_self would be great in that scenario so you can have 200 tokens with just one policy.

@scalp42
Copy link
Contributor

scalp42 commented May 10, 2019

Consul 1.5.0 introduced ACL roles to attach multiple policies which is great but we're still missing a way to be able to interpolate some of the prefixes (like node name).

@Lasering
Copy link

Supporting globs and regular expressions is incompatible.
Variables such as $self would also be incompatible with regex (due to $).
Negation could be implemented with regex using negative lookhead/lookbehind.
Suffixes (and prefixes) could also be implemented with regular expressions.

The only problem is to implement regular expressions while also allowing variables such as $self.

@drawks
Copy link
Contributor Author

drawks commented Apr 16, 2020

Supporting globs and regular expressions is incompatible.
Variables such as $self would also be incompatible with regex (due to $).
Negation could be implemented with regex using negative lookhead/lookbehind.
Suffixes (and prefixes) could also be implemented with regular expressions.

The only problem is to implement regular expressions while also allowing variables such as $self.

It is a green field, nothing says that variables have to use $ as a token. There certainly is some combination of features as listed in the original issue that could be made to work.

I've no clue what the implementation complexity looks like, but if acl matching happened dynamically after running the rules through the template engine (already available in consul-template and vault agent) it would seem that a ton of context specific cars could be exposed to be used to write very flexible expressions.

@mkeeler
Copy link
Member

mkeeler commented Apr 16, 2020

@drawks Could you elaborate a bit about what the "negation" concept would be. Consul already allows you to write rules to deny access with policy = "deny" in the rule body. In that way you can use a default allow policy and specifically deny access to certain resources.

As for globs or regex, while from a user perspective it sounds simple enough, from the perspective of the internals it would require rewriting how ACLs are enforced. We are currently using radix trees for efficient lookups. Moving to something supporting regexes or globs would mean having to scrap that and instead build out something else. Thats not to say it can't/shouldn't be done but it just greatly increases the scope of change.

What are some of the types of globs or regexes that would be desired? Are we talking simple things to just make a rule match 1 of a few things such as the following:

service "(foo|bar|baz)" {
   policy = "write"
}

Or are we talking about more complex regexes?

@drawks
Copy link
Contributor Author

drawks commented Apr 16, 2020

@mkeeler I understand that lookups in radix allow for fast operation on prefixes and that more complex matches would incur some time complexity, but it doesn't need to be all one or the other does it? I don't have anything particularly smart to say about how this is done, but it seems like you could have a two pass look in the case that a user has opted into using the more costly matching syntax. Just because it isn't fast doesn't mean it isn't fast enough. I would, in some use cases, easily eat the lookup cost in exchange for having a much lower operational complexity in my policies.

That said, my personal wish list has variable expansion in policy evaluation as the banner feature request. Making more of the request context available for doing that radix lookup should be relatively cheap and would make things like @scalp42 's example possible.

This entire issue was just meant to capture multiple different suggested improvements into one place such that perhaps a single overarching solution might be developed which address as many of these use cases as possible.

@Lasering
Copy link

Lasering commented Apr 17, 2020

Or are we talking about more complex regexes?

I would be looking for something like this:

node "[^.]+.mongo\.example\.com" {
  policy "write"
}

And also being able to use the $self variable, would come very handy.

@mkeeler
Copy link
Member

mkeeler commented Apr 17, 2020

@drawks None of the ideas presented here are technically impossible and in the case of regex based rule matching, I believe it would be fast enough in most cases and don't see the minor performance overhead as a reason to not do it.

There is clearly a desire for a more expressive ACL rules language in Consul. Before going down the path of implementing any of these I want to try and get a good grasp on what kind of expressiveness would be useful. For example, if the desire is to be able to have a single rule target multiple segments such as a service rule that grants read access on a list of services, then that could be accomplished with regexes like the following rule:

service "(foo|bar|baz)" {
   policy = "read"
}

However it could also be accomplished other ways such as enabling policy templating to the extent of handling looping constructs in the rules language and having the template emit multiple rules in the compiled policy.

However it looks like there are cases such as the example from @Lasering that would be hard to express any way other than a full regex (using character classes so its not just a simple suffix match)

@Lasering
Copy link

Lasering commented Apr 18, 2020

However it looks like there are cases such as the example from @Lasering that would be hard to express any way other than a full regex (using character classes so its not just a simple suffix match)

What I'm really after, right now, is suffix match, it doesn't necessarily have to be using regex with character classes. Being able to interpolate $self would also be very welcome.

You could add node_suffix, kv_suffix, and so fourth. There is already node, and node_prefix, so I don't think that would be a very scalable approach. As an outsider I would suggest just having two "modes" node and node_regex. Both modes supporting $self. node could keep the current radix tree implementation, and node_regex would use a less efficient implementation but support all other cases and expressiveness.

@Lasering
Copy link

I thought a little more about this and globs would already solve most of the problems. It would allow prefixes, suffixes, "middleixes", character classes, and it would also make it easier to interpolate $self:

node "*.mongo.example.com" {
  policy "write"
}
node "mongo.example.*" {
  policy "write"
}
node "*.mongo-*.example.com" {
  policy "write"
}

@mkeeler
Copy link
Member

mkeeler commented Jun 29, 2020

While it doesn't adress many of the ACL improvements talked about in this issue, Consul 1.8.1 will contain ACL Node Identities to ease generation of Consul client agent tokens. For normal node registration, coordinate update and anti-entropy purposes you will be able to create a token with just the desired node identity and it will be granted all the necessary permissions to assume the identity of that node.

@ashwinkupatkar
Copy link

ashwinkupatkar commented Oct 1, 2020

@mkeeler I don't really understand how the node identities help here.

Node identities are not included as part of ACL roles. Also its really confusing with having so many ways and doing the same thing.

what @Lasering suggested is simple and easy to grasp and implement in actual deployments.

I was actually looking for such a possibility and stumbled upon this ticket. I am exactly looking for a way as @Lasering suggested above.

@drawks
Copy link
Contributor Author

drawks commented Sep 1, 2021

Any word of this gaining any internal traction? It is pretty rough that we are still stuck with creating a policy per node, 2.5 years after this issue was created. It seems to be a relatively popular issue with at least a few voices who've taken the time to clearly articulate their specific problem.

@jkirschner-hashicorp jkirschner-hashicorp added the type/umbrella-☂️ Makes issue the "source of truth" for multiple requests relating to the same topic label Sep 8, 2021
@blake
Copy link
Member

blake commented Sep 20, 2021

I don't really understand how the node identities help here.

Node identities are not included as part of ACL roles. Also its really confusing with having so many ways and doing the same thing.

@ashwinkupatkar Node identities allow you to create a token with a templated policy that grants the node permission to register itself into the catalog and perform various internal agent operations.

$ consul acl token create -node-identity=my-node:dc2
AccessorID:       66ef42fd-1b02-03e5-501a-df9ee0558966
SecretID:         f65f7c62-288e-f200-0063-62b653a6d43f
Description:
Local:            false
Create Time:      2021-09-20 18:40:15.89840038 +0000 UTC
Node Identities:
   my-node (Datacenter: dc2)

This grants the token the following permissions which are documented at https://www.consul.io/docs/security/acl/acl-system#acl-node-identities.

# Allow the agent to register its own node in the Catalog and update its network coordinates
node "<Node Name>" {
  policy = "write"
}

# Allows the agent to detect and diff services registered to itself. This is used during
# anti-entropy to reconcile the difference between the agent's knowledge of registered
# services and checks in comparison with what is known in the Catalog.
service_prefix "" {
  policy = "read"
}

Node identities are not included as part of ACL roles.

Node identities can also be applied to roles either created through the CLI or API.

It is pretty rough that we are still stuck with creating a policy per node, 2.5 years after this issue was created.

@drawks Node identities attempt to address this problem of needing to create a policy per node. Would you mind sharing a bit more detail as to why they are not a usable solution for you?

@drawks
Copy link
Contributor Author

drawks commented Sep 20, 2021

@blake Thanks for your note about node identities, I actually have used those some since I originally wrote this issue. They work well for the basic comms required for a node to stay coordinated with the cluster, It doesn't however address the larger goals of general flexibility in the ACL system.

The most basic example is that AFAIK there is currently no way to make a policy which grants write access to kv prefixs that include the current node name.

As an aside, the existence of node identities is currently not mentioned anywhere in the guide to production access control setup, which really isn't doing anyone any favors in discovering this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/acls ACL and token generation type/enhancement Proposed improvement or new feature type/umbrella-☂️ Makes issue the "source of truth" for multiple requests relating to the same topic
Projects
None yet
Development

No branches or pull requests

8 participants