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

RS256Algorithm error with .NET 4.0 #311

Closed
joshtate opened this issue Feb 22, 2021 · 22 comments
Closed

RS256Algorithm error with .NET 4.0 #311

joshtate opened this issue Feb 22, 2021 · 22 comments
Assignees
Labels

Comments

@joshtate
Copy link

joshtate commented Feb 22, 2021

Using version 7.3.0 and .NET 4.0.

The RS256Algorithm does not work with .NET 4.0. We have a working example using .NET 4.7.2 like this:

var store = new X509Store(StoreName.My, StoreLocation.LocalMachine);
store.Open(OpenFlags.ReadOnly);
X509Certificate2 cert = store.Certificates.Find(X509FindType.FindByThumbprint, "my_thumbprint", false)[0];
store.Close();

var rs256 = new RS256Algorithm(cert.GetRSAPublicKey(), cert.GetRSAPrivateKey());

string token = new JwtBuilder()
    .WithAlgorithm(rs256)
    .AddClaim("sub", "my-subject")
    .Encode();

But since .NET 4.0 doesn't have the extension methods GetRSAPublicKey() and GetRSAPrivateKey(), we have tried this:

var rs256 = new RS256Algorithm((RSA)cert.PublicKey.Key, (RSA)cert.PrivateKey);

And also this:

var rs256 = new RS256Algorithm(cert);

But both of those give the following error:

