* Initial plan
* Refactor collection assertions to use CollectionAssert.AreEqual
Replace Assert.IsTrue(xxx.IsEqualTo(yyy)) and Assert.IsTrue(xxx.SequenceEqual(yyy))
with CollectionAssert.AreEqual(expected, actual) across 55 test files
Co-authored-by: Rob-Hague <5132141+Rob-Hague@users.noreply.github.com>
* Fix argument order in CollectionAssert.AreEqual and remove unnecessary using directives
- Fixed argument order in KeyExchangeDhGroupExchangeReplyTest.cs (expected first, actual second)
- Fixed argument order in KeyExchangeInitMessageTest.cs (expected first, actual second)
- Removed unnecessary 'using System.Linq' directives from all affected test files
Co-authored-by: Rob-Hague <5132141+Rob-Hague@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Rob-Hague <5132141+Rob-Hague@users.noreply.github.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Bump the dependencies group with 11 updates
Bumps coverlet.collector from 6.0.4 to 10.0.1
Bumps coverlet.msbuild from 6.0.4 to 10.0.1
Bumps GitHubActionsTestLogger from 3.0.1 to 3.0.4
Bumps Meziantou.Analyzer from 3.0.18 to 3.0.114
Bumps Microsoft.Bcl.Cryptography from 10.0.3 to 10.0.9
Bumps Microsoft.Extensions.Logging.Console from 10.0.3 to 10.0.9
Bumps MSTest from 4.1.0 to 4.2.3
Bumps PolySharp from 1.15.0 to 1.16.0
Bumps SonarAnalyzer.CSharp from 10.20.0.135146 to 10.27.0.140913
Bumps System.Formats.Asn1 from 10.0.3 to 10.0.9
Bumps Testcontainers from 4.10.0 to 4.12.0
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
Support in-place encryption in the cipher types, then use it on the plaintext packets
instead of allocating a new array each time. Removes 2 of 4 bytes allocated for each
byte uploaded over SFTP.
For AES-CTR, supporting in-place encryption in this case means adding a persistent buffer
for the keystream and encrypting in chunks. The performance difference is ~1-2% i.e.
marginal versus one-shotting it. The variance is similar also for different choices of
buffer size (here 4096 is used).
* Bump the dependencies group with 4 updates
Bumps BouncyCastle.Cryptography from 2.6.1 to 2.6.2
Bumps Meziantou.Analyzer from 2.0.205 to 2.0.210
Bumps MSTest from 3.9.3 to 3.10.0
Bumps SonarAnalyzer.CSharp from 10.13.0.120203 to 10.15.0.120848
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
DSA is removed at compile time from OpenSSH 9.8 and higher.
That means we can no longer test it in our integration tests. It seems like a
good time to remove it. From the OpenSSH release notes:
DSA, as specified in the SSHv2 protocol, is inherently weak - being
limited to a 160 bit private key and use of the SHA1 digest. Its
estimated security level is only 80 bits symmetric equivalent.
OpenSSH has disabled DSA keys by default since 2015 but has retained
run-time optional support for them. DSA was the only mandatory-to-
implement algorithm in the SSHv2 RFCs, mostly because alternative
algorithms were encumbered by patents when the SSHv2 protocol was
specified.
This has not been the case for decades at this point and better
algorithms are well supported by all actively-maintained SSH
implementations. We do not consider the costs of maintaining DSA
in OpenSSH to be justified and hope that removing it from OpenSSH
can accelerate its wider deprecation in supporting cryptography
libraries.
* Use System.Security.Cryptography in DesCipher and TripleDesCipher; Fall back to use BouncyCastle if BCL doesn't support
* Drop DesCipher; Replace PKCS7Padding with BouncyCastle's implementation.
* Restore `CbcCipherMode`
* Restore AesCipherMode; Use BlockImpl instead of BouncyCastleImpl for 3DES-CFB on lower targets.
* Restore the xml doc comment
* Tighten private key checking to reveal padding issue
* `Encrypt` should take into account padding for length of `inputBuffer` passed to `EncryptBlock` if padding is specified, no matter input is divisible or not.
* `Decrypt` should take into account unpadding for the final output if padding is specified.
* `Decrypt` should take into account *manual* padding for length of `inputBuffer` passed to `DecryptBlock` and unpadding for the final output if padding is not specified and mode is CFB or OFB.
* `Encrypt` should take into account *manual* padding for length of `inputBuffer` passed to `EncryptBlock` and unpadding for the final output if padding is not specified and mode is CFB or OFB.
* Rectify DES cipher tests. There's no padding in the data.
* Borrow `PadCount` method from BouncyCastle
* Manually pad input in CTR mode as well. Update AesCipherTest.
Co-Authored-By: Rob Hague <5132141+Rob-Hague@users.noreply.github.com>
* Manually pad/unpad for Aes CFB/OFB mode
* Update test/Renci.SshNet.Tests/Classes/Security/Cryptography/Ciphers/AesCipherTest.Gen.cs.txt
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Re-generate AES cipher tests
---------
Co-authored-by: Rob Hague <5132141+Rob-Hague@users.noreply.github.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Use System.Security.Cryptography for DSA
This is the analogue of the RSA change #1373 for DSA. This has a couple of caveats:
- The BCL supports only FIPS 186-compliant keys, that is, public (P, Q) lengths
of (512 <= P <= 1024, 160) for FIPS 186-1/186-2; and (2048, 256), (3072, 256)
for FIPS 186-3/186-4. The latter also specifies (2048, 224) but due to a quirk
in the Windows API, the BCL does not support Q values of length 224[^1].
- OpenSSH, based on the SSH spec, only supports (supported) Q values of length 160,
but appears to also work in non-FIPS-compliant cases such as in our integration
tests with a (2048, 160) host key. That test now fails and I changed that host key
to (1024, 160).
This basically means that (1024, 160) is the largest DSA key size supported by both
SSH.NET and OpenSSH. However, given that OpenSSH deprecated DSA in 2015[^2], and the
alternative that I have been considering is just to delete support for DSA in the
library, this change seems reasonable to me. I don't think we can justify keeping the
current handwritten code around.
I think we may still consider dropping DSA from the library, I just had this branch
laying around and figured I'd finish it off.
[^1]: https://github.com/dotnet/runtime/blob/fadd8313653f71abd0068c8bf914be88edb2c8d3/src/libraries/Common/src/System/Security/Cryptography/DSACng.ImportExport.cs#L259-L265
[^2]: https://www.openssh.com/txt/release-7.0
* Appease mono
* test experiment
* Revert "Appease mono"
This reverts commit 881eefe5e8.
This drops some of the algorithms long-considered legacy/insecure.
The idea is both to improve the theoretical security of the library by not offering
these algorithms, and to improve the practical security of the library by not having
hand-written, barely tested crypto code.
The overarching goal is for the library to have minimal exposure to crypto
implementation, relying firstly on the .NET base libraries, and secondly on
third-party providers, such as BouncyCastle.
This change covers deleting the cipher algorithms arcfour, blowfish, twofish, cast.
It covers deleting the MD5-based and truncated HMAC algorithms.
These were all disabled in OpenSSH server (sshd) in 2014[^1]:
> sshd(8): The default set of ciphers and MACs has been altered to
> remove unsafe algorithms. In particular, CBC ciphers and arcfour*
> are disabled by default.
>
> The full set of algorithms remains available if configured
> explicitly via the Ciphers and MACs sshd_config options.
and in the client in 2016[^2]:
> This release disables a number of legacy cryptographic algorithms
> by default in ssh:
>
> * Several ciphers blowfish-cbc, cast128-cbc, all arcfour variants
> and the rijndael-cbc aliases for AES.
>
> * MD5-based and truncated HMAC algorithms.
>
> These algorithms are already disabled by default in sshd.
This change also drops PKCS5Padding, which is a line-for-line copy of PKCS7Padding,
and StreamCipher, which is now unused (and useless anyway).
[^1]: https://www.openssh.com/txt/release-6.7
[^2]: https://www.openssh.com/txt/release-7.2
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Preparation to enforce formatting and style in CI
- enabled IDE0055 to enforce formatting on build
- disabled SA1137 and SA1025 because they are already
covered by IDE0055
- disabled SA1021 because it conflicts with csharp_space_after_cast
* Cleanup formatting and style on codebase
This commit has no manual changes, it is the result
of running "dotnet format whitespace" and "dotnet format style"
* new formatting fixes after merge
* appveyor: set git autocrlf
as suggested by sharwell to hopefully fix Windows CI.
* use autocrlf input
to hopefully fix Linux tests
* fix formatting
* use autocrlf input for Linux only
* set csharp_space_after_cast to false
* Revert SA1021 suppression
---------
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
* Use BCL for RSA
* Restore benchmark (with quirks for old code)
* Fixup benchmark (remove compatibility with old code)
* cosmetic tweaks
* Add a regression test for #1388
This makes it easier to reason about Key instances in e.g.
DigitalSignature implementations, because we know that the
Key is initialised with its data and will not change.
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Add FEATURE_AES_CSP to use hardware-accelerated AesCryptoServiceProvider
Reduces CPU usage dramatically, allowing more performance on slower machines
* Restructure, move most of the feature code to AesCipher.cs
Fix padding for non-AES blockciphers
Fix IV exception for non-AES blockciphers
* Fix the AES Padding
It looks like the legacy code doesn't correctly remove padding, so this code needs to do the same.
* fix rebase issues
restructure AES CSP code into its own class
* Minor fixes
* Rework based on suggestions
* Move all changes to AesCypher.cs, as per Rob-Hague suggestion
Remove FEATURE_AES_CSP conditional
Fix OFB CipherMode
* update AesCipherTest.cs generator
* Fix continuous session encrypt/decrypt
(preserve IV between calls when Padding is None)
* Reduce CTR memory usage in Net 6+
Small performance increase in CTR buffer mode
Cosmetic changes
* Factor out the implementations and re-add the existing constructor
* remove ctor; revert tests; remove unused _iv member
* Reorder Encryption cipher preference list
* Remove redundant AES tests
Add tests for stream cipher state preservation
* Refactor ArrayXOR()
* Add test for IV overflow
* Performance bump for AES CTR (thanks @robhague)
* fix merge conflict
* Move AesCipherMode enum to its own file
---------
Co-authored-by: Pedro Fonseca <pfonseca@qti.qualcomm.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Increase test coverage for AesCipher
The tests were generated by a script which is also added for posterity.
The script works by running "openssl enc [...]" (via WSL) to generate the
expected encrypted values, and also verifies those values against the .NET
BCL implementation as an extra validation (it uncovered a difference in
CFB mode between the two relating to the feedback size).
* Fix OfbCipherMode
It was an exact copy of CfbCipherMode
* Move test projects to test folder.
Move global.json to root of repo.
Update solution items in solution.
* Move test projects to test folder.
Move global.json to root of repo.
Update solution items in solution.
Update appveyor configuration/
* Attempt to have appveyor use the correct .NET SDK.
* Update .NET SDK to version 7.0.402.
* Move Data folder below Renci.SshNet.Tests.
* Make csinst less chatty.
* Move Data folder directly below test folder as it's used by multiple test projects.
* Remove CS1591 nowarn from concrete test projects as this is already defined in the Directory.Build.props that is in the test folder.
* Fix integration test after moving test projects
---------
Co-authored-by: drieseng <gert.driesen@telenet.be>
Copilot