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

Obsolete the Rijndael and RijndaelManaged classes #46930

Closed
bartonjs opened this issue Jan 13, 2021 · 4 comments · Fixed by #52366
Closed

Obsolete the Rijndael and RijndaelManaged classes #46930

bartonjs opened this issue Jan 13, 2021 · 4 comments · Fixed by #52366
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Security
Milestone

Comments

@bartonjs
Copy link
Member

bartonjs commented Jan 13, 2021

Background and Motivation

.NET Framework added support for the Rijndael algorithm before (a restricted set of it) it got officially accepted as the winner of the contest to become AES. When .NET Framework added a new Aes class the RijndaelManaged class already had a fair amount of use, and Aes wasn't available everywhere (since it was only implemented using system crypto libraries).

For .NET (nee Core), we no longer have the managed implementation of Rijndael, and the RijndaelManaged class just wraps an instance of Aes. Rijndael and RijndaelManaged were brought to Core as part of the .NET Core/Standard 2.0 API alignment wave.

Since the .NET (nee Core) RijndaelManaged doesn't support "full Rijndael" there's no reason to use it in new code.

Proposed API

namespace System.Security.Cryptography.Algorithms
{
+   [Obsolete(someID)]
    [EditorBrowsable(EditorBrowsableState.Never)]
    public partial class Rijndael
    {
    }

+   [Obsolete(someID)]
    [EditorBrowsable(EditorBrowsableState.Never)]
    public partial class RijndaelManaged
    {
    }
}
@bartonjs bartonjs added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 13, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Jan 13, 2021
@ghost
Copy link

ghost commented Jan 13, 2021

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

Issue Details

Background and Motivation

.NET Framework added support for the Rijndael algorithm before (a restricted set of it) it got officially accepted as the winner of the contest to become AES. When .NET Framework added a new Aes class the RijndaelManaged class already had a fair amount of use, and Aes wasn't available everywhere (since it was only implemented using system crypto libraries).

For .NET (nee Core), we no longer have the managed implementation of Rijndael, and the RijndaelManaged class just wraps an instance of Aes. Rijndael and RijndaelManaged were brought to Core as part of the .NET Core/Standard 2.0 API alignment wave.

Since the .NET (nee Core) RijndaelManaged doesn't support "full Rijndael" there's no reason to use it in new code.

Proposed API

namespace System.Security.Cryptography.Algorithms
{
+   [Obsolete(someID)]
    public partial class Rijndael
    {
    }

+   [Obsolete(someID)]
    public partial class RijndaelManaged
    {
    }
}
Author: bartonjs
Assignees: -
Labels:

api-suggestion, area-System.Security, untriaged

Milestone: -

@bartonjs bartonjs added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Jan 13, 2021
@bartonjs bartonjs added this to the 6.0.0 milestone Jan 13, 2021
@vcsjones
Copy link
Member

Perhaps relevant to the discussion: these types are already EditorBrowsableState.Never.

@bartonjs
Copy link
Member Author

Perhaps relevant to the discussion: these types are already EditorBrowsableState.Never.

Good point. Updated the proposal to show that.

@bartonjs
Copy link
Member Author

bartonjs commented Jan 15, 2021

Video

Looks good as proposed.

namespace System.Security.Cryptography.Algorithms
{
+   [Obsolete(someID)]
    [EditorBrowsable(EditorBrowsableState.Never)]
    public partial class Rijndael
    {
    }

+   [Obsolete(someID)]
    [EditorBrowsable(EditorBrowsableState.Never)]
    public partial class RijndaelManaged
    {
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jan 15, 2021
@jeffhandley jeffhandley self-assigned this May 6, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 6, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 6, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants