tailscale / tailscale/tailscale

FR: Improve nftables implementation

Open
#16,286 5 comments 18 reactions 0 assignees View on GitHub
fr needs-triage
Dominant language
Go
Stars
36.5k
Forks
3.2k
Avg merge
2d 3h
Merged PRs (30d)
123

Description

### What are you trying to do?

Although the current nftables implementation is functional, there is much room for improvement. The firewall rules created by Tailscale look as though the entire firewall concept was converted using `iptables-translate`, while completely ignoring all the QoL improvements that come with nftables. Users with complex firewalls have no choice but to manage all Tailscale rules manually.

The following rules are created by Tailscale when using `TS_DEBUG_FIREWALL_MODE=nftables`:

```nft
table ip filter {
chain FORWARD {
type filter hook forward priority filter; policy accept;
counter jump ts-forward
}

chain INPUT {
type filter hook input priority filter; policy accept;
counter jump ts-input
}

chain ts-forward {
iifname "tailscale0*" counter meta mark set meta mark & 0xffff04ff | 0x00000400
meta mark & 0x0000ff00 == 0x00000400 counter accept
oifname "tailscale0*" ip saddr counter drop
oifname "tailscale0*" counter accept
}

chain ts-input {
iifname "lo*" ip saddr counter accept
iifname != "tailscale0*" ip saddr counter return
iifname != "tailscale0*" ip saddr counter drop
iifname "tailscale0*" counter accept
udp dport 41641 counter accept
}
}
table ip nat {
chain POSTROUTING {
type nat hook postrouting priority srcnat; policy accept;
counter jump ts-postrouting
}

chain ts-postrouting {
meta mark & 0x0000ff00 == 0x00000400 counter masquerade
}
}
table ip6 filter {
chain FORWARD {
type filter hook forward priority filter; policy accept;
counter jump ts-forward
}

chain INPUT {
type filter hook input priority filter; policy accept;
counter jump ts-input
}

chain ts-forward {
iifname "tailscale0*" counter meta mark set meta mark & 0xffff04ff | 0x00000400
meta mark & 0x0000ff00 == 0x00000400 counter accept
oifname "tailscale0*" counter accept
}

chain ts-input {
iifname "lo*" ip6 saddr counter accept
iifname "tailscale0*" counter accept
udp dport 41641 counter accept
}
}
table ip6 nat {
chain POSTROUTING {
type nat hook postrouting priority srcnat; policy accept;
counter jump ts-postrouting
}

chain ts-postrouting {
meta mark & 0x0000ff00 == 0x00000400 counter masquerade
}
}
```

### How should we solve this?

Here are a few examples of what could be improved:

---

In nftables, there are no default tables. There is no need to create tables based on those that existed under iptables.

Tailscale creates the following tables:

```nft
table ip6 filter
table ip nat
table ip6 filter
table ip6 nat
```

These tables should be replaced by a single, Tailscale-specific `inet` table. For example:

```nft
table inet ts-table
```

This would put all Tailscale rules in one table, making them way easier and faster to manage.
For example, if Tailscale needs to remove rules (e.g. when the daemon is stopped), rather than removing individual rules, the entire table could simply be deleted using `nft destroy table inet ts-table`.
This would also prevent Tailscale rules from getting mixed up with rules from other applications. Using generic table names like `ip6 filter` means everything gets grouped together, so users might unintentionally delete Tailscale rules when managing their firewall and break their setup.

---

In nftables, there are no default chains. There is no need to create chains based on those that existed under iptables.

Tailsace creates the following (redundant) chains:

```nft
chain INPUT
chain FORWARD
chain POSTROUTING
```

Instead of creating these chains, the chain type, hook and priority could be defined directly within the `ts-*` chains. This would remove unnecessary jumps between chains and reduce clutter.

For example,

```nft
chain POSTROUTING {
type nat hook postrouting priority srcnat; policy accept;
counter jump ts-postrouting
}

chain ts-postrouting {
meta mark & 0x0000ff00 == 0x00000400 counter masquerade
}
```

could be replaced with

```nft
chain ts-postrouting {
type nat hook postrouting priority srcnat; policy accept;
meta mark & 0x0000ff00 == 0x00000400 counter masquerade
}
```

---

The [priority](https://wiki.nftables.org/wiki-nftables/index.php/Netfilter_hooks) setting used by Tailscale should be configurable.

Since there are no default chains, Netfilter uses priorities to determine the order it evaluates chains. Users might have rules that should be evaluated before/after the Tailscale rules. Instead of making users adjust their entire firewall setup, there should be a simple option to tell Tailscale which priority to use.

### What is the impact of not solving this?

I'm currently forced to manage the rules manually, which works but could of course break with every update since I'm not looking at every commit between versions.

### Anything else?

These are just a few low-hanging fruit that you notice after a quick look at the configuration. I'm sure there are more improvements to be found with more time invested. As a general idea of how it should work, I recommend checking out the nftables implementation of Fail2Ban as they've done pretty good work on it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.