From ed2a5dca0d2d88099b98e53b16f44c3344ff8952 Mon Sep 17 00:00:00 2001 From: Aaron U'Ren Date: Fri, 13 Jan 2023 11:41:44 -0600 Subject: [PATCH] fix(NPC): don't add chains for missing family On dual-stack nodes there can still be pods that are single stack. When this happens there won't be a pod IP for a given family and if kube-router tries to add rules with a missing pod IP the iptables rules won't be formatted correctly (because it won't have a valid source or destination for that family). So rather than breaking the whole iptables-restore we warn in the logs and skip the pod policy chains for that family. --- pkg/controllers/netpol/pod.go | 77 +++++++++++++++++++++------------ pkg/controllers/netpol/utils.go | 26 ++++++++++- 2 files changed, 74 insertions(+), 29 deletions(-) diff --git a/pkg/controllers/netpol/pod.go b/pkg/controllers/netpol/pod.go index a9baad3013..1300fc2d93 100644 --- a/pkg/controllers/netpol/pod.go +++ b/pkg/controllers/netpol/pod.go @@ -78,10 +78,17 @@ func (npc *NetworkPolicyController) syncPodFirewallChains(networkPoliciesInfo [] activePodFwChains := make(map[string]bool) - dropUnmarkedTrafficRules := func(podName, podNamespace, podFwChainName string) { - for _, filterTableRules := range npc.filterTableRules { + dropUnmarkedTrafficRules := func(pod podInfo, podFwChainName string) { + for ipFamily, filterTableRules := range npc.filterTableRules { + _, err := getPodIPForFamily(pod, ipFamily) + if err != nil { + klog.V(2).Infof("unable to get address for pod: %s -- skipping drop rules for pod "+ + "(this is normal for pods that are not dual-stack)", err.Error()) + continue + } + // add rule to log the packets that will be dropped due to network policy enforcement - comment := "\"rule to log dropped traffic POD name:" + podName + " namespace: " + podNamespace + "\"" + comment := "\"rule to log dropped traffic POD name:" + pod.name + " namespace: " + pod.namespace + "\"" args := []string{"-A", podFwChainName, "-m", "comment", "--comment", comment, "-m", "mark", "!", "--mark", "0x10000/0x10000", "-j", "NFLOG", "--nflog-group", "100", "-m", "limit", "--limit", "10/minute", "--limit-burst", "10", "\n"} @@ -93,7 +100,8 @@ func (npc *NetworkPolicyController) syncPodFirewallChains(networkPoliciesInfo [] filterTableRules.WriteString(strings.Join(args, " ")) // add rule to DROP if no applicable network policy permits the traffic - comment = "\"rule to REJECT traffic destined for POD name:" + podName + " namespace: " + podNamespace + "\"" + comment = "\"rule to REJECT traffic destined for POD name:" + pod.name + " namespace: " + + pod.namespace + "\"" args = []string{"-A", podFwChainName, "-m", "comment", "--comment", comment, "-m", "mark", "!", "--mark", "0x10000/0x10000", "-j", "REJECT", "\n"} filterTableRules.WriteString(strings.Join(args, " ")) @@ -113,7 +121,17 @@ func (npc *NetworkPolicyController) syncPodFirewallChains(networkPoliciesInfo [] // ensure pod specific firewall chain exist for all the pods that need ingress firewall podFwChainName := podFirewallChainName(pod.namespace, pod.name, version) - for _, filterTableRules := range npc.filterTableRules { + for ipFamily, filterTableRules := range npc.filterTableRules { + _, err := getPodIPForFamily(pod, ipFamily) + if err != nil { + // If the pod doesn't have an address in this family we skip it here and all the various places below + // because there won't be a valid source or destination address for iptables, and it will stop iptables + // restore actions from completing successfully + klog.Infof("unable to get address for pod: %s -- skipping pod chain for pod "+ + "(this is normal for pods that are not dual-stack)", err.Error()) + continue + } + filterTableRules.WriteString(":" + podFwChainName + "\n") } @@ -128,9 +146,16 @@ func (npc *NetworkPolicyController) syncPodFirewallChains(networkPoliciesInfo [] // setup rules to intercept inbound traffic to the pods npc.interceptPodOutboundTraffic(pod, podFwChainName) - dropUnmarkedTrafficRules(pod.name, pod.namespace, podFwChainName) + dropUnmarkedTrafficRules(pod, podFwChainName) + + for ipFamily, filterTableRules := range npc.filterTableRules { + _, err := getPodIPForFamily(pod, ipFamily) + if err != nil { + klog.V(2).Infof("unable to get address for pod: %s -- skipping accept rules for pod "+ + "(this is normal for pods that are not dual-stack)", err.Error()) + continue + } - for _, filterTableRules := range npc.filterTableRules { // set mark to indicate traffic from/to the pod passed network policies. // Mark will be checked to explicitly ACCEPT the traffic comment := "\"set mark to ACCEPT traffic that comply to network policies\"" @@ -151,13 +176,13 @@ func (npc *NetworkPolicyController) setupPodNetpolRules(pod podInfo, podFwChainN hasEgressPolicy := false for ipFamily, filterTableRules := range npc.filterTableRules { - var ip string - switch ipFamily { - case api.IPv4Protocol: - ip, _ = getPodIPv4Address(pod) - case api.IPv6Protocol: - ip, _ = getPodIPv6Address(pod) + ip, err := getPodIPForFamily(pod, ipFamily) + if err != nil { + klog.V(2).Infof("unable to get address for pod: %s -- skipping iptables policy for pod "+ + "(this is normal for pods that are not dual-stack)", err.Error()) + continue } + // add entries in pod firewall to run through applicable network policies for _, policy := range networkPoliciesInfo { if _, ok := policy.targetPods[pod.ip]; !ok { @@ -187,7 +212,7 @@ func (npc *NetworkPolicyController) setupPodNetpolRules(pod podInfo, podFwChainN // if pod does not have any network policy which applies rules for pod's ingress traffic // then apply default network policy if !hasIngressPolicy { - comment := "\"run through default ingress network policy chain\"" + comment := "\"run through default ingress network policy chain\"" args := []string{"-I", podFwChainName, "1", "-d", ip, "-m", "comment", "--comment", comment, "-j", kubeDefaultNetpolChain, "\n"} filterTableRules.WriteString(strings.Join(args, " ")) @@ -196,7 +221,7 @@ func (npc *NetworkPolicyController) setupPodNetpolRules(pod podInfo, podFwChainN // if pod does not have any network policy which applies rules for pod's egress traffic // then apply default network policy if !hasEgressPolicy { - comment := "\"run through default egress network policy chain\"" + comment := "\"run through default egress network policy chain\"" args := []string{"-I", podFwChainName, "1", "-s", ip, "-m", "comment", "--comment", comment, "-j", kubeDefaultNetpolChain, "\n"} filterTableRules.WriteString(strings.Join(args, " ")) @@ -228,12 +253,11 @@ func (npc *NetworkPolicyController) setupPodNetpolRules(pod podInfo, podFwChainN func (npc *NetworkPolicyController) interceptPodInboundTraffic(pod podInfo, podFwChainName string) { for ipFamily, filterTableRules := range npc.filterTableRules { - var ip string - switch ipFamily { - case api.IPv4Protocol: - ip, _ = getPodIPv4Address(pod) - case api.IPv6Protocol: - ip, _ = getPodIPv6Address(pod) + ip, err := getPodIPForFamily(pod, ipFamily) + if err != nil { + klog.V(2).Infof("unable to get address for pod: %s -- skipping iptables inbound intercept "+ + "policy for pod (this is normal for pods that are not dual-stack)", err.Error()) + continue } // ensure there is rule in filter table and FORWARD chain to jump to pod specific firewall chain @@ -266,12 +290,11 @@ func (npc *NetworkPolicyController) interceptPodInboundTraffic(pod podInfo, podF // firewall chain corresponding to the pod so that egress network policies are enforced func (npc *NetworkPolicyController) interceptPodOutboundTraffic(pod podInfo, podFwChainName string) { for ipFamily, filterTableRules := range npc.filterTableRules { - var ip string - switch ipFamily { - case api.IPv4Protocol: - ip, _ = getPodIPv4Address(pod) - case api.IPv6Protocol: - ip, _ = getPodIPv6Address(pod) + ip, err := getPodIPForFamily(pod, ipFamily) + if err != nil { + klog.V(2).Infof("unable to get address for pod: %s -- skipping iptables outbound intercept "+ + "policy for pod (this is normal for pods that are not dual-stack)", err.Error()) + continue } for _, chain := range defaultChains { diff --git a/pkg/controllers/netpol/utils.go b/pkg/controllers/netpol/utils.go index 47a6e03b20..4062fbe115 100644 --- a/pkg/controllers/netpol/utils.go +++ b/pkg/controllers/netpol/utils.go @@ -131,7 +131,8 @@ func getPodIPv6Address(pod podInfo) (string, error) { return ip.IP, nil } } - return "", fmt.Errorf("pod %s has no IPv6Address", pod.name) + return "", fmt.Errorf("pod %s:%s has no IPv6Address, available addresses: %s", + pod.namespace, pod.name, pod.ips) } func getPodIPv4Address(pod podInfo) (string, error) { @@ -140,5 +141,26 @@ func getPodIPv4Address(pod podInfo) (string, error) { return ip.IP, nil } } - return "", fmt.Errorf("pod %s has no IPv4Address", pod.name) + return "", fmt.Errorf("pod %s:%s has no IPv4Address, available addresses: %s", + pod.namespace, pod.name, pod.ips) +} + +func getPodIPForFamily(pod podInfo, ipFamily api.IPFamily) (string, error) { + switch ipFamily { + case api.IPv4Protocol: + if ip, err := getPodIPv4Address(pod); err != nil { + return "", err + } else { + return ip, nil + } + case api.IPv6Protocol: + if ip, err := getPodIPv6Address(pod); err != nil { + return "", err + } else { + return ip, nil + } + } + + return "", fmt.Errorf("did not recognize IP Family for pod: %s:%s family: %s", pod.namespace, pod.name, + ipFamily) }