Connect does not work correctly with ACLs when service.ID != service.Name

[pierresouchay]

Overview of the Issue

When using Connect, the proxy service uses the ID of the proxied service.

So, given a Service:

{
  "ID": "my-id",
  "Name": "web",
}

Consul Connect will create a service:

{
  "ID": "my-id-proxy",
  "Name": "my-id-proxy"
}

If ACL for the user is

"service": {
    "": {
      "policy": "read"
    },
    "web": {
      "policy": "write"
    }
}

the proxy will fail to register in the cluster.

Consul info for both Client and Server

Consul 1.2.2 with patches

[pierresouchay]

@banks @mkeeler Fix provided here: #4620

[pierresouchay]

This fix has been tested successfully on a real cluster

[banks]

Thanks Pierre.

This is a slightly broader issue than the fix PR as I mentioned there although that is worth merging for now.

The biggest problem is that soon ACLs will be exact-match by default and so even with the fix an ACL allowing web service registration will not automatically allow web-proxy too.

I think the correct fix for that is to manage proxy ACLs a little differently based on their destination instead of their actual name, and/or make it easier to grant ACLs for a service and it's proxy. Both need more thought though.

We can close this once the PR merges but we'll consider that case again for new ACLs.

[pierresouchay]

ACL will exact match?
Huh... We are counting a lot on the prefix mechanism now

[banks]

Prefix will still be possible of course just not the default since it allows too many "footguns" where things have much more access than the operator intended. Will also continue to support existing ACLs without breaking back-compat and provide a simple way to migrate the policies to equivalent new ones if you want to upgrade them in bulk too so don't worry too much. It's more a thing for us to consider since the general default/recommendation would be for precise exact-match service ACLs but we still need connect proxies to work.

[banks]

Going to close this as #4620 fixes the immediate issue.

Thanks @pierresouchay and @uepoch!