Skip to content

fix: kill active connections when user is removed via gRPC#6102

Closed
nightcrawler42 wants to merge 1 commit into
XTLS:mainfrom
nightcrawler42:fix/kill-connections-on-removal
Closed

fix: kill active connections when user is removed via gRPC#6102
nightcrawler42 wants to merge 1 commit into
XTLS:mainfrom
nightcrawler42:fix/kill-connections-on-removal

Conversation

@nightcrawler42
Copy link
Copy Markdown

Summary

When a user is removed through the gRPC API (AlterInboundRemoveUser), existing connections survive because user validation only happens during the initial handshake. After authentication, the connection runs independently with a captured User pointer — the validator is never consulted again.

This is a real-world problem for panel operators (Marzban, 3x-ui, etc.) who expect disabled/removed users to be disconnected immediately. Currently, the only way to force-disconnect is to restart the entire xray process, which drops all connections.

Changes

  • New ConnTracker (proxy/conntrack.go): per-user connection tracking using atomic sequence IDs and sync.Mutex. Stores both context.CancelFunc and io.Closer (the underlying net.Conn) for each tracked connection.

  • KillAll(email): cancels contexts AND closes TCP connections. Context cancellation alone is insufficient because buf.Copy (the core data relay loop in common/buf/copy.go) has no context awareness — it only stops on read/write errors. Closing the net.Conn forces an immediate TCP RST.

  • All 5 protocol handlers updated: VLESS, VMess, Trojan, Shadowsocks, Shadowsocks 2022

    • Track() called after user authentication in Process()
    • KillAll() called in RemoveUser() after successful removal
  • Shadowsocks 2022 fixes:

    • Bounds check for user index in NewConnection/NewPacketConnection (prevents panic when user removed during active handoff)
    • Error propagation for UpdateUsersWithPasswords in RemoveUser (was silently discarded)

Why not just cancel the context?

buf.Copy loops forever on reader.ReadMultiBuffer() with no ctx.Done() check. Long-lived streams (e.g. Psiphon tunnels, persistent WebSocket connections) survive context cancellation indefinitely. Force-closing the TCP connection is the only reliable way to break all in-flight relay loops.

Testing

  • Builds cleanly: CGO_ENABLED=0 go build ./...
  • Tested in production with Marzban panel + multiple nodes
  • Verified with Psiphon (long-lived tunneled connections) — connections now terminate within seconds of user removal

When a user is removed through the gRPC API (AlterInbound → RemoveUser),
existing connections survive because user validation only happens during
the initial handshake. After authentication, the connection runs
independently with a captured User pointer.

This commit adds per-user connection tracking that forcibly terminates
active connections on user removal:

- New ConnTracker in proxy/conntrack.go tracks connections per user email
  using atomic sequence IDs and sync.Mutex
- Track() stores both a context.CancelFunc and the underlying net.Conn
  (io.Closer) for each connection
- KillAll() cancels contexts AND closes TCP connections, ensuring
  immediate termination even for long-lived streams (e.g. tunneling
  tools that maintain persistent connections)
- All protocol handlers updated: VLESS, VMess, Trojan, Shadowsocks,
  Shadowsocks 2022
- Shadowsocks 2022: added bounds check for user index in
  NewConnection/NewPacketConnection to prevent panic when user is
  removed during active connection handoff
- Shadowsocks 2022: added error propagation for UpdateUsersWithPasswords
  in RemoveUser (was silently discarded)

Context cancellation alone is insufficient because buf.Copy (the core
data relay loop) has no context awareness — it only stops on read/write
errors. Closing the net.Conn forces an immediate TCP RST that breaks
all in-flight relay loops.
@Fangliding
Copy link
Copy Markdown
Member

Fangliding commented May 9, 2026

dup #5844

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants