Stop narrowing the shared runtime dir, close the injector on stop, allowlist VNC metrics

client/internal/engine_vnc_console_freebsd.go

@@ -10,11 +10,13 @@ import (
 	vncserver "github.com/netbirdio/netbird/client/vnc/server"
 )
 
-// newConsoleVNC builds the FreeBSD console fallback: vt(4) framebuffer
-// for capture, /dev/uinput for input. The uinput device requires the
-// `uinput` kernel module (`kldload uinput`); without it, input init
-// fails and we drop to a stub injector so the user still gets a
-// view-only screen mirror.
+// newConsoleVNC builds the FreeBSD console fallback: the vt(4) framebuffer for
+// capture, and no input.
+//
+// Input injection is not implemented on FreeBSD: the uinput injector is a
+// Linux-only implementation built on UI_DEV_CREATE and friends, so this backend
+// mirrors the console read-only. It is offered anyway because a view-only
+// console is still worth more than nothing on a box with no X server.
 func newConsoleVNC() (vncserver.ScreenCapturer, vncserver.InputInjector, error) {
 	poller := vncserver.NewFBPoller("")
 	w, h := poller.Width(), poller.Height()
@@ -22,10 +24,6 @@ func newConsoleVNC() (vncserver.ScreenCapturer, vncserver.InputInjector, error)
 		poller.Close()
 		return nil, nil, fmt.Errorf("vt framebuffer init failed (vt may not allow mmap on this driver)")
 	}
-	if inj, err := vncserver.NewUInputInjector(w, h); err == nil {
-		return poller, inj, nil
-	} else {
-		log.Infof("VNC console: uinput unavailable (%v); view-only mode. Run `kldload uinput` to enable input.", err)
-		return poller, &vncserver.StubInputInjector{}, nil
-	}
+	log.Info("VNC console: FreeBSD has no input backend, serving the console view-only")
+	return poller, &vncserver.StubInputInjector{}, nil
 }

client/internal/metrics/infra/ingest/main.go

@@ -85,6 +85,26 @@ var allowedMeasurements = map[string]measurementSpec{
 			"peer_id":         true,
 		},
 	},
