#1752 added a persistent buffer into which to decrypt packets, rather than allocating
a new array for each packet. This was on the back of #1733 which added support in the
cipher types for decrypting into a given buffer, but for the case of AES-CTR, not into
the same buffer in-place. #1787 adds that missing support, meaning we can now decrypt
in-place, and the plaintext buffer becomes unnecessary.
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).
* fix Build with newer .NET 10 SDKs
The IDE0370 are a mess since they only affect certain target frameworks.
Maybe we should disable this one completely instead?
Also set a fixed SDK Version in CI so this doesn't randomly break again.
* Remove IDE0370
* global.json: use latestMinor
to make sure that dotnet-setup installs the
exact version, see https://github.com/sshnet/SSH.NET/pull/1772#discussion_r2941495945
* Update global.json
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Bump the dependencies group
---------
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>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Currently an array is allocated to read each packet from the socket, followed by
decryption which allocates another array for the plaintext payload. We can save one
of these two allocations by adding a persistent buffer for socket receives, and
allowing the cipher implementations to decrypt into the given payload array.
We can save the other allocation similarly, but in a separate change.
Several commonly used Write methods on SshDataStream end up calling Write(ReadOnlySpan)
on the base MemoryStream. But since SshDataStream is a derived type, that method just
rents a buffer and hands it to Write(byte[], int, int), which defeats any stackalloc'ing
or renting that SshDataStream does itself. Instead, with a bit extra accounting we can
just write directly into the underlying buffer.
An SFTP download performs several reads from the server in parallel, allocating an array
to store each result until it's ready to be consumed. Since these buffers are short-lived
and normally of the same large-ish size (32KB), it seems like a good candidate for pooling.
ShellStream does not currently override the Read/Write async variants. They fall back to
the base class implementations which run the sync variants on a thread pool thread, only
allowing one call of either at a time in order to protect implementations that would
break if Read/Write were called simultaneously. In ShellStream, reads and writes are
independent so mutually excluding their use is unnecessary and can lead to effective
deadlocks.
We therefore override WriteAsync to get around this restriction. We do not override
ReadAsync because the sync implementation does not lend itself well to async given the
use of Monitor.Wait/Pulse. Note that while reading and writing simultaneously is allowed,
it is not intended that ShellStream is used with multiple simultaneous reads or multiple
simultaneous writes, so it is fine to keep the base one-at-a-time implementation on
ReadAsync.
Another note is that the new WriteAsync will be simple (synchronous) buffer copying in
most cases, with a call to FlushAsync in others. We also do not override FlushAsync, so
that will go onto a thread pool thread and potentially acquire some locks. But given that
the current base implementation of WriteAsync does that unconditionally, it makes the new
WriteAsync slightly better and certainly no worse than the current version.
This adds an SftpException which sits between the existing SftpPathNotFoundException/
SftpPermissionDeniedException and SshException, and which contains the response code
from the SSH_FXP_STATUS packet, along with a default message if one was not provided.
SftpPathNotFoundException also gains a Path property which is populated in cases where
it makes sense.
* Build the read-ahead mechanism into SftpFileStream
This change unifies the SFTP download implementations that exist via DownloadFile and
via SftpFileStream, by rewriting SftpFileStream to perform the same "read-aheads" as
DownloadFile. This brings the performance of downloads via SftpFileStream in line with
DownloadFile, such that the latter is now effectively SftpFileStream.CopyTo. It also
brings the recently added DownloadFileAsync up to speed since that was implemented via
SftpFileStream.CopyToAsync.
The methodology is a mix of the previous one and that within OpenSSH: the first call to
SftpFileStream.Read sends one read request to the server. The second sends two and when
not interrupted by Write or similar, the number of in-flight read requests continues to
scale up in this fashion.
I have measured CopyTo to be 3-20x faster than before, depending on file size and server
round-trip time.
* Check CanSeek in ReadAllBytes
* Squeeze out some performance
* Avoid rounding issues when checking Timeout values (#1700)
AsTimeout is called from the SshCommand constructor with
Timeout.InfiniteTimeSpan. In this scenario the range check should never
fail, but unfortunately it does in certain scenarios, due to a runtime
or compiler bug (as soon as optimizations are turned off the issue
miraculously disappears).
Closes#1700
* fix tests
---------
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
* CI: add Windows Integration Tests for .NET
see https://github.com/sshnet/SSH.NET/pull/1702#issuecomment-3342506642
* fix podman setup with Windows and .NET
* debug
* x
* x
* x
* revert
* Run Windows .NET tests in separate job
so they run in parallel and we avoid the Common_CreateMoreChannelsThanMaxSessions test failure.
* fix coverlet artifacts
* fix missing PermitTTY in RemoteSshdConfig Reset
this fixes a test failure in Common_CreateMoreChannelsThanMaxSessions
when running the tests multiple times against the same SSH server
instance.
see https://github.com/sshnet/SSH.NET/pull/1704#issuecomment-3343210311
* speed up Windows tests
turns out this is caused by DNS resolution taking about
2 seconds on every new connection...
* add windows integration tests to Publish needs:
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
* Fix SftpFileAttributes file type detection
To get the file type, S_IFMT should be used as the mask. Instead it was using each file
type as the mask. It meant that e.g. a symbolic link would also show as a regular file
and a character device.
Also allow setting and retrieving the setuid/setgid/sticky bits
* fix build
The message loop currently sits in a call to Poll until the socket has data to read or
it is closed. This is unnecessary - it can equally just sit in the call to Receive.
The call to Poll in Session.IsConnected is also unnecessary - we can instead just call
Socket.Connected. This only returns the connection state as of the last operation, but
we are always performing operations in the message loop (or else we are not connected),
so it should work equally well while being cheaper.
Lastly, when shutting down the socket, shut down both sides rather than just the sending
side (SocketShutdown.Both rather than SocketShutdown.Send) - at this point we do not care
about reading anything else. This makes it (more) certain that we will break out of the
Receive call in the message loop, as has been noted in #355 for whatever remaining issues
still exist there.
* Use BCL Curve25519 when possible
* Update KeyExchangeMLKem768X25519Sha256 and KeyExchangeSNtruP761X25519Sha512
* Split Start and Finish methods for inheritance
* Some refactor
* Update src/Renci.SshNet/Security/KeyExchangeEC.BclImpl.cs
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* revert
* Create dedicated KeyExchangeECCurve25519 BclImpl
* cleanup
* minor code refactor
* integration test
* Revert "integration test"
This reverts commit 326962664c.
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Bump actions/checkout from 4 to 5
* Bump actions/upload-pages-artifact from 3 to 4
* Bump actions/download-artifact from 4 to 5
* Bump the dependencies group with 5 updates
* fix
* fix locally failing portforwarding tests
port already in use
---------
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>
When the key exchange produces less key material than is needed for the cipher or hmac
algorithms, there is an iterative procedure to produce more.
For example, a SHA-1 key exchange algorithm produces 20 bytes of key material.
A SHA-256 hmac uses a 32 byte key, so one iteration of the procedure produces another
20 bytes of key material for a total of 40 which is sufficient for the hmac key.
The library works correctly in such cases of one iteration, but the logic is wrong if
more than one iteration is needed. In #1660, the connection uses a SHA-1 kex algorithm
with a SHA-512 hmac (64 byte key), requiring 3 iterations of the extension procedure and
resulting in an error upon connection.
This change fixes the logic to use the output of the previous iteration per the spec.
closes#1660
SftpClient.CreateText does not truncate the file if it exists, contrary to
System.IO.File.CreateText which does. It is documented, but seems pretty
unintuitive and more like a mistake than a concious decision.
I considered leaving this 14 year old behaviour as it is, but turns out other
people have hit it as well (it also affects WriteAll{Bytes/Text/Lines}).
* 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>
dependabot[bot]