From 432d249cdeb1f96195579dde393f99dd8994f104 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Sat, 29 Aug 2026 08:24:36 +0200 Subject: [PATCH] Accept bare marker protocols, reject msb_right framebuffers, split the policy row conversion --- client/server/server.go | 6 +- client/server/ssh_gate.go | 9 + client/vnc/server/capture_fb_linux.go | 8 + .../management/network_map_db/policy_test.go | 4 +- .../internals/network_map_db/shared_types.go | 205 +++++++++--------- shared/management/types/policyrule.go | 9 + shared/management/types/policyrule_test.go | 66 ++++++ 7 files changed, 201 insertions(+), 106 deletions(-) create mode 100644 shared/management/types/policyrule_test.go diff --git a/client/server/server.go b/client/server/server.go index 9d1f9bec3..0649d988c 100644 --- a/client/server/server.go +++ b/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()) diff --git a/client/server/ssh_gate.go b/client/server/ssh_gate.go index bbc920f52..0a970b051 100644 --- a/client/server/ssh_gate.go +++ b/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 { diff --git a/client/vnc/server/capture_fb_linux.go b/client/vnc/server/capture_fb_linux.go index 7fe6f3193..227100905 100644 --- a/client/vnc/server/capture_fb_linux.go +++ b/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. diff --git a/integration_tests/management/network_map_db/policy_test.go b/integration_tests/management/network_map_db/policy_test.go index 9daba39c1..976e7f931 100644 --- a/integration_tests/management/network_map_db/policy_test.go +++ b/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)`) diff --git a/management/internals/network_map_db/shared_types.go b/management/internals/network_map_db/shared_types.go index f8cffed50..764e50336 100644 --- a/management/internals/network_map_db/shared_types.go +++ b/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 +} diff --git a/shared/management/types/policyrule.go b/shared/management/types/policyrule.go index 3d2226001..1e0a2d64a 100644 --- a/shared/management/types/policyrule.go +++ b/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 { diff --git a/shared/management/types/policyrule_test.go b/shared/management/types/policyrule_test.go new file mode 100644 index 000000000..9dcc08622 --- /dev/null +++ b/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) + }) + } +}