From 121d1d002c77451c80b6c30b398ce37dc27d2390 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 17 Aug 2023 12:29:51 -0700 Subject: [PATCH] tailcfg: add nodeAttrs for forcing OneCGNAT on/off [capver 71] Updates #8923 Signed-off-by: Brad Fitzpatrick --- control/controlclient/direct.go | 46 ++++++++++++++++++++++++++++----- ipn/ipnlocal/local.go | 9 +++++++ tailcfg/tailcfg.go | 42 ++++++++++++++++++++++-------- 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index d564bcd8b..46d7e0724 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -22,7 +22,6 @@ import ( "os" "reflect" "runtime" - "slices" "strings" "sync" "sync/atomic" @@ -43,12 +42,14 @@ import ( "tailscale.com/net/tlsdial" "tailscale.com/net/tsdial" "tailscale.com/net/tshttpproxy" + "tailscale.com/syncs" "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/tstime" "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netmap" + "tailscale.com/types/opt" "tailscale.com/types/persist" "tailscale.com/types/ptr" "tailscale.com/types/tkatype" @@ -1102,11 +1103,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap // For responses that mutate the self node, check for updated nodeAttrs. if resp.Node != nil { - caps := resp.Node.Capabilities - controlknobs.SetDisableUPnP(slices.Contains(caps, tailcfg.NodeAttrDisableUPnP)) - controlDisableDRPO.Store(slices.Contains(caps, tailcfg.NodeAttrDebugDisableDRPO)) - controlKeepFullWGConfig.Store(slices.Contains(caps, tailcfg.NodeAttrDebugDisableWGTrim)) - controlRandomizeClientPort.Store(slices.Contains(caps, tailcfg.NodeAttrRandomizeClientPort)) + setControlKnobsFromNodeAttrs(resp.Node.Capabilities) } nm := sess.netmapForResponse(&resp) @@ -1322,6 +1319,7 @@ var ( controlDisableDRPO atomic.Bool controlKeepFullWGConfig atomic.Bool controlRandomizeClientPort atomic.Bool + controlOneCGNAT syncs.AtomicValue[opt.Bool] ) // DisableDRPO reports whether control says to disable the @@ -1342,6 +1340,42 @@ func RandomizeClientPort() bool { return controlRandomizeClientPort.Load() } +// ControlOneCGNATSetting returns control's OneCGNAT setting, if any. +func ControlOneCGNATSetting() opt.Bool { + return controlOneCGNAT.Load() +} + +func setControlKnobsFromNodeAttrs(selfNodeAttrs []string) { + var ( + keepFullWG bool + disableDRPO bool + disableUPnP bool + randomizeClientPort bool + oneCGNAT opt.Bool + ) + for _, attr := range selfNodeAttrs { + switch attr { + case tailcfg.NodeAttrDebugDisableWGTrim: + keepFullWG = true + case tailcfg.NodeAttrDebugDisableDRPO: + disableDRPO = true + case tailcfg.NodeAttrDisableUPnP: + disableUPnP = true + case tailcfg.NodeAttrRandomizeClientPort: + randomizeClientPort = true + case tailcfg.NodeAttrOneCGNATEnable: + oneCGNAT.Set(true) + case tailcfg.NodeAttrOneCGNATDisable: + oneCGNAT.Set(false) + } + } + controlKeepFullWGConfig.Store(keepFullWG) + controlDisableDRPO.Store(disableDRPO) + controlknobs.SetDisableUPnP(disableUPnP) + controlRandomizeClientPort.Store(randomizeClientPort) + controlOneCGNAT.Store(oneCGNAT) +} + // ipForwardingBroken reports whether the system's IP forwarding is disabled // and will definitely not work for the routes provided. // diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 545bb5c48..f2bf47eb6 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3052,12 +3052,21 @@ func (b *LocalBackend) authReconfig() { // a runtime.GOOS. func shouldUseOneCGNATRoute(nm *netmap.NetworkMap, logf logger.Logf, versionOS string) bool { // Explicit enabling or disabling always take precedence. + + // Old way from control plane, pre capver 71: + // TODO(bradfitz): delete this path, once the control server starts + // sending it in nodeAttr. if nm.Debug != nil { if v, ok := nm.Debug.OneCGNATRoute.Get(); ok { logf("[v1] shouldUseOneCGNATRoute: explicit=%v", v) return v } } + if v, ok := controlclient.ControlOneCGNATSetting().Get(); ok { + logf("[v1] shouldUseOneCGNATRoute: explicit=%v", v) + return v + } + // Also prefer to do this on the Mac, so that we don't need to constantly // update the network extension configuration (which is disruptive to // Chrome, see https://github.com/tailscale/tailscale/issues/3102). Only diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 0d26f5d1e..1fe2a6ebe 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -108,7 +108,8 @@ type CapabilityVersion int // - 68: 2023-08-09: Client has dedicated updateRoutine; MapRequest.Stream true means ignore Hostinfo+Endpoints // - 69: 2023-08-16: removed Debug.LogHeap* + GoroutineDumpURL; added c2n /debug/logheap // - 70: 2023-08-16: removed most Debug fields; added NodeAttrDisable*, NodeAttrDebug* instead -const CurrentCapabilityVersion CapabilityVersion = 70 +// - 71: 2023-08-17: added NodeAttrOneCGNATEnable, NodeAttrOneCGNATDisable +const CurrentCapabilityVersion CapabilityVersion = 71 type StableID string @@ -1741,10 +1742,10 @@ type ControlIPCandidate struct { Priority int `json:",omitempty"` } -// Debug are instructions from the control server to the client to adjust debug +// Debug were instructions from the control server to the client to adjust debug // settings. // -// Deprecated: these should no longer be used. They're a weird mix of declartive +// Deprecated: these should no longer be used. Most have been deleted except for some They're a weird mix of declartive // and imperative. The imperative ones should be c2n requests instead, and the // declarative ones (at least the bools) should generally be self // Node.Capabilities. @@ -1754,26 +1755,35 @@ type Debug struct { // SleepSeconds requests that the client sleep for the // provided number of seconds. // The client can (and should) limit the value (such as 5 - // minutes). + // minutes). This exists as a safety measure to slow down + // spinning clients, in case we introduce a bug in the + // state machine. SleepSeconds float64 `json:",omitempty"` - // RandomizeClientPort is whether magicsock should UDP bind to - // :0 to get a random local port, ignoring any configured - // fixed port. + // RandomizeClientPort is whether magicsock should UDP bind to :0 to get a + // random local port, ignoring any configured fixed port. // - // Deprecated: use NodeAttrRandomizeClientPort instead. + // Deprecated: use NodeAttrRandomizeClientPort instead. This is kept in code + // only so the control plane can use it to send to old clients. RandomizeClientPort bool `json:",omitempty"` - // OneCGNATRoute controls whether the client should prefer to make one - // big CGNAT /10 route rather than a /32 per peer. + // OneCGNATRoute controls whether the client should prefer to make one big + // CGNAT /10 route rather than a /32 per peer. + // + // Deprecated: use NodeAttrOneCGNATEnable or NodeAttrOneCGNATDisable + // instead. This is kept in code only so the control plane can use it to + // send to old clients. OneCGNATRoute opt.Bool `json:",omitempty"` // DisableLogTail disables the logtail package. Once disabled it can't be // re-enabled for the lifetime of the process. + // + // This is primarily used by Headscale. DisableLogTail bool `json:",omitempty"` // Exit optionally specifies that the client should os.Exit - // with this code. + // with this code. This is a safety measure in case a client is crash + // looping or in an unsafe state and we need to remotely shut it down. Exit *int `json:",omitempty"` } @@ -1986,6 +1996,16 @@ const ( // :0 to get a random local port, ignoring any configured // fixed port. NodeAttrRandomizeClientPort = "randomize-client-port" + + // NodeAttrOneCGNATEnable makes the client prefer one big CGNAT /10 route + // rather than a /32 per peer. At most one of this or + // NodeAttrOneCGNATDisable may be set; if neither are, it's automatic. + NodeAttrOneCGNATEnable = "one-cgnat?v=true" + + // NodeAttrOneCGNATDisable makes the client prefer a /32 route per peer + // rather than one big /10 CGNAT route. At most one of this or + // NodeAttrOneCGNATEnable may be set; if neither are, it's automatic. + NodeAttrOneCGNATDisable = "one-cgnat?v=false" ) // SetDNSRequest is a request to add a DNS record.