diff --git a/client/cmd/vnc_agent.go b/client/cmd/vnc_agent.go index 298ddaae8..905870f58 100644 --- a/client/cmd/vnc_agent.go +++ b/client/cmd/vnc_agent.go @@ -21,7 +21,7 @@ var ( func init() { vncAgentCmd.Flags().StringVar(&vncAgentSocket, "socket", "", "Unix-domain socket path the agent listens on (required)") - vncAgentCmd.Flags().Uint32Var(&vncAgentTargetUID, "target-uid", 0, "uid the agent should drop privileges to before listening (darwin only; 0 = stay as current uid)") + vncAgentCmd.Flags().Uint32Var(&vncAgentTargetUID, "target-uid", 0, "uid the agent drops privileges to before listening (darwin only; required there, and must not be 0)") rootCmd.AddCommand(vncAgentCmd) } diff --git a/client/vnc/server/cursor_x11.go b/client/vnc/server/cursor_x11.go index 8ceb71c99..81964bb5c 100644 --- a/client/vnc/server/cursor_x11.go +++ b/client/vnc/server/cursor_x11.go @@ -69,13 +69,16 @@ func (c *xfixesCursor) Cursor() (*image.RGBA, int, int, uint64, error) { return nil, 0, 0, 0, fmt.Errorf("cursor has zero extent") } // Anything past maxCursorDim is discarded by the encoder, so decoding it - // would allocate and convert a sprite that can only be thrown away. Keep - // showing the last good cursor instead. + // would allocate and convert a sprite that can only be thrown away. Report + // it as "nothing new" rather than an error: the caller latches a cursor + // error for the rest of the session, and an oversized sprite is a property + // of the current cursor, not of the source. The next ordinary cursor is + // still delivered. if w > maxCursorDim || h > maxCursorDim { if c.lastImg != nil { return c.lastImg, c.lastHotX, c.lastHotY, c.lastSerial, nil } - return nil, 0, 0, 0, fmt.Errorf("cursor %dx%d exceeds %d", w, h, maxCursorDim) + return nil, 0, 0, 0, nil } if len(reply.CursorImage) < w*h { if c.lastImg != nil { diff --git a/client/vnc/server/input_x11.go b/client/vnc/server/input_x11.go index 60a703105..a01bc46ce 100644 --- a/client/vnc/server/input_x11.go +++ b/client/vnc/server/input_x11.go @@ -273,15 +273,15 @@ func (x *X11InputInjector) TypeText(text string) { if !ok { continue } - x.typeRuneLocked(keysym, shift) + x.typeRune(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) { +// typeRune emits one rune, framed by Shift-down/up when the keysym needs it. It +// takes inputMu itself, 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) typeRune(keysym uint32, shift bool) { x.inputMu.Lock() defer x.inputMu.Unlock() diff --git a/shared/management/networkmap/networkmapcompute.go b/shared/management/networkmap/networkmapcompute.go index 602937685..66aa71de2 100644 --- a/shared/management/networkmap/networkmapcompute.go +++ b/shared/management/networkmap/networkmapcompute.go @@ -362,26 +362,34 @@ func (nmd *NetworkMapData) getPeersGroupsPoliciesRoutes( } } - // 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 { - authReqs.neededGroupIDs[groupID] = struct{}{} - } - case rule.AuthorizedUser != "": - // Carries its own user; no lookup inputs needed. - default: - authReqs.needAllowedUserIDs = true + } + + // Collected for whichever side the resolver will actually authorize: + // a bidirectional rule grants access in both directions, so a peer + // that appears only in Sources is authorized too. Gating this on + // peerInDestinations alone leaves that peer's rule reaching the + // resolver with none of the inputs it needs to name a user. + // + // Both marker protocols resolve users the same way, so VNC needs + // exactly what SSH needs. + receivingPeer := peerInDestinations || (rule.Bidirectional && peerInSources) + if !receivingPeer { + continue + } + if rule.Protocol == string(types.PolicyRuleProtocolNetbirdSSH) || + rule.Protocol == string(types.PolicyRuleProtocolNetbirdVNC) { + switch { + case len(rule.AuthorizedGroups) > 0: + for groupID := range rule.AuthorizedGroups { + authReqs.neededGroupIDs[groupID] = struct{}{} } - } else if nmdata.PolicyRuleImpliesLegacySSH(rule) && peerSSHEnabled { + case rule.AuthorizedUser != "": + // Carries its own user; no lookup inputs needed. + default: authReqs.needAllowedUserIDs = true } + } else if nmdata.PolicyRuleImpliesLegacySSH(rule) && peerSSHEnabled { + authReqs.needAllowedUserIDs = true } } if policyRelevant { diff --git a/shared/management/networkmap/networkmapcompute_test.go b/shared/management/networkmap/networkmapcompute_test.go index c0abfe25a..85eeaa8d2 100644 --- a/shared/management/networkmap/networkmapcompute_test.go +++ b/shared/management/networkmap/networkmapcompute_test.go @@ -961,12 +961,23 @@ func TestGetPeerNetworkMapComponents_SSHRequirements(t *testing.T) { mutateRule: func(r *nmdata.PolicyRule) { r.Ports = []string{"443"} }, sshEnabled: true, }, + // A bidirectional rule grants access both ways, so the peer is + // authorized from the sources side too and needs the same inputs. { - name: "netbird-ssh only counts on the destination side", + name: "netbird-ssh on the source side of a bidirectional rule", mutateRule: func(r *nmdata.PolicyRule) { r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdSSH) }, targetInSrc: true, + wantAllowed: true, + }, + { + name: "netbird-ssh on the source side of a one-way rule", + mutateRule: func(r *nmdata.PolicyRule) { + r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdSSH) + r.Bidirectional = false + }, + targetInSrc: true, }, // VNC resolves authorized users exactly the way SSH does, so it needs @@ -995,11 +1006,20 @@ func TestGetPeerNetworkMapComponents_SSHRequirements(t *testing.T) { }, }, { - name: "netbird-vnc only counts on the destination side", + name: "netbird-vnc on the source side of a bidirectional rule", mutateRule: func(r *nmdata.PolicyRule) { r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC) }, targetInSrc: true, + wantAllowed: true, + }, + { + name: "netbird-vnc on the source side of a one-way rule", + mutateRule: func(r *nmdata.PolicyRule) { + r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC) + r.Bidirectional = false + }, + targetInSrc: true, }, }