Resolve VNC authorized users on the components path and fix uinput, X11 and macOS input gaps

2026-09-02 04:51:29 +02:00 · 2026-08-29 09:04:02 +02:00
parent 4a0fe09ced
commit 95e86deeb8
6 changed files with 194 additions and 36 deletions
--- a/client/vnc/server/input_darwin.go
+++ b/client/vnc/server/input_darwin.go
@@ -840,6 +840,10 @@ func (m *MacInputInjector) SetClipboard(text string) {
 // login screens, locked-down apps). ASCII printable runes only; others
 // are skipped.
 func (m *MacInputInjector) TypeText(text string) {
+	// Same permission the other injection paths need: without it the posted
+	// events are swallowed, so asking here is what makes the prompt appear
+	// instead of PasteAndType silently doing nothing.
+	m.ensureAccessibility()
 	wakeDisplay()
 	src := ensureEventSource()
 	if src == 0 {
--- a/client/vnc/server/input_uinput_linux.go
+++ b/client/vnc/server/input_uinput_linux.go
@@ -5,6 +5,7 @@ package server
 import (
 	"encoding/binary"
 	"fmt"
+	"slices"
 	"sync"
 	"time"
 	"unicode"
@@ -36,6 +37,10 @@ const (

 	synReport = 0

+	// keyMaxCode is the kernel's KEY_MAX: the largest code UI_SET_KEYBIT
+	// accepts. Anything above it would make the ioctl fail.
+	keyMaxCode = 0x2ff
+
 	absX = 0x00
 	absY = 0x01

@@ -46,14 +51,15 @@ const (
 	btnExtra  = 0x114 // mouse-forward (X2)
 )

-// inputEvent matches struct input_event for x86_64 (timeval is 16 bytes).
-// Total size 24 bytes; Go's natural alignment matches the kernel layout.
+// inputEvent matches struct input_event. The leading timeval is two C longs,
+// so the struct is 24 bytes on a 64-bit kernel and 16 on a 32-bit one; using
+// unix.Timeval rather than a fixed pair of int64 keeps the layout right on
+// both, and a wrong-sized write is rejected outright by uinput.
 type inputEvent struct {
-	TvSec  int64
-	TvUsec int64
-	Type   uint16
-	Code   uint16
-	Value  int32
+	Time  unix.Timeval
+	Type  uint16
+	Code  uint16
+	Value int32
 }

 // UInputInjector synthesizes keyboard and mouse events via /dev/uinput.
@@ -358,7 +364,31 @@ func buildUInputKeymap() []uint16 {
 		keyUp, keyDown, keyLeft, keyRight,
 		keyInsert, keyDelete,
 	}...)
-	return out
+
+	// The QEMU scancode path can emit anything qemuToLinuxKey maps to: the
+	// keypad, the lock keys, PrintScreen, the volume keys and Compose are all
+	// reachable that way and none of them are listed above. A code the device
+	// never advertised is dropped by the kernel without a word, so the set is
+	// closed over that map rather than maintained by hand alongside it.
+	seen := make(map[uint16]struct{}, len(out)+len(qemuToLinuxKey))
+	for _, code := range out {
+		seen[code] = struct{}{}
+	}
+	extra := make([]uint16, 0, len(qemuToLinuxKey))
+	for _, code := range qemuToLinuxKey {
+		if code <= 0 || code > keyMaxCode {
+			continue
+		}
+		if _, ok := seen[uint16(code)]; ok {
+			continue
+		}
+		seen[uint16(code)] = struct{}{}
+		extra = append(extra, uint16(code))
+	}
+	// Sorted so the device registers the same set in the same order on every
+	// run; ranging a map alone would not.
+	slices.Sort(extra)
+	return append(out, extra...)
 }

 // keymapByKeysym maps X11 keysyms (the values our session receives over
--- a/client/vnc/server/input_uinput_linux_test.go
+++ b/client/vnc/server/input_uinput_linux_test.go
@@ -0,0 +1,48 @@
+//go:build linux
+
+package server
+
+import (
+	"sort"
+	"testing"
+	"unsafe"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// The kernel rejects a write whose length is not exactly sizeof(struct
+// input_event), so the mirror has to track the platform's timeval: 24 bytes
+// where a C long is 8, 16 where it is 4.
+func TestInputEventMatchesKernelABI(t *testing.T) {
+	const longSize = unsafe.Sizeof(uintptr(0))
+	want := uintptr(2)*longSize + 2 + 2 + 4
+	// The struct is aligned to its widest field, which is the long.
+	if pad := want % longSize; pad != 0 {
+		want += longSize - pad
+	}
+	if got := unsafe.Sizeof(inputEvent{}); got != want {
+		t.Fatalf("sizeof(inputEvent) = %d, want %d for a %d-byte long", got, want, longSize)
+	}
+}
+
+// uinput only delivers key events whose code was advertised with UI_SET_KEYBIT
+// at device-creation time. Anything qemuToLinuxKey can produce therefore has to
+// appear in buildUInputKeymap, or the kernel silently drops those keys.
+func TestUInputKeymapAdvertisesEveryMappedScancode(t *testing.T) {
+	advertised := make(map[uint16]struct{})
+	for _, code := range buildUInputKeymap() {
+		advertised[code] = struct{}{}
+	}
+
+	var missing []int
+	for _, code := range qemuToLinuxKey {
+		if code == 0 {
+			continue
+		}
+		if _, ok := advertised[uint16(code)]; !ok {
+			missing = append(missing, code)
+		}
+	}
+	sort.Ints(missing)
+	assert.Empty(t, missing, "KEY_ codes reachable through qemuToLinuxKey but never advertised to uinput")
+}
--- a/client/vnc/server/input_x11.go
+++ b/client/vnc/server/input_x11.go
@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"sync"

 	log "github.com/sirupsen/logrus"

@@ -17,6 +18,11 @@ import (

 // X11InputInjector injects keyboard and mouse events via the XTest extension.
 type X11InputInjector struct {
+	// inputMu serializes event emission. Attach mode shares one injector
+	// between every session, so without it two clients interleave their button
+	// transitions, and a keystroke can land between another session's
+	// Shift-down and Shift-up and come out as the wrong character.
+	inputMu           sync.Mutex
 	conn              *xgb.Conn
 	root              xproto.Window
 	screen            *xproto.ScreenInfo
@@ -82,6 +88,13 @@ func NewX11InputInjector(display, cookieHex, authFile string) (*X11InputInjector

 // InjectKey simulates a key press or release. keysym is an X11 KeySym.
 func (x *X11InputInjector) InjectKey(keysym uint32, down bool) {
+	x.inputMu.Lock()
+	defer x.inputMu.Unlock()
+	x.injectKeyLocked(keysym, down)
+}
+
+// injectKeyLocked is InjectKey with inputMu already held.
+func (x *X11InputInjector) injectKeyLocked(keysym uint32, down bool) {
 	keycode := x.keysymToKeycode(keysym)
 	if keycode == 0 {
 		return
@@ -96,9 +109,12 @@ func (x *X11InputInjector) InjectKey(keysym uint32, down bool) {
 // resulting character. Falls back to the keysym path when the scancode
 // has no Linux mapping.
 func (x *X11InputInjector) InjectKeyScancode(scancode, keysym uint32, down bool) {
+	x.inputMu.Lock()
+	defer x.inputMu.Unlock()
+
 	linuxKey := qemuScancodeToLinuxKey(scancode)
 	if linuxKey == 0 {
-		x.InjectKey(keysym, down)
+		x.injectKeyLocked(keysym, down)
 		return
 	}
 	x.fakeKeyEvent(byte(linuxKey+xkbKeycodeOffset), down)
@@ -127,6 +143,11 @@ func (x *X11InputInjector) InjectPointer(buttonMask uint16, px, py, serverW, ser
 		return
 	}

+	// Held across the whole dispatch: each transition below is derived from
+	// lastButtons and the write closes the sequence.
+	x.inputMu.Lock()
+	defer x.inputMu.Unlock()
+
 	// Scale to actual screen coordinates.
 	screenW := int(x.screen.WidthInPixels)
 	screenH := int(x.screen.HeightInPixels)
@@ -252,23 +273,34 @@ func (x *X11InputInjector) TypeText(text string) {
 		if !ok {
 			continue
 		}
-		keycode := x.keysymToKeycode(keysym)
-		if keycode == 0 {
-			continue
-		}
-		var shiftCode byte
-		if shift {
-			shiftCode = x.keysymToKeycode(0xffe1) // Shift_L
-			if shiftCode != 0 {
-				xtest.FakeInput(x.conn, xproto.KeyPress, shiftCode, 0, x.root, 0, 0, 0)
-			}
-		}
-		xtest.FakeInput(x.conn, xproto.KeyPress, keycode, 0, x.root, 0, 0, 0)
-		xtest.FakeInput(x.conn, xproto.KeyRelease, keycode, 0, x.root, 0, 0, 0)
-		if shift && shiftCode != 0 {
-			xtest.FakeInput(x.conn, xproto.KeyRelease, shiftCode, 0, x.root, 0, 0, 0)
+		x.typeRuneLocked(keysym, shift)
+	}
+}
+
+// typeRuneLocked emits one rune, framed by Shift-down/up when the keysym needs
+// it. Locked per rune rather than for the whole string: the framing has to be
+// atomic, but a long paste must not hold another session's pointer off for the
+// length of it.
+func (x *X11InputInjector) typeRuneLocked(keysym uint32, shift bool) {
+	x.inputMu.Lock()
+	defer x.inputMu.Unlock()
+
+	keycode := x.keysymToKeycode(keysym)
+	if keycode == 0 {
+		return
+	}
+	var shiftCode byte
+	if shift {
+		shiftCode = x.keysymToKeycode(0xffe1) // Shift_L
+		if shiftCode != 0 {
+			xtest.FakeInput(x.conn, xproto.KeyPress, shiftCode, 0, x.root, 0, 0, 0)
 		}
 	}
+	xtest.FakeInput(x.conn, xproto.KeyPress, keycode, 0, x.root, 0, 0, 0)
+	xtest.FakeInput(x.conn, xproto.KeyRelease, keycode, 0, x.root, 0, 0, 0)
+	if shift && shiftCode != 0 {
+		xtest.FakeInput(x.conn, xproto.KeyRelease, shiftCode, 0, x.root, 0, 0, 0)
+	}
 }

 func (x *X11InputInjector) resolveClipboardTool() {
--- a/shared/management/networkmap/networkmapcompute.go
+++ b/shared/management/networkmap/networkmapcompute.go
@@ -7,7 +7,11 @@ import (
 	"github.com/netbirdio/netbird/shared/management/types"
 )

-type sshRequirements struct {
+// authRequirements records the authorization inputs a peer's policies actually
+// need, so the components carry the group-to-user mapping and the allowed-user
+// set only when some rule resolves users from them. Both the SSH and the VNC
+// marker protocols do.
+type authRequirements struct {
 	neededGroupIDs     map[string]struct{}
 	needAllowedUserIDs bool
 }
@@ -57,12 +61,12 @@ func (nmd *NetworkMapData) GetPeerNetworkMapComponents(peerID string, peersCusto
 		ForceRoutingPeerDNSResolution: forceRoutingPeerDNS,
 	}

-	relevantPeers, relevantGroups, relevantPolicies, relevantRoutes, sshReqs := nmd.getPeersGroupsPoliciesRoutes(peerID, peer.SSHEnabled, &components.PostureFailedPeers)
+	relevantPeers, relevantGroups, relevantPolicies, relevantRoutes, authReqs := nmd.getPeersGroupsPoliciesRoutes(peerID, peer.SSHEnabled, &components.PostureFailedPeers)

-	if len(sshReqs.neededGroupIDs) > 0 {
-		components.GroupIDToUserIDs = filterGroupIDToUserIDs(nmd.GroupIDToUserIDs, sshReqs.neededGroupIDs)
+	if len(authReqs.neededGroupIDs) > 0 {
+		components.GroupIDToUserIDs = filterGroupIDToUserIDs(nmd.GroupIDToUserIDs, authReqs.neededGroupIDs)
 	}
-	if sshReqs.needAllowedUserIDs {
+	if authReqs.needAllowedUserIDs {
 		components.AllowedUserIDs = nmd.getAllowedUserIDs()
 	}

@@ -204,12 +208,12 @@ func (nmd *NetworkMapData) getPeersGroupsPoliciesRoutes(
 	peerID string,
 	peerSSHEnabled bool,
 	postureFailedPeers *map[string]map[string]struct{},
-) (map[string]*nmdata.Peer, map[string]*nmdata.Group, []*nmdata.Policy, []*nmdata.Route, sshRequirements) {
+) (map[string]*nmdata.Peer, map[string]*nmdata.Group, []*nmdata.Policy, []*nmdata.Route, authRequirements) {
 	relevantPeerIDs := make(map[string]*nmdata.Peer, len(nmd.Peers)/4)
 	relevantGroupIDs := make(map[string]*nmdata.Group, len(nmd.Groups)/4)
 	relevantPolicies := make([]*nmdata.Policy, 0, len(nmd.Policies))
 	relevantRoutes := make([]*nmdata.Route, 0, len(nmd.Routes))
-	sshReqs := sshRequirements{neededGroupIDs: make(map[string]struct{})}
+	authReqs := authRequirements{neededGroupIDs: make(map[string]struct{})}

 	relevantPeerIDs[peerID] = nmd.Peers[peerID]

@@ -358,18 +362,25 @@ func (nmd *NetworkMapData) getPeersGroupsPoliciesRoutes(
 					}
 				}

-				if rule.Protocol == string(types.PolicyRuleProtocolNetbirdSSH) {
+				// Both marker protocols resolve authorized users the same way,
+				// so a VNC rule needs the same inputs an SSH rule does. Leaving
+				// VNC out here strips the group mapping and the allowed-user set
+				// from the components, and the rule then reaches the resolver
+				// with nobody authorized.
+				if rule.Protocol == string(types.PolicyRuleProtocolNetbirdSSH) ||
+					rule.Protocol == string(types.PolicyRuleProtocolNetbirdVNC) {
 					switch {
 					case len(rule.AuthorizedGroups) > 0:
 						for groupID := range rule.AuthorizedGroups {
-							sshReqs.neededGroupIDs[groupID] = struct{}{}
+							authReqs.neededGroupIDs[groupID] = struct{}{}
 						}
 					case rule.AuthorizedUser != "":
+						// Carries its own user; no lookup inputs needed.
 					default:
-						sshReqs.needAllowedUserIDs = true
+						authReqs.needAllowedUserIDs = true
 					}
 				} else if nmdata.PolicyRuleImpliesLegacySSH(rule) && peerSSHEnabled {
-					sshReqs.needAllowedUserIDs = true
+					authReqs.needAllowedUserIDs = true
 				}
 			}
 		}
@@ -378,7 +389,7 @@ func (nmd *NetworkMapData) getPeersGroupsPoliciesRoutes(
 		}
 	}

-	return relevantPeerIDs, relevantGroupIDs, relevantPolicies, relevantRoutes, sshReqs
+	return relevantPeerIDs, relevantGroupIDs, relevantPolicies, relevantRoutes, authReqs
 }

 func (nmd *NetworkMapData) getPeersFromGroups(groups []string, peerID string, sourcePostureChecksIDs []string,
--- a/shared/management/networkmap/networkmapcompute_test.go
+++ b/shared/management/networkmap/networkmapcompute_test.go
@@ -968,6 +968,39 @@ func TestGetPeerNetworkMapComponents_SSHRequirements(t *testing.T) {
 			},
 			targetInSrc: true,
 		},
+
+		// VNC resolves authorized users exactly the way SSH does, so it needs
+		// the same inputs carried into the components. Leaving it out strips
+		// them and the rule reaches the resolver with nobody authorized.
+		{
+			name: "netbird-vnc with authorized groups",
+			mutateRule: func(r *nmdata.PolicyRule) {
+				r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC)
+				r.AuthorizedGroups = map[string][]string{"g-auth": nil}
+			},
+			wantGroupsMap: map[string][]string{"g-auth": {"user-a"}},
+		},
+		{
+			name: "netbird-vnc default needs allowed users",
+			mutateRule: func(r *nmdata.PolicyRule) {
+				r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC)
+			},
+			wantAllowed: true,
+		},
+		{
+			name: "netbird-vnc with authorized user carries its own",
+			mutateRule: func(r *nmdata.PolicyRule) {
+				r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC)
+				r.AuthorizedUser = "user-1"
+			},
+		},
+		{
+			name: "netbird-vnc only counts on the destination side",
+			mutateRule: func(r *nmdata.PolicyRule) {
+				r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC)
+			},
+			targetInSrc: true,
+		},
 	}

 	for _, tc := range cases {