Creating a group with the same name as an existing group is interpreted as a modification to the original group, even by a different user

[axfelix]

Describe the bug

We allow users to grant access other users access to their individual secrets by creating their own group and adding members to this group. However, we've noticed that when two users try to create new groups with the same name, attempts after the first are treated as modifications of the original group -- they are generated with the same unique ID, and the modify_index value of the group is incremented. This appears to be a bug, as the second user does not have permission to edit this group, but can accidentally modify it.

To Reproduce

1. Create multiple users using the userpass auth method and apply the policy "user" to them

# Grant permissions on user specific path
path "user-kv/data/{{identity.entity.name}}/*" {
    capabilities = [ "create", "update", "read", "delete", "list" ]
}

# For Web UI usage
path "user-kv/metadata" {
  capabilities = ["list"]
}

# Create groups
path "identity/group" {
  capabilities = [ "create", "update", "read", "delete", "list" ]
}

2. Login as one user and create a group:

$ vault login --method=userpass username="bob" password="training"
$ vault write --format=json identity/group name="bob_testing_group" member_entity_ids="bb7f63fd-b79a-6f13-cf79-b345247bb3c6"

{
  "request_id": "8b8904fd-b883-c3a1-76a5-a58186607744",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "id": "ab0f0361-5693-efaf-2a76-750600d75c90",
    "name": "bob_testing_group"
  },
  "warnings": null
}

3. Check the group details as root:

$ vault login $VAULT_DEV_ROOT_TOKEN_ID
$ vault read identity/group/id/ab0f0361-5693-efaf-2a76-750600d75c90

Key                  Value
---                  -----
alias                map[]
creation_time        2020-05-13T02:56:29.453492Z
id                   ab0f0361-5693-efaf-2a76-750600d75c90
last_update_time     2020-05-13T02:56:29.453492Z
member_entity_ids    [bb7f63fd-b79a-6f13-cf79-b345247bb3c6]
member_group_ids     <nil>
metadata             <nil>
modify_index         1
name                 bob_testing_group
namespace_id         root
parent_group_ids     <nil>
policies             <nil>
type                 internal

4. As another user, create a group:

$ vault login --method=userpass username="alice" password="training"
$ vault write --format=json identity/group name="bob_testing_group" member_entity_ids="f54bb47e-0636-8a1a-1456-bcd21de5343c"

{
  "request_id": "a8cb9c16-0403-3201-7053-acacc41e295a",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "id": "ab0f0361-5693-efaf-2a76-750600d75c90",
    "name": "bob_testing_group"
  },
  "warnings": null
}

Notice that the same group ID is returned.

5. Check the group details as root again:

$ vault login $VAULT_DEV_ROOT_TOKEN_ID
$ vault read identity/group/id/ab0f0361-5693-efaf-2a76-750600d75c90

Key                  Value
---                  -----
alias                map[]
creation_time        2020-05-13T02:56:29.453492Z
id                   ab0f0361-5693-efaf-2a76-750600d75c90
last_update_time     2020-05-13T02:58:08.100856Z
member_entity_ids    [f54bb47e-0636-8a1a-1456-bcd21de5343c]
member_group_ids     <nil>
metadata             <nil>
modify_index         2
name                 bob_testing_group
namespace_id         root
parent_group_ids     <nil>
policies             <nil>
type                 internal

Note that modify_index has been incremented, as though the API is interpreting this as a modify call rather than a create call.

Expected behavior
Group IDs should not be identical.

Environment:

• Vault Server Version (retrieve with vault status): 1.4.0
• Vault CLI Version (retrieve with vault version): 1.4.0
• macOS 10.13.6

Vault server configuration file(s): Server started in dev mode with no additional config provided.

[ncabatoff]

Hi @axfelix,

I'm not yet persuaded this is a bug. The identity/group endpoint is a little vague in its documentation as to what should happen if a group of the same name exists, but it does say it will update a group given the correct id, so it's not too surprising that it does the same thing by name. My guess is that this was the original API and the identity/group/id and identity/group/name endpoints were added so you could get more specific behaviour.

the second user does not have permission to edit this group, but can accidentally modify it.

The policy you posted didn't include the identity/group endpoint, can you elaborate on this?

[axfelix]

Hi,

When you say it didn't include the identity/group endpoint, looking at this:

# Create groups
path "identity/group" {
  capabilities = [ "create", "update", "read", "delete", "list" ]
}

I'm not totally sure what you mean, can you clarify?

[ncabatoff]

Sorry, I misspoke. What I should have said was the policy you posted doesn't show restrictions on that endpoint that enforce different behaviours for the two users.

[axfelix]

Oh, I see -- you're saying that there's no reason, based on the config we have, that they shouldn't both have access to any new groups?

[ncabatoff]

Yeah.

[axfelix]

Would it be necessary to use a block like identity/group/{{identity.entity.name}} when assigning all but create permissions on groups to avoid that? Is that the correct model?

[ncabatoff]

Sorry, can you specify the exact behaviour you want? I have some idea, but I'd rather you spelled it out. You want users to be able to create groups with arbitrary names (which you don't know in advance) that only they are allowed to modify?

[axfelix]

Yes, exactly! Sorry for the confusion, I think we'd assumed that would be closer to the default behaviour.

[ncabatoff]

I don't think there's any information attached to groups that corresponds to an "owner", so I don't know of any way to do what you want. We could probably make it work if you limited your requirements to allowing users to exclusively create groups with names having a certain prefix, such that e.g. Joe could only create groups starting with "a-" and Sue could only create groups starting with "b-".

[axfelix]

Any way to derive that prefix from a unique subset of, or their entire, username?

[ncabatoff]

I think so yes. What about

path "identity/group/name/{{identity.entity.name}}-*" {
  capabilities = [ "create", "update", "read", "delete", "list" ]
}

?

[axfelix]

That actually seems to confirm this as a bug!

path "identity/group/name/{{identity.entity.name}}-*" {
  capabilities = [ "create", "update", "read", "delete", "list" ]
}

allows users only to modify groups with names having their entity_name as the prefix. However, with the above block users cannot create new groups. To give users permission to create groups, the following is required, according to the API documentation: https://www.vaultproject.io/api-docs/secret/identity/group

path "identity/group" {
  capabilities = [ "create", "update", "read", "delete", "list" ]
}

However, with the above added to the policy, any users can modify any group by creating a new one with the same name.

[ncabatoff]

You've tested the policy I suggested? According to the API documentation at https://www.vaultproject.io/api-docs/secret/identity/group#create-update-group-by-name, re the endpoint I'm suggesting

This endpoint is used to create or update a group by its name.

and it does look like it's going through the same code path.

[axfelix]

Ah, it appears to work when using /identity/group/name/:name to create a group! Thanks for the help, I think this is OK after all.