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

Azure Key Vault auto-unseal fails when authenticating from an Azure instance with more than one MSI #7115

Closed
null-route opened this issue Jul 12, 2019 · 23 comments
Assignees
Labels
bug Used to indicate a potential bug core/seal ecosystem

Comments

@null-route
Copy link

null-route commented Jul 12, 2019

Describe the bug
Azure Key Vault auto-unseal fails when using MSI authentication from an instance with more than one service identity.

To Reproduce
Steps to reproduce the behavior:

  1. Configure Vault on an Azure instance that has access to a Key Vault and key
  2. Assign more than one user assigned managed identity to that instance
  3. Configure Vault to use:
seal "azurekeyvault" {
  tenant_id      = "[tenant_id]"
  client_id      = "[client_id_of_desired_identity]"
  vault_name     = "[name_of_keyvault]"
  key_name       = "[name_of_key]"
}

Expected behavior
Vault would attempt the retrieve credentials for the desired identity by using the provided client_id. Instead, it errors out with:

Error parsing Seal configuration: error fetching Azure Key Vault seal key information: azure.BearerAuthorizer#WithAuthorization: Failed to refresh the Token for request to https://xxx.vault.azure.net/keys/azure-dev-vault-key/?api-version=2016-10-01: StatusCode=400 -- Original Error: adal: Refresh request failed. Status Code = '400'. Response body: {"error":"invalid_request","error_description":"Identity not found"}

I believe this is because it's making a call to auth.NewMSIConfig(), which does not support the use of multiple MSIs. It might make sense to make the client_id a required value and instead call GetMSI():

// GetMSI creates a MSI config object from the available client ID.
func (settings EnvironmentSettings) GetMSI() MSIConfig {
	config := NewMSIConfig()
	config.Resource = settings.Values[Resource]
	config.ClientID = settings.Values[ClientID]
	return config
}

Environment:

  • Vault Server Version (retrieve with vault status): 1.1.1
  • Vault CLI Version (retrieve with vault version): 1.1.1
  • Server Operating System/Architecture: Centos7

Vault server configuration file(s):

ui = true
api_addr = "https://xxx:8200"
cluster_addr = "https://xxx:8201"

listener "tcp" {
  address         = "xxx:8200"
  cluster_address = "xxx:8201"
  tls_disable     = "false"
  tls_key_file    = "xxx"
  tls_cert_file   = "xxx"
}

listener "tcp" {
  address         = "127.0.0.1:8200"
  tls_disable     = "true"
}

storage "consul" {
  address = "127.0.0.1:8500"
  path    = "vault/"
}

seal "azurekeyvault" {
  tenant_id      = "xxx"
  client_id      = "xxx"
  vault_name     = "xxx"
  key_name       = "xxx"
}

telemetry {
  statsd_address = "127.0.0.1:8125"
}
@kalafut kalafut self-assigned this Jul 12, 2019
@chrishoffman chrishoffman added the bug Used to indicate a potential bug label Aug 19, 2019
@catsby
Copy link
Member

catsby commented Feb 5, 2020

Hello! I'm following on your comment about auth.NewMSIConfig(). That operation is done in they hashicorp/go-kms-wrapping library, and it seems we call NewMSIConfig() if both clientID and clientSecret are empty:

I admit I'm not too familiar with auto-unsealing in Azure, but I do notice the example configuration you shared is missing any mention of client_secret. Is it possible that's the issue, or was it simply omitted because it's sensitive information?

@catsby
Copy link
Member

catsby commented Mar 6, 2020

Hello - we haven't heard back in some time. If you have more information please let us know! Otherwise I'll be closing this issue next week, approximately Tuesday, March 10th 2020.

@catsby
Copy link
Member

catsby commented Mar 18, 2020

Hello - I missed the March 10th cut-off for closing this, but seeing how we've passed it without any reply, I'm going to go ahead and close it now. Please let us know if you do find more information. Thanks!

@catsby catsby closed this as completed Mar 18, 2020
@null-route
Copy link
Author

Hey; sorry for the late response but this is still an issue. I've been working around it for a few months but ran into it again when attempting to set up an Azure secrets engine to replicate some of the same functionality we provide with the AWS secrets engine on AWS. @catsby I'm not including client_secret because there's supposed to be a fallback to managed identities[0] when application registration credentials aren't provided directly:

If the client ID or secret are not present and Vault is running on and Azure VM, Vault will attempt to use Managed Service Identity (MSI) to access Azure. Note that when MSI is used, tenant and subscription IDs must still be explicitly provided in the configuration or environment variables.

