Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@uablrek: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

/sig network

@andrewsykim Please review and advise about the update of the filter-INPUT chain.

The update to create a KUBE-IPVS0-IPS ipset was much smaller than I feared 😄

Unfortunately there is more to it. When the iptables rules are applied access to the host ports is rejected ok, but access to the service from main netns on the node doesn't work 😞

This is because traffic from main netns gets the VIP address as source. Example for 10.0.0.0 as VIP;

# ip ro show table local
local 10.0.0.0 dev kube-ipvs0 proto kernel scope host src 10.0.0.0 
...

This causes reply traffic with dest 10.0.0.0:ephemeral-port to be rejected.

Adding a ctstate solves the problem;

# iptables -S INPUT 
-P INPUT ACCEPT
-A INPUT -m comment --comment "kubernetes health check rules" -j KUBE-NODE-PORT
-A INPUT -j KUBE-FIREWALL
-A INPUT -m set --match-set KUBE-LOAD-BALANCER dst,dst -j ACCEPT
-A INPUT -m set --match-set KUBE-CLUSTER-IP dst,dst -j ACCEPT
-A INPUT -m conntrack --ctstate NEW -m set --match-set KUBE-IPVS0-IPS dst -j REJECT --reject-with icmp-port-unreachable

But are all cases covered? How about UDP?

/test all

/test pull-kubernetes-verify

/test pull-kubernetes-e2e-gci-gce-ipvs

/assign @andrewsykim

Now it looks like this;

# iptables -L
Chain INPUT (policy ACCEPT)
target     prot opt source               destination         
KUBE-IPVS-FILTER  all  --  anywhere             anywhere             /* kubernetes ipvs access filter */
KUBE-PROXY-FIREWALL  all  --  anywhere             anywhere             /* kube-proxy firewall rules */
KUBE-NODE-PORT  all  --  anywhere             anywhere             /* kubernetes health check rules */
KUBE-FIREWALL  all  --  anywhere             anywhere            
...

We have both KUBE-PROXY-FIREWALL and KUBE-IPVS-FILTER. I was thinking of re-use KUBE-PROXY-FIREWALL but it is also called from the FORWARD chain, so I didn't.

The "drop packets to ipvs0 addresses" rule wouldn't be useful in FORWARD, but it would be harmless...

@danwinship Should I remove KUBE-IPVS-FILTER and use KUBE-PROXY-FIREWALL?

It would be cleaner, but almost everything passes FORWARD, so...?

I don't have a vote here, but my gut says to keep them broken out unless there is a strong reason not to. For the reasons you said, but also observability/debugging if something has gone wrong. I think it is quite clean as is, and mixing it into another chain seems a step backward.

(Not a contributor to kube-proxy, just a rando that has been worrying about this security bug from the sidelines).

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, thockin, uablrek

The full list of commands accepted by this bot can be found here.

The pull request process is described here