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

"Key not valid for use in specified state" - happens in .Net Framework 4.7.2 only, not on .Net Core 2.2 #1201

Closed
1 of 7 tasks
William-Au-Yeung opened this issue Jun 4, 2019 · 14 comments
Assignees
Milestone

Comments

@William-Au-Yeung
Copy link

William-Au-Yeung commented Jun 4, 2019

Which Version of MSAL are you using ?
Microsoft.Identity.Client Version=3.0.8

Platform

my library in .Net Core
and a web job console in .Net Framework 4.7.2

What authentication flow has the issue?

  • Desktop / Mobile
    • Interactive
    • Integrated Windows Auth
    • Username Password
    • Device code flow (browserless)
  • Web App
    • Authorization code
    • OBO
  • Web API
    • OBO

Other? - please describe;

Is this a new or existing app?

C. This is a new app or experiment

Repro

            SqlConnectionStringBuilder connectionStringBuilder = new SqlConnectionStringBuilder();
            connectionStringBuilder.DataSource = this.Settings.DataSource;
            connectionStringBuilder.InitialCatalog = this.Settings.InitialCatalog;
            connectionStringBuilder.Encrypt = this.Settings.Encrypt;
            connectionStringBuilder.TrustServerCertificate = this.Settings.TrustServerCertificate;

            SqlConnection connection = new SqlConnection(connectionStringBuilder.ConnectionString);
            connection.AccessToken = this.Settings.TokenAuthenticator.GetAccessTokenAsync(DefaultScope).Result;

        public async Task<string> GetAccessTokenAsync(IEnumerable<string> scopes)
        {
            X509Certificate2 certificate = this.GetCertificate();
            ConfidentialClientApplicationOptions option = new ConfidentialClientApplicationOptions
            {
                AzureCloudInstance = this.CloudInstance,
                ClientId = this.ApplicationId,
                TenantId = this.TenantId,
            };
            IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
                .CreateWithApplicationOptions(option)
                .WithCertificate(certificate)
                .Build();

            AuthenticationResult result = await app.AcquireTokenForClient(scopes)
                .ExecuteAsync();

            return result.AccessToken;
        }

Expected behavior
No exception. I should be able to authenticate a connection to my Azure SQL Database. I have run the same code on .Net Core console project, and it has been working.

Actual behavior
I got this exception in my .Net Framework 4.7.2 console project:
Microsoft.Azure.WebJobs.Host.FunctionInvocationException: Exception while executing function: Functions.PollBuilds ---> System.AggregateException: One or more errors occurred. ---> System.Security.Cryptography.CryptographicException: Key not valid for use in specified state.

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.Identity.Client.Platforms.net45.NetDesktopCryptographyManager.GetCryptoProviderForSha256(X509Certificate2 certificate)
at Microsoft.Identity.Client.Platforms.net45.NetDesktopCryptographyManager.SignWithCertificate(String message, X509Certificate2 certificate)
at Microsoft.Identity.Client.Internal.JsonWebToken.Sign(ClientAssertionCertificateWrapper credential, Boolean sendCertificate)
at Microsoft.Identity.Client.Internal.Requests.ClientCredentialHelper.CreateClientCredentialBodyParameters(ICoreLogger logger, ICryptographyManager cryptographyManager, ClientCredentialWrapper clientCredential, String clientId, AuthorityEndpoints endpoints, Boolean sendX5C)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.d__22.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.Internal.Requests.ClientCredentialRequest.d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.d__14.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.ApiConfig.Executors.ConfidentialClientExecutor.d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Scm.Authentication.Settings.ClientCertificateSettings.d__12.MoveNext()
--- End of inner exception stack trace ---
at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
at System.Threading.Tasks.Task1.GetResultCore(Boolean waitCompletionNotification) at System.Threading.Tasks.Task1.get_Result()
at Scm.Authentication.AzureDatabaseBase.CreateConnection()

Possible Solution

Additional context/ Logs / Screenshots
Add any other context about the problem here, such as logs and screebshots. Logging is described at https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/logging

@jmprieur jmprieur self-assigned this Jun 6, 2019
@henrik-me henrik-me added the bug label Jun 21, 2019
@inghak
Copy link

inghak commented Jul 4, 2019

Same issue, .NET 4.7.2 and Microsoft.Identity.Client 4.1.0. Same code works in .NET 4.6.2 and Microsoft.Identity.Client 4.1.0.

@ilya-spv
Copy link

ilya-spv commented Jul 4, 2019

As far as I could figure what happens is that on 4.7.2. the library tries to export certificate with private key and if it is marked as non exportable it fails. If the key is exportable, then it works fine.

It's strange though that depending on framework version the library reads the key in a different manner.

@jmprieur
Copy link
Contributor

jmprieur commented Jul 5, 2019

@henrik-me, @MarkZuber, @bgavrilMS
We might want to provide a better error message when the certificate is not exportable?
Or do we need to use the X509KeyStorageFlags.EphemeralKeySet flag ? to avoid the exception?

@henning-krause
Copy link

Hitting this very problem today. It would be great if the authentication would work without needing the private key to be exportable.

@William-Au-Yeung
Copy link
Author

@henrik-me, @MarkZuber, @bgavrilMS
We might want to provide a better error message when the certificate is not exportable?
Or do we need to use the X509KeyStorageFlags.EphemeralKeySet flag ? to avoid the exception?

If the behavior is not planned to be fixed, then a better error message is a must. This is a really confusing experience.

@jmprieur
Copy link
Contributor

jmprieur commented Jul 8, 2019

@henning-krause @ilya-spv @inghak @William-Au-Yeung
When you read the certificate, do you use the X509KeyStorageFlags.EphemeralKeySet flag` ?

@William-Au-Yeung
Copy link
Author

@henning-krause @ilya-spv @inghak @William-Au-Yeung
When you read the certificate, do you use the X509KeyStorageFlags.EphemeralKeySet flag` ?

Nope. My code is reading the certificate out from the certificate store only.

X509Store store = new X509Store(settings.CertificateStoreName, settings.CertificateStoreLocation);
try
{
    store.Open(OpenFlags.ReadOnly);
    X509Certificate2Collection certificate = store.Certificates.Find(
        findType: X509FindType.FindByThumbprint,
        findValue: settings.CertificateThumbprint,
        validOnly: false); // Don't validate certs, since the test root isn't installed.

    if (certificate == null || certificate.Count == 0)
    {
        throw new FatalErrorException(Invariant($"Cannot load the requested certificate (Thumbprint: {settings.CertificateThumbprint}, Store: {settings.CertificateStoreName}, Location: {settings.CertificateStoreLocation})"));
    }

    X509Certificate2 serviceCertificate = certificate[0];
    try
    {
        var privateKey = serviceCertificate.PrivateKey;
        Debug.Assert(privateKey != null, "Make sure private key is available");
        return serviceCertificate;
    }
    catch (CryptographicException)
    {
        throw new FatalErrorException(Invariant($"Cannot read the private key of the requested certificate (Thumbprint: {serviceCertificate.Thumbprint})"));
    }
}
finally
{
    store.Close();
}

@henning-krause
Copy link

@jmprieur if I import my certificate from a file, it is pretty much irrelevant whether I specify Ephemeral or Exportable - I can do any of those things.

But if I want to use a certificate from a store (like @William-Au-Yeung), ephemeral is not an option (since the key is already persisted on the system) and I might not be able to control the "exportable" setting, because I'm forced to use a specific certificate which was not marked as exportable when the key was imported.

@jmprieur
Copy link
Contributor

I agree @henning-krause
We need to improve the exception to have a meaningfull and actionable message that the certificate should be exportable.

@jmprieur jmprieur removed their assignment Jul 10, 2019
@henning-krause
Copy link

@jmprieur No, the requirement that the certificate must be exportable should be removed. I don't see why this is required. The code should be able to sign the JWT without exporting the private parameters.

@henning-krause
Copy link

henning-krause commented Jul 10, 2019

I just took a quick look at

RSA rsa = x509Key.GetAsymmetricAlgorithm(SecurityAlgorithms.RsaSha256Signature, true) as RSA;
RSACryptoServiceProvider newRsa = null;
try
{
if (rsa is RSACryptoServiceProvider cspRsa)
{
// For .NET 4.6 and below we get the old RSACryptoServiceProvider implementation as the default.
// Try and get an instance of RSACryptoServiceProvider which supports SHA256
newRsa = GetCryptoProviderForSha256(cspRsa);
}
else
{
// For .NET Framework 4.7 and onwards the RSACng implementation is the default.
// Since we're targeting .NET Framework 4.5, we cannot actually use this type as it was
// only introduced with .NET Framework 4.6.
// Instead we try and create an RSACryptoServiceProvider based on the private key from the
// certificate.
newRsa = GetCryptoProviderForSha256(certificate);
}
using (var sha = new SHA256Cng())
{
return newRsa.SignData(messageBytes, sha);
}
}
finally
{
// We only want to dispose of the 'newRsa' instance if it is a *different instance*
// from the original one that was used to create it.
if (newRsa != null && !ReferenceEquals(rsa, newRsa))
{
newRsa.Dispose();
}
}
}

Why can't you just take the certificate.PrivateKey, cast it to RSA and call SignData on it?

var rsa = (RSA) certificate.PrivateKey;
rsa.SignData(data, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);

Still, that code wouldn't work with certificates created with the Windows CNG provider, because X509Certificate2.PrivateKey can't handle those. You would need to target a later .NET version and use the X509Certificate2.GetPrivateKey() extension method for that.

@ilya-spv
Copy link

I completely agree that there should not be any rule that the certificate should be exportable. The code in the old ADAL library works just fine without certificate being exportable. Likewise this library does work on .net 4.6.2. So it would be strange if that just won't be allowed

@zihzhan-msft
Copy link

zihzhan-msft commented Feb 23, 2021

It's 2021, but I am hitting the same issue after updating to net472 from net462. Seems the fix above doesn't work.
@jmprieur

@bgavrilMS
Copy link
Member

@zihzhan - please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants