diff --git a/client/internal/engine_vnc.go b/client/internal/engine_vnc.go index c87d5e25d..6ea099866 100644 --- a/client/internal/engine_vnc.go +++ b/client/internal/engine_vnc.go @@ -194,6 +194,10 @@ func (e *Engine) startVNCServer(authConfig *sshauth.Config) error { listenAddr := netip.AddrPortFrom(netbirdIP, vnc.InternalPort) network := e.wgInterface.Address().Network if err := srv.Start(e.ctx, listenAddr, network); err != nil { + // The server never took ownership, so nothing else will release what + // newPlatformVNC opened: the X11 injector's display connection, the + // uinput device, the framebuffer mapping. + closeVNCResources(capturer, injector) return fmt.Errorf("start VNC server: %w", err) } @@ -394,3 +398,14 @@ func (e *Engine) persistVNCProcesses(state *vncserver.ShutdownState) { log.Debugf("update VNC session state: %v", err) } } + +// closeVNCResources releases a capturer and an injector that implement Close. +// Either may be a stub that holds nothing. +func closeVNCResources(capturer vncserver.ScreenCapturer, injector vncserver.InputInjector) { + if c, ok := capturer.(interface{ Close() }); ok { + c.Close() + } + if i, ok := injector.(interface{ Close() }); ok { + i.Close() + } +} diff --git a/client/internal/engine_vnc_darwin.go b/client/internal/engine_vnc_darwin.go index 39db0e1bb..aa817c382 100644 --- a/client/internal/engine_vnc_darwin.go +++ b/client/internal/engine_vnc_darwin.go @@ -13,18 +13,22 @@ import ( func newPlatformVNC() (vncserver.ScreenCapturer, vncserver.InputInjector, bool) { capturer := vncserver.NewMacPoller() - // Ask only when this process is the one that will capture. Screen Recording - // is a user-scope TCC service, so the request is dropped from a + // Screen Recording is asked for only when this process is the one that will + // capture. It is a user-scope TCC service, so the request is dropped from a // LaunchDaemon: no prompt appears and NetBird never even reaches the Screen // Recording list. In that case the per-user agent asks instead, see - // newAgentResources. - // - // Without service mode there is no agent, so this process captures and - // nothing else will ever raise the prompt — the client would serve a - // windowless desktop with no indication why. - if !vncNeedsServiceMode() { - vncserver.RequestScreenRecording() + // newAgentResources. Without service mode there is no agent, so nothing + // else will ever raise the prompt and the client would serve a windowless + // desktop with no indication why. + if vncNeedsServiceMode() { + // The per-user agent owns capture and input in service mode, so this + // process needs neither. A real injector here would still hold its + // PreventUserIdleDisplaySleep assertion from construction, keeping + // the display awake for the daemon's whole life with no VNC session + // in sight. + return capturer, &vncserver.StubInputInjector{}, true } + vncserver.RequestScreenRecording() injector, err := vncserver.NewMacInputInjector() if err != nil { diff --git a/client/vnc/server/input_darwin.go b/client/vnc/server/input_darwin.go index 8f4c0bed2..1668f444e 100644 --- a/client/vnc/server/input_darwin.go +++ b/client/vnc/server/input_darwin.go @@ -9,6 +9,7 @@ import ( "sync" "sync/atomic" "time" + "unicode/utf16" "unsafe" "github.com/ebitengine/purego" @@ -92,6 +93,10 @@ var ( cgEventSetFlags func(uintptr, uint64) cgEventSetType func(uintptr, int32) cgEventCreateForInput func(uintptr) uintptr + // cgEventKeyboardSetUnicodeString attaches literal text to a keyboard + // event, so the receiving app gets those characters whatever the active + // keyboard layout would have produced for the keycode. + cgEventKeyboardSetUnicodeString func(uintptr, uintptr, *uint16) // CGEventCreateScrollWheelEvent is variadic, call via SyscallN. cgEventCreateScrollWheelEventAddr uintptr @@ -163,6 +168,9 @@ func initDarwinInput() { purego.RegisterLibFunc(&cgEventSetFlags, cg, "CGEventSetFlags") purego.RegisterLibFunc(&cgEventSetType, cg, "CGEventSetType") purego.RegisterLibFunc(&cgEventCreateForInput, cg, "CGEventCreate") + if sym, err := purego.Dlsym(cg, "CGEventKeyboardSetUnicodeString"); err == nil { + purego.RegisterFunc(&cgEventKeyboardSetUnicodeString, sym) + } sym, err := purego.Dlsym(cg, "CGEventCreateScrollWheelEvent") if err == nil { @@ -341,7 +349,13 @@ type MacInputInjector struct { // made so far. Input arrives continuously, so the cold path is paced by time // rather than by event count. axNextTry atomic.Int64 - axAsks atomic.Int32 + // axFirstAsk is when the first ask was made, the start of axAskWindow. + axFirstAsk atomic.Int64 + // keyMu serializes keyboard emission. One injector is shared by every + // attach-mode session, and a modifier transition and the key that follows + // it have to reach the event stream as one step: interleaved with another + // client's, one client's Shift lands on the other's keystroke. + keyMu sync.Mutex // modifiers is the CGEventFlags state the remote client has built up with // its modifier key events, stamped onto everything posted afterwards. modifiers atomic.Uint64 @@ -393,9 +407,13 @@ const ( // dropped without a trace. Asking again is the only way to land it, since // nothing in this process can observe either the dialog or the answer. axAskRetry = 8 * time.Second - // axMaxAsks bounds that, so a user who wants neither is left alone. A later - // connection asks again from a fresh process anyway. - axMaxAsks = 3 + // axAskWindow bounds that, so a user who wants neither is left alone. It is + // a span of time rather than a count of asks because the thing being waited + // out is a person answering the Screen Recording dialog: a fixed three asks + // ran out after about sixteen seconds, and a user slower than that lost + // remote input for the whole agent. A later connection asks again from a + // fresh process anyway. + axAskWindow = 2 * time.Minute ) // postEventAllowed reports whether injected events are allowed to land, reading @@ -406,6 +424,16 @@ const ( // It is deliberately not used anywhere else: AXIsProcessTrusted has the side // effect of filing the caller in the Accessibility list with the box unchecked, // and a decision on file, even that one, stops macOS from ever showing the dialog. +// requestPostEvent asks for kTCCServicePostEvent where the call exists, and +// reports whether it is granted. Where it does not exist, Accessibility is the +// gate and there is nothing further to ask for. +func requestPostEvent() bool { + if cgRequestPostEventAccess == nil { + return true + } + return cgRequestPostEventAccess() +} + func postEventAllowed() bool { if cgPreflightPostEventAccess == nil { return axProcessTrusted() @@ -438,7 +466,8 @@ func (m *MacInputInjector) askAccessibility() { return } m.axNextTry.Store(now.Add(axAskRetry).UnixNano()) - if m.axAsks.Add(1) >= axMaxAsks { + m.axFirstAsk.CompareAndSwap(0, now.UnixNano()) + if now.Sub(time.Unix(0, m.axFirstAsk.Load())) >= axAskWindow { m.axDone.Store(true) } @@ -448,7 +477,11 @@ func (m *MacInputInjector) askAccessibility() { // when the permission is already there, so this cannot produce a stray dialog. switch { case axIsProcessTrustedWithOptions != nil: - if axProcessIsTrusted() { + // Accessibility is what puts the dialog up, but CGEventPost is judged + // by kTCCServicePostEvent. A host can have the first and not the + // second, and returning on Accessibility alone would then never ask + // for the one that is missing, leaving input dead for the session. + if axProcessIsTrusted() && (postEventAllowed() || requestPostEvent()) { return } case cgRequestPostEventAccess != nil: @@ -517,6 +550,8 @@ func (m *MacInputInjector) InjectKey(keysym uint32, down bool) { if keycode == 0xFFFF { return } + m.keyMu.Lock() + defer m.keyMu.Unlock() m.postMacKey(src, keycode, down) } @@ -537,6 +572,8 @@ func (m *MacInputInjector) InjectKeyScancode(scancode, keysym uint32, down bool) m.InjectKey(keysym, down) return } + m.keyMu.Lock() + defer m.keyMu.Unlock() m.postMacKey(src, vk, down) } @@ -572,11 +609,23 @@ func (m *MacInputInjector) postMacKey(src uintptr, keycode uint16, down bool) { // bits off each event they receive, so the state has to be attached to // everything posted afterwards, which is what m.modifiers is for. func (m *MacInputInjector) postModifier(src uintptr, keycode uint16, down bool, bit uint64) { + // Caps Lock is a toggle, not a held modifier: each press flips it and the + // release changes nothing. Treating it like Shift clears it again on + // key-up, so the remote Caps Lock could never stay on. + capsLock := bit == kCGEventFlagMaskAlphaShift + if capsLock && !down { + return + } + var flags uint64 for { old := m.modifiers.Load() - flags = old | bit - if !down { + switch { + case capsLock: + flags = old ^ bit + case down: + flags = old | bit + default: flags = old &^ bit } if m.modifiers.CompareAndSwap(old, flags) { @@ -714,7 +763,7 @@ func (m *MacInputInjector) dispatchPointer(src uintptr, buttonMask uint16, x, y prev := m.lastButtons m.postMoveOrDrag(src, prev&0x01 != 0, prev&0x04 != 0, x, y) m.postButtonTransitions(src, buttonMask, x, y) - m.postScrollWheel(src, buttonMask) + m.postScrollWheel(src, prev, buttonMask) } func (m *MacInputInjector) postMoveOrDrag(src uintptr, leftDown, rightDown bool, x, y float64) { @@ -762,11 +811,16 @@ func (m *MacInputInjector) postButtonTransitions(src uintptr, buttonMask uint16, emit(1<<8, 1<<8, kCGEventOtherMouseDown, kCGEventOtherMouseUp, 4, 4) } -func (m *MacInputInjector) postScrollWheel(src uintptr, buttonMask uint16) { - if buttonMask&0x08 != 0 { +// postScrollWheel posts one tick per press of a wheel button. RFB spells a +// notch as button 4 or 5 going down; a client that keeps the bit set across +// several pointer samples is still describing that one notch, so only the +// rising edge scrolls, as on the Windows and uinput backends. +func (m *MacInputInjector) postScrollWheel(src uintptr, prev, buttonMask uint16) { + pressed := buttonMask &^ prev + if pressed&0x08 != 0 { m.postScroll(src, scrollPixelsPerWheelTick) } - if buttonMask&0x10 != 0 { + if pressed&0x10 != 0 { m.postScroll(src, -scrollPixelsPerWheelTick) } } @@ -853,6 +907,8 @@ func (m *MacInputInjector) TypeText(text string) { } const maxChars = 4096 count := 0 + m.keyMu.Lock() + defer m.keyMu.Unlock() for _, r := range text { if count >= maxChars { break @@ -865,6 +921,9 @@ func (m *MacInputInjector) TypeText(text string) { // typeRune emits the press/release events for a single ASCII rune, framing // the keystroke with Shift-down/up when required by the keysym. func (m *MacInputInjector) typeRune(src uintptr, r rune) { + if m.typeUnicodeRune(src, r) { + return + } keysym, shift, ok := keysymForASCIIRune(r) if !ok { return @@ -1062,3 +1121,31 @@ var specialKeyMap = map[uint32]uint16{ } var _ InputInjector = (*MacInputInjector)(nil) + +// typeUnicodeRune types r as literal text rather than as a key on a US +// layout, reporting false when that path is unavailable or r is a control +// character that has to arrive as its own key (Return, Tab). Mapping a rune to +// a keycode assumes the host's layout matches the table: on AZERTY or QWERTZ +// that types the wrong characters, including into password fields. Text +// attached to the event is inserted as-is, and covers non-ASCII as well. +func (m *MacInputInjector) typeUnicodeRune(src uintptr, r rune) bool { + if cgEventKeyboardSetUnicodeString == nil || r < 0x20 || r == 0x7f { + return false + } + units := utf16.Encode([]rune{r}) + for _, down := range []bool{true, false} { + event := cgEventCreateKeyboardEvent(src, 0, down) + if event == 0 { + return false + } + // No modifiers: a Shift or Option the remote client is holding would + // otherwise be applied on top of the literal character. + if cgEventSetFlags != nil { + cgEventSetFlags(event, 0) + } + cgEventKeyboardSetUnicodeString(event, uintptr(len(units)), &units[0]) + cgEventPost(kCGHIDEventTap, event) + cfRelease(event) + } + return true +} diff --git a/client/vnc/server/input_x11.go b/client/vnc/server/input_x11.go index a01bc46ce..b56811b7f 100644 --- a/client/vnc/server/input_x11.go +++ b/client/vnc/server/input_x11.go @@ -8,6 +8,7 @@ import ( "os/exec" "strings" "sync" + "time" log "github.com/sirupsen/logrus" @@ -22,11 +23,16 @@ type X11InputInjector struct { // 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 - display string + inputMu sync.Mutex + conn *xgb.Conn + root xproto.Window + screen *xproto.ScreenInfo + display string + // cookieHex is kept so the connection can be re-established the same way + // it was first made. + cookieHex string + // lastLiveCheck paces the liveness probe in ensureConnLocked. + lastLiveCheck time.Time keysymMap map[uint32]byte lastButtons uint16 clipboardTool string @@ -49,35 +55,19 @@ func NewX11InputInjector(display, cookieHex, authFile string) (*X11InputInjector return nil, fmt.Errorf("DISPLAY not set and no Xorg process found") } - var conn *xgb.Conn - var err error - if cookieHex != "" { - conn, err = dialXUnixWithCookie(display, cookieHex) - } else { - conn, err = xgb.NewConnDisplay(display) - } + conn, screen, err := dialX11Input(display, cookieHex) if err != nil { - return nil, fmt.Errorf("connect to X11 display %s: %w", display, err) + return nil, err } - if err := xtest.Init(conn); err != nil { - conn.Close() - return nil, fmt.Errorf("init XTest extension: %w", err) - } - - setup := xproto.Setup(conn) - if len(setup.Roots) == 0 { - conn.Close() - return nil, fmt.Errorf("no X11 screens") - } - screen := setup.Roots[0] - inj := &X11InputInjector{ - conn: conn, - root: screen.Root, - screen: &screen, - display: display, - authFile: authFile, + conn: conn, + root: screen.Root, + screen: &screen, + display: display, + cookieHex: cookieHex, + authFile: authFile, + lastLiveCheck: time.Now(), } inj.cacheKeyboardMapping() inj.resolveClipboardTool() @@ -86,10 +76,71 @@ func NewX11InputInjector(display, cookieHex, authFile string) (*X11InputInjector return inj, nil } +// x11InjectorLiveCheck is how often the injector confirms its X connection is +// still alive before injecting. A restarted X server leaves the old connection +// dead, and XTest requests on it fail silently, so without a probe input never +// comes back even after capture has recovered on its own. +const x11InjectorLiveCheck = 2 * time.Second + +// dialX11Input opens an X connection for input injection and initialises XTest +// on it, returning the first screen. +func dialX11Input(display, cookieHex string) (*xgb.Conn, xproto.ScreenInfo, error) { + 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, xproto.ScreenInfo{}, fmt.Errorf("connect to X11 display %s: %w", display, err) + } + + if err := xtest.Init(conn); err != nil { + conn.Close() + return nil, xproto.ScreenInfo{}, fmt.Errorf("init XTest extension: %w", err) + } + + setup := xproto.Setup(conn) + if len(setup.Roots) == 0 { + conn.Close() + return nil, xproto.ScreenInfo{}, fmt.Errorf("no X11 screens") + } + return conn, setup.Roots[0], nil +} + +// ensureConnLocked probes the X connection at most once per +// x11InjectorLiveCheck and reconnects when it has died, refreshing the screen +// and the keyboard mapping, both of which belong to the new server. A failed +// reconnect keeps the dead connection and tries again on the next interval. +// Caller must hold inputMu. +func (x *X11InputInjector) ensureConnLocked() { + if time.Since(x.lastLiveCheck) < x11InjectorLiveCheck { + return + } + x.lastLiveCheck = time.Now() + if _, err := xproto.GetInputFocus(x.conn).Reply(); err == nil { + return + } + + conn, screen, err := dialX11Input(x.display, x.cookieHex) + if err != nil { + log.Debugf("X11 input connection lost, reconnect to %s: %v", x.display, err) + return + } + x.conn.Close() + x.conn = conn + x.root = screen.Root + x.screen = &screen + x.cacheKeyboardMapping() + log.Infof("X11 input injector reconnected (display=%s)", x.display) +} + // 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.ensureConnLocked() x.injectKeyLocked(keysym, down) } @@ -111,6 +162,7 @@ func (x *X11InputInjector) injectKeyLocked(keysym uint32, down bool) { func (x *X11InputInjector) InjectKeyScancode(scancode, keysym uint32, down bool) { x.inputMu.Lock() defer x.inputMu.Unlock() + x.ensureConnLocked() linuxKey := qemuScancodeToLinuxKey(scancode) if linuxKey == 0 { @@ -147,6 +199,7 @@ func (x *X11InputInjector) InjectPointer(buttonMask uint16, px, py, serverW, ser // lastButtons and the write closes the sequence. x.inputMu.Lock() defer x.inputMu.Unlock() + x.ensureConnLocked() // Scale to actual screen coordinates. screenW := int(x.screen.WidthInPixels) @@ -262,6 +315,18 @@ func (x *X11InputInjector) SetClipboard(text string) { // are skipped: a paste workflow for them needs Wayland-aware text input // or layout introspection that this path does not implement. func (x *X11InputInjector) TypeText(text string) { + x.inputMu.Lock() + x.ensureConnLocked() + restoreCaps := x.clearCapsLockLocked() + x.inputMu.Unlock() + if restoreCaps { + defer func() { + x.inputMu.Lock() + defer x.inputMu.Unlock() + x.toggleCapsLockLocked() + }() + } + const maxChars = 4096 count := 0 for _, r := range text { @@ -303,6 +368,34 @@ func (x *X11InputInjector) typeRune(keysym uint32, shift bool) { } } +// xLockMask is the core-protocol modifier bit for Lock, which is Caps Lock on +// every standard keymap. +const xLockMask = 1 << 1 + +// clearCapsLockLocked turns Caps Lock off when it is on and reports whether it +// did, so the caller can put it back. The typed runes carry their case in the +// Shift framing, and with Caps Lock engaged the server inverts it: pasted text +// comes out with every letter's case flipped. Caller must hold inputMu. +func (x *X11InputInjector) clearCapsLockLocked() bool { + reply, err := xproto.QueryPointer(x.conn, x.root).Reply() + if err != nil || reply.Mask&xLockMask == 0 { + return false + } + return x.toggleCapsLockLocked() +} + +// toggleCapsLockLocked presses and releases Caps Lock, reporting whether the +// keymap has one to press. Caller must hold inputMu. +func (x *X11InputInjector) toggleCapsLockLocked() bool { + keycode := x.keysymToKeycode(0xffe5) // Caps_Lock + if keycode == 0 { + return false + } + 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) + return true +} + func (x *X11InputInjector) resolveClipboardTool() { for _, name := range []string{"xclip", "xsel"} { path, err := exec.LookPath(name)