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 - SecretClient.GetSecret(string key) return Status: 401 (Unauthorized) #9737

Closed
macchmie3 opened this issue Jan 31, 2020 · 18 comments · Fixed by #10673
Closed

KeyVault - SecretClient.GetSecret(string key) return Status: 401 (Unauthorized) #9737

macchmie3 opened this issue Jan 31, 2020 · 18 comments · Fixed by #10673
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Milestone

Comments

@macchmie3
Copy link

Describe the bug
When I am calling SecretClient.GetSecret(key) I am sometimes getting: Azure.RequestFailedException: Status: 401 (Unauthorized). This method is usually called multiple times in parallel using single instance of SecretClient. The constructor I use for secret client: new SecretClient(new Uri(keyVaultUrl), new ManagedIdentityCredential()); Example from my logs says that it can fail 3 times for 6 concurrent requests.

Expected behavior
The method should not throw.

Actual behavior (include Exception or Stack Trace)

Azure.RequestFailedException: 
Status: 401 (Unauthorized)

Content:
{""error"":{""code"":""Unauthorized"",""message"":""Request is missing a Bearer or PoP token.""}}

Headers:
Cache-Control: no-cache
Pragma: no-cache
Server: Microsoft-IIS/10.0
WWW-Authenticate: Bearer authorization=""https://login.windows.net/d37240c9-786e-4171-a9f4-37cfd2adf51f"", resource=""https://vault.azure.net""
x-ms-keyvault-region: northeurope
x-ms-request-id: 1f648564-3db0-41b3-bba3-7fadec5198cb
x-ms-keyvault-service-version: 1.1.0.891
x-ms-keyvault-network-info: addr=40.113.80.190;act_addr_fam=InterNetwork;
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Strict-Transport-Security: max-age=31536000;includeSubDomains
X-Content-Type-Options: nosniff
Date: Fri, 31 Jan 2020 09:07:17 GMT
Content-Length: 87
Content-Type: application/json; charset=utf-8
Expires: -1

   at Azure.Security.KeyVault.KeyVaultPipeline.SendRequest(Request request, CancellationToken cancellationToken)
   at Azure.Security.KeyVault.KeyVaultPipeline.SendRequest[TResult](RequestMethod method, Func`1 resultFactory, CancellationToken cancellationToken, String[] path)
   at Azure.Security.KeyVault.Secrets.SecretClient.GetSecret(String name, String version, CancellationToken cancellationToken)

To Reproduce
Steps to reproduce the behavior (include a code snippet, screenshot, or any additional information that might help us reproduce the issue)

Call GetSecret in parralel (for same resource) with KeyVaultTestClient singleton

public class KeyVaultTestClient
    {
        private readonly ConcurrentDictionary<string, string> _cachedSecrets = new ConcurrentDictionary<string, string>();
        private readonly SecretClient _secretClient;

        public KeyVaultTestClient(string keyVaultUrl)
        {
            _secretClient = new SecretClient(new Uri(keyVaultUrl), new ManagedIdentityCredential());
        }

        public string GetSecret(string key)
        {
            if (_cachedSecrets.ContainsKey(key))
            {
                return _cachedSecrets[key];
            }

            var secret = _secretClient.GetSecret(key)?.Value?.Value; // GetSecret throws sometimes

            if (string.IsNullOrEmpty(secret))
            {
                throw new KeyNotFoundException($"Secret=[{key}]");
            }

            _cachedSecrets[key] = secret;
            return secret;
        }
    }

Environment:

  • Name and version of the Library package used: Azure.Security.KeyVault.Secrets 4.0.1 & Azure.Identity 1.1.0
  • Hosting platform or OS and .NET runtime version (dotnet --info output for .NET Core projects): e.g. Azure AppService .NET Core 3.1 - I do not see the problem on my other app in Azure which is using .NET Framework 4.7.2
@triage-new-issues triage-new-issues bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jan 31, 2020
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. KeyVault and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Jan 31, 2020
@jsquire
Copy link
Member

jsquire commented Jan 31, 2020

//cc: @annelo-msft, @schaabs

@AlexGhiondea
Copy link
Contributor

@heaths can you please take a look?

@heaths
Copy link
Member

heaths commented Feb 4, 2020

@macchmie3 just to confirm, you are seeing RequestFailedExceptions being thrown and not just the 401 being logged (this is expected until the client is authenticated and challenges cached)?

@macchmie3
Copy link
Author

@heaths
Yes, it's Exception that is being thrown. It was not some kind of internal log.

@heaths
Copy link
Member

heaths commented Feb 10, 2020

This appears to be a race condition in the challenge cache which we should synchronize. Will try to repro.

@heaths heaths added this to the [2020] March milestone Feb 10, 2020
@macchmie3
Copy link
Author

macchmie3 commented Feb 11, 2020

@heaths

wrapping var secret = _secretClient.GetSecret(key)?.Value?.Value; with a lock statement fixes the problem - as predicted, but it seems like a dirty workaround for me.

@heaths
Copy link
Member

heaths commented Feb 25, 2020

I'm able to repro this but with other work pending for our March release we may need to punt till after that since a workaround - agreeably not ideal - is available.

BTW, just a tip: there is an implicit cast operator for KeyVaultSecret so you can simplify that statement. I generally prefer var myself as well, but this is one case where an explicit type declaration can help:

KeyVaultSecret secret = _secretClient.GetSecret(key);

@macchmie3
Copy link
Author

@heaths
Great to hear that you were able to reproduce the problem.
Thanks for the tip!

@timmydo
Copy link

timmydo commented Mar 3, 2020

I feel like there should be a stronger warning on the main page about using this package in production if concurrent requests cause failures...

@heaths
Copy link
Member

heaths commented Mar 18, 2020

I believe I have a fix and was hoping you could try it to verify before we release it, given the nature of this problem is impacted by machine and scenario differences.

  1. Register our dev feed in your nuget configuration. Typically, you'd run the following in your solution or project root (can also be machine wide, but any CI/CD will also need it):
    dotnet nuget add source -n AzureSDK https://azuresdkartifacts.blob.core.windows.net/azure-sdk-for-net/index.json
    
  2. Install the dev package into your project:
    dotnet add package Azure.Security.KeyVault.Secrets --version 4.0.3-dev.20200318.2
    
  3. Rebuild your project. Make sure the right DLL was copied to your output. It should have the file version 4.0.320.16802 and product version 4.0.3-dev.20200318.2+3a643510066880ee8bffe38c55662e29a6ea6ea4.

Please let me know if this solves your problem and we'll get a release out on nuget.org. Thank you.

@macchmie3
Copy link
Author

I will check it today!

@macchmie3
Copy link
Author

@heaths
I just tested your package and seems like the issue is solved! Of course I double-checked that the workaround-locks are removed :)

@heaths
Copy link
Member

heaths commented Mar 19, 2020

Thanks for verifying the fix. We want to do some additional testing to make sure we didn't regress anything and fixed all the related issues here, and will get a servicing release out on nuget.org shortly.

@nabsul
Copy link

nabsul commented Mar 20, 2020

Great timing. I was just getting this same error, but only when using GetPropertiesOfSecretVersionsAsync() in my code. I just tested the development package and it fixed my problem as well.

@heaths heaths closed this as completed in 3a64351 Apr 7, 2020
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-net that referenced this issue Jun 22, 2020
Add swagger definition for user assigned identity (Azure#9737)
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-net that referenced this issue Jun 23, 2020
Add swagger definition for user assigned identity (Azure#9737)
@manojsachdev1992
Copy link

@heaths
Hi Heath..

Need your help on this, I have updated the nuget package Azure.Security.KeyVault.Secrets -Version 4.0.3, also tried 4.1.0.
But still getting 401 Unauthorized excpetions.

Could you please help on this.

Regards,
Manojkumar Sachdev

@heaths
Copy link
Member

heaths commented Sep 10, 2020

Please open a new issue and provide diagnostic information as described here: https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/keyvault/Azure.Security.KeyVault.Secrets/README.md#troubleshooting

You can also capture logs using ETW without changing code using dotnet-trace: https://heaths.dev/azure/2020/02/04/trace-azure-sdk-for-net.html

@manojsachdev1992
Copy link

Please open a new issue and provide diagnostic information as described here: https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/keyvault/Azure.Security.KeyVault.Secrets/README.md#troubleshooting

You can also capture logs using ETW without changing code using dotnet-trace: https://heaths.dev/azure/2020/02/04/trace-azure-sdk-for-net.html

Hi @heaths
Have raised issue with MS on this and waiting for their update
Ticket number: 120091023001449

@heaths
Copy link
Member

heaths commented Oct 2, 2020

Just wanted to note that we will also consider a challenge-free auth flow as well: #15651

@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants