From d2fb3acfe25aca2ebc004412ea6f3a33f5a3c54b Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Thu, 7 Dec 2023 22:47:43 +0100 Subject: [PATCH 1/3] Enable diagnostics in release --- .editorconfig | 11 ++ src/Renci.SshNet/BaseClient.cs | 5 +- src/Renci.SshNet/Channels/Channel.cs | 6 +- .../Channels/ChannelDirectTcpip.cs | 8 +- .../Channels/ChannelForwardedTcpip.cs | 8 +- src/Renci.SshNet/Connection/ConnectorBase.cs | 11 +- ...iagnosticAbstraction.cs => Diagnostics.cs} | 36 +++--- src/Renci.SshNet/ForwardedPortDynamic.cs | 4 +- src/Renci.SshNet/ForwardedPortLocal.cs | 4 +- src/Renci.SshNet/ForwardedPortRemote.cs | 4 +- .../Messages/Transport/DisconnectMessage.cs | 6 + src/Renci.SshNet/Security/KeyExchange.cs | 105 +++++++++++++----- src/Renci.SshNet/ServiceFactory.cs | 26 ++++- src/Renci.SshNet/Session.cs | 98 ++++++++++++---- src/Renci.SshNet/Sftp/SftpFileReader.cs | 6 +- src/Renci.SshNet/SubsystemSession.cs | 7 +- .../TestsFixtures/IntegrationTestBase.cs | 12 +- 17 files changed, 252 insertions(+), 105 deletions(-) rename src/Renci.SshNet/{Abstractions/DiagnosticAbstraction.cs => Diagnostics.cs} (61%) diff --git a/.editorconfig b/.editorconfig index aa407a9c4..98f1c61f7 100644 --- a/.editorconfig +++ b/.editorconfig @@ -571,6 +571,17 @@ MA0053.exceptions_should_be_sealed = true # Also report diagnostic for types that define (new) virtual members. MA0053.class_with_virtual_member_shoud_be_sealed = true +# MA0076: Do not use implicit culture-sensitive ToString in interpolated strings +# +# Interpolated strings have performance benefits. +dotnet_diagnostic.MA0076.severity = none + +# MA0089: Optimize string method usage +# +# Duplicate of CA1865-CA1867 / +# For string.Join, results in verbose code for very little gain. +dotnet_diagnostic.MA0089.severity = none + # MA0112: Use 'Count > 0' instead of 'Any()' # # This rule is disabled by default, hence we need to explicitly enable it. diff --git a/src/Renci.SshNet/BaseClient.cs b/src/Renci.SshNet/BaseClient.cs index bdfd5cb5f..1f2ef25e4 100644 --- a/src/Renci.SshNet/BaseClient.cs +++ b/src/Renci.SshNet/BaseClient.cs @@ -3,7 +3,6 @@ using System.Threading; using System.Threading.Tasks; -using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Messages.Transport; @@ -323,7 +322,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken) /// The method was called after the client was disposed. public void Disconnect() { - DiagnosticAbstraction.Log("Disconnecting client."); + Diagnostics.Log("Disconnecting client.", System.Diagnostics.TraceEventType.Verbose); CheckDisposed(); @@ -422,7 +421,7 @@ protected virtual void Dispose(bool disposing) if (disposing) { - DiagnosticAbstraction.Log("Disposing client."); + Diagnostics.Log("Disposing client.", System.Diagnostics.TraceEventType.Verbose); Disconnect(); diff --git a/src/Renci.SshNet/Channels/Channel.cs b/src/Renci.SshNet/Channels/Channel.cs index 580c63e0f..5a98b1b34 100644 --- a/src/Renci.SshNet/Channels/Channel.cs +++ b/src/Renci.SshNet/Channels/Channel.cs @@ -1,9 +1,9 @@ using System; +using System.Diagnostics; using System.Globalization; using System.Net.Sockets; using System.Threading; -using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Messages; using Renci.SshNet.Messages.Connection; @@ -554,9 +554,9 @@ protected virtual void Close() // SSH_MSG_CHANNEL_CLOSE as response to a SSH_MSG_CHANNEL_CLOSE sent by the // server var closeWaitResult = _session.TryWait(_channelClosedWaitHandle, ConnectionInfo.ChannelCloseTimeout); - if (closeWaitResult != WaitResult.Success) + if (closeWaitResult != WaitResult.Success && Diagnostics.IsEnabled(TraceEventType.Warning)) { - DiagnosticAbstraction.Log(string.Format("Wait for channel close not successful: {0:G}.", closeWaitResult)); + Diagnostics.Log($"Wait for channel close not successful: {closeWaitResult}.", TraceEventType.Warning); } } } diff --git a/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs b/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs index 6c521bce2..4e983c708 100644 --- a/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs +++ b/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs @@ -1,7 +1,9 @@ using System; +using System.Diagnostics; using System.Net; using System.Net.Sockets; using System.Threading; + using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Messages.Connection; @@ -156,8 +158,10 @@ private void ShutdownSocket(SocketShutdown how) } catch (SocketException ex) { - // TODO: log as warning - DiagnosticAbstraction.Log("Failure shutting down socket: " + ex); + if (Diagnostics.IsEnabled(TraceEventType.Warning)) + { + Diagnostics.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); + } } } } diff --git a/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs b/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs index a8382015a..10161bcca 100644 --- a/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs +++ b/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs @@ -1,6 +1,8 @@ using System; +using System.Diagnostics; using System.Net; using System.Net.Sockets; + using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Messages.Connection; @@ -138,8 +140,10 @@ private void ShutdownSocket(SocketShutdown how) } catch (SocketException ex) { - // TODO: log as warning - DiagnosticAbstraction.Log("Failure shutting down socket: " + ex); + if (Diagnostics.IsEnabled(TraceEventType.Warning)) + { + Diagnostics.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); + } } } } diff --git a/src/Renci.SshNet/Connection/ConnectorBase.cs b/src/Renci.SshNet/Connection/ConnectorBase.cs index 58b45960a..c022c8da7 100644 --- a/src/Renci.SshNet/Connection/ConnectorBase.cs +++ b/src/Renci.SshNet/Connection/ConnectorBase.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Net; using System.Net.Sockets; using System.Threading; @@ -41,7 +42,10 @@ protected Socket SocketConnect(string host, int port, TimeSpan timeout) var ipAddress = DnsAbstraction.GetHostAddresses(host)[0]; var ep = new IPEndPoint(ipAddress, port); - DiagnosticAbstraction.Log(string.Format("Initiating connection to '{0}:{1}'.", host, port)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"Initiating connection to '{host}:{port}'.", TraceEventType.Information); + } var socket = SocketFactory.Create(ep.AddressFamily, SocketType.Stream, ProtocolType.Tcp); @@ -76,7 +80,10 @@ protected async Task SocketConnectAsync(string host, int port, Cancellat var ipAddress = (await DnsAbstraction.GetHostAddressesAsync(host).ConfigureAwait(false))[0]; var ep = new IPEndPoint(ipAddress, port); - DiagnosticAbstraction.Log(string.Format("Initiating connection to '{0}:{1}'.", host, port)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"Initiating connection to '{host}:{port}'.", TraceEventType.Information); + } var socket = SocketFactory.Create(ep.AddressFamily, SocketType.Stream, ProtocolType.Tcp); try diff --git a/src/Renci.SshNet/Abstractions/DiagnosticAbstraction.cs b/src/Renci.SshNet/Diagnostics.cs similarity index 61% rename from src/Renci.SshNet/Abstractions/DiagnosticAbstraction.cs rename to src/Renci.SshNet/Diagnostics.cs index bc1248dc0..7a408973a 100644 --- a/src/Renci.SshNet/Abstractions/DiagnosticAbstraction.cs +++ b/src/Renci.SshNet/Diagnostics.cs @@ -1,28 +1,24 @@ -using System.ComponentModel; +using System; using System.Diagnostics; -namespace Renci.SshNet.Abstractions +namespace Renci.SshNet { /// /// Provides access to the internals of SSH.NET. /// - [EditorBrowsable(EditorBrowsableState.Never)] - public static class DiagnosticAbstraction + public static class Diagnostics { /// /// The instance used by SSH.NET. /// /// /// - /// Currently, the library only traces events when compiled in Debug mode. - /// - /// /// Configuration on .NET Core must be done programmatically, e.g. /// - /// DiagnosticAbstraction.Source.Switch = new SourceSwitch("sourceSwitch", "Verbose"); - /// DiagnosticAbstraction.Source.Listeners.Remove("Default"); - /// DiagnosticAbstraction.Source.Listeners.Add(new ConsoleTraceListener()); - /// DiagnosticAbstraction.Source.Listeners.Add(new TextWriterTraceListener("trace.log")); + /// Diagnostics.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Verbose)); + /// Diagnostics.Source.Listeners.Remove("Default"); + /// Diagnostics.Source.Listeners.Add(new ConsoleTraceListener()); + /// Diagnostics.Source.Listeners.Add(new TextWriterTraceListener("SshNetTrace.log")); /// /// /// @@ -53,17 +49,19 @@ public static class DiagnosticAbstraction public static readonly TraceSource Source = new TraceSource("SshNet.Logging"); /// - /// Logs a message to at the - /// level. + /// Logs a message to with the specified event type. /// /// The message to log. - /// The trace event type. - [Conditional("DEBUG")] - public static void Log(string text, TraceEventType type = TraceEventType.Verbose) + /// The trace event type. + public static void Log(string text, TraceEventType eventType) + { + Source.TraceEvent(eventType, Environment.CurrentManagedThreadId, text); + } + + /// + public static bool IsEnabled(TraceEventType eventType) { - Source.TraceEvent(type, - System.Environment.CurrentManagedThreadId, - text); + return Source.Switch.ShouldTrace(eventType); } } } diff --git a/src/Renci.SshNet/ForwardedPortDynamic.cs b/src/Renci.SshNet/ForwardedPortDynamic.cs index 21ab59a20..0a0a623eb 100644 --- a/src/Renci.SshNet/ForwardedPortDynamic.cs +++ b/src/Renci.SshNet/ForwardedPortDynamic.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Net; @@ -413,8 +414,7 @@ private void InternalStop(TimeSpan timeout) if (!_pendingChannelCountdown.Wait(timeout)) { - // TODO: log as warning - DiagnosticAbstraction.Log("Timeout waiting for pending channels in dynamic forwarded port to close."); + Diagnostics.Log("Timeout waiting for pending channels in dynamic forwarded port to close.", TraceEventType.Warning); } } diff --git a/src/Renci.SshNet/ForwardedPortLocal.cs b/src/Renci.SshNet/ForwardedPortLocal.cs index 3d5b58768..f70b14fb7 100644 --- a/src/Renci.SshNet/ForwardedPortLocal.cs +++ b/src/Renci.SshNet/ForwardedPortLocal.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Net; using System.Net.Sockets; using System.Threading; @@ -402,8 +403,7 @@ private void InternalStop(TimeSpan timeout) if (!_pendingChannelCountdown.Wait(timeout)) { - // TODO: log as warning - DiagnosticAbstraction.Log("Timeout waiting for pending channels in local forwarded port to close."); + Diagnostics.Log("Timeout waiting for pending channels in local forwarded port to close.", TraceEventType.Warning); } } diff --git a/src/Renci.SshNet/ForwardedPortRemote.cs b/src/Renci.SshNet/ForwardedPortRemote.cs index af7fd48a2..0b26ffbf1 100644 --- a/src/Renci.SshNet/ForwardedPortRemote.cs +++ b/src/Renci.SshNet/ForwardedPortRemote.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Globalization; using System.Net; using System.Threading; @@ -216,8 +217,7 @@ protected override void StopPort(TimeSpan timeout) if (!_pendingChannelCountdown.Wait(timeout)) { - // TODO: log as warning - DiagnosticAbstraction.Log("Timeout waiting for pending channels in remote forwarded port to close."); + Diagnostics.Log("Timeout waiting for pending channels in remote forwarded port to close.", TraceEventType.Warning); } _status = ForwardedPortStatus.Stopped; diff --git a/src/Renci.SshNet/Messages/Transport/DisconnectMessage.cs b/src/Renci.SshNet/Messages/Transport/DisconnectMessage.cs index b5f43adfc..0a17e6b3c 100644 --- a/src/Renci.SshNet/Messages/Transport/DisconnectMessage.cs +++ b/src/Renci.SshNet/Messages/Transport/DisconnectMessage.cs @@ -95,5 +95,11 @@ internal override void Process(Session session) { session.OnDisconnectReceived(this); } + + /// + public override string ToString() + { + return $"SSH_MSG_DISCONNECT ({ReasonCode}) {Description}"; + } } } diff --git a/src/Renci.SshNet/Security/KeyExchange.cs b/src/Renci.SshNet/Security/KeyExchange.cs index 44684a92e..47dcb96ba 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -1,9 +1,9 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Security.Cryptography; -using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Compression; using Renci.SshNet.Messages; @@ -72,12 +72,23 @@ public virtual void Start(Session session, KeyExchangeInitMessage message) SendMessage(session.ClientInitMessage); - // Determine encryption algorithm + // Determine client encryption algorithm var clientEncryptionAlgorithmName = (from b in session.ConnectionInfo.Encryptions.Keys from a in message.EncryptionAlgorithmsClientToServer where a == b select a).FirstOrDefault(); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"Encryption client to server: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"Encryption client to server: they offer {string.Join(",", message.EncryptionAlgorithmsClientToServer)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"Encryption client to server: using {clientEncryptionAlgorithmName}", TraceEventType.Information); + } + if (string.IsNullOrEmpty(clientEncryptionAlgorithmName)) { throw new SshConnectionException("Client encryption algorithm not found", DisconnectReason.KeyExchangeFailed); @@ -85,11 +96,23 @@ from a in message.EncryptionAlgorithmsClientToServer session.ConnectionInfo.CurrentClientEncryption = clientEncryptionAlgorithmName; - // Determine encryption algorithm + // Determine server encryption algorithm var serverDecryptionAlgorithmName = (from b in session.ConnectionInfo.Encryptions.Keys from a in message.EncryptionAlgorithmsServerToClient where a == b select a).FirstOrDefault(); + + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"Encryption server to client: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"Encryption server to client: they offer {string.Join(",", message.EncryptionAlgorithmsServerToClient)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"Encryption server to client: using {serverDecryptionAlgorithmName}", TraceEventType.Information); + } + if (string.IsNullOrEmpty(serverDecryptionAlgorithmName)) { throw new SshConnectionException("Server decryption algorithm not found", DisconnectReason.KeyExchangeFailed); @@ -102,6 +125,18 @@ from a in message.EncryptionAlgorithmsServerToClient from a in message.MacAlgorithmsClientToServer where a == b select a).FirstOrDefault(); + + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"MAC client to server: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"MAC client to server: they offer {string.Join(",", message.MacAlgorithmsClientToServer)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"MAC client to server: using {clientHmacAlgorithmName}", TraceEventType.Information); + } + if (string.IsNullOrEmpty(clientHmacAlgorithmName)) { throw new SshConnectionException("Server HMAC algorithm not found", DisconnectReason.KeyExchangeFailed); @@ -114,6 +149,18 @@ from a in message.MacAlgorithmsClientToServer from a in message.MacAlgorithmsServerToClient where a == b select a).FirstOrDefault(); + + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"MAC server to client: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"MAC server to client: they offer {string.Join(",", message.MacAlgorithmsServerToClient)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"MAC server to client: using {serverHmacAlgorithmName}", TraceEventType.Information); + } + if (string.IsNullOrEmpty(serverHmacAlgorithmName)) { throw new SshConnectionException("Server HMAC algorithm not found", DisconnectReason.KeyExchangeFailed); @@ -125,7 +172,19 @@ from a in message.MacAlgorithmsServerToClient var compressionAlgorithmName = (from b in session.ConnectionInfo.CompressionAlgorithms.Keys from a in message.CompressionAlgorithmsClientToServer where a == b - select a).LastOrDefault(); + select a).FirstOrDefault(); + + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"Compression client to server: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"Compression client to server: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"Compression client to server: using {compressionAlgorithmName}", TraceEventType.Information); + } + if (string.IsNullOrEmpty(compressionAlgorithmName)) { throw new SshConnectionException("Compression algorithm not found", DisconnectReason.KeyExchangeFailed); @@ -137,7 +196,19 @@ from a in message.CompressionAlgorithmsClientToServer var decompressionAlgorithmName = (from b in session.ConnectionInfo.CompressionAlgorithms.Keys from a in message.CompressionAlgorithmsServerToClient where a == b - select a).LastOrDefault(); + select a).FirstOrDefault(); + + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"Compression server to client: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"Compression server to client: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"Compression server to client: using {decompressionAlgorithmName}", TraceEventType.Information); + } + if (string.IsNullOrEmpty(decompressionAlgorithmName)) { throw new SshConnectionException("Decompression algorithm not found", DisconnectReason.KeyExchangeFailed); @@ -183,10 +254,6 @@ public Cipher CreateServerCipher() serverKey = GenerateSessionKey(SharedKey, ExchangeHash, serverKey, _serverCipherInfo.KeySize / 8); - DiagnosticAbstraction.Log(string.Format("[{0}] Creating {1} server cipher.", - Session.ToHex(Session.SessionId), - Session.ConnectionInfo.CurrentServerEncryption)); - // Create server cipher return _serverCipherInfo.Cipher(serverKey, serverVector); } @@ -208,10 +275,6 @@ public Cipher CreateClientCipher() clientKey = GenerateSessionKey(SharedKey, ExchangeHash, clientKey, _clientCipherInfo.KeySize / 8); - DiagnosticAbstraction.Log(string.Format("[{0}] Creating {1} client cipher.", - Session.ToHex(Session.SessionId), - Session.ConnectionInfo.CurrentClientEncryption)); - // Create client cipher return _clientCipherInfo.Cipher(clientKey, clientVector); } @@ -232,10 +295,6 @@ public HashAlgorithm CreateServerHash() Hash(GenerateSessionKey(SharedKey, ExchangeHash, 'F', sessionId)), _serverHashInfo.KeySize / 8); - DiagnosticAbstraction.Log(string.Format("[{0}] Creating {1} server hmac algorithm.", - Session.ToHex(Session.SessionId), - Session.ConnectionInfo.CurrentServerHmacAlgorithm)); - return _serverHashInfo.HashAlgorithm(serverKey); } @@ -255,10 +314,6 @@ public HashAlgorithm CreateClientHash() Hash(GenerateSessionKey(SharedKey, ExchangeHash, 'E', sessionId)), _clientHashInfo.KeySize / 8); - DiagnosticAbstraction.Log(string.Format("[{0}] Creating {1} client hmac algorithm.", - Session.ToHex(Session.SessionId), - Session.ConnectionInfo.CurrentClientHmacAlgorithm)); - return _clientHashInfo.HashAlgorithm(clientKey); } @@ -275,10 +330,6 @@ public Compressor CreateCompressor() return null; } - DiagnosticAbstraction.Log(string.Format("[{0}] Creating {1} client compressor.", - Session.ToHex(Session.SessionId), - Session.ConnectionInfo.CurrentClientCompressionAlgorithm)); - var compressor = _compressionType.CreateInstance(); compressor.Init(Session); @@ -299,10 +350,6 @@ public Compressor CreateDecompressor() return null; } - DiagnosticAbstraction.Log(string.Format("[{0}] Creating {1} server decompressor.", - Session.ToHex(Session.SessionId), - Session.ConnectionInfo.CurrentServerCompressionAlgorithm)); - var decompressor = _decompressionType.CreateInstance(); decompressor.Init(Session); diff --git a/src/Renci.SshNet/ServiceFactory.cs b/src/Renci.SshNet/ServiceFactory.cs index af30f11c8..737659304 100644 --- a/src/Renci.SshNet/ServiceFactory.cs +++ b/src/Renci.SshNet/ServiceFactory.cs @@ -1,10 +1,10 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Net.Sockets; using System.Text; -using Renci.SshNet.Abstractions; using Renci.SshNet.Common; using Renci.SshNet.Connection; using Renci.SshNet.Messages.Transport; @@ -104,17 +104,28 @@ public IKeyExchange CreateKeyExchange(IDictionary clientAlgorithms } // find an algorithm that is supported by both client and server - var keyExchangeAlgorithmType = (from c in clientAlgorithms + var keyExchangeAlgorithm = (from c in clientAlgorithms from s in serverAlgorithms where s == c.Key - select c.Value).FirstOrDefault(); + select c).FirstOrDefault(); - if (keyExchangeAlgorithmType is null) + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"Key exchange algorithm: we offer {string.Join(",", clientAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"Key exchange algorithm: they offer {string.Join(",", serverAlgorithms)}", TraceEventType.Verbose); + } + + Diagnostics.Log($"Key exchange algorithm: using {keyExchangeAlgorithm.Key}", TraceEventType.Information); + } + + if (keyExchangeAlgorithm.Key is null) { throw new SshConnectionException("Failed to negotiate key exchange algorithm.", DisconnectReason.KeyExchangeFailed); } - return keyExchangeAlgorithmType.CreateInstance(); + return keyExchangeAlgorithm.Value.CreateInstance(); } /// @@ -170,7 +181,10 @@ public ISftpFileReader CreateSftpFileReader(string fileName, ISftpSession sftpSe fileSize = null; maxPendingReads = defaultMaxPendingReads; - DiagnosticAbstraction.Log(string.Format("Failed to obtain size of file. Allowing maximum {0} pending reads: {1}", maxPendingReads, ex)); + if (Diagnostics.IsEnabled(TraceEventType.Error)) + { + Diagnostics.Log($"Failed to obtain size of file. Allowing maximum {maxPendingReads} pending reads: {ex}", TraceEventType.Error); + } } return sftpSession.CreateFileReader(handle, sftpSession, chunkSize, maxPendingReads, fileSize); diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index c984d3109..8a274fdd1 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Net.Sockets; @@ -621,7 +622,11 @@ public void Connect() ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); ConnectionInfo.ClientVersion = ClientVersion; - DiagnosticAbstraction.Log(string.Format("Server version '{0}'.", serverIdentification)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"Our identification: '{ClientVersion}'.", TraceEventType.Information); + Diagnostics.Log($"Their identification: '{serverIdentification}'.", TraceEventType.Information); + } if (!(serverIdentification.ProtocolVersion.Equals("2.0") || serverIdentification.ProtocolVersion.Equals("1.99"))) { @@ -735,7 +740,11 @@ public async Task ConnectAsync(CancellationToken cancellationToken) ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); ConnectionInfo.ClientVersion = ClientVersion; - DiagnosticAbstraction.Log(string.Format("Server version '{0}'.", serverIdentification)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"Our identification: '{ClientVersion}'.", TraceEventType.Information); + Diagnostics.Log($"Their identification: '{serverIdentification}'.", TraceEventType.Information); + } if (!(serverIdentification.ProtocolVersion.Equals("2.0") || serverIdentification.ProtocolVersion.Equals("1.99"))) { @@ -816,7 +825,10 @@ public async Task ConnectAsync(CancellationToken cancellationToken) /// public void Disconnect() { - DiagnosticAbstraction.Log(string.Format("[{0}] Disconnecting session.", ToHex(SessionId))); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Disconnecting session.", TraceEventType.Information); + } // send SSH_MSG_DISCONNECT message, clear socket read buffer and dispose it Disconnect(DisconnectReason.ByApplication, "Connection terminated by the client."); @@ -1052,7 +1064,10 @@ internal void SendMessage(Message message) WaitOnHandle(_keyExchangeCompletedWaitHandle); } - DiagnosticAbstraction.Log(string.Format("[{0}] Sending message '{1}' to server: '{2}'.", ToHex(SessionId), message.GetType().Name, message)); + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Sending message '{message.GetType().Name}' to server: '{message}'.", TraceEventType.Verbose); + } var paddingMultiplier = _clientCipher is null ? (byte) 8 : Math.Max((byte) 8, _serverCipher.MinimumSize); var packetData = message.GetPacket(paddingMultiplier, _clientCompression); @@ -1157,15 +1172,19 @@ private bool TrySendMessage(Message message) SendMessage(message); return true; } - catch (SshException ex) + catch (Exception ex) { - DiagnosticAbstraction.Log(string.Format("Failure sending message '{0}' to server: '{1}' => {2}", message.GetType().Name, message, ex)); - return false; - } - catch (SocketException ex) - { - DiagnosticAbstraction.Log(string.Format("Failure sending message '{0}' to server: '{1}' => {2}", message.GetType().Name, message, ex)); - return false; + if (Diagnostics.IsEnabled(TraceEventType.Error)) + { + Diagnostics.Log($"Failure sending message '{message.GetType().Name}' to server: '{message}' => {ex}", TraceEventType.Error); + } + + if (ex is SshException or SocketException) + { + return false; + } + + throw; } } @@ -1313,7 +1332,10 @@ private void TrySendDisconnect(DisconnectReason reasonCode, string message) /// message. internal void OnDisconnectReceived(DisconnectMessage message) { - DiagnosticAbstraction.Log(string.Format("[{0}] Disconnect received: {1} {2}.", ToHex(SessionId), message.ReasonCode, message.Description)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Disconnect received: {message.ReasonCode} {message.Description}.", TraceEventType.Information); + } // transition to disconnecting state to avoid throwing exceptions while cleaning up, and to // ensure any exceptions that are raised do not overwrite the SshConnectionException that we @@ -1406,7 +1428,10 @@ internal void OnKeyExchangeInitReceived(KeyExchangeInitMessage message) ConnectionInfo.CurrentKeyExchangeAlgorithm = _keyExchange.Name; - DiagnosticAbstraction.Log(string.Format("[{0}] Performing {1} key exchange.", ToHex(SessionId), ConnectionInfo.CurrentKeyExchangeAlgorithm)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Performing {ConnectionInfo.CurrentKeyExchangeAlgorithm} key exchange.", TraceEventType.Information); + } _keyExchange.HostKeyReceived += KeyExchange_HostKeyReceived; @@ -1711,7 +1736,10 @@ private Message LoadMessage(byte[] data, int offset, int count) var message = _sshMessageFactory.Create(messageType); message.Load(data, offset + 1, count - 1); - DiagnosticAbstraction.Log(string.Format("[{0}] Received message '{1}' from server: '{2}'.", ToHex(SessionId), message.GetType().Name, message)); + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Received message '{message.GetType().Name}' from server: '{message}'.", TraceEventType.Verbose); + } return message; } @@ -1829,7 +1857,10 @@ private void SocketDisconnectAndDispose() { try { - DiagnosticAbstraction.Log(string.Format("[{0}] Shutting down socket.", ToHex(SessionId))); + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Shutting down socket.", TraceEventType.Verbose); + } // 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. @@ -1841,14 +1872,25 @@ private void SocketDisconnectAndDispose() } catch (SocketException ex) { - // TODO: log as warning - DiagnosticAbstraction.Log("Failure shutting down socket: " + ex); + if (Diagnostics.IsEnabled(TraceEventType.Warning)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Failure shutting down socket: " + ex, TraceEventType.Warning); + } } } - DiagnosticAbstraction.Log(string.Format("[{0}] Disposing socket.", ToHex(SessionId))); + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Disposing socket.", TraceEventType.Verbose); + } + _socket.Dispose(); - DiagnosticAbstraction.Log(string.Format("[{0}] Disposed socket.", ToHex(SessionId))); + + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Disposed socket.", TraceEventType.Verbose); + } + _socket = null; } } @@ -1928,7 +1970,10 @@ private void RaiseError(Exception exp) { var connectionException = exp as SshConnectionException; - DiagnosticAbstraction.Log(string.Format("[{0}] Raised exception: {1}", ToHex(SessionId), exp)); + if (Diagnostics.IsEnabled(TraceEventType.Warning)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Raised exception: {exp}", TraceEventType.Warning); + } if (_isDisconnecting) { @@ -1955,7 +2000,11 @@ private void RaiseError(Exception exp) if (connectionException != null) { - DiagnosticAbstraction.Log(string.Format("[{0}] Disconnecting after exception: {1}", ToHex(SessionId), exp)); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Disconnecting after exception: {exp}", TraceEventType.Information); + } + Disconnect(connectionException.DisconnectReason, exp.ToString()); } } @@ -2008,7 +2057,10 @@ protected virtual void Dispose(bool disposing) if (disposing) { - DiagnosticAbstraction.Log(string.Format("[{0}] Disposing session.", ToHex(SessionId))); + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + { + Diagnostics.Log($"[{ToHex(SessionId)}] Disposing session.", TraceEventType.Verbose); + } Disconnect(); diff --git a/src/Renci.SshNet/Sftp/SftpFileReader.cs b/src/Renci.SshNet/Sftp/SftpFileReader.cs index af32f87a7..c20510b74 100644 --- a/src/Renci.SshNet/Sftp/SftpFileReader.cs +++ b/src/Renci.SshNet/Sftp/SftpFileReader.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.Runtime.ExceptionServices; using System.Threading; @@ -278,7 +279,10 @@ private void Dispose(bool disposing) } catch (Exception ex) { - DiagnosticAbstraction.Log("Failure closing handle: " + ex); + if (Diagnostics.IsEnabled(TraceEventType.Error)) + { + Diagnostics.Log("Failure closing handle: " + ex, TraceEventType.Error); + } } } } diff --git a/src/Renci.SshNet/SubsystemSession.cs b/src/Renci.SshNet/SubsystemSession.cs index 70385d903..b6ae7ba45 100644 --- a/src/Renci.SshNet/SubsystemSession.cs +++ b/src/Renci.SshNet/SubsystemSession.cs @@ -1,9 +1,9 @@ using System; +using System.Diagnostics; using System.Globalization; using System.Runtime.ExceptionServices; using System.Threading; -using Renci.SshNet.Abstractions; using Renci.SshNet.Channels; using Renci.SshNet.Common; @@ -191,7 +191,10 @@ protected void RaiseError(Exception error) { _exception = error; - DiagnosticAbstraction.Log("Raised exception: " + error); + if (Diagnostics.IsEnabled(TraceEventType.Warning)) + { + Diagnostics.Log("Raised exception: " + error, TraceEventType.Warning); + } _ = _errorOccuredWaitHandle?.Set(); diff --git a/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs b/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs index d08578e4e..b25eeb612 100644 --- a/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs +++ b/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs @@ -1,7 +1,5 @@ using System.Diagnostics; -using Renci.SshNet.Abstractions; - namespace Renci.SshNet.IntegrationTests.TestsFixtures { /// @@ -88,15 +86,15 @@ protected void CreateTestFile(string fileName, int size) protected void EnableTracing() { - DiagnosticAbstraction.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Verbose)); - DiagnosticAbstraction.Source.Listeners.Remove("Default"); - DiagnosticAbstraction.Source.Listeners.Add(new ConsoleTraceListener() { Name = "TestConsoleLogger" }); + Diagnostics.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Verbose)); + Diagnostics.Source.Listeners.Remove("Default"); + Diagnostics.Source.Listeners.Add(new ConsoleTraceListener() { Name = "TestConsoleLogger" }); } protected void DisableTracing() { - DiagnosticAbstraction.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Off)); - DiagnosticAbstraction.Source.Listeners.Remove("TestConsoleLogger"); + Diagnostics.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Off)); + Diagnostics.Source.Listeners.Remove("TestConsoleLogger"); } } } From fe29a0d7901bea48d3ca5b3473375c630d2c06ed Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Sun, 10 Dec 2023 19:51:26 +0100 Subject: [PATCH 2/3] log SessionId when possible --- src/Renci.SshNet/Security/KeyExchange.cs | 116 +++++++++++++---------- src/Renci.SshNet/Session.cs | 27 +++--- 2 files changed, 81 insertions(+), 62 deletions(-) diff --git a/src/Renci.SshNet/Security/KeyExchange.cs b/src/Renci.SshNet/Security/KeyExchange.cs index 47dcb96ba..bcddaa927 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -72,21 +72,18 @@ public virtual void Start(Session session, KeyExchangeInitMessage message) SendMessage(session.ClientInitMessage); + var sessionId = Diagnostics.IsEnabled(TraceEventType.Verbose) ? Session.ToHex(Session.SessionId) : null; + // Determine client encryption algorithm var clientEncryptionAlgorithmName = (from b in session.ConnectionInfo.Encryptions.Keys from a in message.EncryptionAlgorithmsClientToServer where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"Encryption client to server: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"Encryption client to server: they offer {string.Join(",", message.EncryptionAlgorithmsClientToServer)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"Encryption client to server: using {clientEncryptionAlgorithmName}", TraceEventType.Information); + Diagnostics.Log($"[{sessionId}] Encryption client to server: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"[{sessionId}] Encryption client to server: they offer {string.Join(",", message.EncryptionAlgorithmsClientToServer)}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(clientEncryptionAlgorithmName)) @@ -102,15 +99,10 @@ from a in message.EncryptionAlgorithmsServerToClient where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"Encryption server to client: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"Encryption server to client: they offer {string.Join(",", message.EncryptionAlgorithmsServerToClient)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"Encryption server to client: using {serverDecryptionAlgorithmName}", TraceEventType.Information); + Diagnostics.Log($"[{sessionId}] Encryption server to client: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"[{sessionId}] Encryption server to client: they offer {string.Join(",", message.EncryptionAlgorithmsServerToClient)}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(serverDecryptionAlgorithmName)) @@ -126,15 +118,10 @@ from a in message.MacAlgorithmsClientToServer where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"MAC client to server: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"MAC client to server: they offer {string.Join(",", message.MacAlgorithmsClientToServer)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"MAC client to server: using {clientHmacAlgorithmName}", TraceEventType.Information); + Diagnostics.Log($"[{sessionId}] MAC client to server: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"[{sessionId}] MAC client to server: they offer {string.Join(",", message.MacAlgorithmsClientToServer)}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(clientHmacAlgorithmName)) @@ -150,15 +137,10 @@ from a in message.MacAlgorithmsServerToClient where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"MAC server to client: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"MAC server to client: they offer {string.Join(",", message.MacAlgorithmsServerToClient)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"MAC server to client: using {serverHmacAlgorithmName}", TraceEventType.Information); + Diagnostics.Log($"[{sessionId}] MAC server to client: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"[{sessionId}] MAC server to client: they offer {string.Join(",", message.MacAlgorithmsServerToClient)}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(serverHmacAlgorithmName)) @@ -174,15 +156,10 @@ from a in message.CompressionAlgorithmsClientToServer where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"Compression client to server: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"Compression client to server: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"Compression client to server: using {compressionAlgorithmName}", TraceEventType.Information); + Diagnostics.Log($"[{sessionId}] Compression client to server: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"[{sessionId}] Compression client to server: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(compressionAlgorithmName)) @@ -198,15 +175,10 @@ from a in message.CompressionAlgorithmsServerToClient where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostics.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"Compression server to client: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"Compression server to client: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"Compression server to client: using {decompressionAlgorithmName}", TraceEventType.Information); + Diagnostics.Log($"[{sessionId}] Compression server to client: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); + Diagnostics.Log($"[{sessionId}] Compression server to client: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(decompressionAlgorithmName)) @@ -254,6 +226,14 @@ public Cipher CreateServerCipher() serverKey = GenerateSessionKey(SharedKey, ExchangeHash, serverKey, _serverCipherInfo.KeySize / 8); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log(string.Format("[{0}] Creating {1} server cipher.", + Session.ToHex(Session.SessionId), + Session.ConnectionInfo.CurrentServerEncryption), + TraceEventType.Information); + } + // Create server cipher return _serverCipherInfo.Cipher(serverKey, serverVector); } @@ -275,6 +255,14 @@ public Cipher CreateClientCipher() clientKey = GenerateSessionKey(SharedKey, ExchangeHash, clientKey, _clientCipherInfo.KeySize / 8); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log(string.Format("[{0}] Creating {1} client cipher.", + Session.ToHex(Session.SessionId), + Session.ConnectionInfo.CurrentClientEncryption), + TraceEventType.Information); + } + // Create client cipher return _clientCipherInfo.Cipher(clientKey, clientVector); } @@ -295,6 +283,14 @@ public HashAlgorithm CreateServerHash() Hash(GenerateSessionKey(SharedKey, ExchangeHash, 'F', sessionId)), _serverHashInfo.KeySize / 8); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log(string.Format("[{0}] Creating {1} server hmac algorithm.", + Session.ToHex(Session.SessionId), + Session.ConnectionInfo.CurrentServerHmacAlgorithm), + TraceEventType.Information); + } + return _serverHashInfo.HashAlgorithm(serverKey); } @@ -314,6 +310,14 @@ public HashAlgorithm CreateClientHash() Hash(GenerateSessionKey(SharedKey, ExchangeHash, 'E', sessionId)), _clientHashInfo.KeySize / 8); + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log(string.Format("[{0}] Creating {1} client hmac algorithm.", + Session.ToHex(Session.SessionId), + Session.ConnectionInfo.CurrentClientHmacAlgorithm), + TraceEventType.Information); + } + return _clientHashInfo.HashAlgorithm(clientKey); } @@ -330,6 +334,14 @@ public Compressor CreateCompressor() return null; } + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log(string.Format("[{0}] Creating {1} client compressor.", + Session.ToHex(Session.SessionId), + Session.ConnectionInfo.CurrentClientCompressionAlgorithm), + TraceEventType.Information); + } + var compressor = _compressionType.CreateInstance(); compressor.Init(Session); @@ -350,6 +362,14 @@ public Compressor CreateDecompressor() return null; } + if (Diagnostics.IsEnabled(TraceEventType.Information)) + { + Diagnostics.Log(string.Format("[{0}] Creating {1} server decompressor.", + Session.ToHex(Session.SessionId), + Session.ConnectionInfo.CurrentServerCompressionAlgorithm), + TraceEventType.Information); + } + var decompressor = _decompressionType.CreateInstance(); decompressor.Init(Session); diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 8a274fdd1..15b0c035a 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -4,7 +4,9 @@ using System.Linq; using System.Net.Sockets; using System.Security.Cryptography; +#if !NET using System.Text; +#endif using System.Threading; using System.Threading.Tasks; @@ -1176,7 +1178,7 @@ private bool TrySendMessage(Message message) { if (Diagnostics.IsEnabled(TraceEventType.Error)) { - Diagnostics.Log($"Failure sending message '{message.GetType().Name}' to server: '{message}' => {ex}", TraceEventType.Error); + Diagnostics.Log($"[{ToHex(SessionId)}] Failure sending message '{message.GetType().Name}' to server: '{message}' => {ex}", TraceEventType.Error); } if (ex is SshException or SocketException) @@ -1744,29 +1746,26 @@ private Message LoadMessage(byte[] data, int offset, int count) return message; } - private static string ToHex(byte[] bytes, int offset) + internal static string ToHex(byte[] bytes) { - var byteCount = bytes.Length - offset; + if (bytes is null) + { + return null; + } +#if NET + return Convert.ToHexString(bytes); +#else var builder = new StringBuilder(bytes.Length * 2); - for (var i = offset; i < byteCount; i++) + for (var i = 0; i < bytes.Length; i++) { var b = bytes[i]; _ = builder.Append(b.ToString("X2")); } return builder.ToString(); - } - - internal static string ToHex(byte[] bytes) - { - if (bytes is null) - { - return null; - } - - return ToHex(bytes, 0); +#endif } /// From 2ce4d617918b6e16223687b1001f4941e6788231 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Fri, 26 Jan 2024 20:44:40 +0100 Subject: [PATCH 3/3] Revert .editorconfig changes; appease analyzers; uncondition some logs for clarity --- .editorconfig | 11 --- src/Renci.SshNet/BaseClient.cs | 5 +- src/Renci.SshNet/Channels/Channel.cs | 4 +- .../Channels/ChannelDirectTcpip.cs | 5 +- .../Channels/ChannelForwardedTcpip.cs | 5 +- src/Renci.SshNet/Common/Extensions.cs | 7 ++ src/Renci.SshNet/Connection/ConnectorBase.cs | 10 +-- .../{Diagnostics.cs => Diagnostic.cs} | 2 +- src/Renci.SshNet/ForwardedPortDynamic.cs | 2 +- src/Renci.SshNet/ForwardedPortLocal.cs | 3 +- src/Renci.SshNet/ForwardedPortRemote.cs | 2 +- src/Renci.SshNet/Security/KeyExchange.cs | 74 +++++++++---------- src/Renci.SshNet/ServiceFactory.cs | 18 ++--- src/Renci.SshNet/Session.cs | 73 ++++++------------ src/Renci.SshNet/Sftp/SftpFileReader.cs | 5 +- src/Renci.SshNet/SubsystemSession.cs | 5 +- .../TestsFixtures/IntegrationTestBase.cs | 10 +-- 17 files changed, 94 insertions(+), 147 deletions(-) rename src/Renci.SshNet/{Diagnostics.cs => Diagnostic.cs} (98%) diff --git a/.editorconfig b/.editorconfig index cbd90465c..91bdfb44f 100644 --- a/.editorconfig +++ b/.editorconfig @@ -576,17 +576,6 @@ MA0053.exceptions_should_be_sealed = true # Also report diagnostic for types that define (new) virtual members. MA0053.class_with_virtual_member_shoud_be_sealed = true -# MA0076: Do not use implicit culture-sensitive ToString in interpolated strings -# -# Interpolated strings have performance benefits. -dotnet_diagnostic.MA0076.severity = none - -# MA0089: Optimize string method usage -# -# Duplicate of CA1865-CA1867 / -# For string.Join, results in verbose code for very little gain. -dotnet_diagnostic.MA0089.severity = none - # MA0112: Use 'Count > 0' instead of 'Any()' # # This rule is disabled by default, hence we need to explicitly enable it. diff --git a/src/Renci.SshNet/BaseClient.cs b/src/Renci.SshNet/BaseClient.cs index 731b3e548..25d178f85 100644 --- a/src/Renci.SshNet/BaseClient.cs +++ b/src/Renci.SshNet/BaseClient.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; @@ -316,7 +317,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken) /// The method was called after the client was disposed. public void Disconnect() { - Diagnostics.Log("Disconnecting client.", System.Diagnostics.TraceEventType.Verbose); + Diagnostic.Log("Disconnecting client.", TraceEventType.Verbose); CheckDisposed(); @@ -415,7 +416,7 @@ protected virtual void Dispose(bool disposing) if (disposing) { - Diagnostics.Log("Disposing client.", System.Diagnostics.TraceEventType.Verbose); + Diagnostic.Log("Disposing client.", TraceEventType.Verbose); Disconnect(); diff --git a/src/Renci.SshNet/Channels/Channel.cs b/src/Renci.SshNet/Channels/Channel.cs index 6def1e44a..162f20e73 100644 --- a/src/Renci.SshNet/Channels/Channel.cs +++ b/src/Renci.SshNet/Channels/Channel.cs @@ -554,9 +554,9 @@ protected virtual void Close() // SSH_MSG_CHANNEL_CLOSE as response to a SSH_MSG_CHANNEL_CLOSE sent by the // server var closeWaitResult = _session.TryWait(_channelClosedWaitHandle, ConnectionInfo.ChannelCloseTimeout); - if (closeWaitResult != WaitResult.Success && Diagnostics.IsEnabled(TraceEventType.Warning)) + if (closeWaitResult != WaitResult.Success) { - Diagnostics.Log($"Wait for channel close not successful: {closeWaitResult}.", TraceEventType.Warning); + Diagnostic.Log(string.Format("Wait for channel close not successful: {0:G}.", closeWaitResult), TraceEventType.Warning); } } } diff --git a/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs b/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs index 4e983c708..d60a34885 100644 --- a/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs +++ b/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs @@ -158,10 +158,7 @@ private void ShutdownSocket(SocketShutdown how) } catch (SocketException ex) { - if (Diagnostics.IsEnabled(TraceEventType.Warning)) - { - Diagnostics.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); - } + Diagnostic.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); } } } diff --git a/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs b/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs index 10161bcca..416f3f8b8 100644 --- a/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs +++ b/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs @@ -140,10 +140,7 @@ private void ShutdownSocket(SocketShutdown how) } catch (SocketException ex) { - if (Diagnostics.IsEnabled(TraceEventType.Warning)) - { - Diagnostics.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); - } + Diagnostic.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); } } } diff --git a/src/Renci.SshNet/Common/Extensions.cs b/src/Renci.SshNet/Common/Extensions.cs index ae390d604..35ce537c5 100644 --- a/src/Renci.SshNet/Common/Extensions.cs +++ b/src/Renci.SshNet/Common/Extensions.cs @@ -315,5 +315,12 @@ internal static bool IsConnected(this Socket socket) return socket.Connected; } + + internal static string Join(this IEnumerable values, string separator) + { + // A sly way to prevent analyzers asking to "use an overload with a char parameter" + // which is not available on all targets. + return string.Join(separator, values); + } } } diff --git a/src/Renci.SshNet/Connection/ConnectorBase.cs b/src/Renci.SshNet/Connection/ConnectorBase.cs index 972b1b0fe..0d8949e04 100644 --- a/src/Renci.SshNet/Connection/ConnectorBase.cs +++ b/src/Renci.SshNet/Connection/ConnectorBase.cs @@ -42,10 +42,7 @@ protected Socket SocketConnect(string host, int port, TimeSpan timeout) var ipAddress = Dns.GetHostAddresses(host)[0]; var ep = new IPEndPoint(ipAddress, port); - if (Diagnostics.IsEnabled(TraceEventType.Information)) - { - Diagnostics.Log($"Initiating connection to '{host}:{port}'.", TraceEventType.Information); - } + Diagnostic.Log(string.Format("Initiating connection to '{0}:{1}'.", host, port), TraceEventType.Information); var socket = SocketFactory.Create(ep.AddressFamily, SocketType.Stream, ProtocolType.Tcp); @@ -85,10 +82,7 @@ protected async Task SocketConnectAsync(string host, int port, Cancellat var ep = new IPEndPoint(ipAddress, port); - if (Diagnostics.IsEnabled(TraceEventType.Information)) - { - Diagnostics.Log($"Initiating connection to '{host}:{port}'.", TraceEventType.Information); - } + Diagnostic.Log(string.Format("Initiating connection to '{0}:{1}'.", host, port), TraceEventType.Information); var socket = SocketFactory.Create(ep.AddressFamily, SocketType.Stream, ProtocolType.Tcp); try diff --git a/src/Renci.SshNet/Diagnostics.cs b/src/Renci.SshNet/Diagnostic.cs similarity index 98% rename from src/Renci.SshNet/Diagnostics.cs rename to src/Renci.SshNet/Diagnostic.cs index 7a408973a..e8584981c 100644 --- a/src/Renci.SshNet/Diagnostics.cs +++ b/src/Renci.SshNet/Diagnostic.cs @@ -6,7 +6,7 @@ namespace Renci.SshNet /// /// Provides access to the internals of SSH.NET. /// - public static class Diagnostics + public static class Diagnostic { /// /// The instance used by SSH.NET. diff --git a/src/Renci.SshNet/ForwardedPortDynamic.cs b/src/Renci.SshNet/ForwardedPortDynamic.cs index a5413d0a6..2d354850d 100644 --- a/src/Renci.SshNet/ForwardedPortDynamic.cs +++ b/src/Renci.SshNet/ForwardedPortDynamic.cs @@ -414,7 +414,7 @@ private void InternalStop(TimeSpan timeout) if (!_pendingChannelCountdown.Wait(timeout)) { - Diagnostics.Log("Timeout waiting for pending channels in dynamic forwarded port to close.", TraceEventType.Warning); + Diagnostic.Log("Timeout waiting for pending channels in dynamic forwarded port to close.", TraceEventType.Warning); } } diff --git a/src/Renci.SshNet/ForwardedPortLocal.cs b/src/Renci.SshNet/ForwardedPortLocal.cs index 4907c85a5..e51d205a1 100644 --- a/src/Renci.SshNet/ForwardedPortLocal.cs +++ b/src/Renci.SshNet/ForwardedPortLocal.cs @@ -4,7 +4,6 @@ using System.Net.Sockets; using System.Threading; -using Renci.SshNet.Abstractions; using Renci.SshNet.Common; namespace Renci.SshNet @@ -400,7 +399,7 @@ private void InternalStop(TimeSpan timeout) if (!_pendingChannelCountdown.Wait(timeout)) { - Diagnostics.Log("Timeout waiting for pending channels in local forwarded port to close.", TraceEventType.Warning); + Diagnostic.Log("Timeout waiting for pending channels in local forwarded port to close.", TraceEventType.Warning); } } diff --git a/src/Renci.SshNet/ForwardedPortRemote.cs b/src/Renci.SshNet/ForwardedPortRemote.cs index 5382dd211..112193eef 100644 --- a/src/Renci.SshNet/ForwardedPortRemote.cs +++ b/src/Renci.SshNet/ForwardedPortRemote.cs @@ -214,7 +214,7 @@ protected override void StopPort(TimeSpan timeout) if (!_pendingChannelCountdown.Wait(timeout)) { - Diagnostics.Log("Timeout waiting for pending channels in remote forwarded port to close.", TraceEventType.Warning); + Diagnostic.Log("Timeout waiting for pending channels in remote forwarded port to close.", TraceEventType.Warning); } _status = ForwardedPortStatus.Stopped; diff --git a/src/Renci.SshNet/Security/KeyExchange.cs b/src/Renci.SshNet/Security/KeyExchange.cs index a99e95742..279fb242d 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -71,7 +71,7 @@ public virtual void Start(Session session, KeyExchangeInitMessage message, bool SendMessage(session.ClientInitMessage); } - var sessionId = Diagnostics.IsEnabled(TraceEventType.Verbose) ? Session.ToHex(Session.SessionId) : null; + var sessionId = Session.ToHex(Session.SessionId); // Determine client encryption algorithm var clientEncryptionAlgorithmName = (from b in session.ConnectionInfo.Encryptions.Keys @@ -79,10 +79,10 @@ from a in message.EncryptionAlgorithmsClientToServer where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{sessionId}] Encryption client to server: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"[{sessionId}] Encryption client to server: they offer {string.Join(",", message.EncryptionAlgorithmsClientToServer)}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Encryption client to server: we offer {session.ConnectionInfo.Encryptions.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Encryption client to server: they offer {message.EncryptionAlgorithmsClientToServer.Join(",")}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(clientEncryptionAlgorithmName)) @@ -98,10 +98,10 @@ from a in message.EncryptionAlgorithmsServerToClient where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{sessionId}] Encryption server to client: we offer {string.Join(",", session.ConnectionInfo.Encryptions.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"[{sessionId}] Encryption server to client: they offer {string.Join(",", message.EncryptionAlgorithmsServerToClient)}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Encryption server to client: we offer {session.ConnectionInfo.Encryptions.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Encryption server to client: they offer {message.EncryptionAlgorithmsServerToClient.Join(",")}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(serverDecryptionAlgorithmName)) @@ -117,10 +117,10 @@ from a in message.MacAlgorithmsClientToServer where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{sessionId}] MAC client to server: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"[{sessionId}] MAC client to server: they offer {string.Join(",", message.MacAlgorithmsClientToServer)}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] MAC client to server: we offer {session.ConnectionInfo.HmacAlgorithms.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] MAC client to server: they offer {message.MacAlgorithmsClientToServer.Join(",")}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(clientHmacAlgorithmName)) @@ -136,10 +136,10 @@ from a in message.MacAlgorithmsServerToClient where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{sessionId}] MAC server to client: we offer {string.Join(",", session.ConnectionInfo.HmacAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"[{sessionId}] MAC server to client: they offer {string.Join(",", message.MacAlgorithmsServerToClient)}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] MAC server to client: we offer {session.ConnectionInfo.HmacAlgorithms.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] MAC server to client: they offer {message.MacAlgorithmsServerToClient.Join(",")}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(serverHmacAlgorithmName)) @@ -155,10 +155,10 @@ from a in message.CompressionAlgorithmsClientToServer where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{sessionId}] Compression client to server: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"[{sessionId}] Compression client to server: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Compression client to server: we offer {session.ConnectionInfo.CompressionAlgorithms.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Compression client to server: they offer {message.CompressionAlgorithmsClientToServer.Join(",")}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(compressionAlgorithmName)) @@ -174,10 +174,10 @@ from a in message.CompressionAlgorithmsServerToClient where a == b select a).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{sessionId}] Compression server to client: we offer {string.Join(",", session.ConnectionInfo.CompressionAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"[{sessionId}] Compression server to client: they offer {string.Join(",", message.CompressionAlgorithmsClientToServer)}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Compression server to client: we offer {session.ConnectionInfo.CompressionAlgorithms.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"[{sessionId}] Compression server to client: they offer {message.CompressionAlgorithmsClientToServer.Join(",")}", TraceEventType.Verbose); } if (string.IsNullOrEmpty(decompressionAlgorithmName)) @@ -225,12 +225,12 @@ public Cipher CreateServerCipher() serverKey = GenerateSessionKey(SharedKey, ExchangeHash, serverKey, _serverCipherInfo.KeySize / 8); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log(string.Format("[{0}] Creating {1} server cipher.", + Diagnostic.Log(string.Format("[{0}] Creating {1} server cipher.", Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentServerEncryption), - TraceEventType.Information); + TraceEventType.Information); } // Create server cipher @@ -254,12 +254,12 @@ public Cipher CreateClientCipher() clientKey = GenerateSessionKey(SharedKey, ExchangeHash, clientKey, _clientCipherInfo.KeySize / 8); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log(string.Format("[{0}] Creating {1} client cipher.", + Diagnostic.Log(string.Format("[{0}] Creating {1} client cipher.", Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentClientEncryption), - TraceEventType.Information); + TraceEventType.Information); } // Create client cipher @@ -282,12 +282,12 @@ public HashAlgorithm CreateServerHash() Hash(GenerateSessionKey(SharedKey, ExchangeHash, 'F', sessionId)), _serverHashInfo.KeySize / 8); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log(string.Format("[{0}] Creating {1} server hmac algorithm.", + Diagnostic.Log(string.Format("[{0}] Creating {1} server hmac algorithm.", Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentServerHmacAlgorithm), - TraceEventType.Information); + TraceEventType.Information); } return _serverHashInfo.HashAlgorithm(serverKey); @@ -309,12 +309,12 @@ public HashAlgorithm CreateClientHash() Hash(GenerateSessionKey(SharedKey, ExchangeHash, 'E', sessionId)), _clientHashInfo.KeySize / 8); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log(string.Format("[{0}] Creating {1} client hmac algorithm.", + Diagnostic.Log(string.Format("[{0}] Creating {1} client hmac algorithm.", Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentClientHmacAlgorithm), - TraceEventType.Information); + TraceEventType.Information); } return _clientHashInfo.HashAlgorithm(clientKey); @@ -333,12 +333,12 @@ public Compressor CreateCompressor() return null; } - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log(string.Format("[{0}] Creating {1} client compressor.", + Diagnostic.Log(string.Format("[{0}] Creating {1} client compressor.", Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentClientCompressionAlgorithm), - TraceEventType.Information); + TraceEventType.Information); } var compressor = _compressorFactory(); @@ -361,12 +361,12 @@ public Compressor CreateDecompressor() return null; } - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log(string.Format("[{0}] Creating {1} server decompressor.", + Diagnostic.Log(string.Format("[{0}] Creating {1} server decompressor.", Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentServerCompressionAlgorithm), - TraceEventType.Information); + TraceEventType.Information); } var decompressor = _decompressorFactory(); diff --git a/src/Renci.SshNet/ServiceFactory.cs b/src/Renci.SshNet/ServiceFactory.cs index 5662c3361..89263ab7c 100644 --- a/src/Renci.SshNet/ServiceFactory.cs +++ b/src/Renci.SshNet/ServiceFactory.cs @@ -98,17 +98,14 @@ from s in serverAlgorithms where s == c.Key select c).FirstOrDefault(); - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"Key exchange algorithm: we offer {string.Join(",", clientAlgorithms.Keys)}", TraceEventType.Verbose); - Diagnostics.Log($"Key exchange algorithm: they offer {string.Join(",", serverAlgorithms)}", TraceEventType.Verbose); - } - - Diagnostics.Log($"Key exchange algorithm: using {keyExchangeAlgorithm.Key}", TraceEventType.Information); + Diagnostic.Log($"Key exchange algorithm: we offer {clientAlgorithms.Keys.Join(",")}", TraceEventType.Verbose); + Diagnostic.Log($"Key exchange algorithm: they offer {serverAlgorithms.Join(",")}", TraceEventType.Verbose); } + Diagnostic.Log($"Key exchange algorithm: using {keyExchangeAlgorithm.Key}", TraceEventType.Information); + if (keyExchangeAlgorithm.Value is null) { throw new SshConnectionException("Failed to negotiate key exchange algorithm.", DisconnectReason.KeyExchangeFailed); @@ -170,10 +167,7 @@ public ISftpFileReader CreateSftpFileReader(string fileName, ISftpSession sftpSe fileSize = null; maxPendingReads = defaultMaxPendingReads; - if (Diagnostics.IsEnabled(TraceEventType.Error)) - { - Diagnostics.Log($"Failed to obtain size of file. Allowing maximum {maxPendingReads} pending reads: {ex}", TraceEventType.Error); - } + Diagnostic.Log(string.Format("Failed to obtain size of file. Allowing maximum {0} pending reads: {1}", maxPendingReads, ex), TraceEventType.Error); } return sftpSession.CreateFileReader(handle, sftpSession, chunkSize, maxPendingReads, fileSize); diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 2170a5980..4cf2e75e4 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -619,10 +619,10 @@ public void Connect() ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); ConnectionInfo.ClientVersion = ClientVersion; - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log($"Our identification: '{ClientVersion}'.", TraceEventType.Information); - Diagnostics.Log($"Their identification: '{serverIdentification}'.", TraceEventType.Information); + Diagnostic.Log($"Our identification: '{ClientVersion}'.", TraceEventType.Information); + Diagnostic.Log($"Their identification: '{serverIdentification}'.", TraceEventType.Information); } if (!(serverIdentification.ProtocolVersion.Equals("2.0") || serverIdentification.ProtocolVersion.Equals("1.99"))) @@ -742,10 +742,10 @@ public async Task ConnectAsync(CancellationToken cancellationToken) ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); ConnectionInfo.ClientVersion = ClientVersion; - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log($"Our identification: '{ClientVersion}'.", TraceEventType.Information); - Diagnostics.Log($"Their identification: '{serverIdentification}'.", TraceEventType.Information); + Diagnostic.Log($"Our identification: '{ClientVersion}'.", TraceEventType.Information); + Diagnostic.Log($"Their identification: '{serverIdentification}'.", TraceEventType.Information); } if (!(serverIdentification.ProtocolVersion.Equals("2.0") || serverIdentification.ProtocolVersion.Equals("1.99"))) @@ -832,9 +832,9 @@ public async Task ConnectAsync(CancellationToken cancellationToken) /// public void Disconnect() { - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log($"[{ToHex(SessionId)}] Disconnecting session.", TraceEventType.Information); + Diagnostic.Log(string.Format("[{0}] Disconnecting session.", ToHex(SessionId)), TraceEventType.Information); } // send SSH_MSG_DISCONNECT message, clear socket read buffer and dispose it @@ -1071,9 +1071,9 @@ internal void SendMessage(Message message) WaitOnHandle(_keyExchangeCompletedWaitHandle.WaitHandle); } - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{ToHex(SessionId)}] Sending message '{message.GetType().Name}' to server: '{message}'.", TraceEventType.Verbose); + Diagnostic.Log(string.Format("[{0}] Sending message '{1}' to server: '{2}'.", ToHex(SessionId), message.GetType().Name, message), TraceEventType.Verbose); } var paddingMultiplier = _clientCipher is null ? (byte) 8 : Math.Max((byte) 8, _serverCipher.MinimumSize); @@ -1187,10 +1187,7 @@ private bool TrySendMessage(Message message) } catch (Exception ex) { - if (Diagnostics.IsEnabled(TraceEventType.Error)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Failure sending message '{message.GetType().Name}' to server: '{message}' => {ex}", TraceEventType.Error); - } + Diagnostic.Log(string.Format("Failure sending message '{0}' to server: '{1}' => {2}", message.GetType().Name, message, ex), TraceEventType.Error); if (ex is SshException or SocketException) { @@ -1345,9 +1342,9 @@ private void TrySendDisconnect(DisconnectReason reasonCode, string message) /// message. internal void OnDisconnectReceived(DisconnectMessage message) { - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log($"[{ToHex(SessionId)}] Disconnect received: {message.ReasonCode} {message.Description}.", TraceEventType.Information); + Diagnostic.Log(string.Format("[{0}] Disconnect received: {1} {2}.", ToHex(SessionId), message.ReasonCode, message.Description), TraceEventType.Information); } // transition to disconnecting state to avoid throwing exceptions while cleaning up, and to @@ -1447,9 +1444,9 @@ internal void OnKeyExchangeInitReceived(KeyExchangeInitMessage message) ConnectionInfo.CurrentKeyExchangeAlgorithm = _keyExchange.Name; - if (Diagnostics.IsEnabled(TraceEventType.Information)) + if (Diagnostic.IsEnabled(TraceEventType.Information)) { - Diagnostics.Log($"[{ToHex(SessionId)}] Performing {ConnectionInfo.CurrentKeyExchangeAlgorithm} key exchange.", TraceEventType.Information); + Diagnostic.Log(string.Format("[{0}] Performing {1} key exchange.", ToHex(SessionId), ConnectionInfo.CurrentKeyExchangeAlgorithm), TraceEventType.Information); } _keyExchange.HostKeyReceived += KeyExchange_HostKeyReceived; @@ -1753,9 +1750,9 @@ private Message LoadMessage(byte[] data, int offset, int count) var message = _sshMessageFactory.Create(messageType); message.Load(data, offset + 1, count - 1); - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) + if (Diagnostic.IsEnabled(TraceEventType.Verbose)) { - Diagnostics.Log($"[{ToHex(SessionId)}] Received message '{message.GetType().Name}' from server: '{message}'.", TraceEventType.Verbose); + Diagnostic.Log(string.Format("[{0}] Received message '{1}' from server: '{2}'.", ToHex(SessionId), message.GetType().Name, message), TraceEventType.Verbose); } return message; @@ -1897,10 +1894,7 @@ private void SocketDisconnectAndDispose() { try { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Shutting down socket.", TraceEventType.Verbose); - } + Diagnostic.Log(string.Format("[{0}] Shutting down socket.", ToHex(SessionId)), TraceEventType.Verbose); // 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. @@ -1912,24 +1906,14 @@ private void SocketDisconnectAndDispose() } catch (SocketException ex) { - if (Diagnostics.IsEnabled(TraceEventType.Warning)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Failure shutting down socket: " + ex, TraceEventType.Warning); - } + Diagnostic.Log("Failure shutting down socket: " + ex, TraceEventType.Warning); } } - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Disposing socket.", TraceEventType.Verbose); - } + Diagnostic.Log(string.Format("[{0}] Disposing socket.", ToHex(SessionId)), TraceEventType.Verbose); _socket.Dispose(); - - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Disposed socket.", TraceEventType.Verbose); - } + Diagnostic.Log(string.Format("[{0}] Disposed socket.", ToHex(SessionId)), TraceEventType.Verbose); _socket = null; } @@ -2014,10 +1998,7 @@ private void RaiseError(Exception exp) { var connectionException = exp as SshConnectionException; - if (Diagnostics.IsEnabled(TraceEventType.Warning)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Raised exception: {exp}", TraceEventType.Warning); - } + Diagnostic.Log(string.Format("[{0}] Raised exception: {1}", ToHex(SessionId), exp), TraceEventType.Warning); if (_isDisconnecting) { @@ -2044,10 +2025,7 @@ private void RaiseError(Exception exp) if (connectionException != null) { - if (Diagnostics.IsEnabled(TraceEventType.Information)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Disconnecting after exception: {exp}", TraceEventType.Information); - } + Diagnostic.Log(string.Format("[{0}] Disconnecting after exception: {1}", ToHex(SessionId), exp), TraceEventType.Information); Disconnect(connectionException.DisconnectReason, exp.ToString()); } @@ -2100,10 +2078,7 @@ protected virtual void Dispose(bool disposing) if (disposing) { - if (Diagnostics.IsEnabled(TraceEventType.Verbose)) - { - Diagnostics.Log($"[{ToHex(SessionId)}] Disposing session.", TraceEventType.Verbose); - } + Diagnostic.Log(string.Format("[{0}] Disposing session.", ToHex(SessionId)), TraceEventType.Verbose); Disconnect(); diff --git a/src/Renci.SshNet/Sftp/SftpFileReader.cs b/src/Renci.SshNet/Sftp/SftpFileReader.cs index eb8710f7c..88fdf5af4 100644 --- a/src/Renci.SshNet/Sftp/SftpFileReader.cs +++ b/src/Renci.SshNet/Sftp/SftpFileReader.cs @@ -279,10 +279,7 @@ private void Dispose(bool disposing) } catch (Exception ex) { - if (Diagnostics.IsEnabled(TraceEventType.Error)) - { - Diagnostics.Log("Failure closing handle: " + ex, TraceEventType.Error); - } + Diagnostic.Log("Failure closing handle: " + ex, TraceEventType.Error); } } } diff --git a/src/Renci.SshNet/SubsystemSession.cs b/src/Renci.SshNet/SubsystemSession.cs index b6ae7ba45..2c754a18e 100644 --- a/src/Renci.SshNet/SubsystemSession.cs +++ b/src/Renci.SshNet/SubsystemSession.cs @@ -191,10 +191,7 @@ protected void RaiseError(Exception error) { _exception = error; - if (Diagnostics.IsEnabled(TraceEventType.Warning)) - { - Diagnostics.Log("Raised exception: " + error, TraceEventType.Warning); - } + Diagnostic.Log("Raised exception: " + error, TraceEventType.Warning); _ = _errorOccuredWaitHandle?.Set(); diff --git a/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs b/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs index b25eeb612..dfe8f18bb 100644 --- a/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs +++ b/test/Renci.SshNet.IntegrationTests/TestsFixtures/IntegrationTestBase.cs @@ -86,15 +86,15 @@ protected void CreateTestFile(string fileName, int size) protected void EnableTracing() { - Diagnostics.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Verbose)); - Diagnostics.Source.Listeners.Remove("Default"); - Diagnostics.Source.Listeners.Add(new ConsoleTraceListener() { Name = "TestConsoleLogger" }); + Diagnostic.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Verbose)); + Diagnostic.Source.Listeners.Remove("Default"); + Diagnostic.Source.Listeners.Add(new ConsoleTraceListener() { Name = "TestConsoleLogger" }); } protected void DisableTracing() { - Diagnostics.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Off)); - Diagnostics.Source.Listeners.Remove("TestConsoleLogger"); + Diagnostic.Source.Switch = new SourceSwitch("sourceSwitch", nameof(SourceLevels.Off)); + Diagnostic.Source.Listeners.Remove("TestConsoleLogger"); } } }