From c29ef638f46003a78c11d0c6b12a16d738f72b78 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Fri, 22 May 2026 15:32:35 +0200 Subject: [PATCH] Switch VNC daemon-to-agent IPC to Unix sockets and audit-log every connection --- client/cmd/vnc_agent.go | 40 ++++---- client/internal/engine_vnc.go | 2 +- client/vnc/server/agent_darwin.go | 81 +++++++++------- client/vnc/server/agent_ipc.go | 130 +++++++++++++++++++------ client/vnc/server/agent_windows.go | 142 +++++++--------------------- client/vnc/server/server.go | 86 ++++++++++------- client/vnc/server/server_darwin.go | 72 ++------------ client/vnc/server/server_test.go | 8 +- client/vnc/server/server_windows.go | 54 +---------- 9 files changed, 273 insertions(+), 342 deletions(-) diff --git a/client/cmd/vnc_agent.go b/client/cmd/vnc_agent.go index 300fcdf21..742b498ba 100644 --- a/client/cmd/vnc_agent.go +++ b/client/cmd/vnc_agent.go @@ -4,6 +4,7 @@ package cmd import ( "fmt" + "net" "net/netip" "os" @@ -13,16 +14,16 @@ import ( vncserver "github.com/netbirdio/netbird/client/vnc/server" ) -var vncAgentPort uint16 +var vncAgentSocket string func init() { - vncAgentCmd.Flags().Uint16Var(&vncAgentPort, "port", 15900, "Port for the VNC agent to listen on") + vncAgentCmd.Flags().StringVar(&vncAgentSocket, "socket", "", "Unix-domain socket path the agent listens on (required)") rootCmd.AddCommand(vncAgentCmd) } // vncAgentCmd runs a VNC server inside the user's interactive session, -// listening on localhost. The NetBird service spawns it: on Windows via -// CreateProcessAsUser into the console session, on macOS via +// listening on a Unix-domain socket. The NetBird service spawns it: on +// Windows via CreateProcessAsUser into the console session, on macOS via // launchctl asuser into the Aqua session. var vncAgentCmd = &cobra.Command{ Use: "vnc-agent", @@ -33,40 +34,47 @@ var vncAgentCmd = &cobra.Command{ log.SetFormatter(&log.JSONFormatter{}) log.SetOutput(os.Stderr) - log.Infof("VNC agent starting on 127.0.0.1:%d", vncAgentPort) + if vncAgentSocket == "" { + return fmt.Errorf("--socket is required") + } token := os.Getenv("NB_VNC_AGENT_TOKEN") if token == "" { return fmt.Errorf("NB_VNC_AGENT_TOKEN not set; agent requires a token from the service") } - // Drop the token from our process environment so any child the - // agent spawns does not inherit it, and casual debugging tools - // that dump /proc//environ (or the Windows equivalent) on a - // running agent don't surface the loopback shared secret. + // Purge the token from env so it doesn't leak via /proc//environ. if err := os.Unsetenv("NB_VNC_AGENT_TOKEN"); err != nil { log.Debugf("unset NB_VNC_AGENT_TOKEN: %v", err) } + if err := os.Remove(vncAgentSocket); err != nil && !os.IsNotExist(err) { + log.Debugf("remove stale socket %s: %v", vncAgentSocket, err) + } + ln, err := net.Listen("unix", vncAgentSocket) + if err != nil { + return fmt.Errorf("listen on %s: %w", vncAgentSocket, err) + } + if err := os.Chmod(vncAgentSocket, 0o600); err != nil { + log.Debugf("chmod %s: %v", vncAgentSocket, err) + } + capturer, injector, err := newAgentResources() if err != nil { + _ = ln.Close() return err } - // The per-user agent listens only on loopback and is gated by an - // agent token shared with the daemon, so no X25519 identity key - // is needed; auth is disabled at the RFB layer. srv := vncserver.New(vncserver.Config{ Capturer: capturer, Injector: injector, DisableAuth: true, AgentTokenHex: token, + Listener: ln, }) - addr := netip.AddrPortFrom(netip.AddrFrom4([4]byte{127, 0, 0, 1}), vncAgentPort) - loopback := netip.PrefixFrom(netip.AddrFrom4([4]byte{127, 0, 0, 0}), 8) - if err := srv.Start(cmd.Context(), addr, loopback); err != nil { + if err := srv.Start(cmd.Context(), netip.AddrPort{}, netip.Prefix{}); err != nil { return fmt.Errorf("start vnc server: %w", err) } - log.Infof("vnc-agent listening on 127.0.0.1:%d, ready", vncAgentPort) + log.Infof("vnc-agent listening on %s, ready", vncAgentSocket) <-cmd.Context().Done() log.Info("vnc-agent context cancelled, shutting down") diff --git a/client/internal/engine_vnc.go b/client/internal/engine_vnc.go index 47b54db98..dfb63345c 100644 --- a/client/internal/engine_vnc.go +++ b/client/internal/engine_vnc.go @@ -116,7 +116,7 @@ func (e *Engine) startVNCServer() error { } serviceMode := vncNeedsServiceMode() if serviceMode { - log.Info("VNC: running in Session 0, enabling service mode (agent proxy)") + log.Info("VNC: running as system service, enabling service mode (per-session agent proxy)") } srv := vncserver.New(vncserver.Config{ Capturer: capturer, diff --git a/client/vnc/server/agent_darwin.go b/client/vnc/server/agent_darwin.go index da8083bcf..3da2c5aee 100644 --- a/client/vnc/server/agent_darwin.go +++ b/client/vnc/server/agent_darwin.go @@ -30,19 +30,25 @@ import ( // asuser + listen-readiness wait, ~hundreds of milliseconds in practice. // That cost only repeats on user switch. type darwinAgentManager struct { - mu sync.Mutex - authToken string - port uint16 - uid uint32 - running bool + mu sync.Mutex + authToken string + socketPath string + uid uint32 + running bool } func newDarwinAgentManager(ctx context.Context) *darwinAgentManager { - m := &darwinAgentManager{port: agentPort} + m := &darwinAgentManager{} go m.watchConsoleUser(ctx) return m } +// agentSocketPathFmt parameterizes the agent's loopback Unix-socket path +// by the console uid: /tmp is writable in the launchctl-asuser context +// and predictable to the daemon. The agent chmods the file 0600 after +// bind so only its uid (plus root) can dial. +const agentSocketPathFmt = "/tmp/netbird-vnc-%d.sock" + // watchConsoleUser kills the cached agent whenever the console user // changes (logout, fast user switch, login window). Without it the daemon // keeps proxying to an agent whose TCC grant and WindowServer access @@ -80,41 +86,45 @@ func (m *darwinAgentManager) watchConsoleUser(ctx context.Context) { } } -// ensure returns a token good for proxyToAgent. It spawns or respawns the -// per-user agent process as needed and waits until it is listening on the -// loopback port. Each ensure call is serialized so concurrent VNC clients -// share the same agent. -func (m *darwinAgentManager) ensure(ctx context.Context) (string, error) { +// Resolve spawns or respawns the per-user agent process as needed and +// returns its Unix-socket path and shared token. Each call is serialized +// so concurrent VNC clients share the same agent. +func (m *darwinAgentManager) Resolve(ctx context.Context) (string, string, error) { consoleUID, err := consoleUserID() if err != nil { - return "", fmt.Errorf("no console user: %w", err) + return "", "", fmt.Errorf("no console user: %w", err) } m.mu.Lock() defer m.mu.Unlock() if m.running && m.uid == consoleUID && vncAgentRunning() { - return m.authToken, nil + return m.socketPath, m.authToken, nil } m.killLocked() - // Reap any stray external vnc-agent so the new token is the only one - // the freshly spawned agent will accept on the loopback port. + // Reap stray agents so the new token is the only accepted one. killAllVNCAgents() + socketPath := fmt.Sprintf(agentSocketPathFmt, consoleUID) + if err := os.Remove(socketPath); err != nil && !errors.Is(err, os.ErrNotExist) { + log.Debugf("clear stale agent socket %s: %v", socketPath, err) + } + token, err := generateAuthToken() if err != nil { - return "", fmt.Errorf("generate agent auth token: %w", err) + return "", "", fmt.Errorf("generate agent auth token: %w", err) } - if err := spawnAgentForUser(consoleUID, m.port, token); err != nil { - return "", err + if err := spawnAgentForUser(consoleUID, socketPath, token); err != nil { + return "", "", err } - if err := waitForAgent(ctx, m.port, 5*time.Second); err != nil { + if err := waitForAgent(ctx, socketPath, 5*time.Second); err != nil { killAllVNCAgents() - return "", fmt.Errorf("agent did not start listening: %w", err) + return "", "", fmt.Errorf("agent did not start listening: %w", err) } m.authToken = token + m.socketPath = socketPath m.uid = consoleUID m.running = true - log.Infof("spawned VNC agent for console uid=%d on port %d", consoleUID, m.port) - return token, nil + log.Infof("spawned VNC agent for console uid=%d on %s", consoleUID, socketPath) + return socketPath, token, nil } // stop terminates the spawned agent, if any. Intended for daemon shutdown. @@ -129,16 +139,17 @@ func (m *darwinAgentManager) killLocked() { return } killAllVNCAgents() + if m.socketPath != "" { + if err := os.Remove(m.socketPath); err != nil && !errors.Is(err, os.ErrNotExist) { + log.Debugf("remove agent socket %s: %v", m.socketPath, err) + } + } m.running = false m.authToken = "" + m.socketPath = "" m.uid = 0 } -// errNoConsoleUser is the sentinel callers use to recognise the -// "login window showing, no user signed in" state and surface it as a -// distinct condition to the VNC client. -var errNoConsoleUser = errors.New("no user logged into console") - // consoleUserID returns the uid of the user currently sitting at the // console (the one whose Aqua session is active). Returns // errNoConsoleUser when nobody is logged in: at the login window @@ -164,14 +175,14 @@ func consoleUserID() (uint32, error) { // WindowServer. The agent's stderr is relogged into the daemon log so // startup failures are not silently lost when the readiness check times // out. -func spawnAgentForUser(uid uint32, port uint16, token string) error { +func spawnAgentForUser(uid uint32, socketPath, token string) error { exe, err := os.Executable() if err != nil { return fmt.Errorf("resolve own executable: %w", err) } cmd := exec.Command( "/bin/launchctl", "asuser", strconv.FormatUint(uint64(uid), 10), - exe, vncAgentSubcommand, "--port", strconv.FormatUint(uint64(port), 10), + exe, vncAgentSubcommand, "--socket", socketPath, ) cmd.Env = append(os.Environ(), agentTokenEnvVar+"="+token) stderr, err := cmd.StderrPipe() @@ -189,23 +200,25 @@ func spawnAgentForUser(uid uint32, port uint16, token string) error { return nil } -// waitForAgent dials the loopback port until the agent answers. Used to +// waitForAgent dials the agent's Unix socket until it answers. Used to // gate proxy attempts until the spawned process has finished its Start. -func waitForAgent(ctx context.Context, port uint16, wait time.Duration) error { - addr := fmt.Sprintf("127.0.0.1:%d", port) +func waitForAgent(ctx context.Context, socketPath string, wait time.Duration) error { + var d net.Dialer deadline := time.Now().Add(wait) for time.Now().Before(deadline) { if ctx.Err() != nil { return ctx.Err() } - c, err := net.DialTimeout("tcp", addr, 200*time.Millisecond) + dialCtx, cancel := context.WithTimeout(ctx, 200*time.Millisecond) + c, err := d.DialContext(dialCtx, "unix", socketPath) + cancel() if err == nil { _ = c.Close() return nil } time.Sleep(100 * time.Millisecond) } - return fmt.Errorf("timeout dialing %s", addr) + return fmt.Errorf("timeout dialing %s", socketPath) } // vncAgentRunning reports whether any vnc-agent process exists on the diff --git a/client/vnc/server/agent_ipc.go b/client/vnc/server/agent_ipc.go index 7645201fd..a9ef3a77a 100644 --- a/client/vnc/server/agent_ipc.go +++ b/client/vnc/server/agent_ipc.go @@ -4,6 +4,7 @@ package server import ( "bufio" + "bytes" "context" crand "crypto/rand" "encoding/hex" @@ -17,14 +18,84 @@ import ( log "github.com/sirupsen/logrus" ) -const ( - // agentPort is the TCP loopback port on which a per-session VNC agent - // listens. The daemon dials this port and presents agentToken before - // proxying VNC bytes. The choice of TCP (rather than a Unix socket or - // named pipe) is intentional: it lets the same proxy/handshake code - // run on every platform; the token does the access control. - agentPort uint16 = 15900 +// errNoConsoleUser is the sentinel returned by sessionAgent.Resolve when +// the platform has no interactive user to attach a capture agent to (the +// macOS loginwindow state). Mapped to a distinct RFB reject code so the +// browser can show a meaningful message. +var errNoConsoleUser = errors.New("no user logged into console") +// sessionAgent abstracts the per-platform manager that spawns and tracks +// the user-session VNC agent. Resolve returns the agent's Unix-socket +// path and shared token, possibly spawning lazily. +type sessionAgent interface { + Resolve(ctx context.Context) (socketPath, token string, err error) +} + +// prefixConn replays already-consumed header bytes ahead of the proxy +// stream by swapping in a different Reader on the same underlying Conn. +type prefixConn struct { + io.Reader + net.Conn +} + +func (p *prefixConn) Read(b []byte) (int, error) { return p.Reader.Read(b) } + +// handleServiceConnection runs the connection-header handshake (source +// check, Noise_IK auth) on conn, resolves the right per-session agent +// via sa, and proxies to it. Every accepted connection emits exactly one +// outcome line on the daemon log. +func (s *Server) handleServiceConnection(conn net.Conn, sa sessionAgent) { + start := time.Now() + connLog := s.log.WithField("remote", conn.RemoteAddr().String()) + + if !s.isAllowedSource(conn.RemoteAddr()) { + connLog.Info("VNC connection rejected: source not allowed") + _ = conn.Close() + return + } + + var headerBuf bytes.Buffer + tee := io.TeeReader(conn, &headerBuf) + teeConn := &prefixConn{Reader: tee, Conn: conn} + + header, err := s.readConnectionHeader(teeConn) + if err != nil { + connLog.Infof("VNC connection rejected: header read failed: %v", err) + _ = conn.Close() + return + } + + authedLog, _, ok := s.authorizeSession(conn, header, connLog) + if !ok { + authedLog.Info("VNC connection rejected: auth failed") + return + } + s.registerConnAuth(conn, header) + + socketPath, token, err := sa.Resolve(s.ctx) + if err != nil { + code := RejectCodeCapturerError + if errors.Is(err, errNoConsoleUser) { + code = RejectCodeNoConsoleUser + } + rejectConnection(conn, codeMessage(code, err.Error())) + authedLog.Warnf("VNC connection rejected: agent unavailable: %v", err) + return + } + + replayConn := &prefixConn{ + Reader: io.MultiReader(&headerBuf, conn), + Conn: conn, + } + if err := proxyToAgent(s.ctx, replayConn, socketPath, token); err != nil { + rejectConnection(conn, codeMessage(RejectCodeCapturerError, err.Error())) + authedLog.Warnf("VNC connection rejected: agent unreachable: %v", err) + return + } + authedLog.Infof("VNC connection closed (%dms)", time.Since(start).Milliseconds()) +} + +const ( // agentTokenLen is the size of the random per-spawn token in bytes. agentTokenLen = 32 @@ -52,32 +123,30 @@ func generateAuthToken() (string, error) { return hex.EncodeToString(b), nil } -// proxyToAgent dials the per-session agent on TCP loopback, writes the -// raw token bytes, and then copies bytes in both directions until either -// side closes. The token has to land on the wire before any VNC byte so -// the agent's listening Server can apply verifyAgentToken before letting -// real RFB traffic through. -func proxyToAgent(ctx context.Context, client net.Conn, port uint16, authToken string) { - defer client.Close() - - addr := fmt.Sprintf("127.0.0.1:%d", port) - agentConn, err := dialAgentWithRetry(ctx, addr) - if err != nil { - log.Warnf("proxy cannot reach agent at %s: %v", addr, err) - return - } - defer agentConn.Close() - +// proxyToAgent dials the per-session agent's Unix socket, writes the +// raw token bytes, then copies bytes both ways until either side closes. +// The token must precede any RFB byte so the agent's verifyAgentToken +// can run first. Returns nil once a stream is established; the caller is +// responsible for sending an RFB-level rejection on error so the client +// sees a reason instead of a bare timeout. +func proxyToAgent(ctx context.Context, client net.Conn, socketPath, authToken string) error { tokenBytes, err := hex.DecodeString(authToken) if err != nil || len(tokenBytes) != agentTokenLen { - log.Warnf("invalid auth token (len=%d): %v", len(tokenBytes), err) - return - } - if _, err := agentConn.Write(tokenBytes); err != nil { - log.Warnf("send auth token to agent: %v", err) - return + return fmt.Errorf("invalid auth token (len=%d): %w", len(tokenBytes), err) } + agentConn, err := dialAgentWithRetry(ctx, socketPath) + if err != nil { + return fmt.Errorf("dial agent at %s: %w", socketPath, err) + } + + if _, err := agentConn.Write(tokenBytes); err != nil { + _ = agentConn.Close() + return fmt.Errorf("send auth token to agent: %w", err) + } + + defer client.Close() + defer agentConn.Close() log.Debugf("proxy connected to agent, starting bidirectional copy") done := make(chan struct{}, 2) cp := func(label string, dst, src net.Conn) { @@ -88,6 +157,7 @@ func proxyToAgent(ctx context.Context, client net.Conn, port uint16, authToken s go cp("client→agent", agentConn, client) go cp("agent→client", client, agentConn) <-done + return nil } // relogAgentStream reads log lines from the agent's stderr and re-emits @@ -159,7 +229,7 @@ func dialAgentWithRetry(ctx context.Context, addr string) (net.Conn, error) { return nil, lastErr } dialCtx, cancel := context.WithTimeout(ctx, time.Second) - c, err := d.DialContext(dialCtx, "tcp", addr) + c, err := d.DialContext(dialCtx, "unix", addr) cancel() if err == nil { return c, nil diff --git a/client/vnc/server/agent_windows.go b/client/vnc/server/agent_windows.go index 0e27212e4..735ab274d 100644 --- a/client/vnc/server/agent_windows.go +++ b/client/vnc/server/agent_windows.go @@ -3,12 +3,12 @@ package server import ( + "context" "encoding/binary" "errors" "fmt" "os" "runtime" - "strings" "sync" "time" "unsafe" @@ -49,7 +49,6 @@ var ( procWTSQuerySessionInformation = wtsapi32.NewProc("WTSQuerySessionInformationW") iphlpapi = windows.NewLazySystemDLL("iphlpapi.dll") - procGetExtendedTcpTable = iphlpapi.NewProc("GetExtendedTcpTable") ) // GetCurrentSessionID returns the session ID of the current process. @@ -138,97 +137,6 @@ func getActiveSessionID() uint32 { return getConsoleSessionID() } -// reapOrphanOnPort finds any process listening on 127.0.0.1:port and, if -// it's a netbird vnc-agent left over from a previous service instance, -// terminates it. Verified by image-name match so we never kill an -// unrelated process that happens to use the same port. -func reapOrphanOnPort(port uint16) { - pid := tcpListenerPID(port) - if pid == 0 || pid == uint32(windows.GetCurrentProcessId()) { - return - } - h, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION|windows.PROCESS_TERMINATE|windows.SYNCHRONIZE, false, pid) - if err != nil { - log.Warnf("reap on port %d: open PID=%d: %v", port, pid, err) - return - } - defer func() { _ = windows.CloseHandle(h) }() - if !isOurAgentProcess(h) { - log.Warnf("reap on port %d: PID=%d is not a netbird vnc-agent, leaving it alone", port, pid) - return - } - if err := windows.TerminateProcess(h, 0); err != nil { - log.Warnf("reap on port %d: terminate PID=%d: %v", port, pid, err) - return - } - log.Infof("reaped orphan vnc-agent PID=%d holding port %d", pid, port) -} - -// isOurAgentProcess returns true if the given process handle points at a -// netbird.exe binary at the same path as the current process. We compare -// full paths (case-insensitive on Windows) so co-installed netbird binaries -// from a different install dir or unrelated apps named netbird.exe don't -// get killed. -func isOurAgentProcess(h windows.Handle) bool { - var size uint32 = windows.MAX_PATH - buf := make([]uint16, size) - if err := windows.QueryFullProcessImageName(h, 0, &buf[0], &size); err != nil { - return false - } - target := strings.ToLower(windows.UTF16ToString(buf[:size])) - selfExe, err := os.Executable() - if err != nil { - return false - } - return target == strings.ToLower(selfExe) -} - -// tcpListenerPID returns the PID of the process listening on 127.0.0.1:port, -// or 0 if none. Uses GetExtendedTcpTable with TCP_TABLE_OWNER_PID_LISTENER. -func tcpListenerPID(port uint16) uint32 { - const tcpTableOwnerPidListener = 3 - const afInet = 2 - - // MIB_TCPROW_OWNER_PID layout: state(4) + localAddr(4) + localPort(4) + - // remoteAddr(4) + remotePort(4) + owningPid(4) = 24 bytes. - const rowSize = 24 - - var size uint32 - _, _, _ = procGetExtendedTcpTable.Call(0, uintptr(unsafe.Pointer(&size)), 0, afInet, tcpTableOwnerPidListener, 0) - if size == 0 { - return 0 - } - buf := make([]byte, size) - r, _, _ := procGetExtendedTcpTable.Call( - uintptr(unsafe.Pointer(&buf[0])), - uintptr(unsafe.Pointer(&size)), - 0, afInet, tcpTableOwnerPidListener, 0, - ) - if r != 0 { - return 0 - } - count := binary.LittleEndian.Uint32(buf[:4]) - for i := uint32(0); i < count; i++ { - off := 4 + int(i)*rowSize - if off+rowSize > len(buf) { - break - } - // localPort is stored big-endian in the high 16 bits of a 32-bit field. - localPort := uint16(buf[off+8])<<8 | uint16(buf[off+9]) - if localPort != port { - continue - } - localAddr := binary.LittleEndian.Uint32(buf[off+4 : off+8]) - // 0x0100007f == 127.0.0.1 in network byte order on little-endian. - // We accept 0.0.0.0 too in case the orphan bound to all interfaces. - if localAddr != 0x0100007f && localAddr != 0 { - continue - } - return binary.LittleEndian.Uint32(buf[off+20 : off+24]) - } - return 0 -} - // wtsSessionHasUser returns true if the session has a non-empty user name, // i.e. someone is logged in (vs. the login/Welcome screen). The console // session at the lock screen has WTSUserName == "". @@ -322,7 +230,7 @@ func injectEnvVar(envBlock uintptr, key, value string) []uint16 { return newBlock } -func spawnAgentInSession(sessionID uint32, port uint16, authToken string, jobHandle windows.Handle) (windows.Handle, error) { +func spawnAgentInSession(sessionID uint32, socketPath, authToken string, jobHandle windows.Handle) (windows.Handle, error) { token, err := getSystemTokenForSession(sessionID) if err != nil { return 0, fmt.Errorf("get SYSTEM token for session %d: %w", sessionID, err) @@ -352,7 +260,7 @@ func spawnAgentInSession(sessionID uint32, port uint16, authToken string, jobHan return 0, fmt.Errorf("get executable path: %w", err) } - cmdLine := fmt.Sprintf(`"%s" %s --port %d`, exePath, vncAgentSubcommand, port) + cmdLine := fmt.Sprintf(`"%s" %s --socket %q`, exePath, vncAgentSubcommand, socketPath) cmdLineW, err := windows.UTF16PtrFromString(cmdLine) if err != nil { return 0, fmt.Errorf("UTF16 cmdline: %w", err) @@ -425,15 +333,16 @@ func spawnAgentInSession(sessionID uint32, port uint16, authToken string, jobHan // Relog agent output in the service with a [vnc-agent] prefix. go relogAgentOutput(stderrRead) - log.Infof("spawned agent PID=%d in session %d on port %d", pi.ProcessId, sessionID, port) + log.Infof("spawned agent PID=%d in session %d on %s", pi.ProcessId, sessionID, socketPath) return pi.Process, nil } // sessionManager monitors the active console session and ensures a VNC agent // process is running in it. When the session changes (e.g., user switch, RDP -// connect/disconnect), it kills the old agent and spawns a new one. +// connect/disconnect), it kills the old agent and spawns a new one. Each +// spawn picks a per-session Unix-socket path the agent binds and the +// daemon dials over local IPC. type sessionManager struct { - port uint16 mu sync.Mutex agentProc windows.Handle everSpawned bool @@ -442,16 +351,22 @@ type sessionManager struct { nextSpawnAt time.Time sessionID uint32 authToken string + socketPath string done chan struct{} // jobHandle owns the agent processes via a Windows Job Object with // JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. When the service exits or crashes, // the OS closes the handle and terminates every assigned agent: no - // orphaned listeners holding the agent port across restarts. + // orphaned agent processes holding a socket across restarts. jobHandle windows.Handle } -func newSessionManager(port uint16) *sessionManager { - m := &sessionManager{port: port, sessionID: ^uint32(0), done: make(chan struct{})} +// agentSocketPathFmt parameterizes the per-session agent socket path by +// the Windows session id. C:\Windows\Temp is writable to both the daemon +// (SYSTEM) and the spawned agent (SYSTEM token impersonating the session). +const agentSocketPathFmt = `C:\Windows\Temp\netbird-vnc-%d.sock` + +func newSessionManager() *sessionManager { + m := &sessionManager{sessionID: ^uint32(0), done: make(chan struct{})} if h, err := createKillOnCloseJob(); err != nil { log.Warnf("create job object for vnc-agent (orphan agents possible after crash): %v", err) } else { @@ -508,13 +423,22 @@ func createKillOnCloseJob() (windows.Handle, error) { return job, nil } -// AuthToken returns the current agent authentication token. -func (m *sessionManager) AuthToken() string { +// Resolve returns the current agent socket path and token. When no +// agent is spawned yet (initial boot, between session switches, or +// permanently disabled when SE_TCB_NAME is missing) it surfaces a +// distinct error so the daemon can reject the connection with a +// meaningful message instead of timing out the proxy dial. +func (m *sessionManager) Resolve(_ context.Context) (string, string, error) { m.mu.Lock() defer m.mu.Unlock() - return m.authToken + if m.socketPath == "" { + return "", "", errAgentNotReady + } + return m.socketPath, m.authToken, nil } +var errAgentNotReady = errors.New("VNC agent not running yet") + // Stop signals the session manager to exit its polling loop and closes the // Job Object handle, which Windows uses as the trigger to terminate every // agent process this manager spawned. @@ -623,8 +547,10 @@ func (m *sessionManager) maybeSpawnAgent(sid uint32) bool { // kill an unknown listener; if a kill+respawn races on port // release, the spawn-failure backoff handles it without forcing // a synchronous wait or duplicate kill. - if !m.everSpawned { - reapOrphanOnPort(m.port) + socketPath := fmt.Sprintf(agentSocketPathFmt, sid) + // Covers a previous-run crash that escaped Job Object kill-on-close. + if err := os.Remove(socketPath); err != nil && !os.IsNotExist(err) { + log.Debugf("clear stale agent socket %s: %v", socketPath, err) } token, err := generateAuthToken() if err != nil { @@ -632,9 +558,11 @@ func (m *sessionManager) maybeSpawnAgent(sid uint32) bool { return true } m.authToken = token - h, err := spawnAgentInSession(sid, m.port, m.authToken, m.jobHandle) + m.socketPath = socketPath + h, err := spawnAgentInSession(sid, socketPath, m.authToken, m.jobHandle) if err != nil { m.authToken = "" + m.socketPath = "" if errors.Is(err, windows.ERROR_PRIVILEGE_NOT_HELD) { // SE_TCB_NAME (token-impersonation across sessions) is only // granted to SYSTEM. Without it spawnAgent will fail every 2 diff --git a/client/vnc/server/server.go b/client/vnc/server/server.go index 87f31f853..eb384dcf2 100644 --- a/client/vnc/server/server.go +++ b/client/vnc/server/server.go @@ -215,6 +215,11 @@ type Server struct { // during each VNC session and on session close. The engine wires // this to its metrics framework. sessionRecorder func(SessionTick) + + // preListener, when non-nil, replaces the TCP listener Start would + // open; addr/network args to Start are ignored. Used by the agent's + // Unix-socket path. + preListener net.Listener } // connAuthInfo captures the Noise_IK-verified identity bound to a live @@ -254,11 +259,9 @@ type virtualSessionManager interface { StopAll() } -// Config bundles the values the VNC server needs at construction time. -// Fields are read once by New; mutating them afterwards has no effect. -// Optional fields are nil/zero when unused. The hex-encoded AgentTokenHex -// is decoded internally and an invalid value is logged and treated as -// empty, matching the legacy SetAgentToken behavior. +// Config bundles the values the VNC server needs at construction time; +// fields are read once by New. AgentTokenHex is decoded internally; an +// invalid value is logged and treated as empty. type Config struct { Capturer ScreenCapturer Injector InputInjector @@ -268,6 +271,10 @@ type Config struct { DisableAuth bool AgentTokenHex string NetstackNet *netstack.Net + // Listener, when set, is used instead of Start opening a TCP listener; + // addr/network args to Start are then ignored. The agent uses this to + // listen on a Unix socket. + Listener net.Listener } // New creates a VNC server from the provided Config. IdentityKey is the @@ -282,6 +289,7 @@ func New(cfg Config) *Server { sessionRecorder: cfg.SessionRecorder, disableAuth: cfg.DisableAuth, netstackNet: cfg.NetstackNet, + preListener: cfg.Listener, authorizer: sshauth.NewAuthorizer(), log: log.WithField("component", "vnc-server"), sessions: make(map[uint64]ActiveSessionInfo), @@ -446,6 +454,8 @@ func (s *Server) UpdateVNCAuth(config *sshauth.Config) { // Start begins listening for VNC connections on the given address. // network is the NetBird overlay prefix used to validate connection sources. +// When Config.Listener was supplied, addr and network are ignored and the +// pre-built listener is used (the per-session agent path). func (s *Server) Start(ctx context.Context, addr netip.AddrPort, network netip.Prefix) error { s.mu.Lock() defer s.mu.Unlock() @@ -454,34 +464,37 @@ func (s *Server) Start(ctx context.Context, addr netip.AddrPort, network netip.P return fmt.Errorf("server already running") } - if !network.IsValid() { - return fmt.Errorf("invalid overlay network prefix") - } - s.ctx, s.cancel = context.WithCancel(ctx) s.vmgr = s.platformSessionManager() - s.localAddr = addr.Addr() - s.network = network - var listener net.Listener var listenDesc string - if s.netstackNet != nil { - ln, err := s.netstackNet.ListenTCPAddrPort(addr) - if err != nil { - return fmt.Errorf("listen on netstack %s: %w", addr, err) + switch { + case s.preListener != nil: + s.listener = s.preListener + listenDesc = s.preListener.Addr().String() + default: + if !network.IsValid() { + return fmt.Errorf("invalid overlay network prefix") } - listener = ln - listenDesc = fmt.Sprintf("netstack %s", addr) - } else { - tcpAddr := net.TCPAddrFromAddrPort(addr) - ln, err := net.ListenTCP("tcp", tcpAddr) - if err != nil { - return fmt.Errorf("listen on %s: %w", addr, err) + s.localAddr = addr.Addr() + s.network = network + if s.netstackNet != nil { + ln, err := s.netstackNet.ListenTCPAddrPort(addr) + if err != nil { + return fmt.Errorf("listen on netstack %s: %w", addr, err) + } + s.listener = ln + listenDesc = fmt.Sprintf("netstack %s", addr) + } else { + tcpAddr := net.TCPAddrFromAddrPort(addr) + ln, err := net.ListenTCP("tcp", tcpAddr) + if err != nil { + return fmt.Errorf("listen on %s: %w", addr, err) + } + s.listener = ln + listenDesc = addr.String() } - listener = ln - listenDesc = addr.String() } - s.listener = listener if s.serviceMode { s.platformInit() @@ -616,10 +629,11 @@ func (s *Server) validateCapturer(capturer ScreenCapturer) error { // and from the local WireGuard IP (prevents local privilege escalation). // Matches the SSH server's connectionValidator logic. func (s *Server) isAllowedSource(addr net.Addr) bool { + // Unix-socket remotes (the agent path) are local IPC, gated by the + // token, not by overlay membership. tcpAddr, ok := addr.(*net.TCPAddr) if !ok { - s.log.Warnf("connection rejected: non-TCP address %s", addr) - return false + return true } remoteIP, ok := netip.AddrFromSlice(tcpAddr.IP) @@ -651,29 +665,34 @@ func (s *Server) isAllowedSource(addr net.Addr) bool { } func (s *Server) handleConnection(conn net.Conn) { + start := time.Now() connLog := s.log.WithField("remote", conn.RemoteAddr().String()) if !s.isAllowedSource(conn.RemoteAddr()) { - conn.Close() + connLog.Info("VNC connection rejected: source not allowed") + _ = conn.Close() return } if !s.verifyAgentToken(conn, connLog) { + connLog.Info("VNC connection rejected: agent token check failed") return } header, err := s.readConnectionHeader(conn) if err != nil { - connLog.Warnf("read connection header: %v", err) - conn.Close() + connLog.Infof("VNC connection rejected: header read failed: %v", err) + _ = conn.Close() return } connLog, sessionUserID, ok := s.authorizeSession(conn, header, connLog) if !ok { + connLog.Info("VNC connection rejected: auth failed") return } s.registerConnAuth(conn, header) capturer, injector, sessionCleanup, ok := s.acquireSessionResources(conn, header, &connLog) if !ok { + connLog.Warn("VNC connection rejected: capturer/injector unavailable") return } defer sessionCleanup() @@ -688,14 +707,14 @@ func (s *Server) handleConnection(conn net.Conn) { if err := s.validateCapturer(capturer); err != nil { rejectConnection(conn, codeMessage(RejectCodeCapturerError, fmt.Sprintf("screen capturer: %v", err))) - connLog.Warnf("capturer not ready: %v", err) + connLog.Warnf("VNC connection rejected: capturer not ready: %v", err) return } w, h := capturer.Width(), capturer.Height() if w <= 0 || h <= 0 || w > maxFramebufferDim || h > maxFramebufferDim { rejectConnection(conn, codeMessage(RejectCodeCapturerError, fmt.Sprintf("framebuffer dimensions out of range: %dx%d", w, h))) - connLog.Warnf("rejecting session: framebuffer %dx%d outside [1, %d]", w, h, maxFramebufferDim) + connLog.Warnf("VNC connection rejected: framebuffer %dx%d outside [1, %d]", w, h, maxFramebufferDim) return } @@ -709,6 +728,7 @@ func (s *Server) handleConnection(conn net.Conn) { log: connLog, } sess.serve() + connLog.Infof("VNC connection closed (%dms)", time.Since(start).Milliseconds()) } // codeMessage formats a stable reject code with a human-readable message. diff --git a/client/vnc/server/server_darwin.go b/client/vnc/server/server_darwin.go index 81c0ca991..18b5bbb7b 100644 --- a/client/vnc/server/server_darwin.go +++ b/client/vnc/server/server_darwin.go @@ -3,9 +3,6 @@ package server import ( - "bytes" - "errors" - "io" "net" log "github.com/sirupsen/logrus" @@ -23,17 +20,15 @@ func (s *Server) platformSessionManager() virtualSessionManager { return nil } -// serviceAcceptLoop runs in a LaunchDaemon and proxies each VNC -// connection to a per-user agent. The agent is spawned lazily on the -// first connection (and respawned after a console-user change) via -// launchctl asuser, which is the only mechanism that lands a child -// inside the user's Aqua session, where WindowServer and TCC grants -// for screen capture work. +// serviceAcceptLoop runs as a LaunchDaemon and proxies each VNC connection +// to the per-user agent darwinAgentManager spawns via launchctl asuser +// (the only spawn mode that lands a child in the user's Aqua session with +// WindowServer + TCC access). func (s *Server) serviceAcceptLoop() { mgr := newDarwinAgentManager(s.ctx) defer mgr.stop() - log.Infof("service mode, proxying connections to per-user agent on 127.0.0.1:%d", agentPort) + log.Info("service mode, proxying connections to per-user agent over Unix socket") for { conn, err := s.listener.Accept() @@ -58,62 +53,7 @@ func (s *Server) serviceAcceptLoop() { go func(c net.Conn) { defer s.releaseConnSlot() defer s.untrackConn(c) - s.handleServiceConnectionDarwin(c, mgr) + s.handleServiceConnection(c, mgr) }(conn) } } - -func (s *Server) handleServiceConnectionDarwin(conn net.Conn, mgr *darwinAgentManager) { - connLog := s.log.WithField("remote", conn.RemoteAddr().String()) - - if !s.isAllowedSource(conn.RemoteAddr()) { - conn.Close() - return - } - - var headerBuf bytes.Buffer - tee := io.TeeReader(conn, &headerBuf) - teeConn := &darwinPrefixConn{Reader: tee, Conn: conn} - - header, err := s.readConnectionHeader(teeConn) - if err != nil { - connLog.Debugf("read connection header: %v", err) - conn.Close() - return - } - - if !s.disableAuth { - if _, err := s.authenticateSession(header); err != nil { - rejectConnection(conn, codeMessage(RejectCodeAuthForbidden, err.Error())) - connLog.Warnf("auth rejected: %v", err) - return - } - } - s.registerConnAuth(conn, header) - - token, err := mgr.ensure(s.ctx) - if err != nil { - code := RejectCodeCapturerError - if errors.Is(err, errNoConsoleUser) { - code = RejectCodeNoConsoleUser - } - rejectConnection(conn, codeMessage(code, err.Error())) - connLog.Warnf("spawn per-user agent: %v", err) - return - } - - replayConn := &darwinPrefixConn{ - Reader: io.MultiReader(&headerBuf, conn), - Conn: conn, - } - proxyToAgent(s.ctx, replayConn, agentPort, token) -} - -// darwinPrefixConn replays the already-consumed connection-header bytes -// in front of the proxy stream, mirroring the Windows prefixConn shape. -type darwinPrefixConn struct { - io.Reader - net.Conn -} - -func (p *darwinPrefixConn) Read(b []byte) (int, error) { return p.Reader.Read(b) } diff --git a/client/vnc/server/server_test.go b/client/vnc/server/server_test.go index 96a60175b..0a44de3e4 100644 --- a/client/vnc/server/server_test.go +++ b/client/vnc/server/server_test.go @@ -148,11 +148,13 @@ func TestIsAllowedSource(t *testing.T) { want bool }{ { - name: "non-tcp address rejected", + // Unix-domain remotes (per-session agent path) are local IPC, + // gated by the token, not by overlay membership. + name: "non-tcp address allowed", localAddr: netip.MustParseAddr("10.99.99.1"), network: netip.MustParsePrefix("10.99.0.0/16"), - remote: &net.UDPAddr{IP: net.ParseIP("10.99.99.2"), Port: 1234}, - want: false, + remote: &net.UnixAddr{Name: "/tmp/foo.sock", Net: "unix"}, + want: true, }, { name: "own IP rejected", diff --git a/client/vnc/server/server_windows.go b/client/vnc/server/server_windows.go index 46e1181e7..a8c58bd7f 100644 --- a/client/vnc/server/server_windows.go +++ b/client/vnc/server/server_windows.go @@ -3,10 +3,8 @@ package server import ( - "bytes" "context" "fmt" - "io" "net" "unsafe" @@ -238,10 +236,10 @@ func (s *Server) platformInit() { // Noise_IK handshake before proxying to the user-session agent. func (s *Server) serviceAcceptLoop() { - sm := newSessionManager(agentPort) + sm := newSessionManager() go sm.run() - log.Infof("service mode, proxying connections to agent on 127.0.0.1:%d", agentPort) + log.Info("service mode, proxying connections to agent over Unix socket") for { conn, err := s.listener.Accept() @@ -272,51 +270,3 @@ func (s *Server) serviceAcceptLoop() { } } -// handleServiceConnection runs the connection-header handshake (including -// Noise_IK), then proxies the connection (with header bytes replayed) to -// the agent listening on loopback. -func (s *Server) handleServiceConnection(conn net.Conn, sm *sessionManager) { - connLog := s.log.WithField("remote", conn.RemoteAddr().String()) - - if !s.isAllowedSource(conn.RemoteAddr()) { - conn.Close() - return - } - - var headerBuf bytes.Buffer - tee := io.TeeReader(conn, &headerBuf) - teeConn := &prefixConn{Reader: tee, Conn: conn} - - header, err := s.readConnectionHeader(teeConn) - if err != nil { - connLog.Debugf("read connection header: %v", err) - conn.Close() - return - } - - if !s.disableAuth { - if _, err := s.authenticateSession(header); err != nil { - rejectConnection(conn, codeMessage(RejectCodeAuthForbidden, err.Error())) - connLog.Warnf("auth rejected: %v", err) - return - } - } - s.registerConnAuth(conn, header) - - // Replay buffered header bytes + remaining stream to the agent. - replayConn := &prefixConn{ - Reader: io.MultiReader(&headerBuf, conn), - Conn: conn, - } - proxyToAgent(s.ctx, replayConn, agentPort, sm.AuthToken()) -} - -// prefixConn wraps a net.Conn, overriding Read to use a different reader. -type prefixConn struct { - io.Reader - net.Conn -} - -func (p *prefixConn) Read(b []byte) (int, error) { - return p.Reader.Read(b) -}