From 72c9f0f8201d22cff9b1564097c5862cf5364400 Mon Sep 17 00:00:00 2001 From: Andrea Gottardo Date: Wed, 28 Aug 2024 11:45:27 -0700 Subject: [PATCH] ipnlocal: support automatic exit node disablement when captive portal detected --- control/controlknobs/controlknobs.go | 7 ++++ ipn/ipnlocal/local.go | 60 ++++++++++++++++++++++++---- tailcfg/tailcfg.go | 4 ++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index dd76a3abd..043442162 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -103,6 +103,10 @@ type Knobs struct { // DisableCaptivePortalDetection is whether the node should not perform captive portal detection // automatically when the network state changes. DisableCaptivePortalDetection atomic.Bool + + // DisableExitNodeBehindCaptivePortal is whether the node should temporarily disable exit nodes + // whenever a captive portal is detected. + DisableExitNodeBehindCaptivePortal atomic.Bool } // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self @@ -132,6 +136,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { disableLocalDNSOverrideViaNRPT = has(tailcfg.NodeAttrDisableLocalDNSOverrideViaNRPT) disableCryptorouting = has(tailcfg.NodeAttrDisableMagicSockCryptoRouting) disableCaptivePortalDetection = has(tailcfg.NodeAttrDisableCaptivePortalDetection) + disableExitNodeBehindCaptivePortal = has(tailcfg.NodeAttrDisableExitNodeBehindCaptivePortal) ) if has(tailcfg.NodeAttrOneCGNATEnable) { @@ -159,6 +164,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { k.DisableLocalDNSOverrideViaNRPT.Store(disableLocalDNSOverrideViaNRPT) k.DisableCryptorouting.Store(disableCryptorouting) k.DisableCaptivePortalDetection.Store(disableCaptivePortalDetection) + k.DisableExitNodeBehindCaptivePortal.Store(disableExitNodeBehindCaptivePortal) } // AsDebugJSON returns k as something that can be marshalled with json.Marshal @@ -187,5 +193,6 @@ func (k *Knobs) AsDebugJSON() map[string]any { "DisableLocalDNSOverrideViaNRPT": k.DisableLocalDNSOverrideViaNRPT.Load(), "DisableCryptorouting": k.DisableCryptorouting.Load(), "DisableCaptivePortalDetection": k.DisableCaptivePortalDetection.Load(), + "DisableExitNodeBehindCaptivePortal": k.DisableExitNodeBehindCaptivePortal.Load(), } } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 6a0a094d4..7ea31f982 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -362,8 +362,13 @@ type LocalBackend struct { // captiveCtx will always be non-nil, though it might be a canceled // context. captiveCancel is non-nil if checkCaptivePortalLoop is // running, and is set to nil after being canceled. - captiveCtx context.Context - captiveCancel context.CancelFunc + // + // captiveDetected is true if the backend is currently behind a captive + // portal, and is used to temporarily disable exit nodes and other + // features that require connectivity. + captiveCtx context.Context + captiveCancel context.CancelFunc + captiveDetected bool // needsCaptiveDetection is a channel that is used to signal either // that captive portal detection is required (sending true) or that the // backend is healthy and captive portal detection is not required @@ -812,7 +817,7 @@ func (b *LocalBackend) onHealthChange(w *health.Warnable, us *health.UnhealthySt } else { // If connectivity is not impacted, we know for sure we're not behind a captive portal, // so drop any warning, and signal that we don't need captive portal detection. - b.health.SetHealthy(captivePortalWarnable) + b.setCaptivePortalDetected(false) select { case b.needsCaptiveDetection <- false: case <-ctx.Done(): @@ -820,6 +825,33 @@ func (b *LocalBackend) onHealthChange(w *health.Warnable, us *health.UnhealthySt } } +func (b *LocalBackend) setCaptivePortalDetected(found bool) { + b.mu.Lock() + prev := b.captiveDetected + b.captiveDetected = found + b.mu.Unlock() + b.logf("b.captiveDetected = %v, was %v", found, prev) + + if found { + b.health.SetUnhealthy(captivePortalWarnable, nil) + } else { + b.health.SetHealthy(captivePortalWarnable) + } + + if b.wantsExitNodeDisablementBehindCaptivePortal() && prev != found { + b.logf("b.captiveDetected changed to %v (was %v), calling authReconfig()", found, prev) + b.authReconfig() + } +} + +func (b *LocalBackend) wantsExitNodeDisablementBehindCaptivePortal() bool { + b.mu.Lock() + defer b.mu.Unlock() + res := b.ControlKnobs().DisableExitNodeBehindCaptivePortal.Load() + b.logf("wantsExitNodeDisablementBehindCaptivePortal = %v", res) + return res +} + // Shutdown halts the backend and all its sub-components. The backend // can no longer be used after Shutdown returns. func (b *LocalBackend) Shutdown() { @@ -2318,11 +2350,7 @@ func (b *LocalBackend) performCaptiveDetection() { netMon := b.NetMon() b.mu.Unlock() found := d.Detect(ctx, netMon, dm, preferredDERP) - if found { - b.health.SetUnhealthy(captivePortalWarnable, health.Args{}) - } else { - b.health.SetHealthy(captivePortalWarnable) - } + b.setCaptivePortalDetected(found) } // shouldRunCaptivePortalDetection reports whether captive portal detection @@ -4673,6 +4701,22 @@ func (b *LocalBackend) routerConfig(cfg *wgcfg.Config, prefs ipn.PrefsView, oneC b.logf("warning: ExitNodeAllowLANAccess has no effect on " + runtime.GOOS) } } + + b.logf("routerConfig: b.captiveDetected is %v", b.captiveDetected) + if b.captiveDetected && b.wantsExitNodeDisablementBehindCaptivePortal() { + isZeroRouteFunc := func(p netip.Prefix) bool { + return p == ipv4Default || p == ipv6Default + } + hasZeroRoutes := slices.ContainsFunc(rs.Routes, isZeroRouteFunc) + if hasZeroRoutes { + b.logf("captive portal detected, dropping zero routes") + // If a captive portal is present, remove the zero routes (ipv4Default and ipv6Default) + // to allow the user to authenticate with the captive portal. + rs.Routes = slices.DeleteFunc(rs.Routes, isZeroRouteFunc) + } else { + b.logf("captive portal detected, but no zero routes to drop") + } + } } if slices.ContainsFunc(rs.LocalAddrs, tsaddr.PrefixIs4) { diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 0d4fae3d5..39d89fb87 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2346,6 +2346,10 @@ const ( // NodeAttrSSHEnvironmentVariables enables logic for handling environment variables sent // via SendEnv in the SSH server and applying them to the SSH session. NodeAttrSSHEnvironmentVariables NodeCapability = "ssh-env-vars" + + // NodeAttrDisableExitNodeBehindCaptivePortal instructs the client to temporarily disable exit nodes when + // behind a captive portal. + NodeAttrDisableExitNodeBehindCaptivePortal NodeCapability = "disable-exit-node-behind-captive-portal" ) // SetDNSRequest is a request to add a DNS record.