+	// Emitted per VNC session tick by influxDBMetrics.RecordVNCSessionTick.
+	"netbird_vnc_traffic": {
+		allowedFields: map[string]bool{
+			"period_seconds":     true,
+			"bytes_out":          true,
+			"writes":             true,
+			"fbus":               true,
+			"max_fbu_bytes":      true,
+			"max_fbu_rects":      true,
+			"max_write_bytes":    true,
+			"write_time_seconds": true,
+		},
+		allowedTags: map[string]bool{
+			"deployment_type": true,
+			"version":         true,
+			"os":              true,
+			"arch":            true,
+			"peer_id":         true,
+		},
+	},
 }
 
 func main() {

client/vnc/server/server.go

@@ -761,6 +761,13 @@ func (s *Server) Stop() error {
 	if c, ok := s.capturer.(interface{ Close() }); ok {
 		c.Close()
 	}
+	// The injector owns OS resources of its own: the uinput backend holds a
+	// /dev/uinput descriptor and a registered virtual device, the X11 one an
+	// X connection. Leaving them open leaks one of each every time the VNC
+	// server is stopped and started again.
+	if i, ok := s.injector.(interface{ Close() }); ok {
+		i.Close()
+	}
 
 	if listenerErr != nil {
 		return fmt.Errorf("close VNC listener: %w", listenerErr)

client/vnc/server/xauth_x11.go

@@ -81,8 +81,15 @@ func writeXAuthFile(path, hostname, display string, cookie []byte, uid, gid uint
 }
 
 // ensureTraversable walks up from dir to configs.RuntimeDir (inclusive) and
-// sets mode 0711 on each component. A dir outside the runtime dir is refused
-// before anything is chmodded, so it never touches /var/run or /run.
+// makes each component traversable by the session's user, so the X server can
+// reach its Xauthority file. A dir outside the runtime dir is refused before
+// anything is changed, so it never touches /var/run or /run.
+//
+// Only the execute bits are added, never a whole mode. The runtime dir is
+// shared: the daemon advertises its socket there and unprivileged CLI and UI
+// clients list it to find one, so setting it to 0711 would take away the read
+// bit they need and break socket discovery. Execute alone grants traversal
+// without exposing a listing.
 func ensureTraversable(dir string) error {
 	root := filepath.Clean(configs.RuntimeDir)
 	if root == "" {
@@ -93,8 +100,8 @@ func ensureTraversable(dir string) error {
 		return fmt.Errorf("xauth dir %s is outside the runtime dir %s", cur, root)
 	}
 	for {
-		if err := os.Chmod(cur, 0711); err != nil {
-			return fmt.Errorf("chmod %s: %w", cur, err)
+		if err := addTraversalBits(cur); err != nil {
+			return err
 		}
 		if cur == root {
 			return nil
@@ -107,6 +114,24 @@ func ensureTraversable(dir string) error {
 	}
 }
 
+// addTraversalBits ORs group and other execute onto dir's mode, leaving every
+// other bit as it was. A directory that is already traversable is not touched.
+func addTraversalBits(dir string) error {
+	info, err := os.Stat(dir)
+	if err != nil {
+		return fmt.Errorf("stat %s: %w", dir, err)
+	}
+	const traversal = 0o011
+	mode := info.Mode().Perm()
+	if mode&traversal == traversal {
+		return nil
+	}
+	if err := os.Chmod(dir, mode|traversal); err != nil {
+		return fmt.Errorf("chmod %s: %w", dir, err)
+	}
+	return nil
+}
+
 // 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) {

client/vnc/server/xauth_x11_test.go

@@ -0,0 +1,73 @@
+//go:build (linux && !android) || freebsd
+
+package server
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/configs"
+)
+
+// withRuntimeDir points configs.RuntimeDir at a temp tree for the test.
+func withRuntimeDir(t *testing.T) string {
+	t.Helper()
+	root := t.TempDir()
+	prev := configs.RuntimeDir
+	configs.RuntimeDir = root
+	t.Cleanup(func() { configs.RuntimeDir = prev })
+	return root
+}
+
+func permOf(t *testing.T, dir string) os.FileMode {
+	t.Helper()
+	info, err := os.Stat(dir)
+	require.NoError(t, err)
+	return info.Mode().Perm()
+}
+
+// The runtime dir is shared: the daemon advertises its socket there and
+// unprivileged clients list it to find one. Making the xauth dir traversable
+// must not cost the read bit that listing needs.
+func TestEnsureTraversableKeepsTheRuntimeDirReadable(t *testing.T) {
+	root := withRuntimeDir(t)
+	require.NoError(t, os.Chmod(root, 0o755))
+
+	sub := filepath.Join(root, vncXAuthSubdir)
+	require.NoError(t, os.Mkdir(sub, 0o700))
+
+	require.NoError(t, ensureTraversable(sub))
+
+	assert.Equal(t, os.FileMode(0o755), permOf(t, root), "an already-traversable shared dir must be left alone")
+	assert.Equal(t, os.FileMode(0o711), permOf(t, sub), "the xauth dir gains traversal, keeping its own bits")
+}
+
+// A runtime dir that is not traversable gains execute, and nothing else: no
+// read bit is handed out that was not there before.
+func TestEnsureTraversableAddsOnlyExecute(t *testing.T) {
+	root := withRuntimeDir(t)
+	require.NoError(t, os.Chmod(root, 0o700))
+
+	sub := filepath.Join(root, vncXAuthSubdir)
+	require.NoError(t, os.Mkdir(sub, 0o700))
+
+	require.NoError(t, ensureTraversable(sub))
+
+	assert.Equal(t, os.FileMode(0o711), permOf(t, root))
+	assert.Equal(t, os.FileMode(0o711), permOf(t, sub))
+}
+
+// A path outside the runtime dir is refused before anything is modified.
+func TestEnsureTraversableRefusesOutsidePaths(t *testing.T) {
+	withRuntimeDir(t)
+
+	outside := t.TempDir()
+	require.NoError(t, os.Chmod(outside, 0o700))
+
+	require.Error(t, ensureTraversable(outside))
+	assert.Equal(t, os.FileMode(0o700), permOf(t, outside), "a refused path must not be chmodded")
+}