From 081d3052b45cb0a2968366eeba8cc25ab7465019 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Thu, 2 Oct 2025 02:18:29 +0800 Subject: [PATCH 1/4] Use BCL Curve25519 for Windows 10+ (#1702) * 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 * revert * Create dedicated KeyExchangeECCurve25519 BclImpl * cleanup * minor code refactor * integration test * Revert "integration test" This reverts commit 326962664c70b148b71f80063a227d9e5430ba3b. --------- Co-authored-by: Rob Hague --- src/Renci.SshNet/Security/KeyExchangeEC.cs | 22 +++++- .../KeyExchangeECCurve25519.BclImpl.cs | 57 +++++++++++++++ ...eyExchangeECCurve25519.BouncyCastleImpl.cs | 38 ++++++++++ .../Security/KeyExchangeECCurve25519.cs | 69 +++++++++++++------ .../Security/KeyExchangeECDH.BclImpl.cs | 6 +- .../KeyExchangeECDH.BouncyCastleImpl.cs | 6 +- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 26 +------ .../KeyExchangeMLKem768X25519Sha256.cs | 37 +++------- .../KeyExchangeSNtruP761X25519Sha512.cs | 40 +++-------- 9 files changed, 195 insertions(+), 106 deletions(-) create mode 100644 src/Renci.SshNet/Security/KeyExchangeECCurve25519.BclImpl.cs create mode 100644 src/Renci.SshNet/Security/KeyExchangeECCurve25519.BouncyCastleImpl.cs diff --git a/src/Renci.SshNet/Security/KeyExchangeEC.cs b/src/Renci.SshNet/Security/KeyExchangeEC.cs index a36bb7bc..f0269a6b 100644 --- a/src/Renci.SshNet/Security/KeyExchangeEC.cs +++ b/src/Renci.SshNet/Security/KeyExchangeEC.cs @@ -1,4 +1,6 @@ -using Renci.SshNet.Messages.Transport; +using System; + +using Renci.SshNet.Messages.Transport; namespace Renci.SshNet.Security { @@ -76,5 +78,23 @@ namespace Renci.SshNet.Security _serverPayload = message.GetBytes(); _clientPayload = Session.ClientInitMessage.GetBytes(); } + + protected abstract class Impl : IDisposable + { + public abstract byte[] GenerateClientPublicKey(); + + public abstract byte[] CalculateAgreement(byte[] serverPublicKey); + + protected virtual void Dispose(bool disposing) + { + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + } } } diff --git a/src/Renci.SshNet/Security/KeyExchangeECCurve25519.BclImpl.cs b/src/Renci.SshNet/Security/KeyExchangeECCurve25519.BclImpl.cs new file mode 100644 index 00000000..de3ab55e --- /dev/null +++ b/src/Renci.SshNet/Security/KeyExchangeECCurve25519.BclImpl.cs @@ -0,0 +1,57 @@ +#if NET +using System.Security.Cryptography; + +namespace Renci.SshNet.Security +{ + internal partial class KeyExchangeECCurve25519 + { + protected sealed class BclImpl : Impl + { + private readonly ECCurve _curve; + private readonly ECDiffieHellman _clientECDH; + + public BclImpl() + { + _curve = ECCurve.CreateFromFriendlyName("Curve25519"); + _clientECDH = ECDiffieHellman.Create(); + } + + public override byte[] GenerateClientPublicKey() + { + _clientECDH.GenerateKey(_curve); + + var q = _clientECDH.PublicKey.ExportParameters().Q; + + return q.X; + } + + public override byte[] CalculateAgreement(byte[] serverPublicKey) + { + var parameters = new ECParameters + { + Curve = _curve, + Q = new ECPoint + { + X = serverPublicKey, + Y = new byte[serverPublicKey.Length] + }, + }; + + using var serverECDH = ECDiffieHellman.Create(parameters); + + return _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); + } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + if (disposing) + { + _clientECDH.Dispose(); + } + } + } + } +} +#endif diff --git a/src/Renci.SshNet/Security/KeyExchangeECCurve25519.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/KeyExchangeECCurve25519.BouncyCastleImpl.cs new file mode 100644 index 00000000..0e58a47c --- /dev/null +++ b/src/Renci.SshNet/Security/KeyExchangeECCurve25519.BouncyCastleImpl.cs @@ -0,0 +1,38 @@ +using Org.BouncyCastle.Crypto.Agreement; +using Org.BouncyCastle.Crypto.Generators; +using Org.BouncyCastle.Crypto.Parameters; + +using Renci.SshNet.Abstractions; + +namespace Renci.SshNet.Security +{ + internal partial class KeyExchangeECCurve25519 + { + protected sealed class BouncyCastleImpl : Impl + { + private X25519Agreement _keyAgreement; + + public override byte[] GenerateClientPublicKey() + { + var g = new X25519KeyPairGenerator(); + g.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom)); + + var aKeyPair = g.GenerateKeyPair(); + _keyAgreement = new X25519Agreement(); + _keyAgreement.Init(aKeyPair.Private); + + return ((X25519PublicKeyParameters)aKeyPair.Public).GetEncoded(); + } + + public override byte[] CalculateAgreement(byte[] serverPublicKey) + { + var publicKey = new X25519PublicKeyParameters(serverPublicKey); + + var k1 = new byte[_keyAgreement.AgreementSize]; + _keyAgreement.CalculateAgreement(publicKey, k1, 0); + + return k1; + } + } + } +} diff --git a/src/Renci.SshNet/Security/KeyExchangeECCurve25519.cs b/src/Renci.SshNet/Security/KeyExchangeECCurve25519.cs index 55827887..cbeccd2a 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECCurve25519.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECCurve25519.cs @@ -1,16 +1,18 @@ -using Org.BouncyCastle.Crypto.Agreement; -using Org.BouncyCastle.Crypto.Generators; -using Org.BouncyCastle.Crypto.Parameters; - -using Renci.SshNet.Abstractions; +using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Messages.Transport; namespace Renci.SshNet.Security { - internal sealed class KeyExchangeECCurve25519 : KeyExchangeEC + internal partial class KeyExchangeECCurve25519 : KeyExchangeEC { - private X25519Agreement _keyAgreement; +#pragma warning disable SA1401 // Fields should be private +#if NET + protected Impl _impl; +#else + protected BouncyCastleImpl _impl; +#endif +#pragma warning restore SA1401 // Fields should be private /// /// Gets algorithm name. @@ -35,29 +37,46 @@ namespace Renci.SshNet.Security public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage) { base.Start(session, message, sendClientInitMessage); +#if NET + if (System.OperatingSystem.IsWindowsVersionAtLeast(10)) + { + _impl = new BclImpl(); + } + else +#endif + { + _impl = new BouncyCastleImpl(); + } + StartImpl(); + } + + /// + /// The implementation of start key exchange algorithm. + /// + protected virtual void StartImpl() + { Session.RegisterMessage("SSH_MSG_KEX_ECDH_REPLY"); Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived; - var g = new X25519KeyPairGenerator(); - g.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom)); - - var aKeyPair = g.GenerateKeyPair(); - _keyAgreement = new X25519Agreement(); - _keyAgreement.Init(aKeyPair.Private); - _clientExchangeValue = ((X25519PublicKeyParameters)aKeyPair.Public).GetEncoded(); + _clientExchangeValue = _impl.GenerateClientPublicKey(); SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } - /// - /// Finishes key exchange algorithm. - /// + /// public override void Finish() { base.Finish(); + FinishImpl(); + } + /// + /// The implementation of finish key exchange algorithm. + /// + protected virtual void FinishImpl() + { Session.KeyExchangeEcdhReplyMessageReceived -= Session_KeyExchangeEcdhReplyMessageReceived; } @@ -98,11 +117,19 @@ namespace Renci.SshNet.Security _hostKey = hostKey; _signature = signature; - var publicKey = new X25519PublicKeyParameters(serverExchangeValue); - - var k1 = new byte[_keyAgreement.AgreementSize]; - _keyAgreement.CalculateAgreement(publicKey, k1, 0); + var k1 = _impl.CalculateAgreement(serverExchangeValue); SharedKey = k1.ToBigInteger2().ToByteArray(isBigEndian: true); } + + /// + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + if (disposing) + { + _impl?.Dispose(); + } + } } } diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs index 0cd1dd34..508f0a2a 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs @@ -17,7 +17,7 @@ namespace Renci.SshNet.Security _clientECDH = ECDiffieHellman.Create(); } - public override byte[] GenerateClientECPoint() + public override byte[] GenerateClientPublicKey() { _clientECDH.GenerateKey(_curve); @@ -26,9 +26,9 @@ namespace Renci.SshNet.Security return EncodeECPoint(q); } - public override byte[] CalculateAgreement(byte[] serverECPoint) + public override byte[] CalculateAgreement(byte[] serverPublicKey) { - var q = DecodeECPoint(serverECPoint); + var q = DecodeECPoint(serverPublicKey); var parameters = new ECParameters { diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs index 4d0208ef..9f38c4ab 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs @@ -20,7 +20,7 @@ namespace Renci.SshNet.Security _keyAgreement = new ECDHCBasicAgreement(); } - public override byte[] GenerateClientECPoint() + public override byte[] GenerateClientPublicKey() { var g = new ECKeyPairGenerator(); g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom)); @@ -31,10 +31,10 @@ namespace Renci.SshNet.Security return ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded(); } - public override byte[] CalculateAgreement(byte[] serverECPoint) + public override byte[] CalculateAgreement(byte[] serverPublicKey) { var c = _domainParameters.Curve; - var q = c.DecodePoint(serverECPoint); + var q = c.DecodePoint(serverPublicKey); var publicKey = new ECPublicKeyParameters("ECDH", q, _domainParameters); return _keyAgreement.CalculateAgreement(publicKey).ToByteArray(); diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index 35ef3b5c..c697ee1e 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,6 +1,4 @@ -using System; - -using Org.BouncyCastle.Asn1.X9; +using Org.BouncyCastle.Asn1.X9; using Renci.SshNet.Common; using Renci.SshNet.Messages.Transport; @@ -41,7 +39,7 @@ namespace Renci.SshNet.Security Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived; #if NET - if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) + if (!System.OperatingSystem.IsWindows() || System.OperatingSystem.IsWindowsVersionAtLeast(10)) { _impl = new BclImpl(Curve); } @@ -51,7 +49,7 @@ namespace Renci.SshNet.Security _impl = new BouncyCastleImpl(CurveParameter); } - _clientExchangeValue = _impl.GenerateClientECPoint(); + _clientExchangeValue = _impl.GenerateClientPublicKey(); SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } @@ -106,23 +104,5 @@ namespace Renci.SshNet.Security _impl?.Dispose(); } } - - private abstract class Impl : IDisposable - { - public abstract byte[] GenerateClientECPoint(); - - public abstract byte[] CalculateAgreement(byte[] serverECPoint); - - protected virtual void Dispose(bool disposing) - { - } - - public void Dispose() - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: true); - GC.SuppressFinalize(this); - } - } } } diff --git a/src/Renci.SshNet/Security/KeyExchangeMLKem768X25519Sha256.cs b/src/Renci.SshNet/Security/KeyExchangeMLKem768X25519Sha256.cs index 469a5373..606e3a25 100644 --- a/src/Renci.SshNet/Security/KeyExchangeMLKem768X25519Sha256.cs +++ b/src/Renci.SshNet/Security/KeyExchangeMLKem768X25519Sha256.cs @@ -1,7 +1,6 @@ using System.Globalization; using System.Linq; -using Org.BouncyCastle.Crypto.Agreement; using Org.BouncyCastle.Crypto.Generators; using Org.BouncyCastle.Crypto.Kems; using Org.BouncyCastle.Crypto.Parameters; @@ -12,10 +11,9 @@ using Renci.SshNet.Messages.Transport; namespace Renci.SshNet.Security { - internal sealed class KeyExchangeMLKem768X25519Sha256 : KeyExchangeEC + internal sealed class KeyExchangeMLKem768X25519Sha256 : KeyExchangeECCurve25519 { private MLKemDecapsulator _mlkemDecapsulator; - private X25519Agreement _x25519Agreement; /// /// Gets algorithm name. @@ -37,10 +35,8 @@ namespace Renci.SshNet.Security } /// - public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage) + protected override void StartImpl() { - base.Start(session, message, sendClientInitMessage); - Session.RegisterMessage("SSH_MSG_KEX_HYBRID_REPLY"); Session.KeyExchangeHybridReplyMessageReceived += Session_KeyExchangeHybridReplyMessageReceived; @@ -52,28 +48,18 @@ namespace Renci.SshNet.Security _mlkemDecapsulator = new MLKemDecapsulator(MLKemParameters.ml_kem_768); _mlkemDecapsulator.Init(mlkem768KeyPair.Private); - var x25519KeyPairGenerator = new X25519KeyPairGenerator(); - x25519KeyPairGenerator.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom)); - var x25519KeyPair = x25519KeyPairGenerator.GenerateKeyPair(); - - _x25519Agreement = new X25519Agreement(); - _x25519Agreement.Init(x25519KeyPair.Private); - var mlkem768PublicKey = ((MLKemPublicKeyParameters)mlkem768KeyPair.Public).GetEncoded(); - var x25519PublicKey = ((X25519PublicKeyParameters)x25519KeyPair.Public).GetEncoded(); + + var x25519PublicKey = _impl.GenerateClientPublicKey(); _clientExchangeValue = mlkem768PublicKey.Concat(x25519PublicKey); SendMessage(new KeyExchangeHybridInitMessage(_clientExchangeValue)); } - /// - /// Finishes key exchange algorithm. - /// - public override void Finish() + /// + protected override void FinishImpl() { - base.Finish(); - Session.KeyExchangeHybridReplyMessageReceived -= Session_KeyExchangeHybridReplyMessageReceived; } @@ -114,21 +100,20 @@ namespace Renci.SshNet.Security _hostKey = hostKey; _signature = signature; - if (serverExchangeValue.Length != _mlkemDecapsulator.EncapsulationLength + _x25519Agreement.AgreementSize) + if (serverExchangeValue.Length != _mlkemDecapsulator.EncapsulationLength + X25519PublicKeyParameters.KeySize) { throw new SshConnectionException( string.Format(CultureInfo.CurrentCulture, "Bad S_Reply length: {0}.", serverExchangeValue.Length), DisconnectReason.KeyExchangeFailed); } - var secret = new byte[_mlkemDecapsulator.SecretLength + _x25519Agreement.AgreementSize]; + var mlkemSecret = new byte[_mlkemDecapsulator.SecretLength]; - _mlkemDecapsulator.Decapsulate(serverExchangeValue, 0, _mlkemDecapsulator.EncapsulationLength, secret, 0, _mlkemDecapsulator.SecretLength); + _mlkemDecapsulator.Decapsulate(serverExchangeValue, 0, _mlkemDecapsulator.EncapsulationLength, mlkemSecret, 0, _mlkemDecapsulator.SecretLength); - var x25519PublicKey = new X25519PublicKeyParameters(serverExchangeValue, _mlkemDecapsulator.EncapsulationLength); - _x25519Agreement.CalculateAgreement(x25519PublicKey, secret, _mlkemDecapsulator.SecretLength); + var x25519Agreement = _impl.CalculateAgreement(serverExchangeValue.Take(_mlkemDecapsulator.EncapsulationLength, X25519PublicKeyParameters.KeySize)); - SharedKey = CryptoAbstraction.HashSHA256(secret); + SharedKey = CryptoAbstraction.HashSHA256(mlkemSecret.Concat(x25519Agreement)); } } } diff --git a/src/Renci.SshNet/Security/KeyExchangeSNtruP761X25519Sha512.cs b/src/Renci.SshNet/Security/KeyExchangeSNtruP761X25519Sha512.cs index 1b327f56..90308226 100644 --- a/src/Renci.SshNet/Security/KeyExchangeSNtruP761X25519Sha512.cs +++ b/src/Renci.SshNet/Security/KeyExchangeSNtruP761X25519Sha512.cs @@ -1,9 +1,6 @@ -using System; -using System.Globalization; +using System.Globalization; using System.Linq; -using Org.BouncyCastle.Crypto.Agreement; -using Org.BouncyCastle.Crypto.Generators; using Org.BouncyCastle.Crypto.Parameters; using Org.BouncyCastle.Pqc.Crypto.NtruPrime; @@ -13,10 +10,9 @@ using Renci.SshNet.Messages.Transport; namespace Renci.SshNet.Security { - internal sealed class KeyExchangeSNtruP761X25519Sha512 : KeyExchangeEC + internal sealed class KeyExchangeSNtruP761X25519Sha512 : KeyExchangeECCurve25519 { private SNtruPrimeKemExtractor _sntrup761Extractor; - private X25519Agreement _x25519Agreement; /// /// Gets algorithm name. @@ -38,10 +34,8 @@ namespace Renci.SshNet.Security } /// - public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage) + protected override void StartImpl() { - base.Start(session, message, sendClientInitMessage); - Session.RegisterMessage("SSH_MSG_KEX_ECDH_REPLY"); Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived; @@ -52,28 +46,18 @@ namespace Renci.SshNet.Security _sntrup761Extractor = new SNtruPrimeKemExtractor((SNtruPrimePrivateKeyParameters)sntrup761KeyPair.Private); - var x25519KeyPairGenerator = new X25519KeyPairGenerator(); - x25519KeyPairGenerator.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom)); - var x25519KeyPair = x25519KeyPairGenerator.GenerateKeyPair(); - - _x25519Agreement = new X25519Agreement(); - _x25519Agreement.Init(x25519KeyPair.Private); - var sntrup761PublicKey = ((SNtruPrimePublicKeyParameters)sntrup761KeyPair.Public).GetEncoded(); - var x25519PublicKey = ((X25519PublicKeyParameters)x25519KeyPair.Public).GetEncoded(); + + var x25519PublicKey = _impl.GenerateClientPublicKey(); _clientExchangeValue = sntrup761PublicKey.Concat(x25519PublicKey); SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } - /// - /// Finishes key exchange algorithm. - /// - public override void Finish() + /// + protected override void FinishImpl() { - base.Finish(); - Session.KeyExchangeEcdhReplyMessageReceived -= Session_KeyExchangeEcdhReplyMessageReceived; } @@ -122,14 +106,12 @@ namespace Renci.SshNet.Security } var sntrup761CipherText = serverExchangeValue.Take(_sntrup761Extractor.EncapsulationLength); - var secret = _sntrup761Extractor.ExtractSecret(sntrup761CipherText); - var sntrup761SecretLength = secret.Length; - var x25519PublicKey = new X25519PublicKeyParameters(serverExchangeValue, _sntrup761Extractor.EncapsulationLength); - Array.Resize(ref secret, sntrup761SecretLength + _x25519Agreement.AgreementSize); - _x25519Agreement.CalculateAgreement(x25519PublicKey, secret, sntrup761SecretLength); + var sntrup761Secret = _sntrup761Extractor.ExtractSecret(sntrup761CipherText); - SharedKey = CryptoAbstraction.HashSHA512(secret); + var x25519Agreement = _impl.CalculateAgreement(serverExchangeValue.Take(_sntrup761Extractor.EncapsulationLength, X25519PublicKeyParameters.KeySize)); + + SharedKey = CryptoAbstraction.HashSHA512(sntrup761Secret.Concat(x25519Agreement)); } } } From c335ce2f20d59bdb1b20bf6ce666e90825bd3c74 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Wed, 1 Oct 2025 20:19:28 +0200 Subject: [PATCH 2/4] Remove calls to Socket.Poll and use SocketShutdown.Both (#1706) 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. --- .../Abstractions/SocketAbstraction.cs | 28 -- src/Renci.SshNet/Common/Extensions.cs | 11 - src/Renci.SshNet/Session.cs | 341 +++++------------- .../Classes/AbstractionsTest.cs | 6 - .../SessionTest_Connected_ConnectionReset.cs | 38 +- ...sionTest_Connected_ServerSendsBadPacket.cs | 15 +- 6 files changed, 120 insertions(+), 319 deletions(-) diff --git a/src/Renci.SshNet/Abstractions/SocketAbstraction.cs b/src/Renci.SshNet/Abstractions/SocketAbstraction.cs index 69ec38b2..63dc2bf5 100644 --- a/src/Renci.SshNet/Abstractions/SocketAbstraction.cs +++ b/src/Renci.SshNet/Abstractions/SocketAbstraction.cs @@ -12,34 +12,6 @@ namespace Renci.SshNet.Abstractions { internal static partial class SocketAbstraction { - public static bool CanRead(Socket socket) - { - if (socket.Connected) - { - return socket.Poll(-1, SelectMode.SelectRead) && socket.Available > 0; - } - - return false; - } - - /// - /// Returns a value indicating whether the specified can be used - /// to send data. - /// - /// The to check. - /// - /// if can be written to; otherwise, . - /// - public static bool CanWrite(Socket socket) - { - if (socket != null && socket.Connected) - { - return socket.Poll(-1, SelectMode.SelectWrite); - } - - return false; - } - public static Socket Connect(IPEndPoint remoteEndpoint, TimeSpan connectTimeout) { var socket = new Socket(remoteEndpoint.AddressFamily, SocketType.Stream, ProtocolType.Tcp) { NoDelay = true }; diff --git a/src/Renci.SshNet/Common/Extensions.cs b/src/Renci.SshNet/Common/Extensions.cs index b7a97d06..6cc65a77 100644 --- a/src/Renci.SshNet/Common/Extensions.cs +++ b/src/Renci.SshNet/Common/Extensions.cs @@ -10,7 +10,6 @@ using System.Numerics; using System.Runtime.CompilerServices; using System.Threading; -using Renci.SshNet.Abstractions; using Renci.SshNet.Messages; namespace Renci.SshNet.Common @@ -319,16 +318,6 @@ namespace Renci.SshNet.Common return concat; } - internal static bool CanRead(this Socket socket) - { - return SocketAbstraction.CanRead(socket); - } - - internal static bool CanWrite(this Socket socket) - { - return SocketAbstraction.CanWrite(socket); - } - internal static bool IsConnected(this Socket socket) { if (socket is null) diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index ec3eac87..77fe9d4c 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -81,12 +81,6 @@ namespace Renci.SshNet private readonly ISocketFactory _socketFactory; private readonly ILogger _logger; - /// - /// Holds an object that is used to ensure only a single thread can read from - /// at any given time. - /// - private readonly Lock _socketReadLock = new Lock(); - /// /// Holds an object that is used to ensure only a single thread can write to /// at any given time. @@ -105,7 +99,7 @@ namespace Renci.SshNet /// This is also used to ensure that will not be disposed /// while performing a given operation or set of operations on . /// - private readonly SemaphoreSlim _socketDisposeLock = new SemaphoreSlim(1, 1); + private readonly Lock _socketDisposeLock = new Lock(); /// /// Holds an object that is used to ensure only a single thread can connect @@ -279,17 +273,11 @@ namespace Renci.SshNet { get { - if (_disposed || _isDisconnectMessageSent || !_isAuthenticated) - { - return false; - } - - if (_messageListenerCompleted is null || _messageListenerCompleted.WaitOne(0)) - { - return false; - } - - return IsSocketConnected(); + return !_disposed && + !_isDisconnectMessageSent && + _isAuthenticated && + _messageListenerCompleted?.WaitOne(0) == false && + _socket.IsConnected(); } } @@ -1046,7 +1034,7 @@ namespace Renci.SshNet /// The size of the packet exceeds the maximum size defined by the protocol. internal void SendMessage(Message message) { - if (!_socket.CanWrite()) + if (!_socket.IsConnected()) { throw new SshConnectionException("Client not connected."); } @@ -1161,9 +1149,7 @@ namespace Renci.SshNet /// private void SendPacket(byte[] packet, int offset, int length) { - _socketDisposeLock.Wait(); - - try + lock (_socketDisposeLock) { if (!_socket.IsConnected()) { @@ -1172,10 +1158,6 @@ namespace Renci.SshNet SocketAbstraction.Send(_socket, packet, offset, length); } - finally - { - _ = _socketDisposeLock.Release(); - } } /// @@ -1259,77 +1241,71 @@ namespace Renci.SshNet byte[] data; uint packetLength; - // avoid reading from socket while IsSocketConnected is attempting to determine whether the - // socket is still connected by invoking Socket.Poll(...) and subsequently verifying value of - // Socket.Available - lock (_socketReadLock) + // Read first block - which starts with the packet length + var firstBlock = new byte[blockSize]; + if (TrySocketRead(socket, firstBlock, 0, blockSize) == 0) { - // Read first block - which starts with the packet length - var firstBlock = new byte[blockSize]; - if (TrySocketRead(socket, firstBlock, 0, blockSize) == 0) + // connection with SSH server was closed + return null; + } + + var plainFirstBlock = firstBlock; + + // First block is not encrypted in AES GCM mode. + if (_serverCipher is not null and not Security.Cryptography.Ciphers.AesGcmCipher) + { + _serverCipher.SetSequenceNumber(_inboundPacketSequence); + + // First block is not encrypted in ETM mode. + if (_serverMac == null || !_serverEtm) + { + plainFirstBlock = _serverCipher.Decrypt(firstBlock); + } + } + + packetLength = BinaryPrimitives.ReadUInt32BigEndian(plainFirstBlock); + + // Test packet minimum and maximum boundaries + if (packetLength < Math.Max((byte)8, blockSize) - 4 || packetLength > MaximumSshPacketSize - 4) + { + throw new SshConnectionException(string.Format(CultureInfo.CurrentCulture, "Bad packet length: {0}.", packetLength), + DisconnectReason.ProtocolError); + } + + // Determine the number of bytes left to read; We've already read "blockSize" bytes, but the + // "packet length" field itself - which is 4 bytes - is not included in the length of the packet + var bytesToRead = (int)(packetLength - (blockSize - packetLengthFieldLength)) + serverMacLength; + + // Construct buffer for holding the payload and the inbound packet sequence as we need both in order + // to generate the hash. + // + // The total length of the "data" buffer is an addition of: + // - inboundPacketSequenceLength (4 bytes) + // - packetLength + // - serverMacLength + // + // We include the inbound packet sequence to allow us to have the the full SSH packet in a single + // byte[] for the purpose of calculating the client hash. Room for the server MAC is foreseen + // to read the packet including server MAC in a single pass (except for the initial block). + data = new byte[bytesToRead + blockSize + inboundPacketSequenceLength]; + BinaryPrimitives.WriteUInt32BigEndian(data, _inboundPacketSequence); + + // Use raw packet length field to calculate the mac in AEAD mode. + if (_serverAead) + { + Buffer.BlockCopy(firstBlock, 0, data, inboundPacketSequenceLength, blockSize); + } + else + { + Buffer.BlockCopy(plainFirstBlock, 0, data, inboundPacketSequenceLength, blockSize); + } + + if (bytesToRead > 0) + { + if (TrySocketRead(socket, data, blockSize + inboundPacketSequenceLength, bytesToRead) == 0) { - // connection with SSH server was closed return null; } - - var plainFirstBlock = firstBlock; - - // First block is not encrypted in AES GCM mode. - if (_serverCipher is not null and not Security.Cryptography.Ciphers.AesGcmCipher) - { - _serverCipher.SetSequenceNumber(_inboundPacketSequence); - - // First block is not encrypted in ETM mode. - if (_serverMac == null || !_serverEtm) - { - plainFirstBlock = _serverCipher.Decrypt(firstBlock); - } - } - - packetLength = BinaryPrimitives.ReadUInt32BigEndian(plainFirstBlock); - - // Test packet minimum and maximum boundaries - if (packetLength < Math.Max((byte)8, blockSize) - 4 || packetLength > MaximumSshPacketSize - 4) - { - throw new SshConnectionException(string.Format(CultureInfo.CurrentCulture, "Bad packet length: {0}.", packetLength), - DisconnectReason.ProtocolError); - } - - // Determine the number of bytes left to read; We've already read "blockSize" bytes, but the - // "packet length" field itself - which is 4 bytes - is not included in the length of the packet - var bytesToRead = (int)(packetLength - (blockSize - packetLengthFieldLength)) + serverMacLength; - - // Construct buffer for holding the payload and the inbound packet sequence as we need both in order - // to generate the hash. - // - // The total length of the "data" buffer is an addition of: - // - inboundPacketSequenceLength (4 bytes) - // - packetLength - // - serverMacLength - // - // We include the inbound packet sequence to allow us to have the the full SSH packet in a single - // byte[] for the purpose of calculating the client hash. Room for the server MAC is foreseen - // to read the packet including server MAC in a single pass (except for the initial block). - data = new byte[bytesToRead + blockSize + inboundPacketSequenceLength]; - BinaryPrimitives.WriteUInt32BigEndian(data, _inboundPacketSequence); - - // Use raw packet length field to calculate the mac in AEAD mode. - if (_serverAead) - { - Buffer.BlockCopy(firstBlock, 0, data, inboundPacketSequenceLength, blockSize); - } - else - { - Buffer.BlockCopy(plainFirstBlock, 0, data, inboundPacketSequenceLength, blockSize); - } - - if (bytesToRead > 0) - { - if (TrySocketRead(socket, data, blockSize + inboundPacketSequenceLength, bytesToRead) == 0) - { - return null; - } - } } // validate encrypted message against MAC @@ -1888,84 +1864,6 @@ namespace Renci.SshNet #endif } - /// - /// Gets a value indicating whether the socket is connected. - /// - /// - /// if the socket is connected; otherwise, . - /// - /// - /// - /// As a first check we verify whether is - /// . However, this only returns the state of the socket as of - /// the last I/O operation. - /// - /// - /// Therefore we use the combination of with mode - /// and to verify if the socket is still connected. - /// - /// - /// The MSDN doc mention the following on the return value of - /// with mode : - /// - /// - /// if data is available for reading; - /// - /// - /// if the connection has been closed, reset, or terminated; otherwise, returns . - /// - /// - /// - /// - /// Conclusion: when the return value is - but no data is available for reading - then - /// the socket is no longer connected. - /// - /// - /// When a is used from multiple threads, there's a race condition - /// between the invocation of and the moment - /// when the value of is obtained. To workaround this issue - /// we synchronize reads from the . - /// - /// - /// We assume the socket is still connected if the read lock cannot be acquired immediately. - /// In this case, we just return without actually waiting to acquire - /// the lock. We don't want to wait for the read lock if another thread already has it because - /// there are cases where the other thread holding the lock can be waiting indefinitely for - /// a socket read operation to complete. - /// - /// - private bool IsSocketConnected() - { - _socketDisposeLock.Wait(); - - try - { - if (!_socket.IsConnected()) - { - return false; - } - - if (!_socketReadLock.TryEnter()) - { - return true; - } - - try - { - var connectionClosedOrDataAvailable = _socket.Poll(0, SelectMode.SelectRead); - return !(connectionClosedOrDataAvailable && _socket.Available == 0); - } - finally - { - _socketReadLock.Exit(); - } - } - finally - { - _ = _socketDisposeLock.Release(); - } - } - /// /// Performs a blocking read on the socket until bytes are received. /// @@ -1988,46 +1886,37 @@ namespace Renci.SshNet /// private void SocketDisconnectAndDispose() { - if (_socket != null) + lock (_socketDisposeLock) { - _socketDisposeLock.Wait(); - - try + if (_socket is null) { -#pragma warning disable CA1508 // Avoid dead conditional code; Value could have been changed by another thread. - if (_socket != null) -#pragma warning restore CA1508 // Avoid dead conditional code + return; + } + + if (_socket.Connected) + { + try { - if (_socket.Connected) - { - try - { - _logger.LogDebug("[{SessionId}] Shutting down socket.", SessionIdHex); + _logger.LogDebug("[{SessionId}] Shutting down socket.", SessionIdHex); - // Interrupt any pending reads; should be done outside of socket read lock as we - // actually want shutdown the socket to make sure blocking reads are interrupted. - // - // This may result in a SocketException (eg. An existing connection was forcibly - // closed by the remote host) which we'll log and ignore as it means the socket - // was already shut down. - _socket.Shutdown(SocketShutdown.Send); - } - catch (SocketException ex) - { - _logger.LogInformation(ex, "Failure shutting down socket"); - } - } - - _logger.LogDebug("[{SessionId}] Disposing socket.", SessionIdHex); - _socket.Dispose(); - _logger.LogDebug("[{SessionId}] Disposed socket.", SessionIdHex); - _socket = null; + // Interrupt any pending reads; should be done outside of socket read lock as we + // actually want shutdown the socket to make sure blocking reads are interrupted. + // + // This may result in a SocketException (eg. An existing connection was forcibly + // closed by the remote host) which we'll log and ignore as it means the socket + // was already shut down. + _socket.Shutdown(SocketShutdown.Both); + } + catch (SocketException ex) + { + _logger.LogInformation(ex, "Failure shutting down socket"); } } - finally - { - _ = _socketDisposeLock.Release(); - } + + _logger.LogDebug("[{SessionId}] Disposing socket.", SessionIdHex); + _socket.Dispose(); + _logger.LogDebug("[{SessionId}] Disposed socket.", SessionIdHex); + _socket = null; } } @@ -2048,25 +1937,6 @@ namespace Renci.SshNet break; } - try - { - // Block until either data is available or the socket is closed - var connectionClosedOrDataAvailable = socket.Poll(-1, SelectMode.SelectRead); - if (connectionClosedOrDataAvailable && socket.Available == 0) - { - // connection with SSH server was closed or connection was reset - break; - } - } - catch (ObjectDisposedException) - { - // The socket was disposed by either: - // * a call to Disconnect() - // * a call to Dispose() - // * a SSH_MSG_DISCONNECT received from server - break; - } - var message = ReceiveMessage(socket); if (message is null) { @@ -2102,25 +1972,12 @@ namespace Renci.SshNet /// The . private void RaiseError(Exception exp) { - var connectionException = exp as SshConnectionException; - _logger.LogInformation(exp, "[{SessionId}] Raised exception", SessionIdHex); - if (_isDisconnecting) + if (_isDisconnecting && exp is SshConnectionException or ObjectDisposedException) { - // a connection exception which is raised while isDisconnecting is normal and - // should be ignored - if (connectionException != null) - { - return; - } - - // any timeout while disconnecting can be caused by loss of connectivity - // altogether and should be ignored - if (exp is SocketException socketException && socketException.SocketErrorCode == SocketError.TimedOut) - { - return; - } + // Such an exception raised while isDisconnecting is expected and can be ignored. + return; } // "save" exception and set exception wait handle to ensure any waits are interrupted @@ -2129,10 +1986,10 @@ namespace Renci.SshNet ErrorOccured?.Invoke(this, new ExceptionEventArgs(exp)); - if (connectionException != null) + if (exp is SshConnectionException connectionException) { _logger.LogInformation(exp, "[{SessionId}] Disconnecting after exception", SessionIdHex); - Disconnect(connectionException.DisconnectReason, exp.ToString()); + Disconnect(connectionException.DisconnectReason, exp.Message); } } diff --git a/test/Renci.SshNet.Tests/Classes/AbstractionsTest.cs b/test/Renci.SshNet.Tests/Classes/AbstractionsTest.cs index e79eb856..c37b3dcf 100644 --- a/test/Renci.SshNet.Tests/Classes/AbstractionsTest.cs +++ b/test/Renci.SshNet.Tests/Classes/AbstractionsTest.cs @@ -8,12 +8,6 @@ namespace Renci.SshNet.Tests.Classes [TestClass] public class AbstractionsTest { - [TestMethod] - public void SocketAbstraction_CanWrite_ShouldReturnFalseWhenSocketIsNull() - { - Assert.IsFalse(SocketAbstraction.CanWrite(null)); - } - [TestMethod] public void CryptoAbstraction_GenerateRandom_ShouldPerformNoOpWhenDataIsZeroLength() { diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ConnectionReset.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ConnectionReset.cs index 1b11e702..a09999b3 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ConnectionReset.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ConnectionReset.cs @@ -64,8 +64,6 @@ namespace Renci.SshNet.Tests.Classes var connectionException = (SshConnectionException)exception; Assert.AreEqual(DisconnectReason.ConnectionLost, connectionException.DisconnectReason); - Assert.IsNull(connectionException.InnerException); - Assert.AreEqual("An established connection was aborted by the server.", connectionException.Message); } [TestMethod] @@ -137,45 +135,29 @@ namespace Renci.SshNet.Tests.Classes public void ISession_WaitOnHandle_WaitHandle_ShouldThrowSshConnectionException() { var session = (ISession)Session; - var waitHandle = new ManualResetEvent(false); + using var waitHandle = new ManualResetEvent(false); - try - { - session.WaitOnHandle(waitHandle); - Assert.Fail(); - } - catch (SshConnectionException ex) - { - Assert.AreEqual("An established connection was aborted by the server.", ex.Message); - Assert.IsNull(ex.InnerException); - Assert.AreEqual(DisconnectReason.ConnectionLost, ex.DisconnectReason); - } + var ex = Assert.ThrowsExactly(() => session.WaitOnHandle(waitHandle)); + + Assert.AreEqual(DisconnectReason.ConnectionLost, ex.DisconnectReason); } [TestMethod] public void ISession_WaitOnHandle_WaitHandleAndTimeout_ShouldThrowSshConnectionException() { var session = (ISession)Session; - var waitHandle = new ManualResetEvent(false); + using var waitHandle = new ManualResetEvent(false); - try - { - session.WaitOnHandle(waitHandle, Timeout.InfiniteTimeSpan); - Assert.Fail(); - } - catch (SshConnectionException ex) - { - Assert.AreEqual(DisconnectReason.ConnectionLost, ex.DisconnectReason); - Assert.IsNull(ex.InnerException); - Assert.AreEqual("An established connection was aborted by the server.", ex.Message); - } + var ex = Assert.ThrowsExactly(() => session.WaitOnHandle(waitHandle)); + + Assert.AreEqual(DisconnectReason.ConnectionLost, ex.DisconnectReason); } [TestMethod] public void ISession_TryWait_WaitHandleAndTimeout_ShouldReturnDisconnected() { var session = (ISession)Session; - var waitHandle = new ManualResetEvent(false); + using var waitHandle = new ManualResetEvent(false); var result = session.TryWait(waitHandle, Timeout.InfiniteTimeSpan); @@ -186,7 +168,7 @@ namespace Renci.SshNet.Tests.Classes public void ISession_TryWait_WaitHandleAndTimeoutAndException_ShouldReturnDisconnected() { var session = (ISession)Session; - var waitHandle = new ManualResetEvent(false); + using var waitHandle = new ManualResetEvent(false); var result = session.TryWait(waitHandle, Timeout.InfiniteTimeSpan, out var exception); diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerSendsBadPacket.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerSendsBadPacket.cs index 5dd53a29..61f1b494 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerSendsBadPacket.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerSendsBadPacket.cs @@ -91,13 +91,20 @@ namespace Renci.SshNet.Tests.Classes } [TestMethod] - public void ReceiveOnServerSocketShouldReturnZero() + public void ServerShouldBeDisconnected() { - var buffer = new byte[1]; + try + { + var buffer = new byte[1]; - var actual = ServerSocket.Receive(buffer, 0, buffer.Length, SocketFlags.None); + var actual = ServerSocket.Receive(buffer, 0, buffer.Length, SocketFlags.None); - Assert.AreEqual(0, actual); + Assert.AreEqual(0, actual); // FIN + } + catch (SocketException sx) + { + Assert.AreEqual(SocketError.ConnectionReset, sx.SocketErrorCode); // RST + } } [TestMethod] From af279d206a1c8a60bbd8eb1b91b7a1da719043f8 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Wed, 1 Oct 2025 22:19:42 +0200 Subject: [PATCH 3/4] Fix SftpFileAttributes file type detection (#1688) * 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 --- .../Sftp/Requests/SftpMkDirRequest.cs | 25 +- .../Sftp/Requests/SftpOpenRequest.cs | 17 +- .../Sftp/Responses/SftpVersionResponse.cs | 8 +- src/Renci.SshNet/Sftp/SftpFileAttributes.cs | 628 ++++++++++++------ src/Renci.SshNet/Sftp/SftpSession.cs | 2 +- .../Sftp/Requests/SftpFSetStatRequestTest.cs | 3 +- .../Sftp/Requests/SftpMkDirRequestTest.cs | 3 +- .../Sftp/Requests/SftpOpenRequestTest.cs | 3 +- .../Sftp/Requests/SftpSetStatRequestTest.cs | 3 +- .../Sftp/Responses/SftpAttrsResponseTest.cs | 3 +- .../Classes/Sftp/SftpFileAttributesTest.cs | 223 +++++++ .../Classes/Sftp/SftpFileReaderTestBase.cs | 6 - .../SftpSessionTest_Connected_RequestRead.cs | 3 +- ...ftpSessionTest_Connected_RequestStatVfs.cs | 3 +- ...tipleSftpMessagesInSingleSshDataMessage.cs | 3 +- ...essagesSplitOverMultipleSshDataMessages.cs | 3 +- ...eived_SingleSftpMessageInSshDataMessage.cs | 3 +- .../Sftp/SftpVersionResponseBuilder.cs | 2 +- ...tyAndWriteMoreBytesThanBufferCanContain.cs | 5 +- .../Common/ArrayBuilder`1.cs | 34 - test/Renci.SshNet.Tests/Common/Extensions.cs | 43 +- .../Common/SftpFileAttributesBuilder.cs | 19 +- 22 files changed, 678 insertions(+), 364 deletions(-) create mode 100644 test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs delete mode 100644 test/Renci.SshNet.Tests/Common/ArrayBuilder`1.cs diff --git a/src/Renci.SshNet/Sftp/Requests/SftpMkDirRequest.cs b/src/Renci.SshNet/Sftp/Requests/SftpMkDirRequest.cs index fafd481f..a04a896c 100644 --- a/src/Renci.SshNet/Sftp/Requests/SftpMkDirRequest.cs +++ b/src/Renci.SshNet/Sftp/Requests/SftpMkDirRequest.cs @@ -8,7 +8,6 @@ namespace Renci.SshNet.Sftp.Requests internal sealed class SftpMkDirRequest : SftpRequest { private byte[] _path; - private byte[] _attributesBytes; public override SftpMessageTypes SftpMessageType { @@ -23,18 +22,6 @@ namespace Renci.SshNet.Sftp.Requests public Encoding Encoding { get; private set; } - private SftpFileAttributes Attributes { get; set; } - - private byte[] AttributesBytes - { - get - { - _attributesBytes ??= Attributes.GetBytes(); - - return _attributesBytes; - } - } - /// /// Gets the size of the message in bytes. /// @@ -48,36 +35,30 @@ namespace Renci.SshNet.Sftp.Requests var capacity = base.BufferCapacity; capacity += 4; // Path length capacity += _path.Length; // Path - capacity += AttributesBytes.Length; // Attributes + capacity += 4; // Attributes return capacity; } } public SftpMkDirRequest(uint protocolVersion, uint requestId, string path, Encoding encoding, Action statusAction) - : this(protocolVersion, requestId, path, encoding, SftpFileAttributes.Empty, statusAction) - { - } - - private SftpMkDirRequest(uint protocolVersion, uint requestId, string path, Encoding encoding, SftpFileAttributes attributes, Action statusAction) : base(protocolVersion, requestId, statusAction) { Encoding = encoding; Path = path; - Attributes = attributes; } protected override void LoadData() { base.LoadData(); _path = ReadBinary(); - Attributes = ReadAttributes(); + _ = ReadAttributes(); } protected override void SaveData() { base.SaveData(); WriteBinaryString(_path); - Write(AttributesBytes); + Write(0u); // empty attributes } } } diff --git a/src/Renci.SshNet/Sftp/Requests/SftpOpenRequest.cs b/src/Renci.SshNet/Sftp/Requests/SftpOpenRequest.cs index 6a86b4da..45065f68 100644 --- a/src/Renci.SshNet/Sftp/Requests/SftpOpenRequest.cs +++ b/src/Renci.SshNet/Sftp/Requests/SftpOpenRequest.cs @@ -9,7 +9,6 @@ namespace Renci.SshNet.Sftp.Requests { private readonly Action _handleAction; private byte[] _fileName; - private byte[] _attributes; public override SftpMessageTypes SftpMessageType { @@ -24,12 +23,6 @@ namespace Renci.SshNet.Sftp.Requests public Flags Flags { get; } - public SftpFileAttributes Attributes - { - get { return SftpFileAttributes.FromBytes(_attributes); } - private set { _attributes = value.GetBytes(); } - } - public Encoding Encoding { get; } /// @@ -46,23 +39,17 @@ namespace Renci.SshNet.Sftp.Requests capacity += 4; // FileName length capacity += _fileName.Length; // FileName capacity += 4; // Flags - capacity += _attributes.Length; // Attributes + capacity += 4; // Attributes return capacity; } } public SftpOpenRequest(uint protocolVersion, uint requestId, string fileName, Encoding encoding, Flags flags, Action handleAction, Action statusAction) - : this(protocolVersion, requestId, fileName, encoding, flags, SftpFileAttributes.Empty, handleAction, statusAction) - { - } - - private SftpOpenRequest(uint protocolVersion, uint requestId, string fileName, Encoding encoding, Flags flags, SftpFileAttributes attributes, Action handleAction, Action statusAction) : base(protocolVersion, requestId, statusAction) { Encoding = encoding; Filename = fileName; Flags = flags; - Attributes = attributes; _handleAction = handleAction; } @@ -79,7 +66,7 @@ namespace Renci.SshNet.Sftp.Requests WriteBinaryString(_fileName); Write((uint)Flags); - Write(_attributes); + Write(0u); // empty attributes } public override void Complete(SftpResponse response) diff --git a/src/Renci.SshNet/Sftp/Responses/SftpVersionResponse.cs b/src/Renci.SshNet/Sftp/Responses/SftpVersionResponse.cs index 1ae8e6d6..8c2ff2c4 100644 --- a/src/Renci.SshNet/Sftp/Responses/SftpVersionResponse.cs +++ b/src/Renci.SshNet/Sftp/Responses/SftpVersionResponse.cs @@ -11,14 +11,14 @@ namespace Renci.SshNet.Sftp.Responses public uint Version { get; set; } - public IDictionary Extentions { get; set; } + public IDictionary Extensions { get; set; } protected override void LoadData() { base.LoadData(); Version = ReadUInt32(); - Extentions = ReadExtensionPair(); + Extensions = ReadExtensionPair(); } protected override void SaveData() @@ -27,9 +27,9 @@ namespace Renci.SshNet.Sftp.Responses Write(Version); - if (Extentions != null) + if (Extensions != null) { - Write(Extentions); + Write(Extensions); } } } diff --git a/src/Renci.SshNet/Sftp/SftpFileAttributes.cs b/src/Renci.SshNet/Sftp/SftpFileAttributes.cs index 503e1470..70ca0ddc 100644 --- a/src/Renci.SshNet/Sftp/SftpFileAttributes.cs +++ b/src/Renci.SshNet/Sftp/SftpFileAttributes.cs @@ -1,7 +1,10 @@ -using System; +#nullable enable +using System; using System.Collections.Generic; -using System.Globalization; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Text; using Renci.SshNet.Common; @@ -23,8 +26,8 @@ namespace Renci.SshNet.Sftp private const uint S_IFCHR = 0x2000; // character device private const uint S_IFIFO = 0x1000; // FIFO private const uint S_ISUID = 0x0800; // set UID bit - private const uint S_ISGID = 0x0400; // set-group-ID bit (see below) - private const uint S_ISVTX = 0x0200; // sticky bit (see below) + private const uint S_ISGID = 0x0400; // set-group-ID bit + private const uint S_ISVTX = 0x0200; // sticky bit private const uint S_IRUSR = 0x0100; // owner has read permission private const uint S_IWUSR = 0x0080; // owner has write permission private const uint S_IXUSR = 0x0040; // owner has execute permission @@ -43,12 +46,7 @@ namespace Renci.SshNet.Sftp private readonly int _originalUserId; private readonly int _originalGroupId; private readonly uint _originalPermissions; - private readonly IDictionary _originalExtensions; - - private bool _isBitFiledsBitSet; - private bool _isUIDBitSet; - private bool _isGroupIDBitSet; - private bool _isStickyBitSet; + private readonly Dictionary? _originalExtensions; internal bool IsLastAccessTimeChanged { @@ -82,6 +80,7 @@ namespace Renci.SshNet.Sftp internal bool IsExtensionsChanged { + [MemberNotNullWhen(true, nameof(Extensions))] get { return _originalExtensions != null && Extensions != null && !_originalExtensions.SequenceEqual(Extensions); } } @@ -169,7 +168,13 @@ namespace Renci.SshNet.Sftp /// /// if file represents a socket; otherwise, . /// - public bool IsSocket { get; private set; } + public bool IsSocket + { + get + { + return (Permissions & S_IFMT) == S_IFSOCK; + } + } /// /// Gets a value indicating whether file represents a symbolic link. @@ -177,7 +182,13 @@ namespace Renci.SshNet.Sftp /// /// if file represents a symbolic link; otherwise, . /// - public bool IsSymbolicLink { get; private set; } + public bool IsSymbolicLink + { + get + { + return (Permissions & S_IFMT) == S_IFLNK; + } + } /// /// Gets a value indicating whether file represents a regular file. @@ -185,7 +196,13 @@ namespace Renci.SshNet.Sftp /// /// if file represents a regular file; otherwise, . /// - public bool IsRegularFile { get; private set; } + public bool IsRegularFile + { + get + { + return (Permissions & S_IFMT) == S_IFREG; + } + } /// /// Gets a value indicating whether file represents a block device. @@ -193,7 +210,13 @@ namespace Renci.SshNet.Sftp /// /// if file represents a block device; otherwise, . /// - public bool IsBlockDevice { get; private set; } + public bool IsBlockDevice + { + get + { + return (Permissions & S_IFMT) == S_IFBLK; + } + } /// /// Gets a value indicating whether file represents a directory. @@ -201,7 +224,13 @@ namespace Renci.SshNet.Sftp /// /// if file represents a directory; otherwise, . /// - public bool IsDirectory { get; private set; } + public bool IsDirectory + { + get + { + return (Permissions & S_IFMT) == S_IFDIR; + } + } /// /// Gets a value indicating whether file represents a character device. @@ -209,7 +238,13 @@ namespace Renci.SshNet.Sftp /// /// if file represents a character device; otherwise, . /// - public bool IsCharacterDevice { get; private set; } + public bool IsCharacterDevice + { + get + { + return (Permissions & S_IFMT) == S_IFCHR; + } + } /// /// Gets a value indicating whether file represents a named pipe. @@ -217,7 +252,88 @@ namespace Renci.SshNet.Sftp /// /// if file represents a named pipe; otherwise, . /// - public bool IsNamedPipe { get; private set; } + public bool IsNamedPipe + { + get + { + return (Permissions & S_IFMT) == S_IFIFO; + } + } + + /// + /// Gets or sets a value indicating whether the setuid bit is set. + /// + /// + /// if the setuid bit is set; otherwise, . + /// + public bool IsUIDBitSet + { + get + { + return (Permissions & S_ISUID) == S_ISUID; + } + set + { + if (value) + { + Permissions |= S_ISUID; + } + else + { + Permissions &= ~S_ISUID; + } + } + } + + /// + /// Gets or sets a value indicating whether the setgid bit is set. + /// + /// + /// if the setgid bit is set; otherwise, . + /// + public bool IsGroupIDBitSet + { + get + { + return (Permissions & S_ISGID) == S_ISGID; + } + set + { + if (value) + { + Permissions |= S_ISGID; + } + else + { + Permissions &= ~S_ISGID; + } + } + } + + /// + /// Gets or sets a value indicating whether the sticky bit is set. + /// + /// + /// if the sticky bit is set; otherwise, . + /// + public bool IsStickyBitSet + { + get + { + return (Permissions & S_ISVTX) == S_ISVTX; + } + set + { + if (value) + { + Permissions |= S_ISVTX; + } + else + { + Permissions &= ~S_ISVTX; + } + } + } /// /// Gets or sets a value indicating whether the owner can read from this file. @@ -225,7 +341,24 @@ namespace Renci.SshNet.Sftp /// /// if owner can read from this file; otherwise, . /// - public bool OwnerCanRead { get; set; } + public bool OwnerCanRead + { + get + { + return (Permissions & S_IRUSR) == S_IRUSR; + } + set + { + if (value) + { + Permissions |= S_IRUSR; + } + else + { + Permissions &= ~S_IRUSR; + } + } + } /// /// Gets or sets a value indicating whether the owner can write into this file. @@ -233,7 +366,24 @@ namespace Renci.SshNet.Sftp /// /// if owner can write into this file; otherwise, . /// - public bool OwnerCanWrite { get; set; } + public bool OwnerCanWrite + { + get + { + return (Permissions & S_IWUSR) == S_IWUSR; + } + set + { + if (value) + { + Permissions |= S_IWUSR; + } + else + { + Permissions &= ~S_IWUSR; + } + } + } /// /// Gets or sets a value indicating whether the owner can execute this file. @@ -241,7 +391,24 @@ namespace Renci.SshNet.Sftp /// /// if owner can execute this file; otherwise, . /// - public bool OwnerCanExecute { get; set; } + public bool OwnerCanExecute + { + get + { + return (Permissions & S_IXUSR) == S_IXUSR; + } + set + { + if (value) + { + Permissions |= S_IXUSR; + } + else + { + Permissions &= ~S_IXUSR; + } + } + } /// /// Gets or sets a value indicating whether the group members can read from this file. @@ -249,7 +416,24 @@ namespace Renci.SshNet.Sftp /// /// if group members can read from this file; otherwise, . /// - public bool GroupCanRead { get; set; } + public bool GroupCanRead + { + get + { + return (Permissions & S_IRGRP) == S_IRGRP; + } + set + { + if (value) + { + Permissions |= S_IRGRP; + } + else + { + Permissions &= ~S_IRGRP; + } + } + } /// /// Gets or sets a value indicating whether the group members can write into this file. @@ -257,7 +441,24 @@ namespace Renci.SshNet.Sftp /// /// if group members can write into this file; otherwise, . /// - public bool GroupCanWrite { get; set; } + public bool GroupCanWrite + { + get + { + return (Permissions & S_IWGRP) == S_IWGRP; + } + set + { + if (value) + { + Permissions |= S_IWGRP; + } + else + { + Permissions &= ~S_IWGRP; + } + } + } /// /// Gets or sets a value indicating whether the group members can execute this file. @@ -265,7 +466,24 @@ namespace Renci.SshNet.Sftp /// /// if group members can execute this file; otherwise, . /// - public bool GroupCanExecute { get; set; } + public bool GroupCanExecute + { + get + { + return (Permissions & S_IXGRP) == S_IXGRP; + } + set + { + if (value) + { + Permissions |= S_IXGRP; + } + else + { + Permissions &= ~S_IXGRP; + } + } + } /// /// Gets or sets a value indicating whether the others can read from this file. @@ -273,7 +491,24 @@ namespace Renci.SshNet.Sftp /// /// if others can read from this file; otherwise, . /// - public bool OthersCanRead { get; set; } + public bool OthersCanRead + { + get + { + return (Permissions & S_IROTH) == S_IROTH; + } + set + { + if (value) + { + Permissions |= S_IROTH; + } + else + { + Permissions &= ~S_IROTH; + } + } + } /// /// Gets or sets a value indicating whether the others can write into this file. @@ -281,7 +516,24 @@ namespace Renci.SshNet.Sftp /// /// if others can write into this file; otherwise, . /// - public bool OthersCanWrite { get; set; } + public bool OthersCanWrite + { + get + { + return (Permissions & S_IWOTH) == S_IWOTH; + } + set + { + if (value) + { + Permissions |= S_IWOTH; + } + else + { + Permissions &= ~S_IWOTH; + } + } + } /// /// Gets or sets a value indicating whether the others can execute this file. @@ -289,7 +541,24 @@ namespace Renci.SshNet.Sftp /// /// if others can execute this file; otherwise, . /// - public bool OthersCanExecute { get; set; } + public bool OthersCanExecute + { + get + { + return (Permissions & S_IXOTH) == S_IXOTH; + } + set + { + if (value) + { + Permissions |= S_IXOTH; + } + else + { + Permissions &= ~S_IXOTH; + } + } + } /// /// Gets the extensions. @@ -297,165 +566,11 @@ namespace Renci.SshNet.Sftp /// /// The extensions. /// - public IDictionary Extensions { get; private set; } + public IDictionary? Extensions { get; } - internal uint Permissions - { - get - { - uint permission = 0; + internal uint Permissions { get; private set; } - if (_isBitFiledsBitSet) - { - permission |= S_IFMT; - } - - if (IsSocket) - { - permission |= S_IFSOCK; - } - - if (IsSymbolicLink) - { - permission |= S_IFLNK; - } - - if (IsRegularFile) - { - permission |= S_IFREG; - } - - if (IsBlockDevice) - { - permission |= S_IFBLK; - } - - if (IsDirectory) - { - permission |= S_IFDIR; - } - - if (IsCharacterDevice) - { - permission |= S_IFCHR; - } - - if (IsNamedPipe) - { - permission |= S_IFIFO; - } - - if (_isUIDBitSet) - { - permission |= S_ISUID; - } - - if (_isGroupIDBitSet) - { - permission |= S_ISGID; - } - - if (_isStickyBitSet) - { - permission |= S_ISVTX; - } - - if (OwnerCanRead) - { - permission |= S_IRUSR; - } - - if (OwnerCanWrite) - { - permission |= S_IWUSR; - } - - if (OwnerCanExecute) - { - permission |= S_IXUSR; - } - - if (GroupCanRead) - { - permission |= S_IRGRP; - } - - if (GroupCanWrite) - { - permission |= S_IWGRP; - } - - if (GroupCanExecute) - { - permission |= S_IXGRP; - } - - if (OthersCanRead) - { - permission |= S_IROTH; - } - - if (OthersCanWrite) - { - permission |= S_IWOTH; - } - - if (OthersCanExecute) - { - permission |= S_IXOTH; - } - - return permission; - } - private set - { - _isBitFiledsBitSet = (value & S_IFMT) == S_IFMT; - - IsSocket = (value & S_IFSOCK) == S_IFSOCK; - - IsSymbolicLink = (value & S_IFLNK) == S_IFLNK; - - IsRegularFile = (value & S_IFREG) == S_IFREG; - - IsBlockDevice = (value & S_IFBLK) == S_IFBLK; - - IsDirectory = (value & S_IFDIR) == S_IFDIR; - - IsCharacterDevice = (value & S_IFCHR) == S_IFCHR; - - IsNamedPipe = (value & S_IFIFO) == S_IFIFO; - - _isUIDBitSet = (value & S_ISUID) == S_ISUID; - - _isGroupIDBitSet = (value & S_ISGID) == S_ISGID; - - _isStickyBitSet = (value & S_ISVTX) == S_ISVTX; - - OwnerCanRead = (value & S_IRUSR) == S_IRUSR; - - OwnerCanWrite = (value & S_IWUSR) == S_IWUSR; - - OwnerCanExecute = (value & S_IXUSR) == S_IXUSR; - - GroupCanRead = (value & S_IRGRP) == S_IRGRP; - - GroupCanWrite = (value & S_IWGRP) == S_IWGRP; - - GroupCanExecute = (value & S_IXGRP) == S_IXGRP; - - OthersCanRead = (value & S_IROTH) == S_IROTH; - - OthersCanWrite = (value & S_IWOTH) == S_IWOTH; - - OthersCanExecute = (value & S_IXOTH) == S_IXOTH; - } - } - - private SftpFileAttributes() - { - } - - internal SftpFileAttributes(DateTime lastAccessTimeUtc, DateTime lastWriteTimeUtc, long size, int userId, int groupId, uint permissions, IDictionary extensions) + internal SftpFileAttributes(DateTime lastAccessTimeUtc, DateTime lastWriteTimeUtc, long size, int userId, int groupId, uint permissions, Dictionary? extensions) { LastAccessTimeUtc = _originalLastAccessTimeUtc = lastAccessTimeUtc; LastWriteTimeUtc = _originalLastWriteTimeUtc = lastWriteTimeUtc; @@ -467,31 +582,118 @@ namespace Renci.SshNet.Sftp } /// - /// Sets the permissions. + /// Sets the POSIX permissions for this file. /// - /// The mode. + /// + /// The permission mode as an octal number (e.g., 755, 644, 1777). + /// + /// + /// has more than 4 digits or cannot be interpreted as an octal number. + /// public void SetPermissions(short mode) { - if (mode is < 0 or > 999) + var special = (uint)Math.DivRem(mode, 1000, out var userGroupOther); + + var user = (uint)Math.DivRem(userGroupOther, 100, out var groupOther); + + var group = (uint)Math.DivRem(groupOther, 10, out var iOther); + + var other = (uint)iOther; + + if ((special & ~7u) != 0 || (user & ~7u) != 0 || (group & ~7u) != 0 || (other & ~7u) != 0) { throw new ArgumentOutOfRangeException(nameof(mode)); } - var modeBytes = mode.ToString(CultureInfo.InvariantCulture).PadLeft(3, '0').ToCharArray(); + Permissions = (Permissions & ~0xFFFu) | (special << 9) | (user << 6) | (group << 3) | other; + } - var permission = ((modeBytes[0] & 0x0F) * 8 * 8) + ((modeBytes[1] & 0x0F) * 8) + (modeBytes[2] & 0x0F); + /// + public override string? ToString() + { + var sb = new StringBuilder(); - OwnerCanRead = (permission & S_IRUSR) == S_IRUSR; - OwnerCanWrite = (permission & S_IWUSR) == S_IWUSR; - OwnerCanExecute = (permission & S_IXUSR) == S_IXUSR; + if (Permissions != default) + { + AppendPermissionsString(sb); + sb.Append(' '); + } - GroupCanRead = (permission & S_IRGRP) == S_IRGRP; - GroupCanWrite = (permission & S_IWGRP) == S_IWGRP; - GroupCanExecute = (permission & S_IXGRP) == S_IXGRP; + if (Size != -1) + { + sb.AppendFormat("Size: {0} ", Size); + } - OthersCanRead = (permission & S_IROTH) == S_IROTH; - OthersCanWrite = (permission & S_IWOTH) == S_IWOTH; - OthersCanExecute = (permission & S_IXOTH) == S_IXOTH; + if (LastWriteTime != default) + { + sb.AppendFormat("LastWriteTime: {0:s} ", LastWriteTime); + } + + if (sb.Length > 0) + { + if (sb[sb.Length - 1] == ' ') + { + sb.Length--; + } + + Debug.Assert(sb.Length > 0); + Debug.Assert(sb[^1] != ' '); + + return sb.ToString(); + } + + return base.ToString(); + } + + private void AppendPermissionsString(StringBuilder sb) + { + // https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ls.html + + sb.Append( + IsRegularFile ? '-' : + IsDirectory ? 'd' : + IsSymbolicLink ? 'l' : + IsNamedPipe ? 'p' : + IsSocket ? 's' : + IsCharacterDevice ? 'c' : + IsBlockDevice ? 'b' : + '-'); + + sb.Append(OwnerCanRead ? 'r' : '-'); + sb.Append(OwnerCanWrite ? 'w' : '-'); + + if (OwnerCanExecute) + { + sb.Append(IsUIDBitSet ? 's' : 'x'); + } + else + { + sb.Append(IsUIDBitSet ? 'S' : '-'); + } + + sb.Append(GroupCanRead ? 'r' : '-'); + sb.Append(GroupCanWrite ? 'w' : '-'); + + if (GroupCanExecute) + { + sb.Append(IsGroupIDBitSet ? 's' : 'x'); + } + else + { + sb.Append(IsGroupIDBitSet ? 'S' : '-'); + } + + sb.Append(OthersCanRead ? 'r' : '-'); + sb.Append(OthersCanWrite ? 'w' : '-'); + + if (OthersCanExecute) + { + sb.Append(IsStickyBitSet ? 't' : 'x'); + } + else + { + sb.Append(IsStickyBitSet ? 'T' : '-'); + } } /// @@ -506,7 +708,7 @@ namespace Renci.SshNet.Sftp { uint flag = 0; - if (IsSizeChanged && IsRegularFile) + if (IsSizeChanged) { flag |= 0x00000001; } @@ -533,7 +735,7 @@ namespace Renci.SshNet.Sftp stream.Write(flag); - if (IsSizeChanged && IsRegularFile) + if (IsSizeChanged) { stream.Write((ulong)Size); } @@ -551,9 +753,9 @@ namespace Renci.SshNet.Sftp if (IsLastAccessTimeChanged || IsLastWriteTimeChanged) { - var time = (uint)((LastAccessTimeUtc.ToFileTimeUtc() / 10000000) - 11644473600); + var time = (uint)((DateTimeOffset)DateTime.SpecifyKind(LastAccessTimeUtc, DateTimeKind.Utc)).ToUnixTimeSeconds(); stream.Write(time); - time = (uint)((LastWriteTimeUtc.ToFileTimeUtc() / 10000000) - 11644473600); + time = (uint)((DateTimeOffset)DateTime.SpecifyKind(LastWriteTimeUtc, DateTimeKind.Utc)).ToUnixTimeSeconds(); stream.Write(time); } @@ -561,12 +763,8 @@ namespace Renci.SshNet.Sftp { foreach (var item in Extensions) { - /* - * TODO: we write as ASCII but read as UTF8 !!! - */ - - stream.Write(item.Key, SshData.Ascii); - stream.Write(item.Value, SshData.Ascii); + stream.Write(item.Key, Encoding.UTF8); + stream.Write(item.Value, Encoding.UTF8); } } @@ -574,8 +772,6 @@ namespace Renci.SshNet.Sftp } } - internal static readonly SftpFileAttributes Empty = new SftpFileAttributes(); - internal static SftpFileAttributes FromBytes(SshDataStream stream) { const uint SSH_FILEXFER_ATTR_SIZE = 0x00000001; @@ -592,7 +788,7 @@ namespace Renci.SshNet.Sftp uint permissions = 0; DateTime accessTime; DateTime modifyTime; - Dictionary extensions = null; + Dictionary? extensions = null; if ((flag & SSH_FILEXFER_ATTR_SIZE) == SSH_FILEXFER_ATTR_SIZE) { @@ -613,12 +809,8 @@ namespace Renci.SshNet.Sftp if ((flag & SSH_FILEXFER_ATTR_ACMODTIME) == SSH_FILEXFER_ATTR_ACMODTIME) { - // The incoming times are "Unix times", so they're already in UTC. We need to preserve that - // to avoid losing information in a local time conversion during the "fall back" hour in DST. - var time = stream.ReadUInt32(); - accessTime = DateTime.FromFileTimeUtc((time + 11644473600) * 10000000); - time = stream.ReadUInt32(); - modifyTime = DateTime.FromFileTimeUtc((time + 11644473600) * 10000000); + accessTime = DateTimeOffset.FromUnixTimeSeconds(stream.ReadUInt32()).UtcDateTime; + modifyTime = DateTimeOffset.FromUnixTimeSeconds(stream.ReadUInt32()).UtcDateTime; } else { @@ -632,8 +824,8 @@ namespace Renci.SshNet.Sftp extensions = new Dictionary(extendedCount); for (var i = 0; i < extendedCount; i++) { - var extensionName = stream.ReadString(SshData.Utf8); - var extensionData = stream.ReadString(SshData.Utf8); + var extensionName = stream.ReadString(Encoding.UTF8); + var extensionData = stream.ReadString(Encoding.UTF8); extensions.Add(extensionName, extensionData); } } diff --git a/src/Renci.SshNet/Sftp/SftpSession.cs b/src/Renci.SshNet/Sftp/SftpSession.cs index 1de63eaf..2b4d8c00 100644 --- a/src/Renci.SshNet/Sftp/SftpSession.cs +++ b/src/Renci.SshNet/Sftp/SftpSession.cs @@ -379,7 +379,7 @@ namespace Renci.SshNet.Sftp if (response is SftpVersionResponse versionResponse) { ProtocolVersion = versionResponse.Version; - _supportedExtensions = versionResponse.Extentions; + _supportedExtensions = versionResponse.Extensions; _ = _sftpVersionConfirmed.Set(); } diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpFSetStatRequestTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpFSetStatRequestTest.cs index 69c0096e..2fd3bd1e 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpFSetStatRequestTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpFSetStatRequestTest.cs @@ -8,6 +8,7 @@ using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Requests; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp.Requests { @@ -29,7 +30,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp.Requests _requestId = (uint)random.Next(0, int.MaxValue); _handle = new byte[random.Next(1, 10)]; random.NextBytes(_handle); - _attributes = SftpFileAttributes.Empty; + _attributes = SftpFileAttributesBuilder.Empty; _attributesBytes = _attributes.GetBytes(); } diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpMkDirRequestTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpMkDirRequestTest.cs index ec0cf1af..e01d3f7f 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpMkDirRequestTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpMkDirRequestTest.cs @@ -10,6 +10,7 @@ using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Requests; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp.Requests { @@ -34,7 +35,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp.Requests _encoding = Encoding.Unicode; _path = random.Next().ToString(CultureInfo.InvariantCulture); _pathBytes = _encoding.GetBytes(_path); - _attributes = SftpFileAttributes.Empty; + _attributes = SftpFileAttributesBuilder.Empty; _attributesBytes = _attributes.GetBytes(); } diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpOpenRequestTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpOpenRequestTest.cs index 20838c86..dd7e407a 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpOpenRequestTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpOpenRequestTest.cs @@ -10,6 +10,7 @@ using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Requests; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp.Requests { @@ -36,7 +37,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp.Requests _filename = random.Next().ToString(CultureInfo.InvariantCulture); _filenameBytes = _encoding.GetBytes(_filename); _flags = Flags.Read; - _attributes = SftpFileAttributes.Empty; + _attributes = SftpFileAttributesBuilder.Empty; _attributesBytes = _attributes.GetBytes(); } diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpSetStatRequestTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpSetStatRequestTest.cs index c7111dd1..ae2e2231 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpSetStatRequestTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpSetStatRequestTest.cs @@ -10,6 +10,7 @@ using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Requests; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp.Requests { @@ -34,7 +35,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp.Requests _encoding = Encoding.Unicode; _path = random.Next().ToString(CultureInfo.InvariantCulture); _pathBytes = _encoding.GetBytes(_path); - _attributes = SftpFileAttributes.Empty; + _attributes = SftpFileAttributesBuilder.Empty; _attributesBytes = _attributes.GetBytes(); } diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpAttrsResponseTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpAttrsResponseTest.cs index ed899651..4598b537 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpAttrsResponseTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpAttrsResponseTest.cs @@ -5,6 +5,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp.Responses { @@ -61,7 +62,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp.Responses private SftpFileAttributes CreateSftpFileAttributes() { - var attributes = SftpFileAttributes.Empty; + var attributes = SftpFileAttributesBuilder.Empty; attributes.GroupId = _random.Next(); attributes.LastAccessTime = new DateTime(2014, 8, 23, 17, 43, 50, DateTimeKind.Local); attributes.LastWriteTime = new DateTime(2013, 7, 22, 16, 40, 42, DateTimeKind.Local); diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs new file mode 100644 index 00000000..566379a7 --- /dev/null +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs @@ -0,0 +1,223 @@ +using System; +using System.Buffers.Binary; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Renci.SshNet.Common; +using Renci.SshNet.Sftp; + +namespace Renci.SshNet.Tests.Classes.Sftp +{ + [TestClass] + public class SftpFileAttributesTest + { + [TestMethod] + [DataRow(0xC000u, true, false, false, false, false, false, false)] // Socket + [DataRow(0xA000u, false, true, false, false, false, false, false)] // Symbolic link + [DataRow(0x8000u, false, false, true, false, false, false, false)] // Regular file + [DataRow(0x6000u, false, false, false, true, false, false, false)] // Block device + [DataRow(0x4000u, false, false, false, false, true, false, false)] // Directory + [DataRow(0x2000u, false, false, false, false, false, true, false)] // Character device + [DataRow(0x1000u, false, false, false, false, false, false, true)] // Named pipe + public void FileTypePropertiesAreMutuallyExclusive( + uint permissions, + bool isSocket, + bool isSymbolicLink, + bool isRegularFile, + bool isBlockDevice, + bool isDirectory, + bool isCharacterDevice, + bool isNamedPipe) + { + var attributeBytes = new byte[8]; + attributeBytes[3] = 0x4; // SSH_FILEXFER_ATTR_PERMISSIONS + BinaryPrimitives.WriteUInt32BigEndian(attributeBytes.AsSpan(4), permissions); + + var attributes = SftpFileAttributes.FromBytes(attributeBytes); + + Assert.AreEqual(isSocket, attributes.IsSocket); + Assert.AreEqual(isSymbolicLink, attributes.IsSymbolicLink); + Assert.AreEqual(isRegularFile, attributes.IsRegularFile); + Assert.AreEqual(isBlockDevice, attributes.IsBlockDevice); + Assert.AreEqual(isDirectory, attributes.IsDirectory); + Assert.AreEqual(isCharacterDevice, attributes.IsCharacterDevice); + Assert.AreEqual(isNamedPipe, attributes.IsNamedPipe); + } + + [TestMethod] + public void FromBytesGetBytes() + { + // 81a4 in hex = 100644 in octal + var attributes = SftpFileAttributes.FromBytes([0, 0, 0, 0x4, 0, 0, 0x81, 0xa4]); + + Assert.IsTrue(attributes.IsRegularFile); + + Assert.IsFalse(attributes.IsUIDBitSet); + Assert.IsFalse(attributes.IsGroupIDBitSet); + Assert.IsFalse(attributes.IsStickyBitSet); + Assert.IsTrue(attributes.OwnerCanRead); + Assert.IsTrue(attributes.OwnerCanWrite); + Assert.IsFalse(attributes.OwnerCanExecute); + Assert.IsTrue(attributes.GroupCanRead); + Assert.IsFalse(attributes.GroupCanWrite); + Assert.IsFalse(attributes.GroupCanExecute); + Assert.IsTrue(attributes.OthersCanRead); + Assert.IsFalse(attributes.OthersCanWrite); + Assert.IsFalse(attributes.OthersCanExecute); + + Assert.AreEqual(-1, attributes.Size); // Erm, OK? + Assert.AreEqual(-1, attributes.UserId); + Assert.AreEqual(-1, attributes.GroupId); + + Assert.AreEqual(default, attributes.LastAccessTimeUtc); + Assert.AreEqual(DateTimeKind.Utc, attributes.LastAccessTimeUtc.Kind); + + Assert.AreEqual(default, attributes.LastWriteTimeUtc); + Assert.AreEqual(DateTimeKind.Utc, attributes.LastWriteTimeUtc.Kind); + + Assert.AreEqual("-rw-r--r--", attributes.ToString()); + + // No changes + CollectionAssert.AreEqual( + new byte[] { 0, 0, 0, 0 }, + attributes.GetBytes()); + + + // Permissions change + attributes.IsUIDBitSet = true; + attributes.OwnerCanExecute = true; + + CollectionAssert.AreEqual( + new byte[] { 0, 0, 0, 0x4, 0, 0, 0x89, 0xe4 }, + attributes.GetBytes()); + + Assert.AreEqual("-rwsr--r--", attributes.ToString()); + + // Size change + attributes.Size = 123; + + CollectionAssert.AreEqual( + new byte[] { + 0, 0, 0, 0x1 | 0x4, + 0, 0, 0, 0, 0, 0, 0, 123, + 0, 0, 0x89, 0xe4 }, + attributes.GetBytes()); + + Assert.IsTrue(attributes.ToString().StartsWith("-rwsr--r-- Size: ", StringComparison.Ordinal)); + + // Uid/gid change + attributes.UserId = 99; + attributes.GroupId = 66; + + CollectionAssert.AreEqual( + new byte[] { + 0, 0, 0, 0x1 | 0x2 | 0x4, + 0, 0, 0, 0, 0, 0, 0, 123, + 0, 0, 0, 99, 0, 0, 0, 66, + 0, 0, 0x89, 0xe4 }, + attributes.GetBytes()); + + + // Access/mod time change + attributes.LastAccessTimeUtc = new DateTime(2025, 08, 10, 17, 51, 37, DateTimeKind.Unspecified); + attributes.LastWriteTime = new DateTimeOffset(2016, 12, 02, 13, 18, 20, TimeSpan.FromHours(3)).LocalDateTime; + + var expectedTimeBytes = new byte[8]; + BinaryPrimitives.WriteUInt32BigEndian(expectedTimeBytes, 1754848297); + BinaryPrimitives.WriteUInt32BigEndian(expectedTimeBytes.AsSpan(4), 1480673900); + + CollectionAssert.AreEqual( + new byte[] { + 0, 0, 0, 0x1 | 0x2 | 0x4 | 0x8, + 0, 0, 0, 0, 0, 0, 0, 123, + 0, 0, 0, 99, 0, 0, 0, 66, + 0, 0, 0x89, 0xe4 + }.Concat(expectedTimeBytes), + attributes.GetBytes()); + + Assert.AreEqual(new DateTime(2016, 12, 02, 10, 18, 20, DateTimeKind.Utc), attributes.LastWriteTimeUtc); + Assert.AreEqual(DateTimeKind.Utc, attributes.LastWriteTimeUtc.Kind); + + var attributesString = attributes.ToString(); + Assert.IsTrue(attributesString.StartsWith("-rwsr--r-- Size: ", StringComparison.Ordinal)); + Assert.Contains(" LastWriteTime: ", attributesString, StringComparison.CurrentCulture); + } + + [TestMethod] + [DataRow((short)8888)] + [DataRow((short)10000)] + [DataRow((short)8000)] + [DataRow((short)0080)] + [DataRow((short)0008)] + [DataRow((short)1797)] + [DataRow((short)-1)] + [DataRow(short.MaxValue)] + public void SetPermissions_InvalidMode_ThrowsArgumentOutOfRangeException(short mode) + { + var attributes = SftpFileAttributes.FromBytes([0, 0, 0, 0]); + + var ex = Assert.Throws(() => attributes.SetPermissions(mode)); + Assert.AreEqual("mode", ex.ParamName); + } + + [TestMethod] + [DataRow((short)0777, false, false, false, true, true, true, true, true, true, true, true, true)] + [DataRow((short)0755, false, false, false, true, true, true, true, false, true, true, false, true)] + [DataRow((short)0644, false, false, false, true, true, false, true, false, false, true, false, false)] + [DataRow((short)0444, false, false, false, true, false, false, true, false, false, true, false, false)] + [DataRow((short)0000, false, false, false, false, false, false, false, false, false, false, false, false)] + [DataRow((short)4700, true, false, false, true, true, true, false, false, false, false, false, false)] + [DataRow((short)3001, false, true, true, false, false, false, false, false, false, false, false, true)] + [DataRow((short)7777, true, true, true, true, true, true, true, true, true, true, true, true)] + public void SetPermissions_ValidMode( + short mode, + bool setUid, bool setGid, bool sticky, + bool ownerRead, bool ownerWrite, bool ownerExec, + bool groupRead, bool groupWrite, bool groupExec, + bool othersRead, bool othersWrite, bool othersExec) + { + var attributes = SftpFileAttributes.FromBytes([0, 0, 0, 0]); + + attributes.SetPermissions(mode); + + Assert.AreEqual(setUid, attributes.IsUIDBitSet); + Assert.AreEqual(setGid, attributes.IsGroupIDBitSet); + Assert.AreEqual(sticky, attributes.IsStickyBitSet); + Assert.AreEqual(ownerRead, attributes.OwnerCanRead); + Assert.AreEqual(ownerWrite, attributes.OwnerCanWrite); + Assert.AreEqual(ownerExec, attributes.OwnerCanExecute); + Assert.AreEqual(groupRead, attributes.GroupCanRead); + Assert.AreEqual(groupWrite, attributes.GroupCanWrite); + Assert.AreEqual(groupExec, attributes.GroupCanExecute); + Assert.AreEqual(othersRead, attributes.OthersCanRead); + Assert.AreEqual(othersWrite, attributes.OthersCanWrite); + Assert.AreEqual(othersExec, attributes.OthersCanExecute); + } + + [TestMethod] + [DataRow(0xC000u, (short)1770, "srwxrwx--T")] // Socket + [DataRow(0xA000u, (short)2707, "lrwx--Srwx")] // Symbolic link + [DataRow(0x8000u, (short)4755, "-rwsr-xr-x")] // Regular file + [DataRow(0x8000u, (short)4644, "-rwSr--r--")] // Regular file + [DataRow(0x6000u, (short)2711, "brwx--s--x")] // Block device + [DataRow(0x4000u, (short)1777, "drwxrwxrwt")] // Directory + [DataRow(0x4000u, (short)1776, "drwxrwxrwT")] // Directory + [DataRow(0x2000u, (short)0660, "crw-rw----")] // Character device + [DataRow(0x1000u, (short)0022, "p----w--w-")] // Named pipe + public void ToStringWithPermissions( + uint fileType, + short permissions, + string expected) + { + var attributeBytes = new byte[8]; + attributeBytes[3] = 0x4; // SSH_FILEXFER_ATTR_PERMISSIONS + BinaryPrimitives.WriteUInt32BigEndian(attributeBytes.AsSpan(4), fileType); + + var attributes = SftpFileAttributes.FromBytes(attributeBytes); + + attributes.SetPermissions(permissions); + + Assert.AreEqual(expected, attributes.ToString()); + } + } +} diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileReaderTestBase.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileReaderTestBase.cs index c48836ba..1cb1207b 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileReaderTestBase.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileReaderTestBase.cs @@ -41,12 +41,6 @@ namespace Renci.SshNet.Tests.Classes.Sftp protected abstract void Act(); - protected static SftpFileAttributes CreateSftpFileAttributes(long size) - { - var utcDefault = DateTime.SpecifyKind(default, DateTimeKind.Utc); - return new SftpFileAttributes(utcDefault, utcDefault, size, default, default, default, null); - } - protected static byte[] CreateByteArray(Random random, int length) { var chunk = new byte[length]; diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestRead.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestRead.cs index 50125eb7..3b19a018 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestRead.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestRead.cs @@ -12,6 +12,7 @@ using Renci.SshNet.Channels; using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp { @@ -73,7 +74,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp _sftpNameResponse = new SftpNameResponseBuilder().WithProtocolVersion(_protocolVersion) .WithResponseId(1) .WithEncoding(_encoding) - .WithFile("XYZ", SftpFileAttributes.Empty) + .WithFile("XYZ", SftpFileAttributesBuilder.Empty) .Build(); #endregion SftpSession.Connect() diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestStatVfs.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestStatVfs.cs index a80c744d..a5086d3d 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestStatVfs.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_Connected_RequestStatVfs.cs @@ -10,6 +10,7 @@ using Renci.SshNet.Channels; using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp { @@ -70,7 +71,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp _sftpNameResponse = new SftpNameResponseBuilder().WithProtocolVersion(_protocolVersion) .WithResponseId(1U) .WithEncoding(_encoding) - .WithFile("ABC", SftpFileAttributes.Empty) + .WithFile("ABC", SftpFileAttributesBuilder.Empty) .Build(); #endregion SftpSession.Connect() diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesInSingleSshDataMessage.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesInSingleSshDataMessage.cs index 45bf5c02..eac86ccd 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesInSingleSshDataMessage.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesInSingleSshDataMessage.cs @@ -11,6 +11,7 @@ using Renci.SshNet.Channels; using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp { @@ -75,7 +76,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp _sftpNameResponse = new SftpNameResponseBuilder().WithProtocolVersion(_protocolVersion) .WithResponseId(1) .WithEncoding(_encoding) - .WithFile("/ABC", SftpFileAttributes.Empty) + .WithFile("/ABC", SftpFileAttributesBuilder.Empty) .Build(); #endregion SftpSession.Connect() diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesSplitOverMultipleSshDataMessages.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesSplitOverMultipleSshDataMessages.cs index e0e809af..beb94c48 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesSplitOverMultipleSshDataMessages.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_MultipleSftpMessagesSplitOverMultipleSshDataMessages.cs @@ -11,6 +11,7 @@ using Renci.SshNet.Channels; using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp { @@ -75,7 +76,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp _sftpNameResponse = new SftpNameResponseBuilder().WithProtocolVersion(_protocolVersion) .WithResponseId(1) .WithEncoding(_encoding) - .WithFile("/ABC", SftpFileAttributes.Empty) + .WithFile("/ABC", SftpFileAttributesBuilder.Empty) .Build(); #endregion SftpSession.Connect() diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_SingleSftpMessageInSshDataMessage.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_SingleSftpMessageInSshDataMessage.cs index 88aad7e6..ead62c82 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_SingleSftpMessageInSshDataMessage.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpSessionTest_DataReceived_SingleSftpMessageInSshDataMessage.cs @@ -11,6 +11,7 @@ using Renci.SshNet.Channels; using Renci.SshNet.Common; using Renci.SshNet.Sftp; using Renci.SshNet.Sftp.Responses; +using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Sftp { @@ -71,7 +72,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp _sftpNameResponse = new SftpNameResponseBuilder().WithProtocolVersion(_protocolVersion) .WithResponseId(1) .WithEncoding(_encoding) - .WithFile("/ABC", SftpFileAttributes.Empty) + .WithFile("/ABC", SftpFileAttributesBuilder.Empty) .Build(); #endregion SftpSession.Connect() diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpVersionResponseBuilder.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpVersionResponseBuilder.cs index f69af49b..0590c65e 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpVersionResponseBuilder.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpVersionResponseBuilder.cs @@ -31,7 +31,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp var sftpVersionResponse = new SftpVersionResponse() { Version = _version, - Extentions = _extensions + Extensions = _extensions }; return sftpVersionResponse; } diff --git a/test/Renci.SshNet.Tests/Classes/ShellStreamTest_Write_WriteBufferNotEmptyAndWriteMoreBytesThanBufferCanContain.cs b/test/Renci.SshNet.Tests/Classes/ShellStreamTest_Write_WriteBufferNotEmptyAndWriteMoreBytesThanBufferCanContain.cs index 30de20fb..86817598 100644 --- a/test/Renci.SshNet.Tests/Classes/ShellStreamTest_Write_WriteBufferNotEmptyAndWriteMoreBytesThanBufferCanContain.cs +++ b/test/Renci.SshNet.Tests/Classes/ShellStreamTest_Write_WriteBufferNotEmptyAndWriteMoreBytesThanBufferCanContain.cs @@ -10,7 +10,6 @@ using Moq; using Renci.SshNet.Abstractions; using Renci.SshNet.Channels; using Renci.SshNet.Common; -using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes { @@ -60,9 +59,7 @@ namespace Renci.SshNet.Tests.Classes _offset = 0; _count = _data.Length; - _expectedBytesSent = new ArrayBuilder().Add(_bufferData) - .Add(_data, 0, _bufferSize - _bufferData.Length) - .Build(); + _expectedBytesSent = [.. _bufferData, .. _data.Take(0, _bufferSize - _bufferData.Length)]; } private void CreateMocks() diff --git a/test/Renci.SshNet.Tests/Common/ArrayBuilder`1.cs b/test/Renci.SshNet.Tests/Common/ArrayBuilder`1.cs deleted file mode 100644 index 9f9f7d65..00000000 --- a/test/Renci.SshNet.Tests/Common/ArrayBuilder`1.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System.Collections.Generic; - -namespace Renci.SshNet.Tests.Common -{ - public class ArrayBuilder - { - private readonly List _buffer; - - public ArrayBuilder() - { - _buffer = new List(); - } - - public ArrayBuilder Add(T[] array) - { - return Add(array, 0, array.Length); - } - - public ArrayBuilder Add(T[] array, int index, int length) - { - for (var i = 0; i < length; i++) - { - _buffer.Add(array[index + i]); - } - - return this; - } - - public T[] Build() - { - return _buffer.ToArray(); - } - } -} diff --git a/test/Renci.SshNet.Tests/Common/Extensions.cs b/test/Renci.SshNet.Tests/Common/Extensions.cs index 5f8b1e31..977c182d 100644 --- a/test/Renci.SshNet.Tests/Common/Extensions.cs +++ b/test/Renci.SshNet.Tests/Common/Extensions.cs @@ -1,8 +1,6 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using Renci.SshNet.Common; -using Renci.SshNet.Sftp; namespace Renci.SshNet.Tests.Common { @@ -23,44 +21,5 @@ namespace Renci.SshNet.Tests.Common return reportedExceptions; } - - public static byte[] Copy(this byte[] buffer) - { - var copy = new byte[buffer.Length]; - Buffer.BlockCopy(buffer, 0, copy, 0, buffer.Length); - return copy; - } - - /// - /// Creates a deep clone of the current instance. - /// - /// - /// A deep clone of the current instance. - /// - internal static SftpFileAttributes Clone(this SftpFileAttributes value) - { - Dictionary clonedExtensions; - - if (value.Extensions != null) - { - clonedExtensions = new Dictionary(value.Extensions.Count); - foreach (var entry in value.Extensions) - { - clonedExtensions.Add(entry.Key, entry.Value); - } - } - else - { - clonedExtensions = null; - } - - return new SftpFileAttributes(value.LastAccessTimeUtc, - value.LastWriteTimeUtc, - value.Size, - value.UserId, - value.GroupId, - value.Permissions, - clonedExtensions); - } } } diff --git a/test/Renci.SshNet.Tests/Common/SftpFileAttributesBuilder.cs b/test/Renci.SshNet.Tests/Common/SftpFileAttributesBuilder.cs index ca14b16c..50e772b9 100644 --- a/test/Renci.SshNet.Tests/Common/SftpFileAttributesBuilder.cs +++ b/test/Renci.SshNet.Tests/Common/SftpFileAttributesBuilder.cs @@ -1,4 +1,5 @@ -using System; +#nullable enable +using System; using System.Collections.Generic; using Renci.SshNet.Sftp; @@ -7,18 +8,21 @@ namespace Renci.SshNet.Tests.Common { public class SftpFileAttributesBuilder { + public static SftpFileAttributes Empty + { + get + { + return new SftpFileAttributesBuilder().Build(); + } + } + private DateTime? _lastAccessTime; private DateTime? _lastWriteTime; private long? _size; private int? _userId; private int? _groupId; private uint? _permissions; - private readonly IDictionary _extensions; - - public SftpFileAttributesBuilder() - { - _extensions = new Dictionary(); - } + private Dictionary? _extensions; public SftpFileAttributesBuilder WithLastAccessTime(DateTime lastAccessTime) { @@ -58,6 +62,7 @@ namespace Renci.SshNet.Tests.Common public SftpFileAttributesBuilder WithExtension(string name, string value) { + _extensions ??= []; _extensions.Add(name, value); return this; } From ebdcb3ea7d0e85e00a8dc1b64766378c8a306908 Mon Sep 17 00:00:00 2001 From: mus65 Date: Sat, 4 Oct 2025 11:00:09 +0200 Subject: [PATCH 4/4] CI: add Windows Integration Tests for .NET (#1704) * 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 Co-authored-by: Robert Hague --- .github/workflows/build.yml | 56 +++++++++++++++++-- .github/workflows/docs.yml | 2 +- .../Common/RemoteSshdConfigExtensions.cs | 1 + .../TestsFixtures/InfrastructureFixture.cs | 23 ++++---- 4 files changed, 65 insertions(+), 17 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 63d0021f..9b3759d4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,7 +15,7 @@ jobs: fetch-depth: 0 # needed for Nerdbank.GitVersioning - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 - name: Build Unit Tests .NET run: dotnet build -f net9.0 test/Renci.SshNet.Tests/ @@ -62,7 +62,7 @@ jobs: fetch-depth: 0 # needed for Nerdbank.GitVersioning - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 - name: Build Solution run: dotnet build Renci.SshNet.sln @@ -103,8 +103,8 @@ jobs: -p:CoverletOutput=../../coverlet/windows_unit_test_net_4_6_2_coverage.xml ` test/Renci.SshNet.Tests/ - Windows-Integration-Tests: - name: Windows Integration Tests + Windows-Integration-Tests-NetFramework: + name: Windows Integration Tests .NET Framework runs-on: windows-2025 steps: - name: Checkout @@ -113,7 +113,7 @@ jobs: fetch-depth: 0 # needed for Nerdbank.GitVersioning - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 - name: Setup WSL2 uses: Vampire/setup-wsl@6a8db447be7ed35f2f499c02c6e60ff77ef11278 # v6.0.0 @@ -142,7 +142,49 @@ jobs: - name: Archive Coverlet Results uses: actions/upload-artifact@v4 with: - name: Coverlet Results Windows + name: Coverlet Results Windows .NET Framework + path: coverlet + + Windows-Integration-Tests-Net: + name: Windows Integration Tests .NET + runs-on: windows-2025 + steps: + - name: Checkout + uses: actions/checkout@v5 + with: + fetch-depth: 0 # needed for Nerdbank.GitVersioning + + - name: Setup .NET + uses: actions/setup-dotnet@v5 + + - name: Setup WSL2 + uses: Vampire/setup-wsl@6a8db447be7ed35f2f499c02c6e60ff77ef11278 # v6.0.0 + with: + distribution: Ubuntu-24.04 + + - name: Setup SSH Server + shell: wsl-bash {0} + run: | + apt-get update && apt-get upgrade -y + apt-get install -y podman + podman build -t renci-ssh-tests-server-image -f test/Renci.SshNet.IntegrationTests/Dockerfile test/Renci.SshNet.IntegrationTests/ + podman run --rm -h renci-ssh-tests-server -d -p 2222:22 renci-ssh-tests-server-image + + - name: Run Integration Tests .NET + run: + dotnet test ` + -f net9.0 ` + --logger "console;verbosity=normal" ` + --logger GitHubActions ` + -p:CollectCoverage=true ` + -p:CoverletOutputFormat=cobertura ` + -p:CoverletOutput=..\..\coverlet\windows_integration_test_net_9_coverage.xml ` + test\Renci.SshNet.IntegrationTests\ + + - name: Archive Coverlet Results + uses: actions/upload-artifact@v4 + with: + name: Coverlet Results Windows .NET path: coverlet Publish: @@ -153,6 +195,8 @@ jobs: needs: - Windows - Linux + - Windows-Integration-Tests-NetFramework + - Windows-Integration-Tests-Net steps: - name: Download NuGet Package uses: actions/download-artifact@v5 diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index b989c225..d33343dc 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -28,7 +28,7 @@ jobs: uses: actions/configure-pages@v5 - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 - name: Setup docfx run: dotnet tool update -g docfx diff --git a/test/Renci.SshNet.IntegrationTests/Common/RemoteSshdConfigExtensions.cs b/test/Renci.SshNet.IntegrationTests/Common/RemoteSshdConfigExtensions.cs index 2a2ccaa4..9309be40 100644 --- a/test/Renci.SshNet.IntegrationTests/Common/RemoteSshdConfigExtensions.cs +++ b/test/Renci.SshNet.IntegrationTests/Common/RemoteSshdConfigExtensions.cs @@ -23,6 +23,7 @@ namespace Renci.SshNet.IntegrationTests.Common .ClearHostKeyAlgorithms() .ClearPublicKeyAcceptedAlgorithms() .ClearMessageAuthenticationCodeAlgorithms() + .PermitTTY(true) .WithUsePAM(true) .Update() .Restart(); diff --git a/test/Renci.SshNet.IntegrationTests/TestsFixtures/InfrastructureFixture.cs b/test/Renci.SshNet.IntegrationTests/TestsFixtures/InfrastructureFixture.cs index bcdb00d7..304f294a 100644 --- a/test/Renci.SshNet.IntegrationTests/TestsFixtures/InfrastructureFixture.cs +++ b/test/Renci.SshNet.IntegrationTests/TestsFixtures/InfrastructureFixture.cs @@ -1,4 +1,6 @@ -using DotNet.Testcontainers.Builders; +using System.Runtime.InteropServices; + +using DotNet.Testcontainers.Builders; using DotNet.Testcontainers.Containers; using DotNet.Testcontainers.Images; @@ -28,26 +30,27 @@ namespace Renci.SshNet.IntegrationTests.TestsFixtures private IFutureDockerImage _sshServerImage; - public string SshServerHostName { get; set; } + public string SshServerHostName { get; private set; } - public ushort SshServerPort { get; set; } + public ushort SshServerPort { get; private set; } - public SshUser AdminUser = new SshUser("sshnetadm", "ssh4ever"); + public SshUser AdminUser { get; } = new SshUser("sshnetadm", "ssh4ever"); - public SshUser User = new SshUser("sshnet", "ssh4ever"); + public SshUser User { get; } = new SshUser("sshnet", "ssh4ever"); public async Task InitializeAsync() { - // for the .NET Framework Tests in CI, the Container is set up in WSL2 with Podman -#if NETFRAMEWORK - if (Environment.GetEnvironmentVariable("CI") == "true") +#pragma warning disable MA0144 // use System.OperatingSystem to check the current OS + // for the Windows Tests in CI, the Container is set up in WSL2 with Podman + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && + Environment.GetEnvironmentVariable("CI") == "true") +#pragma warning restore MA0144 // use System.OperatingSystem to check the current OS { SshServerPort = 2222; - SshServerHostName = "localhost"; + SshServerHostName = "127.0.0.1"; await Task.Delay(1_000); return; } -#endif var containerLogger = _loggerFactory.CreateLogger("testcontainers");