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

Use System.Security.Cryptography in AesCipher #1235

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

Rob-Hague
Copy link
Collaborator

System.Security.Cryptography.Aes is supported on all target platforms (it wasn't in previous versions of the library). So we can use it rather than the hand-written code.

The change comes with modest performance gains (#865 (comment)) and from a maintenance perspective, makes it easier to achieve the design proposed in the linked comment in order to get much greater performance gains.

@WojciechNagorski
Copy link
Collaborator

I'm really impressed this is a great job!
Can you describe how this PRis related with #865 ?
Can you provide some Benchmark results?

@Rob-Hague
Copy link
Collaborator Author

I would expect this to become the implementation of SshNetCipherModeImpl in the proposal in #865 (comment), which would be used when using the constructor taking Renci.SshNet.Security.Ciphers.CipherMode. #865 would provide a much faster implementation when using a new constructor taking a System.Security.Cryptography.CipherMode value.

Reposting the benchmarks here (they are the same as in the linked comment)

Results on develop branch 826222f:

Method Mean Error StdDev Gen0 Allocated
Encrypt_CBC 320.4 μs 0.47 μs 0.36 μs 15.6250 32.41 KB
Decrypt_CBC 352.3 μs 2.37 μs 1.85 μs 15.6250 32.41 KB
Encrypt_CFB 335.6 μs 0.43 μs 0.34 μs 15.6250 32.45 KB
Decrypt_CFB 367.9 μs 4.23 μs 3.95 μs 15.6250 32.45 KB
Encrypt_CTR 310.4 μs 0.38 μs 0.31 μs 15.6250 32.45 KB
Decrypt_CTR 315.3 μs 3.96 μs 3.70 μs 15.6250 32.45 KB

Results on these changes:

Method Mean Error StdDev Gen0 Allocated
Encrypt_CBC 221.2 μs 2.01 μs 1.88 μs 15.8691 32.7 KB
Decrypt_CBC 220.0 μs 2.73 μs 2.56 μs 15.8691 32.71 KB
Encrypt_CFB 251.3 μs 0.33 μs 0.27 μs 15.6250 32.75 KB
Decrypt_CFB 255.3 μs 0.67 μs 0.60 μs 15.6250 32.75 KB
Encrypt_CTR 201.3 μs 0.41 μs 0.38 μs 15.8691 32.75 KB
Decrypt_CTR 209.3 μs 0.22 μs 0.19 μs 15.8691 32.75 KB

Results on #865 zybexXL@a9f68fb:

Method Mean Error StdDev Gen0 Allocated
Encrypt_CBC 29.637 μs 0.1278 μs 0.1067 μs 16.1133 33.14 KB
Decrypt_CBC 6.579 μs 0.0160 μs 0.0142 μs 16.1285 33.14 KB
Encrypt_CFB 333.761 μs 0.1710 μs 0.1335 μs 15.6250 32.52 KB
Decrypt_CFB 333.899 μs 0.2090 μs 0.1955 μs 15.6250 32.52 KB
Encrypt_CTR 24.630 μs 0.0327 μs 0.0273 μs 78.1250 161.2 KB
Decrypt_CTR 24.544 μs 0.0817 μs 0.0683 μs 78.1250 161.2 KB

The allocations in CTR mode can easily be removed, and I would expect it to extend to CFB as well. So given how much faster it is, it will be worth having as an implementation. The proposal is to keep the implementations separate so that we keep using a Renci.SshNet.Security.Ciphers.CipherMode if it is provided.

@zybexXL
Copy link
Contributor

zybexXL commented Nov 7, 2023

@WojciechNagorski I've restructured #865 to incorporate suggestions from Rob-Hague in the above linked comment. This makes both PRs functionally similar, but #865 is much faster as this one only accelerates blockwise encryption/decryption, while 865 also accelerates one-shot buffer operations.

#865 already used acceleration (2 years ago) but the code was injected into the CipherMode classes instead of AesCipher.cs. I've now refactored and improved it.

@WojciechNagorski
Copy link
Collaborator

@Rob-Hague, @zybexXL Which of these two PRs should be checked first? #1235 #865 They are related and I don't really know where to start.

@WojciechNagorski
Copy link
Collaborator

I like this kind of PR
image

@WojciechNagorski WojciechNagorski merged commit 5021f6d into sshnet:develop Nov 16, 2023
1 check was pending
@Rob-Hague Rob-Hague deleted the aesssc branch November 16, 2023 07:43
@WojciechNagorski WojciechNagorski added this to the 2023.0.1 milestone Nov 16, 2023
@WojciechNagorski
Copy link
Collaborator

The 2023.0.1 version has been released to Nuget: https://www.nuget.org/packages/SSH.NET/2023.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants