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 RSA.EncryptValue and RSA.DecryptValue #76514

Merged
merged 4 commits into from Oct 5, 2022

Conversation

deeprobin
Copy link
Contributor

Fixes #73969.

namespace System.Security.Cryptography;

public partial class RSA : AsymmetricAlgorithm
{
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public virtual byte[] EncryptValue(byte[] rgb);
    
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public virtual byte[] DecryptValue(byte[] rgb);
}

public partial class RSACryptoServiceProvider : RSA
{
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public override byte[] EncryptValue(byte[] rgb);
    
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public override byte[] DecryptValue(byte[] rgb);
}

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 2, 2022
@ghost
Copy link

ghost commented Oct 2, 2022

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

Issue Details

Fixes #73969.

namespace System.Security.Cryptography;

public partial class RSA : AsymmetricAlgorithm
{
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public virtual byte[] EncryptValue(byte[] rgb);
    
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public virtual byte[] DecryptValue(byte[] rgb);
}

public partial class RSACryptoServiceProvider : RSA
{
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public override byte[] EncryptValue(byte[] rgb);
    
+   [Obsolete("RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.")]
    public override byte[] DecryptValue(byte[] rgb);
}
Author: deeprobin
Assignees: -
Labels:

area-System.Security, new-api-needs-documentation

Milestone: -

@deeprobin
Copy link
Contributor Author

Is there a guideline on obsoletions? Should they theoretically be a constant in Obsoletions.cs or does this not have to be to this small extent (4 public methods)?

@vcsjones
Copy link
Member

vcsjones commented Oct 2, 2022

This should be following the SYSLIB obsoletion pattern. See another PR like #67264 as an example.

  1. Take the next SYSLIB obsolete diagnostic number from list-of-diagnostics.md (and update that table). As of writing, you would use SYSLIB0048.
  2. Diagnostic message and ID are constants in src/libraries/Common/src/System/Obsoletions.cs
  3. The ref file should be generated and it will resolve the constants.

For the tests, you would want to probably surround the obsolete method calls with

#pragma warning disable SYSLIB0048
rsa.EncryptValue(..);
#pragma warning restore SYSLIB0048

@vcsjones
Copy link
Member

vcsjones commented Oct 2, 2022

And make sure the DiagnosticId and UrlFormat is set in the [Obsolete] attribute.

@deeprobin
Copy link
Contributor Author

@vcsjones Ready for re review :D

@vcsjones
Copy link
Member

vcsjones commented Oct 3, 2022

Looks good to me, but @bartonjs will need to give the approval.

@deeprobin
Copy link
Contributor Author

@vcsjones Are you able to request a review from @bartonjs?

@vcsjones vcsjones requested a review from bartonjs October 5, 2022 16:27
@bartonjs bartonjs merged commit d653f2b into dotnet:main Oct 5, 2022
@bartonjs
Copy link
Member

bartonjs commented Oct 5, 2022

@vcsjones Are you able to request a review from @bartonjs?

It was on my TODO list 😄. But, now it's on my TO-DONE! list. Thanks, @deeprobin.

@vcsjones vcsjones added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Oct 5, 2022
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 5, 2022
@ghost
Copy link

ghost commented Oct 5, 2022

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@vcsjones
Copy link
Member

vcsjones commented Oct 5, 2022

Breaking change issue here: dotnet/docs#31614

@vcsjones vcsjones removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 5, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider obsoleting RSA.EncryptValue, RSA.DecryptValue
3 participants