15

I have setup Azure Keyvault on my ASP.Net MVC web application by following the example in Microsoft's Hello Key Vault sample application.

Azure KeyVault (Active Directory) AuthenticationResult by default has a one hour expiry. So after one hour, you must get a new authentication token. KeyVault is working as expected for the first hour after getting my first AuthenticationResult token, but after the 1 hour expiry, it fails to get a new token.

Unfortunately it took a failure on my production environment for me to realize this, as I never tested past one hour in development.

Anyways, after over two days of trying to figure out what was wrong with my keyvault code, I came up with a solution that fixes all of my problems - remove the asynchronous code - but it feels very hacky. I want to find out why it was not working in the first place.

My code looks like this:

public AzureEncryptionProvider() //class constructor
{
   _keyVaultClient = new KeyVaultClient(GetAccessToken);
   _keyBundle = _keyVaultClient
     .GetKeyAsync(_keyVaultUrl, _keyVaultEncryptionKeyName)
     .GetAwaiter().GetResult();
}

private static readonly string _keyVaultAuthClientId = 
    ConfigurationManager.AppSettings["KeyVaultAuthClientId"];

private static readonly string _keyVaultAuthClientSecret =
    ConfigurationManager.AppSettings["KeyVaultAuthClientSecret"];

private static readonly string _keyVaultEncryptionKeyName =
    ConfigurationManager.AppSettings["KeyVaultEncryptionKeyName"];

private static readonly string _keyVaultUrl = 
    ConfigurationManager.AppSettings["KeyVaultUrl"];

private readonly KeyBundle _keyBundle;
private readonly KeyVaultClient _keyVaultClient;

private static async Task<string> GetAccessToken(
    string authority, string resource, string scope)
{
   var clientCredential = new ClientCredential(
       _keyVaultAuthClientId, 
       _keyVaultAuthClientSecret);
   var context = new AuthenticationContext(
       authority, 
       TokenCache.DefaultShared);
   var result = context.AcquireToken(resource, clientCredential);
   return result.AccessToken;
}

The GetAccessToken method signature has to be asynchronous to pass into the new KeyVaultClient constructor, so I left the signature as async, but I removed the await keyword.

With the await keyword in there (the way it should be, and is in the sample):

private static async Task<string> GetAccessToken(string authority, string resource, string scope)
{
   var clientCredential = new ClientCredential(_keyVaultAuthClientId, _keyVaultAuthClientSecret);
   var context = new AuthenticationContext(authority, null);
   var result = await context.AcquireTokenAsync(resource, clientCredential);
   return result.AccessToken;
}

The program works fine the first time I run it. And for an hour, AcquireTokenAsync returns the same original authentication token which is great. But once the token expires, AcquiteTokenAsync should get a new token with a new expiry date. And it doesn't - the application just hangs. No error returned, nothing at all.

So calling AcquireToken instead of AcquireTokenAsync solves the problem, but I have no idea why. You'll also notice that I'm passing 'null' instead of 'TokenCache.DefaultShared' into the AuthenticationContext constructor in my sample code with async. This is to force the toke to expire immediately instead of after one hour. Otherwise, you have to wait an hour to reproduce the behavior.

I was able to reproduce this again in a brand new MVC project, so I don't think it has anything to do with my specific project. Any insight would be appreciated. But for now, I'm just not using async.

Shaun Luttin
  • 133,272
  • 81
  • 405
  • 467
Aaron
  • 282
  • 1
  • 4
  • 11
  • 1
    To reproduce the behavior, you could also call `context.TokenCache.Clear()` instead of passing `null` into the constructor. – Shaun Luttin Sep 16 '15 at 02:29
  • 1
    Hey Shaun, I am using Azure SKD 2.7. And I can successfully reproduce my issue using context.TokenCache.Clear() instead of passing null into the constructor. You are also correct that the problem is that AcquireTokenAsync() hangs once the original token expires, and a workaround is to use the non asynchronous method. I am going to try your solution below to see if it solves my issue. – Aaron Sep 16 '15 at 22:48
  • 1
    Another thing to note here is that on the Azure Website, this only happened when Always On was true - meaning that if IIS was allowed to shut down then website, things would reset and work again after the website spins back up. It would then stop working after an hour of run time again. – Aaron Sep 16 '15 at 22:49
  • 1
    Yes, I don't care if the token is actually expired or not. I just want to forcefully get a new one each time, as the failure is happening during the request for a second token. – Aaron Sep 17 '15 at 16:57
  • 1
    In my production environment I am using dependency injection, however I was able to reproduce this in a brand new MVC project without DI. I am using it in a controller ActionResult method by simply creating an instance of the class, and calling an encrypting method. Thanks for your assistance in this - I really am interested in finding out why this is not working. I don't mind using the non-async method, I just don't understand why async is not working. – Aaron Sep 18 '15 at 01:05
  • Have you viewed the answer about deadlocks? – Shaun Luttin Sep 21 '15 at 22:05
  • Hey sorry - it's been a busy weekend. Back at it all now. I have not had a chance to review this, but will go over it today and get back to you. Thanks so much. – Aaron Sep 22 '15 at 16:50

2 Answers2

26

Problem: deadlock

Your EncryptionProvider() is calling GetAwaiter().GetResult(). This blocks the thread, and on subsequent token requests, causes a deadlock. The following code is the same as yours is but separates things to facilitate explanation.

public AzureEncryptionProvider() // runs in ThreadASP
{
    var client = new KeyVaultClient(GetAccessToken);

    var task = client.GetKeyAsync(KeyVaultUrl, KeyVaultEncryptionKeyName);

    var awaiter = task.GetAwaiter();

    // blocks ThreadASP until GetKeyAsync() completes
    var keyBundle = awaiter.GetResult();
}

In both token requests, the execution starts in the same way:

  • AzureEncryptionProvider() runs in what we'll call ThreadASP.
  • AzureEncryptionProvider() calls GetKeyAsync().

Then things differ. The first token request is multi-threaded:

  1. GetKeyAsync() returns a Task.
  2. We call GetResult() blocking ThreadASP until GetKeyAsync() completes.
  3. GetKeyAsync() calls GetAccessToken() on another thread.
  4. GetAccessToken() and GetKeyAsync() complete, freeing ThreadASP.
  5. Our web page returns to the user. Good.

GetAccessToken is running on its own thread.

The second token request uses a single thread:

  1. GetKeyAsync() calls GetAccessToken() on ThreadASP (not on a separate thread.)
  2. GetKeyAsync() returns a Task.
  3. We call GetResult() blocking ThreadASP until GetKeyAsync() completes.
  4. GetAccessToken() must wait until ThreadASP is free, ThreadASP must wait until GetKeyAsync() completes, GetKeyAsync() must wait until GetAccessToken() completes. Uh oh.
  5. Deadlock.

GetAccessToken is running on the same thread.

Why? Who knows?!?

There must be some flow control within GetKeyAsync() that relies on the state of our access token cache. The flow control decides whether to run GetAccessToken() on its own thread and at what point to return the Task.

Solution: async all the way down

To avoid a deadlock, it is a best practice "to use async all the way down." This is especially true when we are calling an async method, such as GetKeyAsync(), that is from an external library. It is important not force the method to by synchronous with Wait(), Result, or GetResult(). Instead, use async and await because await pauses the method instead of blocking the whole thread.

Async controller action

public class HomeController : Controller
{
    public async Task<ActionResult> Index()
    {
        var provider = new EncryptionProvider();
        await provider.GetKeyBundle();
        var x = provider.MyKeyBundle;
        return View();
    }
}

Async public method

Since a constructor cannot be async (because async methods must return a Task), we can put the async stuff into a separate public method.

public class EncryptionProvider
{
    //
    // authentication properties omitted

    public KeyBundle MyKeyBundle;

    public EncryptionProvider() { }

    public async Task GetKeyBundle()
    {
        var keyVaultClient = new KeyVaultClient(GetAccessToken);
        var keyBundleTask = await keyVaultClient
            .GetKeyAsync(KeyVaultUrl, KeyVaultEncryptionKeyName);
        MyKeyBundle = keyBundleTask;
    }

