pkg/iptables: clean up, remove NAT

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 <lserven@gmail.com>

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)

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
-	}
-}

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)
 				},
 			},
 		},

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())
 	}

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)

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