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

AppRole Multiple Secret IDs Break Metadata #12797

Closed
joeyberkovitz opened this issue Oct 11, 2021 · 3 comments
Closed

AppRole Multiple Secret IDs Break Metadata #12797

joeyberkovitz opened this issue Oct 11, 2021 · 3 comments

Comments

@joeyberkovitz
Copy link

Describe the bug
When using AppRole for machine authentication, I have different machine metadata encoded onto each generated secret ID, as is allowed by the Generate Secret ID API request. The problem with this is that on the Identity backend, only a single entity ID is created, so the metadata is overwritten with the latest value anytime any secret ID is used with the same AppRole Role ID.

This is problematic because some elements of Vault can be setup to use the metadata to define restrictions or policies and having the metadata change unexpectedly can lead to security vulnerabilities where hosts gain access to resources they shouldn't have access to, and similarly denial of service with hosts losing access to resources they should have access to.

I looked at the AppRole code and it appears that a potential fix could involve using the entity.SecretIDAccessor as the Alias name in cases where the AppRole BindSecretID parameter is true. This change would occur here: https://github.com/hashicorp/vault/blob/main/builtin/credential/approle/path_login.go#L306. The only other place that uses the RoleID as the Alias Name is the alias-lookahead operation (https://github.com/hashicorp/vault/blob/main/builtin/credential/approle/path_login.go#L51). I'm not sure what the intention of the lookahead is given that it's not referenced in any documentation that I can find so I don't know if it would be important to use the secret ID accessor here as well since loading it would require a lookup against the Vault backend. Alternatively, the secret ID HMAC could be used as the role name, although this could be interpreted as less secure since the secret ID accessor is meant to be the method for identifying a secret ID without requiring access to that secret ID.

To Reproduce
Steps to reproduce the behavior:

  1. Run vault write -force auth/approle/role/TEST-ROLE to create a test role, note generated role_id
  2. Run vault write auth/approle/role/TEST-ROLE/secret-id metadata='{"key": "secret1"}' and note the secret_id
  3. Run vault write auth/approle/role/TEST-ROLE/secret-id metadata='{"key": "secret2"}' and note the secret_id
  4. Login using first pair of credentials, and find the associated entity using vault write identity/lookup/entity alias_id=ROLE_ID where role ID is the AppRole role ID. Note alias ID
  5. Read metadata from alias: vault read identity/entity-alias/id/ALIAS_ID and note that metadata should indicate secret1
  6. Login using second pair of credentials
  7. Re-read metadata using same alias ID and note that metadata indicates secret2

Expected behavior
Expect that each secret ID has an associated unique alias and potentially a separate entity so that metadata can be stored properly

Environment:

  • Vault Server Version (retrieve with vault status): 1.8.1
  • Vault CLI Version (retrieve with vault version): 1.8.1
  • Server Operating System/Architecture: CentOS 7
@nvx
Copy link
Contributor

nvx commented Oct 13, 2021

Is this the same as #11798?

@joeyberkovitz
Copy link
Author

Appears to be although the existing name doesn't imply any relevance to AppRole

@hghaf099
Copy link
Contributor

Thanks for submitting this issue. As pointed out by @nvx, it seems that this is a duplicate of #11798, as such I am going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants