Accept bare marker protocols, reject msb_right framebuffers, split the policy row conversion

client/server/server.go

@@ -2096,9 +2096,9 @@ func (s *Server) RespondApproval(ctx context.Context, msg *proto.RespondApproval
 	id, ok := ipcauth.CallerIdentity(ctx)
 	if !ok {
 		log.Warnf("refusing approval response for %s: the caller's identity cannot be verified on this control channel", msg.GetRequestId())
-		return nil, gstatus.Errorf(codes.PermissionDenied,
-			"answering a connection approval requires a control channel that carries the caller's identity. "+
-				"Reinstall the service on a socket that does: %s", reinstallCommand())
+		// Same envelope as the privileged-config refusals, so the CLI and the UI
+		// present the guidance instead of a raw gRPC error: see privilegeError.
+		return nil, privilegeError(unidentifiableCallerSummary(), reinstallCommand())
 	}
 	log.Infof("approval response for %s from caller %s: accept=%t view_only=%t",
 		msg.GetRequestId(), id, msg.GetAccept(), msg.GetViewOnly())

client/server/ssh_gate.go

@@ -233,6 +233,15 @@ func unidentifiedSummary(action string) string {
 		"Reinstall the service on a socket that carries the caller's identity.", capitalize(action), ipcauth.PrivilegedActor())
 }
 
+// unidentifiableCallerSummary covers an operation that needs to know who is
+// asking rather than a privileged caller, so unlike unidentifiedSummary it does
+// not name root: elevating would not help, only moving the daemon onto a socket
+// that carries the caller's identity.
+func unidentifiableCallerSummary() string {
+	return "Answering a connection approval requires a control channel that carries the caller's identity, " +
+		"and the daemon's current socket does not. Reinstall the service on one that does."
+}
+
 // reinstallCommand is the command that moves the daemon onto a socket whose
 // callers can be identified.
 func reinstallCommand() string {

client/vnc/server/capture_fb_linux.go

@@ -173,6 +173,14 @@ func validateFBLayout(bpp int, v *fbVarScreenInfo) error {
 			bpp, v.RedOffset, v.RedLen, v.GreenOffset, v.GreenLen, v.BlueOffset, v.BlueLen)
 	}
 
+	// msb_right marks a channel whose bits run the other way inside the pixel.
+	// Every swizzler reads them in normal order, so such a device would be
+	// decoded into mirrored channel values.
+	if v.RedMSBR != 0 || v.GreenMSBR != 0 || v.BlueMSBR != 0 {
+		return fmt.Errorf("unsupported %dbpp framebuffer layout: msb_right set (r=%d g=%d b=%d)",
+			bpp, v.RedMSBR, v.GreenMSBR, v.BlueMSBR)
+	}
+
 	switch bpp {
 	case 32:
 		// Offsets are honoured, channel widths are not.

integration_tests/management/network_map_db/policy_test.go

@@ -54,8 +54,8 @@ func TestGetPolicies(t *testing.T) {
 		        null,'{"ID":"domain-3","Type":"domain"}',null,null,null,null)`)
 
 	// VNC temporary-access rule: the session pubkey and display name are what
-	// the daemon's Noise_IK authorizer matches on, so they have to survive the
-	// components path as well as the legacy one.
+	// the daemon's Noise_IK authorizer matches on, so the components path has
+	// to carry them out of the database rather than dropping them.
 	execQuery(t, ctx,
 		`insert into policies (id, public_id, account_id, enabled, source_posture_checks)
 		 values('policy-5','policy-5-public','account-1',true,null)`)

management/internals/network_map_db/shared_types.go

@@ -370,111 +370,114 @@ func ConvertToNmdataPolicy(policies []Policy) ([]nmdata.Policy, map[string]map[s
 	policyToDestinationResourceIdx := make(map[string]map[string]any) // policy id to destination resource id
 	policyToDestinationGroupIdx := make(map[string]map[string]any)    // policy id to destination group id
 	for _, p := range policies {
-		policy := nmdata.Policy{}
-		err := FromSqlTypesToSharedTypes(
-			reflect.ValueOf(&p), reflect.ValueOf(&policy))
+		policy, err := convertPolicyRow(p, policyToDestinationResourceIdx, policyToDestinationGroupIdx)
 		if err != nil {
-			return nil, nil, nil, err
+			return toret, nil, nil, err
 		}
-
-		var policyRule *nmdata.PolicyRule
-		pr := func() *nmdata.PolicyRule {
-			if policyRule != nil {
-				return policyRule
-			}
-
-			policyRule = &nmdata.PolicyRule{}
-			return policyRule
-		}
-
-		if p.RuleEnabled.Valid {
-			pr().Enabled = p.RuleEnabled.Bool
-		}
-		if p.Action.Valid {
-			pr().Action = p.Action.String
-		}
-		if p.Protocol.Valid {
-			pr().Protocol = p.Protocol.String
-		}
-		if p.Bidirectional.Valid {
-			pr().Bidirectional = p.Bidirectional.Bool
-		}
-		if len(p.Sources) > 0 {
-			err := json.Unmarshal([]byte(p.Sources), &pr().Sources)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-		}
-		if len(p.Destinations) > 0 {
-			err := json.Unmarshal([]byte(p.Destinations), &pr().Destinations)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-
-			if p.RuleEnabled.Valid && p.RuleEnabled.Bool {
-				for _, dst := range pr().Destinations {
-					if _, ok := policyToDestinationGroupIdx[p.ID]; !ok {
-						policyToDestinationGroupIdx[p.ID] = make(map[string]any)
-					}
-					policyToDestinationGroupIdx[p.ID][dst] = struct{}{}
-				}
-			}
-		}
-		if len(p.SourceResource) > 0 {
-			err := json.Unmarshal([]byte(p.SourceResource), &pr().SourceResource)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-		}
-		if len(p.DestinationResource) > 0 {
-			err := json.Unmarshal([]byte(p.DestinationResource), &pr().DestinationResource)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-
-			if p.RuleEnabled.Valid && p.RuleEnabled.Bool {
-				if _, ok := policyToDestinationResourceIdx[p.ID]; !ok {
-					policyToDestinationResourceIdx[p.ID] = make(map[string]any)
-				}
-				policyToDestinationResourceIdx[p.ID][pr().DestinationResource.ID] = struct{}{}
-			}
-		}
-		if len(p.Ports) > 0 {
-			err := json.Unmarshal([]byte(p.Ports), &pr().Ports)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-		}
-		if len(p.PortRanges) > 0 {
-			err := json.Unmarshal([]byte(p.PortRanges), &pr().PortRanges)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-		}
-		if len(p.AuthorizedGroups) > 0 {
-			err := json.Unmarshal([]byte(p.AuthorizedGroups), &pr().AuthorizedGroups)
-			if err != nil {
-				return toret, nil, nil, err
-			}
-		}
-		if p.AuthorizedUser.Valid {
-			pr().AuthorizedUser = p.AuthorizedUser.String
-		}
-		if p.SessionPubKey.Valid {
-			pr().SessionPubKey = p.SessionPubKey.String
-		}
-		if p.SessionDisplayName.Valid {
-			pr().SessionDisplayName = p.SessionDisplayName.String
-		}
-
-		if policyRule != nil {
-			policyRule.ID = p.ID
-			policyRule.PolicyID = p.ID
-			policy.Rules = []*nmdata.PolicyRule{policyRule}
-		}
-
 		toret = append(toret, policy)
 	}
 
 	return toret, policyToDestinationResourceIdx, policyToDestinationGroupIdx, nil
 }
+
+// convertPolicyRow turns one joined policy/rule row into an nmdata.Policy,
+// recording the row's destination groups and resource in the two indexes.
+//
+// The join produces one row per rule, and a policy with no rules still has a
+// row with every rule column NULL, so the rule is only attached when at least
+// one of those columns carried a value.
+func convertPolicyRow(p Policy, resourceIdx, groupIdx map[string]map[string]any) (nmdata.Policy, error) {
+	policy := nmdata.Policy{}
+	if err := FromSqlTypesToSharedTypes(reflect.ValueOf(&p), reflect.ValueOf(&policy)); err != nil {
+		return nmdata.Policy{}, err
+	}
+
+	var policyRule *nmdata.PolicyRule
+	pr := func() *nmdata.PolicyRule {
+		if policyRule != nil {
+			return policyRule
+		}
+
+		policyRule = &nmdata.PolicyRule{}
+		return policyRule
+	}
+
+	if err := decodePolicyRuleColumns(p, pr, resourceIdx, groupIdx); err != nil {
+		return nmdata.Policy{}, err
+	}
+
+	if policyRule != nil {
+		policyRule.ID = p.ID
+		policyRule.PolicyID = p.ID
+		policy.Rules = []*nmdata.PolicyRule{policyRule}
+	}
+	return policy, nil
+}
+
+// decodePolicyRuleColumns reads the rule columns of a joined row onto the rule
+// pr allocates on first use, and records the row's enabled destinations in the
+// two indexes.
+func decodePolicyRuleColumns(p Policy, pr func() *nmdata.PolicyRule, resourceIdx, groupIdx map[string]map[string]any) error {
+	ruleEnabled := p.RuleEnabled.Valid && p.RuleEnabled.Bool
+
+	for _, col := range []struct {
+		raw    []byte
+		target func() any
+	}{
+		{p.Sources, func() any { return &pr().Sources }},
+		{p.Destinations, func() any { return &pr().Destinations }},
+		{p.SourceResource, func() any { return &pr().SourceResource }},
+		{p.DestinationResource, func() any { return &pr().DestinationResource }},
+		{p.Ports, func() any { return &pr().Ports }},
+		{p.PortRanges, func() any { return &pr().PortRanges }},
+		{p.AuthorizedGroups, func() any { return &pr().AuthorizedGroups }},
+	} {
+		if len(col.raw) == 0 {
+			continue
+		}
+		if err := json.Unmarshal(col.raw, col.target()); err != nil {
+			return err
+		}
+	}
+
+	for _, col := range []struct {
+		raw    sql.NullString
+		target func() *string
+	}{
+		{p.Action, func() *string { return &pr().Action }},
+		{p.Protocol, func() *string { return &pr().Protocol }},
+		{p.AuthorizedUser, func() *string { return &pr().AuthorizedUser }},
+		{p.SessionPubKey, func() *string { return &pr().SessionPubKey }},
+		{p.SessionDisplayName, func() *string { return &pr().SessionDisplayName }},
+	} {
+		if col.raw.Valid {
+			*col.target() = col.raw.String
+		}
+	}
+
+	if p.RuleEnabled.Valid {
+		pr().Enabled = p.RuleEnabled.Bool
+	}
+	if p.Bidirectional.Valid {
+		pr().Bidirectional = p.Bidirectional.Bool
+	}
+
+	if !ruleEnabled {
+		return nil
+	}
+	if len(p.Destinations) > 0 {
+		for _, dst := range pr().Destinations {
+			if _, ok := groupIdx[p.ID]; !ok {
+				groupIdx[p.ID] = make(map[string]any)
+			}
+			groupIdx[p.ID][dst] = struct{}{}
+		}
+	}
+	if len(p.DestinationResource) > 0 {
+		if _, ok := resourceIdx[p.ID]; !ok {
+			resourceIdx[p.ID] = make(map[string]any)
+		}
+		resourceIdx[p.ID][pr().DestinationResource.ID] = struct{}{}
+	}
+	return nil
+}

shared/management/types/policyrule.go

@@ -66,6 +66,15 @@ func ParseRuleString(rule string) (PolicyRuleProtocolType, RulePortRange, error)
 	if rule == "icmp" {
 		return PolicyRuleProtocolICMP, RulePortRange{}, nil
 	}
+	// The NetBird marker protocols carry their own port, so they are written
+	// bare, the way "all" and "icmp" are. That is what the temporary-access
+	// flow sends. The protocol/port spellings below stay accepted.
+	if rule == string(PolicyRuleProtocolNetbirdSSH) {
+		return PolicyRuleProtocolNetbirdSSH, RulePortRange{Start: nativeSSHPortNumber, End: nativeSSHPortNumber}, nil
+	}
+	if rule == string(PolicyRuleProtocolNetbirdVNC) {
+		return PolicyRuleProtocolNetbirdVNC, RulePortRange{Start: VNCInternalPort, End: VNCInternalPort}, nil
+	}
 
 	split := strings.Split(rule, "/")
 	if len(split) != 2 {

shared/management/types/policyrule_test.go

@@ -0,0 +1,66 @@
+package types
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseRuleString(t *testing.T) {
+	tests := []struct {
+		name      string
+		rule      string
+		protocol  PolicyRuleProtocolType
+		portRange RulePortRange
+		wantErr   bool
+	}{
+		{name: "all", rule: "all", protocol: PolicyRuleProtocolALL},
+		{name: "icmp", rule: "icmp", protocol: PolicyRuleProtocolICMP},
+		{name: "uppercase and padded", rule: "  ALL ", protocol: PolicyRuleProtocolALL},
+
+		// The marker protocols imply their own port, so the bare spelling is
+		// what the temporary-access flow sends.
+		{
+			name:      "bare netbird-ssh",
+			rule:      "netbird-ssh",
+			protocol:  PolicyRuleProtocolNetbirdSSH,
+			portRange: RulePortRange{Start: nativeSSHPortNumber, End: nativeSSHPortNumber},
+		},
+		{
+			name:      "bare netbird-vnc",
+			rule:      "netbird-vnc",
+			protocol:  PolicyRuleProtocolNetbirdVNC,
+			portRange: RulePortRange{Start: VNCInternalPort, End: VNCInternalPort},
+		},
+		{
+			name:      "netbird-vnc with an explicit port",
+			rule:      "netbird-vnc/25900",
+			protocol:  PolicyRuleProtocolNetbirdVNC,
+			portRange: RulePortRange{Start: VNCInternalPort, End: VNCInternalPort},
+		},
+
+		{name: "tcp port", rule: "tcp/443", protocol: PolicyRuleProtocolTCP, portRange: RulePortRange{Start: 443, End: 443}},
+		{name: "udp range", rule: "udp/5000-5010", protocol: PolicyRuleProtocolUDP, portRange: RulePortRange{Start: 5000, End: 5010}},
+
+		{name: "icmp rejects a port", rule: "icmp/8", wantErr: true},
+		{name: "unknown protocol", rule: "sctp/1", wantErr: true},
+		{name: "no port", rule: "tcp", wantErr: true},
+		{name: "empty port", rule: "tcp/", wantErr: true},
+		{name: "port out of range", rule: "tcp/70000", wantErr: true},
+		{name: "reversed range", rule: "tcp/500-400", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			protocol, portRange, err := ParseRuleString(tt.rule)
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.protocol, protocol)
+			assert.Equal(t, tt.portRange, portRange)
+		})
+	}
+}