Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[client] Account different policies rules for routes firewall rules #2939

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[client] Account different policies rules for routes firewall rules #2939

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fa5d5a
Account different policies rules for routes firewall rules
mlsmaycon Nov 22, 2024
d18b0a7
Merge branch 'main' into fix/enforce-network-firewall-rules-based-on-…
mlsmaycon Nov 22, 2024
78bd1dd
split function
mlsmaycon Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 60 additions & 10 deletions management/server/route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,27 +417,77 @@ func (a *Account) getPeerRoutesFirewallRules(ctx context.Context, peerID string,
continue
}

policies := getAllRoutePoliciesFromGroups(a, route.AccessControlGroups)
for _, policy := range policies {
if !policy.Enabled {
continue
}
distributionPeers := a.getDistributionGroupsPeers(route)

for _, rule := range policy.Rules {
if !rule.Enabled {
for _, accessGroup := range route.AccessControlGroups {
policies := getAllRoutePoliciesFromGroups(a, []string{accessGroup})
for _, policy := range policies {
if !policy.Enabled {
continue
}

distributionGroupPeers, _ := a.getAllPeersFromGroups(ctx, route.Groups, peerID, nil, validatedPeersMap)
rules := generateRouteFirewallRules(ctx, route, rule, distributionGroupPeers, firewallRuleDirectionIN)
routesFirewallRules = append(routesFirewallRules, rules...)
for _, rule := range policy.Rules {
if !rule.Enabled {
continue
}

rulePeers := a.getRulePeers(rule, peerID, distributionPeers, validatedPeersMap)
rules := generateRouteFirewallRules(ctx, route, rule, rulePeers, firewallRuleDirectionIN)
routesFirewallRules = append(routesFirewallRules, rules...)
}
}
}
}

return routesFirewallRules
}

func (a *Account) getRulePeers(rule *PolicyRule, peerID string, distributionPeers map[string]struct{}, validatedPeersMap map[string]struct{}) []*nbpeer.Peer {
distPeersWithPolicy := make(map[string]struct{})
for _, id := range rule.Sources {
group := a.Groups[id]
if group == nil {
continue
}

for _, pID := range group.Peers {
if pID == peerID {
continue
}
_, distPeer := distributionPeers[pID]
_, valid := validatedPeersMap[pID]
if distPeer && valid {
distPeersWithPolicy[pID] = struct{}{}
}
}
}

distributionGroupPeers := make([]*nbpeer.Peer, 0, len(distPeersWithPolicy))
for pID := range distPeersWithPolicy {
peer := a.Peers[pID]
if peer == nil {
continue
}
distributionGroupPeers = append(distributionGroupPeers, peer)
}
return distributionGroupPeers
}

func (a *Account) getDistributionGroupsPeers(route *route.Route) map[string]struct{} {
distPeers := make(map[string]struct{})
for _, id := range route.Groups {
group := a.Groups[id]
if group == nil {
continue
}

for _, pID := range group.Peers {
distPeers[pID] = struct{}{}
}
}
return distPeers
}

func getDefaultPermit(route *route.Route) []*RouteFirewallRule {
var rules []*RouteFirewallRule

Expand Down
132 changes: 128 additions & 4 deletions management/server/route_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net"
"net/netip"
"sort"
"testing"
"time"

Expand Down Expand Up @@ -1487,6 +1488,8 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
peerBIp = "100.65.80.39"
peerCIp = "100.65.254.139"
peerHIp = "100.65.29.55"
peerJIp = "100.65.29.65"
peerKIp = "100.65.29.66"
)

account := &Account{
Expand Down Expand Up @@ -1542,6 +1545,16 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
IP: net.ParseIP(peerHIp),
Status: &nbpeer.PeerStatus{},
},
"peerJ": {
ID: "peerJ",
IP: net.ParseIP(peerJIp),
Status: &nbpeer.PeerStatus{},
},
"peerK": {
ID: "peerK",
IP: net.ParseIP(peerKIp),
Status: &nbpeer.PeerStatus{},
},
},
Groups: map[string]*nbgroup.Group{
"routingPeer1": {
Expand All @@ -1568,6 +1581,11 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
Name: "Route2",
Peers: []string{},
},
"route4": {
ID: "route4",
Name: "route4",
Peers: []string{},
},
"finance": {
ID: "finance",
Name: "Finance",
Expand All @@ -1585,6 +1603,28 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
"peerB",
},
},
"qa": {
ID: "qa",
Name: "QA",
Peers: []string{
"peerJ",
"peerK",
},
},
"restrictQA": {
ID: "restrictQA",
Name: "restrictQA",
Peers: []string{
"peerJ",
},
},
"unrestrictedQA": {
ID: "unrestrictedQA",
Name: "unrestrictedQA",
Peers: []string{
"peerK",
},
},
"contractors": {
ID: "contractors",
Name: "Contractors",
Expand Down Expand Up @@ -1632,6 +1672,19 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
Groups: []string{"contractors"},
AccessControlGroups: []string{},
},
"route4": {
ID: "route4",
Network: netip.MustParsePrefix("192.168.10.0/16"),
NetID: "route4",
NetworkType: route.IPv4Network,
PeerGroups: []string{"routingPeer1"},
Description: "Route4",
Masquerade: false,
Metric: 9999,
Enabled: true,
Groups: []string{"qa"},
AccessControlGroups: []string{"route4"},
},
},
Policies: []*Policy{
{
Expand Down Expand Up @@ -1686,6 +1739,49 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
},
},
},
{
ID: "RuleRoute4",
Name: "RuleRoute4",
Enabled: true,
Rules: []*PolicyRule{
{
ID: "RuleRoute4",
Name: "RuleRoute4",
Bidirectional: true,
Enabled: true,
Protocol: PolicyRuleProtocolTCP,
Action: PolicyTrafficActionAccept,
Ports: []string{"80"},
Sources: []string{
"restrictQA",
},
Destinations: []string{
"route4",
},
},
},
},
{
ID: "RuleRoute5",
Name: "RuleRoute5",
Enabled: true,
Rules: []*PolicyRule{
{
ID: "RuleRoute5",
Name: "RuleRoute5",
Bidirectional: true,
Enabled: true,
Protocol: PolicyRuleProtocolALL,
Action: PolicyTrafficActionAccept,
Sources: []string{
"unrestrictedQA",
},
Destinations: []string{
"route4",
},
},
},
},
},
}

Expand All @@ -1710,7 +1806,7 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {

t.Run("check peer routes firewall rules", func(t *testing.T) {
routesFirewallRules := account.getPeerRoutesFirewallRules(context.Background(), "peerA", validatedPeers)
assert.Len(t, routesFirewallRules, 2)
assert.Len(t, routesFirewallRules, 4)

expectedRoutesFirewallRules := []*RouteFirewallRule{
{
Expand All @@ -1736,12 +1832,32 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
Port: 320,
},
}
assert.ElementsMatch(t, routesFirewallRules, expectedRoutesFirewallRules)
additionalFirewallRule := []*RouteFirewallRule{
{
SourceRanges: []string{
fmt.Sprintf(AllowedIPsFormat, peerJIp),
},
Action: "accept",
Destination: "192.168.10.0/16",
Protocol: "tcp",
Port: 80,
},
{
SourceRanges: []string{
fmt.Sprintf(AllowedIPsFormat, peerKIp),
},
Action: "accept",
Destination: "192.168.10.0/16",
Protocol: "all",
},
}

assert.ElementsMatch(t, orderRuleSourceRanges(routesFirewallRules), orderRuleSourceRanges(append(expectedRoutesFirewallRules, additionalFirewallRule...)))

// peerD is also the routing peer for route1, should contain same routes firewall rules as peerA
routesFirewallRules = account.getPeerRoutesFirewallRules(context.Background(), "peerD", validatedPeers)
assert.Len(t, routesFirewallRules, 2)
assert.ElementsMatch(t, routesFirewallRules, expectedRoutesFirewallRules)
assert.ElementsMatch(t, orderRuleSourceRanges(routesFirewallRules), orderRuleSourceRanges(expectedRoutesFirewallRules))

// peerE is a single routing peer for route 2 and route 3
routesFirewallRules = account.getPeerRoutesFirewallRules(context.Background(), "peerE", validatedPeers)
Expand Down Expand Up @@ -1770,7 +1886,7 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {
IsDynamic: true,
},
}
assert.ElementsMatch(t, routesFirewallRules, expectedRoutesFirewallRules)
assert.ElementsMatch(t, orderRuleSourceRanges(routesFirewallRules), orderRuleSourceRanges(expectedRoutesFirewallRules))
Copy link
Contributor

@lixmal lixmal Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func ElementsMatch(t TestingT, listA interface{}, listB interface{}, msgAndArgs ...interface{}) (ok bool)

ElementsMatch asserts that the specified listA(array, slice...) is equal to specified listB(array, slice...) ignoring the order of the elements. If there are duplicate elements, the number of appearances of each of them in both lists should match.

This works, without the change in this line, doesn't it?

There's another a few lines up

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, I am just keeping the same style to avoid future issues and because this is a testing func.


// peerC is part of route1 distribution groups but should not receive the routes firewall rules
routesFirewallRules = account.getPeerRoutesFirewallRules(context.Background(), "peerC", validatedPeers)
Expand All @@ -1779,6 +1895,14 @@ func TestAccount_getPeersRoutesFirewall(t *testing.T) {

}

// orderList is a helper function to sort a list of strings
func orderRuleSourceRanges(ruleList []*RouteFirewallRule) []*RouteFirewallRule {
for _, rule := range ruleList {
sort.Strings(rule.SourceRanges)
}
return ruleList
}

func TestRouteAccountPeersUpdate(t *testing.T) {
manager, err := createRouterManager(t)
require.NoError(t, err, "failed to create account manager")
Expand Down
Loading