[Bug] CryptographicException when using CNG certificate

[bgavrilMS]

Which Version of MSAL are you using ?
4.10

Platform
net45 (does not repro on netcore)

Repro

ClientCreds with a certificate generated from ADFS 2019 or any CNG certificate
Simplest is to run the integration test ClientCreds_WithCertificate_Adfs_Async which will download such a certificate...

Expected: MSAL should fetch a token
Actual: Cryptographic Exception

System.Security.Cryptography.CryptographicException: Invalid provider type specified.
X509Certificate2.get_PrivateKey()
NetDesktopCryptographyManager.GetCryptoProviderForSha256(X509Certificate2 certificate) line 92
... rest of stack trace is not important ...

Note:

1. Wilson is able to handle this certificate!
2. There seems to be a known issue with CNG type certs but not sure how to get around it. Wilson seems to be able to get around it just fine... The exception occurs when accessing the PrivateKey property.

How to fix this

Wilson knows how to handle these certs. We need to use the same logic in both Wilson and MSAL, but we cannot take a depdency on Wilson.

As part of fixing this issue, we need to asses the possibility of using a github submodule for the shared logic.

Workaround

1. Create a signed assertion manually like here - using Microsoft.IdentityModel.JsonWebTokens nuget package. Just make sure the audience value is set to the token endpoint, which is:

"{authority}oauth2/v2.0/token" for AAD and B2C
"{authority}oauth2/token" for ADFS

So for example if your authority is https://login.microsoftonline.com/12345-56789 the the token endpoint is https://login.microsoftonline.com/12345-56789/oauth2/v2.0/token - this is the aud value.

2. Instead of using .WithCertificate use .WithClientAssertion and pass in the string from previous step (example)

This is what MSAL internally should be extracting from that certificate, but it is unfortunately failing because on .net classic we need to support APIs all the way down to .net 4.5 that are very buggy. We cannot take a dependency on Microsoft.IdentityModel.JsonWebTokens as it is a breaking change.

[bgavrilMS]

Quite a few bugs around certificate usage in MSAL now ... @henrik-me @trwalke Perhaps it's time to bite the bullet and upgrade to use Wilson?

[jmprieur]

yes, that would be a good optimization if this works for all platforms? (or would we have two code paths)?

[bgavrilMS]

Well we would use Wilson for 2 things: Client Creds and PoP, which are really only present on .net classic and .net core;

Wilson has bindings for .net classic 4.5 and .net standard 2.0

So in the short term, we could move to use Wilson without changing our supported frameworks. In the longer term we might have to take a breaking change if we will want to support PoP on UWP for example, or if we want things to work better on pure .netstandard (for things like Unity).

[jmprieur]

Let's do that, then, and if we see the value, let's move to 5.0 (given that it's only about platforms)

[henning-krause]

Just stumbled accross because I got this exact error when using certificates created with PowerShell New-SelfSignedCertificate cmdlet, which uses CNG to create certificates.

[bgavrilMS]

@henning-krause - I added a workaround, please let me know if that works.

[henning-krause]

@bgavrilMS I found another workaround. I you specify -KeySpec KeyExchange as a parameter for New-SelfSignedCertificate, it will revert to the CAPI provider and create legacy certificates. That works.

[bgavrilMS]

@trwalke - I think we need to do what Wilson. Its logic is capured in 2 files:

https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/06cae9a6d422dac7b76f1db9de032b98c1d8c836/src/Microsoft.IdentityModel.Tokens/AsymmetricAdapter.cs#L48

https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/06cae9a6d422dac7b76f1db9de032b98c1d8c836/src/Microsoft.IdentityModel.Tokens/RsaCryptoServiceProviderProxy.cs#L43

These files have seen very little change in the past 1 year, so I am thinking that copying the logic should be sufficient.

[bgavrilMS]

Also see #1937 (comment) for an example of how to create this type of cert.

[bgavrilMS]

Upon further investigation, it seems that Wilson is also not able to handle these certs on .net 45. So the way to move forward here is to add also target .net 461 and to make a simple code change. CC @GeoK

Note that .net45 is really old, but it is tied to Win Server 2008 R2, so it will only be deprecated around 2022 - thus we probably cannot remove .net 45 without a major breaking change....

CC @henrik-me @jmprieur for the go ahead.