diff --git a/management/internals/shared/grpc/components_encoder.go b/management/internals/shared/grpc/components_encoder.go index 91e0db4b9..e43e900c6 100644 --- a/management/internals/shared/grpc/components_encoder.go +++ b/management/internals/shared/grpc/components_encoder.go @@ -173,10 +173,9 @@ func (e *componentEncoder) appendPeer(p *nmdata.Peer) uint32 { return idx } -// indexRouterPeers ensures every router peer is in the peer dedup table -// (c.RouterPeers may contain peers not in c.Peers when validation rules drop -// them) and returns their wire indexes for the RouterPeerIndexes field. Must -// run before any encoder that resolves peer ids via e.peerOrder. +// indexRouterPeers ensures every router peer is in the peer dedup table and +// returns their wire indexes for the RouterPeerIndexes field. Must run before +// any encoder that resolves peer ids via e.peerOrder. func (e *componentEncoder) indexRouterPeers(routers map[string]*nmdata.Peer) []uint32 { if len(routers) == 0 { return nil @@ -333,16 +332,15 @@ func unionPolicies(policies []*nmdata.Policy, resourcePolicies map[string][]*nmd } // encodeAuthorizedGroups translates rule.AuthorizedGroups (map keyed by -// group xid → local-user names) to the wire form (map keyed by group -// account_seq_id → UserNameList). Groups without a seq id are dropped — -// matches how source/destination group references handle the same case. +// group xid → local-user names) to the wire form (map keyed by +// authorizedGroupKey → UserNameList). func (e *componentEncoder) encodeAuthorizedGroups(m map[string][]string) map[string]*proto.UserNameList { if len(m) == 0 { return nil } out := make(map[string]*proto.UserNameList, len(m)) for groupID, names := range m { - id, ok := e.groupPublicXid(groupID) + id, ok := e.authorizedGroupKey(groupID) if !ok { continue } @@ -351,6 +349,24 @@ func (e *componentEncoder) encodeAuthorizedGroups(m map[string][]string) map[str return out } +// authorizedGroupKey resolves the wire key for a group that grants SSH access. +// These are user groups: they hold no peers, so nothing puts them in +// components.Groups and groupPublicXid cannot see them. Dropping them the way a +// missing source/destination group is dropped would strip every authorized user +// from the envelope while PeerConfig still reports SSH enabled, leaving the peer +// running sshd with nobody able to log in — so the id is passed through instead. +// AuthorizedGroups and GroupIDToUserIDs are only ever used against each other, +// on both sides of the wire, so they just have to agree. +func (e *componentEncoder) authorizedGroupKey(groupID string) (string, bool) { + if groupID == "" { + return "", false + } + if id, ok := e.groupPublicXid(groupID); ok { + return id, true + } + return groupID, true +} + func (e *componentEncoder) groupPublicXid(groupID string) (string, bool) { g, ok := e.components.Groups[groupID] if !ok { @@ -650,7 +666,7 @@ func (e *componentEncoder) encodeGroupIDToUserIDs(m map[string][]string) map[str } out := make(map[string]*proto.UserIDList, len(m)) for groupID, userIDs := range m { - id, ok := e.groupPublicXid(groupID) + id, ok := e.authorizedGroupKey(groupID) if !ok || len(userIDs) == 0 { continue } diff --git a/management/internals/shared/grpc/components_encoder_test.go b/management/internals/shared/grpc/components_encoder_test.go index aeced7055..8698e4835 100644 --- a/management/internals/shared/grpc/components_encoder_test.go +++ b/management/internals/shared/grpc/components_encoder_test.go @@ -697,15 +697,20 @@ func TestEncodeNetworkMapEnvelope_RouterPeerNotInComponentsPeers(t *testing.T) { func TestEncodeNetworkMapEnvelope_GroupIDToUserIDs(t *testing.T) { c := newTestComponents() c.GroupIDToUserIDs = map[string][]string{ - "group-src": {"user-1", "user-2"}, - "group-missing": {"user-4"}, // group not in components → drop + "group-src": {"user-1", "user-2"}, + "group-users": {"user-4"}, } full := EncodeNetworkMapEnvelope(ComponentsEnvelopeInput{Components: c}).GetFull() - require.Len(t, full.GroupIdToUserIds, 1, "only present groups survive") + require.Len(t, full.GroupIdToUserIds, 2, + "a peer group is keyed by its public id, and a user group — which never appears in "+ + "components.Groups — keeps its own id rather than being dropped, or the peer would "+ + "receive no authorized SSH users at all") require.Contains(t, full.GroupIdToUserIds, "1") assert.ElementsMatch(t, []string{"user-1", "user-2"}, full.GroupIdToUserIds["1"].UserIds) + require.Contains(t, full.GroupIdToUserIds, "group-users") + assert.ElementsMatch(t, []string{"user-4"}, full.GroupIdToUserIds["group-users"].UserIds) } func TestToProxyPatch_EmptyInputReturnsNil(t *testing.T) { diff --git a/shared/management/networkmap/networkmapcompute.go b/shared/management/networkmap/networkmapcompute.go index 0701da710..067010385 100644 --- a/shared/management/networkmap/networkmapcompute.go +++ b/shared/management/networkmap/networkmapcompute.go @@ -105,7 +105,7 @@ func (nmd *NetworkMapData) GetPeerNetworkMapComponents(peerID string, peersCusto } for _, policy := range policies { - if policy == nil || len(policy.Rules) == 0 || policy.Rules[0] == nil { + if policy == nil || !policy.Enabled || len(policy.Rules) == 0 || policy.Rules[0] == nil { continue } if addSourcePeers { @@ -147,7 +147,7 @@ func (nmd *NetworkMapData) GetPeerNetworkMapComponents(peerID string, peersCusto } for _, rule := range policy.Rules { - if rule == nil { + if rule == nil || !rule.Enabled { continue } for _, srcGroupID := range rule.Sources { @@ -171,15 +171,21 @@ func (nmd *NetworkMapData) GetPeerNetworkMapComponents(peerID string, peersCusto if addSourcePeers { components.RoutersMap[resource.NetworkID] = networkRoutingPeers for peerIDKey := range networkRoutingPeers { - if p := nmd.Peers[peerIDKey]; p != nil { - if _, exists := components.RouterPeers[peerIDKey]; !exists { - components.RouterPeers[peerIDKey] = p - } - if _, exists := components.Peers[peerIDKey]; !exists { - if _, validated := nmd.ValidatedPeers[peerIDKey]; validated { - components.Peers[peerIDKey] = p - } - } + p := nmd.Peers[peerIDKey] + if p == nil { + continue + } + // An unapproved peer must not carry traffic, so it is kept out of + // RouterPeers as well: the envelope encoder indexes that map into + // the wire peer table, from which the client restores every entry. + if _, validated := nmd.ValidatedPeers[peerIDKey]; !validated { + continue + } + if _, exists := components.RouterPeers[peerIDKey]; !exists { + components.RouterPeers[peerIDKey] = p + } + if _, exists := components.Peers[peerIDKey]; !exists { + components.Peers[peerIDKey] = p } } components.NetworkResources = append(components.NetworkResources, resource) diff --git a/shared/management/networkmap/networkmapcompute_test.go b/shared/management/networkmap/networkmapcompute_test.go index 0daae81dd..8c9add8c1 100644 --- a/shared/management/networkmap/networkmapcompute_test.go +++ b/shared/management/networkmap/networkmapcompute_test.go @@ -1103,8 +1103,9 @@ func TestGetPeerNetworkMapComponents_NetworkResources_SourceSide(t *testing.T) { assert.Equal(t, []*nmdata.NetworkResource{res}, c.NetworkResources) assert.Equal(t, map[string][]*nmdata.Policy{"res-1": {rp}}, c.ResourcePoliciesMap) assert.Equal(t, map[string]map[string]*nmdata.NetworkRouter{"net-1": routers}, c.RoutersMap) - assert.ElementsMatch(t, []string{routerOK.ID, routerUnval.ID}, peerIDSet(c.RouterPeers), - "RouterPeers carries all routing peers regardless of validation") + assert.ElementsMatch(t, []string{routerOK.ID}, peerIDSet(c.RouterPeers), + "an unvalidated routing peer is withheld from RouterPeers too, since the envelope encoder "+ + "indexes that map into the wire peer table and the client restores every entry from it") assert.ElementsMatch(t, []string{targetID, routerOK.ID}, peerIDSet(c.Peers), "only validated routing peers are connected") assert.ElementsMatch(t, []string{"g-clients", "g-resource"}, groupIDSet(c.Groups)) diff --git a/shared/management/types/networkmap_components.go b/shared/management/types/networkmap_components.go index 1339fbbdf..d008ece83 100644 --- a/shared/management/types/networkmap_components.go +++ b/shared/management/types/networkmap_components.go @@ -819,7 +819,7 @@ func (c *NetworkMapComponents) processResourcePolicies( var routes []*nmdata.Route for _, policy := range c.ResourcePoliciesMap[resource.ID] { - if policy == nil || len(policy.Rules) == 0 || policy.Rules[0] == nil { + if policy == nil || !policy.Enabled || len(policy.Rules) == 0 || policy.Rules[0] == nil { continue } peers := c.getResourcePolicyPeers(policy)