System.Security.Cryptography.CryptographicException: 'Invalid algorithm specified.'
   at System.Security.Cryptography.CryptographicException.ThrowCryptographicException(Int32 hr)
   at System.Security.Cryptography.Utils.SignValue(SafeKeyHandle hKey, Int32 keyNumber, Int32 calgKey, Int32 calgHash, Byte[] hash, Int32 cbHash, ObjectHandleOnStack retSignature)
   at System.Security.Cryptography.Utils.SignValue(SafeKeyHandle hKey, Int32 keyNumber, Int32 calgKey, Int32 calgHash, Byte[] hash)
   at System.Security.Cryptography.RSACryptoServiceProvider.SignHash(Byte[] rgbHash, Int32 calgHash)
   at System.Security.Cryptography.RSACryptoServiceProvider.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
   at System.Security.Cryptography.RSA.SignData(Byte[] data, Int32 offset, Int32 count, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
   at System.Security.Cryptography.RSA.SignData(Byte[] data, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
   at JWT.Algorithms.RS256Algorithm.Sign(Byte[] bytesToSign)
   at JWT.Algorithms.RS256Algorithm.Sign(Byte[] key, Byte[] bytesToSign)
   at JWT.JwtEncoder.Encode(IDictionary`2 extraHeaders, Object payload, Byte[] key)
   at JWT.Builder.JwtBuilder.Encode()

For the record, trying those other two ways of defining the RS256Algorithm also fail in .NET 4.7.2. The only way to get it to work is by using the extension methods GetRSAPublicKey() and GetRSAPrivateKey().

@abatishchev abatishchev self-assigned this Feb 22, 2021
@abatishchev
Copy link
Member

Hi, thanks for reporting the issue!
Can you please try versions 8.0.0 and 8.1.0 to confirm whether it still doesn't work or not?

The only way to get it to work

So you got it working? Sorry, not super clear from your message. If yes then how's the code that works looks like?

@abatishchev
Copy link
Member

abatishchev commented Feb 22, 2021

Here's a build run and a tests runs that supposedly target .NET 4.0.
However the build drop folder says .NET 4.6 so it might be a bug in the build and it actually targets 4.6, not the supposed 4.0.
I'm taking a deeper look.

@joshtate
Copy link
Author

I just tried with 8.0.0 and 8.1.0 and they don't work either.

Sorry, when I say "the only way to get it to work" I mean that it only works when you can use those extension methods, which aren't available in .NET 4.0. So basically what that boils down to is, I can get it to work in .NET 4.7.2 but not with .NET 4.0.

@abatishchev
Copy link
Member

The test project says this:

<TargetFramework>net46</TargetFramework>

But then this

<SetTargetFramework>TargetFramework=net40</SetTargetFramework>

I'm on vacation for a week so have only my phone.
Can you please download the build drop and check what framework it targets actually?

I mean if it's correct then the tests should be a good and working example how to construct the corresponding algorithm.

@joshtate
Copy link
Author

The test project is definitely referencing the incorrect framework (see image):

image

If I change that to ".NET Framework 4" then the test project doesn't even build because there are many incorrect references:

image

@abatishchev
Copy link
Member

Yes, but SetTargetFramework should (supposedly) do the trick. Can you please download the resulting DLL and open it with DotPeek or ILSpy or a similar tool, it should tell the actual target framework.

@joshtate
Copy link
Author

It's targeting 4.6:

image

@abatishchev
Copy link
Member

Oh, I see. Seems like a bug, indeed. I'll work on a fix upon return from the vacation, in a ~week. Sorry about this!

Maybe that's something @simon-pearson could take a look, please?

@abatishchev abatishchev added bug and removed question labels Feb 23, 2021
@abatishchev
Copy link
Member

abatishchev commented Feb 23, 2021

It seems that the issue in tests was introduced in this commit by @ggeurts.

Gerke, can you please help to figure out why this doesn't actually work as expected:

<SetTargetFramework>TargetFramework=net35</SetTargetFramework>

Also can you please try the latest version of the library and confirm whether it still works for you targeting .NET 3.5 or not?

@abatishchev
Copy link
Member

abatishchev commented Feb 23, 2021

@simon-pearson, here's the initial line of code by @ggeurts:

#if NET35 || NET40

((RSACryptoServiceProvider)_privateKey).SignData(bytesToSign, HashAlgorithmName.SHA256);

#else

which you moved around in your commit and if became this:

#if NET35 || NET40

((RSACryptoServiceProvider)_privateKey).SignData(bytesToSign, this.HashAlgorithmInternal);

#else

Which is the only related change I've managed to locate. I believe it worked fine and then it stopped. Seems like a regression :(

@abatishchev
Copy link
Member

It used to pass HashAlgorithmName and now it passes string on .NET 3.5/4.0 what seems to be the reason for the exception in question:

System.Security.Cryptography.CryptographicException: 'Invalid algorithm specified.'

@simon-pearson
Copy link
Contributor

@abatishchev, @joshtate hi - yeah this certainly looks like a regression introduced by my PR, apologies. I'm looking into it now and will try to get a PR up as a priority.

@ggeurts
Copy link
Contributor

ggeurts commented Feb 24, 2021

The test project says this:

<TargetFramework>net46</TargetFramework>

But then this

<SetTargetFramework>TargetFramework=net40</SetTargetFramework>

The Test project is building a .NET 4.6 Framework assembly but referencing the .NET 4.0 dependency. In effect the .NET 4.6 tests are testing the .NET 4.0 code.

@simon-pearson
Copy link
Contributor

Hi all, the good news is I've been able to replicate this bug. This isn't a regression caused by PR #305 - I checked out the code from December of last year and could still replicate with that code.

@joshtate what is the cryptographic service provider (CSP) of your cert? You can find this out by running certutil.exe on the certificate file. If your CSP is 'Microsoft Enhanced Cryptographic Provider v1.0' then this doesn't support SHA256. One way to fix this would be to regenerate your certificate with a CSP supporting SHA256/384/512, for example "Microsoft Enhanced RSA and AES Cryptographic Provider".

From this SO post it seems there's a workaround we can introduce to the RSAlgorithm.Sign(...) method where we create a new RSACryptoServiceProvider and import the keys from the original RSACryptoServiceProvider, e.g.:

        public byte[] Sign(byte[] bytesToSign)
        {
#if NET35 || NET40
            var rsa = _privateKey as RSACryptoServiceProvider;
            var rsaClear = new RSACryptoServiceProvider();
            rsaClear.ImportParameters(rsa.ExportParameters(true));
            return rsaClear.SignData(bytesToSign, HashAlgorithmInternal);
#else
            return _privateKey.SignData(bytesToSign, HashAlgorithmInternal, RSASignaturePadding.Pkcs1);
#endif
        }

I've tested the above implementation in .NET 4.0 and I can encode/decode a request. The only caveat to this is that the certificate must be exportable. @abatishchev, @ggeurts can either of you think of any potential downsides to implementing this?

@ggeurts
Copy link
Contributor

ggeurts commented Feb 24, 2021

When the RSACryptoServiceProvider does not meet the preconditions for the sign algorithm, an exception is what should be thrown. The client code possibly could use the solution above to generate a correct RSACryptoServiceProvider.

@simon-pearson
Copy link
Contributor

It used to pass HashAlgorithmName and now it passes string on .NET 3.5/4.0 what seems to be the reason for the exception in question:

System.Security.Cryptography.CryptographicException: 'Invalid algorithm specified.'

It was passed a string before @abatishchev. The HashAlgorithmName class was introduced in .NET 4.6. When targeting .NET 3.5/4.0 another file is included, src\JWT\Compatibility\System.Security.Cryptography\HashAlgorithName.cs, to 'polyfill' the missing functionality:

#if NET35 || NET40
namespace System.Security.Cryptography
{
    internal static class HashAlgorithmName
    {
        public static readonly string MD5 = nameof(MD5);
        public static readonly string SHA1 = nameof(SHA1);
        public static readonly string SHA256 = nameof(SHA256);
        public static readonly string SHA384 = nameof(SHA384);
        public static readonly string SHA512 = nameof(SHA512);
    }
}
#endif

@abatishchev
Copy link
Member

@ggeurts thanks for coming back and weighting on the issue!

In effect the .NET 4.6 tests are testing the .NET 4.0 code.

I'm not very familiar with this technique but your explanation sounds good to me, means there is no issue with properly targeting the lower frameworks in the tests.

@ggeurts
Copy link
Contributor

ggeurts commented Feb 24, 2021

See dotnet/sdk#2280 for a bit more <SetTargetFramework> background. Though this metadata element on project reference is not often used, it saved me having to write different test code for the .NET 4.0 and 3.5 versions of the JWT.NET library.

@abatishchev
Copy link
Member

abatishchev commented Feb 24, 2021

Does the library really need to do this (optionally or always):

#if NET35 || NET40
  var rsa = _privateKey as RSACryptoServiceProvider;
  var rsaClear = new RSACryptoServiceProvider();
 
  rsaClear.ImportParameters(rsa.ExportParameters(true));
  return rsaClear.SignData(bytesToSign, HashAlgorithmInternal);
#else

While it can be kept up to the consumer to "cook" the RSA object and pass whatever it deems appropriate?

@simon-pearson
Copy link
Contributor

simon-pearson commented Feb 25, 2021

Is there any outstanding action on this issue? It looks like @joshtate's certificate had a CSP not supporting SHA256 and if he re-generates his certificate with a CSP supporting SHA256/384/512, e.g. "Microsoft Enhanced RSA and AES Cryptographic Provider" then it should work? Alternatively, he could implement the above workaround in his calling code?

@abatishchev abatishchev added question and removed bug labels Feb 25, 2021
@joshtate
Copy link
Author

I'll have to check this later on, but back to my original issue: if there's something about my cert that needs to change then why does this work with .NET 4.7.2:

var rs256 = new RS256Algorithm(cert.GetRSAPublicKey(), cert.GetRSAPrivateKey());

But this doesn't?

var rs256 = new RS256Algorithm((RSA)cert.PublicKey.Key, (RSA)cert.PrivateKey);

If my cert didn't support SHA256 wouldn't it fail in both scenarios?

@ggeurts
Copy link
Contributor

ggeurts commented Feb 26, 2021

@joshtate It looks like GetRSAPrivateKey() does perform a similar operation to the manual code posted above. See reference source of RSACertificateExtensions.GetRSAPrivateKey for example. So the answer to your question is no.

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

No branches or pull requests

4 participants