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

Remove uses of Rijndael from EncryptedXml where possible #54238

Merged
merged 3 commits into from
Jun 16, 2021

Conversation

vcsjones
Copy link
Member

This removes uses of RijndaelManaged and replaces it with Aes since as of .NET Core, the Rijndael types were a simple shim to the AES types, anyway.

This removes the project-level warning ignore and adds targeted error supressions.

Closes #54145.

@ghost
Copy link

ghost commented Jun 15, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

This removes uses of RijndaelManaged and replaces it with Aes since as of .NET Core, the Rijndael types were a simple shim to the AES types, anyway.

This removes the project-level warning ignore and adds targeted error supressions.

Closes #54145.

Author: vcsjones
Assignees: -
Labels:

area-System.Security

Milestone: -

@vcsjones vcsjones changed the title Remove uses of Rijndael where possible Remove uses of Rijndael from EncryptedXml where possible Jun 15, 2021
@bartonjs
Copy link
Member

Most overly signed off change ever? 😄

@bartonjs
Copy link
Member

Interestingly, we already did this fix in .NET Framework. And fixed that the value didn't get disposed.

https://referencesource.microsoft.com/#System.Security/system/security/cryptography/xml/encryptedxml.cs,546

@vcsjones
Copy link
Member Author

And fixed that the value didn't get disposed.

Eh I thought about it and decided not to, but if the Framework did it, I feel more comfortable doing it.

This matches the behavior of the .NET Framework.
@GrabYourPitchforks
Copy link
Member

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vcsjones
Copy link
Member Author

@GrabYourPitchforks It looks like Pipelines are running... statuses just aren't getting reported to GitHub. https://dev.azure.com/dnceng/public/_build/results?buildId=1190227&view=results

@GrabYourPitchforks
Copy link
Member

Per https://dev.azure.com/dnceng/public/_build/results?buildId=1189505&view=results, tests are passing except for some filesystem stuff which is already being tracked.

@GrabYourPitchforks GrabYourPitchforks merged commit 9d0be76 into dotnet:main Jun 16, 2021
@vcsjones vcsjones deleted the fix-54145 branch June 16, 2021 16:46
@ghost ghost locked as resolved and limited conversation to collaborators Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EncryptedXml uses obsolete Rijndael* types
5 participants