Remove unused route acl proto fields (#2432)

netbirdio · Sep 27, 2024 · 024e13b · 024e13b
1 parent 0e1cd9b
commit 024e13b
Show file tree

Hide file tree

Showing 14 changed files with 201 additions and 346 deletions.
diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go
@@ -77,15 +77,22 @@ func (m *Manager) AddPeerFiltering(
 	return m.aclMgr.AddPeerFiltering(ip, protocol, sPort, dPort, direction, action, ipsetName)
 }
 
-func (m *Manager) AddRouteFiltering(sources []netip.Prefix, destination netip.Prefix, proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, direction firewall.RuleDirection, action firewall.Action) (firewall.Rule, error) {
+func (m *Manager) AddRouteFiltering(
+	sources [] netip.Prefix,
+	destination netip.Prefix,
+	proto firewall.Protocol,
+	sPort *firewall.Port,
+	dPort *firewall.Port,
+	action firewall.Action,
+) (firewall.Rule, error) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
 	if !destination.Addr().Is4() {
 		return nil, fmt.Errorf("unsupported IP version: %s", destination.Addr().String())
 	}
 
-	return m.router.AddRouteFiltering(sources, destination, proto, sPort, dPort, direction, action)
+	return m.router.AddRouteFiltering(sources, destination, proto, sPort, dPort, action)
 }
 
 // DeletePeerRule from the firewall by rule definition

diff --git a/client/firewall/iptables/router_linux.go b/client/firewall/iptables/router_linux.go
@@ -97,10 +97,9 @@ func (r *router) AddRouteFiltering(
 	proto firewall.Protocol,
 	sPort *firewall.Port,
 	dPort *firewall.Port,
-	direction firewall.RuleDirection,
 	action firewall.Action,
 ) (firewall.Rule, error) {
-	ruleKey := id.GenerateRouteRuleKey(sources, destination, proto, sPort, dPort, direction, action)
+	ruleKey := id.GenerateRouteRuleKey(sources, destination, proto, sPort, dPort, action)
 	if _, ok := r.rules[string(ruleKey)]; ok {
 		return ruleKey, nil
 	}
@@ -119,7 +118,6 @@ func (r *router) AddRouteFiltering(
 		Proto:       proto,
 		SPort:       sPort,
 		DPort:       dPort,
-		Direction:   direction,
 		Action:      action,
 		SetName:     setName,
 	}
@@ -444,25 +442,13 @@ func genRouteFilteringRuleSpec(params routeFilteringRuleParams) []string {
 	var rule []string
 
 	if params.SetName != "" {
-		if params.Direction == firewall.RuleDirectionIN {
-			rule = append(rule, "-m", "set", matchSet, params.SetName, "src")
-		} else {
-			rule = append(rule, "-m", "set", matchSet, params.SetName, "dst")
-		}
+		rule = append(rule, "-m", "set", matchSet, params.SetName, "src")
 	} else if len(params.Sources) > 0 {
 		source := params.Sources[0]
-		if params.Direction == firewall.RuleDirectionIN {
-			rule = append(rule, "-s", source.String())
-		} else {
-			rule = append(rule, "-d", source.String())
-		}
+		rule = append(rule, "-s", source.String())
 	}
 
-	if params.Direction == firewall.RuleDirectionIN {
-		rule = append(rule, "-d", params.Destination.String())
-	} else {
-		rule = append(rule, "-s", params.Destination.String())
-	}
+	rule = append(rule, "-d", params.Destination.String())
 
 	if params.Proto != firewall.ProtocolALL {
 		rule = append(rule, "-p", strings.ToLower(string(params.Proto)))

diff --git a/client/firewall/iptables/router_linux_test.go b/client/firewall/iptables/router_linux_test.go
@@ -297,7 +297,7 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ruleKey, err := r.AddRouteFiltering(tt.sources, tt.destination, tt.proto, tt.sPort, tt.dPort, tt.direction, tt.action)
+			ruleKey, err := r.AddRouteFiltering(tt.sources, tt.destination, tt.proto, tt.sPort, tt.dPort, tt.action)
 			require.NoError(t, err, "AddRouteFiltering failed")
 
 			// Check if the rule is in the internal map
@@ -319,7 +319,6 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 				Proto:       tt.proto,
 				SPort:       tt.sPort,
 				DPort:       tt.dPort,
-				Direction:   tt.direction,
 				Action:      tt.action,
 				SetName:     "",
 			}

diff --git a/client/firewall/manager/firewall.go b/client/firewall/manager/firewall.go
@@ -76,7 +76,7 @@ type Manager interface {
 	// IsServerRouteSupported returns true if the firewall supports server side routing operations
 	IsServerRouteSupported() bool
 
-	AddRouteFiltering(source []netip.Prefix, destination netip.Prefix, proto Protocol, sPort *Port, dPort *Port, direction RuleDirection, action Action) (Rule, error)
+	AddRouteFiltering(source []netip.Prefix, destination netip.Prefix, proto Protocol, sPort *Port, dPort *Port, action Action) (Rule, error)
 
 	// DeleteRouteRule deletes a routing rule
 	DeleteRouteRule(rule Rule) error

diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go
@@ -84,15 +84,15 @@ func (m *Manager) AddPeerFiltering(
 	return m.aclManager.AddPeerFiltering(ip, proto, sPort, dPort, direction, action, ipsetName, comment)
 }
 
-func (m *Manager) AddRouteFiltering(sources []netip.Prefix, destination netip.Prefix, proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, direction firewall.RuleDirection, action firewall.Action) (firewall.Rule, error) {
+func (m *Manager) AddRouteFiltering(sources []netip.Prefix, destination netip.Prefix, proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, action firewall.Action) (firewall.Rule, error) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
 	if !destination.Addr().Is4() {
 		return nil, fmt.Errorf("unsupported IP version: %s", destination.Addr().String())
 	}
 
-	return m.router.AddRouteFiltering(sources, destination, proto, sPort, dPort, direction, action)
+	return m.router.AddRouteFiltering(sources, destination, proto, sPort, dPort, action)
 }
 
 // DeletePeerRule from the firewall by rule definition

diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go
@@ -186,10 +186,9 @@ func (r *router) AddRouteFiltering(
 	proto firewall.Protocol,
 	sPort *firewall.Port,
 	dPort *firewall.Port,
-	direction firewall.RuleDirection,
 	action firewall.Action,
 ) (firewall.Rule, error) {
-	ruleKey := id.GenerateRouteRuleKey(sources, destination, proto, sPort, dPort, direction, action)
+	ruleKey := id.GenerateRouteRuleKey(sources, destination, proto, sPort, dPort, action)
 	if _, ok := r.rules[string(ruleKey)]; ok {
 		return ruleKey, nil
 	}
@@ -202,7 +201,7 @@ func (r *router) AddRouteFiltering(
 		// If it's 0.0.0.0/0, we don't need to add any source matching
 	case len(sources) == 1:
 		// If there's only one source, we can use it directly
-		exprs = append(exprs, generateCIDRMatcherExpressions(direction == firewall.RuleDirectionIN, sources[0])...)
+		exprs = append(exprs, generateCIDRMatcherExpressions(true, sources[0])...)
 	default:
 		// If there are multiple sources, create or get an ipset
 		var err error
@@ -213,7 +212,7 @@ func (r *router) AddRouteFiltering(
 	}
 
 	// Handle destination
-	exprs = append(exprs, generateCIDRMatcherExpressions(direction == firewall.RuleDirectionOUT, destination)...)
+	exprs = append(exprs, generateCIDRMatcherExpressions(false, destination)...)
 
 	// Handle protocol
 	if proto != firewall.ProtocolALL {

diff --git a/client/firewall/nftables/router_linux_test.go b/client/firewall/nftables/router_linux_test.go
@@ -311,7 +311,7 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ruleKey, err := r.AddRouteFiltering(tt.sources, tt.destination, tt.proto, tt.sPort, tt.dPort, tt.direction, tt.action)
+			ruleKey, err := r.AddRouteFiltering(tt.sources, tt.destination, tt.proto, tt.sPort, tt.dPort, tt.action)
 			require.NoError(t, err, "AddRouteFiltering failed")
 
 			// Check if the rule is in the internal map

diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go
@@ -189,11 +189,11 @@ func (m *Manager) AddPeerFiltering(
 	return []firewall.Rule{&r}, nil
 }
 
-func (m *Manager) AddRouteFiltering(sources []netip.Prefix, destination netip.Prefix, proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, direction firewall.RuleDirection, action firewall.Action) (firewall.Rule, error) {
+func (m *Manager) AddRouteFiltering(sources [] netip.Prefix, destination netip.Prefix, proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, action firewall.Action ) (firewall.Rule, error) {
 	if m.nativeFirewall == nil {
 		return nil, errRouteNotSupported
 	}
-	return m.nativeFirewall.AddRouteFiltering(sources, destination, proto, sPort, dPort, direction, action)
+	return m.nativeFirewall.AddRouteFiltering(sources, destination, proto, sPort, dPort, action)
 }
 
 func (m *Manager) DeleteRouteRule(rule firewall.Rule) error {

diff --git a/client/internal/acl/id/id.go b/client/internal/acl/id/id.go
@@ -13,6 +13,13 @@ func (r RuleID) GetRuleID() string {
 	return string(r)
 }
 
-func GenerateRouteRuleKey(sources []netip.Prefix, destination netip.Prefix, proto manager.Protocol, sPort *manager.Port, dPort *manager.Port, direction manager.RuleDirection, action manager.Action) RuleID {
-	return RuleID(fmt.Sprintf("%s-%s-%s-%s-%s-%d-%d", sources, destination, proto, sPort, dPort, direction, action))
+func GenerateRouteRuleKey(
+	sources []netip.Prefix,
+	destination netip.Prefix,
+	proto manager.Protocol,
+	sPort *manager.Port,
+	dPort *manager.Port,
+	action manager.Action,
+) RuleID {
+	return RuleID(fmt.Sprintf("%s-%s-%s-%s-%s-%d", sources, destination, proto, sPort, dPort, action))
 }
diff --git a/client/internal/acl/manager.go b/client/internal/acl/manager.go
@@ -225,17 +225,8 @@ func (d *DefaultManager) applyRouteACL(rule *mgmProto.RouteFirewallRule) (id.Rul
 	}
 
 	dPorts := convertPortInfo(rule.PortInfo)
-	direction := firewall.RuleDirection(rule.Direction)
-
-	addedRule, err := d.firewall.AddRouteFiltering(
-		sources,
-		destination,
-		protocol,
-		nil,
-		dPorts,
-		direction,
-		action,
-	)
+
+	addedRule, err := d.firewall.AddRouteFiltering(sources, destination, protocol, nil, dPorts, action)
 	if err != nil {
 		return "", fmt.Errorf("add route rule: %w", err)
 	}