From d2ccfa4eddd45d2fd2563b82146f50ad92539b50 Mon Sep 17 00:00:00 2001 From: Mario Minardi Date: Mon, 18 Mar 2024 15:47:21 -0600 Subject: [PATCH] cmd/tailscale,ipn/ipnlocal: enable web client over quad 100 by default (#11419) Enable the web client over 100.100.100.100 by default. Accepting traffic from [tailnet IP]:5252 still requires setting the `webclient` user pref. Updates https://github.com/tailscale/tailscale/issues/10261 Signed-off-by: Mario Minardi --- cmd/tailscale/cli/set.go | 2 +- ipn/ipnlocal/local.go | 56 ++++++++++++++++++++++++++++++---------- ipn/prefs.go | 3 ++- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/cmd/tailscale/cli/set.go b/cmd/tailscale/cli/set.go index 02d4f5a06..4049eb12e 100644 --- a/cmd/tailscale/cli/set.go +++ b/cmd/tailscale/cli/set.go @@ -75,7 +75,7 @@ func newSetFlagSet(goos string, setArgs *setArgsT) *flag.FlagSet { setf.BoolVar(&setArgs.updateCheck, "update-check", true, "notify about available Tailscale updates") setf.BoolVar(&setArgs.updateApply, "auto-update", false, "automatically update to the latest available version") setf.BoolVar(&setArgs.postureChecking, "posture-checking", false, "HIDDEN: allow management plane to gather device posture information") - setf.BoolVar(&setArgs.runWebClient, "webclient", false, "run a web interface for managing this node, served over Tailscale at port 5252") + setf.BoolVar(&setArgs.runWebClient, "webclient", false, "expose the web interface for managing this node over Tailscale at port 5252") if safesocket.GOOSUsesPeerCreds(goos) { setf.StringVar(&setArgs.opUser, "operator", "", "Unix username to allow to operate on tailscaled without sudo") diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index aee9a9f87..47a892c11 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -176,10 +176,15 @@ type LocalBackend struct { logFlushFunc func() // or nil if SetLogFlusher wasn't called em *expiryManager // non-nil sshAtomicBool atomic.Bool - webClientAtomicBool atomic.Bool - shutdownCalled bool // if Shutdown has been called - debugSink *capture.Sink - sockstatLogger *sockstatlog.Logger + // webClientAtomicBool controls whether the web client is running. This should + // be true unless the disable-web-client node attribute has been set. + webClientAtomicBool atomic.Bool + // exposeRemoteWebClientAtomicBool controls whether the web client is exposed over + // Tailscale on port 5252. + exposeRemoteWebClientAtomicBool atomic.Bool + shutdownCalled bool // if Shutdown has been called + debugSink *capture.Sink + sockstatLogger *sockstatlog.Logger // getTCPHandlerForFunnelFlow returns a handler for an incoming TCP flow for // the provided srcAddr and dstPort if one exists. @@ -1160,7 +1165,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control // Perform all reconfiguration based on the netmap here. if st.NetMap != nil { b.capTailnetLock = hasCapability(st.NetMap, tailcfg.CapabilityTailnetLock) - b.setWebClientAtomicBoolLocked(st.NetMap, prefs.View()) + b.setWebClientAtomicBoolLocked(st.NetMap) b.mu.Unlock() // respect locking rules for tkaSyncIfNeeded if err := b.tkaSyncIfNeeded(st.NetMap, prefs.View()); err != nil { @@ -2719,11 +2724,12 @@ func (b *LocalBackend) setTCPPortsIntercepted(ports []uint16) { b.shouldInterceptTCPPortAtomic.Store(f) } -// setAtomicValuesFromPrefsLocked populates sshAtomicBool, containsViaIPFuncAtomic -// and shouldInterceptTCPPortAtomic from the prefs p, which may be !Valid(). +// setAtomicValuesFromPrefsLocked populates sshAtomicBool, containsViaIPFuncAtomic, +// shouldInterceptTCPPortAtomic, and exposeRemoteWebClientAtomicBool from the prefs p, +// which may be !Valid(). func (b *LocalBackend) setAtomicValuesFromPrefsLocked(p ipn.PrefsView) { b.sshAtomicBool.Store(p.Valid() && p.RunSSH() && envknob.CanSSHD()) - b.setWebClientAtomicBoolLocked(b.netMap, p) + b.setExposeRemoteWebClientAtomicBoolLocked(p) if !p.Valid() { b.containsViaIPFuncAtomic.Store(tsaddr.FalseContainsIPFunc()) @@ -3356,6 +3362,8 @@ func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c if hittingServiceIP { switch dst.Port() { case 80: + // TODO(mpminardi): do we want to show an error message if the web client + // has been disabled instead of the more "basic" web UI? if b.ShouldRunWebClient() { return b.handleWebClientConn, opts } @@ -3380,7 +3388,7 @@ func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c return b.handleSSHConn, opts } // TODO(will,sonia): allow customizing web client port ? - if dst.Port() == webClientPort && b.ShouldRunWebClient() { + if dst.Port() == webClientPort && b.ShouldExposeRemoteWebClient() { return b.handleWebClientConn, opts } if port, ok := b.GetPeerAPIPort(dst.Addr()); ok && dst.Port() == port { @@ -4508,19 +4516,39 @@ func (b *LocalBackend) ShouldRunSSH() bool { return b.sshAtomicBool.Load() && en // call regardless of whether b.mu is held or not. func (b *LocalBackend) ShouldRunWebClient() bool { return b.webClientAtomicBool.Load() } +// ShouldExposeRemoteWebClient reports whether the web client should +// accept connections via [tailscale IP]:5252 in addition to the default +// behaviour of accepting local connections over 100.100.100.100. +// +// This function checks both the web client user pref via +// exposeRemoteWebClientAtomicBool and the disable-web-client node attr +// via ShouldRunWebClient to determine whether the web client should be +// exposed. +func (b *LocalBackend) ShouldExposeRemoteWebClient() bool { + return b.ShouldRunWebClient() && b.exposeRemoteWebClientAtomicBool.Load() +} + // setWebClientAtomicBoolLocked sets webClientAtomicBool based on whether -// the RunWebClient pref is set, and whether tailcfg.NodeAttrDisableWebClient -// has been set in the netmap.NetworkMap. +// tailcfg.NodeAttrDisableWebClient has been set in the netmap.NetworkMap. // // b.mu must be held. -func (b *LocalBackend) setWebClientAtomicBoolLocked(nm *netmap.NetworkMap, prefs ipn.PrefsView) { - shouldRun := prefs.Valid() && prefs.RunWebClient() && !hasCapability(nm, tailcfg.NodeAttrDisableWebClient) +func (b *LocalBackend) setWebClientAtomicBoolLocked(nm *netmap.NetworkMap) { + shouldRun := !hasCapability(nm, tailcfg.NodeAttrDisableWebClient) wasRunning := b.webClientAtomicBool.Swap(shouldRun) if wasRunning && !shouldRun { go b.webClientShutdown() // stop web client } } +// setExposeRemoteWebClientAtomicBoolLocked sets exposeRemoteWebClientAtomicBool +// based on whether the RunWebClient pref is set. +// +// b.mu must be held. +func (b *LocalBackend) setExposeRemoteWebClientAtomicBoolLocked(prefs ipn.PrefsView) { + shouldExpose := prefs.Valid() && prefs.RunWebClient() + b.exposeRemoteWebClientAtomicBool.Store(shouldExpose) +} + // ShouldHandleViaIP reports whether ip is an IPv6 address in the // Tailscale ULA's v6 "via" range embedding an IPv4 address to be forwarded to // by Tailscale. @@ -4808,7 +4836,7 @@ func (b *LocalBackend) setTCPPortsInterceptedFromNetmapAndPrefsLocked(prefs ipn. if prefs.Valid() && prefs.RunSSH() && envknob.CanSSHD() { handlePorts = append(handlePorts, 22) } - if b.ShouldRunWebClient() { + if b.ShouldExposeRemoteWebClient() { handlePorts = append(handlePorts, webClientPort) // don't listen on netmap addresses if we're in userspace mode diff --git a/ipn/prefs.go b/ipn/prefs.go index ab0bf2cf7..0055b7fb2 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -118,7 +118,8 @@ type Prefs struct { // policies as configured by the Tailnet's admin(s). RunSSH bool - // RunWebClient bool is whether this node should run a web client, + // RunWebClient bool is whether this node should expose + // its web client over Tailscale at port 5252, // permitting access to peers according to the // policies as configured by the Tailnet's admin(s). RunWebClient bool