Fix panic when role names had a slash in them

[raskchanky]

Overview

This fixes a bug where static roles that have slashes in the name will cause Vault to panic when it starts up.

The reproduction steps in the linked issue are solid, whether you use podman or not, and reliably reproduce the problem. The panic comes from the fact that the role name has a slash in it, dir1/user1 in the linked issue. This causes a problem because when Vault restarts, and initializes this plugin, the plugin populates a rotation queue but it only uses the first part of the role name, dir1/ in this case. That causes a nil entry to get retrieved from storage, and hence the panic. The sequence of events looks like this:

1. https://github.com/hashicorp/vault-plugin-secrets-openldap/blob/main/backend.go#L89
2. https://github.com/hashicorp/vault-plugin-secrets-openldap/blob/main/backend.go#L101
3. https://github.com/hashicorp/vault-plugin-secrets-openldap/blob/main/rotation.go#L449
4. https://github.com/hashicorp/vault-plugin-secrets-openldap/blob/main/rotation.go#L461
5. https://github.com/hashicorp/vault-plugin-secrets-openldap/blob/main/rotation.go#L35
6. https://github.com/hashicorp/vault-plugin-secrets-openldap/blob/main/rotation.go#L45

The problem is that on line 45, from step 6, roles looks like this: []string{"dir1/"} when it should look like []string{"dir1/user1"}. This is due to the mechanics of how Vault's List() storage call works.

The net effect of having a truncated entry in the roles slice is that this line ends up retrieving a nil value for role because b.staticRole() returns nil, nil when something isn't found instead of returning an error. The actual panic occurs here.

I can't imagine that we're going to change the semantics of how List() works, as that would break literally everything, so my instinct here was to 1) not allow role names with slashes in them and 2) if the role is nil, add a warning to the log and skip it.

But, I'm not on the ecosystem team and I don't know a lot about LDAP 😅 so it's entirely possible this is a terrible solution! @davidadeleon mentioned on Slack that hierarchical static roles are a thing, so maybe given that, this is a bad fix. Maybe it's enough to just skip nil role names and warn, but still let people create static roles with slashes in the name? But then those static roles with slashes in the name will never have automatic credential rotation. So yeah, I'm not sure what the right answer here is. But I figured it was worth putting this PR up anyway and maybe saving someone some investigation time. Feel free to close completely if I'm way off base with this.

Related Issues/Pull Requests

• Issue #27907

Contributor Checklist

• Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet
• Add output for any tests not ran in CI to the PR description (eg, acceptance tests)
• Backwards compatible

[raskchanky]

I'm happy to add some tests for any parts of this that look reasonable but I figured it was better to see if it was even the right approach before spending that time.

[fairclothjm]

Closing in favor of #115

Thanks @raskchanky !