* 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.
* GeneratePackageOnBuild
IncludeSymbols
* Add packages to artifacts
* Update src/Renci.SshNet/Renci.SshNet.csproj
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Update Renci.SshNet.csproj
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Delete build/nuget/SSH.NET.nuspec
* Delete SSH.NET.nuspec from .sln file
* Update Renci.SshNet.csproj
* Update build.proj
* init nbgv
* Include version.json in "Solution Items"
* Update Directory.Build.props
* define publicReleaseRefSpec
* update version.json
* update version.json
* set cloud build number
* fix https://github.com/sshnet/SSH.NET/issues/1292
* remove unexpected code format
* Delete version from csproj
* move nbgv from Directory.Build.props to Renci.SshNet.csproj as only this particular project needs versioning.
include package version in ThisAssembly and use nuget package version for the SSH client version. Since we define the precision of nuget package to "build", it has 3 digits. Nuget package version is unique which should be suffient.
* Use package from CI feed
* Bump version to 2024.0.1; Update version precision
* Some tweaks:
- Remove the "release" section. I don't think we will use that for now
- Remove the "cloudBuild" section. Changing the CI build number doesn't seem that useful
- In the "nugetPackageVersion" section:
- Remove "precision". It defaults to build
- Add "semVer"=2. This makes the package version e.g. 2024.1.1-prerelease.1 instead
of 2024.1.1-prerelease-0001
- In "assemblyVersion" change "precision" to revision. Doesn't seem to change anything, I
was just copying nbgv's setup: https://github.com/dotnet/Nerdbank.GitVersioning/blob/main/version.json
- Make sure there are no '-' in the softwareversion string (change the test to a regex)
* Revert unnecessary test changes; remove unnecessary tests
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
* Implements ChaCha20 cipher algorithm.
* Implements chacha20-poly1305@openssh.com
* Update Cipher.cs
* Update ChaCha20Poly1305Cipher.cs
* Note that the length of the concatenation of 'packet_length',
'padding_length', 'payload', and 'random padding' MUST be a multiple
of the cipher block size or 8, whichever is larger.
See https://www.rfc-editor.org/rfc/rfc4253#section-6
* Use Chaos.Nacl Poly1305Donna
* Fix build. Fix typo. Update README
* Update README.md
* Fix build
* Remove trailing whitespace
* Fix build
* Change to BouncyCastle
* Inherit from SymmetricCipher instead of StreamCipher since StreamCipher is deleted
* Resolve conflicts
* Move field to local variable
* Compute poly key stream once
* Update test/Renci.SshNet.IntegrationTests/CipherTests.cs
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Fix build; Add net48 integration test in CI
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
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>
.NET 9 Preview 6 now also scans transitive dependencies, which
causes a build failure in the test projects:
warning NU1902: Package 'BouncyCastle.Cryptography' 2.2.1 has a known moderate severity vulnerability, https://github.com/advisories/GHSA-8xfc-gm6g-vgpv
Don't treat these as errors and update Testcontainers to fix the warning.
* Ref System.Memory for net462 and netstandard2.0.
Use System.Buffers.Binary.BinaryPrimitives instead of Renci.SshNet.Common.Pack
* Remove unnecessary `AsSpan()`
* Apply suggestions from code review
* that's what I get for being lazy
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
* Add SshCommand.ExecuteAsync
After the previous change (#1423), this basically entails swapping out the
IAsyncResult for a TaskCompletionSource and hooking up the cancellation/timeout
logic.
As with the prior Begin/End implementation, the initiation of the command
(SendExecRequest) happens synchronously, so there's a bit of room for improvement
there, but otherwise it is the Task-based async that we know and like.
I chose to make it void (Task)- returning instead of string like in the existing
overloads, so that OutputStream is not automatically consumed (and encoded as a
string) when that may not be desired. As in #650, I was initially considering
changing the other overloads to be void-returning as well, but decided that it was
not worth the break since most people will probably want to change over to
ExecuteAsync anyway.
* Update examples
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
The library currently performs a DNS lookup of the desired host, takes the first
returned IP address and connects to that. Instead, we can just pass the hostname
down to System.Net.Sockets which will do the right thing, potentially trying
multiple addresses if needed.
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Support creating Shell(Stream) without PTY
Fixes https://github.com/sshnet/SSH.NET/issues/1418
* Add integration test for "PermitTTY no"
* Fix Integration Test
* Remove duplicate shell request
* Put common operations in a shared constructor. Update xml doc comments.
* Update comments and method overriding
* Update per code review
* Update integration tests
* Renaming
* Make `bufferSize` optional
* Try fix the test
* Update per code review
* try agian
* try again
* docs
* doc
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* fix flaky SFTP file time tests
These asserts currently assume that DateTime.Now and the
creation timestamp on the server happen within the same second.
Note that I didn't change the asserts below where an explicit
timestamp is set. From my understanding, these should not be affected.
example: https://ci.appveyor.com/project/drieseng/ssh-net/builds/49850731/job/u6lk0cyipbxkp4jd
* remove asserts
Apply similar treatment to PipeStream as #1322 did to ShellStream
PipeStream now behaves much more Stream-like. In particular, it performs partial
reads (instead of blocking until a certain amount of data is available), blocks
until data is available (instead of returning 0 prematurely) and removes the
Stream-unlike properties `BlockLastReadBuffer` and `MaxBufferLength`.
Sadly I gave up trying to make a benchmark compatible with all the quirks of the
previous implementation, but a dumb throughput test (reading and writing simultaneously)
shows about 5.2GB/s with this implementation compared to 140MB/s previously.
Some cleanup of its usage in the library followed.
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* fix flaky Sftp_BeginUploadFile test
this test can randomly fail because it assumes that the callback
has been called when the AsyncWaitHandle was set. But this is not
necessarily the case because AsyncResult.SetAsCompleted does it
the other way around.
example: https://ci.appveyor.com/project/drieseng/ssh-net/builds/49831002/job/1237d4lg46j22pf0
* use ManualResetEventSlim
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* fix flaky ReceiveOnServerSocketShouldReturnZero test
ReceiveOnServerSocketShouldReturnZero assumed that the disconnect
already happened on the server side, but it never waits for this.
This could cause ServerSocket.Receive to still return valid data
and fail the test.
example: https://ci.appveyor.com/project/drieseng/ssh-net/builds/49836561/job/wen5tjd1c7wgxrfh
* Update test/Renci.SshNet.Tests/Classes/SessionTest_Connected_Disconnect.cs
---------
Co-authored-by: Rob Hague <rob.hague00@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>
* Tweak AsyncSocketListener in tests
Some CI runs have been crashing lately from within AsyncSocketListener.
This moves a bit of disconnection & exception handling around. It probably won't
fix the underlying tests which would have otherwise failed, but it might stop the
process from outright crashing.
Also reference the same code for the integration tests.
* Simplify the diff
* Stabilise a couple of tests
* 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
* Add timeouts when reading from sockets in Socks5Connector
* Add a Socks5 timeout test for a connection reply
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Implement OpenSSH strict key exchange extension
* The pseudo-algorithm
is only valid in the initial SSH2_MSG_KEXINIT and MUST be ignored
if they are present in subsequent SSH2_MSG_KEXINIT packets.
* Only send strict kex pseudo algorithm for the first kex.
Strictly disable non-kex massages in strict kex mode.
* Unit tests for strict kex
* More unit tests
* More unit tests
* Correct file name
* Update SessionTest_ConnectingBase.cs
* More unit tests
* Delete SessionTest_Connecting_ServerSendsMaxIgnoreMessagesBeforeKexInit.cs
* Add a comment about throwing exception when inbound sequence number is about to wrap during init kex.
* Delete SessionTest_Connecting_ServerSendsDebugMessageAfterKexInit_NoStrictKex.cs
* Fix build
* Update test/Renci.SshNet.Tests/Classes/SessionTest_Connected.cs
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Init AeadCipher
* Move AeadCipher to parent folder. Move EncryptBlock/DecryptBlock from SymmetricCipher to BlockCipher
* simplify parameter name
* Implement AesGcmCipher
* Update README
* Remove protected IV from AeadCipher; Set offset to outbound sequence just like other ciphers
* Rename associatedData to packetLengthField
* Use Span<byte> to avoid unnecessary allocations
* Use `Span` to improve performance when `IncrementCounter()`
* Add `IsAead` property to `CipherInfo`. Include packet length field and tag field in offset and length when call AesGcm's `Decrypt(...)` method. Do not determine HMAC if cipher is AesGcm during kex.
* Fix build
* Fix UT
* Check `AesGcm.IsSupported` before add to the `Encryptions` collection.
Guard AES-GCM with `NET6_0_OR_GREATER`.
Insert AES-GCM ciphers right after AES-CTR ciphers but before AES-CBC ciphers, which is similar with OpenSSH:
```
debug2: ciphers ctos: chacha20-poly1305@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com
debug2: ciphers stoc: chacha20-poly1305@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com
```
Although Dictionary's order is not defined, from observation, it is in the same order with add. Anyway that would be another topic.
* Suppress CA1859 "Use concrete types when possible for improved performance" for `ConnectionInfo.Encryptions`.
Test `Aes128Gcm` and `Aes256Gcm` only when `NET6_0_OR_GREATER`
* Update xml doc comments. Do not treat AesGcmCipher separately in Session.cs
* Fix build
* Fix build
* Update the comment as ChaCha20Poly1305 uses a separated key to encypt the packet length and the size it 4.
* Update src/Renci.SshNet/Security/Cryptography/Cipher.cs
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Make `AesGcmCipher` internal.
Assert offset when decrypt.
* Fix nullable error in build
* typos
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Integrate `ZLibStream` from .NET 6.0+ with SSH.NET.
* OpenSSH server does not support zlib (pre-auth); OpenSSH client still supports zlib (pre-auth)
* Correct compression algorithm name; Update README.md
* Integrate `ZLibStream` from .NET 6.0+ with SSH.NET.
* OpenSSH server does not support zlib (pre-auth); OpenSSH client still supports zlib (pre-auth)
* Correct compression algorithm name; Update README.md
* Test the compression by upload/download file
* Refactor compression.
* Move delayed compression logic to base class.
* seal Zlib
* update unit test
* update unit test to see if it can trigger integration test
* Flush zlibStream
* Fix integration test
* update test
* Update ConnectionInfo.cs
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Update README.md
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* debugging CI
* apt-get
* print memory
* x
* x
* Remove some unbounded random.Next() calls
Some of which are causing giant array allocations unnecessarily. Should stabilise CI.
* cleanup
* Handle unknown channel messages correctly
See discussion #1218 . Some servers send custom channel messages
like 'keepalive@proftpd.org' as keep alive messages. This currently
causes a NotSupportedException.
According to the spec https://datatracker.ietf.org/doc/html/rfc4254#section-5.4 :
"If the request is not recognized or is not
supported for the channel, SSH_MSG_CHANNEL_FAILURE is returned."
Send a failure message back instead of throwing an exception.
* consider WantReply before sending failure reply
* Use RemoteChannelNumber for failure message
* fix wrong ChannelNumber in SshCommand Channel Response
not directly related to the PR, was noticed during Code Review.
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* SftpClient: handle the SFTP session being closed by the server
If the server closes the SFTP session but keeps the TCP connection open,
this currently causes IsConnected to return true, but any operation
fails with "the session is not open".
SftpClient.IsConnected now also check sftpSession.IsOpen. Connect()
and ConnectAsync() were reworked to take into account that the Session
may already/still be open, but the SFTP session may not. This is needed
so a reconnect works.
fixes#843 and #1153
* Always re-create session
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Co-authored-by: Igor Milavec <igor.milavec@gmail.com>
* Fixed warnings in tests.
* Fixed warnings in main project.
* Fix public key.
* Further warning cleanup.
* Muted remaining warnings.
* Final cleanup.
* Fix warning.
* Fix test classes.
* Fix more internals.
* Revert "internal" in test projects
* Revert more internals.
* Revert all non-analyzer changes.
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Fix CancelAsync Cause Deadlock
* Fix CancelAsync Cause Deadlock
* Support manual cancelling if exit-signal does not cancel
* Fix switch with duplicate case
* Revert wait exit response, use existing OperationCancelledException
* Not executing callback when command is cancelled
* Updated dependencies.
* Reverted Microsoft.Bcl.AsyncInterfaces back to version 1.0.0.
Also added a note so we no longer try to upgrade it.
* Reverted back to Moq.
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Added guard clauses to various timeouts to ensure they don't exceed an Int32 in milliseconds.
* Fixed guard clauses.
* Updated build tags.
* Added guard clauses to various timeouts to ensure they don't exceed an Int32 in milliseconds.
* Fixed tests.
* Added additional tests.
* Replaced NoWarn with .editorconfig setting
* Fixed references to parameter names.
* Restore Write/Flush tests
* Restore write buffer to ShellStream and painfully fix the mocks
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Fix a few issues with ShellStream
The main change is to replace the Queue<byte> with a byte[] and a couple of
variables which index the start and end of the data. The remainder is mainly
slightly more careful locking semantics.
It also implements Expect(string) separately so that it can work on the bytes
and skip a lot of encoding work (this is where I wish ShellStream derived from
StreamReader).
One possibly contentious point: in fixing the Write behaviour I chose to
remove the "outgoing" buffer and immediately send the data across the channel.
Write(string) and WriteLine(string) were already doing this, and I felt it
was better to change Write(byte[]) to match rather than changing the string
methods.
* Integrate expectSize (as "windowSize" parameter)
* Rename "windowSize" to "lookback"
---------
Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>
* Support ETM (Encrypt-then-MAC) variants for HMAC
* Support ETM (Encrypt-then-MAC) variants for HMAC
* Remove `ETM` property from `HashInfo`
* Add support for HmacMd5Etm, HmacMd5_96_Etm, HmacSha1Etm and HmacSha1_96_Etm
Explicitly specify etm even if false
* Add Encrypt-then-MAC variants to README.md
* Add back empty span to prevent auto link
* Store ETM in `HashInfo`;
Change `HMAC Create[...]Hash()` to `HashAlgorithm Create[...]Hash(out bool isEncryptThenMAC)`
Rob Hague