tailscale / tailscale

The easiest, most secure way to use WireGuard and 2FA.
https://tailscale.com
BSD 3-Clause "New" or "Revised" License
16.89k stars 1.28k forks source link

FR: Create "inet" instead of "ip" and "ip6" nftables when existing nftables rules are all "inet". #11925

Open eric-eisenhart opened 3 weeks ago

eric-eisenhart commented 3 weeks ago

What are you trying to do?

On our servers, we have local firewall nftables firewall rules along the lines of:

table inet filter {
    chain INPUT {
        type filter hook input priority filter; policy drop;
        ct state established,related accept
        ct state invalid drop
        meta l4proto { icmp, ipv6-icmp } accept
        iifname "lo" accept
        tcp dport 22 jump SSH
    }
    chain SSH {
      ip saddr { 1.2.3.0/24 } accept comment "ip4 trusted networks"
      ip6 saddr { 1234:5678:9abc::/64 } accept comment "ip6 trusted networks"
   }
}

When tailscaled.service starts up (after nftables.service), we end up with table ip filter, table ip6 filter, and table ip nat in addition to the existing table inet filter.

A chain in a table inet can have both ip (ipv4) and ip6 rules in it. I believe nftables is smart enough to not evaluate an ip or ip6 rule when processing a packet for the other type, so really doesn't add any extra overhead.

How should we solve this?

I think that'd look something like this:

table inet 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 100.64.0.0/10 counter drop
                oifname "tailscale0*" counter accept
        }

        chain ts-input {
                iifname "lo*" ip saddr 100.123.45.67 counter accept
                iifname "lo*" ip6 saddr fd7a:115c:a1e0::1234:5678 counter accept
                iifname != "tailscale0*" ip saddr 100.115.92.0/23 counter return
                iifname != "tailscale0*" ip saddr 100.64.0.0/10 counter drop
                iifname "tailscale0*" counter accept
                udp dport 41641 counter accept
        }
}

Since tailscaled adds table ip nat but not table ip6 nat, I'm assuming it makes sense to keep that as `table ip nat

What is the impact of not solving this?

Leaves our nftables list ruleset output confusing in a way where I'd have to carefully read nftables documentation to fully understand what's really happening to a packet going through our nftables.

Key fragments of the live nftables config on one of our servers with tailscaled running:

table inet filter {
    chain INPUT {
        type filter hook input priority filter; policy drop;
        # various rules
    }
}
table ip filter {
    chain INPUT {
        type filter hook input priority filter; policy accept;
            counter jump ts-input
    }
}

There's two chains with same hook, priority, and name, but different policy and family.

Docs here https://wiki.nftables.org/wiki-nftables/index.php/Configuring_chains#Adding_base_chains say

It's possible to give two base chains the same priority, but there is no guaranteed evaluation order of base chains with identical priority that are attached to the same hook location.

As best I can tell, it's basically random which INPUT chain is evaluated first. If our policy drop INPUT chain is evaluated first, then policy accept INPUT chain won't get hit at all. If tailscaled's policy accept INPUT chain is evaluated first, then this should all work ok (since policy accept will still evaluate for any other base chains with same or later priority value).

Anything else?

No response