- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
Use output size helpers for cryptographic primitives #82800
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
Conversation
| Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsThis updates a few places that can use algorithm output size helpers. Closes #67059 
 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @vcsjones !
One day we need to discuss our benchmarks coverage:
https://github.com/dotnet/performance/tree/main/src/benchmarks/micro/libraries/System.Security.Cryptography.Primitives
https://github.com/dotnet/performance/tree/main/src/benchmarks/micro/libraries/System.Security.Cryptography.X509Certificates
https://github.com/dotnet/performance/tree/main/src/benchmarks/micro/libraries/System.Security.Cryptography
        
          
                src/libraries/Common/src/System/Security/Cryptography/RSACng.EncryptDecrypt.cs
          
            Show resolved
            Hide resolved
        
      | // Since it isn't a fatal error to miscalculate the estimatedSize, don't throw an exception. Just truck along. | ||
| _ => KeySize / 4, | ||
| }; | ||
| int estimatedSize = GetMaxSignatureSize(DSASignatureFormat.IeeeP1363FixedFieldConcatenation); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this more expensive? The implementation of the method isn't trivial, but maybe it doesn't matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in a micro-benchmark, but ECDSA signing and verifying is going to dominate. For IeeeP1363FixedFieldConcatenation it basically boils down to AsymmetricAlgorithmHelpers.BitsToBytes(KeySize) * 2; Where BitsToBytes just does the "ceiling then divide by 8". I wouldn't be surprised if the JIT inlined BitsToBytes.
This updates a few places that can use algorithm output size helpers.
Closes #67059