search

zyclonite / zerotier-docker

ZeroTier One as Docker Image
MIT License
321 stars 84 forks source link

Fixed firewall rules for inbound/outbound scenarios #13

Closed bfg100k closed 2 years ago

Paraphraser commented 2 years ago

Just out of curiosity, is this something I missed from your main.sh ?

Paraphraser commented 2 years ago

Definitions:

Reference model:

managed-route-remote-client

Assume: ZEROTIER_ONE_GATEWAY_MODE=inbound

Observed behaviour:

Test from C bridge router PR13
ping 10.244.0.1 pass pass pass
ping 192.168.203.50 pass pass pass
ping 192.168.203.60 pass pass fail

The default implementation of router is inbound. The intention is that the default behaviour exactly mirrors that of bridge. With these changes in place, we break backwards compatibility.

Assume:

Observed behaviour:

Test from C router PR13
ping 10.244.0.1 pass pass
ping 192.168.203.50 pass pass
ping 192.168.203.60 pass pass

I have repeated the switch between inbound and both several times to assure myself that the "fail" is not some artifact of ZeroTier taking time to catch up as I switch between the modes. I am convinced that inbound prevents ping replies coming from coming back from B.

I also can't SSH from C to B when inbound is active so it isn't just some peculiarity of ICMP Echo.

I don't claim to have any deep understanding of iptables but, given the sequence:

${IPTABLES_CMD} -${1} FORWARD -i ${PHY_IFACE} -o ${ZT_IFACE} -j DROP
${IPTABLES_CMD} -${1} FORWARD -i ${PHY_IFACE} -o ${ZT_IFACE} -m state --state RELATED,ESTABLISHED -j ACCEPT

doesn't that result in all packets from the physical interface aimed at the ZeroTier interface being dropped by the first line, at which point the packet is considered as having "matched" a rule, so no more processing occurs?

Would it not make more sense to reverse the ordering:

${IPTABLES_CMD} -${1} FORWARD -i ${PHY_IFACE} -o ${ZT_IFACE} -m state --state RELATED,ESTABLISHED -j ACCEPT
${IPTABLES_CMD} -${1} FORWARD -i ${PHY_IFACE} -o ${ZT_IFACE} -j DROP

Again, I emphasise iptables-newbie-at-work but I read that as saying:

  1. If a packet is coming from the physical interface, is being sent to the ZeroTier interface, and it is part of a session that is already established, then it should be forwarded.
  2. All packets that do not match the first rule should be dropped.

When I re-order the lines in my local build (which I designate "PR13+"), the observed behaviour with inbound is:

Test from C router PR13+
ping 10.244.0.1 pass pass
ping 192.168.203.50 pass pass
ping 192.168.203.60 pass pass

A variation on the PR13+ theme is to add a static route on B so it knows the path to 10.144/16 is via A, and then ping from B to the ZeroTier interface of C (10.244.0.2):

With PR13+ and outbound in force, I can also confirm that C is unable to ping B so that works as expected too.

To summarise:

  1. If DROP is before ACCEPT, it seems to break inbound, which means we lose backwards compatibility with bridge.
  2. Re-ordering so DROP occurs after ACCEPT seems to implement what I perceive to be the intention of this PR, doesn't break backwards compatibility for inbound and appears to work for outbound too.

but I emphasise for the third time that I am no expert in this so please take all this with a few tons of salt.

bfg100k commented 2 years ago

Hi Phil, you didn't miss anything. This is totally my fault. I usually run ubuntu with ufw (frontend for iptables) which sets the default policy to DENY. At some point when i switched to zyclonite's container which is Alpine based, there is no ufw so the default iptables policy is ACCEPT, I have completely forgotten about it.

And regarding the fix, its also my fault. I tested it by adding the drop rule inline in the running container which works as expected. When I added it to the script, I placed it first thinking the chain is traversed in reverse and in haste (actually laziness) didn't occur to test it first before submitting the PR. Sorry you had to go through the exercise and time to troubleshoot this for me. I've now fixed it in a new commit (yes and tested the modified script).

BTW, with regards to the static route on B, it won't be necessary if your default gateway (i.e. your home router) already has the route defined there.

Paraphraser commented 2 years ago

Whew! But that raises an interesting point, does it not? If rules processing is different on different OSes, is it the "alpine" interpretation that sticks, or the underlying OS? From the way it seems to work (host mode plus the capabilities) it looks to me like the underlying OS is what matters.

BTW, with regards to the static route on B, it won't be necessary if your default gateway (i.e. your home router) already has the route defined there.

Gotcha. In essence I was making the point in terms of the model diagram that I had considered the question of how B could reach C. I didn't want to go back and redraw the diagram with a router. That's me being lazy!

bfg100k commented 2 years ago

iptables by default set all policies to ACCEPT. ufw is a package that is included in ubuntu distros and it hides the complexities of iptables from users (and changes the policy defaults to DENY). iptable rules work the same way regardless of the underlying distro. Its totally my fault for the confusion between ufw and iptables and how rules are interpreted/setup.

On Sat, 30 Jul 2022 at 14:43, Phill @.***> wrote:

Whew! But that raises an interesting point, does it not? If rules processing is different on different OSes, is it the "alpine" interpretation that sticks, or the underlying OS? From the way it seems to work (host mode plus the capabilities) it looks to me like the underlying OS is what matters.

BTW, with regards to the static route on B, it won't be necessary if your default gateway (i.e. your home router) already has the route defined there.

Gotcha. In essence I was making the point in terms of the model diagram that I had considered the question of how B could reach C. I didn't want to go back and redraw the diagram with a router. That's me being lazy!

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>