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

[API Proposal]: RSA.GetMaxOutputSize #78175

Closed
vcsjones opened this issue Nov 10, 2022 · 4 comments · Fixed by #82565
Closed

[API Proposal]: RSA.GetMaxOutputSize #78175

vcsjones opened this issue Nov 10, 2022 · 4 comments · Fixed by #82565
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Security in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@vcsjones
Copy link
Member

vcsjones commented Nov 10, 2022

Background and motivation

This is a proposal for the discussion in #67059. We have some helpers for {EC}DSA for getting signature sizes. This adds a complementary API for RSA. The method handles a "right" size for encryption and signing, and a "worst case" for decryption.

This is useful if you want to stackalloc or rent a buffer to hold a signature and need to know how much data you need.

Remark:

I considered what this might look like for ECDiffieHellman derived keys. It's a little easier for developers to reason about those sizes from DeriveKeyFrom{Hash,Hmac} without a helper, so I didn't propose anything there.

API Proposal

namespace System.Security.Cryptography {
    public partial class RSA {
        public int GetMaxOutputSize();
    }
}

API Usage

byte[] signatureBuffer = ArrayPool<byte>.Shared.Rent(rsa.GetMaxOutputSize());
int written = rsa.Sign(blah, blah, signatureBuffer);
ReadOnlySpan<byte> signature = signatureBuffer.AsSpan(0, written);

Alternative Designs

Do nothing. Expect that developers know (KeySize + 7) / 8 is correct.

Risks

No response

@vcsjones vcsjones added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security labels Nov 10, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 10, 2022
@ghost
Copy link

ghost commented Nov 10, 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

Background and motivation

This is a proposal for the discussion in #67059. We have some helpers for {EC}DSA for getting signature sizes. This adds a complementary API for RSA, both for creating signatures and PKCS1/OAEP encryption.

This is useful if you want to stackalloc or rent a buffer to hold a signature and need to know how much data you need.

Remark:

I considered what this might look like for ECDiffieHellman derived keys. It's a little easier for developers to reason about those sizes from DeriveKeyFrom{Hash,Hmac} without a helper, so I didn't propose anything there.

API Proposal

namespace System.Security.Cryptography {
    public partial class RSA {
        public int GetSignatureSize();
        public int GetEncryptedDataSize();
    }
}

API Usage

byte[] signatureBuffer = ArrayPool<byte>.Shared.Rent(rsa.GetSignatureSize());
int written = rsa.Sign(blah, blah, signatureBuffer);
ReadOnlySpan<byte> signature = signatureBuffer.AsSpan(0, written);

Alternative Designs

Do nothing. Expect that developers know (KeySize + 7) / 8 is correct.

Risks

No response

Author: vcsjones
Assignees: -
Labels:

api-suggestion, area-System.Security

Milestone: -

@bartonjs
Copy link
Member

Those two things are the same value, so it's unfortunate that they're two different methods. What about GetMaxOutputSize()? It's "how much will be written" for sign and encrypt, and the upper bound for decrypt (yeah, since we only allow for padded RSA the answer for decrypt is always at least 11 bytes less than that, but it's still an "if you're this big, you're safe...")

@vcsjones vcsjones changed the title [API Proposal]: RSA.GetSignatureSize [API Proposal]: RSA.GetMaxOutputSize Nov 10, 2022
@vcsjones
Copy link
Member Author

It's "how much will be written" for sign and encrypt, and the upper bound for decrypt (yeah, since we only allow for padded RSA the answer for decrypt is always at least 11 bytes less than that, but it's still an "if you're this big, you're safe...")

I was wrestling with that and was trying to achieve similar naming as we had for {EC}DSA. I'm not sure "Output" is the right thing there. Output of what? ExportRSAPrivateKey? ToXmlString?

I agree the single method is probably "better" but haven't found a way that I was super comfortable with.

@bartonjs bartonjs added this to the 8.0.0 milestone Nov 11, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 11, 2022
@bartonjs bartonjs added untriaged New issue has not been triaged by the area owner 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 labels Nov 11, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 11, 2022
@terrajobst
Copy link
Contributor

terrajobst commented Feb 23, 2023

Video

Looks good as proposed.

namespace System.Security.Cryptography;

public partial class RSA
{
    public int GetMaxOutputSize();
}

@terrajobst terrajobst 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 Feb 23, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 23, 2023
@vcsjones vcsjones self-assigned this Feb 24, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 24, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2023
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 15, 2024
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 in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants