ipn/ipnlocal: warn incompatibility between no-snat-routes and exitnode (#19023)

* ipn/ipnlocal: warn incompatibility between no-snat-routes and exitnode

This commit adds a warning to health check when the --snat-subnet-routes=false flag for subnet router is
set alone side --advertise-exit-node=true. These two would conflict with each other and result internet-bound
traffic from peers using this exit node no masqueraded to the node's source IP and fail to route return
packets back. The described combination is not valid until we figure out a way to separate exitnode masquerade rule and skip it for subnet routes.

Updates #18725

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

* use date instead of for now to clarify effectivness

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>

---------

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>
This commit is contained in:
KevinLiang10
2026-03-26 12:36:31 -04:00
committed by GitHub
parent b4519e97c3
commit 45f989f52a
4 changed files with 107 additions and 0 deletions

View File

@@ -265,6 +265,11 @@ func runSet(ctx context.Context, args []string) (retErr error) {
checkPrefs := curPrefs.Clone()
checkPrefs.ApplyEdits(maskedPrefs)
// We want to make sure user is aware setting --snat-subnet-routes=false with --advertise-exit-node would break exitnode,
// but we won't prevent them from doing it since there are current dependencies on that combination. (as of 2026-03-25)
if checkPrefs.NoSNAT && checkPrefs.AdvertisesExitNode() {
warnf("--snat-subnet-routes=false is set with --advertise-exit-node; internet traffic through this exit node may not work as expected")
}
if err := localClient.CheckPrefs(ctx, checkPrefs); err != nil {
return err
}

View File

@@ -357,6 +357,11 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo
if goos == "linux" {
prefs.NoSNAT = !upArgs.snat
// We want to make sure user is aware setting --snat-subnet-routes=false with --advertise-exit-node would break exitnode,
// but we won't prevent them from doing it since there are current dependencies on that combination. (as of 2026-03-25)
if prefs.NoSNAT && prefs.AdvertisesExitNode() {
warnf("--snat-subnet-routes=false is set with --advertise-exit-node; internet traffic through this exit node may not work as expected")
}
// Backfills for NoStatefulFiltering occur when loading a profile; just set it explicitly here.
prefs.NoStatefulFiltering.Set(!upArgs.statefulFiltering)

View File

@@ -2576,6 +2576,7 @@ func (b *LocalBackend) startLocked(opts ipn.Options) error {
// Reset the always-on override whenever Start is called.
b.resetAlwaysOnOverrideLocked()
b.setAtomicValuesFromPrefsLocked(prefs)
b.updateNoSNATExitNodeWarning(prefs)
wantRunning := prefs.WantRunning()
if wantRunning {
@@ -4745,6 +4746,7 @@ func (b *LocalBackend) setPrefsLocked(newp *ipn.Prefs) ipn.PrefsView {
b.pauseOrResumeControlClientLocked() // for prefs.Sync changes
b.updateWarnSync(prefs)
b.updateNoSNATExitNodeWarning(prefs)
if oldp.ShieldsUp() != newp.ShieldsUp || hostInfoChanged {
b.doSetHostinfoFilterServicesLocked()
@@ -6984,6 +6986,18 @@ func (b *LocalBackend) sshServerOrInit() (_ SSHServer, err error) {
Text: health.StaticMessage("SELinux is enabled; Tailscale SSH may not work. See https://tailscale.com/s/ssh-selinux"),
})
// warnNoSNATWithExitNode is a warnable for when a node is advertising as an
// exit node but has SNAT disabled. In this configuration internet-bound traffic
// from peers using this exit node will not be masqueraded to the node's own
// source IP, so return packets cannot be routed back, causing the exit node to
// not work as expected.
var warnNoSNATWithExitNode = health.Register(&health.Warnable{
Code: "nosnat-with-advertised-exit-node",
Title: "Exit node advertising may not work correctly",
Severity: health.SeverityMedium,
Text: health.StaticMessage("snat-subnet-routes is disabled while advertising as an exit node; internet traffic through this exit node may not work as expected"),
})
func (b *LocalBackend) updateSELinuxHealthWarning() {
if hostinfo.IsSELinuxEnforcing() {
b.health.SetUnhealthy(warnSSHSELinuxWarnable, nil)
@@ -7000,6 +7014,17 @@ func (b *LocalBackend) updateWarnSync(prefs ipn.PrefsView) {
}
}
func (b *LocalBackend) updateNoSNATExitNodeWarning(prefs ipn.PrefsView) {
if !buildfeatures.HasAdvertiseExitNode {
return
}
if prefs.NoSNAT() && prefs.AdvertisesExitNode() {
b.health.SetUnhealthy(warnNoSNATWithExitNode, nil)
} else {
b.health.SetHealthy(warnNoSNATWithExitNode)
}
}
func (b *LocalBackend) handleSSHConn(c net.Conn) (err error) {
s, err := b.sshServerOrInit()
if err != nil {

View File

@@ -7878,3 +7878,75 @@ func TestEditPrefs_InvalidAdvertiseRoutes(t *testing.T) {
})
}
}
func TestNoSNATWithAdvertisedExitNodeWarning(t *testing.T) {
exitRoutes := []netip.Prefix{
netip.MustParsePrefix("0.0.0.0/0"),
netip.MustParsePrefix("::/0"),
}
warnCode := health.WarnableCode("nosnat-with-advertised-exit-node")
tests := []struct {
name string
prefs *ipn.Prefs
wantWarning bool
}{
{
name: "no-snat-without-exit-node",
prefs: &ipn.Prefs{
NoSNAT: true,
AdvertiseRoutes: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")},
},
wantWarning: false,
},
{
name: "snat-enabled-with-exit-node",
prefs: &ipn.Prefs{
NoSNAT: false,
AdvertiseRoutes: exitRoutes,
},
wantWarning: false,
},
{
name: "no-snat-with-exit-node",
prefs: &ipn.Prefs{
NoSNAT: true,
AdvertiseRoutes: exitRoutes,
},
wantWarning: true,
},
{
name: "no-snat-with-exit-node-and-subnet",
prefs: &ipn.Prefs{
NoSNAT: true,
AdvertiseRoutes: append([]netip.Prefix{
netip.MustParsePrefix("10.0.0.0/24"),
}, exitRoutes...),
},
wantWarning: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := newTestLocalBackend(t)
b.SetPrefsForTest(tt.prefs)
_, hasWarning := b.HealthTracker().CurrentState().Warnings[warnCode]
if hasWarning != tt.wantWarning {
t.Errorf("warning present = %v, want %v", hasWarning, tt.wantWarning)
}
})
}
// Verify that the warning clears when the conflicting combination is resolved.
t.Run("warning-clears-on-fix", func(t *testing.T) {
b := newTestLocalBackend(t)
b.SetPrefsForTest(&ipn.Prefs{NoSNAT: true, AdvertiseRoutes: exitRoutes})
if _, ok := b.HealthTracker().CurrentState().Warnings[warnCode]; !ok {
t.Fatal("expected warning to be set")
}
b.SetPrefsForTest(&ipn.Prefs{NoSNAT: false, AdvertiseRoutes: exitRoutes})
if _, ok := b.HealthTracker().CurrentState().Warnings[warnCode]; ok {
t.Fatal("expected warning to be cleared after enabling SNAT")
}
})
}