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

[release/9.0-staging] Fix shimmed implementation of TryGetHashAndReset to handle HMAC. #112015

Open
wants to merge 2 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 30, 2025

Backport of #111929 to release/9.0-staging

/cc @vcsjones

Customer Impact

  • Customer reported
  • Found internally

Reported by a customer in https://stackoverflow.com/questions/79392501/whats-so-dangerous-about-pkcs12loaderlimits-dangerousnolimits (tracked at #111926). Customers that use X509CertificateLoader.LoadPkcs12 from the Microsoft.Bcl.Cryptography package will be unable to use it with the default limits if the platform selects netstandard2.0 assets, and will receive a CryptographicException. This may occur for platforms such as UWP.

This occurred because the validator incorrectly handled the name of HMAC algorithms.

Regression

  • Yes
  • No

Testing

The fix was manually verified by building the changes locally and observing that the exception no long occurred.

Risk

Low. The code that was identified as problematic was removed, as it was determined that checking the algorithm names in that location was not needed.

The TryGetHashAndReset in switches on the algorithm name of IncrementalHash. IncrementalHash prepends "HMAC" in front of the algorithm name, so the shim did not correctly handle the HMAC-prepended algorithm names.
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

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

Successfully merging this pull request may close these issues.

1 participant