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

[KeyVault] Keyvault access is broken in newer version on Managed Identity #21798

Closed
2 of 6 tasks
ashut0shk opened this issue May 10, 2022 · 14 comments
Closed
2 of 6 tasks
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Milestone

Comments

@ashut0shk
Copy link

  • Package Name: @azure/identity
  • Package Version: 4.4.0
  • Operating system: Windows 10
  • nodejs
    • version: 8.17
  • browser
    • name/version:
  • typescript
    • version: 3.9.5
  • Is the bug related to documentation in

Describe the bug
When using DefaultAzureCredential on a Managed Identity machine(azure vms), the credentials throw error while trying to access keyvault.SecretClient(url, cred). This makes complete inaccessibility of keyvault secrets.

To Reproduce
Steps to reproduce the behavior:

  1. On a managed identity use the following code to reproduce the issue.
const identity = require("@azure/identity");
const keyvault = require("@azure/keyvault-secrets");
const credentials = new identity.DefaultAzureCredential();
const vaultName = <yourVaultName>;
const vaultUrl = `https://${vaultName}.vault.azure.net`;
try{
  const client = new keyvault.SecretClient(vaultUrl, credentials);
  const secretValue= await client.getSecret(<secretName>);
} catch (e) {
  throw new Error("Failed accessing secrets from Key-vault: ", e);
}

Expected behavior
The above code snippet should not throw any errors, when all the resources are aligned properly.
Note that the above snippet gives error in v4.4.x while v4.3.x works like charm.

Screenshots
N/A

Additional context
The sdk is tested on a managed identity machine, and previous version seems to be working file.
NB: for Microsoft support team, you can refer to a ticket with TrackingID#2204050050000883, without disclosing further details.(confidential)

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 10, 2022
@azure-sdk azure-sdk added Client This issue points to a problem in the data-plane of the library. KeyVault needs-team-triage Workflow: This issue needs the team to triage. labels May 10, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label May 10, 2022
@maorleger
Copy link
Member

@sadasant / @KarishmaGhiya I wonder if this has something to do with multi-tenant support that was recently added to Identity?

I'm not too familiar with these changes so hopefully you could offer some guidance? https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/keyvault/keyvault-secrets/CHANGELOG.md#features-added-1

@ashut0shk turning on verbose logging could hopefully help here by providing detailed logs from @azure/identity - could you run the same scenario with the azure logger in verbose mode? Instructions can be found here: https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/identity/identity/README.md#logging

You can either set an environment variable AZURE_LOG_LEVEL to "verbose" or set the log level in code as per instructions.

Is there anything in the verbose logs that might help narrow down the problem?

@sadasant
Copy link
Contributor

@maorleger The multi-tenant feature was added through these two PRs:

To disable it, one can set the environment variable AZURE_IDENTITY_DISABLE_MULTITENANTAUTH. That could help us identify whether the multi-tenant feature is at fault.

@KarishmaGhiya
Copy link
Member

KarishmaGhiya commented May 14, 2022

Hello @ashut0shk I have tried the above code snippet with @azure/keyvault-secrets (v 4.4) and @azure/identity (v2.0.4) and it worked, provided the access policies are set correctly.
In order to help with the above issue, let's try a few things -

First, I want to see the exact error message you are seeing on your end. Make sure you don't have any env variables set while doing this, since you want to use managed identity. For that, can you modify your code so that the error message is getting printed on a new line and share that error message with me?

const identity = require("@azure/identity");
const keyvault = require("@azure/keyvault-secrets");
const credentials = new identity.DefaultAzureCredential();
const vaultName = "<vault-name>";
const vaultUrl = `https://${vaultName}.vault.azure.net`;

async function main(){
  try{
    const client = new keyvault.SecretClient(vaultUrl, credentials);
    const value= await client.getSecret("secretName");
    console.log(value);
  } catch (e) {
    console.log(e);
    throw new Error(e);
  }
}

main().catch((err)=>{
  console.error(err);
  process.exit(1);
});

Second, can you verify the following:

  • On the VM resource, Go under "Identity" -> select "System Assigned Managed Identity" -> Switch it to "On". On this same page, under Permissions, click "Azure Role Assignments" -> Add Role Assignment and add the appropriate Azure Role you want to assign to the keyvault resource you want to access

  • On the keyvault resource, Go under "Access Policy" -> Add an access policy. Under this you can select the required permissions for the keyvault resource and under the "Principal" select your VM resource that you have created. After you click apply, make sure to click "Save" on the "Access Policy" page.

@KarishmaGhiya KarishmaGhiya self-assigned this May 14, 2022
@KarishmaGhiya KarishmaGhiya added this to the [2022] June milestone May 14, 2022
@KarishmaGhiya KarishmaGhiya added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed needs-team-triage Workflow: This issue needs the team to triage. labels May 14, 2022
@ashut0shk
Copy link
Author

@maorleger The multi-tenant feature was added through these two PRs:

To disable it, one can set the environment variable AZURE_IDENTITY_DISABLE_MULTITENANTAUTH. That could help us identify whether the multi-tenant feature is at fault.

Hi @sadasant, I tried rechecking with setting up the variable AZURE_IDENTITY_DISABLE_MULTITENANTAUTH but with no luck. The same error is still there.

@ghost ghost added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels May 16, 2022
@ashut0shk
Copy link
Author

Hi @KarishmaGhiya, thank you for the suggestion. I have captures the exact output of the above code snippet.
PS: there aren't any environment variable setup on the machine.

Error: The challenge authorization URI 'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' is invalid.
at ChallengeBasedAuthenticationPolicy.parseWWWAuthenticate (C:\test\node_modules@azure\keyvault-secrets\dist\index.js:1226:19)
at ChallengeBasedAuthenticationPolicy.regenerateChallenge (C:\test\node_modules@azure\keyvault-secrets\dist\index.js:1333:36)
at ChallengeBasedAuthenticationPolicy.sendRequest (C:\test\node_modules@azure\keyvault-secrets\dist\index.js:1391:21)
at
at process._tickCallback (internal/process/next_tick.js:189:7)
Error: Error: The challenge authorization URI 'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' is invalid.
at main (C:\test\test.js:14:11)
at
at process._tickCallback (internal/process/next_tick.js:189:7)

Meanwhile I re-verified the appropriate role as well as access policy assignments. The error is same as appeared previously.

@ashut0shk
Copy link
Author

In case verbose logging information is needed, please refer below.
@maorleger thank you for the suggestion.

azure:identity:info EnvironmentCredential => Found the following environment variables:
azure:core-client:warning The baseUri option for SDK Clients has been deprecated, please use endpoint instead.
azure:core-client:warning The baseUri option for SDK Clients has been deprecated, please use endpoint instead.
azure:identity:info VisualStudioCodeCredential => Failed to load the Visual Studio Code configuration file. Error: ENOENT: no such file or directory, open 'C:\Users\<username>\AppData\Roaming\Code\User\settings.json'
azure:core-client:warning The baseUri option for SDK Clients has been deprecated, please use endpoint instead.
azure:core-http:info ServiceClient: using custom request policies
azure:keyvault-secrets:info Request: {
"streamResponseStatusCodes": {},
"url": "https://<vault-name>.vault.azure.net/secrets/<secret-name>/?api-version=7.3",
"method": "GET",
"headers": {
"_headersMap": {
"accept": "application/json",
"user-agent": "azsdk-js-keyvault-secrets/4.4.0 core-http/2.2.5 Node/v8.17.0 OS/(x64-Windows_NT-10.0.17763)",
"x-ms-client-request-id": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx25"
}
},
"withCredentials": false,
"timeout": 0,
"keepAlive": true,
"requestId": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx25"
}
azure:keyvault-secrets:info Response status code: 401
azure:keyvault-secrets:info Headers: {
"_headersMap": {
"cache-control": "no-cache",
"content-length": "97",
"content-type": "application/json; charset=utf-8",
"date": "Mon, 16 May 2022 07:23:01 GMT",
"expires": "-1",
"pragma": "no-cache",
"strict-transport-security": "REDACTED",
"www-authenticate": "Bearer authorization="https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\", resource="https://vault.azure.net\"",
"x-content-type-options": "REDACTED",
"x-ms-client-request-id": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx25",
"x-ms-keyvault-network-info": "conn_type=Ipv4;addr=xxx.xxx.xxx.xxx;act_addr_fam=InterNetwork;",
"x-ms-keyvault-region": "eastus",
"x-ms-keyvault-service-version": "1.9.395.1",
"x-ms-request-id": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx5f"
}
}
Error: The challenge authorization URI 'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' is invalid.
at ChallengeBasedAuthenticationPolicy.parseWWWAuthenticate (C:\test\node_modules@azure\keyvault-secrets\dist\index.js:1226:19)
at ChallengeBasedAuthenticationPolicy.regenerateChallenge (C:\test\node_modules@azure\keyvault-secrets\dist\index.js:1333:36)
at ChallengeBasedAuthenticationPolicy.sendRequest (C:\test\node_modules@azure\keyvault-secrets\dist\index.js:1391:21)
at
at process._tickCallback (internal/process/next_tick.js:189:7)
Error: Error: The challenge authorization URI 'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' is invalid.
at main (C:\test\test.js:14:11)
at
at process._tickCallback (internal/process/next_tick.js:189:7)

@sadasant
Copy link
Contributor

I currently believe the error comes specifically from this line in the code:

throw new Error(`The challenge authorization URI '${parsed.authorization}' is invalid.`);

However, I can't imagine why the code within the try statement would be a problem. If I try this locally it works:

> parsed = { authorization: https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> parsed = { authorization: 'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' }
{
  authorization: 'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
}
> parsed.authorization
'https://login.windows.net/34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
> tenantId = new URL(parsed.authorization).pathname.substring(1)
'34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
> parsed.tenantId = tenantId
'34xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'

I wonder two things:

  • I wonder if the authorization URI in the logs is incorrect (since the information is incomplete). @ashut0shk do you mind trying locally to see if you have any problem parsing the pathname of the authorization URI you see in the logs? I'd also recommend logging the error in the built code, to see if you spot something that we're currently missing.
  • I also wonder if there's another error in our source code that has the same string pattern, @KarishmaGhiya

@maorleger
Copy link
Member

Hi @ashut0shk - thanks for the verbose logging, this was really helpful!

I now noticed that in the issue description you mentioned you're using Node 8.x - is that correct?

If so, we no longer support Node 8.x as it is outside of our support policy - we highly recommend upgrading to a Node version that is maintained. Please feel free to refer to the Node.JS releases page for information about what versions are currently maintained by the community. Production applications should only use Active LTS or Maintenance LTS releases.

For more context - the URL class was made available on the global object as of Node 10.x and so the underlying error is likely to be "URL is not a function" (or something similar).

There is an issue here though - since we catch any exception and return this blanket "the challenge authorization URI .... is invalid" we are likely masking errors such as this. Maybe we should revisit that and either:

  • Let the original error message bubble up to the user instead of masking it
  • Only catch a specific error type when the URL truly is invalid, letting other error bubble up to the surface

Note to the team: All of this assumes this really is a Node 8 issue, and the author of the issue will confirm

@ashut0shk
Copy link
Author

ashut0shk commented May 16, 2022

Hi @maorleger

That is a viable catch. I wonder, however, how the previous version manages to circumvent the issue.
Meanwhile, I'll try to confirm the issues' regeneration with latest node LTS, that I believe could shed some light on it.

To answer the question, yes, the current node version on the machine is v8.17.xx

@KarishmaGhiya KarishmaGhiya added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels May 26, 2022
@KarishmaGhiya
Copy link
Member

@ashut0shk Checking with you if updating the node version resolves the issue you are having?

@ashut0shk
Copy link
Author

@KarishmaGhiya I have not been able to upgrade the node on the managed identity machine (mainly because of a lot of dependencies) to validate the existence of the issue with latest one. I, however, tried to reproduce the same on a regular machine and it appears that it does vanish away with latest Node LTS, i.e., v16.15.xx

NB: On regular machine with Node v8.xx, I was still hitting the same error. As if it's a underlying node library issue.

Wouldn't it be nice to have a little more specific error message, I wonder.

@ghost ghost added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels May 26, 2022
@KarishmaGhiya
Copy link
Member

I can definitely try to capture the original error message where we are using the URL class and throw that error message. Thanks!

@KarishmaGhiya KarishmaGhiya modified the milestones: [2022] June, [2022] July Jun 2, 2022
@pallavit pallavit removed the KeyVault label Jun 27, 2022
@xirzec xirzec modified the milestones: 2022-07, 2022-08 Jul 8, 2022
@xirzec xirzec modified the milestones: 2022-08, 2022-09 Aug 9, 2022
@GregorieCalderon
Copy link

Has this issue been resolved? Currently running into it now.

@timovv
Copy link
Member

timovv commented Sep 14, 2022

Hey @GregorieCalderon! The original issue here stemmed from the use of Node 8, which we no longer support (see above). If you're using an unsupported Node version, my recommendation would be to upgrade to a supported version, if possible. This would hopefully resolve the underlying issue for you, although the problem with the misleading error message persists for the time being.

@xirzec xirzec modified the milestones: 2022-09, 2022-12 Nov 1, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this issue Jan 4, 2023
Check in Appconfig scenarios and recording assets (Azure#21798)

* add appconfig control-plane scenarios

* touch swagger

* push tag successfully

* check in tag

* move assets.json to scenarios folder

* update tag

* add appconfig data-plane scenarios

* push data-plane apitest recording

* update mgmt recording

* restore swagger

* update recording
@xirzec xirzec modified the milestones: 2022-12, 2023-02 Jan 11, 2023
@xirzec xirzec closed this as completed Mar 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

9 participants