Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.47k stars 4.81k forks source link

KeyVault fails on .Net Framework 4.7 #3521

Closed simonporter closed 4 years ago

simonporter commented 7 years ago

Code that works on .Net Framework 4.61 fails on .Net Framework 4.7

To Repro:

packages: Microsoft.Extensions.Configuration.1.1.2 Microsoft.Extensions.Configuration.AzureKeyVault.1.0.2

namespace ConsoleApp1
{
    using System;
    using System.Security.Cryptography.X509Certificates;
    using Microsoft.Extensions.Configuration;

    class Program
    {
        static void Main(string[] args)
        {
            X509Certificate2 clientCertificate;
            X509Store store = new X509Store(StoreName.My, StoreLocation.CurrentUser);
            try
            {
                store.Open(OpenFlags.ReadOnly);

                var certificates = store.Certificates.Find(X509FindType.FindByThumbprint, "SOME_THUMBPRINT", false);
                clientCertificate = certificates[0];
            }
            finally
            {
                store.Close();
            }

            var builder = new ConfigurationBuilder()
                .AddAzureKeyVault("https://YOUR_VAULT.vault.azure.net/", "CLIENT_ID", clientCertificate);

            builder.Build(); // Exception happens here
            Console.WriteLine("Done!");
            Console.ReadLine();
        }
    }
}

Simplified exception stacktrace

Exception: Object reference not set to an instance of an object.
   at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.GetCryptoProviderForSha256(RSACryptoServiceProvider rsaProvider)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.SignWithCertificate(String message, X509Certificate2 certificate)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.ClientAssertionCertificate.Sign(String message)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.JsonWebToken.Sign(IClientAssertionCertificate credential)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.ClientKey.AddToParameters(IDictionary`2 parameters)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.<SendTokenRequestAsync>d__64.MoveNext()
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.<RunAsync>d__55.MoveNext()
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext.<AcquireTokenForClientCommonAsync>d__49.MoveNext()
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext.<AcquireTokenAsync>d__27.MoveNext()
   at Microsoft.Extensions.Configuration.AzureKeyVaultConfigurationExtensions.<GetTokenFromClientCertificate>d__5.MoveNext()
   at Microsoft.Azure.KeyVault.KeyVaultCredential.<PostAuthenticate>d__9.MoveNext()
   at Microsoft.Azure.KeyVault.KeyVaultCredential.<ProcessHttpRequestAsync>d__10.MoveNext()
   at Microsoft.Azure.KeyVault.KeyVaultClient.<GetSecretsWithHttpMessagesAsync>d__66.MoveNext()
   at Microsoft.Azure.KeyVault.KeyVaultClientExtensions.<GetSecretsAsync>d__49.MoveNext()
   at Microsoft.Extensions.Configuration.AzureKeyVault.AzureKeyVaultConfigurationProvider.<LoadAsync>d__5.MoveNext()
   at Microsoft.Extensions.Configuration.AzureKeyVault.AzureKeyVaultConfigurationProvider.Load()
   at Microsoft.Extensions.Configuration.ConfigurationRoot..ctor(IList`1 providers)
   at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()
herveyw-msft commented 7 years ago

@hovsepm @simonporter The stack trace here suggests the problem is down in the ADAL library and not in the Azure Key Vault SDK.

herveyw-msft commented 7 years ago

@hovsepm @simonporter The ADAL team says this bug is fixed.

shahabhijeet commented 7 years ago

@simonporter have you tried recently and does that fix your issue?

simonporter commented 7 years ago

It appears to get past this step but now gets an exception that is cannot use the key, same code still works for net461 but not net47.

Exception: "Key not valid for use in specified state." System.Security.Cryptography.CryptographicException at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.GetCryptoProviderForSha256(X509Certificate2 certificate) at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.SignWithCertificate(String message, X509Certificate2 certificate) at Microsoft.IdentityModel.Clients.ActiveDirectory.ClientAssertionCertificate.Sign(String message) at Microsoft.IdentityModel.Clients.ActiveDirectory.JsonWebToken.Sign(IClientAssertionCertificate credential) at Microsoft.IdentityModel.Clients.ActiveDirectory.ClientKey.AddToParameters(IDictionary2 parameters) at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.<SendTokenRequestAsync>d__64.MoveNext() at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.<RunAsync>d__55.MoveNext() at Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext.<AcquireTokenForClientCommonAsync>d__51.MoveNext() at Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext.<AcquireTokenAsync>d__27.MoveNext() at Microsoft.Extensions.Configuration.AzureKeyVaultConfigurationExtensions.<GetTokenFromClientCertificate>d__5.MoveNext() at Microsoft.Azure.KeyVault.KeyVaultCredential.<PostAuthenticate>d__9.MoveNext() at Microsoft.Azure.KeyVault.KeyVaultCredential.<ProcessHttpRequestAsync>d__10.MoveNext() at Microsoft.Azure.KeyVault.KeyVaultClient.<GetSecretsWithHttpMessagesAsync>d__66.MoveNext() at Microsoft.Azure.KeyVault.KeyVaultClientExtensions.<GetSecretsAsync>d__49.MoveNext() at Microsoft.Extensions.Configuration.AzureKeyVault.AzureKeyVaultConfigurationProvider.<LoadAsync>d__5.MoveNext() at Microsoft.Extensions.Configuration.AzureKeyVault.AzureKeyVaultConfigurationProvider.Load() at Microsoft.Extensions.Configuration.ConfigurationRoot..ctor(IList1 providers) at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()