The issue is that this fallback only works with system-assigned managed identities (and maybe if the virtual machine in question only has one user-assigned managed identity, I haven't dug around in the code to confirm). If you've assigned multiple user-assigned managed identities to a single virtual machine, Azure requires you to specify the client_id of the managed identity that you intend to assume as part of your API call. From their docs [1]:

The client ID parameter specifies the identity for which the token is requested. This value is required for disambiguation when more than one user-assigned identity is on a single VM.

Vault does not appear to use the specified client_id to request a specific user-assigned identity, even on our current version (1.2.3). I believe the following issue may also be related:

#8082

[0] https://www.vaultproject.io/docs/secrets/azure/#authentication
[1] https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/overview

@null-route
Copy link
Author

hashicorp/vault-plugin-auth-azure#29 may fix it actually. What's the expected ETA on reviewing and releasing a build with those commits?

@null-route
Copy link
Author

New PR for the fix: hashicorp/vault-plugin-auth-azure#33

@null-route
Copy link
Author

@catsby can you reopen this or should I create another issue?

@narayan-iyengar
Copy link

@null-route does this hashicorp/vault-plugin-auth-azure#33 fix your issue?
Sorry just reading comments and haven't gone through the whole history yet...

@null-route
Copy link
Author

null-route commented Apr 9, 2020

@narayan-iyengar I think it should, but I'm not sure. I haven't tested it yet.

@narayan-iyengar
Copy link

ok cool. I think we are going merge the above PR. If you still see an issue please reopen or create a new issue

@null-route
Copy link
Author

@narayan-iyengar when will a version of the azure auth plugin containing that change be included in a Vault release?

@null-route
Copy link
Author

Oh, I see it got added to the 1.4.X milestone, so hopefully soon.

@skatsaounis
Copy link

@catsby it seems the issue is still present in Vault v1.10.0:

Error parsing Seal configuration: error fetching Azure Key Vault wrapper key information: azure.BearerAuthorizer#WithAuthorization: Failed to refresh the Token for request to https://xxx.vault.azure.net/keys/hashicorp-xxx-vault-cloudseal/?api-version=7.0: StatusCode=400 -- Original Error: adal: Refresh request failed. Status Code = '400'. Response body: {"error":"invalid_request","error_description":"Multiple user assigned identities exist, please specify the clientId / resourceId of the identity in the token request"} Endpoint xxx

I think that the linked change is going to fix it eventually: hashicorp/go-kms-wrapping#97

@skatsaounis
Copy link

Can someone please re-open this issue as it is still present? In addition, can someone review my bug-fix PR in the dependent go module so as to open a subsequent PR here with a bump to go.mod?

Thank you in advance 😃

@Badgerati
Copy link

We're suffering from the same issue on v1.11.2 as well.

For us we enabled Insights on an AKS instance to help triage an issue, but it then broke Vault's auto-unsealing from AKV with the "Multiple user assigned identities exist" error. We attempted to set client_id in the seal config block, but it still failed.

To fix it, we had to disable Insights on the cluster (we used Terraform to do so), and then we had to manually restart each VM in the VMSS for the cluster to get the auto-unseal working again (ie: only one user assign identity) - as simply disabling Insights wasn't enough.

Is there any word on when this will get fixed? As I can only imagine we'll face this issue again in the future.

@skatsaounis
Copy link

skatsaounis commented Aug 9, 2022

Hi @Badgerati! My PR on the dependent module was merged (hashicorp/go-kms-wrapping#97) and was back-ported to branch for Vault 1.11.x releases (hashicorp/go-kms-wrapping#104).

I am still waiting for it to be consumed by Vault with a PR to go.mod here and then, back-ported to Vault 1.11.x by someone of the Vault team.

In my opinion, this bug here should be re-opened as it was closed without fixing the issue. @cipherboy is the one who is looking at this issue so we can only wait for his actions :)

@EvertonSA
Copy link

+1

@cipherboy
Copy link
Contributor

Hi sorry all -- I didn't get permissions to tag this in time for 1.11.4, so I'll have to revisit for 1.11.5. In the mean time, 1.12.0-RC1 is out with this fix definitely in that release, and the GA should be around the corner. Thanks again @skatsaounis for the PR and sorry for dropping the ball on the 1.11.x backport.

Pinging @sgmiller to get a new tag of go-kms-wrapping for 1.11.5...

@EvertonSA
Copy link

@cipherboy Failed to pull image "hashicorp/vault:1.12.0-RC1": rpc error: code = NotFound desc = failed to pull and unpack image "docker.io/hashicorp/vault:1.12.0-RC1": failed to resolve reference "docker.io/hashicorp/vault:1.12.0-RC1": docker.io/hashicorp/vault:1.12.0-RC1: not found

@EvertonSA
Copy link

never mind, image is actualy 1.12.0-rc1

@EvertonSA
Copy link

I confirm that 1.12.0-rc1 is working when multiple MSI are available on the node. if we could please add that to 1.11.X it would be great. thanks

@EvertonSA
Copy link

@sgmiller can you help us to have this on 1.11.5?

@pve84
Copy link

pve84 commented Jan 6, 2023

For anyone who is wondering. This issue is not yet fixed in 1.11.5 or 1.11.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/seal ecosystem
Projects
None yet
Development

No branches or pull requests