Azure-sdk-for-net: KeyVault - SecretClient.GetSecret(string key) return Status: 401 (Unauthorized)

[macchmie3]

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

[jsquire]

//cc: @annelo-msft, @schaabs

[AlexGhiondea]

@heaths can you please take a look?

[heaths]

@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]

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

[heaths]

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

[macchmie3]

@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]

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]

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

[timmydo]

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]

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]

I will check it today!

[macchmie3]

@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]

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]

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.

[manojsachdev1992]

@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]

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]

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]

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