From 65f302b698d5709f97d9ab330551b598e6d1ebe1 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Mon, 25 May 2026 13:26:29 +0200 Subject: [PATCH] Authenticate virtual X11 sessions with per-session MIT-MAGIC-COOKIE-1 --- client/internal/engine_vnc_x11.go | 4 +- client/vnc/server/capture_x11.go | 23 ++++-- client/vnc/server/input_x11.go | 31 ++++++-- client/vnc/server/virtual_x11.go | 111 +++++++++++++++++++++++--- client/vnc/server/xauth_linux.go | 124 ++++++++++++++++++++++++++++++ 5 files changed, 267 insertions(+), 26 deletions(-) create mode 100644 client/vnc/server/xauth_linux.go diff --git a/client/internal/engine_vnc_x11.go b/client/internal/engine_vnc_x11.go index d74bd17ab..3b0ddf8ac 100644 --- a/client/internal/engine_vnc_x11.go +++ b/client/internal/engine_vnc_x11.go @@ -11,9 +11,9 @@ import ( func newPlatformVNC() (vncserver.ScreenCapturer, vncserver.InputInjector, bool) { // Prefer X11 when an X server is reachable. NewX11InputInjector probes // DISPLAY (and /proc) eagerly, so a non-nil error here means no X. - injector, err := vncserver.NewX11InputInjector("") + injector, err := vncserver.NewX11InputInjector("", "", "") if err == nil { - return vncserver.NewX11Poller(""), injector, true + return vncserver.NewX11Poller("", ""), injector, true } log.Debugf("VNC: X11 not available: %v", err) diff --git a/client/vnc/server/capture_x11.go b/client/vnc/server/capture_x11.go index c7634bc2e..bdfaf7c42 100644 --- a/client/vnc/server/capture_x11.go +++ b/client/vnc/server/capture_x11.go @@ -210,7 +210,8 @@ func splitNull(data []byte) [][]byte { } // NewX11Capturer connects to the X11 display and sets up shared memory capture. -func NewX11Capturer(display string) (*X11Capturer, error) { +// Empty cookieHex falls back to XAUTHORITY env lookup. +func NewX11Capturer(display, cookieHex string) (*X11Capturer, error) { if display == "" { detectX11Display() display = os.Getenv(envDisplay) @@ -219,7 +220,13 @@ func NewX11Capturer(display string) (*X11Capturer, error) { return nil, fmt.Errorf("DISPLAY not set and no Xorg process found") } - conn, err := xgb.NewConnDisplay(display) + var conn *xgb.Conn + var err error + if cookieHex != "" { + conn, err = dialXUnixWithCookie(display, cookieHex) + } else { + conn, err = xgb.NewConnDisplay(display) + } if err != nil { return nil, fmt.Errorf("connect to X11 display %s: %w", display, err) } @@ -370,6 +377,8 @@ type X11Poller struct { clients atomic.Int32 display string + // cookieHex authenticates the X11 connection; empty falls back to XAUTHORITY env. + cookieHex string } // initRetryBackoff gates capturer re-init attempts after a failure so we @@ -377,10 +386,12 @@ type X11Poller struct { const initRetryBackoff = 2 * time.Second // NewX11Poller creates a lazy on-demand capturer for the given X display. -func NewX11Poller(display string) *X11Poller { +// Empty cookieHex falls back to XAUTHORITY env lookup. +func NewX11Poller(display, cookieHex string) *X11Poller { return &X11Poller{ - display: display, - done: make(chan struct{}), + display: display, + cookieHex: cookieHex, + done: make(chan struct{}), } } @@ -521,7 +532,7 @@ func (p *X11Poller) ensureCapturerLocked() error { if time.Now().Before(p.initBackoffUntil) { return fmt.Errorf("x11 capturer unavailable (retry scheduled)") } - c, err := NewX11Capturer(p.display) + c, err := NewX11Capturer(p.display, p.cookieHex) if err != nil { p.initBackoffUntil = time.Now().Add(initRetryBackoff) log.Debugf("X11 capturer: %v", err) diff --git a/client/vnc/server/input_x11.go b/client/vnc/server/input_x11.go index ed44aff93..d0588470e 100644 --- a/client/vnc/server/input_x11.go +++ b/client/vnc/server/input_x11.go @@ -25,10 +25,13 @@ type X11InputInjector struct { lastButtons uint16 clipboardTool string clipboardToolName string + // authFile points xclip/xsel at the per-session Xauthority via XAUTHORITY env. + authFile string } // NewX11InputInjector connects to the X11 display and initializes XTest. -func NewX11InputInjector(display string) (*X11InputInjector, error) { +// Empty cookieHex/authFile fall back to XAUTHORITY env lookup. +func NewX11InputInjector(display, cookieHex, authFile string) (*X11InputInjector, error) { detectX11Display() if display == "" { @@ -38,7 +41,13 @@ func NewX11InputInjector(display string) (*X11InputInjector, error) { return nil, fmt.Errorf("DISPLAY not set and no Xorg process found") } - conn, err := xgb.NewConnDisplay(display) + var conn *xgb.Conn + var err error + if cookieHex != "" { + conn, err = dialXUnixWithCookie(display, cookieHex) + } else { + conn, err = xgb.NewConnDisplay(display) + } if err != nil { return nil, fmt.Errorf("connect to X11 display %s: %w", display, err) } @@ -56,10 +65,11 @@ func NewX11InputInjector(display string) (*X11InputInjector, error) { screen := setup.Roots[0] inj := &X11InputInjector{ - conn: conn, - root: screen.Root, - screen: &screen, - display: display, + conn: conn, + root: screen.Root, + screen: &screen, + display: display, + authFile: authFile, } inj.cacheKeyboardMapping() inj.resolveClipboardTool() @@ -297,8 +307,13 @@ func (x *X11InputInjector) GetClipboard() string { func (x *X11InputInjector) clipboardEnv() []string { env := []string{envDisplay + "=" + x.display} - if auth := os.Getenv(envXAuthority); auth != "" { - env = append(env, envXAuthority+"="+auth) + switch { + case x.authFile != "": + env = append(env, envXAuthority+"="+x.authFile) + default: + if auth := os.Getenv(envXAuthority); auth != "" { + env = append(env, envXAuthority+"="+auth) + } } return env } diff --git a/client/vnc/server/virtual_x11.go b/client/vnc/server/virtual_x11.go index 9f8886bc2..f19dbdbf5 100644 --- a/client/vnc/server/virtual_x11.go +++ b/client/vnc/server/virtual_x11.go @@ -15,6 +15,8 @@ import ( "time" log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/configs" ) // VirtualSession manages a virtual X11 display (Xvfb) with a desktop session @@ -25,6 +27,9 @@ const ( defaultSessionWidth uint16 = 1280 defaultSessionHeight uint16 = 800 + + vncXAuthSubdir = "vnc-xauth" + vncXAuthNameFmt = "X%s-%d" ) type VirtualSession struct { @@ -44,7 +49,12 @@ type VirtualSession struct { stopped bool clients int idleTimer *time.Timer - onIdle func() // called when idle timeout fires or Xvfb dies + // onIdle fires when the idle timeout elapses or the X server dies. + onIdle func() + // cookieHex authenticates X clients against our Xvfb instance. + cookieHex string + // authFile backs cookieHex on disk for Xvfb (-auth) and the desktop env. + authFile string } // StartVirtualSession creates and starts a virtual X11 session for the given @@ -114,28 +124,36 @@ func (vs *VirtualSession) start() error { } vs.display = display + if err := vs.prepareXAuth(); err != nil { + return fmt.Errorf("prepare xauth: %w", err) + } + if err := vs.startXvfb(); err != nil { + vs.cleanupXAuth() return err } socketPath := fmt.Sprintf("%s/X%s", x11SocketDir, vs.display[1:]) if err := waitForPath(socketPath, 5*time.Second); err != nil { vs.stopXvfb() + vs.cleanupXAuth() return fmt.Errorf("wait for X11 socket %s: %w", socketPath, err) } - // Grant the target user access to the display via xhost. - xhostCmd := exec.Command("xhost", "+SI:localuser:"+vs.user.Username) - xhostCmd.Env = []string{envDisplay + "=" + vs.display} - if out, err := xhostCmd.CombinedOutput(); err != nil { - vs.log.Debugf("xhost: %s (%v)", strings.TrimSpace(string(out)), err) + // Restrict the X socket to root and the target user. + if err := os.Chown(socketPath, int(vs.uid), int(vs.gid)); err != nil { + vs.log.Debugf("chown X socket: %v", err) + } + if err := os.Chmod(socketPath, 0700); err != nil { + vs.log.Debugf("chmod X socket: %v", err) } - vs.poller = NewX11Poller(vs.display) + vs.poller = NewX11Poller(vs.display, vs.cookieHex) - injector, err := NewX11InputInjector(vs.display) + injector, err := NewX11InputInjector(vs.display, vs.cookieHex, vs.authFile) if err != nil { vs.stopXvfb() + vs.cleanupXAuth() return fmt.Errorf("create X11 injector for %s: %w", vs.display, err) } vs.injector = injector @@ -143,6 +161,7 @@ func (vs *VirtualSession) start() error { if err := vs.startDesktop(); err != nil { vs.injector.Close() vs.stopXvfb() + vs.cleanupXAuth() return fmt.Errorf("start desktop: %w", err) } @@ -247,6 +266,7 @@ func (vs *VirtualSession) Stop() { vs.stopDesktop() vs.stopXvfb() + vs.cleanupXAuth() vs.log.Info("virtual session stopped") } @@ -263,6 +283,7 @@ func (vs *VirtualSession) startXvfbDirect() error { vs.xvfb = exec.Command("Xvfb", vs.display, "-screen", "0", geom, "-nolisten", "tcp", + "-auth", vs.authFile, ) vs.xvfb.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Pdeathsig: syscall.SIGTERM} @@ -318,6 +339,7 @@ EndSection "-config", confPath, "-noreset", "-nolisten", "tcp", + "-auth", vs.authFile, ) vs.xvfb.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Pdeathsig: syscall.SIGTERM} @@ -357,6 +379,7 @@ func (vs *VirtualSession) monitorXvfb() { vs.injector.Close() } vs.stopDesktop() + vs.cleanupXAuth() } onIdle := vs.onIdle vs.mu.Unlock() @@ -436,6 +459,7 @@ func (vs *VirtualSession) monitorDesktop() { vs.injector.Close() } vs.stopXvfb() + vs.cleanupXAuth() } onIdle := vs.onIdle vs.mu.Unlock() @@ -459,7 +483,7 @@ func (vs *VirtualSession) stopDesktop() { } func (vs *VirtualSession) buildUserEnv() []string { - return []string{ + env := []string{ envDisplay + "=" + vs.display, "HOME=" + vs.user.HomeDir, "USER=" + vs.user.Username, @@ -469,6 +493,46 @@ func (vs *VirtualSession) buildUserEnv() []string { "XDG_RUNTIME_DIR=/run/user/" + vs.user.Uid, "DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/" + vs.user.Uid + "/bus", } + if vs.authFile != "" { + env = append(env, envXAuthority+"="+vs.authFile) + } + return env +} + +// prepareXAuth generates a per-session cookie and writes it to an +// Xauthority file owned by the target user. +func (vs *VirtualSession) prepareXAuth() error { + if configs.RuntimeDir == "" { + return fmt.Errorf("no runtime directory configured for this platform") + } + cookie, hexStr, err := generateXAuthCookie() + if err != nil { + return err + } + hostname, err := os.Hostname() + if err != nil { + return fmt.Errorf("hostname: %w", err) + } + displayNum := strings.TrimPrefix(vs.display, ":") + authPath := filepath.Join(configs.RuntimeDir, vncXAuthSubdir, fmt.Sprintf(vncXAuthNameFmt, displayNum, vs.uid)) + if err := writeXAuthFile(authPath, hostname, displayNum, cookie, vs.uid, vs.gid); err != nil { + return err + } + vs.cookieHex = hexStr + vs.authFile = authPath + return nil +} + +// cleanupXAuth removes the Xauthority file written by prepareXAuth. +func (vs *VirtualSession) cleanupXAuth() { + if vs.authFile == "" { + return + } + if err := os.Remove(vs.authFile); err != nil && !os.IsNotExist(err) { + vs.log.Debugf("remove xauth: %v", err) + } + vs.authFile = "" + vs.cookieHex = "" } // detectDesktopSession discovers available desktop sessions from the standard @@ -667,10 +731,37 @@ type sessionManager struct { } func newSessionManager(logger *log.Entry) *sessionManager { - return &sessionManager{ + sm := &sessionManager{ sessions: make(map[string]*VirtualSession), log: logger, } + sm.sweepStaleXAuth() + return sm +} + +// sweepStaleXAuth removes Xauthority files left over from a previous daemon +// instance whose X servers are no longer running. +func (sm *sessionManager) sweepStaleXAuth() { + if configs.RuntimeDir == "" { + return + } + dir := filepath.Join(configs.RuntimeDir, vncXAuthSubdir) + entries, err := os.ReadDir(dir) + if err != nil { + if !os.IsNotExist(err) { + sm.log.Debugf("scan stale xauth dir: %v", err) + } + return + } + for _, e := range entries { + if e.IsDir() { + continue + } + p := filepath.Join(dir, e.Name()) + if err := os.Remove(p); err != nil { + sm.log.Debugf("remove stale xauth %s: %v", p, err) + } + } } // GetOrCreate returns an existing virtual session or creates a new one with diff --git a/client/vnc/server/xauth_linux.go b/client/vnc/server/xauth_linux.go new file mode 100644 index 000000000..a8fbae884 --- /dev/null +++ b/client/vnc/server/xauth_linux.go @@ -0,0 +1,124 @@ +//go:build unix && !darwin && !ios && !android + +package server + +import ( + "crypto/rand" + "encoding/binary" + "encoding/hex" + "fmt" + "net" + "os" + "path/filepath" + "strings" + + "github.com/jezek/xgb" + + "github.com/netbirdio/netbird/client/configs" +) + +// xauthFamilyLocal is the Xauth.h family value for AF_UNIX connections. +const ( + xauthFamilyLocal uint16 = 256 + xauthMITMagic = "MIT-MAGIC-COOKIE-1" +) + +// generateXAuthCookie returns a fresh 16-byte MIT-MAGIC-COOKIE-1 and its hex form. +func generateXAuthCookie() (cookie []byte, hexStr string, err error) { + cookie = make([]byte, 16) + if _, err := rand.Read(cookie); err != nil { + return nil, "", fmt.Errorf("read random cookie: %w", err) + } + return cookie, hex.EncodeToString(cookie), nil +} + +// writeXAuthFile writes a single MIT-MAGIC-COOKIE-1 entry in the binary +// Xauthority format, chowned to uid/gid and mode 0600. +func writeXAuthFile(path, hostname, display string, cookie []byte, uid, gid uint32) error { + if len(cookie) != 16 { + return fmt.Errorf("cookie must be 16 bytes") + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0711); err != nil { + return fmt.Errorf("mkdir xauth parent: %w", err) + } + // Ensure every component the daemon owns is traversable so the target + // user's desktop process can reach its file. The leaf file is still + // mode 0600 chowned to the user, and 0711 hides directory listings + // from non-owners. + if err := ensureTraversable(dir); err != nil { + return fmt.Errorf("relax xauth parent perms: %w", err) + } + + var buf []byte + appendField := func(b []byte) { + var l [2]byte + binary.BigEndian.PutUint16(l[:], uint16(len(b))) + buf = append(buf, l[:]...) + buf = append(buf, b...) + } + var fam [2]byte + binary.BigEndian.PutUint16(fam[:], xauthFamilyLocal) + buf = append(buf, fam[:]...) + appendField([]byte(hostname)) + appendField([]byte(display)) + appendField([]byte(xauthMITMagic)) + appendField(cookie) + + tmp := path + ".tmp" + if err := os.WriteFile(tmp, buf, 0600); err != nil { + return fmt.Errorf("write xauth tmp: %w", err) + } + if err := os.Chown(tmp, int(uid), int(gid)); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("chown xauth tmp: %w", err) + } + if err := os.Rename(tmp, path); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("rename xauth: %w", err) + } + return nil +} + +// ensureTraversable walks up from dir to configs.RuntimeDir (inclusive) and +// sets mode 0711 on each component. Stops once it leaves the runtime dir so +// it never touches /var/run or /run. +func ensureTraversable(dir string) error { + root := filepath.Clean(configs.RuntimeDir) + if root == "" { + return nil + } + cur := filepath.Clean(dir) + for { + if err := os.Chmod(cur, 0711); err != nil { + return fmt.Errorf("chmod %s: %w", cur, err) + } + if cur == root { + return nil + } + parent := filepath.Dir(cur) + if parent == cur || !strings.HasPrefix(cur, root+string(os.PathSeparator)) { + return nil + } + cur = parent + } +} + +// dialXUnixWithCookie opens an xgb connection to display over AF_UNIX, +// authenticating with the supplied hex cookie instead of XAUTHORITY env. +func dialXUnixWithCookie(display, cookieHex string) (*xgb.Conn, error) { + if len(display) < 2 || display[0] != ':' { + return nil, fmt.Errorf("invalid X display %q", display) + } + sock := fmt.Sprintf("%s/X%s", x11SocketDir, display[1:]) + nc, err := net.Dial("unix", sock) + if err != nil { + return nil, fmt.Errorf("dial X socket %s: %w", sock, err) + } + conn, err := xgb.NewConnNetWithCookieHex(nc, cookieHex) + if err != nil { + _ = nc.Close() + return nil, fmt.Errorf("xgb auth on %s: %w", display, err) + } + return conn, nil +}