fix(npc): ordering of firewall / service rules (cloudnativelabs#1144)

Author: aauren

pkg/controllers/netpol/network_policy_controller.go

@@ -35,6 +35,10 @@ const (
 	kubeDefaultNetpolChain       = "KUBE-NWPLCY-DEFAULT"
 )
 
+var (
+	defaultChains = map[string]string{"INPUT": kubeInputChainName, "FORWARD": kubeForwardChainName, "OUTPUT": kubeOutputChainName}
+)
+
 // Network policy controller provides both ingress and egress filtering for the pods as per the defined network
 // policies. Two different types of iptables chains are used. Each pod running on the node which either
 // requires ingress or egress filtering gets a pod specific chains. Each network policy has a iptables chain, which
@@ -251,6 +255,10 @@ func (npc *NetworkPolicyController) fullPolicySync() {
 		return
 	}
 
+	// Makes sure that the ACCEPT rules for packets marked with "0x20000" are added to the end of each of kube-router's
+	// top level chains
+	npc.ensureExplicitAccept()
+
 	err = npc.cleanupStaleRules(activePolicyChains, activePodFwChains, false)
 	if err != nil {
 		klog.Errorf("Aborting sync. Failed to cleanup stale iptables rules: %v", err.Error())
@@ -337,9 +345,7 @@ func (npc *NetworkPolicyController) ensureTopLevelChains() {
 		}
 	}
 
-	chains := map[string]string{"INPUT": kubeInputChainName, "FORWARD": kubeForwardChainName, "OUTPUT": kubeOutputChainName}
-
-	for builtinChain, customChain := range chains {
+	for builtinChain, customChain := range defaultChains {
 		err = iptablesCmdHandler.NewChain("filter", customChain)
 		if err != nil && err.(*iptables.Error).ExitStatus() != 1 {
 			klog.Fatalf("Failed to run iptables command to create %s chain due to %s", customChain, err.Error())
@@ -383,16 +389,15 @@ func (npc *NetworkPolicyController) ensureTopLevelChains() {
 		}
 		ensureRuleAtPosition(kubeInputChainName, whitelistServiceVips, uuid, externalIPIndex+4)
 	}
+}
 
+func (npc *NetworkPolicyController) ensureExplicitAccept() {
 	// for the traffic to/from the local pod's let network policy controller be
 	// authoritative entity to ACCEPT the traffic if it complies to network policies
-	for _, chain := range chains {
-		comment := "rule to explicitly ACCEPT traffic that comply to network policies"
+	for _, chain := range defaultChains {
+		comment := "\"rule to explicitly ACCEPT traffic that comply to network policies\""
 		args := []string{"-m", "comment", "--comment", comment, "-m", "mark", "--mark", "0x20000/0x20000", "-j", "ACCEPT"}
-		err = iptablesCmdHandler.AppendUnique("filter", chain, args...)
-		if err != nil {
-			klog.Fatalf("Failed to run iptables command: %s", err.Error())
-		}
+		npc.filterTableRules = utils.AppendUnique(npc.filterTableRules, chain, args)
 	}
 }
 

pkg/controllers/netpol/pod.go

@@ -196,19 +196,19 @@ func (npc *NetworkPolicyController) interceptPodInboundTraffic(pod *podInfo, pod
 	// this rule applies to the traffic getting routed (coming for other node pods)
 	comment := "\"rule to jump traffic destined to POD name:" + pod.name + " namespace: " + pod.namespace +
 		" to chain " + podFwChainName + "\""
-	args := []string{"-I", kubeForwardChainName, "1", "-m", "comment", "--comment", comment, "-d", pod.ip, "-j", podFwChainName + "\n"}
+	args := []string{"-A", kubeForwardChainName, "-m", "comment", "--comment", comment, "-d", pod.ip, "-j", podFwChainName + "\n"}
 	npc.filterTableRules.WriteString(strings.Join(args, " "))
 
 	// ensure there is rule in filter table and OUTPUT chain to jump to pod specific firewall chain
 	// this rule applies to the traffic from a pod getting routed back to another pod on same node by service proxy
-	args = []string{"-I", kubeOutputChainName, "1", "-m", "comment", "--comment", comment, "-d", pod.ip, "-j", podFwChainName + "\n"}
+	args = []string{"-A", kubeOutputChainName, "-m", "comment", "--comment", comment, "-d", pod.ip, "-j", podFwChainName + "\n"}
 	npc.filterTableRules.WriteString(strings.Join(args, " "))
 
 	// ensure there is rule in filter table and forward chain to jump to pod specific firewall chain
 	// this rule applies to the traffic getting switched (coming for same node pods)
 	comment = "\"rule to jump traffic destined to POD name:" + pod.name + " namespace: " + pod.namespace +
 		" to chain " + podFwChainName + "\""
-	args = []string{"-I", kubeForwardChainName, "1", "-m", "physdev", "--physdev-is-bridged",
+	args = []string{"-A", kubeForwardChainName, "-m", "physdev", "--physdev-is-bridged",
 		"-m", "comment", "--comment", comment,
 		"-d", pod.ip,
 		"-j", podFwChainName, "\n"}
@@ -218,22 +218,21 @@ func (npc *NetworkPolicyController) interceptPodInboundTraffic(pod *podInfo, pod
 // setup iptable rules to intercept outbound traffic from pods and run it across the
 // firewall chain corresponding to the pod so that egress network policies are enforced
 func (npc *NetworkPolicyController) interceptPodOutboundTraffic(pod *podInfo, podFwChainName string) {
-	egressFilterChains := []string{kubeInputChainName, kubeForwardChainName, kubeOutputChainName}
-	for _, chain := range egressFilterChains {
+	for _, chain := range defaultChains {
 		// ensure there is rule in filter table and FORWARD chain to jump to pod specific firewall chain
-		// this rule applies to the traffic getting forwarded/routed (traffic from the pod destinted
+		// this rule applies to the traffic getting forwarded/routed (traffic from the pod destined
 		// to pod on a different node)
 		comment := "\"rule to jump traffic from POD name:" + pod.name + " namespace: " + pod.namespace +
 			" to chain " + podFwChainName + "\""
-		args := []string{"-I", chain, "1", "-m", "comment", "--comment", comment, "-s", pod.ip, "-j", podFwChainName, "\n"}
+		args := []string{"-A", chain, "-m", "comment", "--comment", comment, "-s", pod.ip, "-j", podFwChainName, "\n"}
 		npc.filterTableRules.WriteString(strings.Join(args, " "))
 	}
 
 	// ensure there is rule in filter table and forward chain to jump to pod specific firewall chain
 	// this rule applies to the traffic getting switched (coming for same node pods)
 	comment := "\"rule to jump traffic from POD name:" + pod.name + " namespace: " + pod.namespace +
 		" to chain " + podFwChainName + "\""
-	args := []string{"-I", kubeForwardChainName, "1", "-m", "physdev", "--physdev-is-bridged",
+	args := []string{"-A", kubeForwardChainName, "-m", "physdev", "--physdev-is-bridged",
 		"-m", "comment", "--comment", comment,
 		"-s", pod.ip,
 		"-j", podFwChainName, "\n"}

pkg/utils/iptables.go

@@ -70,3 +70,34 @@ func Restore(table string, data []byte) error {
 
 	return nil
 }
+
+// AppendUnique ensures that rule is in chain only once in the buffer and that the occurrence is at the end of the buffer
+func AppendUnique(buffer bytes.Buffer, chain string, rule []string) bytes.Buffer {
+	var desiredBuffer bytes.Buffer
+
+	// First we need to remove any previous instances of the rule that exist, so that we can be sure that our version
+	// is unique and appended to the very end of the buffer
+	rules := strings.Split(buffer.String(), "\n")
+	if len(rules) > 0 && rules[len(rules)-1] == "" {
+		rules = rules[:len(rules)-1]
+	}
+	for _, foundRule := range rules {
+		if strings.Contains(foundRule, chain) {
+			if strings.Contains(foundRule, strings.Join(rule, " ")) {
+				continue
+			}
+		}
+		desiredBuffer.WriteString(foundRule + "\n")
+	}
+
+	// Now append the rule that we wanted to be unique
+	desiredBuffer = Append(desiredBuffer, chain, rule)
+	return desiredBuffer
+}
+
+// Append appends rule to chain at the end of buffer
+func Append(buffer bytes.Buffer, chain string, rule []string) bytes.Buffer {
+	ruleStr := strings.Join(append(append([]string{"-A", chain}, rule...), "\n"), " ")
+	buffer.WriteString(ruleStr)
+	return buffer
+}