Skip to content

Commit

Permalink
fix(NPC): don't add chains for missing family
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@aauren
aauren committed Jan 22, 2023
1 parent 97df1fb commit ed2a5dc
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 29 deletions.
77 changes: 50 additions & 27 deletions pkg/controllers/netpol/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand All @@ -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, " "))
Expand All @@ -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")
}

Expand All @@ -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\""
Expand All @@ -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 {
Expand Down Expand Up @@ -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, " "))
Expand All @@ -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, " "))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 24 additions & 2 deletions pkg/controllers/netpol/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}

0 comments on commit ed2a5dc

Please sign in to comment.