Fix: Bind screen/terminal sub-conns only to the device's source IP
A screen or terminal sub-connection was keyed solely by a MasterID that any TCP peer may supply, so any internet host could claim an online device's MasterID and bind a sub-conn in its place, hijacking the live screen stream or a pending shell session. The ConnAuth signature cannot be verified on the server because its key lives in a vendored client library, so the trust anchor here is the source IP: sub-conns are always opened device-to-server from the same host as the login conn. BindScreenConn and BindTerminalConn now reject a sub-conn whose peer IP differs from the device's recorded main-connection PeerIP. Co-Authored-By: deepseek-v4-pro
This commit is contained in:
1 parent
53b7151fb7
commit
2de21b2b6b
2 files changed
+72
No files matched your search
@@ -299,6 +299,17 @@ func (h *Hub) BindScreenConn(deviceID string, ctx *connection.Context) bool {
|
|||||||
h.mu.Unlock()
|
h.mu.Unlock()
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
// Bind only sub-connections that originate from the device's own main
|
||||||
|
// connection source IP. Sub-conns are always opened device→server from
|
||||||
|
// the same host as the login conn, so a source-IP mismatch means the
|
||||||
|
// caller is not that device. This is the trust anchor for screen/terminal
|
||||||
|
// sub-conns: the ConnAuth signature key lives in a vendored client
|
||||||
|
// library and cannot be verified here, so without this check any internet
|
||||||
|
// host could claim an online device's MasterID and hijack its stream.
|
||||||
|
if d.PeerIP != "" && ctx.GetPeerIP() != d.PeerIP {
|
||||||
|
h.mu.Unlock()
|
||||||
|
return false
|
||||||
|
}
|
||||||
old := d.screenConn
|
old := d.screenConn
|
||||||
d.screenConn = ctx
|
d.screenConn = ctx
|
||||||
// The cached resolution and last IDR belong to the old encoder's
|
// The cached resolution and last IDR belong to the old encoder's
|
||||||
@@ -693,6 +704,13 @@ func (h *Hub) BindTerminalConn(deviceID string, ctx *connection.Context, isPTY b
|
|||||||
h.mu.Unlock()
|
h.mu.Unlock()
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
// Same trust anchor as BindScreenConn: a terminal sub-conn must come from
|
||||||
|
// the device's own main-connection source IP, otherwise any internet host
|
||||||
|
// could claim the pending session and hijack the shell.
|
||||||
|
if d.PeerIP != "" && ctx.GetPeerIP() != d.PeerIP {
|
||||||
|
h.mu.Unlock()
|
||||||
|
return false
|
||||||
|
}
|
||||||
d.terminalConn = ctx
|
d.terminalConn = ctx
|
||||||
d.terminalIsPTY = isPTY
|
d.terminalIsPTY = isPTY
|
||||||
d.terminalPending = false
|
d.terminalPending = false
|
||||||
|
|||||||
@@ -143,6 +143,60 @@ func TestHubRegisterOverwrites(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ctxFromIP builds a *connection.Context whose source IP resolves to ip.
|
||||||
|
// BindScreenConn/BindTerminalConn compare this against Device.PeerIP.
|
||||||
|
func ctxFromIP(ip string) *connection.Context {
|
||||||
|
return &connection.Context{RemoteAddr: ip + ":54321"}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Sub-conns (screen/terminal) are opened device→server from the same host as
|
||||||
|
// the login conn. A caller claiming a MasterID from a different source IP is
|
||||||
|
// not that device and must not be able to bind, replace, or hijack the stream.
|
||||||
|
func TestHubBindScreenConnIPBinding(t *testing.T) {
|
||||||
|
h := New()
|
||||||
|
h.Register(&Device{ID: "dev", PeerIP: "10.0.0.1"}, stubCtx())
|
||||||
|
|
||||||
|
// First bind from the device's own IP succeeds.
|
||||||
|
if !h.BindScreenConn("dev", ctxFromIP("10.0.0.1")) {
|
||||||
|
t.Fatal("same-IP screen bind should succeed")
|
||||||
|
}
|
||||||
|
// A second conn claiming the same MasterID from a foreign IP is refused,
|
||||||
|
// so it cannot displace the legitimate stream.
|
||||||
|
if h.BindScreenConn("dev", ctxFromIP("203.0.113.9")) {
|
||||||
|
t.Fatal("foreign-IP screen bind should be rejected")
|
||||||
|
}
|
||||||
|
|
||||||
|
// A device that has never bound a screen conn is equally protected.
|
||||||
|
h.Register(&Device{ID: "fresh", PeerIP: "10.0.0.2"}, stubCtx())
|
||||||
|
if h.BindScreenConn("fresh", ctxFromIP("203.0.113.9")) {
|
||||||
|
t.Fatal("foreign-IP first screen bind should be rejected")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestHubBindTerminalConnIPBinding(t *testing.T) {
|
||||||
|
h := New()
|
||||||
|
h.Register(&Device{ID: "dev", PeerIP: "10.0.0.1"}, stubCtx())
|
||||||
|
|
||||||
|
openPending := func(id string) {
|
||||||
|
h.mu.Lock()
|
||||||
|
h.devices[id].terminalPending = true
|
||||||
|
h.mu.Unlock()
|
||||||
|
}
|
||||||
|
openPending("dev")
|
||||||
|
|
||||||
|
// Same-IP terminal bind succeeds and consumes the pending slot.
|
||||||
|
if !h.BindTerminalConn("dev", ctxFromIP("10.0.0.1"), true) {
|
||||||
|
t.Fatal("same-IP terminal bind should succeed")
|
||||||
|
}
|
||||||
|
|
||||||
|
// A second device with a pending session cannot be claimed from elsewhere.
|
||||||
|
h.Register(&Device{ID: "dev2", PeerIP: "10.0.0.2"}, stubCtx())
|
||||||
|
openPending("dev2")
|
||||||
|
if h.BindTerminalConn("dev2", ctxFromIP("203.0.113.9"), true) {
|
||||||
|
t.Fatal("foreign-IP terminal bind should be rejected")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Race detector should not fire under `go test -race ./hub/...`.
|
// Race detector should not fire under `go test -race ./hub/...`.
|
||||||
func TestHubConcurrent(t *testing.T) {
|
func TestHubConcurrent(t *testing.T) {
|
||||||
h := New()
|
h := New()
|
||||||
|
|||||||
Reference in new issue
Block a user