Lean iptables updates (#324)

* Split iptables rules into append and prepend rules

* fix existing tests

* add iptables tests which include prepend rules

* fix iptables test storage usage

* add test to check reconcile behaviour

* Reconcile prepend rules

* Make usage of `RuleSet` prettier

* Properly implement `Insert` for fake enc

* Make `Rule.Prepend()` behave like `Rule.Append()` regarding uniqueness

* Implement `Insert()` for `metricsClientWrapper`

* Use `iptables.InsertUnique()` instead of `iptables.Insert()`

---------

Co-authored-by: Clive Jevons <clive@jevons-it.net>

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/campoy/embedmd v1.0.0
 	github.com/containernetworking/cni v1.0.1
 	github.com/containernetworking/plugins v1.1.1
-	github.com/coreos/go-iptables v0.6.0
+	github.com/coreos/go-iptables v0.6.1-0.20220901214115-d2b8608923d1
 	github.com/go-kit/kit v0.9.0
 	github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348
 	github.com/metalmatze/signal v0.0.0-20210307161603-1c9aa721a97a

go.sum

@@ -106,8 +106,8 @@ github.com/containernetworking/plugins v1.1.1 h1:+AGfFigZ5TiQH00vhR8qPeSatj53eNG
 github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19sZPp3ry5uHSkI4LPxV8=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
-github.com/coreos/go-iptables v0.6.0 h1:is9qnZMPYjLd8LYqmm/qlE+wwEgJIkTYdhV3rfZo4jk=
-github.com/coreos/go-iptables v0.6.0/go.mod h1:Qe8Bv2Xik5FyTXwgIbLAnv2sWSBmvWdFETJConOQ//Q=
+github.com/coreos/go-iptables v0.6.1-0.20220901214115-d2b8608923d1 h1:zSiUKnogKeEwIIeUQP/WPH7m0BJ/IvW0VyL4muaauUY=
+github.com/coreos/go-iptables v0.6.1-0.20220901214115-d2b8608923d1/go.mod h1:Qe8Bv2Xik5FyTXwgIbLAnv2sWSBmvWdFETJConOQ//Q=
 github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
 github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
 github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=

pkg/encapsulation/cilium.go

@@ -96,8 +96,8 @@ func (f *cilium) Init(_ int) error {
 }
 
 // Rules is a no-op.
-func (f *cilium) Rules(_ []*net.IPNet) []iptables.Rule {
-	return nil
+func (f *cilium) Rules(_ []*net.IPNet) iptables.RuleSet {
+	return iptables.RuleSet{}
 }
 
 // Set is a no-op.

pkg/encapsulation/encapsulation.go

@@ -49,7 +49,7 @@ type Encapsulator interface {
 	Gw(net.IP, net.IP, *net.IPNet) net.IP
 	Index() int
 	Init(int) error
-	Rules([]*net.IPNet) []iptables.Rule
+	Rules([]*net.IPNet) iptables.RuleSet
 	Set(*net.IPNet) error
 	Strategy() Strategy
 }

pkg/encapsulation/flannel.go

@@ -95,8 +95,8 @@ func (f *flannel) Init(_ int) error {
 }
 
 // Rules is a no-op.
-func (f *flannel) Rules(_ []*net.IPNet) []iptables.Rule {
-	return nil
+func (f *flannel) Rules(_ []*net.IPNet) iptables.RuleSet {
+	return iptables.RuleSet{}
 }
 
 // Set is a no-op.

pkg/encapsulation/ipip.go

@@ -65,20 +65,20 @@ func (i *ipip) Init(base int) error {
 
 // Rules returns a set of iptables rules that are necessary
 // when traffic between nodes must be encapsulated.
-func (i *ipip) Rules(nodes []*net.IPNet) []iptables.Rule {
-	var rules []iptables.Rule
+func (i *ipip) Rules(nodes []*net.IPNet) iptables.RuleSet {
+	rules := iptables.RuleSet{}
 	proto := ipipProtocolName()
-	rules = append(rules, iptables.NewIPv4Chain("filter", "KILO-IPIP"))
-	rules = append(rules, iptables.NewIPv6Chain("filter", "KILO-IPIP"))
-	rules = append(rules, iptables.NewIPv4Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: jump to IPIP chain", "-j", "KILO-IPIP"))
-	rules = append(rules, iptables.NewIPv6Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: jump to IPIP chain", "-j", "KILO-IPIP"))
+	rules.AddToAppend(iptables.NewIPv4Chain("filter", "KILO-IPIP"))
+	rules.AddToAppend(iptables.NewIPv6Chain("filter", "KILO-IPIP"))
+	rules.AddToAppend(iptables.NewIPv4Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: jump to IPIP chain", "-j", "KILO-IPIP"))
+	rules.AddToAppend(iptables.NewIPv6Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: jump to IPIP chain", "-j", "KILO-IPIP"))
 	for _, n := range nodes {
 		// Accept encapsulated traffic from peers.
-		rules = append(rules, iptables.NewRule(iptables.GetProtocol(n.IP), "filter", "KILO-IPIP", "-s", n.String(), "-m", "comment", "--comment", "Kilo: allow IPIP traffic", "-j", "ACCEPT"))
+		rules.AddToPrepend(iptables.NewRule(iptables.GetProtocol(n.IP), "filter", "KILO-IPIP", "-s", n.String(), "-m", "comment", "--comment", "Kilo: allow IPIP traffic", "-j", "ACCEPT"))
 	}
 	// Drop all other IPIP traffic.
-	rules = append(rules, iptables.NewIPv4Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: reject other IPIP traffic", "-j", "DROP"))
-	rules = append(rules, iptables.NewIPv6Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: reject other IPIP traffic", "-j", "DROP"))
+	rules.AddToAppend(iptables.NewIPv4Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: reject other IPIP traffic", "-j", "DROP"))
+	rules.AddToAppend(iptables.NewIPv6Rule("filter", "INPUT", "-p", proto, "-m", "comment", "--comment", "Kilo: reject other IPIP traffic", "-j", "DROP"))
 
 	return rules
 }

pkg/encapsulation/noop.go

@@ -44,8 +44,8 @@ func (n Noop) Init(_ int) error {
 }
 
 // Rules will also do nothing.
-func (n Noop) Rules(_ []*net.IPNet) []iptables.Rule {
-	return nil
+func (n Noop) Rules(_ []*net.IPNet) iptables.RuleSet {
+	return iptables.RuleSet{}
 }
 
 // Set will also do nothing.

pkg/iptables/fake.go

@@ -46,6 +46,24 @@ type fakeClient struct {
 
 var _ Client = &fakeClient{}
 
+func (f *fakeClient) InsertUnique(table, chain string, pos int, spec ...string) error {
+	atomic.AddUint64(&f.calls, 1)
+	exists, err := f.Exists(table, chain, spec...)
+	if err != nil {
+		return err
+	}
+	if exists {
+		return nil
+	}
+	index := pos - 1 // iptables are 1-based
+	rule := &rule{table: table, chain: chain, spec: spec}
+	prefix := append([]Rule{}, f.storage[:index]...)
+	suffix := append([]Rule{}, f.storage[index:]...)
+	prefix = append(prefix, rule)
+	f.storage = append(prefix, suffix...)
+	return nil
+}
+
 func (f *fakeClient) AppendUnique(table, chain string, spec ...string) error {
 	atomic.AddUint64(&f.calls, 1)
 	exists, err := f.Exists(table, chain, spec...)

pkg/iptables/iptables.go

@@ -46,13 +46,33 @@ func ipv6Disabled() (bool, error) {
 // Protocol represents an IP protocol.
 type Protocol byte
 
+type RuleSet struct {
+	appendRules  []Rule // Rules to append to the chain - order matters.
+	prependRules []Rule // Rules to prepend to the chain - order does not matter.
+}
+
 const (
 	// ProtocolIPv4 represents the IPv4 protocol.
 	ProtocolIPv4 Protocol = iota
 	// ProtocolIPv6 represents the IPv6 protocol.
 	ProtocolIPv6
 )
 
+func (rs *RuleSet) AddToAppend(rule Rule) {
+	rs.appendRules = append(rs.appendRules, rule)
+}
+
+func (rs *RuleSet) AddToPrepend(rule Rule) {
+	rs.prependRules = append(rs.prependRules, rule)
+}
+
+func (rs *RuleSet) AppendRuleSet(other RuleSet) RuleSet {
+	return RuleSet{
+		appendRules:  append(rs.appendRules, other.appendRules...),
+		prependRules: append(rs.prependRules, other.prependRules...),
+	}
+}
+
 // GetProtocol will return a protocol from the length of an IP address.
 func GetProtocol(ip net.IP) Protocol {
 	if len(ip) == net.IPv4len || ip.To4() != nil {
@@ -64,6 +84,7 @@ func GetProtocol(ip net.IP) Protocol {
 // Client represents any type that can administer iptables rules.
 type Client interface {
 	AppendUnique(table string, chain string, rule ...string) error
+	InsertUnique(table, chain string, pos int, rule ...string) error
 	Delete(table string, chain string, rule ...string) error
 	Exists(table string, chain string, rule ...string) (bool, error)
 	List(table string, chain string) ([]string, error)
@@ -75,7 +96,8 @@ type Client interface {
 
 // Rule is an interface for interacting with iptables objects.
 type Rule interface {
-	Add(Client) error
+	Append(Client) error
+	Prepend(Client) error
 	Delete(Client) error
 	Exists(Client) (bool, error)
 	String() string
@@ -106,7 +128,14 @@ func NewIPv6Rule(table, chain string, spec ...string) Rule {
 	return &rule{table, chain, spec, ProtocolIPv6}
 }
 
-func (r *rule) Add(client Client) error {
+func (r *rule) Prepend(client Client) error {
+	if err := client.InsertUnique(r.table, r.chain, 1, r.spec...); err != nil {
+		return fmt.Errorf("failed to add iptables rule: %v", err)
+	}
+	return nil
+}
+
+func (r *rule) Append(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)
 	}
@@ -162,7 +191,11 @@ func NewIPv6Chain(table, name string) Rule {
 	return &chain{table, name, ProtocolIPv6}
 }
 
-func (c *chain) Add(client Client) error {
+func (c *chain) Prepend(client Client) error {
+	return c.Append(client)
+}
+
+func (c *chain) Append(client Client) error {
 	// Note: `ClearChain` creates a chain if it does not exist.
 	if err := client.ClearChain(c.table, c.chain); err != nil {
 		return fmt.Errorf("failed to add iptables chain: %v", err)
@@ -224,8 +257,9 @@ type Controller struct {
 	registerer   prometheus.Registerer
 
 	sync.Mutex
-	rules      []Rule
-	subscribed bool
+	appendRules  []Rule
+	prependRules []Rule
+	subscribed   bool
 }
 
 // ControllerOption modifies the controller's configuration.
@@ -333,14 +367,21 @@ func (c *Controller) reconcile() error {
 	c.Lock()
 	defer c.Unlock()
 	var rc ruleCache
-	for i, r := range c.rules {
+	if err := c.reconcileAppendRules(rc); err != nil {
+		return err
+	}
+	return c.reconcilePrependRules(rc)
+}
+
+func (c *Controller) reconcileAppendRules(rc ruleCache) error {
+	for i, r := range c.appendRules {
 		ok, err := rc.exists(c.client(r.Proto()), r)
 		if err != nil {
 			return fmt.Errorf("failed to check if rule exists: %v", err)
 		}
 		if !ok {
-			level.Info(c.logger).Log("msg", fmt.Sprintf("applying %d iptables rules", len(c.rules)-i))
-			if err := c.resetFromIndex(i, c.rules); err != nil {
+			level.Info(c.logger).Log("msg", fmt.Sprintf("applying %d iptables rules", len(c.appendRules)-i))
+			if err := c.resetFromIndex(i, c.appendRules); err != nil {
 				return fmt.Errorf("failed to add rule: %v", err)
 			}
 			break
@@ -349,6 +390,22 @@ func (c *Controller) reconcile() error {
 	return nil
 }
 
+func (c *Controller) reconcilePrependRules(rc ruleCache) error {
+	for _, r := range c.prependRules {
+		ok, err := rc.exists(c.client(r.Proto()), r)
+		if err != nil {
+			return fmt.Errorf("failed to check if rule exists: %v", err)
+		}
+		if !ok {
+			level.Info(c.logger).Log("msg", "prepending iptables rule")
+			if err := r.Prepend(c.client(r.Proto())); err != nil {
+				return fmt.Errorf("failed to prepend rule: %v", err)
+			}
+		}
+	}
+	return nil
+}
+
 // resetFromIndex re-adds all rules starting from the given index.
 func (c *Controller) resetFromIndex(i int, rules []Rule) error {
 	if i >= len(rules) {
@@ -358,7 +415,7 @@ func (c *Controller) resetFromIndex(i int, rules []Rule) error {
 		if err := rules[j].Delete(c.client(rules[j].Proto())); err != nil {
 			return fmt.Errorf("failed to delete rule: %v", err)
 		}
-		if err := rules[j].Add(c.client(rules[j].Proto())); err != nil {
+		if err := rules[j].Append(c.client(rules[j].Proto())); err != nil {
 			return fmt.Errorf("failed to add rule: %v", err)
 		}
 	}
@@ -383,34 +440,87 @@ func (c *Controller) deleteFromIndex(i int, rules *[]Rule) error {
 
 // Set idempotently overwrites any iptables rules previously defined
 // for the controller with the given set of rules.
-func (c *Controller) Set(rules []Rule) error {
+func (c *Controller) Set(rules RuleSet) error {
 	c.Lock()
 	defer c.Unlock()
+	if err := c.setAppendRules(rules.appendRules); err != nil {
+		return err
+	}
+	return c.setPrependRules(rules.prependRules)
+}
+
+func (c *Controller) setAppendRules(appendRules []Rule) error {
 	var i int
-	for ; i < len(rules); i++ {
-		if i < len(c.rules) {
-			if rules[i].String() != c.rules[i].String() {
-				if err := c.deleteFromIndex(i, &c.rules); err != nil {
+	for ; i < len(appendRules); i++ {
+		if i < len(c.appendRules) {
+			if appendRules[i].String() != c.appendRules[i].String() {
+				if err := c.deleteFromIndex(i, &c.appendRules); err != nil {
 					return err
 				}
 			}
 		}
-		if i >= len(c.rules) {
-			if err := rules[i].Add(c.client(rules[i].Proto())); err != nil {
+		if i >= len(c.appendRules) {
+			if err := appendRules[i].Append(c.client(appendRules[i].Proto())); err != nil {
 				return fmt.Errorf("failed to add rule: %v", err)
 			}
-			c.rules = append(c.rules, rules[i])
+			c.appendRules = append(c.appendRules, appendRules[i])
 		}
+	}
+	err := c.deleteFromIndex(i, &c.appendRules)
+	if err != nil {
+		return fmt.Errorf("failed to delete rule: %v", err)
+	}
+	return nil
+}
 
+func (c *Controller) setPrependRules(prependRules []Rule) error {
+	for _, prependRule := range prependRules {
+		if !containsRule(c.prependRules, prependRule) {
+			if err := prependRule.Prepend(c.client(prependRule.Proto())); err != nil {
+				return fmt.Errorf("failed to add rule: %v", err)
+			}
+			c.prependRules = append(c.prependRules, prependRule)
+		}
 	}
-	return c.deleteFromIndex(i, &c.rules)
+	for _, existingRule := range c.prependRules {
+		if !containsRule(prependRules, existingRule) {
+			if err := existingRule.Delete(c.client(existingRule.Proto())); err != nil {
+				return fmt.Errorf("failed to delete rule: %v", err)
+			}
+			c.prependRules = removeRule(c.prependRules, existingRule)
+		}
+	}
+	return nil
+}
+
+func removeRule(rules []Rule, toRemove Rule) []Rule {
+	ret := make([]Rule, 0, len(rules))
+	for _, rule := range rules {
+		if rule.String() != toRemove.String() {
+			ret = append(ret, rule)
+		}
+	}
+	return ret
+}
+
+func containsRule(haystack []Rule, needle Rule) bool {
+	for _, element := range haystack {
+		if element.String() == needle.String() {
+			return true
+		}
+	}
+	return false
 }
 
 // CleanUp will clean up any rules created by the controller.
 func (c *Controller) CleanUp() error {
 	c.Lock()
 	defer c.Unlock()
-	return c.deleteFromIndex(0, &c.rules)
+	err := c.deleteFromIndex(0, &c.prependRules)
+	if err != nil {
+		return err
+	}
+	return c.deleteFromIndex(0, &c.appendRules)
 }
 
 func (c *Controller) client(p Protocol) Client {