From 07d2d8981a09169f8c9115770556f61a437ea3b1 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Thu, 8 Feb 2024 13:03:55 -0600 Subject: [PATCH 1/7] remove Debug log (an alt handling is yet to come) This is phase 1: make it stop creating the Debug log, and just generally getting my head around the flow of the deep transport logic that writes to this log. (Current debug-log calls are either eliminated or temporarily routed to console-writes) The reason these log lines are weird is because they happen in code that isn't specific to a session, so it can't follow the existing paradigm (where each session has its own logger). But some of those logs are still valuable, so phase 2 will be logging them in an alternate way, likely in a way that will only create the logs when absolutely necessary (as opposed to the current impl which can create a ton of empty logs). --- QuickFIXn/AcceptorSocketDescriptor.cs | 6 ++--- QuickFIXn/ClientHandlerThread.cs | 30 +---------------------- QuickFIXn/SessionSettings.cs | 1 - QuickFIXn/SocketReader.cs | 32 +++++++++++++++---------- QuickFIXn/ThreadedSocketAcceptor.cs | 3 --- QuickFIXn/ThreadedSocketReactor.cs | 5 ++-- QuickFIXn/Transport/StreamFactory.cs | 34 +++++++++++++-------------- 7 files changed, 42 insertions(+), 69 deletions(-) diff --git a/QuickFIXn/AcceptorSocketDescriptor.cs b/QuickFIXn/AcceptorSocketDescriptor.cs index e20eec943..39f49ec8a 100644 --- a/QuickFIXn/AcceptorSocketDescriptor.cs +++ b/QuickFIXn/AcceptorSocketDescriptor.cs @@ -26,7 +26,7 @@ public AcceptorSocketDescriptor(IPEndPoint socketEndPoint, SocketSettings socket SocketReactor = new ThreadedSocketReactor(Address, socketSettings, sessionDict, this); } - public void AcceptSession(Session session) + internal void AcceptSession(Session session) { lock (_acceptedSessions) { @@ -39,7 +39,7 @@ public void AcceptSession(Session session) /// /// ID of session to be removed /// true if session removed, false if not found - public bool RemoveSession(SessionID sessionId) + internal bool RemoveSession(SessionID sessionId) { lock (_acceptedSessions) { @@ -47,7 +47,7 @@ public bool RemoveSession(SessionID sessionId) } } - public Dictionary GetAcceptedSessions() + internal Dictionary GetAcceptedSessions() { lock (_acceptedSessions) { diff --git a/QuickFIXn/ClientHandlerThread.cs b/QuickFIXn/ClientHandlerThread.cs index f0a64284b..08c29cba4 100755 --- a/QuickFIXn/ClientHandlerThread.cs +++ b/QuickFIXn/ClientHandlerThread.cs @@ -31,21 +31,10 @@ public ExitedEventArgs(ClientHandlerThread clientHandlerThread) private Thread? _thread = null; private volatile bool _isShutdownRequested = false; private readonly SocketReader _socketReader; - private readonly FileLog _log; internal ClientHandlerThread(TcpClient tcpClient, long clientId, QuickFix.SettingsDictionary settingsDict, SocketSettings socketSettings, AcceptorSocketDescriptor? acceptorDescriptor) { - string debugLogFilePath = "log"; - if (settingsDict.Has(SessionSettings.DEBUG_FILE_LOG_PATH)) - debugLogFilePath = settingsDict.GetString(SessionSettings.DEBUG_FILE_LOG_PATH); - else if (settingsDict.Has(SessionSettings.FILE_LOG_PATH)) - debugLogFilePath = settingsDict.GetString(SessionSettings.FILE_LOG_PATH); - - // FIXME - do something more flexible than hardcoding a filelog - _log = new FileLog(debugLogFilePath, new SessionID( - "ClientHandlerThread", clientId.ToString(), "Debug-" + Guid.NewGuid())); - Id = clientId; _socketReader = new SocketReader(tcpClient, socketSettings, this, acceptorDescriptor); } @@ -58,7 +47,7 @@ public void Start() public void Shutdown(string reason) { - Log("shutdown requested: " + reason); + // TODO - need the reason param? _isShutdownRequested = true; } @@ -85,7 +74,6 @@ private void Run() } } - Log("shutdown"); OnExited(); } @@ -93,21 +81,6 @@ private void OnExited() { Exited?.Invoke(this, new ExitedEventArgs(this)); } - /// FIXME do real logging - public void Log(string s) - { - _log.OnEvent(s); - } - - /// - /// Provide StreamReader with access to the log - /// - /// - internal ILog GetLog() - { - return _log; - } - #region Responder Members public bool Send(string data) @@ -136,7 +109,6 @@ protected virtual void Dispose(bool disposing) if (disposing) { _socketReader.Dispose(); - _log.Dispose(); } _disposed = true; } diff --git a/QuickFIXn/SessionSettings.cs b/QuickFIXn/SessionSettings.cs index 6cb5f49d1..e536d9d47 100755 --- a/QuickFIXn/SessionSettings.cs +++ b/QuickFIXn/SessionSettings.cs @@ -37,7 +37,6 @@ public class SessionSettings public const string SOCKET_CONNECT_PORT = "SocketConnectPort"; public const string RECONNECT_INTERVAL = "ReconnectInterval"; public const string FILE_LOG_PATH = "FileLogPath"; - public const string DEBUG_FILE_LOG_PATH = "DebugFileLogPath"; public const string FILE_STORE_PATH = "FileStorePath"; public const string REFRESH_ON_LOGON = "RefreshOnLogon"; public const string RESET_ON_LOGON = "ResetOnLogon"; diff --git a/QuickFIXn/SocketReader.cs b/QuickFIXn/SocketReader.cs index d12b87b3e..19b769eb1 100755 --- a/QuickFIXn/SocketReader.cs +++ b/QuickFIXn/SocketReader.cs @@ -34,7 +34,7 @@ internal SocketReader( _tcpClient = tcpClient; _responder = responder; _acceptorDescriptor = acceptorDescriptor; - _stream = Transport.StreamFactory.CreateServerStream(tcpClient, settings, responder.GetLog()); + _stream = Transport.StreamFactory.CreateServerStream(tcpClient, settings); } public void Read() @@ -104,7 +104,8 @@ protected virtual int ReadSome(byte[] buffer, int timeoutMilliseconds) return 0; } - if (inner is not null) { + if (inner is not null) + { throw inner; //rethrow SocketException part (which we have exception logic for) } @@ -119,9 +120,9 @@ private void OnMessageFound(string msg) if (_qfSession is null) { _qfSession = Session.LookupSession(Message.GetReverseSessionId(msg)); - if (_qfSession is null || IsAssumedSession(_qfSession.SessionID)) + if (_qfSession is null || IsUnknownSession(_qfSession.SessionID)) { - Log("ERROR: Disconnecting; received message for unknown session: " + msg); + LogSessionEvent("ERROR: Disconnecting; received message for unknown session: " + msg); _qfSession = null; DisconnectClient(); return; @@ -146,7 +147,7 @@ private void OnMessageFound(string msg) } catch (Exception e) { - Log($"Error on Session '{_qfSession.SessionID}': {e}"); + _qfSession.Log.OnEvent($"Error on Session '{_qfSession.SessionID}': {e}"); } } catch (InvalidMessage e) @@ -165,12 +166,13 @@ protected void HandleBadMessage(string msg, Exception e) { if (Fields.MsgType.LOGON.Equals(Message.GetMsgType(msg))) { - Log("ERROR: Invalid LOGON message, disconnecting: " + e.Message); + LogSessionEvent($"ERROR: Invalid LOGON message, disconnecting: {e.Message}"); + // TODO: else session-agnostic log DisconnectClient(); } else { - Log("ERROR: Invalid message: " + e.Message); + LogSessionEvent($"ERROR: Invalid message: {e.Message}"); } } catch (InvalidMessage) @@ -201,7 +203,7 @@ protected void DisconnectClient() Dispose(); } - private bool IsAssumedSession(SessionID sessionId) + private bool IsUnknownSession(SessionID sessionId) { return _acceptorDescriptor is not null && !_acceptorDescriptor.GetAcceptedSessions().Any(kv => kv.Key.Equals(sessionId)); @@ -239,7 +241,7 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) break; } - Log($"SocketReader Error: {reason}"); + LogSessionEvent($"SocketReader Error: {reason}"); if (disconnectNeeded) { @@ -251,12 +253,18 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) } /// - /// FIXME do proper logging + /// Log event if session can be identified (TODO: logging if not specific to a session) /// /// - private void Log(string s) + private void LogSessionEvent(string s) { - _responder.Log(s); + if(_qfSession is not null) + _qfSession.Log.OnEvent(s); + else { + // Can't tie this to a session, need a generic log. + // TODO this is a temp console log until I do something better + Console.WriteLine(s); + } } public int Send(string data) diff --git a/QuickFIXn/ThreadedSocketAcceptor.cs b/QuickFIXn/ThreadedSocketAcceptor.cs index 88e460bd0..4e313644c 100755 --- a/QuickFIXn/ThreadedSocketAcceptor.cs +++ b/QuickFIXn/ThreadedSocketAcceptor.cs @@ -147,11 +147,9 @@ private void StartAcceptingConnections() { lock (_sync) { - // FIXME StartSessionTimer(); foreach (AcceptorSocketDescriptor socketDescriptor in _socketDescriptorForAddress.Values) { socketDescriptor.SocketReactor.Start(); - // FIXME log_.Info("Listening for connections on " + socketDescriptor.getAddress()); } } } @@ -163,7 +161,6 @@ private void StopAcceptingConnections() foreach (AcceptorSocketDescriptor socketDescriptor in _socketDescriptorForAddress.Values) { socketDescriptor.SocketReactor.Shutdown(); - // FIXME log_.Info("No longer accepting connections on " + socketDescriptor.getAddress()); } } } diff --git a/QuickFIXn/ThreadedSocketReactor.cs b/QuickFIXn/ThreadedSocketReactor.cs index 7a676e499..744600e5d 100755 --- a/QuickFIXn/ThreadedSocketReactor.cs +++ b/QuickFIXn/ThreadedSocketReactor.cs @@ -41,6 +41,7 @@ public State ReactorState #endregion + // TODO: internalize. Only used by test. public ThreadedSocketReactor( IPEndPoint serverSocketEndPoint, SocketSettings socketSettings, @@ -137,8 +138,6 @@ public void Run() _clientThreads.Add(t.Id, t); } - // FIXME set the client thread's exception handler here - t.Log("connected"); t.Start(); } else @@ -214,7 +213,7 @@ private void ShutdownClientHandlerThreads() } catch (Exception e) { - t.Log("Error shutting down: " + e.Message); + Log($"Error shutting down: {e.Message}"); } t.Dispose(); } diff --git a/QuickFIXn/Transport/StreamFactory.cs b/QuickFIXn/Transport/StreamFactory.cs index 3e00887c9..8b6c087e9 100644 --- a/QuickFIXn/Transport/StreamFactory.cs +++ b/QuickFIXn/Transport/StreamFactory.cs @@ -109,7 +109,7 @@ public static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings sett Stream stream = new NetworkStream(socket, true); if (settings.UseSSL) - stream = new SslStreamFactory(logger, settings).CreateClientStreamAndAuthenticate(stream); + stream = new SslStreamFactory(settings).CreateClientStreamAndAuthenticate(stream); return stream; } @@ -119,10 +119,9 @@ public static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings sett /// /// The TCP client. /// The socket settings. - /// Logger to use. /// an opened and initiated stream which can be read and written to /// tcp client must be connected in order to get stream;tcpClient - public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings settings, ILog logger) + public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings settings) { if (tcpClient.Connected == false) throw new ArgumentException("tcp client must be connected in order to get stream", nameof(tcpClient)); @@ -130,7 +129,7 @@ public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings sett Stream stream = tcpClient.GetStream(); if (settings.UseSSL) { - stream = new SslStreamFactory(logger, settings).CreateServerStreamAndAuthenticate(stream); + stream = new SslStreamFactory(settings).CreateServerStreamAndAuthenticate(stream); } return stream; @@ -234,14 +233,12 @@ public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings sett /// private sealed class SslStreamFactory { - private readonly ILog _log; private readonly SocketSettings _socketSettings; private const string CLIENT_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.2"; private const string SERVER_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.1"; - public SslStreamFactory(ILog log, SocketSettings settings) + public SslStreamFactory(SocketSettings settings) { - _log = log; _socketSettings = settings; } @@ -272,7 +269,7 @@ public Stream CreateClientStreamAndAuthenticate(Stream innerStream) } catch (System.Security.Authentication.AuthenticationException ex) { - _log.OnEvent("Unable to perform authentication against server: " + ex.GetFullMessage()); + Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); throw; } @@ -313,7 +310,7 @@ public Stream CreateServerStreamAndAuthenticate(Stream innerStream) } catch (System.Security.Authentication.AuthenticationException ex) { - _log.OnEvent("Unable to perform authentication against server: " + ex.GetFullMessage()); + Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); throw; } @@ -383,26 +380,22 @@ private bool VerifyRemoteCertificate( // Validate enhanced key usage if (!ContainsEnhancedKeyUsage(certificate, enhancedKeyUsage)) { if (enhancedKeyUsage == CLIENT_AUTHENTICATION_OID) - _log.OnEvent( - "Remote certificate is not intended for client authentication: It is missing enhanced key usage " + - enhancedKeyUsage); + Log($"Remote certificate is not intended for client authentication: It is missing enhanced key usage {enhancedKeyUsage}"); else - _log.OnEvent( - "Remote certificate is not intended for server authentication: It is missing enhanced key usage " + - enhancedKeyUsage); + Log($"Remote certificate is not intended for server authentication: It is missing enhanced key usage {enhancedKeyUsage}"); return false; } if (string.IsNullOrEmpty(_socketSettings.CACertificatePath)) { - _log.OnEvent("CACertificatePath is not specified"); + Log("CACertificatePath is not specified"); return false; } // If CA Certficiate is specified then validate agains the CA certificate, otherwise it is validated against the installed certificates X509Certificate2? cert = LoadCertificate(_socketSettings.CACertificatePath, null); if (cert is null) { - _log.OnEvent("Remote certificate was not recognized as a valid certificate: " + sslPolicyErrors); + Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); return false; } @@ -427,7 +420,7 @@ private bool VerifyRemoteCertificate( // Any basic authentication check failed, do after checking CA if (sslPolicyErrors != SslPolicyErrors.None) { - _log.OnEvent("Remote certificate was not recognized as a valid certificate: " + sslPolicyErrors); + Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); return false; } @@ -501,6 +494,11 @@ private static bool ContainsEnhancedKeyUsage(X509Certificate certificate, string return null; } + + private void Log(string s) { + // TODO this is just a temp console log until I do something better + Console.WriteLine(s); + } } } } From 45982b131d6e615b129c02e3dca1f7b533f1a04a Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Fri, 31 May 2024 16:33:06 -0500 Subject: [PATCH 2/7] split SSL classes into their own files --- QuickFIXn/SslCertCache.cs | 102 ++++++++ QuickFIXn/SslStreamFactory.cs | 282 +++++++++++++++++++++ QuickFIXn/Transport/StreamFactory.cs | 366 --------------------------- 3 files changed, 384 insertions(+), 366 deletions(-) create mode 100644 QuickFIXn/SslCertCache.cs create mode 100644 QuickFIXn/SslStreamFactory.cs diff --git a/QuickFIXn/SslCertCache.cs b/QuickFIXn/SslCertCache.cs new file mode 100644 index 000000000..93e11fbb0 --- /dev/null +++ b/QuickFIXn/SslCertCache.cs @@ -0,0 +1,102 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.IO; +using System.Security.Cryptography.X509Certificates; + +namespace QuickFix; + +internal static class SslCertCache { + + /// + /// Cache loaded certificates since loading a certificate can be a costly operation + /// + private static readonly Dictionary CertificateCache = new (); + + /// + /// Loads the specified certificate given a path, DistinguishedName or subject name + /// + /// The certificate path or DistinguishedName/subjectname if it should be loaded from the personal certificate store. + /// The certificate password. + /// The specified certificate, or null if no certificate is found + internal static X509Certificate2? LoadCertificate(string name, string? password) + { + // TODO: Change _certificateCache's type to ConcurrentDictionary once we start targeting .NET 4, + // then remove this lock and use GetOrAdd function of concurrent dictionary + // e.g.: certificate = _certificateCache.GetOrAdd(name, (key) => LoadCertificateInner(name, password)); + lock (CertificateCache) + { + if (CertificateCache.TryGetValue(name, out X509Certificate2? certificate)) + return certificate; + + certificate = LoadCertificateInner(name, password); + + if (certificate is not null) + CertificateCache.Add(name, certificate); + + return certificate; + } + } + + /// + /// Perform the actual loading of a certificate + /// + /// The certificate path or DistinguishedName/subjectname if it should be loaded from the personal certificate store. + /// The certificate password. + /// The specified certificate, or null if no certificate is found + private static X509Certificate2? LoadCertificateInner(string name, string? password) + { + X509Certificate2? certificate; + + // If no extension is found try to get from certificate store + if (!File.Exists(name)) + { + certificate = GetCertificateFromStore(name); + } + else { + certificate = password is not null + ? new X509Certificate2(name, password) + : new X509Certificate2(name); + } + return certificate; + } + + /// + /// Gets the certificate from store. + /// + /// See http://msdn.microsoft.com/en-us/library/system.security.cryptography.x509certificates.x509certificate2.aspx for complete example + /// Name of the cert. + /// The cert, or null if not found + private static X509Certificate2? GetCertificateFromStore(string certName) + { + return GetCertificateFromStoreHelper(certName, new X509Store(StoreLocation.LocalMachine)) + ?? GetCertificateFromStoreHelper(certName, new X509Store(StoreLocation.CurrentUser)); + } + + private static X509Certificate2? GetCertificateFromStoreHelper(string certName, X509Store store) + { + try + { + store.Open(OpenFlags.ReadOnly); + + // Place all certificates in an X509Certificate2Collection object. + X509Certificate2Collection certCollection = store.Certificates; + // If using a certificate with a trusted root you do not need to FindByTimeValid, instead: + // currentCerts.Find(X509FindType.FindBySubjectDistinguishedName, certName, true); + X509Certificate2Collection currentCerts = certCollection.Find(X509FindType.FindByTimeValid, DateTime.Now, false); + + currentCerts = currentCerts.Find(certName.Contains("CN=") + ? X509FindType.FindBySubjectDistinguishedName + : X509FindType.FindBySubjectName, certName, false); + + if (currentCerts.Count == 0) + return null; + + return currentCerts[0]; + } + finally + { + store.Close(); + } + } +} diff --git a/QuickFIXn/SslStreamFactory.cs b/QuickFIXn/SslStreamFactory.cs new file mode 100644 index 000000000..ecf1d99d5 --- /dev/null +++ b/QuickFIXn/SslStreamFactory.cs @@ -0,0 +1,282 @@ +#nullable enable +using System; +using System.Diagnostics; +using System.IO; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using QuickFix.Util; + +namespace QuickFix; + +/// +/// The SSLClientStreamFactory is responsible for setting up a SSLStream in either client or server mode +/// +internal sealed class SslStreamFactory +{ + private readonly SocketSettings _socketSettings; + private const string CLIENT_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.2"; + private const string SERVER_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.1"; + + public SslStreamFactory(SocketSettings settings) + { + _socketSettings = settings; + } + + /// + /// Creates a SslStream in client mode and authenticate. + /// + /// The stream to use for the actual (ssl encrypted) communication. + /// a ssl enabled stream + public Stream CreateClientStreamAndAuthenticate(Stream innerStream) + { + SslStream sslStream = new SslStream( + innerStream, + false, + ValidateServerCertificate, +#pragma warning disable CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). + // Per MS docs, this delete /should/ have a nullable return type + SelectLocalCertificate); +#pragma warning restore CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). + + try + { + // Setup secure SSL Communication + X509CertificateCollection clientCertificates = GetClientCertificates(); + sslStream.AuthenticateAsClient(_socketSettings.ServerCommonName, + clientCertificates, + _socketSettings.SslProtocol, + _socketSettings.CheckCertificateRevocation); + } + catch (System.Security.Authentication.AuthenticationException ex) + { + Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); + throw; + } + + return sslStream; + } + + /// + /// Creates a SslStream in server mode and authenticate. + /// + /// The stream to use for the actual (ssl encrypted) communication. + /// a ssl enabled stream + public Stream CreateServerStreamAndAuthenticate(Stream innerStream) + { + SslStream sslStream = new SslStream( + innerStream, + false, + ValidateClientCertificate, +#pragma warning disable CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). + // Per MS docs, this delete /should/ have a nullable return type + SelectLocalCertificate); +#pragma warning restore CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). + + try + { + if (string.IsNullOrEmpty(_socketSettings.CertificatePath)) + throw new Exception($"No server certificate specified, the {SessionSettings.SSL_CERTIFICATE} setting must be configured"); + + // Setup secure SSL Communication + X509Certificate2? serverCertificate = SslCertCache.LoadCertificate(_socketSettings.CertificatePath, _socketSettings.CertificatePassword); + sslStream.AuthenticateAsServer(new SslServerAuthenticationOptions + { + ServerCertificate = serverCertificate, + ClientCertificateRequired = _socketSettings.RequireClientCertificate, + EnabledSslProtocols = _socketSettings.SslProtocol, + CertificateRevocationCheckMode = _socketSettings.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, + EncryptionPolicy = EncryptionPolicy.RequireEncryption + }); + } + catch (System.Security.Authentication.AuthenticationException ex) + { + Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); + throw; + } + + return sslStream; + } + + private X509CertificateCollection GetClientCertificates() + { + var rv = new X509Certificate2Collection(); + if (!string.IsNullOrEmpty(_socketSettings.CertificatePath)) + { + X509Certificate2? clientCert = SslCertCache.LoadCertificate(_socketSettings.CertificatePath, _socketSettings.CertificatePassword); + if (clientCert is not null) + rv.Add(clientCert); + } + + return rv; + } + + /// + /// Perform validation of the servers certificate. (the initiator validates the server/acceptors certificate) + /// (Satisfies interface to delegate System.Net.Security.RemoteCertificateValidationCallback) + /// + /// The sender. + /// The certificate. + /// The chain. + /// The SSL policy errors. + /// true if the certificate should be treated as trusted; otherwise false + private bool ValidateServerCertificate(object sender, X509Certificate? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors) + { + return VerifyRemoteCertificate(certificate, sslPolicyErrors, SERVER_AUTHENTICATION_OID); + } + + /// + /// Perform validation of a a client certificate.(the acceptor validates the client/initiators certificate) + /// (Satisfies interface to delegate System.Net.Security.RemoteCertificateValidationCallback) + /// + /// The sender. + /// The certificate. + /// The chain. + /// The SSL policy errors. + /// true if the certificate should be treated as trusted; otherwise false + private bool ValidateClientCertificate(object sender, X509Certificate? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors) + { + return VerifyRemoteCertificate(certificate, sslPolicyErrors, CLIENT_AUTHENTICATION_OID); + } + + /// + /// Perform certificate validation common for both server and client. + /// + /// The remote certificate to validate. + /// The SSL policy errors supplied by .Net. + /// Enhanced key usage, which the remote computers certificate should contain. + /// true if the certificate should be treated as trusted; otherwise false + private bool VerifyRemoteCertificate( + X509Certificate? certificate, + SslPolicyErrors sslPolicyErrors, + string enhancedKeyUsage) + { + // Accept without looking at if the certificate is valid if validation is disabled + if (_socketSettings.ValidateCertificates == false) + return true; + + if (certificate is null) + return false; + + // Validate enhanced key usage + if (!ContainsEnhancedKeyUsage(certificate, enhancedKeyUsage)) { + if (enhancedKeyUsage == CLIENT_AUTHENTICATION_OID) + Log($"Remote certificate is not intended for client authentication: It is missing enhanced key usage {enhancedKeyUsage}"); + else + Log($"Remote certificate is not intended for server authentication: It is missing enhanced key usage {enhancedKeyUsage}"); + + return false; + } + + if (string.IsNullOrEmpty(_socketSettings.CACertificatePath)) { + Log("CACertificatePath is not specified"); + return false; + } + + // If CA Certficiate is specified then validate agains the CA certificate, otherwise it is validated against the installed certificates + X509Certificate2? cert = SslCertCache.LoadCertificate(_socketSettings.CACertificatePath, null); + if (cert is null) { + Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); + return false; + } + + X509Chain chain0 = new X509Chain(); + chain0.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + // add all your extra certificate chain + + chain0.ChainPolicy.ExtraStore.Add(cert); + chain0.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; + bool isValid = chain0.Build((X509Certificate2)certificate); + + if (isValid) + { + // resets the sslPolicyErrors.RemoteCertificateChainErrors status + sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateChainErrors; + } + else + { + sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; + } + + // Any basic authentication check failed, do after checking CA + if (sslPolicyErrors != SslPolicyErrors.None) + { + Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); + return false; + } + + // No errors found accept the certificate + return true; + } + + /// + /// Check if the given certificate contains the given enhanced key usage Oid + /// + /// X509 certificate + /// the oid to check if it is specified + /// true if the oid is specified as an enhanced key usage; otherwise false + private static bool ContainsEnhancedKeyUsage(X509Certificate certificate, string enhancedKeyOid) + { + X509Certificate2 cert2 = certificate as X509Certificate2 ?? new X509Certificate2(certificate); + + foreach (X509Extension extension in cert2.Extensions) + { + if (extension is X509EnhancedKeyUsageExtension keyUsage) + { + foreach (System.Security.Cryptography.Oid oid in keyUsage.EnhancedKeyUsages) + { + if (oid.Value == enhancedKeyOid) + return true; + } + } + } + + return false; + } + + /// + /// (Satisfies interface to delegate System.Net.Security.LocalCertificateSelectionCallback) + /// + /// + /// + /// + /// + /// + /// + private static X509Certificate? SelectLocalCertificate( + object sender, + string targetHost, + X509CertificateCollection localCertificates, + X509Certificate? remoteCertificate, + string[] acceptableIssuers) + { + // No certificate can be selected if we have no local certificates at all + if (localCertificates.Count <= 0) + return null; + + Debug.Assert(localCertificates is not null && localCertificates.Count > 0); + + //Otherwise we select the first availible certificate as per msdn documentation + // http://msdn.microsoft.com/en-us/library/system.net.security.localcertificateselectioncallback.aspx + if (acceptableIssuers.Length > 0) + { + // Use the first certificate that is from an acceptable issuer. + foreach (X509Certificate certificate in localCertificates) + { + string issuer = certificate.Issuer; + if (Array.IndexOf(acceptableIssuers, issuer) != -1) + return certificate; + } + } + + // Just use any certificate (if there is one) + if (localCertificates.Count > 0) + return localCertificates[0]; + + return null; + } + + private void Log(string s) { + // TODO this is just a temp console log until I do something better + Console.WriteLine(s); + } +} diff --git a/QuickFIXn/Transport/StreamFactory.cs b/QuickFIXn/Transport/StreamFactory.cs index 8b6c087e9..1f4fc1ceb 100644 --- a/QuickFIXn/Transport/StreamFactory.cs +++ b/QuickFIXn/Transport/StreamFactory.cs @@ -134,371 +134,5 @@ public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings sett return stream; } - - /// - /// Cache loaded certificates since loading a certificate can be a costly operation - /// - private static readonly Dictionary CertificateCache = new (); - - /// - /// Loads the specified certificate given a path, DistinguishedName or subject name - /// - /// The certificate path or DistinguishedName/subjectname if it should be loaded from the personal certificate store. - /// The certificate password. - /// The specified certificate, or null if no certificate is found - private static X509Certificate2? LoadCertificate(string name, string? password) - { - // TODO: Change _certificateCache's type to ConcurrentDictionary once we start targeting .NET 4, - // then remove this lock and use GetOrAdd function of concurrent dictionary - // e.g.: certificate = _certificateCache.GetOrAdd(name, (key) => LoadCertificateInner(name, password)); - lock (CertificateCache) - { - if (CertificateCache.TryGetValue(name, out X509Certificate2? certificate)) - return certificate; - - certificate = LoadCertificateInner(name, password); - - if (certificate is not null) - CertificateCache.Add(name, certificate); - - return certificate; - } - } - - /// - /// Perform the actual loading of a certificate - /// - /// The certificate path or DistinguishedName/subjectname if it should be loaded from the personal certificate store. - /// The certificate password. - /// The specified certificate, or null if no certificate is found - private static X509Certificate2? LoadCertificateInner(string name, string? password) - { - X509Certificate2? certificate; - - // If no extension is found try to get from certificate store - if (!File.Exists(name)) - { - certificate = GetCertificateFromStore(name); - } - else { - certificate = password is not null - ? new X509Certificate2(name, password) - : new X509Certificate2(name); - } - return certificate; - } - - /// - /// Gets the certificate from store. - /// - /// See http://msdn.microsoft.com/en-us/library/system.security.cryptography.x509certificates.x509certificate2.aspx for complete example - /// Name of the cert. - /// The cert, or null if not found - private static X509Certificate2? GetCertificateFromStore(string certName) - { - return GetCertificateFromStoreHelper(certName, new X509Store(StoreLocation.LocalMachine)) - ?? GetCertificateFromStoreHelper(certName, new X509Store(StoreLocation.CurrentUser)); - } - - private static X509Certificate2? GetCertificateFromStoreHelper(string certName, X509Store store) - { - try - { - store.Open(OpenFlags.ReadOnly); - - // Place all certificates in an X509Certificate2Collection object. - X509Certificate2Collection certCollection = store.Certificates; - // If using a certificate with a trusted root you do not need to FindByTimeValid, instead: - // currentCerts.Find(X509FindType.FindBySubjectDistinguishedName, certName, true); - X509Certificate2Collection currentCerts = certCollection.Find(X509FindType.FindByTimeValid, DateTime.Now, false); - - currentCerts = currentCerts.Find(certName.Contains("CN=") - ? X509FindType.FindBySubjectDistinguishedName - : X509FindType.FindBySubjectName, certName, false); - - if (currentCerts.Count == 0) - return null; - - return currentCerts[0]; - } - finally - { - store.Close(); - } - } - - - /// - /// The SSLClientStreamFactory is responsible for setting up a SSLStream in either client or server mode - /// - private sealed class SslStreamFactory - { - private readonly SocketSettings _socketSettings; - private const string CLIENT_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.2"; - private const string SERVER_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.1"; - - public SslStreamFactory(SocketSettings settings) - { - _socketSettings = settings; - } - - /// - /// Creates a SslStream in client mode and authenticate. - /// - /// The stream to use for the actual (ssl encrypted) communication. - /// a ssl enabled stream - public Stream CreateClientStreamAndAuthenticate(Stream innerStream) - { - SslStream sslStream = new SslStream( - innerStream, - false, - ValidateServerCertificate, -#pragma warning disable CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). - // Per MS docs, this delete /should/ have a nullable return type - SelectLocalCertificate); -#pragma warning restore CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). - - try - { - // Setup secure SSL Communication - X509CertificateCollection clientCertificates = GetClientCertificates(); - sslStream.AuthenticateAsClient(_socketSettings.ServerCommonName, - clientCertificates, - _socketSettings.SslProtocol, - _socketSettings.CheckCertificateRevocation); - } - catch (System.Security.Authentication.AuthenticationException ex) - { - Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); - throw; - } - - return sslStream; - } - - /// - /// Creates a SslStream in server mode and authenticate. - /// - /// The stream to use for the actual (ssl encrypted) communication. - /// a ssl enabled stream - public Stream CreateServerStreamAndAuthenticate(Stream innerStream) - { - SslStream sslStream = new SslStream( - innerStream, - false, - ValidateClientCertificate, -#pragma warning disable CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). - // Per MS docs, this delete /should/ have a nullable return type - SelectLocalCertificate); -#pragma warning restore CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). - - try - { - if (string.IsNullOrEmpty(_socketSettings.CertificatePath)) - throw new Exception($"No server certificate specified, the {SessionSettings.SSL_CERTIFICATE} setting must be configured"); - - // Setup secure SSL Communication - X509Certificate2? serverCertificate = LoadCertificate(_socketSettings.CertificatePath, _socketSettings.CertificatePassword); - sslStream.AuthenticateAsServer(new SslServerAuthenticationOptions - { - ServerCertificate = serverCertificate, - ClientCertificateRequired = _socketSettings.RequireClientCertificate, - EnabledSslProtocols = _socketSettings.SslProtocol, - CertificateRevocationCheckMode = _socketSettings.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, - EncryptionPolicy = EncryptionPolicy.RequireEncryption - }); - } - catch (System.Security.Authentication.AuthenticationException ex) - { - Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); - throw; - } - - return sslStream; - } - - private X509CertificateCollection GetClientCertificates() - { - var rv = new X509Certificate2Collection(); - if (!string.IsNullOrEmpty(_socketSettings.CertificatePath)) - { - X509Certificate2? clientCert = LoadCertificate(_socketSettings.CertificatePath, _socketSettings.CertificatePassword); - if (clientCert is not null) - rv.Add(clientCert); - } - - return rv; - } - - /// - /// Perform validation of the servers certificate. (the initiator validates the server/acceptors certificate) - /// (Satisfies interface to delegate System.Net.Security.RemoteCertificateValidationCallback) - /// - /// The sender. - /// The certificate. - /// The chain. - /// The SSL policy errors. - /// true if the certificate should be treated as trusted; otherwise false - private bool ValidateServerCertificate(object sender, X509Certificate? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors) - { - return VerifyRemoteCertificate(certificate, sslPolicyErrors, SERVER_AUTHENTICATION_OID); - } - - /// - /// Perform validation of a a client certificate.(the acceptor validates the client/initiators certificate) - /// (Satisfies interface to delegate System.Net.Security.RemoteCertificateValidationCallback) - /// - /// The sender. - /// The certificate. - /// The chain. - /// The SSL policy errors. - /// true if the certificate should be treated as trusted; otherwise false - private bool ValidateClientCertificate(object sender, X509Certificate? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors) - { - return VerifyRemoteCertificate(certificate, sslPolicyErrors, CLIENT_AUTHENTICATION_OID); - } - - /// - /// Perform certificate validation common for both server and client. - /// - /// The remote certificate to validate. - /// The SSL policy errors supplied by .Net. - /// Enhanced key usage, which the remote computers certificate should contain. - /// true if the certificate should be treated as trusted; otherwise false - private bool VerifyRemoteCertificate( - X509Certificate? certificate, - SslPolicyErrors sslPolicyErrors, - string enhancedKeyUsage) - { - // Accept without looking at if the certificate is valid if validation is disabled - if (_socketSettings.ValidateCertificates == false) - return true; - - if (certificate is null) - return false; - - // Validate enhanced key usage - if (!ContainsEnhancedKeyUsage(certificate, enhancedKeyUsage)) { - if (enhancedKeyUsage == CLIENT_AUTHENTICATION_OID) - Log($"Remote certificate is not intended for client authentication: It is missing enhanced key usage {enhancedKeyUsage}"); - else - Log($"Remote certificate is not intended for server authentication: It is missing enhanced key usage {enhancedKeyUsage}"); - - return false; - } - - if (string.IsNullOrEmpty(_socketSettings.CACertificatePath)) { - Log("CACertificatePath is not specified"); - return false; - } - - // If CA Certficiate is specified then validate agains the CA certificate, otherwise it is validated against the installed certificates - X509Certificate2? cert = LoadCertificate(_socketSettings.CACertificatePath, null); - if (cert is null) { - Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); - return false; - } - - X509Chain chain0 = new X509Chain(); - chain0.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - // add all your extra certificate chain - - chain0.ChainPolicy.ExtraStore.Add(cert); - chain0.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; - bool isValid = chain0.Build((X509Certificate2)certificate); - - if (isValid) - { - // resets the sslPolicyErrors.RemoteCertificateChainErrors status - sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateChainErrors; - } - else - { - sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; - } - - // Any basic authentication check failed, do after checking CA - if (sslPolicyErrors != SslPolicyErrors.None) - { - Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); - return false; - } - - // No errors found accept the certificate - return true; - } - - /// - /// Check if the given certificate contains the given enhanced key usage Oid - /// - /// X509 certificate - /// the oid to check if it is specified - /// true if the oid is specified as an enhanced key usage; otherwise false - private static bool ContainsEnhancedKeyUsage(X509Certificate certificate, string enhancedKeyOid) - { - X509Certificate2 cert2 = certificate as X509Certificate2 ?? new X509Certificate2(certificate); - - foreach (X509Extension extension in cert2.Extensions) - { - if (extension is X509EnhancedKeyUsageExtension keyUsage) - { - foreach (System.Security.Cryptography.Oid oid in keyUsage.EnhancedKeyUsages) - { - if (oid.Value == enhancedKeyOid) - return true; - } - } - } - - return false; - } - - /// - /// (Satisfies interface to delegate System.Net.Security.LocalCertificateSelectionCallback) - /// - /// - /// - /// - /// - /// - /// - private static X509Certificate? SelectLocalCertificate( - object sender, - string targetHost, - X509CertificateCollection localCertificates, - X509Certificate? remoteCertificate, - string[] acceptableIssuers) - { - // No certificate can be selected if we have no local certificates at all - if (localCertificates.Count <= 0) - return null; - - Debug.Assert(localCertificates is not null && localCertificates.Count > 0); - - //Otherwise we select the first availible certificate as per msdn documentation - // http://msdn.microsoft.com/en-us/library/system.net.security.localcertificateselectioncallback.aspx - if (acceptableIssuers.Length > 0) - { - // Use the first certificate that is from an acceptable issuer. - foreach (X509Certificate certificate in localCertificates) - { - string issuer = certificate.Issuer; - if (Array.IndexOf(acceptableIssuers, issuer) != -1) - return certificate; - } - } - - // Just use any certificate (if there is one) - if (localCertificates.Count > 0) - return localCertificates[0]; - - return null; - } - - private void Log(string s) { - // TODO this is just a temp console log until I do something better - Console.WriteLine(s); - } - } } } From 5ddb532051bbaca21f5a036d352dc6596107d712 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Thu, 6 Jun 2024 17:59:01 -0500 Subject: [PATCH 3/7] minor cleanups: mostly comments/no logic changes --- QuickFIXn/AcceptorSocketDescriptor.cs | 9 --------- QuickFIXn/SessionID.cs | 16 +--------------- QuickFIXn/SocketInitiatorThread.cs | 6 +++--- QuickFIXn/SocketReader.cs | 17 ++++++++++------- QuickFIXn/SslStreamFactory.cs | 4 ++-- QuickFIXn/ThreadedSocketReactor.cs | 14 +++----------- QuickFIXn/Transport/StreamFactory.cs | 5 ----- 7 files changed, 19 insertions(+), 52 deletions(-) diff --git a/QuickFIXn/AcceptorSocketDescriptor.cs b/QuickFIXn/AcceptorSocketDescriptor.cs index 39f49ec8a..a84611c9b 100644 --- a/QuickFIXn/AcceptorSocketDescriptor.cs +++ b/QuickFIXn/AcceptorSocketDescriptor.cs @@ -6,20 +6,11 @@ namespace QuickFix { internal class AcceptorSocketDescriptor { - #region Properties - public ThreadedSocketReactor SocketReactor { get; } - public IPEndPoint Address { get; } - #endregion - - #region Private Members - private readonly Dictionary _acceptedSessions = new (); - #endregion - public AcceptorSocketDescriptor(IPEndPoint socketEndPoint, SocketSettings socketSettings, QuickFix.SettingsDictionary sessionDict) { Address = socketEndPoint; diff --git a/QuickFIXn/SessionID.cs b/QuickFIXn/SessionID.cs index 55fc2cb98..e6f2198f1 100755 --- a/QuickFIXn/SessionID.cs +++ b/QuickFIXn/SessionID.cs @@ -11,20 +11,12 @@ namespace QuickFix /// public class SessionID { - #region Properties - public string BeginString { get; } - public string SenderCompID { get; } - public string SenderSubID { get; } - public string SenderLocationID { get; } - public string TargetCompID { get; } - public string TargetSubID { get; } - public string TargetLocationID { get; } /// @@ -39,17 +31,11 @@ public class SessionID /// public bool IsFIXT { get; } - #endregion - - #region Public Members + // TODO just make the values nullable, jeez public const string NOT_SET = ""; - #endregion - #region Private Members private readonly string _id; - #endregion - public SessionID(string beginString, string senderCompId, string senderSubId, string senderLocationId, string targetCompId, string targetSubId, string targetLocationId, string? sessionQualifier = NOT_SET) { BeginString = beginString ?? throw new ArgumentNullException(nameof(beginString)); diff --git a/QuickFIXn/SocketInitiatorThread.cs b/QuickFIXn/SocketInitiatorThread.cs index 117204574..acb64a652 100755 --- a/QuickFIXn/SocketInitiatorThread.cs +++ b/QuickFIXn/SocketInitiatorThread.cs @@ -23,7 +23,7 @@ public class SocketInitiatorThread : IResponder private readonly byte[] _readBuffer = new byte[BUF_SIZE]; private readonly Parser _parser = new(); private Stream? _stream; - private CancellationTokenSource _readCancellationTokenSource = new(); + private readonly CancellationTokenSource _readCancellationTokenSource = new(); private readonly IPEndPoint _socketEndPoint; private readonly SocketSettings _socketSettings; private bool _isDisconnectRequested = false; @@ -127,8 +127,8 @@ protected virtual int ReadSome(byte[] buffer, int timeoutMilliseconds) _currentReadTask ??= _stream.ReadAsync(buffer, 0, buffer.Length, _readCancellationTokenSource.Token); if (_currentReadTask.Wait(timeoutMilliseconds)) { - // Dispose/nullify currentReadTask *before* retreiving .Result. - // Accessting .Result can throw an exception, so we need to reset currentReadTask + // Dispose/nullify currentReadTask *before* retrieving .Result. + // Accessing .Result can throw an exception, so we need to reset currentReadTask // first, to set us up for the next read even if an exception is thrown. Task? request = _currentReadTask; _currentReadTask = null; diff --git a/QuickFIXn/SocketReader.cs b/QuickFIXn/SocketReader.cs index 19b769eb1..debfff801 100755 --- a/QuickFIXn/SocketReader.cs +++ b/QuickFIXn/SocketReader.cs @@ -78,8 +78,8 @@ protected virtual int ReadSome(byte[] buffer, int timeoutMilliseconds) _currentReadTask ??= _stream.ReadAsync(buffer, 0, buffer.Length, _readCancellationTokenSource.Token); if (_currentReadTask.Wait(timeoutMilliseconds)) { - // Dispose/nullify currentReadTask *before* retreiving .Result. - // Accessting .Result can throw an exception, so we need to reset currentReadTask + // Dispose/nullify currentReadTask *before* retrieving .Result. + // Accessing .Result can throw an exception, so we need to reset currentReadTask // first, to set us up for the next read even if an exception is thrown. Task? request = _currentReadTask; _currentReadTask = null; @@ -96,7 +96,6 @@ protected virtual int ReadSome(byte[] buffer, int timeoutMilliseconds) catch (AggregateException ex) // Timeout { _currentReadTask = null; - IOException? ioException = ex.InnerException as IOException; SocketException? inner = ioException?.InnerException as SocketException; if (inner is not null && inner.SocketErrorCode == SocketError.TimedOut) { @@ -104,8 +103,7 @@ protected virtual int ReadSome(byte[] buffer, int timeoutMilliseconds) return 0; } - if (inner is not null) - { + if (inner is not null) { throw inner; //rethrow SocketException part (which we have exception logic for) } @@ -122,8 +120,8 @@ private void OnMessageFound(string msg) _qfSession = Session.LookupSession(Message.GetReverseSessionId(msg)); if (_qfSession is null || IsUnknownSession(_qfSession.SessionID)) { - LogSessionEvent("ERROR: Disconnecting; received message for unknown session: " + msg); _qfSession = null; + LogSessionEvent("ERROR: Disconnecting; received message for unknown session: " + msg); DisconnectClient(); return; } @@ -150,6 +148,11 @@ private void OnMessageFound(string msg) _qfSession.Log.OnEvent($"Error on Session '{_qfSession.SessionID}': {e}"); } } + /* + * TODO: Are these catches reachable? I don't think they are! + * The only line that could throw them is _qfSession.Next above, + * but it has its own catch. + */ catch (InvalidMessage e) { HandleBadMessage(msg, e); @@ -253,7 +256,7 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) } /// - /// Log event if session can be identified (TODO: logging if not specific to a session) + /// Log event to session if known, else do... TODO /// /// private void LogSessionEvent(string s) diff --git a/QuickFIXn/SslStreamFactory.cs b/QuickFIXn/SslStreamFactory.cs index ecf1d99d5..f384ceb39 100644 --- a/QuickFIXn/SslStreamFactory.cs +++ b/QuickFIXn/SslStreamFactory.cs @@ -34,7 +34,7 @@ public Stream CreateClientStreamAndAuthenticate(Stream innerStream) false, ValidateServerCertificate, #pragma warning disable CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). - // Per MS docs, this delete /should/ have a nullable return type + // Per MS docs, this delegate /should/ have a nullable return type SelectLocalCertificate); #pragma warning restore CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). @@ -68,7 +68,7 @@ public Stream CreateServerStreamAndAuthenticate(Stream innerStream) false, ValidateClientCertificate, #pragma warning disable CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). - // Per MS docs, this delete /should/ have a nullable return type + // Per MS docs, this delegate /should/ have a nullable return type SelectLocalCertificate); #pragma warning restore CS8621 // Nullability of reference types in return type doesn't match the target delegate (possibly because of nullability attributes). diff --git a/QuickFIXn/ThreadedSocketReactor.cs b/QuickFIXn/ThreadedSocketReactor.cs index 744600e5d..b3fb4feaa 100755 --- a/QuickFIXn/ThreadedSocketReactor.cs +++ b/QuickFIXn/ThreadedSocketReactor.cs @@ -17,17 +17,11 @@ public class ThreadedSocketReactor { public enum State { RUNNING, SHUTDOWN_REQUESTED, SHUTDOWN_COMPLETE } - #region Properties - public State ReactorState { get { lock (_sync) { return _state; } } } - #endregion - - #region Private Members - private readonly object _sync = new (); private State _state = State.RUNNING; private long _nextClientId = 0; @@ -35,24 +29,22 @@ public State ReactorState private readonly Dictionary _clientThreads = new (); private readonly TcpListener _tcpListener; private readonly SocketSettings _socketSettings; - private readonly QuickFix.SettingsDictionary _sessionDict; + private readonly SettingsDictionary _sessionDict; private readonly IPEndPoint _serverSocketEndPoint; private readonly AcceptorSocketDescriptor? _acceptorSocketDescriptor; - #endregion - // TODO: internalize. Only used by test. public ThreadedSocketReactor( IPEndPoint serverSocketEndPoint, SocketSettings socketSettings, - QuickFix.SettingsDictionary sessionDict + SettingsDictionary sessionDict ) : this(serverSocketEndPoint, socketSettings, sessionDict, null) { } internal ThreadedSocketReactor( IPEndPoint serverSocketEndPoint, SocketSettings socketSettings, - QuickFix.SettingsDictionary sessionDict, + SettingsDictionary sessionDict, AcceptorSocketDescriptor? acceptorSocketDescriptor) { _socketSettings = socketSettings; diff --git a/QuickFIXn/Transport/StreamFactory.cs b/QuickFIXn/Transport/StreamFactory.cs index 1f4fc1ceb..cb732f0c6 100644 --- a/QuickFIXn/Transport/StreamFactory.cs +++ b/QuickFIXn/Transport/StreamFactory.cs @@ -1,16 +1,11 @@ #nullable enable using System; -using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Linq; using System.Net; -using System.Net.Security; using System.Net.Sockets; using System.Text; -using System.Security.Cryptography.X509Certificates; using QuickFix.Logger; -using QuickFix.Util; namespace QuickFix.Transport { From e8bd11c0d4b26e99ed8b741af5bc98f0ead1d3e3 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Thu, 6 Jun 2024 22:34:12 -0500 Subject: [PATCH 4/7] delete namesp prefix: QuickFix.SettingisDictionary no longer needed after I renamed QuickFix.Dictionary to QuickFix.SettingsDictionary This commit makes no other changes to these files --- QuickFIXn/SessionFactory.cs | 6 +++--- QuickFIXn/SessionSettings.cs | 14 +++++++------- QuickFIXn/Settings.cs | 12 ++++++------ QuickFIXn/Transport/SocketInitiator.cs | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/QuickFIXn/SessionFactory.cs b/QuickFIXn/SessionFactory.cs index ea8bd5a67..0323a5d7a 100755 --- a/QuickFIXn/SessionFactory.cs +++ b/QuickFIXn/SessionFactory.cs @@ -36,7 +36,7 @@ public SessionFactory( _messageFactory = messageFactory ?? new DefaultMessageFactory(); } - private static bool DetectIfInitiator(QuickFix.SettingsDictionary settings) + private static bool DetectIfInitiator(SettingsDictionary settings) { switch (settings.GetString(SessionSettings.CONNECTION_TYPE)) { @@ -46,7 +46,7 @@ private static bool DetectIfInitiator(QuickFix.SettingsDictionary settings) throw new ConfigError("Invalid ConnectionType"); } - public Session Create(SessionID sessionId, QuickFix.SettingsDictionary settings) + public Session Create(SessionID sessionId, SettingsDictionary settings) { bool isInitiator = SessionFactory.DetectIfInitiator(settings); @@ -161,7 +161,7 @@ public Session Create(SessionID sessionId, QuickFix.SettingsDictionary settings) return session; } - protected DataDictionary.DataDictionary CreateDataDictionary(SessionID sessionId, QuickFix.SettingsDictionary settings, string settingsKey, string beginString) + protected DataDictionary.DataDictionary CreateDataDictionary(SessionID sessionId, SettingsDictionary settings, string settingsKey, string beginString) { string path; if (settings.Has(settingsKey)) diff --git a/QuickFIXn/SessionSettings.cs b/QuickFIXn/SessionSettings.cs index e536d9d47..fdddfc395 100755 --- a/QuickFIXn/SessionSettings.cs +++ b/QuickFIXn/SessionSettings.cs @@ -162,7 +162,7 @@ public bool Has(SessionID sessionId) /// Get global default settings /// /// Dictionary of settings from the [DEFAULT] section - public QuickFix.SettingsDictionary Get() + public SettingsDictionary Get() { return _defaults; } @@ -179,10 +179,10 @@ public SettingsDictionary Get(SessionID sessionId) return dict; } - public void Set(QuickFix.SettingsDictionary defaults) + public void Set(SettingsDictionary defaults) { _defaults = defaults; - foreach (KeyValuePair entry in _settings) + foreach (KeyValuePair entry in _settings) entry.Value.Merge(_defaults); } @@ -201,7 +201,7 @@ public bool Remove(SessionID sessionId) /// /// ID of session for which to add config /// session config - public void Set(SessionID sessionId, QuickFix.SettingsDictionary settings) + public void Set(SessionID sessionId, SettingsDictionary settings) { if (Has(sessionId)) throw new ConfigError($"Duplicate Session {sessionId}"); @@ -224,7 +224,7 @@ public void Set(SessionID sessionId, QuickFix.SettingsDictionary settings) public HashSet GetSessions() { HashSet result = new HashSet(); - foreach (KeyValuePair entry in _settings) + foreach (KeyValuePair entry in _settings) result.Add(entry.Key); return result; } @@ -237,7 +237,7 @@ public override string ToString() foreach (System.Collections.Generic.KeyValuePair entry in _defaults) s.Append(entry.Key).Append('=').AppendLine(entry.Value); - foreach (KeyValuePair entry in _settings) + foreach (KeyValuePair entry in _settings) { s.AppendLine().AppendLine("[SESSION]"); foreach (System.Collections.Generic.KeyValuePair kvp in entry.Value) @@ -251,7 +251,7 @@ public override string ToString() return s.ToString(); } - protected void Validate(QuickFix.SettingsDictionary settingsDictionary) + protected void Validate(SettingsDictionary settingsDictionary) { string beginString = settingsDictionary.GetString(BEGINSTRING); if (beginString != Values.BeginString_FIX40 && diff --git a/QuickFIXn/Settings.cs b/QuickFIXn/Settings.cs index 1175ec2f3..eeaf2878b 100755 --- a/QuickFIXn/Settings.cs +++ b/QuickFIXn/Settings.cs @@ -5,11 +5,11 @@ namespace QuickFix { public class Settings { - private readonly LinkedList _sections = new(); + private readonly LinkedList _sections = new(); public Settings(System.IO.TextReader conf) { - QuickFix.SettingsDictionary? currentSection = null; + SettingsDictionary? currentSection = null; string? line; while ((line = conf.ReadLine()) != null) @@ -61,7 +61,7 @@ public static bool IsSection(string s) return s[0] == '[' && s[^1] == ']'; } - public QuickFix.SettingsDictionary Add(QuickFix.SettingsDictionary section) + public SettingsDictionary Add(SettingsDictionary section) { _sections.AddLast(section); return section; @@ -73,10 +73,10 @@ public QuickFix.SettingsDictionary Add(QuickFix.SettingsDictionary section) /// /// (case is ignored) /// - public LinkedList Get(string sectionName) + public LinkedList Get(string sectionName) { - LinkedList result = new(); - foreach (QuickFix.SettingsDictionary dict in _sections) + LinkedList result = new(); + foreach (SettingsDictionary dict in _sections) if (sectionName.ToUpperInvariant() == dict.Name.ToUpperInvariant()) result.AddLast(dict); return result; diff --git a/QuickFIXn/Transport/SocketInitiator.cs b/QuickFIXn/Transport/SocketInitiator.cs index d8e24a430..995a3264e 100644 --- a/QuickFIXn/Transport/SocketInitiator.cs +++ b/QuickFIXn/Transport/SocketInitiator.cs @@ -138,7 +138,7 @@ private void RemoveThread(SessionID sessionId) } } - private IPEndPoint GetNextSocketEndPoint(SessionID sessionId, QuickFix.SettingsDictionary settings) + private IPEndPoint GetNextSocketEndPoint(SessionID sessionId, SettingsDictionary settings) { if (!_sessionToHostNum.TryGetValue(sessionId, out var num)) num = 0; From e9abea3e9a2720646345a61ab0a6187306f8dc13 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Fri, 7 Jun 2024 11:11:25 -0500 Subject: [PATCH 5/7] new NonSessionLog Initialized and passed through the stream/socket hierarchy so that logging can be performed for messages that cannot be tied to a session --- QuickFIXn/AbstractInitiator.cs | 5 +-- QuickFIXn/AcceptorSocketDescriptor.cs | 9 +++- QuickFIXn/ClientHandlerThread.cs | 12 +++-- QuickFIXn/IInitiator.cs | 2 +- QuickFIXn/Logger/CompositeLogFactory.cs | 7 ++- QuickFIXn/Logger/FileLogFactory.cs | 12 ++--- QuickFIXn/Logger/ILogFactory.cs | 15 ++++++- QuickFIXn/Logger/NonSessionLog.cs | 28 ++++++++++++ QuickFIXn/Logger/NullLogFactory.cs | 5 +++ QuickFIXn/Logger/ScreenLogFactory.cs | 6 +-- QuickFIXn/SocketInitiatorThread.cs | 12 ++++- QuickFIXn/SocketReader.cs | 28 ++++++------ QuickFIXn/SocketSettings.cs | 2 +- QuickFIXn/ThreadedSocketAcceptor.cs | 13 +++--- QuickFIXn/ThreadedSocketReactor.cs | 40 +++++++---------- QuickFIXn/Transport/SocketInitiator.cs | 3 +- QuickFIXn/{ => Transport}/SslCertCache.cs | 45 ++++++++++++------- QuickFIXn/{ => Transport}/SslStreamFactory.cs | 42 ++++++++--------- QuickFIXn/Transport/StreamFactory.cs | 13 +++--- UnitTests/ThreadedSocketReactorTests.cs | 29 +++++------- 20 files changed, 198 insertions(+), 130 deletions(-) create mode 100644 QuickFIXn/Logger/NonSessionLog.cs rename QuickFIXn/{ => Transport}/SslCertCache.cs (71%) rename QuickFIXn/{ => Transport}/SslStreamFactory.cs (87%) diff --git a/QuickFIXn/AbstractInitiator.cs b/QuickFIXn/AbstractInitiator.cs index fda3f2b35..ecf9f924e 100644 --- a/QuickFIXn/AbstractInitiator.cs +++ b/QuickFIXn/AbstractInitiator.cs @@ -21,12 +21,10 @@ public abstract class AbstractInitiator : IInitiator private readonly SessionFactory _sessionFactory; private Thread? _thread; - #region Properties + protected readonly NonSessionLog _nonSessionLog; public bool IsStopped { get; private set; } = true; - #endregion - protected AbstractInitiator( IApplication app, IMessageStoreFactory storeFactory, @@ -38,6 +36,7 @@ protected AbstractInitiator( var logFactory = logFactoryNullable ?? new NullLogFactory(); var msgFactory = messageFactoryNullable ?? new DefaultMessageFactory(); _sessionFactory = new SessionFactory(app, storeFactory, logFactory, msgFactory); + _nonSessionLog = new NonSessionLog(logFactory); HashSet definedSessions = _settings.GetSessions(); if (0 == definedSessions.Count) diff --git a/QuickFIXn/AcceptorSocketDescriptor.cs b/QuickFIXn/AcceptorSocketDescriptor.cs index a84611c9b..47bf92810 100644 --- a/QuickFIXn/AcceptorSocketDescriptor.cs +++ b/QuickFIXn/AcceptorSocketDescriptor.cs @@ -1,6 +1,7 @@ #nullable enable using System.Collections.Generic; using System.Net; +using QuickFix.Logger; namespace QuickFix { @@ -11,10 +12,14 @@ internal class AcceptorSocketDescriptor private readonly Dictionary _acceptedSessions = new (); - public AcceptorSocketDescriptor(IPEndPoint socketEndPoint, SocketSettings socketSettings, QuickFix.SettingsDictionary sessionDict) + public AcceptorSocketDescriptor( + IPEndPoint socketEndPoint, + SocketSettings socketSettings, + SettingsDictionary sessionDict, + NonSessionLog nonSessionLog) { Address = socketEndPoint; - SocketReactor = new ThreadedSocketReactor(Address, socketSettings, sessionDict, this); + SocketReactor = new ThreadedSocketReactor(Address, socketSettings, sessionDict, this, nonSessionLog); } internal void AcceptSession(Session session) diff --git a/QuickFIXn/ClientHandlerThread.cs b/QuickFIXn/ClientHandlerThread.cs index 08c29cba4..042bfd4ed 100755 --- a/QuickFIXn/ClientHandlerThread.cs +++ b/QuickFIXn/ClientHandlerThread.cs @@ -32,11 +32,15 @@ public ExitedEventArgs(ClientHandlerThread clientHandlerThread) private volatile bool _isShutdownRequested = false; private readonly SocketReader _socketReader; - internal ClientHandlerThread(TcpClient tcpClient, long clientId, QuickFix.SettingsDictionary settingsDict, - SocketSettings socketSettings, AcceptorSocketDescriptor? acceptorDescriptor) - { + internal ClientHandlerThread( + TcpClient tcpClient, + long clientId, + SocketSettings socketSettings, + AcceptorSocketDescriptor? acceptorDescriptor, + NonSessionLog nonSessionLog + ) { Id = clientId; - _socketReader = new SocketReader(tcpClient, socketSettings, this, acceptorDescriptor); + _socketReader = new SocketReader(tcpClient, socketSettings, this, acceptorDescriptor, nonSessionLog); } public void Start() diff --git a/QuickFIXn/IInitiator.cs b/QuickFIXn/IInitiator.cs index 873f8d4d3..6ad70d3e0 100644 --- a/QuickFIXn/IInitiator.cs +++ b/QuickFIXn/IInitiator.cs @@ -48,7 +48,7 @@ public interface IInitiator : IDisposable /// ID of session to be added /// session settings /// true if session added successfully, false if session already exists or is not an initiator - bool AddSession(SessionID sessionID, QuickFix.SettingsDictionary dict); + bool AddSession(SessionID sessionID, SettingsDictionary dict); /// /// Remove an existing session after initiator has been started diff --git a/QuickFIXn/Logger/CompositeLogFactory.cs b/QuickFIXn/Logger/CompositeLogFactory.cs index f63c1d6fa..68180a805 100644 --- a/QuickFIXn/Logger/CompositeLogFactory.cs +++ b/QuickFIXn/Logger/CompositeLogFactory.cs @@ -4,7 +4,8 @@ namespace QuickFix.Logger; /// -/// Allows multiple log factories to be used with QuickFIX/N. For example, you could log events to the console and also log all events and messages to a file. +/// Allows multiple log factories to be used with QuickFIX/N. +/// For example, you could log events to the console and also log all events and messages to a file. /// public class CompositeLogFactory : ILogFactory { @@ -24,4 +25,8 @@ public ILog Create(SessionID sessionID) { return new CompositeLog(_factories.Select(f => f.Create(sessionID)).ToArray()); } + + public ILog CreateNonSessionLog() { + return new CompositeLog(_factories.Select(f => f.Create(new SessionID("Non", "Session", "Log"))).ToArray()); + } } diff --git a/QuickFIXn/Logger/FileLogFactory.cs b/QuickFIXn/Logger/FileLogFactory.cs index 9b71e7ead..ecaed22f4 100755 --- a/QuickFIXn/Logger/FileLogFactory.cs +++ b/QuickFIXn/Logger/FileLogFactory.cs @@ -9,22 +9,24 @@ public class FileLogFactory : ILogFactory { private readonly SessionSettings _settings; - #region LogFactory Members - public FileLogFactory(SessionSettings settings) { _settings = settings; } /// - /// Creates a file-based message store + /// Creates a file-based message log /// - /// session ID for the message store + /// session ID for the message log /// public ILog Create(SessionID sessionId) { return new FileLog(_settings.Get(sessionId).GetString(SessionSettings.FILE_LOG_PATH), sessionId); } - #endregion + public ILog CreateNonSessionLog() { + return new FileLog( + _settings.Get().GetString(SessionSettings.FILE_LOG_PATH), + new SessionID("Non", "Session", "Log")); + } } diff --git a/QuickFIXn/Logger/ILogFactory.cs b/QuickFIXn/Logger/ILogFactory.cs index c2ddb4376..5bb364779 100755 --- a/QuickFIXn/Logger/ILogFactory.cs +++ b/QuickFIXn/Logger/ILogFactory.cs @@ -3,14 +3,25 @@ namespace QuickFix.Logger; /// -/// Used by a session to create a log implementation +/// Creates a log instance /// public interface ILogFactory { /// - /// Create a log implementation + /// Create a log instance for a session /// /// session ID usually used for configuration access /// ILog Create(SessionID sessionId); + + /// + /// Create a log instance that is not tied to a session. + /// This log will + /// (1) only be used for messages that cannot be linked to a session + /// (2) only have its OnEvent() method called + /// (3) only be created when the first message is logged (to avoid e.g. empty log files) + /// This log is written to only on rare occasions. It's possible you may never see it created. + /// + /// + ILog CreateNonSessionLog(); } diff --git a/QuickFIXn/Logger/NonSessionLog.cs b/QuickFIXn/Logger/NonSessionLog.cs new file mode 100644 index 000000000..5e6953e22 --- /dev/null +++ b/QuickFIXn/Logger/NonSessionLog.cs @@ -0,0 +1,28 @@ +#nullable enable +using System; + +namespace QuickFix.Logger; + +/// +/// A logger that can be used when the calling logic cannot identify a session (which is rare). +/// Does not create a file until first write. +/// +public class NonSessionLog { + + private readonly ILogFactory _logFactory; + private ILog? _log; + + private readonly object _sync = new(); + + internal NonSessionLog(ILogFactory logFactory) { + _logFactory = logFactory; + } + + internal void OnEvent(string s) { + lock (_sync) { + _log ??= _logFactory.CreateNonSessionLog(); + } + _log.OnEvent(s); + } +} + diff --git a/QuickFIXn/Logger/NullLogFactory.cs b/QuickFIXn/Logger/NullLogFactory.cs index 90b65025e..5649b3ed0 100644 --- a/QuickFIXn/Logger/NullLogFactory.cs +++ b/QuickFIXn/Logger/NullLogFactory.cs @@ -10,4 +10,9 @@ public ILog Create(SessionID _x) { return new NullLog(); } + + public ILog CreateNonSessionLog() + { + return new NullLog(); + } } diff --git a/QuickFIXn/Logger/ScreenLogFactory.cs b/QuickFIXn/Logger/ScreenLogFactory.cs index 04ce904f7..cd67bd7e2 100755 --- a/QuickFIXn/Logger/ScreenLogFactory.cs +++ b/QuickFIXn/Logger/ScreenLogFactory.cs @@ -28,8 +28,6 @@ public ScreenLogFactory(bool logIncoming, bool logOutgoing, bool logEvent) _settings = new SessionSettings(); } - #region LogFactory Members - public ILog Create(SessionID sessionId) { bool logIncoming = _logIncoming; bool logOutgoing = _logOutgoing; @@ -47,5 +45,7 @@ public ILog Create(SessionID sessionId) { return new ScreenLog(logIncoming, logOutgoing, logEvent); } - #endregion + public ILog CreateNonSessionLog() { + return new ScreenLog(true, true, true); + } } diff --git a/QuickFIXn/SocketInitiatorThread.cs b/QuickFIXn/SocketInitiatorThread.cs index acb64a652..a1bc1b794 100755 --- a/QuickFIXn/SocketInitiatorThread.cs +++ b/QuickFIXn/SocketInitiatorThread.cs @@ -6,6 +6,7 @@ using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; +using QuickFix.Logger; namespace QuickFix { @@ -27,18 +28,25 @@ public class SocketInitiatorThread : IResponder private readonly IPEndPoint _socketEndPoint; private readonly SocketSettings _socketSettings; private bool _isDisconnectRequested = false; + private readonly NonSessionLog _nonSessionLog; /// /// Keep a task for handling async read /// private Task? _currentReadTask; - public SocketInitiatorThread(Transport.SocketInitiator initiator, Session session, IPEndPoint socketEndPoint, SocketSettings socketSettings) + public SocketInitiatorThread( + Transport.SocketInitiator initiator, + Session session, + IPEndPoint socketEndPoint, + SocketSettings socketSettings, + NonSessionLog nonSessionLog) { Initiator = initiator; Session = session; _socketEndPoint = socketEndPoint; _socketSettings = socketSettings; + _nonSessionLog = nonSessionLog; } public void Start() @@ -74,7 +82,7 @@ public void Connect() /// Stream representing the (network)connection to the other party protected virtual Stream SetupStream() { - return Transport.StreamFactory.CreateClientStream(_socketEndPoint, _socketSettings, Session.Log); + return Transport.StreamFactory.CreateClientStream(_socketEndPoint, _socketSettings, _nonSessionLog); } public bool Read() diff --git a/QuickFIXn/SocketReader.cs b/QuickFIXn/SocketReader.cs index debfff801..0cf97608f 100755 --- a/QuickFIXn/SocketReader.cs +++ b/QuickFIXn/SocketReader.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using QuickFix.Logger; namespace QuickFix { @@ -19,6 +20,7 @@ public class SocketReader : IDisposable private readonly TcpClient _tcpClient; private readonly ClientHandlerThread _responder; private readonly AcceptorSocketDescriptor? _acceptorDescriptor; + private readonly NonSessionLog _nonSessionLog; /// /// Keep a task for handling async read @@ -29,12 +31,14 @@ internal SocketReader( TcpClient tcpClient, SocketSettings settings, ClientHandlerThread responder, - AcceptorSocketDescriptor? acceptorDescriptor) + AcceptorSocketDescriptor? acceptorDescriptor, + NonSessionLog nonSessionLog) { _tcpClient = tcpClient; _responder = responder; _acceptorDescriptor = acceptorDescriptor; - _stream = Transport.StreamFactory.CreateServerStream(tcpClient, settings); + _stream = Transport.StreamFactory.CreateServerStream(tcpClient, settings, nonSessionLog); + _nonSessionLog = nonSessionLog; } public void Read() @@ -121,7 +125,7 @@ private void OnMessageFound(string msg) if (_qfSession is null || IsUnknownSession(_qfSession.SessionID)) { _qfSession = null; - LogSessionEvent("ERROR: Disconnecting; received message for unknown session: " + msg); + _nonSessionLog.OnEvent("ERROR: Disconnecting; received message for unknown session: " + msg); DisconnectClient(); return; } @@ -169,13 +173,12 @@ protected void HandleBadMessage(string msg, Exception e) { if (Fields.MsgType.LOGON.Equals(Message.GetMsgType(msg))) { - LogSessionEvent($"ERROR: Invalid LOGON message, disconnecting: {e.Message}"); - // TODO: else session-agnostic log + LogEvent($"ERROR: Invalid LOGON message, disconnecting: {e.Message}"); DisconnectClient(); } else { - LogSessionEvent($"ERROR: Invalid message: {e.Message}"); + LogEvent($"ERROR: Invalid message: {e.Message}"); } } catch (InvalidMessage) @@ -244,7 +247,7 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) break; } - LogSessionEvent($"SocketReader Error: {reason}"); + LogEvent($"SocketReader Error: {reason}"); if (disconnectNeeded) { @@ -256,18 +259,15 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) } /// - /// Log event to session if known, else do... TODO + /// Log event to session log if session is known, else to nonSessionLog /// /// - private void LogSessionEvent(string s) + private void LogEvent(string s) { if(_qfSession is not null) _qfSession.Log.OnEvent(s); - else { - // Can't tie this to a session, need a generic log. - // TODO this is a temp console log until I do something better - Console.WriteLine(s); - } + else + _nonSessionLog.OnEvent(s); } public int Send(string data) diff --git a/QuickFIXn/SocketSettings.cs b/QuickFIXn/SocketSettings.cs index 872c9aa64..02a535b9f 100644 --- a/QuickFIXn/SocketSettings.cs +++ b/QuickFIXn/SocketSettings.cs @@ -87,7 +87,7 @@ public class SocketSettings : ICloneable public bool ValidateCertificates { get; internal set; } /// - /// Gets the path the the client/server-certificate. + /// Gets the path to the client/server-certificate. /// /// /// The certificate path. diff --git a/QuickFIXn/ThreadedSocketAcceptor.cs b/QuickFIXn/ThreadedSocketAcceptor.cs index 4e313644c..7e16e5a65 100755 --- a/QuickFIXn/ThreadedSocketAcceptor.cs +++ b/QuickFIXn/ThreadedSocketAcceptor.cs @@ -21,6 +21,7 @@ public class ThreadedSocketAcceptor : IAcceptor private bool _isStarted = false; private bool _disposed = false; private readonly object _sync = new(); + private readonly NonSessionLog _nonSessionLog; #region Constructors @@ -43,12 +44,13 @@ public ThreadedSocketAcceptor( IMessageFactory mf = messageFactory ?? new DefaultMessageFactory(); _settings = settings; _sessionFactory = new SessionFactory(application, storeFactory, lf, mf); + _nonSessionLog = new NonSessionLog(lf); try { foreach (SessionID sessionId in settings.GetSessions()) { - QuickFix.SettingsDictionary dict = settings.Get(sessionId); + SettingsDictionary dict = settings.Get(sessionId); CreateSession(sessionId, dict); } } @@ -93,7 +95,7 @@ private AcceptorSocketDescriptor GetAcceptorSocketDescriptor(SettingsDictionary if (!_socketDescriptorForAddress.TryGetValue(socketEndPoint, out var descriptor)) { - descriptor = new AcceptorSocketDescriptor(socketEndPoint, socketSettings, dict); + descriptor = new AcceptorSocketDescriptor(socketEndPoint, socketSettings, dict, _nonSessionLog); _socketDescriptorForAddress[socketEndPoint] = descriptor; } @@ -175,7 +177,7 @@ private void LogoutAllSessions(bool force) } catch (Exception e) { - System.Console.WriteLine("Error during logout of Session " + session.SessionID + ": " + e.Message); + session.Log.OnEvent($"Error during logout of Session {session.SessionID}: {e.Message}"); } } @@ -190,7 +192,7 @@ private void LogoutAllSessions(bool force) } catch (Exception e) { - System.Console.WriteLine("Error during disconnect of Session " + session.SessionID + ": " + e.Message); + session.Log.OnEvent($"Error during disconnect of Session {session.SessionID}: {e.Message}"); } } } @@ -200,11 +202,10 @@ private void LogoutAllSessions(bool force) } /// - /// FIXME implement WaitForLogout + /// TODO implement WaitForLogout /// private void WaitForLogout() { - System.Console.WriteLine("TODO - ThreadedSocketAcceptor.WaitForLogout not implemented!"); /* int start = System.Environment.TickCount; HashSet sessions = new HashSet(sessions_.Values); diff --git a/QuickFIXn/ThreadedSocketReactor.cs b/QuickFIXn/ThreadedSocketReactor.cs index b3fb4feaa..1f17c816c 100755 --- a/QuickFIXn/ThreadedSocketReactor.cs +++ b/QuickFIXn/ThreadedSocketReactor.cs @@ -4,6 +4,7 @@ using System.Net.Sockets; using System.Threading; using System; +using QuickFix.Logger; namespace QuickFix { @@ -29,29 +30,22 @@ public State ReactorState private readonly Dictionary _clientThreads = new (); private readonly TcpListener _tcpListener; private readonly SocketSettings _socketSettings; - private readonly SettingsDictionary _sessionDict; private readonly IPEndPoint _serverSocketEndPoint; private readonly AcceptorSocketDescriptor? _acceptorSocketDescriptor; - - // TODO: internalize. Only used by test. - public ThreadedSocketReactor( - IPEndPoint serverSocketEndPoint, - SocketSettings socketSettings, - SettingsDictionary sessionDict - ) : this(serverSocketEndPoint, socketSettings, sessionDict, null) { - } + private readonly NonSessionLog _nonSessionLog; internal ThreadedSocketReactor( IPEndPoint serverSocketEndPoint, SocketSettings socketSettings, SettingsDictionary sessionDict, - AcceptorSocketDescriptor? acceptorSocketDescriptor) + AcceptorSocketDescriptor? acceptorSocketDescriptor, + NonSessionLog nonSessionLog) { _socketSettings = socketSettings; _serverSocketEndPoint = serverSocketEndPoint; _tcpListener = new TcpListener(_serverSocketEndPoint); - _sessionDict = sessionDict; _acceptorSocketDescriptor = acceptorSocketDescriptor; + _nonSessionLog = nonSessionLog; } public void Start() @@ -84,13 +78,13 @@ public void Shutdown() } catch (Exception e) { - Log("Tried to interrupt server socket but was already closed: " + e.Message); + LogError("Tried to interrupt server socket but was already closed", e); } } } catch (Exception e) { - Log("Error while closing server socket: " + e.Message); + LogError("Error while closing server socket", e); } } } @@ -108,7 +102,7 @@ public void Run() } catch(Exception e) { - Log("Error starting listener: " + e.Message); + LogError("Error starting listener", e); throw; } } @@ -122,8 +116,8 @@ public void Run() if (State.RUNNING == ReactorState) { ApplySocketOptions(client, _socketSettings); - ClientHandlerThread t = - new ClientHandlerThread(client, _nextClientId++, _sessionDict, _socketSettings, _acceptorSocketDescriptor); + ClientHandlerThread t = new ClientHandlerThread( + client, _nextClientId++, _socketSettings, _acceptorSocketDescriptor, _nonSessionLog); t.Exited += OnClientHandlerThreadExited; lock (_sync) { @@ -140,7 +134,7 @@ public void Run() catch (Exception e) { if (State.RUNNING == ReactorState) - Log("Error accepting connection: " + e.Message); + LogError("Error accepting connection", e); } } _tcpListener.Server.Close(); @@ -193,8 +187,6 @@ private void ShutdownClientHandlerThreads() { if (State.SHUTDOWN_COMPLETE != _state) { - Log("shutting down..."); - foreach (ClientHandlerThread t in _clientThreads.Values) { t.Exited -= OnClientHandlerThreadExited; @@ -205,7 +197,7 @@ private void ShutdownClientHandlerThreads() } catch (Exception e) { - Log($"Error shutting down: {e.Message}"); + LogError("Error shutting down", e); } t.Dispose(); } @@ -216,12 +208,12 @@ private void ShutdownClientHandlerThreads() } /// - /// FIXME do real logging + /// Write to the NonSessionLog /// /// - private void Log(string s) - { - Console.WriteLine(s); + /// + private void LogError(string s, Exception? ex = null) { + _nonSessionLog.OnEvent(ex is null ? $"{s}" : $"{s}: {ex}"); } } } diff --git a/QuickFIXn/Transport/SocketInitiator.cs b/QuickFIXn/Transport/SocketInitiator.cs index 995a3264e..64c0c251d 100644 --- a/QuickFIXn/Transport/SocketInitiator.cs +++ b/QuickFIXn/Transport/SocketInitiator.cs @@ -249,7 +249,8 @@ protected override void DoConnect(Session session, SettingsDictionary settings) socketSettings.Configure(settings); // Create a Ssl-SocketInitiatorThread if a certificate is given - SocketInitiatorThread t = new SocketInitiatorThread(this, session, socketEndPoint, socketSettings); + SocketInitiatorThread t = new SocketInitiatorThread( + this, session, socketEndPoint, socketSettings, _nonSessionLog); t.Start(); AddThread(t); } diff --git a/QuickFIXn/SslCertCache.cs b/QuickFIXn/Transport/SslCertCache.cs similarity index 71% rename from QuickFIXn/SslCertCache.cs rename to QuickFIXn/Transport/SslCertCache.cs index 93e11fbb0..1308e6692 100644 --- a/QuickFIXn/SslCertCache.cs +++ b/QuickFIXn/Transport/SslCertCache.cs @@ -3,8 +3,9 @@ using System.Collections.Generic; using System.IO; using System.Security.Cryptography.X509Certificates; +using QuickFix.Util; -namespace QuickFix; +namespace QuickFix.Transport; internal static class SslCertCache { @@ -16,9 +17,10 @@ internal static class SslCertCache { /// /// Loads the specified certificate given a path, DistinguishedName or subject name /// - /// The certificate path or DistinguishedName/subjectname if it should be loaded from the personal certificate store. + /// The certificate path or DistinguishedName/subjectname + /// if it should be loaded from the personal certificate store. /// The certificate password. - /// The specified certificate, or null if no certificate is found + /// The specified certificate internal static X509Certificate2? LoadCertificate(string name, string? password) { // TODO: Change _certificateCache's type to ConcurrentDictionary once we start targeting .NET 4, @@ -29,12 +31,16 @@ internal static class SslCertCache { if (CertificateCache.TryGetValue(name, out X509Certificate2? certificate)) return certificate; - certificate = LoadCertificateInner(name, password); - - if (certificate is not null) + try { + certificate = LoadCertificateInner(name, password); CertificateCache.Add(name, certificate); - return certificate; + return certificate; + } catch (ApplicationException) { + // TODO refactor this function+callers to throw an exception up the stack instead of returning null + // Callers should log as appropriate + return null; + } } } @@ -43,22 +49,29 @@ internal static class SslCertCache { /// /// The certificate path or DistinguishedName/subjectname if it should be loaded from the personal certificate store. /// The certificate password. - /// The specified certificate, or null if no certificate is found - private static X509Certificate2? LoadCertificateInner(string name, string? password) + /// Certificate could not be loaded from file or store + /// The specified certificate + private static X509Certificate2 LoadCertificateInner(string name, string? password) { - X509Certificate2? certificate; + var certPath = StringUtil.FixSlashes(name); // If no extension is found try to get from certificate store - if (!File.Exists(name)) + if (!File.Exists(certPath)) { - certificate = GetCertificateFromStore(name); + var certFromStore = GetCertificateFromStore(StringUtil.FixSlashes(name)); + if (certFromStore is not null) + return certFromStore; + + // see TODO in LoadCertificate() + string msg = + $"Certificate '{name}' could not be loaded from store or path '{Directory.GetCurrentDirectory()}'"; + Console.WriteLine(msg); + throw new ApplicationException(msg); } - else { - certificate = password is not null + + return password is not null ? new X509Certificate2(name, password) : new X509Certificate2(name); - } - return certificate; } /// diff --git a/QuickFIXn/SslStreamFactory.cs b/QuickFIXn/Transport/SslStreamFactory.cs similarity index 87% rename from QuickFIXn/SslStreamFactory.cs rename to QuickFIXn/Transport/SslStreamFactory.cs index f384ceb39..296129f86 100644 --- a/QuickFIXn/SslStreamFactory.cs +++ b/QuickFIXn/Transport/SslStreamFactory.cs @@ -3,10 +3,12 @@ using System.Diagnostics; using System.IO; using System.Net.Security; +using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; +using QuickFix.Logger; using QuickFix.Util; -namespace QuickFix; +namespace QuickFix.Transport; /// /// The SSLClientStreamFactory is responsible for setting up a SSLStream in either client or server mode @@ -14,12 +16,14 @@ namespace QuickFix; internal sealed class SslStreamFactory { private readonly SocketSettings _socketSettings; + private readonly NonSessionLog _nonSessionLog; private const string CLIENT_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.2"; private const string SERVER_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.1"; - public SslStreamFactory(SocketSettings settings) + public SslStreamFactory(SocketSettings settings, NonSessionLog nonSessionLog) { _socketSettings = settings; + _nonSessionLog = nonSessionLog; } /// @@ -47,9 +51,9 @@ public Stream CreateClientStreamAndAuthenticate(Stream innerStream) _socketSettings.SslProtocol, _socketSettings.CheckCertificateRevocation); } - catch (System.Security.Authentication.AuthenticationException ex) + catch (AuthenticationException ex) { - Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); + _nonSessionLog.OnEvent($"Unable to perform authentication against server: {ex.GetFullMessage()}"); throw; } @@ -79,6 +83,10 @@ public Stream CreateServerStreamAndAuthenticate(Stream innerStream) // Setup secure SSL Communication X509Certificate2? serverCertificate = SslCertCache.LoadCertificate(_socketSettings.CertificatePath, _socketSettings.CertificatePassword); + if (serverCertificate is null) { + throw new AuthenticationException("Failed to load ServerCertificate"); + } + sslStream.AuthenticateAsServer(new SslServerAuthenticationOptions { ServerCertificate = serverCertificate, @@ -88,9 +96,9 @@ public Stream CreateServerStreamAndAuthenticate(Stream innerStream) EncryptionPolicy = EncryptionPolicy.RequireEncryption }); } - catch (System.Security.Authentication.AuthenticationException ex) + catch (AuthenticationException ex) { - Log($"Unable to perform authentication against server: {ex.GetFullMessage()}"); + _nonSessionLog.OnEvent($"Unable to perform authentication against server: {ex.GetFullMessage()}"); throw; } @@ -159,23 +167,22 @@ private bool VerifyRemoteCertificate( // Validate enhanced key usage if (!ContainsEnhancedKeyUsage(certificate, enhancedKeyUsage)) { - if (enhancedKeyUsage == CLIENT_AUTHENTICATION_OID) - Log($"Remote certificate is not intended for client authentication: It is missing enhanced key usage {enhancedKeyUsage}"); - else - Log($"Remote certificate is not intended for server authentication: It is missing enhanced key usage {enhancedKeyUsage}"); - + var role = enhancedKeyUsage == CLIENT_AUTHENTICATION_OID ? "client" : "server"; + _nonSessionLog.OnEvent( + $"Remote certificate is not intended for {role} authentication: It is missing enhanced key usage {enhancedKeyUsage}"); return false; } if (string.IsNullOrEmpty(_socketSettings.CACertificatePath)) { - Log("CACertificatePath is not specified"); + _nonSessionLog.OnEvent("CACertificatePath is not specified"); return false; } - // If CA Certficiate is specified then validate agains the CA certificate, otherwise it is validated against the installed certificates + // If CA Certificate is specified then validate against the CA certificate, otherwise it is validated against the installed certificates X509Certificate2? cert = SslCertCache.LoadCertificate(_socketSettings.CACertificatePath, null); if (cert is null) { - Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); + _nonSessionLog.OnEvent( + $"Certificate '{_socketSettings.CACertificatePath}' could not be loaded from store or path '{Directory.GetCurrentDirectory()}'"); return false; } @@ -200,7 +207,7 @@ private bool VerifyRemoteCertificate( // Any basic authentication check failed, do after checking CA if (sslPolicyErrors != SslPolicyErrors.None) { - Log($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); + _nonSessionLog.OnEvent($"Remote certificate was not recognized as a valid certificate: {sslPolicyErrors}"); return false; } @@ -274,9 +281,4 @@ private static bool ContainsEnhancedKeyUsage(X509Certificate certificate, string return null; } - - private void Log(string s) { - // TODO this is just a temp console log until I do something better - Console.WriteLine(s); - } } diff --git a/QuickFIXn/Transport/StreamFactory.cs b/QuickFIXn/Transport/StreamFactory.cs index cb732f0c6..edb23ef2e 100644 --- a/QuickFIXn/Transport/StreamFactory.cs +++ b/QuickFIXn/Transport/StreamFactory.cs @@ -13,7 +13,7 @@ namespace QuickFix.Transport /// StreamFactory is responsible for initiating for communication. /// If any SSL setup is required it is performed here /// - public static class StreamFactory + internal static class StreamFactory { private static Socket? CreateTunnelThruProxy(string destIp, int destPort) { @@ -65,9 +65,9 @@ public static class StreamFactory /// /// The endpoint. /// The socket settings. - /// Logger to use. + /// Logger that is not tied to a particular session /// an opened and initiated stream which can be read and written to - public static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings settings, ILog logger) + internal static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings settings, NonSessionLog nonSessionLog) { Socket? socket = null; @@ -104,7 +104,7 @@ public static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings sett Stream stream = new NetworkStream(socket, true); if (settings.UseSSL) - stream = new SslStreamFactory(settings).CreateClientStreamAndAuthenticate(stream); + stream = new SslStreamFactory(settings, nonSessionLog).CreateClientStreamAndAuthenticate(stream); return stream; } @@ -114,9 +114,10 @@ public static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings sett /// /// The TCP client. /// The socket settings. + /// Logger that is not tied to a particular session /// an opened and initiated stream which can be read and written to /// tcp client must be connected in order to get stream;tcpClient - public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings settings) + internal static Stream CreateServerStream(TcpClient tcpClient, SocketSettings settings, NonSessionLog nonSessionLog) { if (tcpClient.Connected == false) throw new ArgumentException("tcp client must be connected in order to get stream", nameof(tcpClient)); @@ -124,7 +125,7 @@ public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings sett Stream stream = tcpClient.GetStream(); if (settings.UseSSL) { - stream = new SslStreamFactory(settings).CreateServerStreamAndAuthenticate(stream); + stream = new SslStreamFactory(settings, nonSessionLog).CreateServerStreamAndAuthenticate(stream); } return stream; diff --git a/UnitTests/ThreadedSocketReactorTests.cs b/UnitTests/ThreadedSocketReactorTests.cs index 1fc357a2e..57c300b99 100644 --- a/UnitTests/ThreadedSocketReactorTests.cs +++ b/UnitTests/ThreadedSocketReactorTests.cs @@ -4,6 +4,7 @@ using System.IO; using System.Net; using System.Net.Sockets; +using QuickFix.Logger; namespace UnitTests { @@ -45,28 +46,18 @@ public void TestStartOnBusyPort() var port = OccupyAPort(); var settings = new SocketSettings(); - var testingObject = new ThreadedSocketReactor(new IPEndPoint(IPAddress.Loopback, port), settings, sessionDict: null); + var testingObject = new ThreadedSocketReactor( + new IPEndPoint(IPAddress.Loopback, port), + settings, + sessionDict: null, + acceptorSocketDescriptor: null, + new NonSessionLog(new ScreenLogFactory(true, true, true))); var stdOut = GetStdOut(); + var ex = Assert.Throws(delegate { testingObject.Run(); })!; - Exception exceptionResult = null; - string stdOutResult = null; - - try - { - testingObject.Run(); - } - catch (Exception ex) - { - exceptionResult = ex; - stdOutResult = stdOut.ToString(); - } - - Assert.IsNotNull(exceptionResult); - Assert.IsNotNull(stdOutResult); - - Assert.AreEqual(typeof(SocketException), exceptionResult.GetType()); - Assert.IsTrue(stdOutResult.StartsWith("Error starting listener: ", StringComparison.Ordinal)); + StringAssert.StartsWith(" Error starting listener:", stdOut.ToString()); + StringAssert.StartsWith("Address already in use", ex.Message); } [TearDown] From 7dd3298f0920bc2adfabd265dd09cfd585d9bdf0 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Mon, 10 Jun 2024 18:07:45 -0500 Subject: [PATCH 6/7] release notes for PR #830 --- QuickFIXn/Logger/ILogFactory.cs | 4 ++-- QuickFIXn/Session.cs | 2 +- RELEASE_NOTES.md | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/QuickFIXn/Logger/ILogFactory.cs b/QuickFIXn/Logger/ILogFactory.cs index 5bb364779..d5db1eec2 100755 --- a/QuickFIXn/Logger/ILogFactory.cs +++ b/QuickFIXn/Logger/ILogFactory.cs @@ -19,8 +19,8 @@ public interface ILogFactory /// This log will /// (1) only be used for messages that cannot be linked to a session /// (2) only have its OnEvent() method called - /// (3) only be created when the first message is logged (to avoid e.g. empty log files) - /// This log is written to only on rare occasions. It's possible you may never see it created. + /// (3) only be created when a message is logged (to avoid empty log files) + /// Messages are written to this log only on rare occasions. It's possible you may never see it created. /// /// ILog CreateNonSessionLog(); diff --git a/QuickFIXn/Session.cs b/QuickFIXn/Session.cs index 1d37b57e6..a359ea31c 100755 --- a/QuickFIXn/Session.cs +++ b/QuickFIXn/Session.cs @@ -396,7 +396,7 @@ public void Disconnect(string reason) } else { - Log.OnEvent("Session {SessionID} already disconnected: {reason}"); + Log.OnEvent($"Session {SessionID} already disconnected: {reason}"); } if (_state.ReceivedLogon || _state.SentLogon) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a50aedc02..69148e5c4 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -60,6 +60,11 @@ What's New * Also refactor the heck out of DateTimeConverter & tests: many functions renamed/deprecated * #847 - remove setting MillisecondsInTimeStamp (gbirchmeier) * Use TimestampPrecision instead (same as QF/j) +* #830 - replace ClientThreadHandler "Debug" logs with NonSessionLog (gbirchmeier) + * ILogFactory extended with a `CreateNonSessionLog()`. Pretty easy to implement though. + * Some classes were internalized, but I can't imagine people are using them in their app code. + * See details/explanation at https://github.com/connamara/quickfixn/pull/830 + **Non-breaking changes** * #400 - added DDTool, a C#-based codegen, and deleted Ruby-based generator (gbirchmeier) From d931e20c3edd022b9134daf7772ede099cfdf8e5 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Tue, 11 Jun 2024 14:46:16 -0500 Subject: [PATCH 7/7] found another spot that needed NonSessionLogging --- QuickFIXn/SocketInitiatorThread.cs | 6 +- QuickFIXn/Transport/SocketInitiator.cs | 86 ++++++++++---------------- 2 files changed, 36 insertions(+), 56 deletions(-) diff --git a/QuickFIXn/SocketInitiatorThread.cs b/QuickFIXn/SocketInitiatorThread.cs index a1bc1b794..96a73c6b3 100755 --- a/QuickFIXn/SocketInitiatorThread.cs +++ b/QuickFIXn/SocketInitiatorThread.cs @@ -17,6 +17,7 @@ public class SocketInitiatorThread : IResponder { public Session Session { get; } public Transport.SocketInitiator Initiator { get; } + public NonSessionLog NonSessionLog { get; } public const int BUF_SIZE = 512; @@ -28,7 +29,6 @@ public class SocketInitiatorThread : IResponder private readonly IPEndPoint _socketEndPoint; private readonly SocketSettings _socketSettings; private bool _isDisconnectRequested = false; - private readonly NonSessionLog _nonSessionLog; /// /// Keep a task for handling async read @@ -44,9 +44,9 @@ public SocketInitiatorThread( { Initiator = initiator; Session = session; + NonSessionLog = nonSessionLog; _socketEndPoint = socketEndPoint; _socketSettings = socketSettings; - _nonSessionLog = nonSessionLog; } public void Start() @@ -82,7 +82,7 @@ public void Connect() /// Stream representing the (network)connection to the other party protected virtual Stream SetupStream() { - return Transport.StreamFactory.CreateClientStream(_socketEndPoint, _socketSettings, _nonSessionLog); + return Transport.StreamFactory.CreateClientStream(_socketEndPoint, _socketSettings, NonSessionLog); } public bool Read() diff --git a/QuickFIXn/Transport/SocketInitiator.cs b/QuickFIXn/Transport/SocketInitiator.cs index 64c0c251d..4add51c5c 100644 --- a/QuickFIXn/Transport/SocketInitiator.cs +++ b/QuickFIXn/Transport/SocketInitiator.cs @@ -8,7 +8,6 @@ using System.Threading; using QuickFix.Logger; using QuickFix.Store; -using QuickFix.Util; namespace QuickFix.Transport { @@ -43,61 +42,34 @@ public static void SocketInitiatorThreadStart(object? socketInitiatorThread) SocketInitiatorThread? t = socketInitiatorThread as SocketInitiatorThread; if (t == null) return; - string? exceptionEvent = null; try { - try - { - t.Connect(); - t.Initiator.SetConnected(t.Session.SessionID); - t.Session.Log.OnEvent("Connection succeeded"); - t.Session.Next(); - while (t.Read()) - { - } - - if (t.Initiator.IsStopped) - t.Initiator.RemoveThread(t); - t.Initiator.SetDisconnected(t.Session.SessionID); - } - catch (IOException ex) // Can be exception when connecting, during ssl authentication or when reading - { - exceptionEvent = $"Connection failed: {ex.Message}"; - } - catch (SocketException e) - { - exceptionEvent = $"Connection failed: {e.Message}"; - } - catch (System.Security.Authentication.AuthenticationException ex) // some certificate problems - { - exceptionEvent = $"Connection failed (AuthenticationException): {ex.GetFullMessage()}"; - } - catch (Exception ex) - { - exceptionEvent = $"Unexpected exception: {ex}"; + t.Connect(); + t.Initiator.SetConnected(t.Session.SessionID); + t.Session.Log.OnEvent("Connection succeeded"); + t.Session.Next(); + while (t.Read()) { } - if (exceptionEvent is not null) - { - if (t.Session.Disposed) - { - // The session is disposed, and so is its log. We cannot use it to log the event, - // so we resort to storing it in a local file. - try - { - // TODO: temporary hack, need to implement a session-independent log - File.AppendAllText("DisposedSessionEvents.log", $"{DateTime.Now:G}: {exceptionEvent}{Environment.NewLine}"); - } - catch (IOException) - { - // Prevent IO exceptions from crashing the application - } - } - else - { - t.Session.Log.OnEvent(exceptionEvent); - } - } + if (t.Initiator.IsStopped) + t.Initiator.RemoveThread(t); + t.Initiator.SetDisconnected(t.Session.SessionID); + } + catch (IOException ex) // Can be exception when connecting, during ssl authentication or when reading + { + LogThreadStartConnectionFailed(t, ex); + } + catch (SocketException ex) + { + LogThreadStartConnectionFailed(t, ex); + } + catch (System.Security.Authentication.AuthenticationException ex) // some certificate problems + { + LogThreadStartConnectionFailed(t, ex); + } + catch (Exception ex) + { + LogThreadStartConnectionFailed(t, ex); } finally { @@ -105,7 +77,15 @@ public static void SocketInitiatorThreadStart(object? socketInitiatorThread) t.Initiator.SetDisconnected(t.Session.SessionID); } } - + + private static void LogThreadStartConnectionFailed(SocketInitiatorThread t, Exception e) { + if (t.Session.Disposed) { + t.NonSessionLog.OnEvent($"Connection failed [session {t.Session.SessionID}]: {e}"); + return; + } + t.Session.Log.OnEvent($"Connection failed: {e}"); + } + private void AddThread(SocketInitiatorThread thread) { lock (_sync)