util/linuxfw: add nftable support for linux router #8555

KevinLiang10 · 2023-07-07T20:59:56Z

Summary:

This change implements netfilterRunner interface for nftables manipulation as a low level nftables runner. This change allows manipulation to netfilter rules via nftables.

Issue related

Updates: #391

Changes made

Added nftableRunner that implements netfilterRunner and nftable as a container of the nftables.Table abstraction.
Added envknob TS_DEBUG_USE_NETLINK_NFTABLES Which controls using nftables or iptables.
Added nftables rule creaters via google/nftables package.
Added unit test for nftables, the tests are splitted in to two parts. The first part tests on the correctness of rule we insert against the expected binary representation. The second part tests if the rules are added and deleted by interface methods in a testing namespace.

Thought Process

nftablesRunner contains a single connection for all nftables operation. This design was for testing with customized connections.
The second part of tests has to run with sudo privilege since we created a new network namespace for testing the rule operations.
All ts rules has counter expression for basic debug log purposes provided to user. If we want we can also add log expressions for logging in system
Since nftables doesn't have default table, we also created tailscale tables to contain the chains.
Since nftables doesn't have default chains, we have promoted our chain to a little bit higher priority than usual (-1 for filter type chains and -100 for NAT type chain) to be processed ahead of "normal fw rules" (basically the ones that follow convensions). The NAT type chain has reached highest possible priority but filter type chain can be promoted more if we want to.
Table cleaning was implemented for nftables rules.

Expected Result

tailscaled should be manipulating rules using nftables when it's running with envknob TS_DEBUG_USE_NETLINK_NFTABLES=true set. We expect that the app works the exact same way as we use iptables, and also expect these rules to be present when we run sudo nft list ruleset. Notice that the rule matching iifname "lo*" has local machine address as saddr, and the rules have the bytes displayed in little endian in the file paste file)

As an example for IPv4 rules.

How to test

On linux environment do the following:

Bring up tailscaled by running sudo TS_DEBUG_USE_NETLINK_NFTABLES=true ./tool/go run ./cmd/tailscaled
Start tailscale with a testing tailnet on corp.
Run sudo nft list ruleset to view the rules added.
Test the CGNAT rule and ChromeOSVMRange rule by creating packet from corresponding saddr range to a none tailscale network interface.
Shut down tailscale
Run tailscale as an exit node by running sudo ./tool/go run ./cmd/tailscale up --advertise-exit-node, the way of allowing forward and exit node can be found here.
Connect to exit node from another node on tailscale.
Check traffic on NAT table and ts-forward chain respectly. (About setting and checking ts marks and controlling outgoing packets)

Note

If we want to build then run, the env var should come with run, not build. So basically something like

go build -o tailscaled ./cmd/tailscaled/ && env TS_DEBUG_USE_NETLINK_NFTABLES=1 sudo -E ./tailscaled --statedir /tmp/ts/ --socket /tmp/ts/ts1.sock --verbose 99

catzkorn

A first pass, will need other eyes on too!

util/linuxfw/iptables_runner.go

util/linuxfw/linuxfw.go

util/linuxfw/nftables_runner.go

wgengine/router/router_linux.go

wgengine/watchdog.go

KevinLiang10 · 2023-07-12T04:14:19Z

nftables package should work in most cases, the only case wouldn't work is that system has security policy (say using SElinux) that prevents user space program to work with netlink api but still supports system utilities like nft to function. Since nftables rely on golang.org/x/sys/unix, it should compile even when netlink header is missing.

The solution I can think of are :

Somehow change security policy, but this is usually done by system admin we might not want to do this.
We can use r.cmd.run to use nft in this case.

I'm not sure it if worth talking about this case, since it's kind of like an edge case that usually wouldn't happen? Tho I do recall that @raggi mentioned to me there are distros only giveing us access to nft but not netlink api.

I didn't solve this problem when I implemented this solution because we thought the nftables package would solve this case, but I did some further investigation today and it doesn't seem like the case. We might need @raggi to provide further information

twitchyliquid64

Luv it

