From 01c10bbe99f592372834c4a23b21104698cf8ecf Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Thu, 1 Oct 2026 15:55:36 -0400 Subject: [PATCH 01/10] fix: stop mutating _userAppBaseHref in HandleUserAppRequest HandleUserAppRequest evaluated `_userAppBaseHref += "/"` inside an if-condition. That appended a slash to the shared field the first time the sub-expression was evaluated and changed path handling for every later request. It now compares against `_userAppBaseHref + "/"` and leaves the field alone. Normal request paths short-circuit before that sub-expression, so they behave as before. Co-Authored-By: Claude Sonnet 5.5 --- .../WebSocketServer/MobileControlWebsocketServer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 0e1ed0d8c..458c718cf 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -1361,7 +1361,7 @@ private void HandleUserAppRequest(HttpListenerRequest req, HttpListenerResponse //string filePath = path.Replace(string.Format("?token={0}", token), ""); // if there's no file suffix strip any extra path data after the base href - if (filePath != _userAppBaseHref && !filePath.Contains(".") && (!filePath.EndsWith(_userAppBaseHref) || !filePath.EndsWith(_userAppBaseHref += "/"))) + if (filePath != _userAppBaseHref && !filePath.Contains(".") && (!filePath.EndsWith(_userAppBaseHref) || !filePath.EndsWith(_userAppBaseHref + "/"))) { var suffix = filePath.Substring(_userAppBaseHref.Length, filePath.Length - _userAppBaseHref.Length); if (suffix != "/") From e5c0ee60d321c2612b22fe82168b14f99c028e22 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Thu, 1 Oct 2026 15:55:37 -0400 Subject: [PATCH 02/10] feat: allowedClientNetworks allowlist and source-address logging for the Mobile Control direct server The direct server listens on every interface, and the Mobile Control port is commonly published to the LAN with a router port map (ADDPORTMAP) so that laptops and tablets can reach a server that Crestron places on the Control Subnet. There was no way to limit who could use it, and GET requests did not log the client address, so unwanted traffic could not be attributed to a host. allowedClientNetworks (directServer, list of CIDR strings, default empty): - When it has entries, HTTP requests (GET, POST, OPTIONS) from any other address get a 403. Loopback and clients on the Control Subnet are always allowed, so touchpanels keep working without being listed. - Null or empty means no filtering, so existing deployments are unchanged. - Invalid entries are logged and skipped. A list of only invalid entries still turns filtering on, so a typo cannot silently open the server back up. - Websocket connections are not covered; their paths already embed a per-client token. - IPv4-mapped IPv6 addresses (::ffff:a.b.c.d) are unwrapped before matching. The check runs first in each handler, before any routing or file access. It cannot help if a fault is in WebSocketSharp's own request parsing, which runs before these handlers. Logging: - GET requests now log the source address. The POST handler already did. - Logged paths are truncated at 200 characters, since a hostile request can carry an arbitrarily long one. - Refused requests are logged at Warning, and requests to unrecognised paths at Information, each at most once per minute per source address. A scan sending hundreds of requests produces a handful of lines. The rate-limit table is capped and cleared if it grows past 512 entries. Tested on a CP4N (firmware 2.8006) with a port map 50001 -> Control Subnet address in place. A client on the LAN got 200 with no allowlist, 403 with an allowlist that excluded it (logged at Warning with its address), and 200 with an allowlist that included it. The server saw the client's real address through the port map, so the allowlist still applies to forwarded traffic. CIDR parsing and matching were also checked in a standalone harness (28 cases including /25 and /29 boundaries, bare addresses, malformed input and mapped addresses); there is no test project for this assembly. Co-Authored-By: Claude Sonnet 5.5 --- .../MobileControlConfig.cs | 13 + .../MobileControlWebsocketServer.cs | 250 +++++++++++++++++- 2 files changed, 261 insertions(+), 2 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs index 963e7fd58..5225bf41a 100644 --- a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs +++ b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs @@ -77,6 +77,19 @@ public class MobileControlDirectServerPropertiesConfig [JsonProperty("automaticallyForwardPortToCSLAN")] public bool? AutomaticallyForwardPortToCSLAN { get; set; } + /// + /// Gets or sets the networks (CIDR notation) allowed to make HTTP requests to the direct server + /// + /// + /// Example: ["192.168.10.0/24", "192.168.5.10/32"]. When the list has any entries, HTTP requests + /// (GET, POST, OPTIONS) from any other address receive a 403, except loopback and clients on the + /// Control Subnet, which are always allowed. When null or empty, no filtering is done (default). + /// Invalid entries are logged and skipped, so a list containing only invalid entries still turns + /// filtering on. Does not apply to websocket connections, which are already gated by a per-client token. + /// + [JsonProperty("allowedClientNetworks")] + public List AllowedClientNetworks { get; set; } + /// /// Gets or sets the CSLanUiDeviceKeys /// diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 458c718cf..c92154723 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -273,6 +273,225 @@ private void AddConsoleCommands() CrestronConsole.AddNewConsoleCommand(RemoveAllTokens, "MobileRemoveAllClients", "Removes all clients", ConsoleAccessLevelEnum.AccessOperator); } + private struct AllowedNetwork + { + public byte[] Address; + public int PrefixLength; + } + + // null = no filtering configured + private List _allowedNetworks; + + // Last time a rate-limited message was logged, keyed by message kind + source address + private readonly ConcurrentDictionary _lastLogged = new ConcurrentDictionary(); + + private static readonly TimeSpan _logInterval = TimeSpan.FromSeconds(60); + + private const int MaxLoggedPathLength = 200; + + private const int MaxRateLimitEntries = 512; + + /// + /// Parses allowedClientNetworks. Invalid entries are logged and skipped. + /// + private void LoadAllowedNetworks() + { + var configured = _parent.Config.DirectServer.AllowedClientNetworks; + + if (configured == null || configured.Count == 0) + { + _allowedNetworks = null; + return; + } + + var parsed = new List(); + + foreach (var entry in configured) + { + if (TryParseCidr(entry, out var network)) + { + parsed.Add(network); + continue; + } + + this.LogWarning("Ignoring invalid allowedClientNetworks entry '{entry}'. Expected CIDR notation like 192.168.10.0/24", entry); + } + + _allowedNetworks = parsed; + + this.LogInformation("Restricting HTTP clients to the Control Subnet, loopback and {count} configured network(s)", parsed.Count); + } + + private static bool TryParseCidr(string value, out AllowedNetwork network) + { + network = default(AllowedNetwork); + + if (string.IsNullOrWhiteSpace(value)) + { + return false; + } + + var parts = value.Trim().Split('/'); + + if (parts.Length > 2 || !System.Net.IPAddress.TryParse(parts[0], out var address)) + { + return false; + } + + var bytes = address.GetAddressBytes(); + var prefix = bytes.Length * 8; + + if (parts.Length == 2 && (!int.TryParse(parts[1], out prefix) || prefix < 0 || prefix > bytes.Length * 8)) + { + return false; + } + + network = new AllowedNetwork { Address = bytes, PrefixLength = prefix }; + return true; + } + + /// + /// True for ::ffff:a.b.c.d, the form an IPv4 client takes on a dual-stack listener. + /// + private static bool IsIPv4MappedBytes(byte[] bytes) + { + for (var i = 0; i < 10; i++) + { + if (bytes[i] != 0) + { + return false; + } + } + + return bytes[10] == 0xFF && bytes[11] == 0xFF; + } + + private static bool IsInNetwork(byte[] remote, AllowedNetwork network) + { + if (remote.Length != network.Address.Length) + { + return false; + } + + var fullBytes = network.PrefixLength / 8; + var remainingBits = network.PrefixLength % 8; + + for (var i = 0; i < fullBytes; i++) + { + if (remote[i] != network.Address[i]) + { + return false; + } + } + + if (remainingBits == 0) + { + return true; + } + + var mask = (byte)(0xFF << (8 - remainingBits)); + return (remote[fullBytes] & mask) == (network.Address[fullBytes] & mask); + } + + /// + /// True if a request from this address should be served. + /// + private bool IsClientAllowed(System.Net.IPAddress remote) + { + if (_allowedNetworks == null) + { + return true; + } + + if (remote == null) + { + return false; + } + + if (System.Net.IPAddress.IsLoopback(remote)) + { + return true; + } + + var bytes = remote.GetAddressBytes(); + + // An IPv4 address can arrive as an IPv4-mapped IPv6 address (::ffff:a.b.c.d) + if (bytes.Length == 16 && IsIPv4MappedBytes(bytes)) + { + var v4 = new byte[4]; + Array.Copy(bytes, 12, v4, 0, 4); + bytes = v4; + remote = new System.Net.IPAddress(v4); + } + + if (csIpAddress != null && csSubnetMask != null && remote.IsInSameSubnet(csIpAddress, csSubnetMask)) + { + return true; + } + + foreach (var network in _allowedNetworks) + { + if (IsInNetwork(bytes, network)) + { + return true; + } + } + + return false; + } + + /// + /// Refuses the request with a 403 if the client is not allowed. Returns true if it was refused. + /// + private bool RejectIfNotAllowed(HttpListenerRequest req, HttpListenerResponse res) + { + var remote = req.RemoteEndPoint?.Address; + + if (IsClientAllowed(remote)) + { + return false; + } + + LogRateLimited("rejected", remote, () => + this.LogWarning("Refused HTTP request from {host}: not in the Control Subnet or allowedClientNetworks", remote)); + + res.StatusCode = 403; + res.Close(); + return true; + } + + /// + /// Runs the log action at most once per interval for each (kind, address) pair, so that a scan + /// producing hundreds of requests cannot flood the log. + /// + private void LogRateLimited(string kind, System.Net.IPAddress remote, Action log) + { + var key = kind + "|" + (remote?.ToString() ?? "unknown"); + var now = DateTime.UtcNow; + + if (_lastLogged.Count > MaxRateLimitEntries) + { + _lastLogged.Clear(); + } + + if (_lastLogged.TryGetValue(key, out var last) && now - last < _logInterval) + { + return; + } + + _lastLogged[key] = now; + log(); + } + + private static string TruncateForLog(string value) + { + if (value == null || value.Length <= MaxLoggedPathLength) + { + return value; + } + + return value.Substring(0, MaxLoggedPathLength) + "...(" + value.Length + " chars)"; + } /// /// Initialize method @@ -284,6 +503,8 @@ public override void Initialize() { base.Initialize(); + LoadAllowedNetworks(); + _server = new HttpServer(Port, _parent.Config.DirectServer.Secure); _server.OnGet += Server_OnGet; @@ -1059,6 +1280,12 @@ private void Server_OnGet(object sender, HttpRequestEventArgs e) { var req = e.Request; var res = e.Response; + + if (RejectIfNotAllowed(req, res)) + { + return; + } + res.ContentEncoding = Encoding.UTF8; res.AddHeader("Access-Control-Allow-Origin", "*"); @@ -1066,8 +1293,11 @@ private void Server_OnGet(object sender, HttpRequestEventArgs e) AddNoCacheHeaders(res); var path = req.RawUrl; + var remote = req.RemoteEndPoint?.Address; - this.LogVerbose("GET Request received at path: {path}", path); + // Source address included so a scan can be attributed to a host. Path is truncated + // because a hostile request can carry an arbitrarily long one. + this.LogVerbose("GET Request received at path: {path} from host {host}", TruncateForLog(path), remote); // Call for user app to join the room with a token if (path.StartsWith("/mc/api/ui/joinroom")) @@ -1091,6 +1321,9 @@ private void Server_OnGet(object sender, HttpRequestEventArgs e) else { // All other paths + LogRateLimited("unrecognised", remote, () => + this.LogInformation("Unrecognised request path from {host}: {path}", remote, TruncateForLog(path))); + res.StatusCode = 404; res.Close(); } @@ -1109,6 +1342,11 @@ private async void Server_OnPost(object sender, HttpRequestEventArgs e) var req = e.Request; var res = e.Response; + if (RejectIfNotAllowed(req, res)) + { + return; + } + res.AddHeader("Access-Control-Allow-Origin", "*"); AddNoCacheHeaders(res); @@ -1116,7 +1354,7 @@ private async void Server_OnPost(object sender, HttpRequestEventArgs e) var path = req.RawUrl; var ip = req.RemoteEndPoint.Address.ToString(); - this.LogVerbose("POST Request received at path: {path} from host {host}", path, ip); + this.LogVerbose("POST Request received at path: {path} from host {host}", TruncateForLog(path), ip); var body = new StreamReader(req.InputStream).ReadToEnd(); @@ -1155,6 +1393,11 @@ private void Server_OnOptions(object sender, HttpRequestEventArgs e) { var res = e.Response; + if (RejectIfNotAllowed(e.Request, res)) + { + return; + } + res.AddHeader("Access-Control-Allow-Origin", "*"); res.AddHeader("Access-Control-Allow-Methods", "GET, POST, OPTIONS"); res.AddHeader("Access-Control-Allow-Headers", "Content-Type, Accept, X-Requested-With, remember-me"); @@ -1361,6 +1604,9 @@ private void HandleUserAppRequest(HttpListenerRequest req, HttpListenerResponse //string filePath = path.Replace(string.Format("?token={0}", token), ""); // if there's no file suffix strip any extra path data after the base href + // Note: this used to be `_userAppBaseHref += "/"` inside the condition, which silently appended a + // slash to the shared field the first time it was evaluated and changed every later request's + // path handling. Compare against a copy instead. if (filePath != _userAppBaseHref && !filePath.Contains(".") && (!filePath.EndsWith(_userAppBaseHref) || !filePath.EndsWith(_userAppBaseHref + "/"))) { var suffix = filePath.Substring(_userAppBaseHref.Length, filePath.Length - _userAppBaseHref.Length); From 446dfded1c7be7c9072ce98429861eb08ae6b433 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Thu, 1 Oct 2026 16:18:37 -0400 Subject: [PATCH 03/10] fix: drop refused connections with Abort() instead of replying 403 A refused request was answered with a 403 and Close(), which writes a response. Writing to a socket the other end has already reset throws from inside the HTTP stack. In the field this showed up as IOException: Unable to write data to the transport connection: Connection reset by peer at Socket.Send ... ResponseStream.flushHeaders ... HttpConnection.Close at HttpListenerResponse.Close () at MobileControlWebsocketServer.Server_OnGet logged one second before a process abort during a vulnerability scan. That particular exception was caught by the handler, but the same failure on a thread-pool thread outside a try/catch would not be. HttpListenerResponse.Abort() closes the connection without writing anything, so there is nothing to fail on a reset connection. It also gives an unwanted client no reply to work with. Failures from Abort() itself are swallowed and logged at Debug: the connection being gone is the outcome we wanted. Behavior change: a refused client now sees a closed connection instead of a 403. Allowed clients are unaffected. Co-Authored-By: Claude Sonnet 5.5 --- .../MobileControlConfig.cs | 5 ++-- .../MobileControlWebsocketServer.cs | 27 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs index 5225bf41a..29868fc3e 100644 --- a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs +++ b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs @@ -82,8 +82,9 @@ public class MobileControlDirectServerPropertiesConfig /// /// /// Example: ["192.168.10.0/24", "192.168.5.10/32"]. When the list has any entries, HTTP requests - /// (GET, POST, OPTIONS) from any other address receive a 403, except loopback and clients on the - /// Control Subnet, which are always allowed. When null or empty, no filtering is done (default). + /// (GET, POST, OPTIONS) from any other address have the connection closed without a response, + /// except loopback and clients on the Control Subnet, which are always allowed. + /// When null or empty, no filtering is done (default). /// Invalid entries are logged and skipped, so a list containing only invalid entries still turns /// filtering on. Does not apply to websocket connections, which are already gated by a per-client token. /// diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index c92154723..7bdd21569 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -441,7 +441,7 @@ private bool IsClientAllowed(System.Net.IPAddress remote) } /// - /// Refuses the request with a 403 if the client is not allowed. Returns true if it was refused. + /// Drops the connection without a response if the client is not allowed. Returns true if it was refused. /// private bool RejectIfNotAllowed(HttpListenerRequest req, HttpListenerResponse res) { @@ -455,11 +455,32 @@ private bool RejectIfNotAllowed(HttpListenerRequest req, HttpListenerResponse re LogRateLimited("rejected", remote, () => this.LogWarning("Refused HTTP request from {host}: not in the Control Subnet or allowedClientNetworks", remote)); - res.StatusCode = 403; - res.Close(); + DropConnection(res); return true; } + /// + /// Closes the connection without writing a response. + /// + /// + /// Used for requests we are refusing. Writing even a short reply means a send on a socket the other end + /// may already have reset, which throws from inside the HTTP stack (seen in the field as + /// "Unable to write data to the transport connection: Connection reset by peer" from + /// HttpListenerResponse.Close). Abort() writes nothing, so there is nothing to fail. + /// + private void DropConnection(HttpListenerResponse res) + { + try + { + res.Abort(); + } + catch (Exception ex) + { + // The connection is already gone, which is the outcome we wanted + this.LogDebug("Exception dropping connection: {message}", ex.Message); + } + } + /// /// Runs the log action at most once per interval for each (kind, address) pair, so that a scan /// producing hundreds of requests cannot flood the log. From b0a9ab551e7bc7a0997ab8a5409b3d0c5b10fa03 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Thu, 1 Oct 2026 16:19:10 -0400 Subject: [PATCH 04/10] fix: drop connections for unrecognised GET paths instead of replying 404 Server_OnGet answered any path it does not handle with a 404 and Close(), which writes a response. The field exception recorded one second before a process abort came through exactly this branch: a request for an unhandled path, then "Connection reset by peer" from HttpListenerResponse.Close(). Use the same DropConnection() helper as refused requests, so no reply is written. The handled paths (/mc/app, /mc/api/version, /mc/api/ui/joinroom, /mc/app/logo) are unchanged. The unrecognised-path log line is kept, so these requests are still recorded with their source address. Behavior change: a client asking for an unhandled path (for example a browser requesting /favicon.ico) now sees a closed connection instead of a 404. This is a separate commit so it can be dropped on its own if that is not wanted. Co-Authored-By: Claude Sonnet 5.5 --- .../WebSocketServer/MobileControlWebsocketServer.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 7bdd21569..2003ab75e 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -1345,8 +1345,9 @@ private void Server_OnGet(object sender, HttpRequestEventArgs e) LogRateLimited("unrecognised", remote, () => this.LogInformation("Unrecognised request path from {host}: {path}", remote, TruncateForLog(path))); - res.StatusCode = 404; - res.Close(); + // No reply: nothing legitimate asks for these paths, and a reply is a write that can + // fail on a connection the client has already reset + DropConnection(res); } } catch (Exception ex) From 38667f3e4785b27930ec783c4ccb743cd4d63a58 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Thu, 1 Oct 2026 18:14:18 -0400 Subject: [PATCH 05/10] feat: autoBlock for the Mobile Control direct server; make dropping unrecognised requests opt-in Two changes, both off unless configured, so a config without these settings behaves exactly as it did before. dropUnrecognisedRequests (directServer, default false) The previous commit dropped the connection for any GET path the server does not handle, for every deployment. That changed default behavior (a 404 reply became a closed connection), so it is now opt-in. Absent or false replies 404 as before. autoBlock (directServer.autoBlock, default off) Counts requests that no legitimate client sends: requests for unrecognised paths and requests refused by allowedClientNetworks. When one address reaches requestsPerMinute (default 10) within a minute it is added to the processor's blocked-IP list with ADDBLOCKEDIP, which blocks it completely, and Essentials removes it again after blockMinutes (default 30). The processor's own SETLOCKOUTTIME does not apply to manual blocks (they list as "blocked indefinitely"), so the removal has to be done here. - Never blocks loopback, the Control Subnet, the processor's own addresses, allowedClientNetworks, or networks listed in neverBlock. - IPv4 only, 4-series appliances only. Anything that goes into a console command must first pass a strict dotted-decimal check. - dryRun logs what would be blocked and blocks nothing, for trying thresholds safely. - maxConcurrentBlocks (default 8) caps how many blocks Essentials holds. - Blocks survive a reboot and the program can stop at any time, so ownership is written to autoBlockedIps.json BEFORE the command runs. After a restart the program removes whatever has expired, even if autoBlock has since been turned off. It only ever removes addresses it added itself, never one added by hand, and never uses "remblockedip ALL". - Removal is confirmed against listblocked rather than the wording of the reply; if the address is still listed the entry is kept and retried. - Console calls run off the request thread. Blocks and removals log at Warning and Information with the address and the reason. Tested on a CP4N (firmware 2.8006) with ordinary requests to made-up paths from one LAN client: - no new settings: unknown path -> 404, nothing blocked, no state file - dropUnrecognisedRequests: unknown path -> connection closed, real requests fine - autoBlock + dryRun: logged "Would block" at exactly the 10th request, nothing blocked - autoBlock with the client in neverBlock: 14 requests, not blocked Not tested on hardware: an actual block and its expiry, because that needs a second client (the only route to the test processor was the address that would be blocked). The decision logic (window counting, address validation, parsing the block list, IPv4-mapped addresses; 41 cases) was tested separately by extracting it from this file. Co-Authored-By: Claude Sonnet 5.5 --- .../MobileControlConfig.cs | 69 +++ .../MobileControlWebsocketServer.cs | 519 +++++++++++++++++- 2 files changed, 585 insertions(+), 3 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs index 29868fc3e..5dd1d1b46 100644 --- a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs +++ b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs @@ -91,6 +91,26 @@ public class MobileControlDirectServerPropertiesConfig [JsonProperty("allowedClientNetworks")] public List AllowedClientNetworks { get; set; } + /// + /// Gets or sets whether a request for a path the server does not handle gets no reply + /// + /// + /// When true the connection is closed without a response. When false or absent (default) the server + /// replies 404, as it always has. Replying means writing to a connection the client may already have + /// reset, which throws from inside the HTTP stack, so noisy environments may prefer true. + /// + [JsonProperty("dropUnrecognisedRequests")] + public bool? DropUnrecognisedRequests { get; set; } + + /// + /// Gets or sets the automatic blocking of addresses that send a burst of unwanted requests + /// + /// + /// Absent or "enabled": false (default) means no automatic blocking. + /// + [JsonProperty("autoBlock")] + public MobileControlAutoBlockConfig AutoBlock { get; set; } + /// /// Gets or sets the CSLanUiDeviceKeys /// @@ -121,6 +141,55 @@ public MobileControlDirectServerPropertiesConfig() /// /// Represents a MobileControlLoggingConfig /// + /// + /// Settings for blocking, at the processor, an address that sends a burst of unwanted requests + /// + /// + /// Counts requests for unrecognised paths and requests refused by allowedClientNetworks. When one address + /// reaches requestsPerMinute within a minute it is added to the processor's blocked-IP list (a total block, + /// every port) and removed again after blockMinutes. The processor's own lockout setting does not apply to + /// manual blocks, so Essentials removes only the blocks it added. 4-series appliances only. + /// Never blocks loopback, the Control Subnet, the processor's own addresses, allowedClientNetworks or neverBlock. + /// + public class MobileControlAutoBlockConfig + { + /// + /// Gets or sets whether automatic blocking is on (default false) + /// + [JsonProperty("enabled")] + public bool Enabled { get; set; } + + /// + /// Gets or sets whether to only log what would be blocked, without blocking anything + /// + [JsonProperty("dryRun")] + public bool DryRun { get; set; } + + /// + /// Gets or sets how many unwanted requests from one address within a minute trigger a block (default 10, minimum 3) + /// + [JsonProperty("requestsPerMinute")] + public int RequestsPerMinute { get; set; } = 10; + + /// + /// Gets or sets how long a block lasts, in minutes (default 30, 1 to 1440) + /// + [JsonProperty("blockMinutes")] + public int BlockMinutes { get; set; } = 30; + + /// + /// Gets or sets the most blocks Essentials will hold at once (default 8, 1 to 64) + /// + [JsonProperty("maxConcurrentBlocks")] + public int MaxConcurrentBlocks { get; set; } = 8; + + /// + /// Gets or sets networks (CIDR notation) that are never blocked, for example VPN and monitoring hosts + /// + [JsonProperty("neverBlock")] + public List NeverBlock { get; set; } + } + public class MobileControlLoggingConfig { diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 2003ab75e..c43744e73 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -455,6 +455,8 @@ private bool RejectIfNotAllowed(HttpListenerRequest req, HttpListenerResponse re LogRateLimited("rejected", remote, () => this.LogWarning("Refused HTTP request from {host}: not in the Control Subnet or allowedClientNetworks", remote)); + RecordUnwantedRequest(remote); + DropConnection(res); return true; } @@ -514,6 +516,499 @@ private static string TruncateForLog(string value) return value.Substring(0, MaxLoggedPathLength) + "...(" + value.Length + " chars)"; } + // ------------------------------------------------------------------------------------------------ + // Automatic blocking of addresses that send a burst of unwanted requests (off unless configured) + // ------------------------------------------------------------------------------------------------ + + /// + /// Counts events per key inside a sliding window. Not thread-safe: callers lock. + /// + private sealed class SlidingWindowCounter + { + private readonly Dictionary> _events = new Dictionary>(); + private readonly TimeSpan _window; + private readonly int _maxKeys; + + public SlidingWindowCounter(TimeSpan window, int maxKeys) + { + _window = window; + _maxKeys = maxKeys; + } + + /// + /// Records an event and returns how many fall inside the window, including this one. + /// + public int Record(string key, DateTime now) + { + Queue queue; + + if (!_events.TryGetValue(key, out queue)) + { + // Bounded memory. Losing counts only delays a block; it can never cause one. + if (_events.Count >= _maxKeys) + { + _events.Clear(); + } + + queue = new Queue(); + _events[key] = queue; + } + + queue.Enqueue(now); + + while (queue.Count > 0 && now - queue.Peek() > _window) + { + queue.Dequeue(); + } + + return queue.Count; + } + + public void Reset(string key) + { + _events.Remove(key); + } + } + + private static readonly System.Text.RegularExpressions.Regex Ipv4Literal = new System.Text.RegularExpressions.Regex( + @"^(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}\z"); + + /// + /// True only for a plain dotted-decimal IPv4 address. Anything that goes into a console command + /// has to pass this first. + /// + private static bool IsIpv4Literal(string value) + { + return value != null && Ipv4Literal.IsMatch(value); + } + + /// + /// True if the output of listblocked names this address as an entry of its own. + /// + private static bool BlockListContains(string listOutput, string address) + { + if (string.IsNullOrEmpty(listOutput) || !IsIpv4Literal(address)) + { + return false; + } + + return System.Text.RegularExpressions.Regex.IsMatch( + listOutput, + @"(^|\s)" + System.Text.RegularExpressions.Regex.Escape(address) + @"(\s|$)", + System.Text.RegularExpressions.RegexOptions.Multiline); + } + + private const string AutoBlockFileName = "autoBlockedIps.json"; + + private const int SuspiciousWindowSeconds = 60; + + private const int MaxTrackedAddresses = 512; + + private bool _autoBlockEnabled; + private bool _autoBlockDryRun; + private int _autoBlockThreshold = 10; + private TimeSpan _autoBlockDuration = TimeSpan.FromMinutes(30); + private int _autoBlockMaxConcurrent = 8; + private List _neverBlockNetworks = new List(); + private System.Net.IPAddress _lanIpAddress; + private CTimer _autoBlockTimer; + private int _expiryRunning; + + private readonly object _autoBlockLock = new object(); + private readonly SlidingWindowCounter _unwantedRequests = new SlidingWindowCounter(TimeSpan.FromSeconds(SuspiciousWindowSeconds), MaxTrackedAddresses); + + // address -> when Essentials removes the block (UTC). Only blocks Essentials added are ever listed here, + // so a block someone added by hand is never removed. + private readonly Dictionary _autoBlocked = new Dictionary(); + + private string AutoBlockFilePath + { + get { return Global.FilePathPrefix + AutoBlockFileName; } + } + + private List ParseNetworks(List entries, string settingName) + { + var parsed = new List(); + + if (entries == null) + { + return parsed; + } + + foreach (var entry in entries) + { + AllowedNetwork network; + + if (TryParseCidr(entry, out network)) + { + parsed.Add(network); + continue; + } + + this.LogWarning("Ignoring invalid {setting} entry '{entry}'. Expected CIDR notation like 192.168.10.0/24", settingName, entry); + } + + return parsed; + } + + private void LoadAutoBlockSettings() + { + var config = _parent.Config.DirectServer.AutoBlock; + var wanted = config != null && config.Enabled; + + if (CrestronEnvironment.DevicePlatform != eDevicePlatform.Appliance) + { + if (wanted) + { + this.LogWarning("autoBlock needs a 4-series appliance and is ignored on this platform"); + } + + return; + } + + // Blocks survive a reboot, so anything Essentials added before has to be removed on schedule even if + // the setting has since been turned off. + LoadAutoBlockedFromDisk(); + + if (wanted) + { + _autoBlockDryRun = config.DryRun; + _autoBlockThreshold = Math.Max(3, config.RequestsPerMinute); + _autoBlockDuration = TimeSpan.FromMinutes(Math.Min(1440, Math.Max(1, config.BlockMinutes))); + _autoBlockMaxConcurrent = Math.Min(64, Math.Max(1, config.MaxConcurrentBlocks)); + _neverBlockNetworks = ParseNetworks(config.NeverBlock, "autoBlock.neverBlock"); + + try + { + var lanAdapterId = CrestronEthernetHelper.GetAdapterdIdForSpecifiedAdapterType(EthernetAdapterType.EthernetLANAdapter); + _lanIpAddress = System.Net.IPAddress.Parse(CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_CURRENT_IP_ADDRESS, lanAdapterId)); + } + catch (Exception ex) + { + this.LogDebug("Could not read the LAN address for the auto-block exemptions: {message}", ex.Message); + } + + _autoBlockEnabled = true; + + this.LogInformation( + "Auto-block is on{dryRun}: {threshold} unwanted requests in a minute blocks an address for {minutes} minutes (at most {max} at once)", + _autoBlockDryRun ? " (dry run, nothing will be blocked)" : string.Empty, + _autoBlockThreshold, (int)_autoBlockDuration.TotalMinutes, _autoBlockMaxConcurrent); + } + + bool outstanding; + lock (_autoBlockLock) + { + outstanding = _autoBlocked.Count > 0; + } + + if (_autoBlockEnabled || outstanding) + { + _autoBlockTimer = new CTimer(CheckAutoBlockExpiry, null, 5000, 30000); + } + } + + /// + /// The IPv4 form of an address, unwrapping IPv4-mapped IPv6. False for anything else. + /// + private static bool TryGetIpv4(System.Net.IPAddress address, out System.Net.IPAddress ipv4) + { + ipv4 = null; + + if (address == null) + { + return false; + } + + var bytes = address.GetAddressBytes(); + + if (bytes.Length == 16 && IsIPv4MappedBytes(bytes)) + { + var v4 = new byte[4]; + Array.Copy(bytes, 12, v4, 0, 4); + ipv4 = new System.Net.IPAddress(v4); + return true; + } + + if (bytes.Length == 4) + { + ipv4 = address; + return true; + } + + return false; + } + + /// + /// Addresses that are never blocked: this processor, its Control Subnet, loopback and anything the + /// installer has listed as trusted. + /// + private bool IsExemptFromAutoBlock(System.Net.IPAddress ipv4) + { + if (System.Net.IPAddress.IsLoopback(ipv4)) + { + return true; + } + + if (csIpAddress != null && csSubnetMask != null && ipv4.IsInSameSubnet(csIpAddress, csSubnetMask)) + { + return true; + } + + if (ipv4.Equals(csIpAddress) || ipv4.Equals(_lanIpAddress)) + { + return true; + } + + var bytes = ipv4.GetAddressBytes(); + + foreach (var network in _neverBlockNetworks) + { + if (IsInNetwork(bytes, network)) + { + return true; + } + } + + if (_allowedNetworks != null) + { + foreach (var network in _allowedNetworks) + { + if (IsInNetwork(bytes, network)) + { + return true; + } + } + } + + return false; + } + + /// + /// Counts a request that no legitimate client sends. Blocks the address once it sends too many. + /// + private void RecordUnwantedRequest(System.Net.IPAddress remote) + { + if (!_autoBlockEnabled) + { + return; + } + + System.Net.IPAddress ipv4; + + if (!TryGetIpv4(remote, out ipv4) || IsExemptFromAutoBlock(ipv4)) + { + return; + } + + var address = ipv4.ToString(); + int count; + + lock (_autoBlockLock) + { + if (_autoBlocked.ContainsKey(address)) + { + return; + } + + count = _unwantedRequests.Record(address, DateTime.UtcNow); + + if (count < _autoBlockThreshold) + { + return; + } + + _unwantedRequests.Reset(address); + } + + BlockAddress(address, count); + } + + private void BlockAddress(string address, int count) + { + if (!IsIpv4Literal(address)) + { + return; + } + + if (_autoBlockDryRun) + { + LogRateLimited("dryrun", System.Net.IPAddress.Parse(address), () => + this.LogWarning("[dry run] Would block {address} for {minutes} minutes: {count} unwanted requests within a minute", address, (int)_autoBlockDuration.TotalMinutes, count)); + return; + } + + lock (_autoBlockLock) + { + if (_autoBlocked.Count >= _autoBlockMaxConcurrent) + { + LogRateLimited("autoblock-cap", null, () => + this.LogWarning("Not blocking {address}: already holding {max} automatic blocks", address, _autoBlockMaxConcurrent)); + return; + } + + // Recorded before the command runs. If the program stopped between the two, the block would + // otherwise outlive it with nobody responsible for removing it. + _autoBlocked[address] = DateTime.UtcNow + _autoBlockDuration; + SaveAutoBlocked(); + } + + // The console call can take a moment, so keep it off the request thread + CrestronInvoke.BeginInvoke(o => ExecuteBlock(address, count)); + } + + private void ExecuteBlock(string address, int count) + { + try + { + var response = string.Empty; + CrestronConsole.SendControlSystemCommand("addblockedip " + address, ref response); + + if (response != null && response.IndexOf("Added IP", StringComparison.OrdinalIgnoreCase) >= 0) + { + this.LogWarning("Blocked {address} for {minutes} minutes: {count} unwanted requests within a minute", address, (int)_autoBlockDuration.TotalMinutes, count); + return; + } + + this.LogWarning("Could not block {address}. The console said: {response}", address, response == null ? string.Empty : response.Trim()); + } + catch (Exception ex) + { + this.LogError("Exception blocking {address}: {message}", address, ex.Message); + } + + // Not blocked, so Essentials has nothing to remove later + lock (_autoBlockLock) + { + _autoBlocked.Remove(address); + SaveAutoBlocked(); + } + } + + private void CheckAutoBlockExpiry(object unused) + { + // One pass at a time: the console calls can outlast the timer interval + if (System.Threading.Interlocked.CompareExchange(ref _expiryRunning, 1, 0) != 0) + { + return; + } + + try + { + List due; + + lock (_autoBlockLock) + { + var now = DateTime.UtcNow; + due = _autoBlocked.Where(kv => kv.Value <= now).Select(kv => kv.Key).ToList(); + } + + foreach (var address in due) + { + RemoveBlock(address); + } + } + catch (Exception ex) + { + this.LogError("Exception removing expired blocks: {message}", ex.Message); + } + finally + { + System.Threading.Interlocked.Exchange(ref _expiryRunning, 0); + } + } + + private void RemoveBlock(string address) + { + if (IsIpv4Literal(address)) + { + var response = string.Empty; + CrestronConsole.SendControlSystemCommand("remblockedip " + address, ref response); + + // Confirm it is gone rather than trusting the wording of the reply. If it is still listed the entry + // stays and the next pass tries again, so a failed removal is never forgotten. + var list = string.Empty; + CrestronConsole.SendControlSystemCommand("listblocked", ref list); + + if (BlockListContains(list, address)) + { + this.LogWarning("{address} is still blocked after trying to remove it. Will retry", address); + return; + } + } + + lock (_autoBlockLock) + { + _autoBlocked.Remove(address); + SaveAutoBlocked(); + } + + this.LogInformation("Unblocked {address}: its automatic block expired", address); + } + + // Caller holds _autoBlockLock + private void SaveAutoBlocked() + { + try + { + var path = AutoBlockFilePath; + + if (_autoBlocked.Count == 0) + { + if (File.Exists(path)) + { + File.Delete(path); + } + + return; + } + + var data = _autoBlocked.ToDictionary(kv => kv.Key, kv => kv.Value.ToString("o")); + File.WriteAllText(path, JsonConvert.SerializeObject(data, Formatting.Indented)); + } + catch (Exception ex) + { + this.LogError("Could not save the list of automatic blocks: {message}", ex.Message); + } + } + + private void LoadAutoBlockedFromDisk() + { + try + { + var path = AutoBlockFilePath; + + if (!File.Exists(path)) + { + return; + } + + var data = JsonConvert.DeserializeObject>(File.ReadAllText(path)); + + if (data == null) + { + return; + } + + lock (_autoBlockLock) + { + foreach (var entry in data) + { + DateTime expiry; + + if (IsIpv4Literal(entry.Key) && DateTime.TryParse(entry.Value, null, System.Globalization.DateTimeStyles.RoundtripKind, out expiry)) + { + _autoBlocked[entry.Key] = expiry.ToUniversalTime(); + } + } + } + } + catch (Exception ex) + { + this.LogError("Could not read the list of automatic blocks: {message}", ex.Message); + } + } + /// /// Initialize method /// @@ -526,6 +1021,8 @@ public override void Initialize() LoadAllowedNetworks(); + LoadAutoBlockSettings(); + _server = new HttpServer(Port, _parent.Config.DirectServer.Secure); _server.OnGet += Server_OnGet; @@ -1266,6 +1763,12 @@ private void CrestronEnvironment_ProgramStatusEventHandler(eProgramStatusEventTy { if (programEventType == eProgramStatusEventType.Stopping) { + if (_autoBlockTimer != null) + { + _autoBlockTimer.Stop(); + _autoBlockTimer.Dispose(); + } + foreach (var client in UiClients.Values) { if (client != null && client.Context.WebSocket.IsAlive) @@ -1345,9 +1848,19 @@ private void Server_OnGet(object sender, HttpRequestEventArgs e) LogRateLimited("unrecognised", remote, () => this.LogInformation("Unrecognised request path from {host}: {path}", remote, TruncateForLog(path))); - // No reply: nothing legitimate asks for these paths, and a reply is a write that can - // fail on a connection the client has already reset - DropConnection(res); + RecordUnwantedRequest(remote); + + if (_parent.Config.DirectServer.DropUnrecognisedRequests == true) + { + // No reply: nothing legitimate asks for these paths, and a reply is a write that can + // fail on a connection the client has already reset + DropConnection(res); + } + else + { + res.StatusCode = 404; + res.Close(); + } } } catch (Exception ex) From 9216f7051181b1930375f797aa52d753e4156b63 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Thu, 1 Oct 2026 18:35:23 -0400 Subject: [PATCH 06/10] fix: don't count ordinary browser requests as unwanted traffic Loading the app in a browser makes two requests that this server has never answered, and autoBlock was counting them: - The app's index.html sets its own from an inline script, but the browser's preload scanner requests ./assets/index-*.js and *.css first, relative to /mc/. Every page load therefore asks for /mc/assets/... and gets a 404, then succeeds a moment later under /mc/app/assets/... once the base is applied. This has always happened; it only became visible with the unrecognised-path log line. - Browsers ask for /favicon.ico. At the default of 10 unwanted requests per minute, five page loads in a minute would have been counted as an attack, so a person reloading the app a few times (or a tablet reconnecting) could have been blocked. Requests for /mc/assets/* and /favicon.ico are now neither logged at Information nor counted. They are still answered exactly as before (404, or a dropped connection when dropUnrecognisedRequests is set). Every other unrecognised path still counts, including near-misses such as /mc/assets, /mc/assetsx/ and /x/favicon.ico. Because /mc/assets/* is no longer counted, a client that sent only those could never be auto-blocked. Scanners send a wide spread of paths, and a flood of that one prefix is no worse than the same flood before this change. Verified on a CP4N with autoBlock in dry run at the default threshold: eight real Chrome page loads (16 requests to /mc/assets/) produced no "Would block", and 14 requests to made-up paths still produced one at the 10th. The path matching was tested separately (18 cases). Co-Authored-By: Claude Sonnet 5.5 --- .../MobileControlWebsocketServer.cs | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index c43744e73..47de32a9b 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -598,6 +598,29 @@ private static bool BlockListContains(string listOutput, string address) System.Text.RegularExpressions.RegexOptions.Multiline); } + /// + /// Requests that every ordinary browser makes and this server has never answered. + /// + /// + /// The app's index.html sets its own <base> from an inline script, but the browser's preload + /// scanner requests ./assets/* first, relative to /mc/, so each page load asks for /mc/assets/* and gets a + /// 404 before the real requests succeed under /mc/app/assets/. Browsers also ask for /favicon.ico. These + /// must not count as unwanted traffic, or a person reloading the app a few times would be blocked. + /// + private static bool IsBenignBrowserRequest(string path) + { + if (string.IsNullOrEmpty(path)) + { + return false; + } + + var queryStart = path.IndexOf('?'); + var withoutQuery = queryStart >= 0 ? path.Substring(0, queryStart) : path; + + return withoutQuery.StartsWith("/mc/assets/", StringComparison.Ordinal) + || string.Equals(withoutQuery, "/favicon.ico", StringComparison.OrdinalIgnoreCase); + } + private const string AutoBlockFileName = "autoBlockedIps.json"; private const int SuspiciousWindowSeconds = 60; @@ -1844,11 +1867,15 @@ private void Server_OnGet(object sender, HttpRequestEventArgs e) } else { - // All other paths - LogRateLimited("unrecognised", remote, () => - this.LogInformation("Unrecognised request path from {host}: {path}", remote, TruncateForLog(path))); + // All other paths. Browsers make a couple of these on every page load, so those are neither + // logged at Information nor counted towards an automatic block. + if (!IsBenignBrowserRequest(path)) + { + LogRateLimited("unrecognised", remote, () => + this.LogInformation("Unrecognised request path from {host}: {path}", remote, TruncateForLog(path))); - RecordUnwantedRequest(remote); + RecordUnwantedRequest(remote); + } if (_parent.Config.DirectServer.DropUnrecognisedRequests == true) { From 058f3b357307dd17e24d58ae5421a714ff71ee48 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Fri, 2 Oct 2026 07:33:32 -0400 Subject: [PATCH 07/10] fix(mobile-control): address review feedback on client filtering - Check loopback after unwrapping IPv4-mapped addresses, so a local request arriving as ::ffff:127.0.0.1 is not refused when filtering is on - Apply dropUnrecognisedRequests to unhandled POST paths too, matching GET Co-Authored-By: Claude Opus 5.5 --- .../MobileControlWebsocketServer.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 47de32a9b..a8503e6a8 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -408,11 +408,6 @@ private bool IsClientAllowed(System.Net.IPAddress remote) return false; } - if (System.Net.IPAddress.IsLoopback(remote)) - { - return true; - } - var bytes = remote.GetAddressBytes(); // An IPv4 address can arrive as an IPv4-mapped IPv6 address (::ffff:a.b.c.d) @@ -424,6 +419,12 @@ private bool IsClientAllowed(System.Net.IPAddress remote) remote = new System.Net.IPAddress(v4); } + // After unwrapping: IsLoopback is false for ::ffff:127.0.0.1 + if (System.Net.IPAddress.IsLoopback(remote)) + { + return true; + } + if (csIpAddress != null && csSubnetMask != null && remote.IsInSameSubnet(csIpAddress, csSubnetMask)) { return true; @@ -1937,6 +1938,11 @@ private async void Server_OnPost(object sender, HttpRequestEventArgs e) this.LogVerbose("Log data sent to {host}:{port}", _parent.Config.DirectServer.Logging.Host, _parent.Config.DirectServer.Logging.Port); } + else if (_parent.Config.DirectServer.DropUnrecognisedRequests == true) + { + // Same as an unrecognised GET: no reply, so there is no write to fail on a reset connection + DropConnection(res); + } else { res.StatusCode = 404; From be8515535fa851f999dca2766d695e889213de6e Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Fri, 2 Oct 2026 11:44:38 -0400 Subject: [PATCH 08/10] fix(mobile-control): harden auto-block ownership and POST filtering - Register the POST handler always, so POSTs go through the allowlist even when remote logging is off; log forwarding is gated inside the handler - Log unhandled POST paths and count them towards auto-block, like GETs - Only read the request body for /mc/api/log - Do not issue ADDBLOCKEDIP unless the ownership record was saved; write the record via temp file + rename - Recheck ownership under the lock before blocking, so two bursts cannot schedule the same block and drop each other's entry - Keep a block's entry when listblocked fails instead of treating an empty list as "removed" - Per-instance block file (autoBlockedIps-.json); the old shared file is migrated on load and then deleted - Make the log rate-limit check-and-set atomic - Restore the MobileControlLoggingConfig summary to its own class Co-Authored-By: Claude Opus 5.5 --- .../MobileControlConfig.cs | 6 +- .../MobileControlWebsocketServer.cs | 171 ++++++++++++++---- 2 files changed, 139 insertions(+), 38 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs index 5dd1d1b46..d6ffd1190 100644 --- a/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs +++ b/src/PepperDash.Essentials.MobileControl/MobileControlConfig.cs @@ -138,9 +138,6 @@ public MobileControlDirectServerPropertiesConfig() } } - /// - /// Represents a MobileControlLoggingConfig - /// /// /// Settings for blocking, at the processor, an address that sends a burst of unwanted requests /// @@ -190,6 +187,9 @@ public class MobileControlAutoBlockConfig public List NeverBlock { get; set; } } + /// + /// Represents a MobileControlLoggingConfig + /// public class MobileControlLoggingConfig { diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index a8503e6a8..b9c7af5c5 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -493,17 +493,23 @@ private void LogRateLimited(string kind, System.Net.IPAddress remote, Action log var key = kind + "|" + (remote?.ToString() ?? "unknown"); var now = DateTime.UtcNow; - if (_lastLogged.Count > MaxRateLimitEntries) + // Check and update together, or a burst of concurrent requests from one source would each see a + // stale timestamp and all log + lock (_lastLogged) { - _lastLogged.Clear(); - } + if (_lastLogged.Count > MaxRateLimitEntries) + { + _lastLogged.Clear(); + } - if (_lastLogged.TryGetValue(key, out var last) && now - last < _logInterval) - { - return; + if (_lastLogged.TryGetValue(key, out var last) && now - last < _logInterval) + { + return; + } + + _lastLogged[key] = now; } - _lastLogged[key] = now; log(); } @@ -622,7 +628,8 @@ private static bool IsBenignBrowserRequest(string path) || string.Equals(withoutQuery, "/favicon.ico", StringComparison.OrdinalIgnoreCase); } - private const string AutoBlockFileName = "autoBlockedIps.json"; + // Used by builds before the file was made per-instance. Read once and then removed. + private const string LegacyAutoBlockFileName = "autoBlockedIps.json"; private const int SuspiciousWindowSeconds = 60; @@ -645,9 +652,14 @@ private static bool IsBenignBrowserRequest(string path) // so a block someone added by hand is never removed. private readonly Dictionary _autoBlocked = new Dictionary(); + // One file per instance: each server owns the blocks it added, and more than one controller can be configured private string AutoBlockFilePath { - get { return Global.FilePathPrefix + AutoBlockFileName; } + get + { + var safeKey = new string(Key.Select(c => Path.GetInvalidFileNameChars().Contains(c) ? '_' : c).ToArray()); + return Global.FilePathPrefix + "autoBlockedIps-" + safeKey + ".json"; + } } private List ParseNetworks(List entries, string settingName) @@ -864,6 +876,13 @@ private void BlockAddress(string address, int count) lock (_autoBlockLock) { + // Another burst may have crossed the threshold since RecordUnwantedRequest released the lock. + // Without this a second ADDBLOCKEDIP would fail as a duplicate and drop the first one's entry. + if (_autoBlocked.ContainsKey(address)) + { + return; + } + if (_autoBlocked.Count >= _autoBlockMaxConcurrent) { LogRateLimited("autoblock-cap", null, () => @@ -872,9 +891,16 @@ private void BlockAddress(string address, int count) } // Recorded before the command runs. If the program stopped between the two, the block would - // otherwise outlive it with nobody responsible for removing it. + // otherwise outlive it with nobody responsible for removing it. For the same reason, no record + // on disk means no block. _autoBlocked[address] = DateTime.UtcNow + _autoBlockDuration; - SaveAutoBlocked(); + + if (!SaveAutoBlocked()) + { + _autoBlocked.Remove(address); + this.LogWarning("Not blocking {address}: the block could not be recorded, so it could not be removed after a restart", address); + return; + } } // The console call can take a moment, so keep it off the request thread @@ -952,7 +978,13 @@ private void RemoveBlock(string address) // Confirm it is gone rather than trusting the wording of the reply. If it is still listed the entry // stays and the next pass tries again, so a failed removal is never forgotten. var list = string.Empty; - CrestronConsole.SendControlSystemCommand("listblocked", ref list); + + // A failed query leaves the list empty, which would read as "removed" + if (!CrestronConsole.SendControlSystemCommand("listblocked", ref list)) + { + this.LogWarning("Could not confirm {address} was unblocked: listblocked failed. Will retry", address); + return; + } if (BlockListContains(list, address)) { @@ -970,8 +1002,11 @@ private void RemoveBlock(string address) this.LogInformation("Unblocked {address}: its automatic block expired", address); } - // Caller holds _autoBlockLock - private void SaveAutoBlocked() + /// + /// Writes the list of blocks Essentials owns. Returns false if it could not be written. + /// + /// Caller holds _autoBlockLock. + private bool SaveAutoBlocked() { try { @@ -984,34 +1019,84 @@ private void SaveAutoBlocked() File.Delete(path); } - return; + return true; } + // Written to a temporary file and then renamed over the real one, so a power loss mid-write cannot + // leave a truncated file that loses track of every block var data = _autoBlocked.ToDictionary(kv => kv.Key, kv => kv.Value.ToString("o")); - File.WriteAllText(path, JsonConvert.SerializeObject(data, Formatting.Indented)); + var tempPath = path + ".tmp"; + File.WriteAllText(tempPath, JsonConvert.SerializeObject(data, Formatting.Indented)); + + if (File.Exists(path)) + { + File.Replace(tempPath, path, null); + } + else + { + File.Move(tempPath, path); + } + + return true; } catch (Exception ex) { this.LogError("Could not save the list of automatic blocks: {message}", ex.Message); + return false; } } private void LoadAutoBlockedFromDisk() { - try + var path = AutoBlockFilePath; + var legacyPath = Global.FilePathPrefix + LegacyAutoBlockFileName; + + ReadAutoBlockedFile(path); + + if (!File.Exists(legacyPath)) { - var path = AutoBlockFilePath; + return; + } + // Take over blocks recorded under the old shared file name, then remove it once they are saved here + if (ReadAutoBlockedFile(legacyPath)) + { + lock (_autoBlockLock) + { + if (!SaveAutoBlocked()) + { + return; + } + } + + try + { + File.Delete(legacyPath); + } + catch (Exception ex) + { + this.LogDebug("Could not remove {path}: {message}", legacyPath, ex.Message); + } + } + } + + /// + /// Adds the entries in a block-list file to the blocks Essentials owns. Returns true if the file was read. + /// + private bool ReadAutoBlockedFile(string path) + { + try + { if (!File.Exists(path)) { - return; + return false; } var data = JsonConvert.DeserializeObject>(File.ReadAllText(path)); if (data == null) { - return; + return true; } lock (_autoBlockLock) @@ -1026,10 +1111,13 @@ private void LoadAutoBlockedFromDisk() } } } + + return true; } catch (Exception ex) { - this.LogError("Could not read the list of automatic blocks: {message}", ex.Message); + this.LogError("Could not read the list of automatic blocks from {path}: {message}", path, ex.Message); + return false; } } @@ -1053,10 +1141,8 @@ public override void Initialize() _server.OnOptions += Server_OnOptions; - if (_parent.Config.DirectServer.Logging.EnableRemoteLogging) - { - _server.OnPost += Server_OnPost; - } + // Always subscribed so POST requests go through the allowlist; log forwarding is gated inside + _server.OnPost += Server_OnPost; if (_parent.Config.DirectServer.Secure) { @@ -1915,17 +2001,24 @@ private async void Server_OnPost(object sender, HttpRequestEventArgs e) AddNoCacheHeaders(res); var path = req.RawUrl; - var ip = req.RemoteEndPoint.Address.ToString(); + var remote = req.RemoteEndPoint?.Address; + var ip = remote?.ToString(); this.LogVerbose("POST Request received at path: {path} from host {host}", TruncateForLog(path), ip); - var body = new StreamReader(req.InputStream).ReadToEnd(); - if (path.StartsWith("/mc/api/log")) { + var body = new StreamReader(req.InputStream).ReadToEnd(); + res.StatusCode = 200; res.Close(); + // The app posts here whether or not forwarding is on, so this is not an unwanted request + if (!_parent.Config.DirectServer.Logging.EnableRemoteLogging) + { + return; + } + // remote log collector has no dedicated secure flag; keep it on http regardless of DirectServer.Secure var logRequest = new HttpRequestMessage(HttpMethod.Post, $"http://{_parent.Config.DirectServer.Logging.Host}:{_parent.Config.DirectServer.Logging.Port}/logs") { @@ -1938,15 +2031,23 @@ private async void Server_OnPost(object sender, HttpRequestEventArgs e) this.LogVerbose("Log data sent to {host}:{port}", _parent.Config.DirectServer.Logging.Host, _parent.Config.DirectServer.Logging.Port); } - else if (_parent.Config.DirectServer.DropUnrecognisedRequests == true) - { - // Same as an unrecognised GET: no reply, so there is no write to fail on a reset connection - DropConnection(res); - } else { - res.StatusCode = 404; - res.Close(); + // Treated like an unrecognised GET: logged, counted towards an automatic block, then dropped or 404 + LogRateLimited("unrecognised", remote, () => + this.LogInformation("Unrecognised POST path from {host}: {path}", remote, TruncateForLog(path))); + + RecordUnwantedRequest(remote); + + if (_parent.Config.DirectServer.DropUnrecognisedRequests == true) + { + DropConnection(res); + } + else + { + res.StatusCode = 404; + res.Close(); + } } } catch (Exception ex) From acfab3ecfb4587865037aa6738f5b39651d4f6d9 Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Sat, 3 Oct 2026 12:32:40 -0400 Subject: [PATCH 09/10] fix(mobile-control): close block/expiry race and two request edge cases - Track queued/in-flight ADDBLOCKEDIP calls; expiry skips them, so a delayed command can no longer run after its record was removed and leave an unowned block. Expiry now starts when the block is added. - Acknowledge /mc/api/log without reading the body when remote logging is off - Skip the Control Subnet comparison for an address of a different family, so an IPv6 client no longer throws before the allowlist is checked Co-Authored-By: Claude Opus 5.5 --- .../MobileControlWebsocketServer.cs | 52 +++++++++++++++---- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index b9c7af5c5..406519572 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -425,7 +425,9 @@ private bool IsClientAllowed(System.Net.IPAddress remote) return true; } - if (csIpAddress != null && csSubnetMask != null && remote.IsInSameSubnet(csIpAddress, csSubnetMask)) + // Only compare like with like: IsInSameSubnet throws for an IPv6 client against the IPv4 Control Subnet + if (csIpAddress != null && csSubnetMask != null && remote.AddressFamily == csIpAddress.AddressFamily + && remote.IsInSameSubnet(csIpAddress, csSubnetMask)) { return true; } @@ -652,6 +654,10 @@ private static bool IsBenignBrowserRequest(string path) // so a block someone added by hand is never removed. private readonly Dictionary _autoBlocked = new Dictionary(); + // Addresses whose ADDBLOCKEDIP is queued or running. Expiry skips them, so a delayed command can never run + // after its record has been removed and leave a block nobody owns. + private readonly HashSet _pendingBlocks = new HashSet(); + // One file per instance: each server owns the blocks it added, and more than one controller can be configured private string AutoBlockFilePath { @@ -901,10 +907,26 @@ private void BlockAddress(string address, int count) this.LogWarning("Not blocking {address}: the block could not be recorded, so it could not be removed after a restart", address); return; } + + _pendingBlocks.Add(address); } // The console call can take a moment, so keep it off the request thread - CrestronInvoke.BeginInvoke(o => ExecuteBlock(address, count)); + try + { + CrestronInvoke.BeginInvoke(o => ExecuteBlock(address, count)); + } + catch (Exception ex) + { + this.LogError("Could not queue the block for {address}: {message}", address, ex.Message); + + lock (_autoBlockLock) + { + _pendingBlocks.Remove(address); + _autoBlocked.Remove(address); + SaveAutoBlocked(); + } + } } private void ExecuteBlock(string address, int count) @@ -916,6 +938,14 @@ private void ExecuteBlock(string address, int count) if (response != null && response.IndexOf("Added IP", StringComparison.OrdinalIgnoreCase) >= 0) { + lock (_autoBlockLock) + { + // The block lasts from when it was actually added, however long the command waited in the queue + _pendingBlocks.Remove(address); + _autoBlocked[address] = DateTime.UtcNow + _autoBlockDuration; + SaveAutoBlocked(); + } + this.LogWarning("Blocked {address} for {minutes} minutes: {count} unwanted requests within a minute", address, (int)_autoBlockDuration.TotalMinutes, count); return; } @@ -930,6 +960,7 @@ private void ExecuteBlock(string address, int count) // Not blocked, so Essentials has nothing to remove later lock (_autoBlockLock) { + _pendingBlocks.Remove(address); _autoBlocked.Remove(address); SaveAutoBlocked(); } @@ -950,7 +981,7 @@ private void CheckAutoBlockExpiry(object unused) lock (_autoBlockLock) { var now = DateTime.UtcNow; - due = _autoBlocked.Where(kv => kv.Value <= now).Select(kv => kv.Key).ToList(); + due = _autoBlocked.Where(kv => kv.Value <= now && !_pendingBlocks.Contains(kv.Key)).Select(kv => kv.Key).ToList(); } foreach (var address in due) @@ -2008,17 +2039,20 @@ private async void Server_OnPost(object sender, HttpRequestEventArgs e) if (path.StartsWith("/mc/api/log")) { - var body = new StreamReader(req.InputStream).ReadToEnd(); - - res.StatusCode = 200; - res.Close(); - - // The app posts here whether or not forwarding is on, so this is not an unwanted request + // The app posts here whether or not forwarding is on, so this is not an unwanted request. + // Acknowledged without reading the body, so a large or slow upload costs nothing when it is off. if (!_parent.Config.DirectServer.Logging.EnableRemoteLogging) { + res.StatusCode = 200; + res.Close(); return; } + var body = new StreamReader(req.InputStream).ReadToEnd(); + + res.StatusCode = 200; + res.Close(); + // remote log collector has no dedicated secure flag; keep it on http regardless of DirectServer.Secure var logRequest = new HttpRequestMessage(HttpMethod.Post, $"http://{_parent.Config.DirectServer.Logging.Host}:{_parent.Config.DirectServer.Logging.Port}/logs") { From 939ea5ec716518b8d32629b806988d85d06339dc Mon Sep 17 00:00:00 2001 From: anthony-lopez-pd Date: Sat, 3 Oct 2026 12:49:35 -0400 Subject: [PATCH 10/10] fix(mobile-control): match IPv4-mapped CIDRs and guard unreadable block state - Store ::ffff:a.b.c.d/N entries (N >= 96) as IPv4 /N-96, so they match clients, which are compared in IPv4 form - If the auto-block state file exists but cannot be read, turn automatic blocking off and never write the file, so the records it holds are not overwritten with an incomplete list. The HTTP server keeps running. Co-Authored-By: Claude Opus 5.5 --- .../MobileControlWebsocketServer.cs | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 406519572..3fdc4f68a 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -346,6 +346,15 @@ private static bool TryParseCidr(string value, out AllowedNetwork network) return false; } + // Clients in ::ffff:a.b.c.d form are compared as IPv4, so store a mapped entry the same way or it never matches + if (bytes.Length == 16 && prefix >= 96 && IsIPv4MappedBytes(bytes)) + { + var v4 = new byte[4]; + Array.Copy(bytes, 12, v4, 0, 4); + bytes = v4; + prefix -= 96; + } + network = new AllowedNetwork { Address = bytes, PrefixLength = prefix }; return true; } @@ -647,6 +656,9 @@ private static bool IsBenignBrowserRequest(string path) private CTimer _autoBlockTimer; private int _expiryRunning; + // Set when the state file exists but could not be read. No new blocks and no writes to the file until restart. + private bool _autoBlockStateUnreadable; + private readonly object _autoBlockLock = new object(); private readonly SlidingWindowCounter _unwantedRequests = new SlidingWindowCounter(TimeSpan.FromSeconds(SuspiciousWindowSeconds), MaxTrackedAddresses); @@ -712,7 +724,8 @@ private void LoadAutoBlockSettings() // the setting has since been turned off. LoadAutoBlockedFromDisk(); - if (wanted) + // The HTTP server keeps running; only automatic blocking is held off + if (wanted && !_autoBlockStateUnreadable) { _autoBlockDryRun = config.DryRun; _autoBlockThreshold = Math.Max(3, config.RequestsPerMinute); @@ -1039,6 +1052,13 @@ private void RemoveBlock(string address) /// Caller holds _autoBlockLock. private bool SaveAutoBlocked() { + // The file on disk may hold records this instance could not load. Writing would replace them with an + // incomplete list and lose track of those blocks for good, so leave it untouched for someone to recover. + if (_autoBlockStateUnreadable) + { + return false; + } + try { var path = AutoBlockFilePath; @@ -1082,7 +1102,12 @@ private void LoadAutoBlockedFromDisk() var path = AutoBlockFilePath; var legacyPath = Global.FilePathPrefix + LegacyAutoBlockFileName; - ReadAutoBlockedFile(path); + // A missing file just means no blocks. A file that exists but cannot be read is a different matter. + if (File.Exists(path) && !ReadAutoBlockedFile(path)) + { + _autoBlockStateUnreadable = true; + this.LogError("{path} could not be read. Automatic blocking is off and the file is left as it is, so blocks it lists will not be removed automatically. Fix or remove the file and restart", path); + } if (!File.Exists(legacyPath)) {