From 4857d10da1a2efa2a927672b45aaf8cde0b3b995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Serv=C3=A9n=20Mar=C3=ADn?= Date: Thu, 20 Feb 2020 12:24:52 +0100 Subject: [PATCH] pkg/iptables: clean up, remove NAT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit cleans up the iptables package to allow other packages to create rules. This commit also removes all NAT from Kilo. Signed-off-by: Lucas Servén Marín --- pkg/iptables/fake.go | 12 +-- pkg/iptables/iptables.go | 143 +++++++++++++--------------------- pkg/iptables/iptables_test.go | 25 +++--- pkg/mesh/cni.go | 1 + pkg/mesh/mesh.go | 17 ++-- pkg/mesh/topology.go | 12 --- 6 files changed, 77 insertions(+), 133 deletions(-) diff --git a/pkg/iptables/fake.go b/pkg/iptables/fake.go index 98b28fd..9c0156b 100644 --- a/pkg/iptables/fake.go +++ b/pkg/iptables/fake.go @@ -41,7 +41,7 @@ type fakeClient struct { storage []Rule } -var _ iptablesClient = &fakeClient{} +var _ Client = &fakeClient{} func (f *fakeClient) AppendUnique(table, chain string, spec ...string) error { exists, err := f.Exists(table, chain, spec...) @@ -51,12 +51,12 @@ func (f *fakeClient) AppendUnique(table, chain string, spec ...string) error { if exists { return nil } - f.storage = append(f.storage, &rule{table, chain, spec, nil}) + f.storage = append(f.storage, &rule{table, chain, spec}) return nil } func (f *fakeClient) Delete(table, chain string, spec ...string) error { - r := &rule{table, chain, spec, nil} + r := &rule{table, chain, spec} for i := range f.storage { if f.storage[i].String() == r.String() { copy(f.storage[i:], f.storage[i+1:]) @@ -69,7 +69,7 @@ func (f *fakeClient) Delete(table, chain string, spec ...string) error { } func (f *fakeClient) Exists(table, chain string, spec ...string) (bool, error) { - r := &rule{table, chain, spec, nil} + r := &rule{table, chain, spec} for i := range f.storage { if f.storage[i].String() == r.String() { return true, nil @@ -103,7 +103,7 @@ func (f *fakeClient) DeleteChain(table, name string) error { return fmt.Errorf("cannot delete chain %s; rules exist", name) } } - c := &chain{table, name, nil} + c := &chain{table, name} for i := range f.storage { if f.storage[i].String() == c.String() { copy(f.storage[i:], f.storage[i+1:]) @@ -116,7 +116,7 @@ func (f *fakeClient) DeleteChain(table, name string) error { } func (f *fakeClient) NewChain(table, name string) error { - c := &chain{table, name, nil} + c := &chain{table, name} for i := range f.storage { if f.storage[i].String() == c.String() { return statusError(1) diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index aec9808..56b9442 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -24,39 +24,47 @@ import ( "github.com/coreos/go-iptables/iptables" ) -type iptablesClient interface { - AppendUnique(string, string, ...string) error - Delete(string, string, ...string) error - Exists(string, string, ...string) (bool, error) - ClearChain(string, string) error - DeleteChain(string, string) error - NewChain(string, string) error +// Client represents any type that can administer iptables rules. +type Client interface { + AppendUnique(table string, chain string, rule ...string) error + Delete(table string, chain string, rule ...string) error + Exists(table string, chain string, rule ...string) (bool, error) + ClearChain(table string, chain string) error + DeleteChain(table string, chain string) error + NewChain(table string, chain string) error +} + +// Rule is an interface for interacting with iptables objects. +type Rule interface { + Add(Client) error + Delete(Client) error + Exists(Client) (bool, error) + String() string } // rule represents an iptables rule. type rule struct { - table string - chain string - spec []string - client iptablesClient + table string + chain string + spec []string } -func (r *rule) Add() error { - if err := r.client.AppendUnique(r.table, r.chain, r.spec...); err != nil { +func (r *rule) Add(client Client) error { + if err := client.AppendUnique(r.table, r.chain, r.spec...); err != nil { return fmt.Errorf("failed to add iptables rule: %v", err) } return nil } -func (r *rule) Delete() error { +func (r *rule) Delete(client Client) error { // Ignore the returned error as an error likely means // that the rule doesn't exist, which is fine. - r.client.Delete(r.table, r.chain, r.spec...) + client.Delete(r.table, r.chain, r.spec...) return nil } -func (r *rule) Exists() (bool, error) { - return r.client.Exists(r.table, r.chain, r.spec...) +func (r *rule) Exists(client Client) (bool, error) { + return client.Exists(r.table, r.chain, r.spec...) } func (r *rule) String() string { @@ -68,39 +76,38 @@ func (r *rule) String() string { // chain represents an iptables chain. type chain struct { - table string - chain string - client iptablesClient + table string + chain string } -func (c *chain) Add() error { - if err := c.client.ClearChain(c.table, c.chain); err != nil { +func (c *chain) Add(client Client) error { + if err := client.ClearChain(c.table, c.chain); err != nil { return fmt.Errorf("failed to add iptables chain: %v", err) } return nil } -func (c *chain) Delete() error { +func (c *chain) Delete(client Client) error { // The chain must be empty before it can be deleted. - if err := c.client.ClearChain(c.table, c.chain); err != nil { + if err := client.ClearChain(c.table, c.chain); err != nil { return fmt.Errorf("failed to clear iptables chain: %v", err) } // Ignore the returned error as an error likely means // that the chain doesn't exist, which is fine. - c.client.DeleteChain(c.table, c.chain) + client.DeleteChain(c.table, c.chain) return nil } -func (c *chain) Exists() (bool, error) { +func (c *chain) Exists(client Client) (bool, error) { // The code for "chain already exists". existsErr := 1 - err := c.client.NewChain(c.table, c.chain) + err := client.NewChain(c.table, c.chain) se, ok := err.(statusExiter) switch { case err == nil: // If there was no error adding a new chain, then it did not exist. // Delete it and return false. - c.client.DeleteChain(c.table, c.chain) + client.DeleteChain(c.table, c.chain) return false, nil case ok && se.ExitStatus() == existsErr: return true, nil @@ -116,17 +123,9 @@ func (c *chain) String() string { return fmt.Sprintf("%s_%s", c.table, c.chain) } -// Rule is an interface for interacting with iptables objects. -type Rule interface { - Add() error - Delete() error - Exists() (bool, error) - String() string -} - // Controller is able to reconcile a given set of iptables rules. type Controller struct { - client iptablesClient + client Client errors chan error sync.Mutex @@ -187,12 +186,12 @@ func (c *Controller) reconcile() error { c.Lock() defer c.Unlock() for i, r := range c.rules { - ok, err := r.Exists() + ok, err := r.Exists(c.client) if err != nil { return fmt.Errorf("failed to check if rule exists: %v", err) } if !ok { - if err := resetFromIndex(i, c.rules); err != nil { + if err := c.resetFromIndex(i, c.rules); err != nil { return fmt.Errorf("failed to add rule: %v", err) } break @@ -202,15 +201,15 @@ func (c *Controller) reconcile() error { } // resetFromIndex re-adds all rules starting from the given index. -func resetFromIndex(i int, rules []Rule) error { +func (c *Controller) resetFromIndex(i int, rules []Rule) error { if i >= len(rules) { return nil } for j := i; j < len(rules); j++ { - if err := rules[j].Delete(); err != nil { + if err := rules[j].Delete(c.client); err != nil { return fmt.Errorf("failed to delete rule: %v", err) } - if err := rules[j].Add(); err != nil { + if err := rules[j].Add(c.client); err != nil { return fmt.Errorf("failed to add rule: %v", err) } } @@ -218,12 +217,12 @@ func resetFromIndex(i int, rules []Rule) error { } // deleteFromIndex deletes all rules starting from the given index. -func deleteFromIndex(i int, rules *[]Rule) error { +func (c *Controller) deleteFromIndex(i int, rules *[]Rule) error { if i >= len(*rules) { return nil } for j := i; j < len(*rules); j++ { - if err := (*rules)[j].Delete(); err != nil { + if err := (*rules)[j].Delete(c.client); err != nil { return fmt.Errorf("failed to delete rule: %v", err) } (*rules)[j] = nil @@ -241,42 +240,41 @@ func (c *Controller) Set(rules []Rule) error { for ; i < len(rules); i++ { if i < len(c.rules) { if rules[i].String() != c.rules[i].String() { - if err := deleteFromIndex(i, &c.rules); err != nil { + if err := c.deleteFromIndex(i, &c.rules); err != nil { return err } } } if i >= len(c.rules) { - setRuleClient(rules[i], c.client) - if err := rules[i].Add(); err != nil { + if err := rules[i].Add(c.client); err != nil { return fmt.Errorf("failed to add rule: %v", err) } c.rules = append(c.rules, rules[i]) } } - return deleteFromIndex(i, &c.rules) + return c.deleteFromIndex(i, &c.rules) } // CleanUp will clean up any rules created by the controller. func (c *Controller) CleanUp() error { c.Lock() defer c.Unlock() - return deleteFromIndex(0, &c.rules) + return c.deleteFromIndex(0, &c.rules) } // IPIPRules returns a set of iptables rules that are necessary // when traffic between nodes must be encapsulated with IPIP. func IPIPRules(nodes []*net.IPNet) []Rule { var rules []Rule - rules = append(rules, &chain{"filter", "KILO-IPIP", nil}) - rules = append(rules, &rule{"filter", "INPUT", []string{"-m", "comment", "--comment", "Kilo: jump to IPIP chain", "-p", "4", "-j", "KILO-IPIP"}, nil}) + rules = append(rules, &chain{"filter", "KILO-IPIP"}) + rules = append(rules, &rule{"filter", "INPUT", []string{"-m", "comment", "--comment", "Kilo: jump to IPIP chain", "-p", "4", "-j", "KILO-IPIP"}}) for _, n := range nodes { // Accept encapsulated traffic from peers. - rules = append(rules, &rule{"filter", "KILO-IPIP", []string{"-m", "comment", "--comment", "Kilo: allow IPIP traffic", "-s", n.IP.String(), "-j", "ACCEPT"}, nil}) + rules = append(rules, &rule{"filter", "KILO-IPIP", []string{"-m", "comment", "--comment", "Kilo: allow IPIP traffic", "-s", n.IP.String(), "-j", "ACCEPT"}}) } // Drop all other IPIP traffic. - rules = append(rules, &rule{"filter", "INPUT", []string{"-m", "comment", "--comment", "Kilo: reject other IPIP traffic", "-p", "4", "-j", "DROP"}, nil}) + rules = append(rules, &rule{"filter", "INPUT", []string{"-m", "comment", "--comment", "Kilo: reject other IPIP traffic", "-p", "4", "-j", "DROP"}}) return rules } @@ -289,51 +287,16 @@ func ForwardRules(subnets ...*net.IPNet) []Rule { s := subnet.String() rules = append(rules, []Rule{ // Forward traffic to and from the overlay. - &rule{"filter", "FORWARD", []string{"-s", s, "-j", "ACCEPT"}, nil}, - &rule{"filter", "FORWARD", []string{"-d", s, "-j", "ACCEPT"}, nil}, + &rule{"filter", "FORWARD", []string{"-s", s, "-j", "ACCEPT"}}, + &rule{"filter", "FORWARD", []string{"-d", s, "-j", "ACCEPT"}}, }...) } return rules } -// MasqueradeRules returns a set of iptables rules that are necessary -// to NAT traffic from the local Pod subnet to the Internet and out of the Kilo interface. -func MasqueradeRules(kilo, private, localPodSubnet *net.IPNet, remotePodSubnet, peers []*net.IPNet) []Rule { - var rules []Rule - rules = append(rules, &chain{"nat", "KILO-NAT", nil}) - - // NAT packets from Kilo interface. - rules = append(rules, &rule{"mangle", "PREROUTING", []string{"-m", "comment", "--comment", "Kilo: jump to mark chain", "-i", "kilo+", "-j", "MARK", "--set-xmark", "0x1107/0x1107"}, nil}) - rules = append(rules, &rule{"nat", "POSTROUTING", []string{"-m", "comment", "--comment", "Kilo: NAT packets from Kilo interface", "-m", "mark", "--mark", "0x1107/0x1107", "-j", "KILO-NAT"}, nil}) - - // NAT packets from pod subnet. - rules = append(rules, &rule{"nat", "POSTROUTING", []string{"-m", "comment", "--comment", "Kilo: jump to NAT chain", "-s", localPodSubnet.String(), "-j", "KILO-NAT"}, nil}) - rules = append(rules, &rule{"nat", "KILO-NAT", []string{"-m", "comment", "--comment", "Kilo: do not NAT packets destined for the local Pod subnet", "-d", localPodSubnet.String(), "-j", "RETURN"}, nil}) - rules = append(rules, &rule{"nat", "KILO-NAT", []string{"-m", "comment", "--comment", "Kilo: do not NAT packets destined for the Kilo subnet", "-d", kilo.String(), "-j", "RETURN"}, nil}) - rules = append(rules, &rule{"nat", "KILO-NAT", []string{"-m", "comment", "--comment", "Kilo: do not NAT packets destined for the local private IP", "-d", private.String(), "-j", "RETURN"}, nil}) - for _, r := range remotePodSubnet { - rules = append(rules, &rule{"nat", "KILO-NAT", []string{"-m", "comment", "--comment", "Kilo: do not NAT packets from local pod subnet to remote pod subnets", "-s", localPodSubnet.String(), "-d", r.String(), "-j", "RETURN"}, nil}) - } - for _, p := range peers { - rules = append(rules, &rule{"nat", "KILO-NAT", []string{"-m", "comment", "--comment", "Kilo: do not NAT packets from local pod subnet to peers", "-s", localPodSubnet.String(), "-d", p.String(), "-j", "RETURN"}, nil}) - } - rules = append(rules, &rule{"nat", "KILO-NAT", []string{"-m", "comment", "--comment", "Kilo: NAT remaining packets", "-j", "MASQUERADE"}, nil}) - return rules -} - func nonBlockingSend(errors chan<- error, err error) { select { case errors <- err: default: } } - -// setRuleClient is a helper to set the iptables client on different kinds of rules. -func setRuleClient(r Rule, c iptablesClient) { - switch v := r.(type) { - case *rule: - v.client = c - case *chain: - v.client = c - } -} diff --git a/pkg/iptables/iptables_test.go b/pkg/iptables/iptables_test.go index ef5eeb2..e3ee749 100644 --- a/pkg/iptables/iptables_test.go +++ b/pkg/iptables/iptables_test.go @@ -19,8 +19,8 @@ import ( ) var rules = []Rule{ - &rule{"filter", "FORWARD", []string{"-s", "10.4.0.0/16", "-j", "ACCEPT"}, nil}, - &rule{"filter", "FORWARD", []string{"-d", "10.4.0.0/16", "-j", "ACCEPT"}, nil}, + &rule{"filter", "FORWARD", []string{"-s", "10.4.0.0/16", "-j", "ACCEPT"}}, + &rule{"filter", "FORWARD", []string{"-d", "10.4.0.0/16", "-j", "ACCEPT"}}, } func TestSet(t *testing.T) { @@ -28,7 +28,7 @@ func TestSet(t *testing.T) { name string sets [][]Rule out []Rule - actions []func(iptablesClient) error + actions []func(Client) error }{ { name: "empty", @@ -61,14 +61,12 @@ func TestSet(t *testing.T) { {rules[0], rules[1]}, }, out: []Rule{rules[0], rules[1]}, - actions: []func(c iptablesClient) error{ - func(c iptablesClient) error { - setRuleClient(rules[0], c) - return rules[0].Delete() + actions: []func(c Client) error{ + func(c Client) error { + return rules[0].Delete(c) }, - func(c iptablesClient) error { - setRuleClient(rules[1], c) - return rules[1].Delete() + func(c Client) error { + return rules[1].Delete(c) }, }, }, @@ -78,10 +76,9 @@ func TestSet(t *testing.T) { {rules[0], rules[1]}, }, out: []Rule{rules[0], rules[1]}, - actions: []func(c iptablesClient) error{ - func(c iptablesClient) error { - setRuleClient(rules[0], c) - return rules[0].Delete() + actions: []func(c Client) error{ + func(c Client) error { + return rules[0].Delete(c) }, }, }, diff --git a/pkg/mesh/cni.go b/pkg/mesh/cni.go index d6fe2d1..a69cb7f 100644 --- a/pkg/mesh/cni.go +++ b/pkg/mesh/cni.go @@ -68,6 +68,7 @@ func (m *Mesh) updateCNIConfig() { level.Info(m.logger).Log("msg", "CIDR in CNI file is not empty; overwriting", "old", cidr.String(), "new", n.Subnet.String()) } + level.Info(m.logger).Log("msg", "setting CIDR in CNI file", "CIDR", n.Subnet.String()) if err := setCIDRInCNI(m.cniPath, n.Subnet); err != nil { level.Warn(m.logger).Log("msg", "failed to set CIDR in CNI file", "err", err.Error()) } diff --git a/pkg/mesh/mesh.go b/pkg/mesh/mesh.go index b47a0ea..7bed151 100644 --- a/pkg/mesh/mesh.go +++ b/pkg/mesh/mesh.go @@ -605,12 +605,13 @@ func (m *Mesh) applyTopology() { return } rules := iptables.ForwardRules(m.subnet) - var peerCIDRs []*net.IPNet - for _, p := range peers { - rules = append(rules, iptables.ForwardRules(p.AllowedIPs...)...) - peerCIDRs = append(peerCIDRs, p.AllowedIPs...) + // Finx the Kilo interface name. + link, err := linkByIndex(m.kiloIface) + if err != nil { + level.Error(m.logger).Log("error", err) + m.errorCounter.WithLabelValues("apply").Inc() + return } - rules = append(rules, iptables.MasqueradeRules(m.subnet, oneAddressCIDR(t.privateIP.IP), nodes[m.hostname].Subnet, t.RemoteSubnets(), peerCIDRs)...) // If we are handling local routes, ensure the local // tunnel has an IP address and IPIP traffic is allowed. if m.enc.Strategy() != encapsulation.Never && m.local { @@ -644,12 +645,6 @@ func (m *Mesh) applyTopology() { m.errorCounter.WithLabelValues("apply").Inc() return } - link, err := linkByIndex(m.kiloIface) - if err != nil { - level.Error(m.logger).Log("error", err) - m.errorCounter.WithLabelValues("apply").Inc() - return - } oldConf, err := wireguard.ShowConf(link.Attrs().Name) if err != nil { level.Error(m.logger).Log("error", err) diff --git a/pkg/mesh/topology.go b/pkg/mesh/topology.go index b8697d4..c4014c6 100644 --- a/pkg/mesh/topology.go +++ b/pkg/mesh/topology.go @@ -163,18 +163,6 @@ func NewTopology(nodes map[string]*Node, peers map[string]*Peer, granularity Gra return &t, nil } -// RemoteSubnets identifies the subnets of the hosts in segments different than the host's. -func (t *Topology) RemoteSubnets() []*net.IPNet { - var remote []*net.IPNet - for _, s := range t.segments { - if s == nil || s.location == t.location { - continue - } - remote = append(remote, s.cidrs...) - } - return remote -} - // Routes generates a slice of routes for a given Topology. func (t *Topology) Routes(kiloIface, privIface, tunlIface int, local bool, enc encapsulation.Encapsulator) []*netlink.Route { var routes []*netlink.Route