wgengine/router: add auto selection heuristic for iptables/nftables #8762

KevinLiang10 · 2023-08-01T02:41:04Z

Summary:

This change implements chooseFireWallMode that decides which one of iptables/nftables will be used.

Issue related

Updates: #391
Fix: #8111 #8733

Changes made

Added chooseFireWallMode that automatically decides which one of iptables ornftables should be used.
Replaced the TS_DEBUG_USE_NETLINK_NFTABLES envknob with TS_DEBUG_FIREWALL_MODE that should be set to either 'iptables' or 'nftables' to select firewall mode manually.
Reimplemented DetectIptables that detects iptables binary and returns number of rules in iptables in current network namespace.

Thought Process

It is a very edge case when iptables binary is not available, nft is also not available and iptables is in use. So we are not supporting this case for now.
It is a very edge case when nftables is only available throught nft binary but not netlinkAPI, so we will skip this case for now also. This is usually due to system configuration.
We always prioritize nftables ahead of iptables due to it's excellence in performance, and less likely to fail in containerized environment.
When In a container, if there was other app in the same container using iptables, we use iptables since if iptables wouldn't work the container would fail anyways. Otherwise we tend to use nftables since netlinkAPI is less likely to run into incompatibility issues.

Expected Result

If either 'iptables' or 'nftables' is set for TS_DEBUG_FIREWALL_MODE use the tool respectively.
If nftables is in use already use nftables.
if iptables is already in use user iptables, notice that since iptables-nft would write to nftables subsystem, iptables here refer to iptables-legacy.
Otherwise if nftables is available throught netlink API, use nftables. This case includes both ipt, nft are in use, neither is used or iptables-nft was used.
If only iptables binary is available, use iptables.
If neither iptables or nftables is available, return error.

How to test

On linux environment do the following:

Bring up tailscaled by running sudo TS_DEBUG_FIREWALL_MODE=iptables ./tool/go run ./cmd/tailscaled
Start tailscale by running sudo ./tool/go run ./cmd/tailscale up.
Run sudo iptables -L to view the rules added. In log of tailscaled, there should be a line router: router: envknob TS_DEBUG_NETFILTER_MODE=iptables set.
Turn everything down.
Bring up tailscaled by running sudo TS_DEBUG_FIREWALL_MODE=nftables ./tool/go run ./cmd/tailscaled
Start tailscale by running sudo ./tool/go run ./cmd/tailscale up.
Run sudo nft list ruleset to view the rules added. In the output log of tailscaled, it should say router: envknob TS_DEBUG_NETFILTER_MODE=nftables set.
Turn everything down.
Buring up tailscaled by running sudo ./tool/go run ./cmd/tailscaled
Start tailscale
You should see the count of iptables rules and nftables rule currently in system from the log in the form

router: router: iptables rule count: {number}
router: router: nftables rule count: {number}

if iptables is associated with iptables-nft, or nftables is used, it's likely that bother number are non zero depending on system implementation.
12. Your tailscale should be working, with which ever mode is selected.

andrew-d

Looking good! I did a preliminary review, and I'd appreciate if @raggi and/or @catzkorn could do one as well since they're more familiar with this than I am.

wgengine/router/router_linux.go

wgengine/router/router_linux_test.go

util/linuxfw/iptables.go

raggi

Looks good overall, but please adjust the logic so that we can still detect and function correctly on systems that only have iptables and are missing ip6tables, as we are able to function on those configurations today.

This commit replaces the TS_DEBUG_USE_NETLINK_NFTABLES envknob with a TS_DEBUG_FIREWALL_MODE that should be set to either 'iptables' or 'nftables' to select firewall mode manually, other wise tailscaled will automatically choose between iptables and nftables depending on environment and system availability. updates: #319 Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>

joesankey · 2023-08-11T03:56:41Z

I was getting the following error while trying these changes with the Kubernetes operator to expose a service on a tailnet:

│ modprobe: can't change directory to '/lib/modules': No such file or directory                                                                                               │
│ modprobe: can't change directory to '/lib/modules': No such file or directory                                                                                               │
│ modprobe: can't change directory to '/lib/modules': No such file or directory                                                                                               │
│ iptables v1.8.8 (legacy): can't initialize iptables table `nat': Table does not exist (do you need to insmod?)                                                              │
│ Perhaps iptables or your kernel needs to be upgraded.                                                                                                                       │
│ boot: 2023/08/11 01:42:37 installing proxy rules: executing iptables failed: exit status 3

Looks like containerboot needs to be updated to use iptables/nftables autoselection this PR introduced. When in nftable mode, the nat table no longer exists for container boot to add rules to.

I added this commit to my fork to fix the issue, at least for nftables.

We were previously using the netlink API to see if there are chains/rules that already exist. This works fine in environments where there is either full nftable support or no support at all. However, we have identified certain environments which have partial nftable support and the only feasible way of detecting such an environment is to try to create some of the chains that we need. This adds a check to create a dummy postrouting chain which is immediately deleted. The goal of the check is to ensure we are able to use nftables and that it won't error out later. This check is only done in the path where we detected that the system has no nftable rules. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

These tests were broken at HEAD. CI currently does not run these as root, will figure out how to do that in a followup. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

We were previously using the netlink API to see if there are chains/rules that already exist. This works fine in environments where there is either full nftable support or no support at all. However, we have identified certain environments which have partial nftable support and the only feasible way of detecting such an environment is to try to create some of the chains that we need. This adds a check to create a dummy postrouting chain which is immediately deleted. The goal of the check is to ensure we are able to use nftables and that it won't error out later. This check is only done in the path where we detected that the system has no preexisting nftable rules. Updates #5621 Updates #8555 Updates #8762

These tests were broken at HEAD. CI currently does not run these as root, will figure out how to do that in a followup. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

We were previously using the netlink API to see if there are chains/rules that already exist. This works fine in environments where there is either full nftable support or no support at all. However, we have identified certain environments which have partial nftable support and the only feasible way of detecting such an environment is to try to create some of the chains that we need. This adds a check to create a dummy postrouting chain which is immediately deleted. The goal of the check is to ensure we are able to use nftables and that it won't error out later. This check is only done in the path where we detected that the system has no preexisting nftable rules. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

These tests were broken at HEAD. CI currently does not run these as root, will figure out how to do that in a followup. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

We were previously using the netlink API to see if there are chains/rules that already exist. This works fine in environments where there is either full nftable support or no support at all. However, we have identified certain environments which have partial nftable support and the only feasible way of detecting such an environment is to try to create some of the chains that we need. This adds a check to create a dummy postrouting chain which is immediately deleted. The goal of the check is to ensure we are able to use nftables and that it won't error out later. This check is only done in the path where we detected that the system has no preexisting nftable rules. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

These tests were broken at HEAD. CI currently does not run these as root, will figure out how to do that in a followup. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

We were previously using the netlink API to see if there are chains/rules that already exist. This works fine in environments where there is either full nftable support or no support at all. However, we have identified certain environments which have partial nftable support and the only feasible way of detecting such an environment is to try to create some of the chains that we need. This adds a check to create a dummy postrouting chain which is immediately deleted. The goal of the check is to ensure we are able to use nftables and that it won't error out later. This check is only done in the path where we detected that the system has no preexisting nftable rules. Updates #5621 Updates #8555 Updates #8762 Signed-off-by: Maisem Ali <maisem@tailscale.com>

These tests were broken at HEAD. CI currently does not run these as root, will figure out how to do that in a followup. Updates tailscale#5621 Updates tailscale#8555 Updates tailscale#8762 Signed-off-by: Maisem Ali <maisem@tailscale.com> Signed-off-by: Alex Paguis <alex@windscribe.com>

We were previously using the netlink API to see if there are chains/rules that already exist. This works fine in environments where there is either full nftable support or no support at all. However, we have identified certain environments which have partial nftable support and the only feasible way of detecting such an environment is to try to create some of the chains that we need. This adds a check to create a dummy postrouting chain which is immediately deleted. The goal of the check is to ensure we are able to use nftables and that it won't error out later. This check is only done in the path where we detected that the system has no preexisting nftable rules. Updates tailscale#5621 Updates tailscale#8555 Updates tailscale#8762 Signed-off-by: Maisem Ali <maisem@tailscale.com> Signed-off-by: Alex Paguis <alex@windscribe.com>

KevinLiang10 force-pushed the KevinLiang10/Netfilter_selection_autoadapt_to_system_environments branch from a412a08 to 3915c0d Compare August 1, 2023 02:43

KevinLiang10 marked this pull request as ready for review August 1, 2023 15:49

Shaxine mentioned this pull request Aug 1, 2023

cmd/containerboot, cmd/k8s-operator: Add flag to k8s-operator and proxy to enable NFTABLES #8749

Open

rodrigc mentioned this pull request Aug 2, 2023

tailscale-operator not working on AKS #8111

Closed

KevinLiang10 requested review from raggi, catzkorn and andrew-d August 2, 2023 19:48

andrew-d reviewed Aug 3, 2023

View reviewed changes

raggi reviewed Aug 8, 2023

View reviewed changes

util/linuxfw/iptables.go Outdated Show resolved Hide resolved

raggi reviewed Aug 8, 2023

View reviewed changes

util/linuxfw/iptables.go Show resolved Hide resolved

raggi approved these changes Aug 8, 2023

View reviewed changes

KevinLiang10 force-pushed the KevinLiang10/Netfilter_selection_autoadapt_to_system_environments branch from 1d231a0 to be3372a Compare August 8, 2023 18:27

KevinLiang10 merged commit ae63c51 into main Aug 8, 2023
36 checks passed

KevinLiang10 deleted the KevinLiang10/Netfilter_selection_autoadapt_to_system_environments branch August 8, 2023 18:59

maisem mentioned this pull request Oct 17, 2023

util/linuxfw: add additional nftable detection logic #9846

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wgengine/router: add auto selection heuristic for iptables/nftables #8762

wgengine/router: add auto selection heuristic for iptables/nftables #8762

KevinLiang10 commented Aug 1, 2023 •

edited

andrew-d left a comment

raggi left a comment

joesankey commented Aug 11, 2023

wgengine/router: add auto selection heuristic for iptables/nftables #8762

wgengine/router: add auto selection heuristic for iptables/nftables #8762

Conversation

KevinLiang10 commented Aug 1, 2023 • edited

Summary:

Issue related

Changes made

Thought Process

Expected Result

How to test

andrew-d left a comment

Choose a reason for hiding this comment

raggi left a comment

Choose a reason for hiding this comment

joesankey commented Aug 11, 2023

KevinLiang10 commented Aug 1, 2023 •

edited