diff --git a/client/vnc/server/input_darwin.go b/client/vnc/server/input_darwin.go index 68b3729e8..ed04fda40 100644 --- 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 { diff --git a/client/vnc/server/input_uinput_linux.go b/client/vnc/server/input_uinput_linux.go index 505fbc880..971aef9ef 100644 --- 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 diff --git a/client/vnc/server/input_uinput_linux_test.go b/client/vnc/server/input_uinput_linux_test.go new file mode 100644 index 000000000..749fd5bd5 --- /dev/null +++ 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") +} diff --git a/client/vnc/server/input_x11.go b/client/vnc/server/input_x11.go index 3c9432dfc..60a703105 100644 --- 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() { diff --git a/shared/management/networkmap/networkmapcompute.go b/shared/management/networkmap/networkmapcompute.go index 65e76d097..602937685 100644 --- 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, diff --git a/shared/management/networkmap/networkmapcompute_test.go b/shared/management/networkmap/networkmapcompute_test.go index ad21fd70f..c0abfe25a 100644 --- 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 {