From 6c72c537354816c37d9a2de1707c9ad107461340 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Fri, 7 Jun 2024 11:11:25 -0500 Subject: [PATCH] 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 | 25 +++++------ 20 files changed, 199 insertions(+), 125 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..f81d86ce4 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,24 @@ 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(); Exception exceptionResult = null; string stdOutResult = null; - try - { - testingObject.Run(); - } - catch (Exception ex) - { - exceptionResult = ex; - stdOutResult = stdOut.ToString(); - } + var ex = Assert.Throws(delegate { testingObject.Run(); })!; + 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: System.Net.Sockets.SocketException (48): Address already in use", stdOutResult); + StringAssert.StartsWith("Address already in use", ex.Message); } [TearDown]