sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-10-26 22:39:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

[management] fix posture check evaluation for direct peers in policy definition (#7348)

Browse Source
This commit is contained in:
Pascal Fischer
2026-08-28 16:46:42 +02:00
committed by GitHub
parent 611a9291cd
commit 353251d886
19 changed files with 695 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
management/server/affected_peers_router_paths_test.go
+20 -4
View File
@@ -146,7 +146,7 @@ func TestAffectedPeers_GroupAddResource_RefreshesRoutingPeer(t *testing.T) {
assert.NotContains(t, affected, s.unrelatedPeerID, "unrelated peer must not be affected")
}
func (s *routerScenario) createPostureCheckGatedPolicy(t *testing.T, ctx context.Context) string {
func (s *routerScenario) createPostureCheckGatedPolicy(t *testing.T, ctx context.Context, policy *types.Policy) string {
t.Helper()
check, err := s.manager.SavePostureChecks(ctx, s.accountID, userID, &posture.Checks{
@@ -157,7 +157,6 @@ func (s *routerScenario) createPostureCheckGatedPolicy(t *testing.T, ctx context
}, true)
require.NoError(t, err)
policy := peerToResourcePolicyByGroup(s.sourceGroupID, s.resourceGroupID)
policy.SourcePostureChecks = []string{check.ID}
_, err = s.manager.SavePolicy(ctx, s.accountID, userID, policy, true)
require.NoError(t, err)
@@ -169,7 +168,7 @@ func TestAffectedPeers_E2E_SavePostureCheck_RefreshesRoutingPeer(t *testing.T) {
s := setupRouterScenario(t, true)
ctx := context.Background()
checkID := s.createPostureCheckGatedPolicy(t, ctx)
checkID := s.createPostureCheckGatedPolicy(t, ctx, peerToResourcePolicyByGroup(s.sourceGroupID, s.resourceGroupID))
srcCh := s.updateManager.CreateChannel(ctx, s.sourcePeerID)
routerCh := s.updateManager.CreateChannel(ctx, s.routerPeerID)
@@ -347,11 +346,28 @@ func TestAffectedPeers_PeerChange_RouterInOtherNetworkNotAffected(t *testing.T)
// shortcut (the denied peer's map holds no router) and the allow direction
// depends on which meta field moved, leaving the routers with a stale map.
func TestAffectedPeers_E2E_PostureFlip_RefreshesRoutingPeer(t *testing.T) {
runPostureFlipRefreshesRoutingPeer(t, func(s *routerScenario) *types.Policy {
return peerToResourcePolicyByGroup(s.sourceGroupID, s.resourceGroupID)
})
}
// TestAffectedPeers_E2E_PostureFlip_DirectSourcePeer_RefreshesRoutingPeer is the same
// scenario with the source peer named directly in the rule: it must receive its posture
// checks and have its flips detected exactly like a group member.
func TestAffectedPeers_E2E_PostureFlip_DirectSourcePeer_RefreshesRoutingPeer(t *testing.T) {
runPostureFlipRefreshesRoutingPeer(t, func(s *routerScenario) *types.Policy {
return peerToResourcePolicyByPeer(s.sourcePeerID, s.resourceGroupID)
})
}
func runPostureFlipRefreshesRoutingPeer(t *testing.T, policyFor func(s *routerScenario) *types.Policy) {
t.Helper()
manager, updateManager := createManagerWithNetworkMapStore(t)
s := buildRouterScenario(t, manager, updateManager, true)
ctx := context.Background()
s.createPostureCheckGatedPolicy(t, ctx)
s.createPostureCheckGatedPolicy(t, ctx, policyFor(s))
source, err := s.manager.Store.GetPeerByID(ctx, store.LockingStrengthNone, s.accountID, s.sourcePeerID)
require.NoError(t, err)
management/server/affected_peers_router_test.go
+17
View File
@@ -173,6 +173,23 @@ func peerToResourcePolicyByGroup(sourceGroupID, resourceGroupID string) *types.P
}
}
// peerToResourcePolicyByPeer builds a policy naming the source peer directly via
// SourceResource rather than through a group.
func peerToResourcePolicyByPeer(sourcePeerID, resourceGroupID string) *types.Policy {
return &types.Policy{
Enabled: true,
Name: "peer-to-resource-by-peer",
Rules: []*types.PolicyRule{
{
Enabled: true,
SourceResource: types.Resource{ID: sourcePeerID, Type: types.ResourceTypePeer},
Destinations: []string{resourceGroupID},
Action: types.PolicyTrafficActionAccept,
},
},
}
}
// peerToResourcePolicyByResource builds a policy referencing the resource
// directly via DestinationResource rather than its group.
func peerToResourcePolicyByResource(sourceGroupID, resourceID string) *types.Policy {
management/server/peer.go
+9 -5
View File
@@ -1349,7 +1349,7 @@ func getPeerLoginInfo(ctx context.Context, transaction store.Store, accountID st
return nil, nil, false, err
}
postureChecks, err := getPeerPostureChecks(ctx, transaction, accountID, peerGroupIDs, policies)
postureChecks, err := getPeerPostureChecks(ctx, transaction, accountID, peer.ID, peerGroupIDs, policies)
if err != nil {
return nil, nil, false, err
}
@@ -1371,7 +1371,7 @@ func isPeerSSHEnabled(ctx context.Context, peer *nbpeer.Peer, policies []*types.
}
// getPeerPostureChecks returns the posture checks for the peer.
func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountID string, peerGroupIDs []string, policies []*types.Policy) ([]*nmdata.PostureChecks, error) {
func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountID, peerID string, peerGroupIDs []string, policies []*types.Policy) ([]*nmdata.PostureChecks, error) {
if len(policies) == 0 {
return nil, nil
}
@@ -1383,7 +1383,7 @@ func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountI
continue
}
postureChecksIDs := processPeerPostureChecks(policy, peerGroupIDs)
postureChecksIDs := processPeerPostureChecks(policy, peerID, peerGroupIDs)
peerPostureChecksIDs = append(peerPostureChecksIDs, postureChecksIDs...)
}
@@ -1395,13 +1395,17 @@ func getPeerPostureChecks(ctx context.Context, transaction store.Store, accountI
return types.TwinPostureChecksList(maps.Values(peerPostureChecks)), nil
}
// processPeerPostureChecks checks if the peer is in the source group of the policy and returns the posture checks.
func processPeerPostureChecks(policy *types.Policy, peerGroupIDs []string) []string {
// processPeerPostureChecks returns the policy's posture checks when the peer is a source of the policy, directly or through a source group.
func processPeerPostureChecks(policy *types.Policy, peerID string, peerGroupIDs []string) []string {
for _, rule := range policy.Rules {
if !rule.Enabled {
continue
}
if rule.SourceResource.Type == types.ResourceTypePeer && rule.SourceResource.ID == peerID {
return policy.SourcePostureChecks
}
for _, sourceGroup := range rule.Sources {
if slices.Contains(peerGroupIDs, sourceGroup) {
return policy.SourcePostureChecks
management/server/peer_posture_test.go
+20
View File
@@ -9,6 +9,7 @@ import (
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/posture"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
)
@@ -181,3 +182,22 @@ func TestMetaDiffAffectsPosture_NoChecks(t *testing.T) {
)
assert.False(t, metaDiffAffectsPosture(diff, nil))
}
func TestProcessPeerPostureChecks(t *testing.T) {
policy := &types.Policy{
Enabled: true,
SourcePostureChecks: []string{"pc1"},
Rules: []*types.PolicyRule{
{Enabled: false, Sources: []string{"g-disabled"}, SourceResource: types.Resource{ID: "peer-disabled", Type: types.ResourceTypePeer}},
{Enabled: true, Sources: []string{"g-src"}, Destinations: []string{"g-dst"}},
{Enabled: true, SourceResource: types.Resource{ID: "peer-direct", Type: types.ResourceTypePeer}, Destinations: []string{"g-dst"}},
{Enabled: true, SourceResource: types.Resource{ID: "peer-as-host", Type: types.ResourceTypeHost}, Destinations: []string{"g-dst"}},
},
}
assert.Equal(t, []string{"pc1"}, processPeerPostureChecks(policy, "peer-in-group", []string{"g-src"}), "source group member")
assert.Equal(t, []string{"pc1"}, processPeerPostureChecks(policy, "peer-direct", nil), "direct source peer")
assert.Empty(t, processPeerPostureChecks(policy, "peer-elsewhere", []string{"g-dst"}), "destination-only peer")
assert.Empty(t, processPeerPostureChecks(policy, "peer-disabled", []string{"g-disabled"}), "disabled rule")
assert.Empty(t, processPeerPostureChecks(policy, "peer-as-host", nil), "source resource of a non-peer type")
}
management/server/types/account.go
+12 -16
View File
@@ -909,13 +909,13 @@ func (a *Account) GetPeerConnectionResources(ctx context.Context, peer *nbpeer.P
var peerInSources, peerInDestinations bool
if rule.SourceResource.Type == ResourceTypePeer && rule.SourceResource.ID != "" {
sourcePeers, peerInSources = a.getPeerFromResource(rule.SourceResource, peer.ID)
sourcePeers, peerInSources = a.getPeerFromResource(ctx, rule.SourceResource, peer.ID, policy.SourcePostureChecks, validatedPeersMap)
} else {
sourcePeers, peerInSources = a.getAllPeersFromGroups(ctx, rule.Sources, peer.ID, policy.SourcePostureChecks, validatedPeersMap)
}
if rule.DestinationResource.Type == ResourceTypePeer && rule.DestinationResource.ID != "" {
destinationPeers, peerInDestinations = a.getPeerFromResource(rule.DestinationResource, peer.ID)
destinationPeers, peerInDestinations = a.getPeerFromResource(ctx, rule.DestinationResource, peer.ID, nil, validatedPeersMap)
} else {
destinationPeers, peerInDestinations = a.getAllPeersFromGroups(ctx, rule.Destinations, peer.ID, nil, validatedPeersMap)
}
@@ -1120,8 +1120,17 @@ func ruleHasDestination(rule *PolicyRule, peerID string, peerGroupIDs map[string
// Important: Posture checks are applicable only to source group peers,
// for destination group peers, call this method with an empty list of sourcePostureChecksIDs
func (a *Account) getAllPeersFromGroups(ctx context.Context, groups []string, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) {
return a.filterPolicyPeers(ctx, a.getUniquePeerIDsFromGroupsIDs(ctx, groups), peerID, sourcePostureChecksIDs, validatedPeersMap)
}
// getPeerFromResource resolves a rule side that names a peer directly, admitting it
// like a member of a group holding only that peer.
func (a *Account) getPeerFromResource(ctx context.Context, resource Resource, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) {
return a.filterPolicyPeers(ctx, []string{resource.ID}, peerID, sourcePostureChecksIDs, validatedPeersMap)
}
func (a *Account) filterPolicyPeers(ctx context.Context, uniquePeerIDs []string, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) {
peerInGroups := false
uniquePeerIDs := a.getUniquePeerIDsFromGroupsIDs(ctx, groups)
filteredPeers := make([]*nbpeer.Peer, 0, len(uniquePeerIDs))
for _, p := range uniquePeerIDs {
peer, ok := a.Peers[p]
@@ -1150,19 +1159,6 @@ func (a *Account) getAllPeersFromGroups(ctx context.Context, groups []string, pe
return filteredPeers, peerInGroups
}
func (a *Account) getPeerFromResource(resource Resource, peerID string) ([]*nbpeer.Peer, bool) {
peer := a.GetPeer(resource.ID)
if peer == nil {
return []*nbpeer.Peer{}, false
}
if peer.ID == peerID {
return []*nbpeer.Peer{}, true
}
return []*nbpeer.Peer{peer}, false
}
// validatePostureChecksOnPeer validates the posture checks on a peer
func (a *Account) validatePostureChecksOnPeer(ctx context.Context, sourcePostureChecksID []string, peerID string) bool {
peer, ok := a.Peers[peerID]
management/server/types/networkmap_components_correctness_test.go
+83
View File
@@ -875,6 +875,89 @@ func TestComponents_PeerAsSourceResource(t *testing.T) {
assert.True(t, has443, "peer-0 as source resource should have port 443 rule")
}
func hasFirewallRuleTo(nm *types.NetworkMap, peerIP, port string) bool {
for _, rule := range nm.FirewallRules {
if rule.PeerIP == peerIP && rule.Port == port {
return true
}
}
return false
}
// TestComponents_PeerAsSourceResource_PostureChecks verifies that a directly referenced
// source peer is gated by the policy's posture checks like a member of a group holding only
// that peer: peer-1 (0.25.0) fails the 0.26.0 minimum, peer-2 (0.40.0) passes.
func TestComponents_PeerAsSourceResource_PostureChecks(t *testing.T) {
account, validatedPeers := scalableTestAccountWithoutDefaultPolicy(20, 2)
for _, sourcePeerID := range []string{"peer-1", "peer-2"} {
account.Policies = append(account.Policies, &types.Policy{
ID: "policy-peer-src-" + sourcePeerID, Name: "Peer Source " + sourcePeerID, Enabled: true, AccountID: "test-account",
SourcePostureChecks: []string{"posture-check-ver"},
Rules: []*types.PolicyRule{{
ID: "rule-peer-src-" + sourcePeerID, Enabled: true,
Action: types.PolicyTrafficActionAccept,
Protocol: types.PolicyRuleProtocolTCP,
Bidirectional: true,
Ports: []string{"9443"},
SourceResource: types.Resource{ID: sourcePeerID, Type: types.ResourceTypePeer},
Destinations: []string{"group-0"},
}},
})
}
nm0 := componentsNetworkMap(account, "peer-0", validatedPeers)
require.NotNil(t, nm0)
assert.False(t, hasFirewallRuleTo(nm0, "100.64.0.1", "9443"), "destination must not see the direct source peer failing the posture check")
assert.True(t, hasFirewallRuleTo(nm0, "100.64.0.2", "9443"), "destination must see the direct source peer passing the posture check")
nm1 := componentsNetworkMap(account, "peer-1", validatedPeers)
require.NotNil(t, nm1)
assert.False(t, hasFirewallRuleTo(nm1, "100.64.0.0", "9443"), "a direct source peer failing the posture check gets no policy connectivity")
nm2 := componentsNetworkMap(account, "peer-2", validatedPeers)
require.NotNil(t, nm2)
assert.True(t, hasFirewallRuleTo(nm2, "100.64.0.0", "9443"), "a direct source peer passing the posture check gets policy connectivity")
}
// TestComponents_PeerAsResource_Unvalidated verifies that a directly referenced peer is
// subject to approval like a group member, whether it is the rule's source or destination.
func TestComponents_PeerAsResource_Unvalidated(t *testing.T) {
account, validatedPeers := scalableTestAccountWithoutDefaultPolicy(20, 2)
delete(validatedPeers, "peer-2")
account.Policies = append(account.Policies,
&types.Policy{
ID: "policy-unval-src", Name: "Unvalidated Source", Enabled: true, AccountID: "test-account",
Rules: []*types.PolicyRule{{
ID: "rule-unval-src", Enabled: true,
Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolTCP, Bidirectional: true,
Ports: []string{"9443"},
SourceResource: types.Resource{ID: "peer-2", Type: types.ResourceTypePeer},
Destinations: []string{"group-0"},
}},
},
&types.Policy{
ID: "policy-unval-dst", Name: "Unvalidated Destination", Enabled: true, AccountID: "test-account",
Rules: []*types.PolicyRule{{
ID: "rule-unval-dst", Enabled: true,
Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolTCP, Bidirectional: true,
Ports: []string{"9444"},
Sources: []string{"group-0"},
DestinationResource: types.Resource{ID: "peer-2", Type: types.ResourceTypePeer},
}},
},
)
nm0 := componentsNetworkMap(account, "peer-0", validatedPeers)
require.NotNil(t, nm0)
assert.False(t, hasFirewallRuleTo(nm0, "100.64.0.2", "9443"), "an unvalidated direct source peer must not be admitted")
assert.False(t, hasFirewallRuleTo(nm0, "100.64.0.2", "9444"), "an unvalidated direct destination peer must not be admitted")
for _, p := range nm0.Peers {
assert.NotEqual(t, "peer-2", p.ID, "an unvalidated direct peer must not be shipped as a remote peer")
}
}
// TestComponents_PeerAsDestinationResource verifies that a policy with DestinationResource.Type=Peer
// targets only that specific peer as the destination.
func TestComponents_PeerAsDestinationResource(t *testing.T) {