[xxx@tslptp:~]$ sudo nft list ruleset
table ip filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
		counter packets 4578 bytes 9213931 jump ts-input
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
		counter packets 0 bytes 0 jump ts-forward
	}

	chain ts-input {
		iifname "lo" ip saddr 100.82.9.85 counter packets 0 bytes 0 accept
		iifname != "tailscale0" ip saddr 100.115.92.0/23 counter packets 0 bytes 0 return
		iifname != "tailscale0" ip saddr 100.64.0.0/10 counter packets 0 bytes 0 drop
	}

	chain ts-forward {
		iifname "tailscale0" counter packets 0 bytes 0 meta mark set mark and 0xff00ffff xor 0x40000
		meta mark & 0x00ff0000 == 0x00040000 counter packets 0 bytes 0 accept
		oifname "tailscale0" ip saddr 100.64.0.0/10 counter packets 0 bytes 0 drop
		oifname "tailscale0" counter packets 0 bytes 0 accept
	}
}
table ip6 filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
		counter packets 1 bytes 88 jump ts-input
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
		counter packets 0 bytes 0 jump ts-forward
	}

	chain ts-input {
		iifname "lo" ip6 saddr fd7a:115c:a1e0:ab12:4843:cd96:6252:955 counter packets 0 bytes 0 accept
	}

	chain ts-forward {
		iifname "tailscale0" counter packets 0 bytes 0 meta mark set mark and 0xff00ffff xor 0x40000
		meta mark & 0x00ff0000 == 0x00040000 counter packets 0 bytes 0 accept
		oifname "tailscale0" counter packets 0 bytes 0 accept
	}
}
table ip nat {
	chain POSTROUTING {
		type nat hook postrouting priority srcnat; policy accept;
		counter packets 423 bytes 90219 jump ts-postrouting
	}

	chain ts-postrouting {
		meta mark & 0x00ff0000 == 0x00040000 counter packets 0 bytes 0 masquerade
	}
}
table ip6 nat {
	chain POSTROUTING {
		type nat hook postrouting priority srcnat; policy accept;
		counter packets 0 bytes 0 jump ts-postrouting
	}

	chain ts-postrouting {
		meta mark & 0x00ff0000 == 0x00040000 counter packets 0 bytes 0 masquerade
	}
}

@danderson fyi - is this something you want to take a pass at?

danderson · 2023-07-13T16:41:19Z

@twitchyliquid64 I don't need to take a pass at this, if it implements the same behavior as the iptables implementation then it's good to go. I also frankly don't know nftables well enough to find subtle issues. Do you want additional scrutiny?

twitchyliquid64

Also, what platforms has this been tested on?

util/linuxfw/nftables_runner.go

twitchyliquid64 · 2023-07-13T16:56:29Z

@twitchyliquid64 I don't need to take a pass at this, if it implements the same behavior as the iptables implementation then it's good to go. I also frankly don't know nftables well enough to find subtle issues. Do you want additional scrutiny?

Gotcha - I'm happy, and ive done some local testing too. I'm feeling good about this going in after 1.46 is cut (for 1.48). We should also try and get it on some subnet routers / exit nodes.

util/linuxfw/iptables_runner.go

wgengine/router/router_linux.go

wgengine/router/router_linux_test.go

util/linuxfw/nftables_runner.go

util/linuxfw/linuxfw.go

util/linuxfw/iptables_runner.go

util/linuxfw/nftables_runner.go

util/linuxfw/nftables_runner_test.go

raggi

Looking really good Kevin! I've not had a chance to test this out locally yet, but I just did a pass through, some comments included. The key thing I would like to understand is how evolution will work as we change the rules

util/linuxfw/nftables_runner.go

raggi · 2023-07-18T20:00:23Z

util/linuxfw/nftables_runner.go

+
+	// If TestDial is set, we are running in test mode and we should not
+	// find rule because header will mismatch.
+	if conn.TestDial == nil {


Related to the comment above on findRule around the longer term lifetime of matching and upgrading these rules, related considerations show up here.

Perhaps what we want to find is a rule that matches a pattern, rather than exact expression list. We might for example look for a rule that matches the core spirit of the operation, rather than an exact expression list. If we do that it may also avoid the need to relax conditions or skip over implementation in the tests. Did you try something like this?

Relatedly, is there an expression we can include that has no runtime effect, but acts as an explicit marker that demonstrates that it is a rule we inserted?

I think this topic Adrian brought up was the answer for the marker. We can do this by adding comment to rule but don't have a simple way to do this.

util/linuxfw/nftables_runner.go

raggi · 2023-07-18T21:28:30Z

util/linuxfw/nftables_runner_test.go

+
+// nfdump returns a hexdump of 4 bytes per line (like nft --debug=all), allowing
+// users to make sense of large byte literals more easily.
+func nfdump(b []byte) string {


You might like https://pkg.go.dev/encoding/hex#Dump which does most of what I think you want here, it takes a []byte and gives you a string containing a hexdump of the format:

00000000 47 6f 20 69 73 20 61 6e 20 6f 70 65 6e 20 73 6f |Go is an open so| 00000010 75 72 63 65 20 70 72 6f 67 72 61 6d 6d 69 6e 67 |urce programming| 00000020 20 6c 61 6e 67 75 61 67 65 2e | language.|

Tried Hex.Dump, the fact that I can't change number of bytes in single like makes me don't like it. If only one bit goes wrong it takes much more effort to find the mismatch (at least for me)

sailorfrag

I feel like there are still some details regarding rule life cycle that we might need to work out further but I also feel like it's probably fine to give this a try for now (especially since it requires an env var to opt in) and we can make incremental improvements later. I suspect it's already more robust than the iptables path.

sailorfrag · 2023-07-19T17:18:28Z

util/linuxfw/nftables_runner_test.go

+	}
+
+	conn, ns := newSysConn(t)
+	defer cleanupSysConn(t, ns)


consider having newSysConn call t.Cleanup(cleanupSysConn) (which will require reworking how t and ns are stored a little, maybe the cleanup would need to be a closure) to make it a bit easier for test writers

raggi

Looks good, and now had a chance to test on my router and it's working there too!

raggi · 2023-07-19T18:09:32Z

Please squash when you submit!

catzkorn

Great job Kevin!

This commit adds nftable rule injection for tailscaled. If tailscaled is started with envknob TS_DEBUG_USE_NETLINK_NFTABLES = true, the router will use nftables to manage firewall rules. Updates: #391 Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>

…le TS_DEBUG_USE_NETLINK_NFTABLES in tailscaled that was introduced in tailscale#8555 Fixes tailscale#8111, tailscale#8733

Add flag to k8s-operator to enable TS_DEBUG_USE_NETLINK_NFTABLES in tailscaled that was introduced in tailscale#8555 Fixes tailscale#8111, tailscale#8733

Add flag to k8s-operator to enable TS_DEBUG_USE_NETLINK_NFTABLES in tailscaled that was introduced in tailscale#8555 Fixes tailscale#8111, tailscale#8733 Signed-off-by: James Clarke <james@clarkezone.net>

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 changed the title ~~Draft:: Kevinliang10/iptables and nftables code refactoring~~ Kevinliang10/iptables and nftables code refactoring Jul 7, 2023

KevinLiang10 force-pushed the kevinliang10/Iptables_and_nftables_code_refactoring branch 3 times, most recently from 661c6cb to 2a47cc8 Compare July 7, 2023 23:42

KevinLiang10 requested review from catzkorn and raggi July 10, 2023 15:27

KevinLiang10 changed the title ~~Kevinliang10/iptables and nftables code refactoring~~ util/linuxfw: add nftable support for linux router Jul 10, 2023

KevinLiang10 requested a review from twitchyliquid64 July 10, 2023 18:03

KevinLiang10 marked this pull request as ready for review July 10, 2023 18:59

catzkorn reviewed Jul 11, 2023

View reviewed changes

KevinLiang10 requested a review from catzkorn July 12, 2023 14:26

twitchyliquid64 reviewed Jul 13, 2023

View reviewed changes

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

sailorfrag reviewed Jul 14, 2023

View reviewed changes

saljam reviewed Jul 15, 2023

View reviewed changes

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

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

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

KevinLiang10 force-pushed the kevinliang10/Iptables_and_nftables_code_refactoring branch 2 times, most recently from b748be3 to f348f17 Compare July 17, 2023 21:50

KevinLiang10 requested review from saljam and sailorfrag July 17, 2023 22:04

saljam reviewed Jul 18, 2023

View reviewed changes

util/linuxfw/nftables_runner_test.go Show resolved Hide resolved

saljam reviewed Jul 18, 2023

View reviewed changes

util/linuxfw/nftables_runner_test.go Show resolved Hide resolved

raggi reviewed Jul 18, 2023

View reviewed changes

KevinLiang10 requested a review from raggi July 19, 2023 00:18

sailorfrag approved these changes Jul 19, 2023

View reviewed changes

raggi approved these changes Jul 19, 2023

View reviewed changes

twitchyliquid64 approved these changes Jul 19, 2023

View reviewed changes

catzkorn approved these changes Jul 19, 2023

View reviewed changes

KevinLiang10 force-pushed the kevinliang10/Iptables_and_nftables_code_refactoring branch from f5ef839 to bbb87f7 Compare July 19, 2023 18:17

KevinLiang10 merged commit a3c7b21 into main Jul 19, 2023
36 checks passed

KevinLiang10 deleted the kevinliang10/Iptables_and_nftables_code_refactoring branch July 19, 2023 18:33

KevinLiang10 mentioned this pull request Jul 19, 2023

Program routes and policy rules using netlink, not iptables binary #391

Open

clarkezone mentioned this pull request Jul 30, 2023

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

Open

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

util/linuxfw: add nftable support for linux router #8555

util/linuxfw: add nftable support for linux router #8555

KevinLiang10 commented Jul 7, 2023 •

edited

catzkorn left a comment

KevinLiang10 commented Jul 12, 2023 •

edited

twitchyliquid64 left a comment

danderson commented Jul 13, 2023

twitchyliquid64 left a comment

twitchyliquid64 commented Jul 13, 2023

raggi left a comment

raggi Jul 18, 2023

raggi Jul 18, 2023

KevinLiang10 Jul 18, 2023

raggi Jul 18, 2023

KevinLiang10 Jul 19, 2023

sailorfrag left a comment

sailorfrag Jul 19, 2023

KevinLiang10 Jul 19, 2023

raggi left a comment

raggi commented Jul 19, 2023

catzkorn left a comment

util/linuxfw: add nftable support for linux router #8555

util/linuxfw: add nftable support for linux router #8555

Conversation

KevinLiang10 commented Jul 7, 2023 • edited

Summary:

Issue related

Changes made

Thought Process

Expected Result

How to test

Note

catzkorn left a comment

Choose a reason for hiding this comment

KevinLiang10 commented Jul 12, 2023 • edited

twitchyliquid64 left a comment

Choose a reason for hiding this comment

danderson commented Jul 13, 2023

twitchyliquid64 left a comment

Choose a reason for hiding this comment

twitchyliquid64 commented Jul 13, 2023

raggi left a comment

Choose a reason for hiding this comment

raggi Jul 18, 2023

Choose a reason for hiding this comment

raggi Jul 18, 2023

Choose a reason for hiding this comment

KevinLiang10 Jul 18, 2023

Choose a reason for hiding this comment

raggi Jul 18, 2023

Choose a reason for hiding this comment

KevinLiang10 Jul 19, 2023

Choose a reason for hiding this comment

sailorfrag left a comment

Choose a reason for hiding this comment

sailorfrag Jul 19, 2023

Choose a reason for hiding this comment

KevinLiang10 Jul 19, 2023

Choose a reason for hiding this comment

raggi left a comment

Choose a reason for hiding this comment

raggi commented Jul 19, 2023

catzkorn left a comment

Choose a reason for hiding this comment

KevinLiang10 commented Jul 7, 2023 •

edited

KevinLiang10 commented Jul 12, 2023 •

edited