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>
* Drop net7.0 target
.NET 7 is EOL since May. The only .NET 7 features we use are
`ObjectDisposedException.ThrowIf` (moved to a throw helper) and
some newer regex features.
This feels a bit weird, but I suppose it is the expected course of action.
* fix build warning-as-error which is suddenly appearing on net6.0
IsAotCompatible not supported on net6.0
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Replace DiagnosticAbstrations with Microsoft.Extensions.Logging.Abstractions
* add documentation
* reduce allocations by SessionId hex conversion
generate the hex string once instead of every log
call and optimize ToHex().
* Update docfx/logging.md
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* reduce log levels
* hook up testcontainers logging
* drop packet logs further down to trace
* add kex traces
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Add .NET 9 target
* Disable SonarSource S3236
This following change in the runtime now causes this analyzer
to complain about some Debug.Assert calls which doesn't make sense.
https://github.com/dotnet/core/blob/main/release-notes/9.0/preview/preview7/libraries.md#debugassert-now-reports-assert-condition-by-defaulthttps://rules.sonarsource.com/csharp/RSPEC-3236/
* make use of .NET 9 Lock type
see https://github.com/dotnet/runtime/issues/34812
* Define own Lock type to avoid ifdefs
* revert irrelevant style changes
* update global.json
* Keep net8.0 target in IntegrationTests
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* fix Package Downgrade Warning
for some reason this happens starting with .NET 9.0 RC2:
/home/mus/git/SSH.NET/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj : error NU1605:
Warning As Error: Detected package downgrade: BouncyCastle.Cryptography from 2.4.0 to 2.3.1. Reference the package directly from the project to select a different version.
Renci.SshNet.IntegrationTests -> SSH.NET 1.0.0 -> BouncyCastle.Cryptography (>= 2.4.0)
Renci.SshNet.IntegrationTests -> Testcontainers 3.10.0 -> BouncyCastle.Cryptography (>= 2.3.1)
* update global.json to RC2
* update global.json to .NET 9 GA
* update GitHub Actions for .NET 9
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Migrate from AppVeyor to GitHub Actions
* also run on pull_request
* small formatting improvements
* add on: workflow_dispatch
this is needed to re-run jobs manually from the web UI
* Publish NuGet package to GitHub Registry
only on develop branch.
* re-add empty appveyor.yml
so it doesn't fail CI until AppVeyor integration is disabled
* fix appveyor
* typo
* Split PrivateKeyFile into different implementations.
* Remove duplicate keyName check. Get cipherName and salt only if the key is PKCS1 format.
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Handle lower-case hex in private key's salt field
I'm using BouncyCastle (http://bouncycastle.org/) to produce public/private key pairs. In later versions of SSH.NET an exception is thrown (SshException: "Invalid private key file.") while establishing connection using the private keys previously generated.
It seems to be an issue with the regex matching the private key file data which this commit handles properly.
* add test
---------
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
for some reason this happens starting with .NET 9.0 RC2:
/home/mus/git/SSH.NET/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj : error NU1605:
Warning As Error: Detected package downgrade: BouncyCastle.Cryptography from 2.4.0 to 2.3.1. Reference the package directly from the project to select a different version.
Renci.SshNet.IntegrationTests -> SSH.NET 1.0.0 -> BouncyCastle.Cryptography (>= 2.4.0)
Renci.SshNet.IntegrationTests -> Testcontainers 3.10.0 -> BouncyCastle.Cryptography (>= 2.3.1)
* Added support for deleting directories asynchronously
* Clarify that the task represents the asynchronous delete operation
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Added DeleteAsync and DeleteDirectoryAsync to ISftpClient
* Inherit docs from interface
* Added additional tests for new async delete functions
* Update list directory test to use async delete methods
* x
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Add support for OpenSSL PKCS#8 private key format
* Update comments
* Convert public key to ssh format
* Convert existing keys instead of generate new keys; Use DataRow for testing
* Minimize the change
* Minimize the change
* Fix build
* Update SonarAnalyzer.CSharp
* fix S3993: Custom attributes should be marked with "System.AttributeUsageAttribute"
https://rules.sonarsource.com/csharp/RSPEC-3993/
* fix S6966: Awaitable method should be used
Introduced abstractions for CancellationTokenSource.CancelAsync()
and Stream.DisposeAsync() to avoid #ifdef.
Supressed pipeStream.WriteAsync because it deadlocks the test.
I assume because PipeStream doesn't override WriteAsync.
https://rules.sonarsource.com/csharp/RSPEC-6966/
temp
* fix S3431: "[ExpectedException]" should not be used
Removed the Connect() from Multifactor_PublicKeyWithEmptyPassPhrase
because the Exception is already thrown in the factory.
https://rules.sonarsource.com/csharp/RSPEC-3431/
* fix S2325: Methods and properties that don't access instance data should be static
This one is pretty redundant with CA1822 (which is also disabled in the
tests).
It caught a few more cases in the library itself, most of which can't
be changed because they are public API.
https://rules.sonarsource.com/csharp/RSPEC-2325/
* fix S127: "for" loop stop conditions should be invariant
not sure if this one is worth having. The only cases it found
are imho legitimate or not worth fixing, so I supressed them.
https://rules.sonarsource.com/csharp/RSPEC-127/
* fix S1964: An abstract class should have both abstract and concrete methods
Suppressed for public APIs, changed ExtendedReplyInfo to interface.
https://rules.sonarsource.com/csharp/RSPEC-1694/
* Remove redundant test
this is already covered by Test_PrivateKey_SSH2_Encrypted_ShouldThrowSshPassPhraseNullOrEmptyExceptionWhenPassphraseIsNull
* Revert "fix S2325: Methods and properties that don't access instance data should be static"
suppress it instead
This reverts commit 2020604958.
* Revert "fix S127: "for" loop stop conditions should be invariant"
suppress it instead
This reverts commit 1d8b4ac335.
* fix "client not connected" after SFTP reconnect
if the server closes the session and the client reconnects,
this currently leads to a broken state because the session
is re-created, but the SFTP subsession is not and still
references the old session.
This causes all operations to fail with "client not connected" or
even throwing the "An established connection was aborted by the server."
exception of the old session.
Always re-create the SFTP subsession to fix this.
fixes#1474
* Dispose old session on reconnect
All of the finalizers in the library are no-ops, but their existence means that
when Dispose (and thus GC.SuppressFinalize) is not called, the objects' lifetimes
are extended unnecessarily while they are waiting to be finalized.
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Use BouncyCastle ECDsa when runtime is Mono
* Falls back to use BouncyCastle if CngKey.Import throws NotImplementedException (in Mono)
* Take NETStandard into consideration
* Adjust some comments
* Change #if NETFRAEWORK to #if NET462 for CngKey
* Separate implementations
* Consolidate Ecdsa property and HashAlgorithm property
* Rename Import_Cng and Import_Bcl to Import; Rename Export_Cng and Export_Bcl to Export;
* Add comments
* refactor
* add host key tests
---------
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* [AesGcmCipher] Use BouncyCastle as a fallback if BCL does not support.
* Switch back to collection initializer
* Remove conditional compilation
* Throw SshConnectionException with Reason MacError when authentication tag mismatch
* Separate BCL and BouncyCastle implementation
* Update AesGcmCipher.BclImpl.cs
* Naming enhancement
* Remove empty line
* Disable S1199. See https://github.com/sshnet/SSH.NET/pull/1371#discussion_r1704293356
* Set InnerException when MAC error. Remove Message check.
* Store KeyParameter as private field
* Use GcmCipher.ProcessAadBytes to avoid the copy of associated data
* Move nonce to constructor to avoid creating AeadParameters each packet
* Use const int for tag size
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
These tests were presumably once shared with the old integration tests repo
but have since been sat doing nothing. This brings them into the unit tests
project.
Co-authored-by: Wojciech Nagórski <wojtpl2@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.
These have started failing for me locally (Win11). SocketError.NoData means
"The requested name or IP address was not found on the name server." which
lines up with what we are testing here.
* Use BCL ECDiffieHellman for KeyExchange (.NET 8.0 onward only)
* Add back an empty line
* Remove the BouncyCastle dependency when target .NET 8.0 onward.
* Run KeyExchangeAlgorithmTests for .NET 6.0
* Build Renci.SshNet.IntegrationTests.csproj for net6.0
* Update filter
* Add back BouncyCastle as fallback
* Add back the missing `SendMessage`
* Run ECDH KEX integration tests under .NET48
* Use SshNamedCurves instead of SecNamedCurves for BouncyCastle.
BCL supports both names. See https://github.com/dotnet/runtime/blob/main/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OidLookup.cs#L200-L202
* typo
* Fix build
* Use System.Security.Cryptography namespace if NET8_0_OR_GREATER;
Use one parameter constructor for class ECDomainParameters
* Separate BCL and BouncyCastle implementation
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
Rob Hague