diff --git a/client/internal/approval/broker.go b/client/internal/approval/broker.go index cac0d8430..e9db47bf9 100644 --- a/client/internal/approval/broker.go +++ b/client/internal/approval/broker.go @@ -68,10 +68,18 @@ const ( // DefaultTimeout is the wall-clock window the user has to accept or deny a // pending approval before the broker fails closed and returns ErrTimeout. -// Kept well under typical VNC client and dashboard connection timeouts so -// the RFB rejection actually reaches the browser instead of racing the -// browser's own "connection timed out" message. -const DefaultTimeout = 15 * time.Second +// +// The connection spends this whole window in silence: the gate runs before the +// protocol handshake, so nothing reaches the client until the user answers. A +// client therefore has to tolerate a quiet connection for longer than this, or +// it gives up before the answer arrives. +// +// A minute is what a prompt on a phone needs: it arrives as a notification on +// a device that is in a pocket, and answering it means noticing it, unlocking, +// and working through the system's screen-capture dialog. A desktop dialog +// appears on the screen the user is already looking at and rarely uses more +// than a few seconds of it. +const DefaultTimeout = 60 * time.Second // timeoutValue returns the active timeout. It's a var so tests in this // package can shorten the wait without exposing a setter on the public diff --git a/client/wasm/internal/vnc/proxy.go b/client/wasm/internal/vnc/proxy.go index fa3472782..5a9de8999 100644 --- a/client/wasm/internal/vnc/proxy.go +++ b/client/wasm/internal/vnc/proxy.go @@ -102,6 +102,16 @@ const ( vncProxyScheme = "ws" vncDialTimeout = 15 * time.Second + // idleReadDeadline bounds silence on a running session, where the server + // emits empty FramebufferUpdate responses several times a second. + idleReadDeadline = 30 * time.Second + + // approvalReadDeadline bounds silence before the session has started. The + // host sends nothing while it waits for the user to accept the connection, + // so this has to outlast that prompt and still leave room for the + // rejection a denial sends afterwards. + approvalReadDeadline = 90 * time.Second + // Connection modes matching server/server.go constants. modeAttach byte = 0 modeSession byte = 1 @@ -542,6 +552,7 @@ func writeAll(conn net.Conn, buf []byte) error { func (p *VNCProxy) forwardConnToWS(conn *vncConnection) { buf := make([]byte, 32*1024) + started := false for { if conn.ctx.Err() != nil { @@ -551,7 +562,15 @@ func (p *VNCProxy) forwardConnToWS(conn *vncConnection) { if !ok { return } - if err := vc.SetReadDeadline(time.Now().Add(30 * time.Second)); err != nil { + // Silence means something different before the server has said + // anything at all: the host may be sitting on an approval prompt, and + // nothing is sent until the user answers it. Only once the session is + // running does a gap mean the peer is gone. + deadline := approvalReadDeadline + if started { + deadline = idleReadDeadline + } + if err := vc.SetReadDeadline(time.Now().Add(deadline)); err != nil { log.Debugf("set VNC read deadline: %v", err) } n, err := vc.Read(buf) @@ -562,6 +581,7 @@ func (p *VNCProxy) forwardConnToWS(conn *vncConnection) { continue } if n > 0 { + started = true p.sendToWebSocket(conn, buf[:n]) } }