simonporter commented 7 years ago

Note this is with: Microsoft.Extensions.Configuration 2.0 Microsoft.Extensions.Configuration.AzureKeyVault 2.0 Microsoft.IdentityModel.Clients.ActiveDirectory 3.16 (which lists the original issue as a fixed)

herveyw-msft commented 7 years ago

@kpanwar Kanishk: it doesn't appear that this issue is fixed in ADAL 3.16? Either that or there is some lower level issue with the certificate and its key?

kpanwar commented 7 years ago

@herveyw - we have started seeing this issue in 4.7 and I don't fully understand the root cause. I plan on engaging the .net team when I am back from vacation.

vnvaditya commented 7 years ago

I get the same error after upgrading to .NET 4.7. Microsoft.IdentityModel.Clients.ActiveDirectory 3.16 Microsoft.Azure.KeyVault 2.0.6

using System;
using System.Security.Cryptography.X509Certificates;
using System.Threading.Tasks;
using Microsoft.Azure.KeyVault;
using Microsoft.IdentityModel.Clients.ActiveDirectory;

namespace ADALIssue
{

class Program
    {

        static void Main(string[] args)
        {
            test2();
            Console.ReadLine();
        }

        private static void test2()
        {
            var vaultAddress = "https://edogkeyvault.vault.azure.net/";
            KeyVaultClient keyVaultClient = new KeyVaultClient(GetAccessTokenByCert);
            var secret = keyVaultClient.GetSecretAsync(vaultAddress,"PipeTableStorage").GetAwaiter().GetResult();

            var sKey = secret.Value;
            Console.WriteLine(sKey);
        }

        private static async Task<string> GetAccessTokenByCert(string authority, string resource, string scope)
        {
            var authenticationContext = new AuthenticationContext(authority, null);
            X509Certificate2 clientCertificate;
            X509Store store = new X509Store(StoreName.My, StoreLocation.LocalMachine);
            try
            {
                store.Open(OpenFlags.ReadOnly);

                var certificates = store.Certificates.Find(X509FindType.FindByThumbprint, "123abc", false);
                clientCertificate = certificates[0];
            }
            finally
            {
                store.Close();
            }

            if (clientCertificate == null)
            {
                throw new Exception("Certificate null exception");
            }

            var clientAssertionCertificate = new ClientAssertionCertificate("abcdefghijk", clientCertificate);
            var result = await authenticationContext.AcquireTokenAsync(resource, clientAssertionCertificate).ConfigureAwait(false);
            return result.AccessToken;
        }
    }
}

Data: 0 key/value pairs Inner Exception:Key not valid for use in specified state.

Exception type: CryptographicException Additional Data: Source: mscorlib Stack Tracer.Trace: _at System.Security.Cryptography.CryptographicException.ThrowCryptographicException(Int32 hr) at System.Security.Cryptography.Utils._ExportKey(SafeKeyHandle hKey, Int32 blobType, Object cspObject) at System.Security.Cryptography.RSACryptoServiceProvider.ExportParameters(Boolean includePrivateParameters) at System.Security.Cryptography.RSA.ToXmlString(Boolean includePrivateParameters) at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.GetCryptoProviderForSha256(X509Certificate2 certificate) at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.SignWithCertificate(String message, X509Certificate2 certificate) at Microsoft.IdentityModel.Clients.ActiveDirectory.JsonWebToken.Sign(IClientAssertionCertificate credential) at Microsoft.IdentityModel.Clients.ActiveDirectory.ClientKey.AddToParameters(IDictionary`2 parameters) at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.d_64.MoveNext()

kpanwar commented 7 years ago

We have a nightly DEV package out with the fix. Please try it out and let us know if it worked for you too. We will be cutting an official release following the feedback. Install-Package Microsoft.IdentityModel.Clients.ActiveDirectory -Version 3.16.1-alpha24505 -Source https://www.myget.org/F/aad-clients-nightly/api/v3/index.json

vnvaditya commented 7 years ago

Thank you. We have tested it and the issue isresolved.

andriysavin commented 6 years ago

Team, are you going to update the package to reference the version of Microsoft.IdentityModel.Clients.ActiveDirectory package where the bug was fixed? In some scenarios when you don't control the host application (e.g. Azure Functions compiled library) adding different version of the Microsoft.IdentityModel.Clients.ActiveDirectory is not possible because no binding redirects are generated, so the runtime tries to find the exact package version which was referenced by the KeyVault package.

stijnherreman commented 6 years ago

Is there anything blocking progress on this issue? If I understand correctly, Microsoft.Azure.Services.AppAuthentication must reference a newer version of Microsoft.IdentityModel.Clients.ActiveDirectory and Microsoft.Extensions.Configuration.AzureKeyVault must reference the new version of Microsoft.Azure.Services.AppAuthentication?

LukeDearden commented 6 years ago

I doubt anything will happen with this. In light of the new AZ powershell module https://azure.microsoft.com/en-gb/blog/azure-powershell-cross-platform-az-module-replacing-azurerm/

glennsills commented 5 years ago

This still seems to be broken. Is it still being looked into?

glennsills commented 5 years ago

I am still getting the same stack trace that the original poster @simonporter got.

I am using newer packages though

The code is the same, same result. I am accessing KeyVault in another way to get KeyVault manages storage SAS tokens. That works fine. This seems specific to this extension for some reason.


{System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.GetCryptoProviderForSha256(RSACryptoServiceProvider rsaProvider)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.CryptographyHelper.SignWithCertificate(String message, X509Certificate2 certificate)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.JsonWebToken.Sign(IClientAssertionCertificate credential)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.ClientKey.AddToParameters(IDictionary`2 parameters)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.<SendTokenRequestAsync>d__64.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AcquireTokenHandlerBase.<RunAsync>d__55.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext.<AcquireTokenForClientCommonAsync>d__49.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext.<AcquireTokenAsync>d__27.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Extensions.Configuration.AzureKeyVaultConfigurationExtensions.<GetTokenFromClientCertificate>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Azure.KeyVault.KeyVaultCredential.<PostAuthenticate>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Azure.KeyVault.KeyVaultCredential.<ProcessHttpRequestAsync>d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Azure.KeyVault.KeyVaultClient.<GetSecretsWithHttpMessagesAsync>d__66.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Azure.KeyVault.KeyVaultClientExtensions.<GetSecretsAsync>d__49.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Extensions.Configuration.AzureKeyVault.AzureKeyVaultConfigurationProvider.<LoadAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Extensions.Configuration.AzureKeyVault.AzureKeyVaultConfigurationProvider.Load()
   at Microsoft.Extensions.Configuration.ConfigurationRoot..ctor(IList`1 providers)
   at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()
   at PwC.Notification.Batch.Program.<Main>d__3.MoveNext() in C:\code\ob_notifications\PwC.Notification.Batch\PwC.Notification.Batch\Program.cs:line 29}````
TPucke commented 5 years ago

Possible insight: My project targets framework 4.7.1 and uses v2.2.0 of Microsoft.Extensions.Configuration.AzureKeyVault, and is having what looks exactly like this issue (same stack trace).

I see that one of the dependencies of that package is Microsoft.Azure.Services.AppAuthentication 1.0.1, which in turn depends on Microsoft.IdentityModel.Clients.ActiveDirectory 3.14.2.

I searched and found what reads like the correct discussion and fix here. From the package history it appears to me that the fix is released in version 3.15.0, which is later than the current nested dependency.

You might be able to fix this problem by getting the dependencies updated, I guess in Microsoft.Azure.Services.AppAuthentication.

TPucke commented 5 years ago

OK so there is a fairly simple workaround for this; not completely elegant, but pragmatic.

I added to my project an explicit nuget package reference to a later minor version of Microsoft.IdentityModel.Clients.ActiveDirectory and this issue went away. So instead of the default nested reference of 3.14.2, the project specifies 3.19.8 and all is good. You can see by browsing the nested dependencies that this later version is substituted there also.

If anyone thinks there is risk with this please do register your thoughts.

Petermarcu commented 4 years ago

New KeyVault libraries shipped last Nov that should not have this issue anymore. The are based on the new MSAL auth library. If you are still hitting this issue, consider updating to the latest libraries. You can see the full list of new libraries here: https://azure.github.io/azure-sdk/releases/latest/index.html