From 2b9c6107dd8e2f661dfde0860ca9ad431c9bafd6 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 7 Jan 2025 20:30:32 +0100 Subject: [PATCH] Remove outbound chains --- client/firewall/iptables/acl_linux.go | 70 +++------------ client/firewall/iptables/manager_linux.go | 6 +- .../firewall/iptables/manager_linux_test.go | 65 ++------------ client/firewall/manager/firewall.go | 1 - client/firewall/nftables/acl_linux.go | 89 ++++--------------- client/firewall/nftables/manager_linux.go | 12 ++- .../firewall/nftables/manager_linux_test.go | 26 +----- client/firewall/uspfilter/rule.go | 3 - client/firewall/uspfilter/uspfilter.go | 38 +++----- client/firewall/uspfilter/uspfilter_test.go | 44 ++------- client/internal/acl/manager.go | 40 +++------ client/internal/dnsfwd/manager.go | 2 +- client/internal/engine.go | 1 - 13 files changed, 75 insertions(+), 322 deletions(-) diff --git a/client/firewall/iptables/acl_linux.go b/client/firewall/iptables/acl_linux.go index d774f45381b..2592ff840b0 100644 --- a/client/firewall/iptables/acl_linux.go +++ b/client/firewall/iptables/acl_linux.go @@ -19,8 +19,7 @@ const ( tableName = "filter" // rules chains contains the effective ACL rules - chainNameInputRules = "NETBIRD-ACL-INPUT" - chainNameOutputRules = "NETBIRD-ACL-OUTPUT" + chainNameInputRules = "NETBIRD-ACL-INPUT" ) type aclEntries map[string][][]string @@ -84,7 +83,6 @@ func (m *aclManager) AddPeerFiltering( protocol firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, - direction firewall.RuleDirection, action firewall.Action, ipsetName string, ) ([]firewall.Rule, error) { @@ -97,15 +95,10 @@ func (m *aclManager) AddPeerFiltering( sPortVal = strconv.Itoa(sPort.Values[0]) } - var chain string - if direction == firewall.RuleDirectionOUT { - chain = chainNameOutputRules - } else { - chain = chainNameInputRules - } + chain := chainNameInputRules ipsetName = transformIPsetName(ipsetName, sPortVal, dPortVal) - specs := filterRuleSpecs(ip, string(protocol), sPortVal, dPortVal, direction, action, ipsetName) + specs := filterRuleSpecs(ip, string(protocol), sPortVal, dPortVal, action, ipsetName) if ipsetName != "" { if ipList, ipsetExists := m.ipsetStore.ipset(ipsetName); ipsetExists { if err := ipset.Add(ipsetName, ip.String()); err != nil { @@ -214,28 +207,7 @@ func (m *aclManager) Reset() error { // todo write less destructive cleanup mechanism func (m *aclManager) cleanChains() error { - ok, err := m.iptablesClient.ChainExists(tableName, chainNameOutputRules) - if err != nil { - log.Debugf("failed to list chains: %s", err) - return err - } - if ok { - rules := m.entries["OUTPUT"] - for _, rule := range rules { - err := m.iptablesClient.DeleteIfExists(tableName, "OUTPUT", rule...) - if err != nil { - log.Errorf("failed to delete rule: %v, %s", rule, err) - } - } - - err = m.iptablesClient.ClearAndDeleteChain(tableName, chainNameOutputRules) - if err != nil { - log.Debugf("failed to clear and delete %s chain: %s", chainNameOutputRules, err) - return err - } - } - - ok, err = m.iptablesClient.ChainExists(tableName, chainNameInputRules) + ok, err := m.iptablesClient.ChainExists(tableName, chainNameInputRules) if err != nil { log.Debugf("failed to list chains: %s", err) return err @@ -295,12 +267,6 @@ func (m *aclManager) createDefaultChains() error { return err } - // chain netbird-acl-output-rules - if err := m.iptablesClient.NewChain(tableName, chainNameOutputRules); err != nil { - log.Debugf("failed to create '%s' chain: %s", chainNameOutputRules, err) - return err - } - for chainName, rules := range m.entries { for _, rule := range rules { if err := m.iptablesClient.InsertUnique(tableName, chainName, 1, rule...); err != nil { @@ -329,8 +295,6 @@ func (m *aclManager) createDefaultChains() error { // The existing FORWARD rules/policies decide outbound traffic towards our interface. // In case the FORWARD policy is set to "drop", we add an established/related rule to allow return traffic for the inbound rule. - -// The OUTPUT chain gets an extra rule to allow traffic to any set up routes, the return traffic is handled by the INPUT related/established rule. func (m *aclManager) seedInitialEntries() { established := getConntrackEstablished() @@ -390,30 +354,18 @@ func (m *aclManager) updateState() { } // filterRuleSpecs returns the specs of a filtering rule -func filterRuleSpecs( - ip net.IP, protocol string, sPort, dPort string, direction firewall.RuleDirection, action firewall.Action, ipsetName string, -) (specs []string) { +func filterRuleSpecs(ip net.IP, protocol, sPort, dPort string, action firewall.Action, ipsetName string) (specs []string) { matchByIP := true // don't use IP matching if IP is ip 0.0.0.0 if ip.String() == "0.0.0.0" { matchByIP = false } - switch direction { - case firewall.RuleDirectionIN: - if matchByIP { - if ipsetName != "" { - specs = append(specs, "-m", "set", "--set", ipsetName, "src") - } else { - specs = append(specs, "-s", ip.String()) - } - } - case firewall.RuleDirectionOUT: - if matchByIP { - if ipsetName != "" { - specs = append(specs, "-m", "set", "--set", ipsetName, "dst") - } else { - specs = append(specs, "-d", ip.String()) - } + + if matchByIP { + if ipsetName != "" { + specs = append(specs, "-m", "set", "--set", ipsetName, "src") + } else { + specs = append(specs, "-s", ip.String()) } } if protocol != "all" { diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index da8e2c08f7e..75f082fc4c0 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/client/firewall/iptables/manager_linux.go @@ -100,15 +100,14 @@ func (m *Manager) AddPeerFiltering( protocol firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, - direction firewall.RuleDirection, action firewall.Action, ipsetName string, - comment string, + _ string, ) ([]firewall.Rule, error) { m.mutex.Lock() defer m.mutex.Unlock() - return m.aclMgr.AddPeerFiltering(ip, protocol, sPort, dPort, direction, action, ipsetName) + return m.aclMgr.AddPeerFiltering(ip, protocol, sPort, dPort, action, ipsetName) } func (m *Manager) AddRouteFiltering( @@ -201,7 +200,6 @@ func (m *Manager) AllowNetbird() error { "all", nil, nil, - firewall.RuleDirectionIN, firewall.ActionAccept, "", "", diff --git a/client/firewall/iptables/manager_linux_test.go b/client/firewall/iptables/manager_linux_test.go index ebdb831376f..f175de08a39 100644 --- a/client/firewall/iptables/manager_linux_test.go +++ b/client/firewall/iptables/manager_linux_test.go @@ -68,27 +68,13 @@ func TestIptablesManager(t *testing.T) { time.Sleep(time.Second) }() - var rule1 []fw.Rule - t.Run("add first rule", func(t *testing.T) { - ip := net.ParseIP("10.20.0.2") - port := &fw.Port{Values: []int{8080}} - rule1, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionOUT, fw.ActionAccept, "", "accept HTTP traffic") - require.NoError(t, err, "failed to add rule") - - for _, r := range rule1 { - checkRuleSpecs(t, ipv4Client, chainNameOutputRules, true, r.(*Rule).specs...) - } - - }) - var rule2 []fw.Rule t.Run("add second rule", func(t *testing.T) { ip := net.ParseIP("10.20.0.3") port := &fw.Port{ Values: []int{8043: 8046}, } - rule2, err = manager.AddPeerFiltering( - ip, "tcp", port, nil, fw.RuleDirectionIN, fw.ActionAccept, "", "accept HTTPS traffic from ports range") + rule2, err = manager.AddPeerFiltering(ip, "tcp", port, nil, fw.ActionAccept, "", "accept HTTPS traffic from ports range") require.NoError(t, err, "failed to add rule") for _, r := range rule2 { @@ -97,15 +83,6 @@ func TestIptablesManager(t *testing.T) { } }) - t.Run("delete first rule", func(t *testing.T) { - for _, r := range rule1 { - err := manager.DeletePeerRule(r) - require.NoError(t, err, "failed to delete rule") - - checkRuleSpecs(t, ipv4Client, chainNameOutputRules, false, r.(*Rule).specs...) - } - }) - t.Run("delete second rule", func(t *testing.T) { for _, r := range rule2 { err := manager.DeletePeerRule(r) @@ -119,7 +96,7 @@ func TestIptablesManager(t *testing.T) { // add second rule ip := net.ParseIP("10.20.0.3") port := &fw.Port{Values: []int{5353}} - _, err = manager.AddPeerFiltering(ip, "udp", nil, port, fw.RuleDirectionOUT, fw.ActionAccept, "", "accept Fake DNS traffic") + _, err = manager.AddPeerFiltering(ip, "udp", nil, port, fw.ActionAccept, "", "accept Fake DNS traffic") require.NoError(t, err, "failed to add rule") err = manager.Reset(nil) @@ -135,9 +112,6 @@ func TestIptablesManager(t *testing.T) { } func TestIptablesManagerIPSet(t *testing.T) { - ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) - require.NoError(t, err) - mock := &iFaceMock{ NameFunc: func() string { return "lo" @@ -167,33 +141,13 @@ func TestIptablesManagerIPSet(t *testing.T) { time.Sleep(time.Second) }() - var rule1 []fw.Rule - t.Run("add first rule with set", func(t *testing.T) { - ip := net.ParseIP("10.20.0.2") - port := &fw.Port{Values: []int{8080}} - rule1, err = manager.AddPeerFiltering( - ip, "tcp", nil, port, fw.RuleDirectionOUT, - fw.ActionAccept, "default", "accept HTTP traffic", - ) - require.NoError(t, err, "failed to add rule") - - for _, r := range rule1 { - checkRuleSpecs(t, ipv4Client, chainNameOutputRules, true, r.(*Rule).specs...) - require.Equal(t, r.(*Rule).ipsetName, "default-dport", "ipset name must be set") - require.Equal(t, r.(*Rule).ip, "10.20.0.2", "ipset IP must be set") - } - }) - var rule2 []fw.Rule t.Run("add second rule", func(t *testing.T) { ip := net.ParseIP("10.20.0.3") port := &fw.Port{ Values: []int{443}, } - rule2, err = manager.AddPeerFiltering( - ip, "tcp", port, nil, fw.RuleDirectionIN, fw.ActionAccept, - "default", "accept HTTPS traffic from ports range", - ) + rule2, err = manager.AddPeerFiltering(ip, "tcp", port, nil, fw.ActionAccept, "default", "accept HTTPS traffic from ports range") for _, r := range rule2 { require.NoError(t, err, "failed to add rule") require.Equal(t, r.(*Rule).ipsetName, "default-sport", "ipset name must be set") @@ -201,15 +155,6 @@ func TestIptablesManagerIPSet(t *testing.T) { } }) - t.Run("delete first rule", func(t *testing.T) { - for _, r := range rule1 { - err := manager.DeletePeerRule(r) - require.NoError(t, err, "failed to delete rule") - - require.NotContains(t, manager.aclMgr.ipsetStore.ipsets, r.(*Rule).ruleID, "rule must be removed form the ruleset index") - } - }) - t.Run("delete second rule", func(t *testing.T) { for _, r := range rule2 { err := manager.DeletePeerRule(r) @@ -271,9 +216,9 @@ func TestIptablesCreatePerformance(t *testing.T) { for i := 0; i < testMax; i++ { port := &fw.Port{Values: []int{1000 + i}} if i%2 == 0 { - _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionOUT, fw.ActionAccept, "", "accept HTTP traffic") + _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.ActionAccept, "", "accept HTTP traffic") } else { - _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionIN, fw.ActionAccept, "", "accept HTTP traffic") + _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.ActionAccept, "", "accept HTTP traffic") } require.NoError(t, err, "failed to add rule") diff --git a/client/firewall/manager/firewall.go b/client/firewall/manager/firewall.go index 9391b47ecab..f46e5eb5d57 100644 --- a/client/firewall/manager/firewall.go +++ b/client/firewall/manager/firewall.go @@ -69,7 +69,6 @@ type Manager interface { proto Protocol, sPort *Port, dPort *Port, - direction RuleDirection, action Action, ipsetName string, comment string, diff --git a/client/firewall/nftables/acl_linux.go b/client/firewall/nftables/acl_linux.go index 852cfec8de6..8c1d89e6833 100644 --- a/client/firewall/nftables/acl_linux.go +++ b/client/firewall/nftables/acl_linux.go @@ -22,8 +22,7 @@ import ( const ( // rules chains contains the effective ACL rules - chainNameInputRules = "netbird-acl-input-rules" - chainNameOutputRules = "netbird-acl-output-rules" + chainNameInputRules = "netbird-acl-input-rules" // filter chains contains the rules that jump to the rules chains chainNameInputFilter = "netbird-acl-input-filter" @@ -45,9 +44,8 @@ type AclManager struct { wgIface iFaceMapper routingFwChainName string - workTable *nftables.Table - chainInputRules *nftables.Chain - chainOutputRules *nftables.Chain + workTable *nftables.Table + chainInputRules *nftables.Chain ipsetStore *ipsetStore rules map[string]*Rule @@ -89,7 +87,6 @@ func (m *AclManager) AddPeerFiltering( proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, - direction firewall.RuleDirection, action firewall.Action, ipsetName string, comment string, @@ -104,7 +101,7 @@ func (m *AclManager) AddPeerFiltering( } newRules := make([]firewall.Rule, 0, 2) - ioRule, err := m.addIOFiltering(ip, proto, sPort, dPort, direction, action, ipset, comment) + ioRule, err := m.addIOFiltering(ip, proto, sPort, dPort, action, ipset, comment) if err != nil { return nil, err } @@ -214,38 +211,6 @@ func (m *AclManager) createDefaultAllowRules() error { Exprs: expIn, }) - expOut := []expr.Any{ - &expr.Payload{ - DestRegister: 1, - Base: expr.PayloadBaseNetworkHeader, - Offset: 16, - Len: 4, - }, - // mask - &expr.Bitwise{ - SourceRegister: 1, - DestRegister: 1, - Len: 4, - Mask: []byte{0, 0, 0, 0}, - Xor: []byte{0, 0, 0, 0}, - }, - // net address - &expr.Cmp{ - Register: 1, - Data: []byte{0, 0, 0, 0}, - }, - &expr.Verdict{ - Kind: expr.VerdictAccept, - }, - } - - _ = m.rConn.InsertRule(&nftables.Rule{ - Table: m.workTable, - Chain: m.chainOutputRules, - Position: 0, - Exprs: expOut, - }) - if err := m.rConn.Flush(); err != nil { return fmt.Errorf(flushError, err) } @@ -264,15 +229,19 @@ func (m *AclManager) Flush() error { log.Errorf("failed to refresh rule handles ipv4 input chain: %v", err) } - if err := m.refreshRuleHandles(m.chainOutputRules); err != nil { - log.Errorf("failed to refresh rule handles IPv4 output chain: %v", err) - } - return nil } -func (m *AclManager) addIOFiltering(ip net.IP, proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, direction firewall.RuleDirection, action firewall.Action, ipset *nftables.Set, comment string) (*Rule, error) { - ruleId := generatePeerRuleId(ip, sPort, dPort, direction, action, ipset) +func (m *AclManager) addIOFiltering( + ip net.IP, + proto firewall.Protocol, + sPort *firewall.Port, + dPort *firewall.Port, + action firewall.Action, + ipset *nftables.Set, + comment string, +) (*Rule, error) { + ruleId := generatePeerRuleId(ip, sPort, dPort, action, ipset) if r, ok := m.rules[ruleId]; ok { return &Rule{ r.nftRule, @@ -310,9 +279,6 @@ func (m *AclManager) addIOFiltering(ip net.IP, proto firewall.Protocol, sPort *f if !bytes.HasPrefix(anyIP, rawIP) { // source address position addrOffset := uint32(12) - if direction == firewall.RuleDirectionOUT { - addrOffset += 4 // is ipv4 address length - } expressions = append(expressions, &expr.Payload{ @@ -383,12 +349,7 @@ func (m *AclManager) addIOFiltering(ip net.IP, proto firewall.Protocol, sPort *f userData := []byte(strings.Join([]string{ruleId, comment}, " ")) - var chain *nftables.Chain - if direction == firewall.RuleDirectionIN { - chain = m.chainInputRules - } else { - chain = m.chainOutputRules - } + chain := m.chainInputRules nftRule := m.rConn.AddRule(&nftables.Rule{ Table: m.workTable, Chain: chain, @@ -419,15 +380,6 @@ func (m *AclManager) createDefaultChains() (err error) { } m.chainInputRules = chain - // chainNameOutputRules - chain = m.createChain(chainNameOutputRules) - err = m.rConn.Flush() - if err != nil { - log.Debugf("failed to create chain (%s): %s", chainNameOutputRules, err) - return err - } - m.chainOutputRules = chain - // netbird-acl-input-filter // type filter hook input priority filter; policy accept; chain = m.createFilterChainWithHook(chainNameInputFilter, nftables.ChainHookInput) @@ -720,15 +672,8 @@ func (m *AclManager) refreshRuleHandles(chain *nftables.Chain) error { return nil } -func generatePeerRuleId( - ip net.IP, - sPort *firewall.Port, - dPort *firewall.Port, - direction firewall.RuleDirection, - action firewall.Action, - ipset *nftables.Set, -) string { - rulesetID := ":" + strconv.Itoa(int(direction)) + ":" +func generatePeerRuleId(ip net.IP, sPort *firewall.Port, dPort *firewall.Port, action firewall.Action, ipset *nftables.Set) string { + rulesetID := ":" if sPort != nil { rulesetID += sPort.String() } diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index 8e1aa0d8043..a78626dbcd5 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/client/firewall/nftables/manager_linux.go @@ -117,7 +117,6 @@ func (m *Manager) AddPeerFiltering( proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, - direction firewall.RuleDirection, action firewall.Action, ipsetName string, comment string, @@ -130,10 +129,17 @@ func (m *Manager) AddPeerFiltering( return nil, fmt.Errorf("unsupported IP version: %s", ip.String()) } - return m.aclManager.AddPeerFiltering(ip, proto, sPort, dPort, direction, action, ipsetName, comment) + return m.aclManager.AddPeerFiltering(ip, proto, sPort, dPort, action, ipsetName, comment) } -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) { +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() diff --git a/client/firewall/nftables/manager_linux_test.go b/client/firewall/nftables/manager_linux_test.go index 33fdc4b3d7e..bde53c705ea 100644 --- a/client/firewall/nftables/manager_linux_test.go +++ b/client/firewall/nftables/manager_linux_test.go @@ -74,16 +74,7 @@ func TestNftablesManager(t *testing.T) { testClient := &nftables.Conn{} - rule, err := manager.AddPeerFiltering( - ip, - fw.ProtocolTCP, - nil, - &fw.Port{Values: []int{53}}, - fw.RuleDirectionIN, - fw.ActionDrop, - "", - "", - ) + rule, err := manager.AddPeerFiltering(ip, fw.ProtocolTCP, nil, &fw.Port{Values: []int{53}}, fw.ActionDrop, "", "") require.NoError(t, err, "failed to add rule") err = manager.Flush() @@ -211,9 +202,9 @@ func TestNFtablesCreatePerformance(t *testing.T) { for i := 0; i < testMax; i++ { port := &fw.Port{Values: []int{1000 + i}} if i%2 == 0 { - _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionOUT, fw.ActionAccept, "", "accept HTTP traffic") + _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.ActionAccept, "", "accept HTTP traffic") } else { - _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionIN, fw.ActionAccept, "", "accept HTTP traffic") + _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.ActionAccept, "", "accept HTTP traffic") } require.NoError(t, err, "failed to add rule") @@ -296,16 +287,7 @@ func TestNftablesManagerCompatibilityWithIptables(t *testing.T) { }) ip := net.ParseIP("100.96.0.1") - _, err = manager.AddPeerFiltering( - ip, - fw.ProtocolTCP, - nil, - &fw.Port{Values: []int{80}}, - fw.RuleDirectionIN, - fw.ActionAccept, - "", - "test rule", - ) + _, err = manager.AddPeerFiltering(ip, fw.ProtocolTCP, nil, &fw.Port{Values: []int{80}}, fw.ActionAccept, "", "test rule") require.NoError(t, err, "failed to add peer filtering rule") _, err = manager.AddRouteFiltering( diff --git a/client/firewall/uspfilter/rule.go b/client/firewall/uspfilter/rule.go index 5c1daccaf53..1f98ef43e57 100644 --- a/client/firewall/uspfilter/rule.go +++ b/client/firewall/uspfilter/rule.go @@ -4,8 +4,6 @@ import ( "net" "github.com/google/gopacket" - - firewall "github.com/netbirdio/netbird/client/firewall/manager" ) // Rule to handle management of rules @@ -15,7 +13,6 @@ type Rule struct { ipLayer gopacket.LayerType matchByIP bool protoLayer gopacket.LayerType - direction firewall.RuleDirection sPort uint16 dPort uint16 drop bool diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index ebe04caee59..1fce59d2651 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -39,7 +39,9 @@ type RuleSet map[string]Rule // Manager userspace firewall manager type Manager struct { - outgoingRules map[string]RuleSet + // outgoingRules is used for hooks only + outgoingRules map[string]RuleSet + // incomingRules is used for filtering and hooks incomingRules map[string]RuleSet wgNetwork *net.IPNet decoders sync.Pool @@ -156,9 +158,8 @@ func (m *Manager) AddPeerFiltering( proto firewall.Protocol, sPort *firewall.Port, dPort *firewall.Port, - direction firewall.RuleDirection, action firewall.Action, - ipsetName string, + _ string, comment string, ) ([]firewall.Rule, error) { r := Rule{ @@ -166,7 +167,6 @@ func (m *Manager) AddPeerFiltering( ip: ip, ipLayer: layers.LayerTypeIPv6, matchByIP: true, - direction: direction, drop: action == firewall.ActionDrop, comment: comment, } @@ -202,17 +202,10 @@ func (m *Manager) AddPeerFiltering( } m.mutex.Lock() - if direction == firewall.RuleDirectionIN { - if _, ok := m.incomingRules[r.ip.String()]; !ok { - m.incomingRules[r.ip.String()] = make(RuleSet) - } - m.incomingRules[r.ip.String()][r.id] = r - } else { - if _, ok := m.outgoingRules[r.ip.String()]; !ok { - m.outgoingRules[r.ip.String()] = make(RuleSet) - } - m.outgoingRules[r.ip.String()][r.id] = r + if _, ok := m.incomingRules[r.ip.String()]; !ok { + m.incomingRules[r.ip.String()] = make(RuleSet) } + m.incomingRules[r.ip.String()][r.id] = r m.mutex.Unlock() return []firewall.Rule{&r}, nil } @@ -241,19 +234,10 @@ func (m *Manager) DeletePeerRule(rule firewall.Rule) error { return fmt.Errorf("delete rule: invalid rule type: %T", rule) } - if r.direction == firewall.RuleDirectionIN { - _, ok := m.incomingRules[r.ip.String()][r.id] - if !ok { - return fmt.Errorf("delete rule: no rule with such id: %v", r.id) - } - delete(m.incomingRules[r.ip.String()], r.id) - } else { - _, ok := m.outgoingRules[r.ip.String()][r.id] - if !ok { - return fmt.Errorf("delete rule: no rule with such id: %v", r.id) - } - delete(m.outgoingRules[r.ip.String()], r.id) + if _, ok := m.incomingRules[r.ip.String()][r.id]; !ok { + return fmt.Errorf("delete rule: no rule with such id: %v", r.id) } + delete(m.incomingRules[r.ip.String()], r.id) return nil } @@ -566,7 +550,6 @@ func (m *Manager) AddUDPPacketHook( protoLayer: layers.LayerTypeUDP, dPort: dPort, ipLayer: layers.LayerTypeIPv6, - direction: firewall.RuleDirectionOUT, comment: fmt.Sprintf("UDP Hook direction: %v, ip:%v, dport:%d", in, ip, dPort), udpHook: hook, } @@ -577,7 +560,6 @@ func (m *Manager) AddUDPPacketHook( m.mutex.Lock() if in { - r.direction = firewall.RuleDirectionIN if _, ok := m.incomingRules[r.ip.String()]; !ok { m.incomingRules[r.ip.String()] = make(map[string]Rule) } diff --git a/client/firewall/uspfilter/uspfilter_test.go b/client/firewall/uspfilter/uspfilter_test.go index d3563e6f251..e8d3002b143 100644 --- a/client/firewall/uspfilter/uspfilter_test.go +++ b/client/firewall/uspfilter/uspfilter_test.go @@ -70,11 +70,10 @@ func TestManagerAddPeerFiltering(t *testing.T) { ip := net.ParseIP("192.168.1.1") proto := fw.ProtocolTCP port := &fw.Port{Values: []int{80}} - direction := fw.RuleDirectionOUT action := fw.ActionDrop comment := "Test rule" - rule, err := m.AddPeerFiltering(ip, proto, nil, port, direction, action, "", comment) + rule, err := m.AddPeerFiltering(ip, proto, nil, port, action, "", comment) if err != nil { t.Errorf("failed to add filtering: %v", err) return @@ -105,37 +104,15 @@ func TestManagerDeleteRule(t *testing.T) { ip := net.ParseIP("192.168.1.1") proto := fw.ProtocolTCP port := &fw.Port{Values: []int{80}} - direction := fw.RuleDirectionOUT action := fw.ActionDrop - comment := "Test rule" - - rule, err := m.AddPeerFiltering(ip, proto, nil, port, direction, action, "", comment) - if err != nil { - t.Errorf("failed to add filtering: %v", err) - return - } + comment := "Test rule 2" - ip = net.ParseIP("192.168.1.1") - proto = fw.ProtocolTCP - port = &fw.Port{Values: []int{80}} - direction = fw.RuleDirectionIN - action = fw.ActionDrop - comment = "Test rule 2" - - rule2, err := m.AddPeerFiltering(ip, proto, nil, port, direction, action, "", comment) + rule2, err := m.AddPeerFiltering(ip, proto, nil, port, action, "", comment) if err != nil { t.Errorf("failed to add filtering: %v", err) return } - for _, r := range rule { - err = m.DeletePeerRule(r) - if err != nil { - t.Errorf("failed to delete rule: %v", err) - return - } - } - for _, r := range rule2 { if _, ok := m.incomingRules[ip.String()][r.GetRuleID()]; !ok { t.Errorf("rule2 is not in the incomingRules") @@ -225,10 +202,6 @@ func TestAddUDPPacketHook(t *testing.T) { t.Errorf("expected protoLayer %s, got %s", layers.LayerTypeUDP, addedRule.protoLayer) return } - if tt.expDir != addedRule.direction { - t.Errorf("expected direction %d, got %d", tt.expDir, addedRule.direction) - return - } if addedRule.udpHook == nil { t.Errorf("expected udpHook to be set") return @@ -251,11 +224,10 @@ func TestManagerReset(t *testing.T) { ip := net.ParseIP("192.168.1.1") proto := fw.ProtocolTCP port := &fw.Port{Values: []int{80}} - direction := fw.RuleDirectionOUT action := fw.ActionDrop comment := "Test rule" - _, err = m.AddPeerFiltering(ip, proto, nil, port, direction, action, "", comment) + _, err = m.AddPeerFiltering(ip, proto, nil, port, action, "", comment) if err != nil { t.Errorf("failed to add filtering: %v", err) return @@ -293,7 +265,7 @@ func TestNotMatchByIP(t *testing.T) { action := fw.ActionAccept comment := "Test rule" - _, err = m.AddPeerFiltering(ip, proto, nil, nil, direction, action, "", comment) + _, err = m.AddPeerFiltering(ip, proto, nil, nil, action, "", comment) if err != nil { t.Errorf("failed to add filtering: %v", err) return @@ -493,11 +465,7 @@ func TestUSPFilterCreatePerformance(t *testing.T) { start := time.Now() for i := 0; i < testMax; i++ { port := &fw.Port{Values: []int{1000 + i}} - if i%2 == 0 { - _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionOUT, fw.ActionAccept, "", "accept HTTP traffic") - } else { - _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.RuleDirectionIN, fw.ActionAccept, "", "accept HTTP traffic") - } + _, err = manager.AddPeerFiltering(ip, "tcp", nil, port, fw.ActionAccept, "", "accept HTTP traffic") require.NoError(t, err, "failed to add rule") } diff --git a/client/internal/acl/manager.go b/client/internal/acl/manager.go index 5bb0905d2a7..ff51954ef3d 100644 --- a/client/internal/acl/manager.go +++ b/client/internal/acl/manager.go @@ -151,7 +151,7 @@ func (d *DefaultManager) applyPeerACLs(networkMap *mgmProto.NetworkMap) { d.rollBack(newRulePairs) break } - if len(rules) > 0 { + if len(rulePair) > 0 { d.peerRulesPairs[pairID] = rulePair newRulePairs[pairID] = rulePair } @@ -288,6 +288,8 @@ func (d *DefaultManager) protoRuleToFirewallRule( case mgmProto.RuleDirection_IN: rules, err = d.addInRules(ip, protocol, port, action, ipsetName, "") case mgmProto.RuleDirection_OUT: + // TODO: Remove this soon. Outbound rules are obsolete. + // We only maintain this for return traffic (in dir) which is now handled by the stateful firewall already rules, err = d.addOutRules(ip, protocol, port, action, ipsetName, "") default: return "", nil, fmt.Errorf("invalid direction, skipping firewall rule") @@ -308,25 +310,12 @@ func (d *DefaultManager) addInRules( ipsetName string, comment string, ) ([]firewall.Rule, error) { - var rules []firewall.Rule - rule, err := d.firewall.AddPeerFiltering( - ip, protocol, nil, port, firewall.RuleDirectionIN, action, ipsetName, comment) + rule, err := d.firewall.AddPeerFiltering(ip, protocol, nil, port, action, ipsetName, comment) if err != nil { - return nil, fmt.Errorf("failed to add firewall rule: %v", err) - } - rules = append(rules, rule...) - - if shouldSkipInvertedRule(protocol, port) { - return rules, nil + return nil, fmt.Errorf("add firewall rule: %w", err) } - rule, err = d.firewall.AddPeerFiltering( - ip, protocol, port, nil, firewall.RuleDirectionOUT, action, ipsetName, comment) - if err != nil { - return nil, fmt.Errorf("failed to add firewall rule: %v", err) - } - - return append(rules, rule...), nil + return rule, nil } func (d *DefaultManager) addOutRules( @@ -337,25 +326,16 @@ func (d *DefaultManager) addOutRules( ipsetName string, comment string, ) ([]firewall.Rule, error) { - var rules []firewall.Rule - rule, err := d.firewall.AddPeerFiltering( - ip, protocol, nil, port, firewall.RuleDirectionOUT, action, ipsetName, comment) - if err != nil { - return nil, fmt.Errorf("failed to add firewall rule: %v", err) - } - rules = append(rules, rule...) - if shouldSkipInvertedRule(protocol, port) { - return rules, nil + return nil, nil } - rule, err = d.firewall.AddPeerFiltering( - ip, protocol, port, nil, firewall.RuleDirectionIN, action, ipsetName, comment) + rule, err := d.firewall.AddPeerFiltering(ip, protocol, port, nil, action, ipsetName, comment) if err != nil { - return nil, fmt.Errorf("failed to add firewall rule: %v", err) + return nil, fmt.Errorf("add firewall rule: %w", err) } - return append(rules, rule...), nil + return rule, nil } // getPeerRuleID() returns unique ID for the rule based on its parameters. diff --git a/client/internal/dnsfwd/manager.go b/client/internal/dnsfwd/manager.go index e6dfd278e5a..968f2d39853 100644 --- a/client/internal/dnsfwd/manager.go +++ b/client/internal/dnsfwd/manager.go @@ -88,7 +88,7 @@ func (h *Manager) allowDNSFirewall() error { return nil } - dnsRules, err := h.firewall.AddPeerFiltering(net.IP{0, 0, 0, 0}, firewall.ProtocolUDP, nil, dport, firewall.RuleDirectionIN, firewall.ActionAccept, "", "") + dnsRules, err := h.firewall.AddPeerFiltering(net.IP{0, 0, 0, 0}, firewall.ProtocolUDP, nil, dport, firewall.ActionAccept, "", "") if err != nil { log.Errorf("failed to add allow DNS router rules, err: %v", err) return err diff --git a/client/internal/engine.go b/client/internal/engine.go index a5247bc2799..1042f003dc1 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -495,7 +495,6 @@ func (e *Engine) initFirewall() error { manager.ProtocolUDP, nil, &port, - manager.RuleDirectionIN, manager.ActionAccept, "", "",