From c28f5767bfbc2ef037a41ee7cc13e9bff6a81713 Mon Sep 17 00:00:00 2001 From: Andrew Lytvynov Date: Mon, 6 May 2024 15:22:17 -0700 Subject: [PATCH] various: implement stateful firewalling on Linux (#12025) Updates https://github.com/tailscale/corp/issues/19623 Change-Id: I7980e1fb736e234e66fa000d488066466c96ec85 Signed-off-by: Andrew Dunham Co-authored-by: Andrew Dunham --- cmd/tailscale/cli/cli_test.go | 41 +++--- cmd/tailscale/cli/up.go | 18 ++- ipn/ipn_clone.go | 2 + ipn/ipn_view.go | 3 + ipn/ipnlocal/local.go | 32 ++++- ipn/ipnlocal/profiles.go | 30 +++- ipn/ipnlocal/profiles_test.go | 86 ++++++++++++ ipn/prefs.go | 24 ++++ ipn/prefs_test.go | 1 + tailcfg/tailcfg.go | 3 +- util/linuxfw/fake.go | 9 ++ util/linuxfw/iptables_runner.go | 63 +++++++++ util/linuxfw/nftables_runner.go | 196 +++++++++++++++++++++++++++ wgengine/router/router.go | 7 +- wgengine/router/router_linux.go | 59 ++++++-- wgengine/router/router_linux_test.go | 91 ++++++++++++- wgengine/router/router_test.go | 14 +- 17 files changed, 632 insertions(+), 47 deletions(-) diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index 90a7063f4..1e516f040 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -655,12 +655,13 @@ func TestPrefsFromUpArgs(t *testing.T) { goos: "linux", args: upArgsFromOSArgs("linux"), want: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - WantRunning: true, - NoSNAT: false, - NetfilterMode: preftype.NetfilterOn, - CorpDNS: true, - AllowSingleHosts: true, + ControlURL: ipn.DefaultControlURL, + WantRunning: true, + NoSNAT: false, + NoStatefulFiltering: "false", + NetfilterMode: preftype.NetfilterOn, + CorpDNS: true, + AllowSingleHosts: true, AutoUpdate: ipn.AutoUpdatePrefs{ Check: true, }, @@ -694,7 +695,8 @@ func TestPrefsFromUpArgs(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, - NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: "false", + NetfilterMode: preftype.NetfilterOn, AutoUpdate: ipn.AutoUpdatePrefs{ Check: true, }, @@ -781,9 +783,10 @@ func TestPrefsFromUpArgs(t *testing.T) { }, wantWarn: "netfilter=nodivert; add iptables calls to ts-* chains manually.", want: &ipn.Prefs{ - WantRunning: true, - NetfilterMode: preftype.NetfilterNoDivert, - NoSNAT: true, + WantRunning: true, + NetfilterMode: preftype.NetfilterNoDivert, + NoSNAT: true, + NoStatefulFiltering: "true", AutoUpdate: ipn.AutoUpdatePrefs{ Check: true, }, @@ -797,9 +800,10 @@ func TestPrefsFromUpArgs(t *testing.T) { }, wantWarn: "netfilter=off; configure iptables yourself.", want: &ipn.Prefs{ - WantRunning: true, - NetfilterMode: preftype.NetfilterOff, - NoSNAT: true, + WantRunning: true, + NetfilterMode: preftype.NetfilterOff, + NoSNAT: true, + NoStatefulFiltering: "true", AutoUpdate: ipn.AutoUpdatePrefs{ Check: true, }, @@ -813,8 +817,9 @@ func TestPrefsFromUpArgs(t *testing.T) { netfilterMode: "off", }, want: &ipn.Prefs{ - WantRunning: true, - NoSNAT: true, + WantRunning: true, + NoSNAT: true, + NoStatefulFiltering: "true", AdvertiseRoutes: []netip.Prefix{ netip.MustParsePrefix("fd7a:115c:a1e0:b1a::bb:10.0.0.0/112"), }, @@ -831,8 +836,9 @@ func TestPrefsFromUpArgs(t *testing.T) { netfilterMode: "off", }, want: &ipn.Prefs{ - WantRunning: true, - NoSNAT: true, + WantRunning: true, + NoSNAT: true, + NoStatefulFiltering: "true", AdvertiseRoutes: []netip.Prefix{ netip.MustParsePrefix("fd7a:115c:a1e0:b1a::aabb:10.0.0.0/112"), }, @@ -1031,6 +1037,7 @@ func TestUpdatePrefs(t *testing.T) { HostnameSet: true, NetfilterModeSet: true, NoSNATSet: true, + NoStatefulFilteringSet: true, OperatorUserSet: true, RouteAllSet: true, RunSSHSet: true, diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index bc5fd0ab2..18e932e8d 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -121,6 +121,7 @@ func newUpFlagSet(goos string, upArgs *upArgsT, cmd string) *flag.FlagSet { switch goos { case "linux": upf.BoolVar(&upArgs.snat, "snat-subnet-routes", true, "source NAT traffic to local routes advertised with --advertise-routes") + upf.BoolVar(&upArgs.statefulFiltering, "stateful-filtering", true, "apply stateful filtering to forwarded packets (subnet routers, exit nodes, etc.)") upf.StringVar(&upArgs.netfilterMode, "netfilter-mode", defaultNetfilterMode(), "netfilter mode (one of on, nodivert, off)") case "windows": upf.BoolVar(&upArgs.forceDaemon, "unattended", false, "run in \"Unattended Mode\" where Tailscale keeps running even after the current GUI user logs out (Windows-only)") @@ -168,6 +169,7 @@ type upArgsT struct { advertiseTags string advertiseConnector bool snat bool + statefulFiltering bool netfilterMode string authKeyOrFile string // "secret" or "file:/path/to/secret" hostname string @@ -291,6 +293,9 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo if goos == "linux" { prefs.NoSNAT = !upArgs.snat + // Backfills for NoStatefulFiltering occur when loading a profile; just set it explicitly here. + prefs.NoStatefulFiltering.Set(!upArgs.statefulFiltering) + switch upArgs.netfilterMode { case "on": prefs.NetfilterMode = preftype.NetfilterOn @@ -711,6 +716,7 @@ func init() { addPrefFlagMapping("netfilter-mode", "NetfilterMode") addPrefFlagMapping("shields-up", "ShieldsUp") addPrefFlagMapping("snat-subnet-routes", "NoSNAT") + addPrefFlagMapping("stateful-filtering", "NoStatefulFiltering") addPrefFlagMapping("exit-node-allow-lan-access", "ExitNodeAllowLANAccess") addPrefFlagMapping("unattended", "ForceDaemon") addPrefFlagMapping("operator", "OperatorUser") @@ -895,7 +901,7 @@ func applyImplicitPrefs(prefs, oldPrefs *ipn.Prefs, env upCheckEnv) { func flagAppliesToOS(flag, goos string) bool { switch flag { - case "netfilter-mode", "snat-subnet-routes": + case "netfilter-mode", "snat-subnet-routes", "stateful-filtering": return goos == "linux" case "unattended": return goos == "windows" @@ -970,6 +976,16 @@ func prefsToFlags(env upCheckEnv, prefs *ipn.Prefs) (flagVal map[string]any) { set(prefs.AppConnector.Advertise) case "snat-subnet-routes": set(!prefs.NoSNAT) + case "stateful-filtering": + // We only set the stateful-filtering flag to false if + // the pref (negated!) is explicitly set to true; unset + // or false is treated as enabled. + val, ok := prefs.NoStatefulFiltering.Get() + if ok && val { + set(false) + } else { + set(true) + } case "netfilter-mode": set(prefs.NetfilterMode.String()) case "unattended": diff --git a/ipn/ipn_clone.go b/ipn/ipn_clone.go index a5accfb3f..abbee9fa1 100644 --- a/ipn/ipn_clone.go +++ b/ipn/ipn_clone.go @@ -11,6 +11,7 @@ import ( "tailscale.com/drive" "tailscale.com/tailcfg" + "tailscale.com/types/opt" "tailscale.com/types/persist" "tailscale.com/types/preftype" ) @@ -57,6 +58,7 @@ var _PrefsCloneNeedsRegeneration = Prefs(struct { Egg bool AdvertiseRoutes []netip.Prefix NoSNAT bool + NoStatefulFiltering opt.Bool NetfilterMode preftype.NetfilterMode OperatorUser string ProfileName string diff --git a/ipn/ipn_view.go b/ipn/ipn_view.go index 716d89e0e..7d30f3571 100644 --- a/ipn/ipn_view.go +++ b/ipn/ipn_view.go @@ -12,6 +12,7 @@ import ( "tailscale.com/drive" "tailscale.com/tailcfg" + "tailscale.com/types/opt" "tailscale.com/types/persist" "tailscale.com/types/preftype" "tailscale.com/types/views" @@ -86,6 +87,7 @@ func (v PrefsView) AdvertiseRoutes() views.Slice[netip.Prefix] { return views.SliceOf(v.ж.AdvertiseRoutes) } func (v PrefsView) NoSNAT() bool { return v.ж.NoSNAT } +func (v PrefsView) NoStatefulFiltering() opt.Bool { return v.ж.NoStatefulFiltering } func (v PrefsView) NetfilterMode() preftype.NetfilterMode { return v.ж.NetfilterMode } func (v PrefsView) OperatorUser() string { return v.ж.OperatorUser } func (v PrefsView) ProfileName() string { return v.ж.ProfileName } @@ -120,6 +122,7 @@ var _PrefsViewNeedsRegeneration = Prefs(struct { Egg bool AdvertiseRoutes []netip.Prefix NoSNAT bool + NoStatefulFiltering opt.Bool NetfilterMode preftype.NetfilterMode OperatorUser string ProfileName string diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 77ab05ef8..cf1726e90 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -4153,13 +4153,33 @@ func (b *LocalBackend) routerConfig(cfg *wgcfg.Config, prefs ipn.PrefsView, oneC netfilterKind = prefs.NetfilterKind() } + var doStatefulFiltering bool + if v, ok := prefs.NoStatefulFiltering().Get(); !ok { + // The stateful filtering preference isn't explicitly set; this is + // unexpected since we expect it to be set during the profile + // backfill, but to be safe let's enable stateful filtering + // absent further information. + doStatefulFiltering = true + b.logf("[unexpected] NoStatefulFiltering preference not set; enabling stateful filtering") + } else if v { + // The preferences explicitly say "no stateful filtering", so + // we don't do it. + doStatefulFiltering = false + } else { + // The preferences explicitly "do stateful filtering" is turned + // off, or to expand the double negative, to do stateful + // filtering. Do so. + doStatefulFiltering = true + } + rs := &router.Config{ - LocalAddrs: unmapIPPrefixes(cfg.Addresses), - SubnetRoutes: unmapIPPrefixes(prefs.AdvertiseRoutes().AsSlice()), - SNATSubnetRoutes: !prefs.NoSNAT(), - NetfilterMode: prefs.NetfilterMode(), - Routes: peerRoutes(b.logf, cfg.Peers, singleRouteThreshold), - NetfilterKind: netfilterKind, + LocalAddrs: unmapIPPrefixes(cfg.Addresses), + SubnetRoutes: unmapIPPrefixes(prefs.AdvertiseRoutes().AsSlice()), + SNATSubnetRoutes: !prefs.NoSNAT(), + StatefulFiltering: doStatefulFiltering, + NetfilterMode: prefs.NetfilterMode(), + Routes: peerRoutes(b.logf, cfg.Peers, singleRouteThreshold), + NetfilterKind: netfilterKind, } if distro.Get() == distro.Synology { diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 8a092f3a1..b0a002947 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -371,12 +371,39 @@ func (pm *profileManager) loadSavedPrefs(key ipn.StateKey) (ipn.PrefsView, error // https://github.com/tailscale/tailscale/pull/11814/commits/1613b18f8280c2bce786980532d012c9f0454fa2#diff-314ba0d799f70c8998940903efb541e511f352b39a9eeeae8d475c921d66c2ac // prefs could set AutoUpdate.Apply=true via EditPrefs or tailnet // auto-update defaults. After that change, such value is "invalid" and - // cause any EditPrefs calls to fail (other than disabling autp-updates). + // cause any EditPrefs calls to fail (other than disabling auto-updates). // // Reset AutoUpdate.Apply if we detect such invalid prefs. if savedPrefs.AutoUpdate.Apply.EqualBool(true) && !clientupdate.CanAutoUpdate() { savedPrefs.AutoUpdate.Apply.Clear() } + + // Backfill a missing NoStatefulFiltering field based on the value of + // the NoSNAT field; we want to apply stateful filtering in all cases + // *except* where the user has disabled SNAT. + // + // Only backfill if the user hasn't set a value for + // NoStatefulFiltering, however. + _, haveNoStateful := savedPrefs.NoStatefulFiltering.Get() + if !haveNoStateful { + if savedPrefs.NoSNAT { + pm.logf("backfilling NoStatefulFiltering field to true because NoSNAT is set") + + // No SNAT: no stateful filtering + savedPrefs.NoStatefulFiltering.Set(true) + } else { + pm.logf("backfilling NoStatefulFiltering field to false because NoSNAT is not set") + + // SNAT (default): apply stateful filtering + savedPrefs.NoStatefulFiltering.Set(false) + } + + // Write back to the preferences store now that we've updated it. + if err := pm.writePrefsToStore(key, savedPrefs.View()); err != nil { + return ipn.PrefsView{}, err + } + } + return savedPrefs.View(), nil } @@ -468,6 +495,7 @@ var defaultPrefs = func() ipn.PrefsView { prefs := ipn.NewPrefs() prefs.LoggedOut = true prefs.WantRunning = false + prefs.NoStatefulFiltering = "false" return prefs.View() }() diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 6a8e15751..0acd0dcab 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -4,6 +4,7 @@ package ipnlocal import ( + "encoding/json" "fmt" "os/user" "strconv" @@ -12,12 +13,14 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "tailscale.com/clientupdate" + "tailscale.com/envknob" "tailscale.com/health" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" "tailscale.com/types/key" "tailscale.com/types/logger" + "tailscale.com/types/opt" "tailscale.com/types/persist" "tailscale.com/util/must" ) @@ -600,3 +603,86 @@ func TestProfileManagementWindows(t *testing.T) { t.Fatalf("CurrentUserID = %q; want %q", pm.CurrentUserID(), uid) } } + +func TestProfileBackfillStatefulFiltering(t *testing.T) { + envknob.Setenv("TS_DEBUG_PROFILES", "true") + + tests := []struct { + noSNAT bool + noStateful opt.Bool + want bool + }{ + // Default: NoSNAT is false, NoStatefulFiltering is false, so + // we want it to stay false. + {false, "false", false}, + + // NoSNAT being set to true and NoStatefulFiltering being false + // should result in NoStatefulFiltering still being false, + // since it was explicitly set. + {true, "false", false}, + + // If NoSNAT is false, and NoStatefulFiltering is unset, we + // backfill it to 'false'. + {false, "", false}, + + // If NoSNAT is true, and NoStatefulFiltering is unset, we + // backfill to 'true' to not break users of NoSNAT. + // + // In other words: if the user is not using SNAT, they almost + // certainly also don't want to use stateful filtering. + {true, "", true}, + + // However, if the user specifies both NoSNAT and stateful + // filtering, don't change that. + {true, "true", true}, + {false, "true", true}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("noSNAT=%v,noStateful=%q", tt.noSNAT, tt.noStateful), func(t *testing.T) { + prefs := ipn.NewPrefs() + prefs.Persist = &persist.Persist{ + NodeID: tailcfg.StableNodeID("node1"), + UserProfile: tailcfg.UserProfile{ + ID: tailcfg.UserID(1), + LoginName: "user1@example.com", + }, + } + + prefs.NoSNAT = tt.noSNAT + prefs.NoStatefulFiltering = tt.noStateful + + // Make enough of a state store to load the prefs. + const profileName = "profile1" + bn := must.Get(json.Marshal(map[string]any{ + string(ipn.CurrentProfileStateKey): []byte(profileName), + string(ipn.KnownProfilesStateKey): must.Get(json.Marshal(map[ipn.ProfileID]*ipn.LoginProfile{ + profileName: { + ID: "profile1-id", + Key: profileName, + }, + })), + profileName: prefs.ToBytes(), + })) + + store := new(mem.Store) + err := store.LoadFromJSON([]byte(bn)) + if err != nil { + t.Fatal(err) + } + + ht := new(health.Tracker) + pm, err := newProfileManagerWithGOOS(store, t.Logf, ht, "linux") + if err != nil { + t.Fatal(err) + } + + // Get the current profile and verify that we backfilled our + // StatefulFiltering boolean. + pf := pm.CurrentPrefs() + if !pf.NoStatefulFiltering().EqualBool(tt.want) { + t.Fatalf("got NoStatefulFiltering=%v, want %v", pf.NoStatefulFiltering(), tt.want) + } + }) + } +} diff --git a/ipn/prefs.go b/ipn/prefs.go index f4edeb2d1..2a353a531 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -203,6 +203,21 @@ type Prefs struct { // Linux-only. NoSNAT bool + // NoStatefulFiltering specifies whether to apply stateful filtering + // when advertising routes in AdvertiseRoutes. The default is to apply + // stateful filtering. + // + // To allow inbound connections from advertised routes, both NoSNAT and + // NoStatefulFiltering must be true. + // + // This is an opt.Bool because it was added after NoSNAT, but is backfilled + // based on the value of that parameter. We need to treat it as a tristate: + // true, false, or unset, and backfill based on that value. See + // ipn/ipnlocal for more details on the backfill. + // + // Linux-only. + NoStatefulFiltering opt.Bool `json:",omitempty"` + // NetfilterMode specifies how much to manage netfilter rules for // Tailscale, if at all. NetfilterMode preftype.NetfilterMode @@ -302,6 +317,7 @@ type MaskedPrefs struct { EggSet bool `json:",omitempty"` AdvertiseRoutesSet bool `json:",omitempty"` NoSNATSet bool `json:",omitempty"` + NoStatefulFilteringSet bool `json:",omitempty"` NetfilterModeSet bool `json:",omitempty"` OperatorUserSet bool `json:",omitempty"` ProfileNameSet bool `json:",omitempty"` @@ -501,6 +517,13 @@ func (p *Prefs) pretty(goos string) string { if len(p.AdvertiseRoutes) > 0 || p.NoSNAT { fmt.Fprintf(&sb, "snat=%v ", !p.NoSNAT) } + if len(p.AdvertiseRoutes) > 0 || p.NoStatefulFiltering.EqualBool(true) { + // Only print if we're advertising any routes, or the user has + // turned off stateful filtering (NoStatefulFiltering=true ⇒ + // StatefulFiltering=false). + bb, _ := p.NoStatefulFiltering.Get() + fmt.Fprintf(&sb, "statefulFiltering=%v ", !bb) + } if len(p.AdvertiseTags) > 0 { fmt.Fprintf(&sb, "tags=%s ", strings.Join(p.AdvertiseTags, ",")) } @@ -569,6 +592,7 @@ func (p *Prefs) Equals(p2 *Prefs) bool { p.NotepadURLs == p2.NotepadURLs && p.ShieldsUp == p2.ShieldsUp && p.NoSNAT == p2.NoSNAT && + p.NoStatefulFiltering == p2.NoStatefulFiltering && p.NetfilterMode == p2.NetfilterMode && p.OperatorUser == p2.OperatorUser && p.Hostname == p2.Hostname && diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index e9d9cc22e..ad6758cd7 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -56,6 +56,7 @@ func TestPrefsEqual(t *testing.T) { "Egg", "AdvertiseRoutes", "NoSNAT", + "NoStatefulFiltering", "NetfilterMode", "OperatorUser", "ProfileName", diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 11f67386d..29028d3c8 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -133,7 +133,8 @@ type CapabilityVersion int // - 90: 2024-04-03: Client understands PeerCapabilityTaildrive. // - 91: 2024-04-24: Client understands PeerCapabilityTaildriveSharer. // - 92: 2024-05-06: Client understands NodeAttrUserDialUseRoutes. -const CurrentCapabilityVersion CapabilityVersion = 92 +// - 93: 2024-05-06: added support for stateful firewalling. +const CurrentCapabilityVersion CapabilityVersion = 93 type StableID string diff --git a/util/linuxfw/fake.go b/util/linuxfw/fake.go index 8fd26dca7..c3c9ed00b 100644 --- a/util/linuxfw/fake.go +++ b/util/linuxfw/fake.go @@ -85,6 +85,15 @@ func (n *fakeIPTables) Delete(table, chain string, args ...string) error { } } +func (n *fakeIPTables) List(table, chain string) ([]string, error) { + k := table + "/" + chain + if rules, ok := n.n[k]; ok { + return rules, nil + } else { + return nil, fmt.Errorf("unknown table/chain %s", k) + } +} + func (n *fakeIPTables) ClearChain(table, chain string) error { k := table + "/" + chain if _, ok := n.n[k]; ok { diff --git a/util/linuxfw/iptables_runner.go b/util/linuxfw/iptables_runner.go index 237bc0870..dea764aca 100644 --- a/util/linuxfw/iptables_runner.go +++ b/util/linuxfw/iptables_runner.go @@ -12,6 +12,7 @@ import ( "net/netip" "os" "os/exec" + "slices" "strconv" "strings" @@ -36,6 +37,7 @@ type iptablesInterface interface { Append(table, chain string, args ...string) error Exists(table, chain string, args ...string) (bool, error) Delete(table, chain string, args ...string) error + List(table, chain string) ([]string, error) ClearChain(table, chain string) error NewChain(table, chain string) error DeleteChain(table, chain string) error @@ -530,6 +532,67 @@ func (i *iptablesRunner) DelSNATRule() error { return nil } +func statefulRuleArgs(tunname string) []string { + return []string{"-o", tunname, "-m", "conntrack", "!", "--ctstate", "ESTABLISHED,RELATED", "-j", "DROP"} +} + +// AddStatefulRule adds a netfilter rule for stateful packet filtering using +// conntrack. +func (i *iptablesRunner) AddStatefulRule(tunname string) error { + // Drop packets that are destined for the tailscale interface if + // they're a new connection, per conntrack, to prevent hosts on the + // same subnet from being able to use this device as a way to forward + // packets on to the Tailscale network. + // + // The conntrack states are: + // NEW A packet which creates a new connection. + // ESTABLISHED A packet which belongs to an existing connection + // (i.e., a reply packet, or outgoing packet on a + // connection which has seen replies). + // RELATED A packet which is related to, but not part of, an + // existing connection, such as an ICMP error. + // INVALID A packet which could not be identified for some + // reason: this includes running out of memory and ICMP + // errors which don't correspond to any known + // connection. Generally these packets should be + // dropped. + // + // We drop NEW packets to prevent connections from coming "into" + // Tailscale from other hosts on the same network segment; we drop + // INVALID packets as well. + args := statefulRuleArgs(tunname) + for _, ipt := range i.getTables() { + // First, find the final "accept" rule. + rules, err := ipt.List("filter", "ts-forward") + if err != nil { + return fmt.Errorf("listing rules in filter/ts-forward: %w", err) + } + want := fmt.Sprintf("-A %s -o %s -j ACCEPT", "ts-forward", tunname) + + pos := slices.Index(rules, want) + if pos < 0 { + return fmt.Errorf("couldn't find final ACCEPT rule in filter/ts-forward") + } + + if err := ipt.Insert("filter", "ts-forward", pos, args...); err != nil { + return fmt.Errorf("adding %v in filter/ts-forward: %w", args, err) + } + } + return nil +} + +// DelStatefulRule removes the netfilter rule for stateful packet filtering +// using conntrack. +func (i *iptablesRunner) DelStatefulRule(tunname string) error { + args := statefulRuleArgs(tunname) + for _, ipt := range i.getTables() { + if err := ipt.Delete("filter", "ts-forward", args...); err != nil { + return fmt.Errorf("deleting %v in filter/ts-forward: %w", args, err) + } + } + return nil +} + // buildMagicsockPortRule generates the string slice containing the arguments // to describe a rule accepting traffic on a particular port to iptables. It is // separated out here to avoid repetition in AddMagicsockPortRule and diff --git a/util/linuxfw/nftables_runner.go b/util/linuxfw/nftables_runner.go index 1c8a68361..d239ac12f 100644 --- a/util/linuxfw/nftables_runner.go +++ b/util/linuxfw/nftables_runner.go @@ -514,6 +514,14 @@ type NetfilterRunner interface { // DelSNATRule removes the rule added by AddSNATRule. DelSNATRule() error + // AddStatefulRule adds a netfilter rule for stateful packet filtering + // using conntrack. + AddStatefulRule(tunname string) error + + // DelStatefulRule removes a netfilter rule for stateful packet filtering + // using conntrack. + DelStatefulRule(tunname string) error + // HasIPV6 reports true if the system supports IPv6. HasIPV6() bool @@ -1748,6 +1756,194 @@ func (n *nftablesRunner) DelSNATRule() error { return nil } +func nativeUint32(v uint32) []byte { + b := make([]byte, 4) + binary.NativeEndian.PutUint32(b, v) + return b +} + +func makeStatefulRuleExprs(tunname string) []expr.Any { + return []expr.Any{ + // Check if the output interface is the Tailscale interface by + // first loding the OIFNAME into register 1 and comparing it + // against our tunname. + // + // 'cmp' implicitly breaks from a rule if a comparison fails, + // so if we continue past this rule we know that the packet is + // going to our TUN. + &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpNeq, + Register: 1, + Data: []byte(tunname), + }, + + // Store the conntrack state in register 1 + &expr.Ct{ + Register: 1, + Key: expr.CtKeySTATE, + }, + // Mask the state in register 1 to "hide" the ESTABLISHED and + // RELATED bits (which are expected and fine); if there are any + // other bits, we want them to remain. + // + // This operation is, in the kernel: + // dst[i] = (src[i] & mask[i]) ^ xor[i] + // + // So, we can mask by setting the inverse of the bits we want + // to remove; i.e. ESTABLISHED = 0b00000010, RELATED = + // 0b00000100, so, if we assume an 8-bit state (in reality, + // it's 32-bit), we can mask with 0b11111001 to clear those + // bits and keep everything else (e.g. the INVALID bit which is + // 0b00000001). + // + // TODO(andrew-d): for now, let's also allow + // CtStateBitUNTRACKED, which is a state for packets that are not + // tracked (marked so explicitly with an iptables rule using + // --notrack); we should figure out if we want to allow this or not. + &expr.Bitwise{ + SourceRegister: 1, + DestRegister: 1, + Len: 4, + Mask: nativeUint32(^(0 | + expr.CtStateBitESTABLISHED | + expr.CtStateBitRELATED | + expr.CtStateBitUNTRACKED)), + + // Xor is unused but must be specified + Xor: nativeUint32(0), + }, + // Compare against the expected state (0, i.e. no bits set + // other than maybe ESTABLISHED and RELATED). We want this + // comparison to fail if there are no bits set, so that this + // rule's evaluation stops and we don't fall through to the + // "Drop" verdict. + // + // For example, if the state is ESTABLISHED (and we want to + // break from this rule/accept this packet): + // state = ESTABLISHED + // register1 = 0b0 (since the bitwise operation cleared the ESTABLISHED bit) + // + // compare register1 (0b0) != 0: false + // -> comparison implicitly breaks + // -> continue to the next rule + // + // For example, if the state is NEW (and we want to continue to + // the next expression and thus drop this packet): + // state = NEW + // register1 = 0b1000 + // + // compare register1 (0b1000) != 0: true + // -> comparison continues to next expr + &expr.Cmp{ + Op: expr.CmpOpNeq, + Register: 1, + Data: []byte{0, 0, 0, 0}, + }, + // If we get here, we know that this packet is going to our TUN + // device, and has a conntrack state set other than ESTABLISHED + // or RELATED. We thus count and drop the packet. + &expr.Counter{}, + &expr.Verdict{Kind: expr.VerdictDrop}, + } + + // TODO(andrew-d): iptables-nft writes a rule that dumps as: + // + // match name conntrack rev 3 + // + // I think this is using expr.Match against the following struct + // (xt_conntrack_mtinfo3): + // + // https://github.com/torvalds/linux/blob/master/include/uapi/linux/netfilter/xt_conntrack.h#L64-L77 + // + // We could probably do something similar here, but I'm not sure if + // there's any advantage. Below is an example Match statement if we + // decide to do that, based on dumping the rule that iptables-nft + // generates: + // + // _ = expr.Match{ + // Name: "conntrack", + // Rev: 3, + // Info: &xt.ConntrackMtinfo3{ + // ConntrackMtinfo2: xt.ConntrackMtinfo2{ + // ConntrackMtinfoBase: xt.ConntrackMtinfoBase{ + // MatchFlags: xt.ConntrackState, + // InvertFlags: xt.ConntrackState, + // }, + // // Mask the state to remove ESTABLISHED and + // // RELATED before comparing. + // StateMask: expr.CtStateBitESTABLISHED | expr.CtStateBitRELATED, + // }, + // }, + // } +} + +// AddStatefulRule adds a netfilter rule for stateful packet filtering using +// conntrack. +func (n *nftablesRunner) AddStatefulRule(tunname string) error { + conn := n.conn + + exprs := makeStatefulRuleExprs(tunname) + for _, table := range n.getTables() { + chain, err := getChainFromTable(conn, table.Filter, chainNameForward) + if err != nil { + return fmt.Errorf("get forward chain: %w", err) + } + + // First, find the 'accept' rule that we want to insert our rule before. + acceptRule := createAcceptOutgoingPacketRule(table.Filter, chain, tunname) + rule, err := findRule(conn, acceptRule) + if err != nil { + return fmt.Errorf("find accept rule: %w", err) + } + + conn.InsertRule(&nftables.Rule{ + Table: table.Filter, + Chain: chain, + Exprs: exprs, + + // Specifying Position in an Insert operation means to + // insert this rule before the specified rule. + Position: rule.Handle, + }) + } + + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush add stateful rule: %w", err) + } + return nil +} + +// DelStatefulRule removes the netfilter rule for stateful packet filtering +// using conntrack. +func (n *nftablesRunner) DelStatefulRule(tunname string) error { + conn := n.conn + + exprs := makeStatefulRuleExprs(tunname) + for _, table := range n.getTables() { + chain, err := getChainFromTable(conn, table.Filter, chainNameForward) + if err != nil { + return fmt.Errorf("get forward chain: %w", err) + } + rule, err := findRule(conn, &nftables.Rule{ + Table: table.Nat, + Chain: chain, + Exprs: exprs, + }) + if err != nil { + return fmt.Errorf("find stateful rule: %w", err) + } + + if rule != nil { + conn.DelRule(rule) + } + } + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush del stateful rule: %w", err) + } + return nil +} + // cleanupChain removes a jump rule from hookChainName to tsChainName, and then // the entire chain tsChainName. Errors are logged, but attempts to remove both // the jump rule and chain continue even if one errors. diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 5200343cc..423008978 100644 --- a/wgengine/router/router.go +++ b/wgengine/router/router.go @@ -88,9 +88,10 @@ type Config struct { SubnetRoutes []netip.Prefix // Linux-only things below, ignored on other platforms. - SNATSubnetRoutes bool // SNAT traffic to local subnets - NetfilterMode preftype.NetfilterMode // how much to manage netfilter rules - NetfilterKind string // what kind of netfilter to use (nftables, iptables) + SNATSubnetRoutes bool // SNAT traffic to local subnets + StatefulFiltering bool // Apply stateful filtering to inbound connections + NetfilterMode preftype.NetfilterMode // how much to manage netfilter rules + NetfilterKind string // what kind of netfilter to use (nftables, iptables) } func (a *Config) Equal(b *Config) bool { diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index 4b1a0ae5f..0f154fd09 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -38,17 +38,18 @@ const ( ) type linuxRouter struct { - closed atomic.Bool - logf func(fmt string, args ...any) - tunname string - netMon *netmon.Monitor - unregNetMon func() - addrs map[netip.Prefix]bool - routes map[netip.Prefix]bool - localRoutes map[netip.Prefix]bool - snatSubnetRoutes bool - netfilterMode preftype.NetfilterMode - netfilterKind string + closed atomic.Bool + logf func(fmt string, args ...any) + tunname string + netMon *netmon.Monitor + unregNetMon func() + addrs map[netip.Prefix]bool + routes map[netip.Prefix]bool + localRoutes map[netip.Prefix]bool + snatSubnetRoutes bool + statefulFiltering bool + netfilterMode preftype.NetfilterMode + netfilterKind string // ruleRestorePending is whether a timer has been started to // restore deleted ip rules. @@ -390,6 +391,7 @@ func (r *linuxRouter) Set(cfg *Config) error { } r.addrs = newAddrs + // Ensure that the SNAT rule is added or removed as needed. switch { case cfg.SNATSubnetRoutes == r.snatSubnetRoutes: // state already correct, nothing to do. @@ -404,6 +406,21 @@ func (r *linuxRouter) Set(cfg *Config) error { } r.snatSubnetRoutes = cfg.SNATSubnetRoutes + // As above, for stateful filtering + switch { + case cfg.StatefulFiltering == r.statefulFiltering: + // state already correct, nothing to do. + case cfg.StatefulFiltering: + if err := r.addStatefulRule(); err != nil { + errs = append(errs, err) + } + default: + if err := r.delStatefulRule(); err != nil { + errs = append(errs, err) + } + } + r.statefulFiltering = cfg.StatefulFiltering + // Issue 11405: enable IP forwarding on gokrazy. advertisingRoutes := len(cfg.SubnetRoutes) > 0 if distro.Get() == distro.Gokrazy && advertisingRoutes { @@ -1327,6 +1344,26 @@ func (r *linuxRouter) delSNATRule() error { return nil } +// addStatefulRule adds a netfilter rule to perform stateful filtering from +// subnets onto the tailnet. +func (r *linuxRouter) addStatefulRule() error { + if r.netfilterMode == netfilterOff { + return nil + } + + return r.nfr.AddStatefulRule(r.tunname) +} + +// delStatefulRule removes the netfilter rule to perform stateful filtering +// from subnets onto the tailnet. +func (r *linuxRouter) delStatefulRule() error { + if r.netfilterMode == netfilterOff { + return nil + } + + return r.nfr.DelStatefulRule(r.tunname) +} + // cidrDiff calls add and del as needed to make the set of prefixes in // old and new match. Returns a map reflecting the actual new state // (which may be somewhere in between old and new if some commands diff --git a/wgengine/router/router_linux_test.go b/wgengine/router/router_linux_test.go index cf8e7f517..3dc5a4b8e 100644 --- a/wgengine/router/router_linux_test.go +++ b/wgengine/router/router_linux_test.go @@ -94,11 +94,49 @@ ip route add 192.168.16.0/24 dev tailscale0 table 52` + basic, { name: "addr and routes and subnet routes with netfilter", in: &Config{ - LocalAddrs: mustCIDRs("100.101.102.104/10"), - Routes: mustCIDRs("100.100.100.100/32", "10.0.0.0/8"), - SubnetRoutes: mustCIDRs("200.0.0.0/8"), - SNATSubnetRoutes: true, - NetfilterMode: netfilterOn, + LocalAddrs: mustCIDRs("100.101.102.104/10"), + Routes: mustCIDRs("100.100.100.100/32", "10.0.0.0/8"), + SubnetRoutes: mustCIDRs("200.0.0.0/8"), + SNATSubnetRoutes: true, + StatefulFiltering: true, + NetfilterMode: netfilterOn, + }, + want: ` +up +ip addr add 100.101.102.104/10 dev tailscale0 +ip route add 10.0.0.0/8 dev tailscale0 table 52 +ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic + + `v4/filter/FORWARD -j ts-forward +v4/filter/INPUT -j ts-input +v4/filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000/0xff0000 +v4/filter/ts-forward -m mark --mark 0x40000/0xff0000 -j ACCEPT +v4/filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP +v4/filter/ts-forward -o tailscale0 -m conntrack ! --ctstate ESTABLISHED,RELATED -j DROP +v4/filter/ts-forward -o tailscale0 -j ACCEPT +v4/filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT +v4/filter/ts-input ! -i tailscale0 -s 100.115.92.0/23 -j RETURN +v4/filter/ts-input ! -i tailscale0 -s 100.64.0.0/10 -j DROP +v4/nat/POSTROUTING -j ts-postrouting +v4/nat/ts-postrouting -m mark --mark 0x40000/0xff0000 -j MASQUERADE +v6/filter/FORWARD -j ts-forward +v6/filter/INPUT -j ts-input +v6/filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000/0xff0000 +v6/filter/ts-forward -m mark --mark 0x40000/0xff0000 -j ACCEPT +v6/filter/ts-forward -o tailscale0 -m conntrack ! --ctstate ESTABLISHED,RELATED -j DROP +v6/filter/ts-forward -o tailscale0 -j ACCEPT +v6/nat/POSTROUTING -j ts-postrouting +v6/nat/ts-postrouting -m mark --mark 0x40000/0xff0000 -j MASQUERADE +`, + }, + { + name: "addr and routes and subnet routes with netfilter but no stateful filtering", + in: &Config{ + LocalAddrs: mustCIDRs("100.101.102.104/10"), + Routes: mustCIDRs("100.100.100.100/32", "10.0.0.0/8"), + SubnetRoutes: mustCIDRs("200.0.0.0/8"), + SNATSubnetRoutes: true, + StatefulFiltering: false, + NetfilterMode: netfilterOn, }, want: ` up @@ -411,6 +449,22 @@ func insertRule(n *fakeIPTablesRunner, curIPT map[string][]string, chain, newRul return nil } +func insertRuleAt(n *fakeIPTablesRunner, curIPT map[string][]string, chain string, pos int, newRule string) { + rules, ok := curIPT[chain] + if !ok { + n.t.Fatalf("no %s chain exists", chain) + } + + // If the given position is after the end of the chain, error. + if pos > len(rules) { + n.t.Fatalf("position %d > len(chain %s) %d", pos, chain, len(chain)) + } + + // Insert the rule at the given position + rules = slices.Insert(rules, pos, newRule) + curIPT[chain] = rules +} + func appendRule(n *fakeIPTablesRunner, curIPT map[string][]string, chain, newRule string) error { // Get current rules for filter/ts-input chain with according IP version curTSInputRules, ok := curIPT[chain] @@ -611,6 +665,33 @@ func (n *fakeIPTablesRunner) DelSNATRule() error { return nil } +func (n *fakeIPTablesRunner) AddStatefulRule(tunname string) error { + newRule := fmt.Sprintf("-o %s -m conntrack ! --ctstate ESTABLISHED,RELATED -j DROP", tunname) + for _, ipt := range []map[string][]string{n.ipt4, n.ipt6} { + // Mimic the real runner and insert after the 'accept all' rule + wantRule := fmt.Sprintf("-o %s -j ACCEPT", tunname) + + const chain = "filter/ts-forward" + pos := slices.Index(ipt[chain], wantRule) + if pos < 0 { + n.t.Fatalf("no rule %q in chain %s", wantRule, chain) + } + + insertRuleAt(n, ipt, chain, pos, newRule) + } + return nil +} + +func (n *fakeIPTablesRunner) DelStatefulRule(tunname string) error { + delRule := fmt.Sprintf("-o %s -m conntrack ! --ctstate ESTABLISHED,RELATED -j DROP", tunname) + for _, ipt := range []map[string][]string{n.ipt4, n.ipt6} { + if err := deleteRule(n, ipt, "filter/ts-forward", delRule); err != nil { + return err + } + } + return nil +} + // buildMagicsockPortRule builds a fake rule to use in AddMagicsockPortRule and // DelMagicsockPortRule below. func buildMagicsockPortRule(port uint16) string { diff --git a/wgengine/router/router_test.go b/wgengine/router/router_test.go index 9c83acfe7..8842173d7 100644 --- a/wgengine/router/router_test.go +++ b/wgengine/router/router_test.go @@ -23,8 +23,8 @@ func mustCIDRs(ss ...string) []netip.Prefix { func TestConfigEqual(t *testing.T) { testedFields := []string{ "LocalAddrs", "Routes", "LocalRoutes", "NewMTU", - "SubnetRoutes", "SNATSubnetRoutes", "NetfilterMode", - "NetfilterKind", + "SubnetRoutes", "SNATSubnetRoutes", "StatefulFiltering", + "NetfilterMode", "NetfilterKind", } configType := reflect.TypeFor[Config]() configFields := []string{} @@ -125,6 +125,16 @@ func TestConfigEqual(t *testing.T) { &Config{SNATSubnetRoutes: false}, true, }, + { + &Config{StatefulFiltering: false}, + &Config{StatefulFiltering: true}, + false, + }, + { + &Config{StatefulFiltering: false}, + &Config{StatefulFiltering: false}, + true, + }, { &Config{NetfilterMode: preftype.NetfilterOff},