Collect auth requirements for bidirectional source peers and fix follow-up review findings

This commit is contained in:
Viktor Liu
2026-08-29 09:57:07 +02:00
parent 1c10c19d45
commit ec320278e3
5 changed files with 60 additions and 29 deletions
+1 -1
View File
@@ -21,7 +21,7 @@ var (
func init() { func init() {
vncAgentCmd.Flags().StringVar(&vncAgentSocket, "socket", "", "Unix-domain socket path the agent listens on (required)") 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) rootCmd.AddCommand(vncAgentCmd)
} }
+6 -3
View File
@@ -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") return nil, 0, 0, 0, fmt.Errorf("cursor has zero extent")
} }
// Anything past maxCursorDim is discarded by the encoder, so decoding it // Anything past maxCursorDim is discarded by the encoder, so decoding it
// would allocate and convert a sprite that can only be thrown away. Keep // would allocate and convert a sprite that can only be thrown away. Report
// showing the last good cursor instead. // 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 w > maxCursorDim || h > maxCursorDim {
if c.lastImg != nil { if c.lastImg != nil {
return c.lastImg, c.lastHotX, c.lastHotY, c.lastSerial, 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 len(reply.CursorImage) < w*h {
if c.lastImg != nil { if c.lastImg != nil {
+6 -6
View File
@@ -273,15 +273,15 @@ func (x *X11InputInjector) TypeText(text string) {
if !ok { if !ok {
continue continue
} }
x.typeRuneLocked(keysym, shift) x.typeRune(keysym, shift)
} }
} }
// typeRuneLocked emits one rune, framed by Shift-down/up when the keysym needs // typeRune emits one rune, framed by Shift-down/up when the keysym needs it. It
// it. Locked per rune rather than for the whole string: the framing has to be // takes inputMu itself, per rune rather than for the whole string: the framing
// atomic, but a long paste must not hold another session's pointer off for the // has to be atomic, but a long paste must not hold another session's pointer
// length of it. // off for the length of it.
func (x *X11InputInjector) typeRuneLocked(keysym uint32, shift bool) { func (x *X11InputInjector) typeRune(keysym uint32, shift bool) {
x.inputMu.Lock() x.inputMu.Lock()
defer x.inputMu.Unlock() defer x.inputMu.Unlock()
@@ -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 // Collected for whichever side the resolver will actually authorize:
// from the components, and the rule then reaches the resolver // a bidirectional rule grants access in both directions, so a peer
// with nobody authorized. // that appears only in Sources is authorized too. Gating this on
if rule.Protocol == string(types.PolicyRuleProtocolNetbirdSSH) || // peerInDestinations alone leaves that peer's rule reaching the
rule.Protocol == string(types.PolicyRuleProtocolNetbirdVNC) { // resolver with none of the inputs it needs to name a user.
switch { //
case len(rule.AuthorizedGroups) > 0: // Both marker protocols resolve users the same way, so VNC needs
for groupID := range rule.AuthorizedGroups { // exactly what SSH needs.
authReqs.neededGroupIDs[groupID] = struct{}{} receivingPeer := peerInDestinations || (rule.Bidirectional && peerInSources)
} if !receivingPeer {
case rule.AuthorizedUser != "": continue
// Carries its own user; no lookup inputs needed. }
default: if rule.Protocol == string(types.PolicyRuleProtocolNetbirdSSH) ||
authReqs.needAllowedUserIDs = true 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 authReqs.needAllowedUserIDs = true
} }
} else if nmdata.PolicyRuleImpliesLegacySSH(rule) && peerSSHEnabled {
authReqs.needAllowedUserIDs = true
} }
} }
if policyRelevant { if policyRelevant {
@@ -961,12 +961,23 @@ func TestGetPeerNetworkMapComponents_SSHRequirements(t *testing.T) {
mutateRule: func(r *nmdata.PolicyRule) { r.Ports = []string{"443"} }, mutateRule: func(r *nmdata.PolicyRule) { r.Ports = []string{"443"} },
sshEnabled: true, 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) { mutateRule: func(r *nmdata.PolicyRule) {
r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdSSH) r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdSSH)
}, },
targetInSrc: true, 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 // 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) { mutateRule: func(r *nmdata.PolicyRule) {
r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC) r.Protocol = string(nbtypes.PolicyRuleProtocolNetbirdVNC)
}, },
targetInSrc: true, 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,
}, },
} }