Skip to content

security(ws): bound total concurrent WebSocket clients #369

Closed

Description

@dborup

Problem

On dborup/CoreScope master 1f423e0627976c961fb7a40f45016e817e582cc3, the existing webSocket limiter from PR #47 has a default concurrent per-IP cap of 0 (off) and no global concurrent-client ceiling. Its per-IP checks are skipped when clients arrive through an unconfigured reverse proxy because they cannot be distinguished safely. Each accepted /ws client owns a connection, goroutines and a 256-message send queue, so unlimited concurrent clients remain a resource-exhaustion path.

Reference: nashmesh/CoreScope PR 20 (reference only). Its implementation cannot be copied verbatim because our fork already has per-IP upgrade-rate limits, a deny list and its own webSocket configuration shape.

Requested change

  • Add a nonzero, bounded global concurrent-client ceiling that remains effective even when per-client identity is unavailable behind a proxy.
  • Preserve the existing per-IP rate limits, deny list and explicit trusted-proxy policy. Do not trust arbitrary X-Forwarded-For values.
  • Reject excess clients before upgrading, provide an appropriate retry response, and release a reserved slot on failed upgrades, disconnect and hub shutdown.
  • Document the default and any configuration option without changing WebSocket message semantics.

Acceptance

  • Real-handshake tests demonstrate that only the configured number of clients can connect concurrently, including many connections through one proxy address.
  • Failed upgrades and all close paths return their slots exactly once; race tests cover concurrent admission.
  • Origin rejection and the existing deny/rate rules retain their intended precedence and behavior.
  • Verify our Caddy/reverse-proxy topology before choosing any per-IP default.

This issue concerns repository behavior; it does not establish current production connection counts.

Activity

  1. added a commit that references this issue on Oct 13, 2026
  2. dborup commented on Oct 13, 2026

    @dborup
    OwnerAuthor

    Fixed by #421 (merged as 09b6385accbcc3a5ad1f2374c45739e1e2cec908). Note for operators: the global WebSocket ceiling is ON by default at 512 concurrent clients (webSocket.maxClients); previously unbounded. Remaining nits (ceiling value not shown in /api/stats, unthrottled reject log) are in the review on #421.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions