Monitoring WebSocket connections have no limit and can exhaust resources

[rustytrees]

What is happening?

The monitoring UI accepts WebSocket connections without a connection limit. Each accepted connection is stored in a process-wide map and receives a dedicated read goroutine. An authenticated client, or any client when monitoring authentication is disabled, can repeatedly open /api/ws connections and grow the number of live sockets and goroutines until idle deadlines expire. Connections kept active with Pong frames can remain registered indefinitely.

There is also a broadcast amplification path: each metrics update walks every registered connection while holding global client and write locks, and writes to the clients sequentially with a five-second deadline per client. Slow clients can therefore delay all WebSocket writes, with worst-case delay proportional to the number of connections.

Relevant code

The client registry is an unbounded map; the monitoring configuration has no maximum-WebSocket-clients field:

• MonitoringUI.clients and locks, lines 120-137
• MonitoringUIConfig, lines 23-37
• Map initialization, lines 184-207

Every successful upgrade is registered without checking a count and then starts a goroutine:

conn, err := ui.upgrader.Upgrade(w, r, nil)
// ...
ui.clientsMutex.Lock()
ui.clients[conn] = true
ui.clientsMutex.Unlock()
// ...
go func() {
    // ...
    for {
        var msg map[string]any
        err := conn.ReadJSON(&msg)
        // ...
    }
}()

• Upgrade and unbounded registration, lines 1137-1169
• Per-connection goroutine and read loop, lines 1171-1220

The 120-second read deadline limits completely idle connections, but Pong frames extend it without limiting connection age or count:

• Deadline and Pong handler, lines 1181-1187

Broadcasting holds global locks and performs sequential writes with a per-client deadline:

ui.writesMutex.Lock()
defer ui.writesMutex.Unlock()
ui.clientsMutex.Lock()
defer ui.clientsMutex.Unlock()

for client := range ui.clients {
    client.SetWriteDeadline(time.Now().Add(5 * time.Second))
    err := client.WriteJSON(metrics)
    // ...
}

• Broadcast loop, lines 1315-1333

The general DNS max_clients limit does not cover this HTTP/WebSocket server; it is enforced in the UDP/TCP DNS listeners instead:

• max_clients enforcement, lines 455-500

Exposure and impact

The monitoring UI is disabled and loopback-bound by default, which substantially limits the default exposure. The issue matters when an administrator enables the UI and makes it reachable by other hosts, uses a reverse proxy, disables authentication, or when authenticated-but-untrusted clients exist. Basic authentication restricts who can reach the handler, but does not impose a resource quota on an authenticated client.

Potential effects include growing socket/file-descriptor and goroutine usage, increased memory and CPU consumption, and WebSocket update stalls caused by slow-client writes. At sufficient scale this may degrade or terminate the dnscrypt-proxy process, affecting DNS service.

How to reproduce

1. Enable the monitoring UI and use a test instance with an appropriate file-descriptor limit. Keep the listener restricted to a test environment.
2. Authenticate if configured and repeatedly complete WebSocket handshakes to /api/ws without closing the connections.
3. Send timely WebSocket Pong frames so the read deadlines continue to be extended.
4. Observe that all connections remain in ui.clients and each retains a goroutine; there is no rejection threshold.
5. Generate DNS queries to trigger metrics broadcasts and observe broadcast cost increasing with the number of clients. Slow or non-reading peers exercise the sequential write-deadline path.

Expected behavior / possible fix

• Add a configurable, conservative maximum number of monitoring WebSocket clients and reject excess upgrades (preferably before allocating the WebSocket).
• Consider per-IP limits and/or rate limiting for upgrade attempts.
• Do not hold the client registry lock during network writes; snapshot the connections first.
• Avoid a single global write lock across every connection. Serialize writes per connection and disconnect slow consumers promptly.
• Add tests that verify the client cap, cleanup/reuse after disconnect, bounded goroutine/connection behavior, and that one slow client cannot block broadcasts to others.

This is distinct from discussion #2901, which concerned concurrent WebSocket writes and was addressed in 2.1.13. The current concern is the absence of resource limits and the linear, globally serialized broadcast path.

[jedisct1]

I can run Swival /audit, too.