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 e20eec943..47bf92810 100644 --- a/QuickFIXn/AcceptorSocketDescriptor.cs +++ b/QuickFIXn/AcceptorSocketDescriptor.cs @@ -1,32 +1,28 @@ #nullable enable using System.Collections.Generic; using System.Net; +using QuickFix.Logger; 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) + 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); } - public void AcceptSession(Session session) + internal void AcceptSession(Session session) { lock (_acceptedSessions) { @@ -39,7 +35,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 +43,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..042bfd4ed 100755 --- a/QuickFIXn/ClientHandlerThread.cs +++ b/QuickFIXn/ClientHandlerThread.cs @@ -31,23 +31,16 @@ 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())); + 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() @@ -58,7 +51,7 @@ public void Start() public void Shutdown(string reason) { - Log("shutdown requested: " + reason); + // TODO - need the reason param? _isShutdownRequested = true; } @@ -85,7 +78,6 @@ private void Run() } } - Log("shutdown"); OnExited(); } @@ -93,21 +85,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 +113,6 @@ protected virtual void Dispose(bool disposing) if (disposing) { _socketReader.Dispose(); - _log.Dispose(); } _disposed = true; } 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..d5db1eec2 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 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/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/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/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/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/SessionSettings.cs b/QuickFIXn/SessionSettings.cs index 6cb5f49d1..fdddfc395 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"; @@ -163,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; } @@ -180,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); } @@ -202,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}"); @@ -225,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; } @@ -238,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) @@ -252,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/SocketInitiatorThread.cs b/QuickFIXn/SocketInitiatorThread.cs index 117204574..96a73c6b3 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 { @@ -16,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; @@ -23,7 +25,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; @@ -33,10 +35,16 @@ public class SocketInitiatorThread : IResponder /// 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; + NonSessionLog = nonSessionLog; _socketEndPoint = socketEndPoint; _socketSettings = socketSettings; } @@ -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() @@ -127,8 +135,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 d12b87b3e..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, responder.GetLog()); + _stream = Transport.StreamFactory.CreateServerStream(tcpClient, settings, nonSessionLog); + _nonSessionLog = nonSessionLog; } public void Read() @@ -78,8 +82,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 +100,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) { @@ -119,10 +122,10 @@ 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); _qfSession = null; + _nonSessionLog.OnEvent("ERROR: Disconnecting; received message for unknown session: " + msg); DisconnectClient(); return; } @@ -146,9 +149,14 @@ private void OnMessageFound(string msg) } catch (Exception e) { - Log($"Error on Session '{_qfSession.SessionID}': {e}"); + _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); @@ -165,12 +173,12 @@ protected void HandleBadMessage(string msg, Exception e) { if (Fields.MsgType.LOGON.Equals(Message.GetMsgType(msg))) { - Log("ERROR: Invalid LOGON message, disconnecting: " + e.Message); + LogEvent($"ERROR: Invalid LOGON message, disconnecting: {e.Message}"); DisconnectClient(); } else { - Log("ERROR: Invalid message: " + e.Message); + LogEvent($"ERROR: Invalid message: {e.Message}"); } } catch (InvalidMessage) @@ -201,7 +209,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 +247,7 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) break; } - Log($"SocketReader Error: {reason}"); + LogEvent($"SocketReader Error: {reason}"); if (disconnectNeeded) { @@ -251,12 +259,15 @@ private void HandleExceptionInternal(Session? quickFixSession, Exception cause) } /// - /// FIXME do proper logging + /// Log event to session log if session is known, else to nonSessionLog /// /// - private void Log(string s) + private void LogEvent(string s) { - _responder.Log(s); + if(_qfSession is not null) + _qfSession.Log.OnEvent(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 88e460bd0..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; } @@ -147,11 +149,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 +163,6 @@ private void StopAcceptingConnections() foreach (AcceptorSocketDescriptor socketDescriptor in _socketDescriptorForAddress.Values) { socketDescriptor.SocketReactor.Shutdown(); - // FIXME log_.Info("No longer accepting connections on " + socketDescriptor.getAddress()); } } } @@ -178,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}"); } } @@ -193,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}"); } } } @@ -203,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 7a676e499..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 { @@ -17,17 +18,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,30 +30,22 @@ public State ReactorState private readonly Dictionary _clientThreads = new (); private readonly TcpListener _tcpListener; private readonly SocketSettings _socketSettings; - private readonly QuickFix.SettingsDictionary _sessionDict; private readonly IPEndPoint _serverSocketEndPoint; private readonly AcceptorSocketDescriptor? _acceptorSocketDescriptor; - - #endregion - - public ThreadedSocketReactor( - IPEndPoint serverSocketEndPoint, - SocketSettings socketSettings, - QuickFix.SettingsDictionary sessionDict - ) : this(serverSocketEndPoint, socketSettings, sessionDict, null) { - } + private readonly NonSessionLog _nonSessionLog; internal ThreadedSocketReactor( IPEndPoint serverSocketEndPoint, SocketSettings socketSettings, - QuickFix.SettingsDictionary sessionDict, - AcceptorSocketDescriptor? acceptorSocketDescriptor) + SettingsDictionary sessionDict, + AcceptorSocketDescriptor? acceptorSocketDescriptor, + NonSessionLog nonSessionLog) { _socketSettings = socketSettings; _serverSocketEndPoint = serverSocketEndPoint; _tcpListener = new TcpListener(_serverSocketEndPoint); - _sessionDict = sessionDict; _acceptorSocketDescriptor = acceptorSocketDescriptor; + _nonSessionLog = nonSessionLog; } public void Start() @@ -91,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); } } } @@ -115,7 +102,7 @@ public void Run() } catch(Exception e) { - Log("Error starting listener: " + e.Message); + LogError("Error starting listener", e); throw; } } @@ -129,16 +116,14 @@ 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) { _clientThreads.Add(t.Id, t); } - // FIXME set the client thread's exception handler here - t.Log("connected"); t.Start(); } else @@ -149,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(); @@ -202,8 +187,6 @@ private void ShutdownClientHandlerThreads() { if (State.SHUTDOWN_COMPLETE != _state) { - Log("shutting down..."); - foreach (ClientHandlerThread t in _clientThreads.Values) { t.Exited -= OnClientHandlerThreadExited; @@ -214,7 +197,7 @@ private void ShutdownClientHandlerThreads() } catch (Exception e) { - t.Log("Error shutting down: " + e.Message); + LogError("Error shutting down", e); } t.Dispose(); } @@ -225,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 d8e24a430..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) @@ -138,7 +118,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; @@ -249,7 +229,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/Transport/SslCertCache.cs b/QuickFIXn/Transport/SslCertCache.cs new file mode 100644 index 000000000..1308e6692 --- /dev/null +++ b/QuickFIXn/Transport/SslCertCache.cs @@ -0,0 +1,115 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.IO; +using System.Security.Cryptography.X509Certificates; +using QuickFix.Util; + +namespace QuickFix.Transport; + +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 + 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; + + try { + certificate = LoadCertificateInner(name, password); + CertificateCache.Add(name, 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; + } + } + } + + /// + /// 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. + /// Certificate could not be loaded from file or store + /// The specified certificate + private static X509Certificate2 LoadCertificateInner(string name, string? password) + { + var certPath = StringUtil.FixSlashes(name); + + // If no extension is found try to get from certificate store + if (!File.Exists(certPath)) + { + 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); + } + + return password is not null + ? new X509Certificate2(name, password) + : new X509Certificate2(name); + } + + /// + /// 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/Transport/SslStreamFactory.cs b/QuickFIXn/Transport/SslStreamFactory.cs new file mode 100644 index 000000000..296129f86 --- /dev/null +++ b/QuickFIXn/Transport/SslStreamFactory.cs @@ -0,0 +1,284 @@ +#nullable enable +using System; +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.Transport; + +/// +/// The SSLClientStreamFactory is responsible for setting up a SSLStream in either client or server mode +/// +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, NonSessionLog nonSessionLog) + { + _socketSettings = settings; + _nonSessionLog = nonSessionLog; + } + + /// + /// 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 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). + + try + { + // Setup secure SSL Communication + X509CertificateCollection clientCertificates = GetClientCertificates(); + sslStream.AuthenticateAsClient(_socketSettings.ServerCommonName, + clientCertificates, + _socketSettings.SslProtocol, + _socketSettings.CheckCertificateRevocation); + } + catch (AuthenticationException ex) + { + _nonSessionLog.OnEvent($"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 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). + + 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); + if (serverCertificate is null) { + throw new AuthenticationException("Failed to load ServerCertificate"); + } + + sslStream.AuthenticateAsServer(new SslServerAuthenticationOptions + { + ServerCertificate = serverCertificate, + ClientCertificateRequired = _socketSettings.RequireClientCertificate, + EnabledSslProtocols = _socketSettings.SslProtocol, + CertificateRevocationCheckMode = _socketSettings.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, + EncryptionPolicy = EncryptionPolicy.RequireEncryption + }); + } + catch (AuthenticationException ex) + { + _nonSessionLog.OnEvent($"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)) { + 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)) { + _nonSessionLog.OnEvent("CACertificatePath is not specified"); + return false; + } + + // 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) { + _nonSessionLog.OnEvent( + $"Certificate '{_socketSettings.CACertificatePath}' could not be loaded from store or path '{Directory.GetCurrentDirectory()}'"); + 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) + { + _nonSessionLog.OnEvent($"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; + } +} diff --git a/QuickFIXn/Transport/StreamFactory.cs b/QuickFIXn/Transport/StreamFactory.cs index 3e00887c9..edb23ef2e 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 { @@ -18,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) { @@ -70,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; @@ -109,7 +104,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, nonSessionLog).CreateClientStreamAndAuthenticate(stream); return stream; } @@ -119,10 +114,10 @@ public static Stream CreateClientStream(IPEndPoint endpoint, SocketSettings sett /// /// The TCP client. /// 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 /// tcp client must be connected in order to get stream;tcpClient - public static Stream CreateServerStream(TcpClient tcpClient, SocketSettings settings, ILog logger) + 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)); @@ -130,377 +125,10 @@ 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, nonSessionLog).CreateServerStreamAndAuthenticate(stream); } 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 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) - { - _log = log; - _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.OnEvent("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.OnEvent("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.OnEvent( - "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); - - return false; - } - - if (string.IsNullOrEmpty(_socketSettings.CACertificatePath)) { - _log.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 - X509Certificate2? cert = LoadCertificate(_socketSettings.CACertificatePath, null); - if (cert is null) { - _log.OnEvent("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.OnEvent("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; - } - } } } 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) 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]