Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

CryptoConfigShims incorrectly calls an internal .NET core method using reflection #266

Closed
mgiles opened this issue Jul 28, 2020 · 1 comment · Fixed by #272
Closed

CryptoConfigShims incorrectly calls an internal .NET core method using reflection #266

mgiles opened this issue Jul 28, 2020 · 1 comment · Fixed by #272

Comments

@mgiles
Copy link

mgiles commented Jul 28, 2020

When attempting to sign a VSTO ClickOnce archive, I encounter the following error:

Late bound operations cannot be performed on types or methods for which ContainsGenericParameters is true.

In my case, this is being raised from line 35 in CryptoConfigShims:

var item = (AsymmetricSignatureFormatter)CryptoHelpersCreateFromName.Invoke(null, new object[] { FormatterAlgorithm });

Where CryptoHelpersCreateFromName is fetched from an internal .NET core assembly using reflection:

var helperType = Type.GetType("System.Security.Cryptography.Xml.CryptoHelpers, System.Security.Cryptography.Xml");
CryptoHelpersCreateFromName = helperType.GetTypeInfo().GetDeclaredMethod("CreateFromName");

I believe the error is being raised because, in current versions of .NET core, the CreateFromName method is generic, but the generic type parameter is not being instantiated before the method is Invoke'd. It looks to me like originally, when this code was written, the method was not generic, but in .NET core 2.0.8 it was made generic.

As a workaround, I changed the call to the public method CryptoConfig.CreateFromName, and this seemed to make signing work correctly in my case. This has slightly different behaviour than the old reflection call, though.

I have a few ideas for fixes, but would appreciate some guidance:

  1. Update the existing code to instantiate the generic parameter before Invokeing the method. This would most closely match the previous behaviour, but would still depend on an internal method that may unexpectedly change again in the future.
  2. Call a different, public method like CryptoConfig.CreateFromName (I have no idea if the change in behaviour is important!)
  3. Reproduce the code from the internal CryptoHelpers class inside this project, to keep the previous behaviour without needing to call the internal code

I lean towards the third option, but am unsure. I'm happy to open a PR for any of these, or a different solution. Any thoughts on the best approach?

@clairernovotny
Copy link
Contributor

Thanks for looking into this. I'm sure you're right in your analysis as this code hasn't been touched in a long time. I'm sure that changes in the underlying runtime broke this.

I'm not sure if the change in behavior is relevant? If you're able to get it working and have VSTO working, then that's all that really matters, I think. I'd gladly take a PR.

mgiles pushed a commit to mgiles/SignService that referenced this issue Aug 10, 2020
dotnet#266

The previous code attempts to invoke an internal method in .NET Core as
if it's non-generic, but in recent framework versions it _is_ generic.
This commit replaces the call with a non-reflective call on a public
method to avoid similar backwards-breaking changes in the future.

Note: There are some special cases handled by the internal method
System.Security.Cryptography.Xml.CryptoHelpers.CreateFromName that
are not handled by the replacement method, CryptoConfig.CreateFromName.
However, the only names used in CryptoConfigShims are for
RSAPKCS1SignatureFormatter and RSAPKCS1SignatureDeformatter, neither of
which appear to be in the list of special cases. So for the purposes of
CryptoConfigShims, I think the new behaviour is equivalent.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants