diff --git a/VERSION.txt b/VERSION.txt index 3cb930ab0301b..26c8251f61486 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.66.3 +1.66.4 diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index c497076125890..fa329a0fdbc7e 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -24,6 +24,7 @@ import ( "tailscale.com/tka" "tailscale.com/tstest" "tailscale.com/types/logger" + "tailscale.com/types/opt" "tailscale.com/types/persist" "tailscale.com/types/preftype" "tailscale.com/version/distro" @@ -176,9 +177,10 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "bare_up_means_up", flags: []string{}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - WantRunning: false, - Hostname: "foo", + ControlURL: ipn.DefaultControlURL, + WantRunning: false, + Hostname: "foo", + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -186,12 +188,13 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "losing_hostname", flags: []string{"--accept-dns"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - WantRunning: false, - Hostname: "foo", - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, - AllowSingleHosts: true, + ControlURL: ipn.DefaultControlURL, + WantRunning: false, + Hostname: "foo", + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + AllowSingleHosts: true, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --accept-dns --hostname=foo", }, @@ -199,11 +202,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "hostname_changing_explicitly", flags: []string{"--hostname=bar"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, - AllowSingleHosts: true, - Hostname: "foo", + ControlURL: ipn.DefaultControlURL, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + AllowSingleHosts: true, + Hostname: "foo", + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -211,11 +215,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "hostname_changing_empty_explicitly", flags: []string{"--hostname="}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, - AllowSingleHosts: true, - Hostname: "foo", + ControlURL: ipn.DefaultControlURL, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + AllowSingleHosts: true, + Hostname: "foo", + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -231,11 +236,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "implicit_operator_change", flags: []string{"--hostname=foo"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - OperatorUser: "alice", - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: ipn.DefaultControlURL, + OperatorUser: "alice", + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, curUser: "eve", want: accidentalUpPrefix + " --hostname=foo --operator=alice", @@ -244,11 +250,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "implicit_operator_matches_shell_user", flags: []string{"--hostname=foo"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, - OperatorUser: "alice", + ControlURL: ipn.DefaultControlURL, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + OperatorUser: "alice", + NoStatefulFiltering: opt.NewBool(true), }, curUser: "alice", want: "", @@ -266,6 +273,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --advertise-routes=10.0.42.0/24 --advertise-exit-node", }, @@ -282,6 +290,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -298,6 +307,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -305,10 +315,11 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "advertise_exit_node", // Issue 1859 flags: []string{"--advertise-exit-node"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: ipn.DefaultControlURL, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -324,6 +335,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { AdvertiseRoutes: []netip.Prefix{ netip.MustParsePrefix("1.2.0.0/16"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --advertise-exit-node --advertise-routes=1.2.0.0/16", }, @@ -340,6 +352,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("::/0"), netip.MustParsePrefix("1.2.0.0/16"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --advertise-exit-node --advertise-routes=1.2.0.0/16", }, @@ -352,7 +365,8 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { CorpDNS: true, NetfilterMode: preftype.NetfilterOn, - ExitNodeID: "fooID", + ExitNodeID: "fooID", + NoStatefulFiltering: opt.NewBool(true), }, want: "", }, @@ -375,8 +389,9 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, - NetfilterMode: preftype.NetfilterNoDivert, - OperatorUser: "alice", + NetfilterMode: preftype.NetfilterNoDivert, + OperatorUser: "alice", + NoStatefulFiltering: opt.NewBool(true), }, curUser: "eve", want: accidentalUpPrefix + " --force-reauth --accept-dns=false --accept-routes --advertise-exit-node --advertise-routes=10.0.0.0/16 --advertise-tags=tag:foo,tag:bar --exit-node=100.64.5.6 --host-routes=false --hostname=myhostname --netfilter-mode=nodivert --operator=alice --shields-up", @@ -398,8 +413,9 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { AdvertiseRoutes: []netip.Prefix{ netip.MustParsePrefix("10.0.0.0/16"), }, - NetfilterMode: preftype.NetfilterNoDivert, - OperatorUser: "alice", + NetfilterMode: preftype.NetfilterNoDivert, + OperatorUser: "alice", + NoStatefulFiltering: opt.NewBool(true), }, curUser: "eve", want: accidentalUpPrefix + " --hostname=newhostname --accept-dns=false --accept-routes --advertise-routes=10.0.0.0/16 --advertise-tags=tag:foo,tag:bar --exit-node=100.64.5.6 --host-routes=false --netfilter-mode=nodivert --operator=alice --shields-up", @@ -408,11 +424,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "loggedout_is_implicit", flags: []string{"--hostname=foo"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - LoggedOut: true, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: ipn.DefaultControlURL, + LoggedOut: true, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, want: "", // not an error. LoggedOut is implicit. }, @@ -458,6 +475,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("::/0"), netip.MustParsePrefix("1.2.0.0/16"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --operator=expbits --advertise-exit-node --advertise-routes=1.2.0.0/16", }, @@ -474,6 +492,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { netip.MustParsePrefix("::/0"), netip.MustParsePrefix("1.2.0.0/16"), }, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --advertise-routes=1.2.0.0/16 --operator=expbits --advertise-exit-node", }, @@ -487,7 +506,8 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { NetfilterMode: preftype.NetfilterOn, AllowSingleHosts: true, - Hostname: "foo", + Hostname: "foo", + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --auth-key=secretrand --force-reauth=false --reset --hostname=foo", }, @@ -500,7 +520,8 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { CorpDNS: true, NetfilterMode: preftype.NetfilterOn, - ExitNodeIP: netip.MustParseAddr("100.64.5.4"), + ExitNodeIP: netip.MustParseAddr("100.64.5.4"), + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --hostname=foo --exit-node=100.64.5.4", }, @@ -514,7 +535,8 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { CorpDNS: true, NetfilterMode: preftype.NetfilterOn, - ExitNodeID: "some_stable_id", + ExitNodeID: "some_stable_id", + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --hostname=foo --exit-node=100.64.5.7", }, @@ -530,6 +552,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { ExitNodeAllowLANAccess: true, ExitNodeID: "some_stable_id", + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --hostname=foo --exit-node-allow-lan-access --exit-node=100.2.3.4", }, @@ -537,10 +560,11 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "ignore_login_server_synonym", flags: []string{"--login-server=https://controlplane.tailscale.com"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, want: "", // not an error }, @@ -548,10 +572,11 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "ignore_login_server_synonym_on_other_change", flags: []string{"--netfilter-mode=off"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - AllowSingleHosts: true, - CorpDNS: false, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + AllowSingleHosts: true, + CorpDNS: false, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, want: accidentalUpPrefix + " --netfilter-mode=off --accept-dns=false", }, @@ -561,11 +586,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "synology_permit_omit_accept_routes", flags: []string{"--hostname=foo"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - CorpDNS: true, - AllowSingleHosts: true, - RouteAll: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + CorpDNS: true, + AllowSingleHosts: true, + RouteAll: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, goos: "linux", distro: distro.Synology, @@ -577,11 +603,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "not_synology_dont_permit_omit_accept_routes", flags: []string{"--hostname=foo"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - CorpDNS: true, - AllowSingleHosts: true, - RouteAll: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + CorpDNS: true, + AllowSingleHosts: true, + RouteAll: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, goos: "linux", distro: "", // not Synology @@ -591,11 +618,12 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { name: "profile_name_ignored_in_up", flags: []string{"--hostname=foo"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - CorpDNS: true, - AllowSingleHosts: true, - NetfilterMode: preftype.NetfilterOn, - ProfileName: "foo", + ControlURL: "https://login.tailscale.com", + CorpDNS: true, + AllowSingleHosts: true, + NetfilterMode: preftype.NetfilterOn, + ProfileName: "foo", + NoStatefulFiltering: opt.NewBool(true), }, goos: "linux", want: "", @@ -658,7 +686,7 @@ func TestPrefsFromUpArgs(t *testing.T) { ControlURL: ipn.DefaultControlURL, WantRunning: true, NoSNAT: false, - NoStatefulFiltering: "false", + NoStatefulFiltering: "true", NetfilterMode: preftype.NetfilterOn, CorpDNS: true, AllowSingleHosts: true, @@ -678,7 +706,7 @@ func TestPrefsFromUpArgs(t *testing.T) { AllowSingleHosts: true, RouteAll: true, NoSNAT: false, - NoStatefulFiltering: "false", + NoStatefulFiltering: "true", NetfilterMode: preftype.NetfilterOn, AutoUpdate: ipn.AutoUpdatePrefs{ Check: true, @@ -697,7 +725,7 @@ func TestPrefsFromUpArgs(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, - NoStatefulFiltering: "false", + NoStatefulFiltering: "true", NetfilterMode: preftype.NetfilterOn, AutoUpdate: ipn.AutoUpdatePrefs{ Check: true, @@ -1062,11 +1090,12 @@ func TestUpdatePrefs(t *testing.T) { name: "change_login_server", flags: []string{"--login-server=https://localhost:1000"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, env: upCheckEnv{backendState: "Running"}, wantSimpleUp: true, @@ -1077,11 +1106,12 @@ func TestUpdatePrefs(t *testing.T) { name: "change_tags", flags: []string{"--advertise-tags=tag:foo"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, env: upCheckEnv{backendState: "Running"}, }, @@ -1090,11 +1120,12 @@ func TestUpdatePrefs(t *testing.T) { name: "explicit_empty_operator", flags: []string{"--operator="}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - CorpDNS: true, - AllowSingleHosts: true, - NetfilterMode: preftype.NetfilterOn, - OperatorUser: "somebody", + ControlURL: "https://login.tailscale.com", + CorpDNS: true, + AllowSingleHosts: true, + NetfilterMode: preftype.NetfilterOn, + OperatorUser: "somebody", + NoStatefulFiltering: opt.NewBool(true), }, env: upCheckEnv{user: "somebody", backendState: "Running"}, wantJustEditMP: &ipn.MaskedPrefs{ @@ -1111,11 +1142,12 @@ func TestUpdatePrefs(t *testing.T) { name: "enable_ssh", flags: []string{"--ssh"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ RunSSHSet: true, @@ -1132,12 +1164,13 @@ func TestUpdatePrefs(t *testing.T) { name: "disable_ssh", flags: []string{"--ssh=false"}, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - RunSSH: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + RunSSH: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ RunSSHSet: true, @@ -1157,12 +1190,13 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--ssh=false"}, sshOverTailscale: true, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, - RunSSH: true, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + RunSSH: true, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ RunSSHSet: true, @@ -1181,11 +1215,12 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--ssh=true"}, sshOverTailscale: true, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ RunSSHSet: true, @@ -1204,11 +1239,12 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--ssh=true", "--accept-risk=lose-ssh"}, sshOverTailscale: true, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ RunSSHSet: true, @@ -1226,12 +1262,13 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--ssh=false", "--accept-risk=lose-ssh"}, sshOverTailscale: true, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, - AllowSingleHosts: true, - CorpDNS: true, - RunSSH: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, + AllowSingleHosts: true, + CorpDNS: true, + RunSSH: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ RunSSHSet: true, @@ -1249,10 +1286,11 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--force-reauth"}, sshOverTailscale: true, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, env: upCheckEnv{backendState: "Running"}, wantErrSubtr: "aborted, no changes made", @@ -1262,10 +1300,11 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--force-reauth", "--accept-risk=lose-ssh"}, sshOverTailscale: true, curPrefs: &ipn.Prefs{ - ControlURL: "https://login.tailscale.com", - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: "https://login.tailscale.com", + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: nil, env: upCheckEnv{backendState: "Running"}, @@ -1274,10 +1313,11 @@ func TestUpdatePrefs(t *testing.T) { name: "advertise_connector", flags: []string{"--advertise-connector"}, curPrefs: &ipn.Prefs{ - ControlURL: ipn.DefaultControlURL, - AllowSingleHosts: true, - CorpDNS: true, - NetfilterMode: preftype.NetfilterOn, + ControlURL: ipn.DefaultControlURL, + AllowSingleHosts: true, + CorpDNS: true, + NetfilterMode: preftype.NetfilterOn, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ AppConnectorSet: true, @@ -1301,6 +1341,7 @@ func TestUpdatePrefs(t *testing.T) { AppConnector: ipn.AppConnectorPrefs{ Advertise: true, }, + NoStatefulFiltering: opt.NewBool(true), }, wantJustEditMP: &ipn.MaskedPrefs{ AppConnectorSet: true, diff --git a/cmd/tailscale/cli/set.go b/cmd/tailscale/cli/set.go index 1261056690fc5..0e9d35924174c 100644 --- a/cmd/tailscale/cli/set.go +++ b/cmd/tailscale/cli/set.go @@ -103,7 +103,7 @@ func newSetFlagSet(goos string, setArgs *setArgsT) *flag.FlagSet { switch goos { case "linux": setf.BoolVar(&setArgs.snat, "snat-subnet-routes", true, "source NAT traffic to local routes advertised with --advertise-routes") - setf.BoolVar(&setArgs.statefulFiltering, "stateful-filtering", true, "apply stateful filtering to forwarded packets (subnet routers, exit nodes, etc.)") + setf.BoolVar(&setArgs.statefulFiltering, "stateful-filtering", false, "apply stateful filtering to forwarded packets (subnet routers, exit nodes, etc.)") setf.StringVar(&setArgs.netfilterMode, "netfilter-mode", defaultNetfilterMode(), "netfilter mode (one of on, nodivert, off)") case "windows": setf.BoolVar(&setArgs.forceDaemon, "unattended", false, "run in \"Unattended Mode\" where Tailscale keeps running even after the current GUI user logs out (Windows-only)") diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index 16b28233ed193..5a70285e1dfb4 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -121,7 +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.BoolVar(&upArgs.statefulFiltering, "stateful-filtering", false, "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)") diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 2ca261f201875..c9aaa4d2e747c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -4189,18 +4189,7 @@ func (b *LocalBackend) routerConfig(cfg *wgcfg.Config, prefs ipn.PrefsView, oneC } 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 { + if v, ok := prefs.NoStatefulFiltering().Get(); ok && !v { // The preferences explicitly "do stateful filtering" is turned // off, or to expand the double negative, to do stateful // filtering. Do so. diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index d50a141ad2811..b01ff836d8420 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -354,10 +354,6 @@ func (pm *profileManager) loadSavedPrefs(key ipn.StateKey) (ipn.PrefsView, error return ipn.PrefsView{}, err } savedPrefs := ipn.NewPrefs() - // NewPrefs sets a default NoStatefulFiltering, but we want to actually see - // if the saved state had an empty value. The empty value gets migrated - // based on NoSNAT, while a default "false" does not. - savedPrefs.NoStatefulFiltering = "" if err := ipn.PrefsFromBytes(bs, savedPrefs); err != nil { return ipn.PrefsView{}, fmt.Errorf("parsing saved prefs: %v", err) } @@ -382,32 +378,6 @@ func (pm *profileManager) loadSavedPrefs(key ipn.StateKey) (ipn.PrefsView, error 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 } diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 113aaec65d17d..01d49c2300082 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -4,7 +4,6 @@ package ipnlocal import ( - "encoding/json" "fmt" "os/user" "strconv" @@ -13,14 +12,12 @@ 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" ) @@ -604,89 +601,6 @@ func TestProfileManagementWindows(t *testing.T) { } } -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=%q, want %v", pf.NoStatefulFiltering(), tt.want) - } - }) - } -} - // TestDefaultPrefs tests that defaultPrefs is just NewPrefs with // LoggedOut=true (the Prefs we use before connecting to control). We shouldn't // be putting any defaulting there, and instead put all defaults in NewPrefs. diff --git a/ipn/prefs.go b/ipn/prefs.go index 3a603ecf26754..8fd3bf85e2915 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -203,17 +203,16 @@ type Prefs struct { // Linux-only. NoSNAT bool - // NoStatefulFiltering specifies whether to apply stateful filtering - // when advertising routes in AdvertiseRoutes. The default is to apply + // NoStatefulFiltering specifies whether to apply stateful filtering when + // advertising routes in AdvertiseRoutes. The default is to not 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. + // This is an opt.Bool because it was first added after NoSNAT, with a + // backfill based on the value of that parameter. The backfill has been + // removed since then, but the field remains an opt.Bool. // // Linux-only. NoStatefulFiltering opt.Bool `json:",omitempty"` @@ -667,7 +666,7 @@ func NewPrefs() *Prefs { CorpDNS: true, WantRunning: false, NetfilterMode: preftype.NetfilterOn, - NoStatefulFiltering: opt.NewBool(false), + NoStatefulFiltering: opt.NewBool(true), AutoUpdate: AutoUpdatePrefs{ Check: true, Apply: opt.Bool("unset"), diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index ebf06527a78e9..06296815d9556 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -915,8 +915,6 @@ func (t *Wrapper) Read(buffs [][]byte, sizes []int, offset int) (int, error) { for _, data := range res.data { p.Decode(data[res.dataOffset:]) - pc.snat(p) - if m := t.destIPActivity.Load(); m != nil { if fn := m[p.Dst.Addr()]; fn != nil { fn() @@ -932,6 +930,10 @@ func (t *Wrapper) Read(buffs [][]byte, sizes []int, offset int) (int, error) { continue } } + + // Make sure to do SNAT after filtering, so that any flow tracking in + // the filter sees the original source address. See #12133. + pc.snat(p) n := copy(buffs[buffsPos][offset:], p.Buffer()) if n != len(data)-res.dataOffset { panic(fmt.Sprintf("short copy: %d != %d", n, len(data)-res.dataOffset)) diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index c7763229d7355..41c5b128c8ac4 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -469,7 +469,7 @@ func (r *linuxRouter) updateStatefulFilteringWithDockerWarning(cfg *Config) { if _, found := ifstate.Interface["docker0"]; found { r.health.SetWarnable(warnStatefulFilteringWithDocker, fmt.Errorf(""+ "Stateful filtering is enabled and Docker was detected; this may prevent Docker containers "+ - "on this host from connecting to Tailscale nodes. "+ + "on this host from resolving DNS and connecting to Tailscale nodes. "+ "See https://tailscale.com/s/stateful-docker", )) return