Add deny by default to netpol#2079
Open
aauren wants to merge 3 commits into
Open
Conversation
This is the initial work done to optionally stop pod race conditions where the CNI has already allowed the pod to start, but a policy sync has not run yet. This potentially allows pods to begin traffic patterns that would otherwise be prohibited by networkpolicy. Since kube-router does not control the CNI layer like other k8s network plugins, we don't have the ability to gate the return of the pod network sandbox on policy synchronization. So we essentially have two options: * Fail open (which is what the project has done so far) * Fail closed (which can now be optionally set via --netpol-default-deny parameter) This essentially gives cluster administrators more options.
The previous attempt only blocked traffic for non-cluster ingress / egress. Workload to workload communication had problems because traffic would already be marked as long as the existing workload's networkpolicy did not deny it during startup, thus the traffic would be accepted by the mark on the packet before getting to our reject rules. This change fixes that, by pre-emptively rejecting traffic that has a source or destination within the node's PodCIDR, but who's pod IP has not yet been synced (as tracked by an IPset).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
feature
What this PR does / why we need it:
Closes the race window where a new pod is routable on the node before its
KUBE-POD-FW-*chain has been programmed, by introducing--netpol-default-denycommand line option. The initial commit on this branch added CIDR-scoped REJECTs at the tail ofKUBE-NWPLCY-TAIL, but those failed for pod-to-pod traffic on the same node because every per-pod chain marks accepted packets with0x20000, and theKUBE-NWPLCY-TAILACCEPT-on-mark rule fired before the CIDR REJECTs could.This PR finishes the feature by introducing a per-IP-family
kube-router-local-podsipset of pod IPs whose firewall chain has been programmed this sync. Two new ipset-gated REJECT rules are prepended toKUBE-NWPLCY-TAILahead of the ACCEPT-on-mark; they fire for any local pod IP not yet in the set. The existing CIDR-scoped REJECTs stay at the tail as defense-in-depth.When
--netpol-default-denyis disabled the ipset is never created and the rule layout is unchanged from today. The feature self-disables (with an error log) when this node's pod CIDRs cannot be detected vianode.spec.podCIDRsor thekube-router.io/pod-cidrsannotation.The user-guide gets a brief section on the feature; the troubleshooting guide gets a symptom-first entry ("NetworkPolicy not enforced on freshly-launched pods") with the operational details.
Which issue(s) this PR is related to:
Further protects against findings discussed in #873
Was AI used during the creation of this PR?
feat(npc): enable default deny for pod<->pod) and the third commit (doc: how --netpol-default-deny works) from a detailed plan. The first commit on the branch was hand-written.What, if any, amount of integration testing was done with this change in a Kubernetes environment?
Unit tests cover the new rule layout, ipset refresh, drift recovery, IPv6 prefix handling, and cleanup. Manual cluster validation (deploying the
netpol-race-testerDaemonSet fromkube-router-automation, verifyingiptables -L KUBE-NWPLCY-TAILandipset list kube-router-local-pods, toggling the flag off and confirming the ipset and extra rules are gone) is the remaining checklist item before merge.I also tested against a local Kubernetes cluster with a custom pod workload that attempted to make early connections both to cluster internal and cluster external endpoints. I toggled default-deny on and off and watched the workloads.
Does this PR introduce a breaking change?
Anything else the reviewer should know that wasn't already covered?
--netpol-default-deny(off by default), so this is purely additive for existing deployments.--allocate-node-cidrs=truefrom kube-controller-manager. It can also be used by other more custom configurations as long as the user annotates the node with the kube-routerkube-router.io/pod-cidrsannotation. If neither or these are configured, kube-router cannot safely gate traffic and will warn and continue.ensureTailChainPositioninstalls the TAIL jump in the iptables-restore buffer, so the new rules cannot fire against a stale ipset during a fresh kube-router start.