Handle nodes with iptables FORWARD DROP better #39823
Comments
|
Weave issue: weaveworks/weave#2758 |
|
I think this is going to hit a lot of people as Docker 1.13 is now setting the FORWARD chain to drop by default if ip_forward is set. See moby/moby#28257 |
|
See also #40182 |
dcbw
added
area/kube-proxy
sig/network
thockin
added
sig/network
|
overlap with #35069 ? |
|
I think a solution for this issue or #39658 (which is closer to what I feel like a proper solution is) should be considered for inclusion for K8s 1.8 since it will officially support docker 1.13. The problem as I see it is that kube-proxy sets up rules to forward traffic for services (specifically I have seen the problem with NodePorts) yet does not ensure or have any requirement (to my knowledge) that the forwarded traffic be allowed when the traffic is forwarded to a host other than the one receiving the traffic. I observed this when using K8s v1.7.5 installed with kubeadm v1.7.5 and Calico v2.5. It can be remedied with the addition of I saw the comment on the Docker 1.13 validation #42926 issue #42926 (comment) linking to the configuration script used in the GCE docker images which is why the e2es are not having any issues. |
|
If we can be more precise about which traffic we require to ALLOW, I'm OK
with adding rules to open that.
…On Fri, Sep 8, 2017 at 10:36 AM, Erik Stidham ***@***.***> wrote:
I think a solution for this issue or #39658
<#39658> (which is closer
to what I feel like a proper solution is) should be considered for
inclusion for K8s 1.8 since it will officially support docker 1.13.
The problem as I see it is that kube-proxy sets up rules to forward
traffic for services (specifically I have seen the problem with NodePorts)
yet does not ensure or have any requirement (to my knowledge) that the
forwarded traffic be allowed when the traffic is forwarded to a host other
than the one receiving the traffic. I observed this when using K8s v1.7.5
installed with kubeadm v1.7.5 and Calico v2.5.
It can be remedied with the addition of iptables -A FORWARD ACCEPT but
hopefully there is a better answer than doing that manually and hopefully
something more specific to what is needed.
I saw the comment on the Docker 1.13 validation #42926
<#42926> issue #42926
(comment)
<#42926 (comment)>
linking to the configuration script used in the GCE docker images which is
why the e2es are not having any issues.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#39823 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVFhtymhmvzMxLxDU7fXanM__aaDjks5sgXsNgaJpZM4LiOC4>
.
|
|
I've been doing some testing with the first two options and chatted with @fasaxc and @caseydavenport about these potential options for adding some forwarding rules. They all are a certainly lot more specific than the previous blanket all ALLOW.
@thockin, do any of those options appeal to you or do you see any problems with them? |
|
@tmjd If we add some more rules, we shouldn't put them in the top-level chain since it makes them hard to manage/clean up. Best to add them to a chain that kube-proxy owns that is jumped to from the top-level chain. |
|
Very good point @bboreham, I should have mentioned that the options I presented are assuming clusterCIDR is set. If clusterCIDR is not defined then the first option is not possible (except for the wide open option that these rules are attempting to avoid). I guess on options 2 & 3 if it was not set then a conntrack rule could be added without the clusterCIDR. I would appreciate feedback on the desired behavior if clusterCIDR is not set too. |
Automatic merge from submit-queue (batch tested with PRs 55009, 55532, 55601, 52569, 55533). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Kube-proxy adds forward rules to ensure NodePorts work **What this PR does / why we need it**: Updates kube-proxy to set up proper forwarding so that NodePorts work with docker 1.13 without depending on iptables FORWARD being changed manually/externally. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #39823 **Special notes for your reviewer**: @thockin I used option number 2 that I mentioned in the #39823 issue, please let me know what you think about this change. If you are happy with the change then I can try to add tests but may need a little direction about what and where to add them. **Release note**: ```release-note Add iptables rules to allow Pod traffic even when default iptables policy is to reject. ```
|
The PR that was merged solved the issue for pods inside clusterCIDR. But it doesn't handle hostNetwork pods that use the node IP, like when you want to expose the API server as a load-balanced service externally. I'm not sure why the “kubernetes forwarding conntrack pod source rule” has to be limited to the pod CIDR, since --ctstate RELATED,ESTABLISHED should already ensure it does not allow traffic that wasn't already allowed by another rule. |
|
I'd argue that any iptables/firewalling for a pod using There's no reasonable way for k8s to actually know what interfaces a host-network container binds to, what ports it listens on, or what expectations it has. The cluster operator should probably be responsible for that networking case. |
|
I meant explicitly regarding services where those pods are the endpoints of a service, so Kubernetes does know the address and the port. |
|
Due to limitation of state RELATED,ESTABLISHED in FORWARD rule i.e. current implementation, the unidirectional UDP traffic apps won't work. Because packets (except first) aren't ESTABILISHED from iptables's perspective (no reply from endpoint, no handshake). Shouldn't this be something like |
|
To add to @skbly7's comment, bidirectional UDP apps are brittle as well. Dropping or failing to reply to the first packet will prevent the app from ever receiving a second. More discussion on projectcalico/calico#2113. |
Is this a BUG REPORT or FEATURE REQUEST? (choose one): BUG REPORT
Kubernetes version (use
kubectl version): 1.5.1Environment:
uname -a): 4.4.0-57-genericWhat happened:
I originally opened #39658 to make Canal CNI work when the node's FORWARD chain default policy is set to DROP. However, I realized that the DROP policy was the root cause for why my other CNI tests also were failing. When either weave and flannel CNI is used and kube-proxy has --masquerade-all enabled, pod to pod and pod to service communication fails. Specifically, kubernetes-dashboard and tiller-deploy fail to reach the kube-apiserver and enter a crash loop. Also, the kube-dns pod also crashes.
kubectl get pods --all-namespaces -o wide
kubectl logs --namespace=kube-system kubernetes-dashboard-3203831700-5vzdq
kubectl describe --namespace=kube-system pod kubernetes-dashboard-3203831700-5vzdq
kubectl describe --namespace=kube-system pod kube-dns-2924299975-gz34l
sudo iptables -t filter -v -L FORWARD --line-numbers (197 currently has the dashboard pod scheduled. Note the 2 dropped packets.)
What you expected to happen:
Either it should be better documented that the default FORWARD chain policy should be ACCEPT or rules should be added to prevent packets from hitting the default policy. As a note, Ubuntu 16.04 defaults the FORWARD chain to DROP. I assume the
--masquerade-allis why the packets are going through the FORWARD chain in the first place but without--masquerade-all, the dashboard still can not access the kube-apiserver.One option that I suggested in #39658 is to have kube-proxy add an iptables FORWARD rule to ACCEPT packets that have been marked by KUBE-MARK-MASQ.
How to reproduce it (as minimally and precisely as possible):
Create a cluster with:
iptables -P FORWARD DROPon all nodes--masquerade-allAnything else do we need to know:
#39658 is closed because I thought the issue was specific to Canal and instead opened projectcalico/canal#31. However, this looks to be a larger issue that kubernetes itself should address.
The text was updated successfully, but these errors were encountered: