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.

Sign up for GitHub

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

Jump to bottom

Use System.Security.Cryptography for TripleDesCipher and fall back to use BouncyCastle if BCL doesn't support #1546

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Use System.Security.Cryptography for TripleDesCipher and fall back to use BouncyCastle if BCL doesn't support #1546

wants to merge 2 commits into from

Conversation

scott-xu
Copy link
Collaborator

@scott-xu scott-xu commented Nov 30, 2024 •
edited
Loading

This PR changes TripleDesCipher to use BCL TripleDES where possible, and falls back to use BoucyCastle if BCL doesn't support.

This PR drops support for DesCipher and changes PKCS7Padding to use BouncyCaslte's implementation.

This PR renames AesCipherMode to BlockCipherMode. Some previous discussion: #865 (comment)

This PR is on top of #1545 and can diff using this link.

Benchmarks based on .NET 9.0.0 (9.0.24.52809), X64 RyuJIT AVX2

Before:

Method Mean Error StdDev Gen0 Gen1 Allocated
Encrypt_CBC 1.379 ms 0.0268 ms 0.0402 ms 39.0625 1.9531 161.19 KB
Decrypt_CBC 1.333 ms 0.0202 ms 0.0189 ms 39.0625 1.9531 161.19 KB
Encrypt_CFB 1.371 ms 0.0263 ms 0.0281 ms 39.0625 1.9531 161.23 KB
Decrypt_CFB 1.379 ms 0.0164 ms 0.0146 ms 39.0625 1.9531 161.23 KB

After:

Method Mean Error StdDev Gen0 Allocated
Encrypt_CBC 1,018.1 us 20.33 us 28.50 us 7.8125 33.18 KB
Decrypt_CBC 966.5 us 19.25 us 20.60 us 7.8125 33.18 KB
Encrypt_CFB 1,016.6 us 15.57 us 14.57 us 7.8125 33.18 KB
Decrypt_CFB 970.4 us 13.38 us 12.52 us 7.8125 33.18 KB

@scott-xu scott-xu force-pushed the des branch 3 times, most recently from eadc5ab to 232e446 Compare December 1, 2024 09:32
@scott-xu scott-xu mentioned this pull request Dec 22, 2024
Merged
@Rob-Hague
Copy link
Collaborator

We could consider dropping DesCipher

@scott-xu
Copy link
Collaborator Author

Agree

@scott-xu scott-xu changed the title Use System.Security.Cryptography for DesCipher and TripleDesCipher and falls back to use BouncyCastle if BCL doesn't support Use System.Security.Cryptography for TripleDesCipher and falls back to use BouncyCastle if BCL doesn't support Dec 22, 2024
@scott-xu scott-xu marked this pull request as ready for review December 22, 2024 16:02
@scott-xu scott-xu changed the title Use System.Security.Cryptography for TripleDesCipher and falls back to use BouncyCastle if BCL doesn't support Use System.Security.Cryptography for TripleDesCipher and fall back to use BouncyCastle if BCL doesn't support Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drieseng drieseng Awaiting requested review from drieseng drieseng is a code owner

@WojciechNagorski WojciechNagorski Awaiting requested review from WojciechNagorski WojciechNagorski is a code owner

@Rob-Hague Rob-Hague Awaiting requested review from Rob-Hague Rob-Hague is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@scott-xu @Rob-Hague