Skip to content

Use big endian buffers instead of BigInteger for RSA ASN types #117851

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

Merged

Conversation

PranavSenthilnathan
Copy link
Member

RSA parameters are big endian in PKCS1/ASN.1 and in RSAParameters for Windows. Currently we use the RSAPrivateKeyAsn intermediary type to go between the two. However, RSAPrivateKeyAsn uses BigInteger which is little endian and thus causes 2 endianness switches during the conversion. This PR changes RSAPrivateKeyAsn to be big endian and makes the various RSA implementations consume it.

@PranavSenthilnathan PranavSenthilnathan self-assigned this Jul 19, 2025
@Copilot Copilot AI review requested due to automatic review settings July 19, 2025 14:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes RSA key parsing by replacing BigInteger intermediary types with big endian byte buffers in RSA ASN.1 types. The change eliminates unnecessary endianness conversions (BigInteger is little endian, but RSA parameters are big endian in PKCS1/ASN.1 and RSAParameters). The refactoring introduces callback-based APIs for better memory management and consolidates RSA key format handling logic.

Key Changes:

  • Modified RSA ASN.1 types (RSAPrivateKeyAsn, RSAPublicKeyAsn) to use ReadOnlyMemory<byte> instead of BigInteger
  • Introduced callback-based APIs in RSAKeyFormatHelper for efficient parameter extraction
  • Removed duplicate RSA parsing logic from CompositeMLDsaManaged.RSA.cs by leveraging the centralized helper methods

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
RSAPrivateKeyAsn.xml Updated ASN.1 schema to use ReadOnlyMemory backing type
RSAPrivateKeyAsn.xml.cs Generated code changes for ReadOnlyMemory-based integer fields
RSAPublicKeyAsn.xml Updated ASN.1 schema to use ReadOnlyMemory backing type
RSAPublicKeyAsn.xml.cs Generated code changes for ReadOnlyMemory-based integer fields
RSAKeyFormatHelper.Pkcs1.cs Added callback-based APIs for RSA parameter extraction
RSAKeyFormatHelper.cs Simplified existing methods to use new callback APIs
KeyBlobHelpers.cs Updated helper methods to work with ReadOnlyMemory instead of BigInteger
CompositeMLDsaManaged.RSA.cs Replaced custom parsing logic with centralized helper methods
RSA.cs Simplified ImportRSAPrivateKey to use callback-based API
System.Security.Cryptography.csproj Added PinAndClear.cs to Browser target
Microsoft.Bcl.Cryptography.csproj Added RSA ASN.1 types to project
Comments suppressed due to low confidence (1)

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.

@PranavSenthilnathan PranavSenthilnathan enabled auto-merge (squash) July 21, 2025 17:49
@PranavSenthilnathan
Copy link
Member Author

/ba-g wasm timeouts

@PranavSenthilnathan PranavSenthilnathan merged commit c9ccccf into dotnet:main Jul 22, 2025
83 of 87 checks passed
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.

2 participants