From 5ac3be1dcd5bfbdc1d28c71cf4de6459acf17cfa Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Thu, 14 Nov 2024 17:13:19 +0200 Subject: [PATCH 1/6] Added a rogue client tracker to track potential rogue client behaviors --- .../Stack/Tcp/TcpListenerChannel.cs | 12 + .../Stack/Tcp/TcpTransportListener.cs | 249 ++++++++++++++++++ 2 files changed, 261 insertions(+) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs index 763b71f1a..9c074ef56 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs @@ -11,6 +11,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. */ using System; +using System.Net; using System.Net.Sockets; using System.Security.Cryptography.X509Certificates; using Microsoft.Extensions.Logging; @@ -267,6 +268,17 @@ protected void ForceChannelFault(ServiceResult reason) if (close) { + // mark the RemoteAddress as potential rogue + if (reason.StatusCode == StatusCodes.BadSecurityChecksFailed || reason.StatusCode == StatusCodes.BadTcpMessageTypeInvalid) + { + var tcpTransportListener = m_listener as TcpTransportListener; + if (tcpTransportListener != null) + { + tcpTransportListener.MarkAsPotentialRogue + (((IPEndPoint)Socket.RemoteEndpoint).Address); + } + } + // close channel immediately. ChannelFaulted(); } diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index 38bcd7616..07f784937 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -13,6 +13,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Net; using System.Net.Sockets; @@ -37,6 +38,230 @@ public override ITransportListener Create() } } + /// + /// Represents a potential RogueClient + /// + public class RogueClient + { + #region Properties + /// + /// Time of the last recorded rogue action + /// + public int LastRogueActionTicks + { + get + { + return m_lastRogueActionTicks; + } + set + { + m_lastRogueActionTicks = value; + } + } + + /// + /// Counter for number of rogue recorded actions + /// + public int RogueActionCount + { + get + { + return m_rogueActionCount; + } + set + { + m_rogueActionCount = value; + } + } + + /// + /// Ticks until the client is Blocked + /// + public int BlockedUntilTicks + { + get + { + return m_blockedUntilTicks; + } + set + { + m_blockedUntilTicks = value; + } + } + #endregion + + #region Private members + int m_lastRogueActionTicks; + int m_rogueActionCount; + int m_blockedUntilTicks; + #endregion + } + + /// + /// Manages potential rogue clients + /// + public class RogueClientTracker : IDisposable + { + #region Public + /// + /// Constructor + /// + public RogueClientTracker() + { + m_cleanupTimer = new Timer(CleanupExpiredEntries, null, m_kCleanupIntervalMs, m_kCleanupIntervalMs); + } + + /// + /// Checks if an IP address is currently blocked + /// + /// + /// + public bool IsBlocked(IPAddress ipAddress) + { + if (m_rogueClients.TryGetValue(ipAddress, out RogueClient client)) + { + int currentTicks = HiResClock.TickCount; + return IsBlockedTicks(client.BlockedUntilTicks, currentTicks); + } + return false; + } + + /// + /// Adds an action entry for a potential rogue client + /// + /// + public void AddRogueClientAction(IPAddress ipAddress) + { + int currentTicks = HiResClock.TickCount; + + m_rogueClients.AddOrUpdate(ipAddress, + // If client is new , create a new entry + key => new RogueClient { + LastRogueActionTicks = currentTicks, + RogueActionCount = 1, + BlockedUntilTicks = 0 + }, + // If the client exists, update its entry + (key, existingEntry) => { + // If IP currently blocked simply do nothing + if (IsBlockedTicks(existingEntry.BlockedUntilTicks, currentTicks)) + { + return existingEntry; + } + + // Elapsed time since last recorded action + int elapsedSinceLastRecAction = currentTicks - existingEntry.LastRogueActionTicks; + + if (elapsedSinceLastRecAction <= m_kRogueIntervalMs) + { + existingEntry.RogueActionCount++; + + if (existingEntry.RogueActionCount > m_kNrActionsTillBlock) + { + // Block the IP + existingEntry.BlockedUntilTicks = currentTicks + m_kBlockDurationMs; + Utils.LogError("RemoteClient IPAddress: {0} blocked for {1} ms due to exceeding {2} actions under {3} ms ", + ipAddress.ToString(), + m_kBlockDurationMs, + m_kNrActionsTillBlock, + m_kRogueIntervalMs); + + } + } + else + { + // Reset the count as the last action was outside the interval + existingEntry.RogueActionCount = 1; + } + + existingEntry.LastRogueActionTicks = currentTicks; + + return existingEntry; + } + ); + } + + /// + /// Dispose the cleanup timer + /// + public void Dispose() + { + m_cleanupTimer?.Dispose(); + } + + #endregion + #region Private methods + + /// + /// Periodically cleans up expired RogueClientEntries to avoid memory leak and unblock clients whose duration has expired. + /// + /// + private void CleanupExpiredEntries(object state) + { + int currentTicks = HiResClock.TickCount; + + foreach (var entry in m_rogueClients) + { + IPAddress clientIp = entry.Key; + RogueClient rClient = entry.Value; + + // Unblock client if blocking duration has been exceeded + if (rClient.BlockedUntilTicks != 0 && !IsBlockedTicks(rClient.BlockedUntilTicks, currentTicks)) + { + rClient.BlockedUntilTicks = 0; + rClient.RogueActionCount = 0; + Utils.LogInfo("Rogue Client with IP {0} is now unblocked, blocking duration has been exceeded", clientIp.ToString()); + } + + // Remove clients that haven't had any rogue actions in the last m_kEntryExpirationMs interval + int elapsedSinceBadActionTicks = currentTicks - rClient.LastRogueActionTicks; + if (elapsedSinceBadActionTicks > m_kEntryExpirationMs) + { + // Even if TryRemove fails it will most probably succeed at the next execution + if (m_rogueClients.TryRemove(clientIp, out _)) + { + Utils.LogInfo("Rogue Client with IP {0} is not tracked any longer, hasn't had rogue actions for more than {1} ms", + clientIp.ToString(), + m_kEntryExpirationMs); + } + } + } + } + + /// + /// Determines if the IP is currently blocked based on the block expiration ticks and current ticks + /// + /// + /// + /// + private bool IsBlockedTicks(int blockedUntilTicks, int currentTicks) + { + if (blockedUntilTicks == 0) + { + return false; + } + // C# signed arithmetic + int diff = blockedUntilTicks - currentTicks; + // If currentTicks < blockedUntilTicks then it is still blocked + // Works even if TickCount has wrapped around due to C# signed integer arithmetic + return diff > 0; + } + + + #endregion + #region Private members + private ConcurrentDictionary m_rogueClients = new ConcurrentDictionary(); + + private const int m_kRogueIntervalMs = 10_000; + private const int m_kBlockDurationMs = 300_000; // 5 minutes + private const int m_kCleanupIntervalMs = 60_000; + private const int m_kEntryExpirationMs = 600_000; // 10 minutes + private const int m_kNrActionsTillBlock = 3; + + private Timer m_cleanupTimer; + #endregion + } + /// /// Manages the transport for a UA TCP server. /// @@ -410,6 +635,8 @@ public void Start() StatusCodes.BadNoCommunication, "Failed to establish tcp listener sockets for Ipv4 and IPv6."); } + + m_rogueClientTracker = new RogueClientTracker(); } } @@ -497,12 +724,32 @@ public void CertificateUpdate( } #endregion + #region Internal + /// + /// Mark a remote endpoint as potential rogue + /// + /// + internal void MarkAsPotentialRogue(IPAddress remoteEndpoint) + { + Utils.LogError("MarkClientAsPotentialRogue address: {0} ", remoteEndpoint.ToString()); + m_rogueClientTracker.AddRogueClientAction(remoteEndpoint); + } + #endregion + #region Socket Event Handler /// /// Handles a new connection. /// private void OnAccept(object sender, SocketAsyncEventArgs e) { + // Filter out the RemoveAddresses which are detected with unwanted behavior + if (m_rogueClientTracker.IsBlocked(((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address)) + { + Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", + ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); + e.Dispose(); + return; + } TcpListenerChannel channel = null; bool repeatAccept = false; do @@ -806,6 +1053,8 @@ private void SetUri(Uri baseAddress, string relativeAddress) private int m_inactivityDetectPeriod; private Timer m_inactivityDetectionTimer; private int m_maxChannelCount; + + private RogueClientTracker m_rogueClientTracker; #endregion } From 65a88c035331b19705020c7406948d02075f2ec2 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 18 Nov 2024 16:47:10 +0200 Subject: [PATCH 2/6] Fixed connect bug and impreved logging --- .../Stack/Tcp/TcpTransportListener.cs | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index 07f784937..4f0728aa2 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -160,7 +160,7 @@ public void AddRogueClientAction(IPAddress ipAddress) { // Block the IP existingEntry.BlockedUntilTicks = currentTicks + m_kBlockDurationMs; - Utils.LogError("RemoteClient IPAddress: {0} blocked for {1} ms due to exceeding {2} actions under {3} ms ", + Utils.LogError("RemoteClient IPAddress: {0} blocked for {1} ms due to exceeding {2} rogue actions under {3} ms ", ipAddress.ToString(), m_kBlockDurationMs, m_kNrActionsTillBlock, @@ -210,7 +210,9 @@ private void CleanupExpiredEntries(object state) { rClient.BlockedUntilTicks = 0; rClient.RogueActionCount = 0; - Utils.LogInfo("Rogue Client with IP {0} is now unblocked, blocking duration has been exceeded", clientIp.ToString()); + Utils.LogInfo("Rogue Client with IP {0} is now unblocked, blocking duration of {1} ms has been exceeded", + clientIp.ToString(), + m_kBlockDurationMs); } // Remove clients that haven't had any rogue actions in the last m_kEntryExpirationMs interval @@ -253,11 +255,12 @@ private bool IsBlockedTicks(int blockedUntilTicks, int currentTicks) private ConcurrentDictionary m_rogueClients = new ConcurrentDictionary(); private const int m_kRogueIntervalMs = 10_000; - private const int m_kBlockDurationMs = 300_000; // 5 minutes - private const int m_kCleanupIntervalMs = 60_000; - private const int m_kEntryExpirationMs = 600_000; // 10 minutes private const int m_kNrActionsTillBlock = 3; + private const int m_kBlockDurationMs = 30_000; // 30 seconds + private const int m_kCleanupIntervalMs = 15_000; + private const int m_kEntryExpirationMs = 600_000; // 10 minutes + private Timer m_cleanupTimer; #endregion } @@ -742,18 +745,21 @@ internal void MarkAsPotentialRogue(IPAddress remoteEndpoint) /// private void OnAccept(object sender, SocketAsyncEventArgs e) { - // Filter out the RemoveAddresses which are detected with unwanted behavior - if (m_rogueClientTracker.IsBlocked(((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address)) - { - Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", - ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); - e.Dispose(); - return; - } + TcpListenerChannel channel = null; bool repeatAccept = false; do { + bool isRogue = false; + + // Filter out the RemoveAddresses which are detected with rogue behavior + if (m_rogueClientTracker.IsBlocked(((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address)) + { + Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", + ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); + isRogue = true; + } + repeatAccept = false; lock (m_lock) { @@ -765,7 +771,7 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) } var channels = m_channels; - if (channels != null) + if (channels != null && !isRogue) { // TODO: .Count is flagged as hotpath, implement separate counter int channelCount = channels.Count; From 8c66f7a0e25996704372304a3cc68a139dcea484 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Tue, 19 Nov 2024 11:20:58 +0200 Subject: [PATCH 3/6] Instantiate rogue client tracker at the begining of Start --- Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index 4f0728aa2..e86fb2745 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -551,6 +551,8 @@ public void Start() { lock (m_lock) { + m_rogueClientTracker = new RogueClientTracker(); + // ensure a valid port. int port = m_uri.Port; @@ -638,8 +640,6 @@ public void Start() StatusCodes.BadNoCommunication, "Failed to establish tcp listener sockets for Ipv4 and IPv6."); } - - m_rogueClientTracker = new RogueClientTracker(); } } From 4fbe161dc0baa689aba534892761a1ce8ed2d23e Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Tue, 19 Nov 2024 12:03:16 +0200 Subject: [PATCH 4/6] Null checks added --- Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index e86fb2745..b8f184276 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -753,7 +753,7 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) bool isRogue = false; // Filter out the RemoveAddresses which are detected with rogue behavior - if (m_rogueClientTracker.IsBlocked(((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address)) + if ((bool)m_rogueClientTracker?.IsBlocked(((IPEndPoint)e?.AcceptSocket?.RemoteEndPoint)?.Address)) { Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); From ae96ded6178be4a02b594b06722d50d73e9e4f07 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Tue, 19 Nov 2024 12:20:23 +0200 Subject: [PATCH 5/6] Ignore null remote IP Addresses --- Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index b8f184276..886aa05cb 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -752,8 +752,9 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) { bool isRogue = false; - // Filter out the RemoveAddresses which are detected with rogue behavior - if ((bool)m_rogueClientTracker?.IsBlocked(((IPEndPoint)e?.AcceptSocket?.RemoteEndPoint)?.Address)) + // Filter out the Remote IP addresses which are detected with rogue behavior + IPAddress ipAddress = ((IPEndPoint)e?.AcceptSocket?.RemoteEndPoint)?.Address; + if (ipAddress != null && m_rogueClientTracker.IsBlocked(ipAddress)) { Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); From 9d1b738fc96dc4a3782f8959ce29ed531e66d8a1 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Wed, 20 Nov 2024 14:14:46 +0200 Subject: [PATCH 6/6] Track rogue client behavior only under Basic128Rsa15 security policy --- .../Stack/Tcp/TcpListenerChannel.cs | 5 ++-- .../Stack/Tcp/TcpTransportListener.cs | 26 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs index 9c074ef56..328252f0e 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs @@ -268,8 +268,9 @@ protected void ForceChannelFault(ServiceResult reason) if (close) { - // mark the RemoteAddress as potential rogue - if (reason.StatusCode == StatusCodes.BadSecurityChecksFailed || reason.StatusCode == StatusCodes.BadTcpMessageTypeInvalid) + // mark the RemoteAddress as potential rogue if Basic128Rsa15 + if ((SecurityPolicyUri == SecurityPolicies.Basic128Rsa15) && + (reason.StatusCode == StatusCodes.BadSecurityChecksFailed || reason.StatusCode == StatusCodes.BadTcpMessageTypeInvalid)) { var tcpTransportListener = m_listener as TcpTransportListener; if (tcpTransportListener != null) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index 886aa05cb..686827119 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -551,7 +551,11 @@ public void Start() { lock (m_lock) { - m_rogueClientTracker = new RogueClientTracker(); + // Track rogue client behavior only if Basic128Rsa15 security policy is offered + if (m_descriptions.Any(d => d.SecurityPolicyUri == SecurityPolicies.Basic128Rsa15)) + { + m_rogueClientTracker = new RogueClientTracker(); + } // ensure a valid port. int port = m_uri.Port; @@ -734,8 +738,8 @@ public void CertificateUpdate( /// internal void MarkAsPotentialRogue(IPAddress remoteEndpoint) { - Utils.LogError("MarkClientAsPotentialRogue address: {0} ", remoteEndpoint.ToString()); - m_rogueClientTracker.AddRogueClientAction(remoteEndpoint); + Utils.LogInfo("MarkClientAsPotentialRogue address: {0} ", remoteEndpoint.ToString()); + m_rogueClientTracker?.AddRogueClientAction(remoteEndpoint); } #endregion @@ -752,13 +756,17 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) { bool isRogue = false; - // Filter out the Remote IP addresses which are detected with rogue behavior - IPAddress ipAddress = ((IPEndPoint)e?.AcceptSocket?.RemoteEndPoint)?.Address; - if (ipAddress != null && m_rogueClientTracker.IsBlocked(ipAddress)) + // Track rogue client behavior only if Basic128Rsa15 security policy is offered + if (m_rogueClientTracker != null) { - Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", - ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); - isRogue = true; + // Filter out the Remote IP addresses which are detected with rogue behavior + IPAddress ipAddress = ((IPEndPoint)e?.AcceptSocket?.RemoteEndPoint)?.Address; + if (ipAddress != null && m_rogueClientTracker.IsBlocked(ipAddress)) + { + Utils.LogError("OnAccept: RemoteEndpoint address: {0} refused access for behaving as potential rogue ", + ((IPEndPoint)e.AcceptSocket.RemoteEndPoint).Address.ToString()); + isRogue = true; + } } repeatAccept = false;