    private async Task<string> GetAccessToken(
        string authority, string resource, string scope)
    {
        TokenCache.DefaultShared.Clear(); // reproduce issue 
        var authContext = new AuthenticationContext(authority, TokenCache.DefaultShared);
        var clientCredential = new ClientCredential(ClientIdWeb, ClientSecretWeb);
        var result = await authContext.AcquireTokenAsync(resource, clientCredential);
        var token = result.AccessToken;
        return token;
    }
}

Mystery solved. :) Here is a final reference that helped my understanding.

Console App

My original answer had this console app. It worked as an initial troubleshooting step. It did not reproduce the problem.

The console app loops every five minutes, repeatedly asking for a new access token. At each loop, it outputs the current time, the expiry time, and the name of the retrieved key.

On my machine, the console app ran for 1.5 hours and successfully retrieved a key after expiration of the original.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.Azure.KeyVault;
using Microsoft.IdentityModel.Clients.ActiveDirectory;

namespace ConsoleApp
{
    class Program
    {
        private static async Task RunSample()
        {
            var keyVaultClient = new KeyVaultClient(GetAccessToken);

            // create a key :)
            var keyCreate = await keyVaultClient.CreateKeyAsync(
                vault: _keyVaultUrl,
                keyName: _keyVaultEncryptionKeyName,
                keyType: _keyType,
                keyAttributes: new KeyAttributes()
                {
                    Enabled = true,
                    Expires = UnixEpoch.FromUnixTime(int.MaxValue),
                    NotBefore = UnixEpoch.FromUnixTime(0),
                },
                tags: new Dictionary<string, string> {
                    { "purpose", "StackOverflow Demo" }
                });

            Console.WriteLine(string.Format(
                "Created {0} ",
                keyCreate.KeyIdentifier.Name));

            // retrieve the key
            var keyRetrieve = await keyVaultClient.GetKeyAsync(
                _keyVaultUrl,
                _keyVaultEncryptionKeyName);

            Console.WriteLine(string.Format(
                "Retrieved {0} ",
                keyRetrieve.KeyIdentifier.Name));
        }

        private static async Task<string> GetAccessToken(
            string authority, string resource, string scope)
        {
            var clientCredential = new ClientCredential(
                _keyVaultAuthClientId,
                _keyVaultAuthClientSecret);

            var context = new AuthenticationContext(
                authority,
                TokenCache.DefaultShared);

            var result = await context.AcquireTokenAsync(resource, clientCredential);

            _expiresOn = result.ExpiresOn.DateTime;

            Console.WriteLine(DateTime.UtcNow.ToShortTimeString());
            Console.WriteLine(_expiresOn.ToShortTimeString());

            return result.AccessToken;
        }

        private static DateTime _expiresOn;
        private static string
            _keyVaultAuthClientId = "xxxxx-xxx-xxxxx-xxx-xxxxx",
            _keyVaultAuthClientSecret = "xxxxx-xxx-xxxxx-xxx-xxxxx",
            _keyVaultEncryptionKeyName = "MYENCRYPTIONKEY",
            _keyVaultUrl = "https://xxxxx.vault.azure.net/",
            _keyType = "RSA";

        static void Main(string[] args)
        {
            var keepGoing = true;
            while (keepGoing)
            {
                RunSample().GetAwaiter().GetResult();
                // sleep for five minutes
                System.Threading.Thread.Sleep(new TimeSpan(0, 5, 0)); 
                if (DateTime.UtcNow > _expiresOn)
                {
                    Console.WriteLine("---Expired---");
                    Console.ReadLine();
                }
            }
        }
    }
}
Community
  • 1
  • 1
Shaun Luttin
  • 133,272
  • 81
  • 405
  • 467
  • 1
    I am able to copy this into a new console project and run it, and it runs perfectly fine. However, running this same code from within an MVC application does not. So I believe the problem has something to do with the fact that this is within an MVC application. Another interesting thing to note is that within an MVC application, I am able to loop over the AcquireTokenAsync line over and over again in a single request (tested with a for loop) and it works fine. But once a second web request hits, acquiring the token fails. – Aaron Sep 16 '15 at 23:32
  • 1
    Sorry - it is failing in the same way that my original question was failing. It hangs on the second attempt to get a new access token asynchronously. Literally nothing happens at all, the program just sits there waiting and waiting. Nothing is ever returned, nothing crashes. It also seems to have to do with web requests, as I can generate many new tokens from within a single web request. But a second request to the server causes it to hang. – Aaron Sep 17 '15 at 16:54
  • Okay, you are correct - using async all the way down does fix the problem. However, I still don't understand why GetAwaiter().GetResult() doesn't work. I don't really want to make my Encrypt/Decrypt methods asynchronous. I don't mind just using AcquireToken intead of AcquireTokenAsync. I just did not understand why it was deadlocking the application. – Aaron Sep 22 '15 at 17:57
  • 1
    But I see here [https://msdn.microsoft.com/en-us/magazine/jj991977.aspx] that mixing blocking and async code is a big NO - so I just won't do it – Aaron Sep 22 '15 at 18:25
  • 2
    @Aaron `GetAwaiter().GetResult()` does not work because it blocks the thread until a task *on that thread* completes. This leads to a situation that has no resolution. – Shaun Luttin Sep 23 '15 at 00:01
  • 1
    @ShaunLuttin I've had exactly the same problem now and when searching for a solution I stumbled over your helpful answer. In my case I solved the problem by calling 'Task.Run(() => keyVaultClient.GetKeyAsync(_keyVaultUrl, _keyVaultEncryptionKeyName)).GetAwaiter().GetResult()'. But I'm wondering now, why it is not sufficient to add '.ConfigureAwait(false)' to this line: 'var result = await context.AcquireTokenAsync(resource, clientCredential)' Maybe you could explain the reason to me? – Peter Feb 15 '17 at 14:46
-1

I have the same challenge you have. I am assuming that you've also seen the sample published at https://azure.microsoft.com/en-us/documentation/articles/key-vault-use-from-web-application/

There is a big difference between what that sample does and what my code does (and I think the intent of your code is). In the sample, they retrieve a secrete and store it in the web application as a static member of their Utils class. Thus, the sample retrieves a secret one time for the entire run time of the application.

In my case, I am retrieving a different key for different purposes at different times during the application's run time.

Additionally, the sample download you linked to uses an X.509 certificate to authenticate the web application to KeyVault, rather than client secret. It's possible there is an issue with that too.

I saw the chat with @shaun-luttin concluding you caused a deadlock, but that's not the whole story I think. I don't use .GetAwaiter().GetResult() or call an async method from a ctor.

SvenAelterman
  • 1,532
  • 1
  • 15
  • 26
  • I've put the console app back in to the answer. Let me now if that helps you at all. – Shaun Luttin Sep 19 '15 at 04:35
  • 1
    I am afraid not... I am using MVC too. After 2 hours the app stops authenticating to KV. I did in fact make some changes and go "async all the way" without it making a difference. I'll post code too when able, but it's just like the OPs. – SvenAelterman Sep 19 '15 at 12:17
  • Does the console app work if you let it run continuously for two or three hours? – Shaun Luttin Sep 19 '15 at 16:14
  • I just ran it again on my machine, and it kept working for 2 hours. – Shaun Luttin Sep 19 '15 at 18:21
  • Yup. In a web context. I published it to Azure for you. http://mvp25015-azure-key-vault.azurewebsites.net Let me know if you have any further questions. – Shaun Luttin Sep 19 '15 at 18:41
  • All right, I will put your code in an MVC app and report back. Note the original poster's comment about multiple requests. I will call the code from a web test – SvenAelterman Sep 19 '15 at 18:42
  • Sure. If you want the entire code, let me know, and I will post it to GitHub. I want to make sure that we've solved this, and that some other niggling things is not presenting and obstacle. Thank you for persisting. :) – Shaun Luttin Sep 19 '15 at 18:51
  • 1
    Quick update: haven't been able to try this yet, but I will – SvenAelterman Sep 21 '15 at 19:50