From 4c56578b0c6c64eaebee0730d8d3f178ab3f9a16 Mon Sep 17 00:00:00 2001 From: Givi Khojanashvili Date: Thu, 23 Mar 2023 16:54:04 +0400 Subject: [PATCH] Add default firewall rules and fix tests in the github flow. --- .github/workflows/golang-test-linux.yml | 4 +- client/firewall/iptables/manager_linux.go | 96 ++++++++++++++----- .../firewall/iptables/manager_linux_test.go | 9 +- client/internal/engine.go | 2 +- client/internal/engine_test.go | 7 ++ client/internal/firewall.go | 13 +-- client/internal/firewall_linux.go | 18 ++++ 7 files changed, 111 insertions(+), 38 deletions(-) create mode 100644 client/internal/firewall_linux.go diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index 4186baf38..d600575e6 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -72,7 +72,7 @@ jobs: run: go test -c -o routemanager-testing.bin ./client/internal/routemanager/... - name: Generate Engine Test bin - run: go test -c -o engine-testing.bin ./client/internal/*.go + run: go test -c -o engine-testing.bin ./client/internal - name: Generate Peer Test bin run: go test -c -o peer-testing.bin ./client/internal/peer/... @@ -89,4 +89,4 @@ jobs: run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/engine-testing.bin -test.timeout 5m -test.parallel 1 - name: Run Peer tests in docker - run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal/peer --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/peer-testing.bin -test.timeout 5m -test.parallel 1 \ No newline at end of file + run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal/peer --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/peer-testing.bin -test.timeout 5m -test.parallel 1 diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index dc7e9daeb..b3a66f02b 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/client/firewall/iptables/manager_linux.go @@ -10,6 +10,7 @@ import ( "github.com/google/uuid" fw "github.com/netbirdio/netbird/client/firewall" + log "github.com/sirupsen/logrus" ) const ( @@ -17,17 +18,30 @@ const ( ChainFilterName = "NETBIRD-ACL" ) +// jumpNetbirdDefaultRule always added by manager to the input chain for all trafic from the Netbird interface +var jumpNetbirdDefaultRule = []string{ + "-j", ChainFilterName, "-m", "comment", "--comment", "Netbird traffic chain jump"} + +// pingSupportDefaultRule always added by the manager to the Netbird ACL chain +var pingSupportDefaultRule = []string{ + "-p", "icmp", "--icmp-type", "echo-request", "-j", + "ACCEPT", "-m", "comment", "--comment", "Allow pings from the Netbird Devices"} + // Manager of iptables firewall type Manager struct { mutex sync.Mutex ipv4Client *iptables.IPTables ipv6Client *iptables.IPTables + + wgIfaceName string } // Create iptables firewall manager -func Create() (*Manager, error) { - m := &Manager{} +func Create(wgIfaceName string) (*Manager, error) { + m := &Manager{ + wgIfaceName: wgIfaceName, + } // init clients for booth ipv4 and ipv6 ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) @@ -38,14 +52,14 @@ func Create() (*Manager, error) { ipv6Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv6) if err != nil { - return nil, fmt.Errorf("ip6tables is not installed in the system or not supported") + log.Errorf("ip6tables is not installed in the system or not supported") + } else { + m.ipv6Client = ipv6Client } - m.ipv6Client = ipv6Client if err := m.Reset(); err != nil { - return nil, fmt.Errorf("failed to reset firewall: %s", err) + return nil, fmt.Errorf("failed to reset firewall: %v", err) } - return m, nil } @@ -63,7 +77,7 @@ func (m *Manager) AddFiltering( m.mutex.Lock() defer m.mutex.Unlock() - client, err := m.clientWithChain(ip) + client, err := m.client(ip) if err != nil { return nil, err } @@ -89,7 +103,16 @@ func (m *Manager) AddFiltering( action, comment, ) - if err := client.AppendUnique("filter", ChainFilterName, specs...); err != nil { + + ok, err := client.Exists("filter", ChainFilterName, specs...) + if err != nil { + return nil, fmt.Errorf("check is rule already exists: %w", err) + } + if ok { + return nil, fmt.Errorf("rule already exists") + } + + if err := client.Insert("filter", ChainFilterName, 1, specs...); err != nil { return nil, err } @@ -100,12 +123,17 @@ func (m *Manager) AddFiltering( func (m *Manager) DeleteRule(rule fw.Rule) error { m.mutex.Lock() defer m.mutex.Unlock() + r, ok := rule.(*Rule) if !ok { return fmt.Errorf("invalid rule type") } + client := m.ipv4Client if r.v6 { + if m.ipv6Client == nil { + return fmt.Errorf("ipv6 is not supported") + } client = m.ipv6Client } return client.Delete("filter", ChainFilterName, r.specs...) @@ -115,11 +143,14 @@ func (m *Manager) DeleteRule(rule fw.Rule) error { func (m *Manager) Reset() error { m.mutex.Lock() defer m.mutex.Unlock() + if err := m.reset(m.ipv4Client, "filter", ChainFilterName); err != nil { return fmt.Errorf("clean ipv4 firewall ACL chain: %w", err) } - if err := m.reset(m.ipv6Client, "filter", ChainFilterName); err != nil { - return fmt.Errorf("clean ipv6 firewall ACL chain: %w", err) + if m.ipv6Client != nil { + if err := m.reset(m.ipv6Client, "filter", ChainFilterName); err != nil { + return fmt.Errorf("clean ipv6 firewall ACL chain: %w", err) + } } return nil } @@ -133,10 +164,13 @@ func (m *Manager) reset(client *iptables.IPTables, table, chain string) error { if !ok { return nil } - if err := client.ClearChain(table, ChainFilterName); err != nil { - return fmt.Errorf("failed to clear chain: %w", err) + + specs := append([]string{"-i", m.wgIfaceName}, jumpNetbirdDefaultRule...) + if err := client.Delete("filter", "INPUT", specs...); err != nil { + return fmt.Errorf("failed to delete default rule: %w", err) } - return client.DeleteChain(table, ChainFilterName) + + return client.ClearAndDeleteChain(table, chain) } // filterRuleSpecs returns the specs of a filtering rule @@ -158,31 +192,43 @@ func (m *Manager) filterRuleSpecs( return append(specs, "-m", "comment", "--comment", comment) } -// client returns corresponding iptables client for the given ip -func (m *Manager) client(ip net.IP) *iptables.IPTables { +// rawClient returns corresponding iptables client for the given ip +func (m *Manager) rawClient(ip net.IP) (*iptables.IPTables, error) { if ip.To4() != nil { - return m.ipv4Client + return m.ipv4Client, nil } - return m.ipv6Client + if m.ipv6Client == nil { + return nil, fmt.Errorf("ipv6 is not supported") + } + return m.ipv6Client, nil } -// clientWithChain returns client with initialized chain and default rules -func (m *Manager) clientWithChain(ip net.IP) (*iptables.IPTables, error) { - client := m.client(ip) +// client returns client with initialized chain and default rules +func (m *Manager) client(ip net.IP) (*iptables.IPTables, error) { + client, err := m.rawClient(ip) + if err != nil { + return nil, err + } + ok, err := client.ChainExists("filter", ChainFilterName) if err != nil { - return nil, fmt.Errorf("failed to check if chain exists: %s", err) + return nil, fmt.Errorf("failed to check if chain exists: %w", err) } if !ok { if err := client.NewChain("filter", ChainFilterName); err != nil { - return nil, fmt.Errorf("failed to create chain: %s", err) + return nil, fmt.Errorf("failed to create chain: %w", err) } - specs := []string{"-p", "icmp", "--icmp-type", "echo-request", "-j", "ACCEPT"} - if err := client.Insert("input", ChainFilterName, 1, specs...); err != nil { - return nil, fmt.Errorf("failed to create chain: %s", err) + if err := client.AppendUnique("filter", ChainFilterName, pingSupportDefaultRule...); err != nil { + return nil, fmt.Errorf("failed to create default ping allow rule: %w", err) } + + specs := append([]string{"-i", m.wgIfaceName}, jumpNetbirdDefaultRule...) + if err := client.AppendUnique("filter", "INPUT", specs...); err != nil { + return nil, fmt.Errorf("failed to create chain: %w", err) + } + } return client, nil } diff --git a/client/firewall/iptables/manager_linux_test.go b/client/firewall/iptables/manager_linux_test.go index 5d175834c..4dc68d4e5 100644 --- a/client/firewall/iptables/manager_linux_test.go +++ b/client/firewall/iptables/manager_linux_test.go @@ -14,7 +14,8 @@ func TestNewManager(t *testing.T) { t.Fatal(err) } - manager, err := Create() + // just check on the local interface + manager, err := Create("lo") if err != nil { t.Fatal(err) } @@ -26,6 +27,7 @@ func TestNewManager(t *testing.T) { rule1, err = manager.AddFiltering(ip, "tcp", port, fw.DirectionDst, fw.ActionAccept, "accept HTTP traffic") if err != nil { t.Errorf("failed to add rule: %v", err) + return } checkRuleSpecs(t, ipv4Client, true, rule1.(*Rule).specs...) @@ -41,6 +43,7 @@ func TestNewManager(t *testing.T) { ip, "tcp", port, fw.DirectionDst, fw.ActionAccept, "accept HTTPS traffic from ports range") if err != nil { t.Errorf("failed to add rule: %v", err) + return } checkRuleSpecs(t, ipv4Client, true, rule2.(*Rule).specs...) @@ -57,6 +60,7 @@ func TestNewManager(t *testing.T) { t.Run("delete second rule", func(t *testing.T) { if err := manager.DeleteRule(rule2); err != nil { t.Errorf("failed to delete rule: %v", err) + return } checkRuleSpecs(t, ipv4Client, false, rule2.(*Rule).specs...) @@ -69,15 +73,18 @@ func TestNewManager(t *testing.T) { _, err = manager.AddFiltering(ip, "udp", port, fw.DirectionDst, fw.ActionAccept, "accept Fake DNS traffic") if err != nil { t.Errorf("failed to add rule: %v", err) + return } if err := manager.Reset(); err != nil { t.Errorf("failed to reset: %v", err) + return } ok, err := ipv4Client.ChainExists("filter", ChainFilterName) if err != nil { t.Errorf("failed to drop chain: %v", err) + return } if ok { diff --git a/client/internal/engine.go b/client/internal/engine.go index 0640ceedf..29176b88e 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -223,7 +223,7 @@ func (e *Engine) Start() error { e.routeManager = routemanager.NewManager(e.ctx, e.config.WgPrivateKey.PublicKey().String(), e.wgInterface, e.statusRecorder) - e.firewallManager, err = buildFirewallManager() + e.firewallManager, err = buildFirewallManager(wgIfaceName) if err != nil { log.Errorf("failed to create firewall manager, ACL policy will not work: %s", err.Error()) } else { diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 55da11aa2..fe58a8ef9 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -6,6 +6,7 @@ import ( "net" "net/netip" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -945,6 +946,12 @@ func TestEngine_firewallManager(t *testing.T) { if runtime.GOOS != "linux" { t.Skipf("firewall manager not supported in the: %s", runtime.GOOS) return + } else { + _, err := exec.LookPath("iptables") + if err != nil { + t.Skipf("iptables not found: %v", err) + return + } } ctx, cancel := context.WithTimeout(CtxInitState(context.Background()), 10*time.Second) diff --git a/client/internal/firewall.go b/client/internal/firewall.go index 16f008ff3..29ed37bb4 100644 --- a/client/internal/firewall.go +++ b/client/internal/firewall.go @@ -1,3 +1,5 @@ +//go:build !linux + package internal import ( @@ -5,15 +7,8 @@ import ( "runtime" "github.com/netbirdio/netbird/client/firewall" - "github.com/netbirdio/netbird/client/firewall/iptables" ) -func buildFirewallManager() (fw firewall.Manager, err error) { - switch runtime.GOOS { - case "linux": - return iptables.Create() - - default: - return nil, fmt.Errorf("not implemented for this OS: %s", runtime.GOOS) - } +func buildFirewallManager(wgIfaceName string) (fw firewall.Manager, err error) { + return nil, fmt.Errorf("not implemented for this OS: %s", runtime.GOOS) } diff --git a/client/internal/firewall_linux.go b/client/internal/firewall_linux.go new file mode 100644 index 000000000..f6c541b86 --- /dev/null +++ b/client/internal/firewall_linux.go @@ -0,0 +1,18 @@ +package internal + +import ( + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/firewall" + "github.com/netbirdio/netbird/client/firewall/iptables" +) + +// buildFirewallManager creates a firewall manager instance for the Linux +func buildFirewallManager(wgIfaceName string) (firewall.Manager, error) { + fw, err := iptables.Create(wgIfaceName) + if err != nil { + log.Debugf("failed to create iptables manager: %s", err) + return nil, err + } + return fw, nil +}