From 74674b110daeb2cdda6f76ad3d69ef4cfba6e5b9 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 14 Sep 2022 12:49:39 -0700 Subject: [PATCH] envknob: support changing envknobs post-init Updates #5114 Change-Id: Ia423fc7486e1b3f3180a26308278be0086fae49b Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/tailscaled.go | 7 +- cmd/tailscaled/tailscaled_windows.go | 4 +- control/controlclient/direct.go | 34 ++--- control/controlclient/map.go | 6 +- control/controlknobs/controlknobs.go | 10 +- derp/derp_server.go | 6 +- envknob/envknob.go | 175 +++++++++++++++++++++++-- health/health.go | 4 +- ipn/ipnlocal/local.go | 15 +-- ipn/ipnlocal/local_test.go | 4 +- ipn/ipnlocal/network-lock.go | 4 +- ipn/localapi/cert.go | 4 +- net/dns/manager_windows.go | 4 +- net/dns/resolver/forwarder.go | 4 +- net/dnscache/dnscache.go | 26 ++-- net/dnscache/dnscache_test.go | 6 +- net/netcheck/netcheck.go | 4 +- net/netns/netns_linux.go | 4 +- net/tlsdial/tlsdial.go | 10 +- net/tstun/tun.go | 12 +- portlist/portlist.go | 4 +- ssh/tailssh/tailssh.go | 13 +- types/logger/logger.go | 4 +- wgengine/magicsock/debugknobs.go | 14 +- wgengine/magicsock/debugknobs_stubs.go | 20 +-- wgengine/magicsock/magicsock.go | 36 ++--- wgengine/magicsock/magicsock_linux.go | 4 +- wgengine/monitor/monitor_linux.go | 14 +- wgengine/netstack/netstack.go | 18 +-- wgengine/router/router_linux.go | 4 +- wgengine/userspace.go | 4 +- 31 files changed, 311 insertions(+), 167 deletions(-) diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 2397bbe48..f19281858 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -132,6 +132,7 @@ var subCommands = map[string]*func([]string) error{ var beCLI func() // non-nil if CLI is linked in func main() { + envknob.PanicIfAnyEnvCheckedInInit() printVersion := false flag.IntVar(&args.verbose, "verbose", 0, "log verbosity level; 0 is default, 1 or higher are increasingly verbose") flag.BoolVar(&args.cleanup, "cleanup", false, "clean up system state and exit") @@ -376,7 +377,7 @@ func run() error { return fmt.Errorf("newNetstack: %w", err) } ns.ProcessLocalIPs = useNetstack - ns.ProcessSubnets = useNetstack || wrapNetstack + ns.ProcessSubnets = useNetstack || shouldWrapNetstack() if useNetstack { dialer.UseNetstackForIP = func(ip netip.Addr) bool { @@ -477,8 +478,6 @@ func createEngine(logf logger.Logf, linkMon *monitor.Mon, dialer *tsdial.Dialer) return nil, false, multierr.New(errs...) } -var wrapNetstack = shouldWrapNetstack() - func shouldWrapNetstack() bool { if v, ok := envknob.LookupBool("TS_DEBUG_WRAP_NETSTACK"); ok { return v @@ -549,7 +548,7 @@ func tryEngine(logf logger.Logf, linkMon *monitor.Mon, dialer *tsdial.Dialer, na } conf.DNS = d conf.Router = r - if wrapNetstack { + if shouldWrapNetstack() { conf.Router = netstack.NewSubnetRouterWrapper(conf.Router) } } diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index 36133eea9..e43b140cd 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -274,7 +274,7 @@ func startIPNServer(ctx context.Context, logid string) error { dev.Close() return nil, nil, fmt.Errorf("router: %w", err) } - if wrapNetstack { + if shouldWrapNetstack() { r = netstack.NewSubnetRouterWrapper(r) } d, err := dns.NewOSConfigurator(logf, devName) @@ -301,7 +301,7 @@ func startIPNServer(ctx context.Context, logid string) error { return nil, nil, fmt.Errorf("newNetstack: %w", err) } ns.ProcessLocalIPs = false - ns.ProcessSubnets = wrapNetstack + ns.ProcessSubnets = shouldWrapNetstack() if err := ns.Start(); err != nil { return nil, nil, fmt.Errorf("failed to start netstack: %w", err) } diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 12de6d2ca..65a06b849 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -490,7 +490,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new c.logf("RegisterReq sign error: %v", err) } } - if debugRegister { + if debugRegister() { j, _ := json.MarshalIndent(request, "", "\t") c.logf("RegisterRequest: %s", j) } @@ -533,7 +533,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new c.logf("error decoding RegisterResponse with server key %s and machine key %s: %v", serverKey, machinePrivKey.Public(), err) return regen, opt.URL, fmt.Errorf("register request: %v", err) } - if debugRegister { + if debugRegister() { j, _ := json.MarshalIndent(resp, "", "\t") c.logf("RegisterResponse: %s", j) } @@ -715,7 +715,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool c.logf("[v1] PollNetMap: stream=%v ep=%v", allowStream, epStrs) vlogf := logger.Discard - if DevKnob.DumpNetMaps { + if DevKnob.DumpNetMaps() { // TODO(bradfitz): update this to use "[v2]" prefix perhaps? but we don't // want to upload it always. vlogf = c.logf @@ -963,12 +963,12 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool controlTrimWGConfig.Store(d.TrimWGConfig) } - if DevKnob.StripEndpoints { + if DevKnob.StripEndpoints() { for _, p := range resp.Peers { p.Endpoints = nil } } - if DevKnob.StripCaps { + if DevKnob.StripCaps() { nm.SelfNode.Capabilities = nil } @@ -1012,8 +1012,8 @@ func decode(res *http.Response, v any, serverKey, serverNoiseKey key.MachinePubl } var ( - debugMap = envknob.Bool("TS_DEBUG_MAP") - debugRegister = envknob.Bool("TS_DEBUG_REGISTER") + debugMap = envknob.RegisterBool("TS_DEBUG_MAP") + debugRegister = envknob.RegisterBool("TS_DEBUG_REGISTER") ) var jsonEscapedZero = []byte(`\u0000`) @@ -1051,7 +1051,7 @@ func (c *Direct) decodeMsg(msg []byte, v any, mkey key.MachinePrivate) error { return err } } - if debugMap { + if debugMap() { var buf bytes.Buffer json.Indent(&buf, b, "", " ") log.Printf("MapResponse: %s", buf.Bytes()) @@ -1088,7 +1088,7 @@ func encode(v any, serverKey, serverNoiseKey key.MachinePublic, mkey key.Machine if err != nil { return nil, err } - if debugMap { + if debugMap() { if _, ok := v.(*tailcfg.MapRequest); ok { log.Printf("MapRequest: %s", b) } @@ -1139,18 +1139,18 @@ func loadServerPubKeys(ctx context.Context, httpc *http.Client, serverURL string var DevKnob = initDevKnob() type devKnobs struct { - DumpNetMaps bool - ForceProxyDNS bool - StripEndpoints bool // strip endpoints from control (only use disco messages) - StripCaps bool // strip all local node's control-provided capabilities + DumpNetMaps func() bool + ForceProxyDNS func() bool + StripEndpoints func() bool // strip endpoints from control (only use disco messages) + StripCaps func() bool // strip all local node's control-provided capabilities } func initDevKnob() devKnobs { return devKnobs{ - DumpNetMaps: envknob.Bool("TS_DEBUG_NETMAP"), - ForceProxyDNS: envknob.Bool("TS_DEBUG_PROXY_DNS"), - StripEndpoints: envknob.Bool("TS_DEBUG_STRIP_ENDPOINTS"), - StripCaps: envknob.Bool("TS_DEBUG_STRIP_CAPS"), + DumpNetMaps: envknob.RegisterBool("TS_DEBUG_NETMAP"), + ForceProxyDNS: envknob.RegisterBool("TS_DEBUG_PROXY_DNS"), + StripEndpoints: envknob.RegisterBool("TS_DEBUG_STRIP_ENDPOINTS"), + StripCaps: envknob.RegisterBool("TS_DEBUG_STRIP_CAPS"), } } diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 52c532b2a..73a309e50 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -190,7 +190,7 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo } ms.addUserProfile(peer.User) } - if DevKnob.ForceProxyDNS { + if DevKnob.ForceProxyDNS() { nm.DNS.Proxied = true } ms.netMapBuilding = nil @@ -356,13 +356,13 @@ func cloneNodes(v1 []*tailcfg.Node) []*tailcfg.Node { return v2 } -var debugSelfIPv6Only = envknob.Bool("TS_DEBUG_SELF_V6_ONLY") +var debugSelfIPv6Only = envknob.RegisterBool("TS_DEBUG_SELF_V6_ONLY") func filterSelfAddresses(in []netip.Prefix) (ret []netip.Prefix) { switch { default: return in - case debugSelfIPv6Only: + case debugSelfIPv6Only(): for _, a := range in { if a.Addr().Is6() { ret = append(ret, a) diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index c5fd28d20..429d0af42 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -13,20 +13,18 @@ import ( ) // disableUPnP indicates whether to attempt UPnP mapping. -var disableUPnP atomic.Bool +var disableUPnPControl atomic.Bool -func init() { - SetDisableUPnP(envknob.Bool("TS_DISABLE_UPNP")) -} +var disableUPnpEnv = envknob.RegisterBool("TS_DISABLE_UPNP") // DisableUPnP reports the last reported value from control // whether UPnP portmapping should be disabled. func DisableUPnP() bool { - return disableUPnP.Load() + return disableUPnPControl.Load() || disableUPnpEnv() } // SetDisableUPnP sets whether control says that UPnP should be // disabled. func SetDisableUPnP(v bool) { - disableUPnP.Store(v) + disableUPnPControl.Store(v) } diff --git a/derp/derp_server.go b/derp/derp_server.go index 52680cace..b2c4e420e 100644 --- a/derp/derp_server.go +++ b/derp/derp_server.go @@ -47,8 +47,6 @@ import ( "tailscale.com/version" ) -var debug = envknob.Bool("DERP_DEBUG_LOGS") - // verboseDropKeys is the set of destination public keys that should // verbosely log whenever DERP drops a packet. var verboseDropKeys = map[key.NodePublic]bool{} @@ -106,6 +104,7 @@ type Server struct { limitedLogf logger.Logf metaCert []byte // the encoded x509 cert to send after LetsEncrypt cert+intermediate dupPolicy dupPolicy + debug bool // Counters: packetsSent, bytesSent expvar.Int @@ -299,6 +298,7 @@ func NewServer(privateKey key.NodePrivate, logf logger.Logf) *Server { runtime.ReadMemStats(&ms) s := &Server{ + debug: envknob.Bool("DERP_DEBUG_LOGS"), privateKey: privateKey, publicKey: privateKey.Public(), logf: logf, @@ -980,7 +980,7 @@ func (s *Server) recordDrop(packetBytes []byte, srcKey, dstKey key.NodePublic, r msg := fmt.Sprintf("drop (%s) %s -> %s", srcKey.ShortString(), reason, dstKey.ShortString()) s.limitedLogf(msg) } - if debug { + if s.debug { s.logf("dropping packet reason=%s dst=%s disco=%v", reason, dstKey, disco.LooksLikeDiscoWrapper(packetBytes)) } } diff --git a/envknob/envknob.go b/envknob/envknob.go index ecc731b3d..9c19f6b99 100644 --- a/envknob/envknob.go +++ b/envknob/envknob.go @@ -19,28 +19,36 @@ package envknob import ( "log" "os" + "runtime" + "sort" "strconv" + "strings" "sync" + "sync/atomic" "tailscale.com/types/opt" ) var ( - mu sync.Mutex - set = map[string]string{} - list []string + mu sync.Mutex + set = map[string]string{} + regStr = map[string]*string{} + regBool = map[string]*bool{} + regOptBool = map[string]*opt.Bool{} ) func noteEnv(k, v string) { - if v == "" { - return - } mu.Lock() defer mu.Unlock() - if _, ok := set[k]; !ok { - list = append(list, k) + noteEnvLocked(k, v) +} + +func noteEnvLocked(k, v string) { + if v != "" { + set[k] = v + } else { + delete(set, k) } - set[k] = v } // logf is logger.Logf, but logger depends on envknob, so for circular @@ -52,11 +60,39 @@ type logf = func(format string, args ...any) func LogCurrent(logf logf) { mu.Lock() defer mu.Unlock() + + list := make([]string, 0, len(set)) + for k := range set { + list = append(list, k) + } + sort.Strings(list) for _, k := range list { logf("envknob: %s=%q", k, set[k]) } } +// Setenv changes an environment variable. +// +// It is not safe for concurrent reading of environment variables via the +// Register functions. All Setenv calls are meant to happen early in main before +// any goroutines are started. +func Setenv(envVar, val string) { + mu.Lock() + defer mu.Unlock() + os.Setenv(envVar, val) + noteEnvLocked(envVar, val) + + if p := regStr[envVar]; p != nil { + *p = val + } + if p := regBool[envVar]; p != nil { + setBoolLocked(p, envVar, val) + } + if p := regOptBool[envVar]; p != nil { + setOptBoolLocked(p, envVar, val) + } +} + // String returns the named environment variable, using os.Getenv. // // If the variable is non-empty, it's also tracked & logged as being @@ -67,6 +103,82 @@ func String(envVar string) string { return v } +// RegisterString returns a func that gets the named environment variable, +// without a map lookup per call. It assumes that mutations happen via +// envknob.Setenv. +func RegisterString(envVar string) func() string { + mu.Lock() + defer mu.Unlock() + p, ok := regStr[envVar] + if !ok { + val := os.Getenv(envVar) + if val != "" { + noteEnvLocked(envVar, val) + } + p = &val + regStr[envVar] = p + } + return func() string { return *p } +} + +// RegisterBool returns a func that gets the named environment variable, +// without a map lookup per call. It assumes that mutations happen via +// envknob.Setenv. +func RegisterBool(envVar string) func() bool { + mu.Lock() + defer mu.Unlock() + p, ok := regBool[envVar] + if !ok { + var b bool + p = &b + setBoolLocked(p, envVar, os.Getenv(envVar)) + regBool[envVar] = p + } + return func() bool { return *p } +} + +// RegisterOptBool returns a func that gets the named environment variable, +// without a map lookup per call. It assumes that mutations happen via +// envknob.Setenv. +func RegisterOptBool(envVar string) func() opt.Bool { + mu.Lock() + defer mu.Unlock() + p, ok := regOptBool[envVar] + if !ok { + var b opt.Bool + p = &b + setOptBoolLocked(p, envVar, os.Getenv(envVar)) + regOptBool[envVar] = p + } + return func() opt.Bool { return *p } +} + +func setBoolLocked(p *bool, envVar, val string) { + noteEnvLocked(envVar, val) + if val == "" { + *p = false + return + } + var err error + *p, err = strconv.ParseBool(val) + if err != nil { + log.Fatalf("invalid boolean environment variable %s value %q", envVar, val) + } +} + +func setOptBoolLocked(p *opt.Bool, envVar, val string) { + noteEnvLocked(envVar, val) + if val == "" { + *p = "" + return + } + b, err := strconv.ParseBool(val) + if err != nil { + log.Fatalf("invalid boolean environment variable %s value %q", envVar, val) + } + p.Set(b) +} + // Bool returns the boolean value of the named environment variable. // If the variable is not set, it returns false. // An invalid value exits the binary with a failure. @@ -81,6 +193,7 @@ func BoolDefaultTrue(envVar string) bool { } func boolOr(envVar string, implicitValue bool) bool { + assertNotInInit() val := os.Getenv(envVar) if val == "" { return implicitValue @@ -98,6 +211,7 @@ func boolOr(envVar string, implicitValue bool) bool { // The ok result is whether a value was set. // If the value isn't a valid int, it exits the program with a failure. func LookupBool(envVar string) (v bool, ok bool) { + assertNotInInit() val := os.Getenv(envVar) if val == "" { return false, false @@ -113,6 +227,7 @@ func LookupBool(envVar string) (v bool, ok bool) { // OptBool is like Bool, but returns an opt.Bool, so the caller can // distinguish between implicitly and explicitly false. func OptBool(envVar string) opt.Bool { + assertNotInInit() b, ok := LookupBool(envVar) if !ok { return "" @@ -126,6 +241,7 @@ func OptBool(envVar string) opt.Bool { // The ok result is whether a value was set. // If the value isn't a valid int, it exits the program with a failure. func LookupInt(envVar string) (v int, ok bool) { + assertNotInInit() val := os.Getenv(envVar) if val == "" { return 0, false @@ -164,5 +280,44 @@ func NoLogsNoSupport() bool { // SetNoLogsNoSupport enables no-logs-no-support mode. func SetNoLogsNoSupport() { - os.Setenv("TS_NO_LOGS_NO_SUPPORT", "true") + Setenv("TS_NO_LOGS_NO_SUPPORT", "true") +} + +// notInInit is set true the first time we've seen a non-init stack trace. +var notInInit atomic.Bool + +func assertNotInInit() { + if notInInit.Load() { + return + } + skip := 0 + for { + pc, _, _, ok := runtime.Caller(skip) + if !ok { + notInInit.Store(true) + return + } + fu := runtime.FuncForPC(pc) + if fu == nil { + return + } + name := fu.Name() + name = strings.TrimRightFunc(name, func(r rune) bool { return r >= '0' && r <= '9' }) + if strings.HasSuffix(name, ".init") || strings.HasSuffix(name, ".init.") { + stack := make([]byte, 1<<10) + stack = stack[:runtime.Stack(stack, false)] + envCheckedInInitStack = stack + } + skip++ + } +} + +var envCheckedInInitStack []byte + +// PanicIfAnyEnvCheckedInInit panics if environment variables were read during +// init. +func PanicIfAnyEnvCheckedInInit() { + if envCheckedInInitStack != nil { + panic("envknob check of called from init function: " + string(envCheckedInInitStack)) + } } diff --git a/health/health.go b/health/health.go index ba5bed636..80474857e 100644 --- a/health/health.go +++ b/health/health.go @@ -325,7 +325,7 @@ func OverallError() error { return overallErrorLocked() } -var fakeErrForTesting = envknob.String("TS_DEBUG_FAKE_HEALTH_ERROR") +var fakeErrForTesting = envknob.RegisterString("TS_DEBUG_FAKE_HEALTH_ERROR") func overallErrorLocked() error { if !anyInterfaceUp { @@ -383,7 +383,7 @@ func overallErrorLocked() error { for _, s := range controlHealth { errs = append(errs, errors.New(s)) } - if e := fakeErrForTesting; len(errs) == 0 && e != "" { + if e := fakeErrForTesting(); len(errs) == 0 && e != "" { return errors.New(e) } sort.Slice(errs, func(i, j int) bool { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 0d1f75870..c721f765f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -68,7 +68,6 @@ import ( ) var controlDebugFlags = getControlDebugFlags() -var canSSH = envknob.CanSSHD() func getControlDebugFlags() []string { if e := envknob.String("TS_DEBUG_CONTROL_FLAGS"); e != "" { @@ -1510,12 +1509,12 @@ func (b *LocalBackend) tellClientToBrowseToURL(url string) { } // For testing lazy machine key generation. -var panicOnMachineKeyGeneration = envknob.Bool("TS_DEBUG_PANIC_MACHINE_KEY") +var panicOnMachineKeyGeneration = envknob.RegisterBool("TS_DEBUG_PANIC_MACHINE_KEY") func (b *LocalBackend) createGetMachinePrivateKeyFunc() func() (key.MachinePrivate, error) { var cache syncs.AtomicValue[key.MachinePrivate] return func() (key.MachinePrivate, error) { - if panicOnMachineKeyGeneration { + if panicOnMachineKeyGeneration() { panic("machine key generated") } if v, ok := cache.LoadOk(); ok { @@ -1752,7 +1751,7 @@ func (b *LocalBackend) loadStateLocked(key ipn.StateKey, prefs *ipn.Prefs) (err // setAtomicValuesFromPrefs populates sshAtomicBool and containsViaIPFuncAtomic // from the prefs p, which may be nil. func (b *LocalBackend) setAtomicValuesFromPrefs(p *ipn.Prefs) { - b.sshAtomicBool.Store(p != nil && p.RunSSH && canSSH) + b.sshAtomicBool.Store(p != nil && p.RunSSH && envknob.CanSSHD()) if p == nil { b.containsViaIPFuncAtomic.Store(tsaddr.NewContainsIPFunc(nil)) @@ -1967,7 +1966,7 @@ func (b *LocalBackend) checkSSHPrefsLocked(p *ipn.Prefs) error { default: return errors.New("The Tailscale SSH server is not supported on " + runtime.GOOS) } - if !canSSH { + if !envknob.CanSSHD() { return errors.New("The Tailscale SSH server has been administratively disabled.") } if envknob.SSHIgnoreTailnetPolicy() || envknob.SSHPolicyFile() != "" { @@ -2032,7 +2031,7 @@ func (b *LocalBackend) EditPrefs(mp *ipn.MaskedPrefs) (*ipn.Prefs, error) { b.logf("EditPrefs check error: %v", err) return nil, err } - if p1.RunSSH && !canSSH { + if p1.RunSSH && !envknob.CanSSHD() { b.mu.Unlock() b.logf("EditPrefs requests SSH, but disabled by envknob; returning error") return nil, errors.New("Tailscale SSH server administratively disabled.") @@ -2854,7 +2853,7 @@ func (b *LocalBackend) applyPrefsToHostinfo(hi *tailcfg.Hostinfo, prefs *ipn.Pre hi.ShieldsUp = prefs.ShieldsUp var sshHostKeys []string - if prefs.RunSSH && canSSH { + if prefs.RunSSH && envknob.CanSSHD() { // TODO(bradfitz): this is called with b.mu held. Not ideal. // If the filesystem gets wedged or something we could block for // a long time. But probably fine. @@ -3073,7 +3072,7 @@ func (b *LocalBackend) ResetForClientDisconnect() { b.setAtomicValuesFromPrefs(nil) } -func (b *LocalBackend) ShouldRunSSH() bool { return b.sshAtomicBool.Load() && canSSH } +func (b *LocalBackend) ShouldRunSSH() bool { return b.sshAtomicBool.Load() && envknob.CanSSHD() } // ShouldHandleViaIP reports whether whether ip is an IPv6 address in the // Tailscale ULA's v6 "via" range embedding an IPv4 address to be forwarded to diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 4d8d25c1c..f38a96c3b 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -478,8 +478,8 @@ func (panicOnUseTransport) RoundTrip(*http.Request) (*http.Response, error) { // Issue 1573: don't generate a machine key if we don't want to be running. func TestLazyMachineKeyGeneration(t *testing.T) { - defer func(old bool) { panicOnMachineKeyGeneration = old }(panicOnMachineKeyGeneration) - panicOnMachineKeyGeneration = true + defer func(old func() bool) { panicOnMachineKeyGeneration = old }(panicOnMachineKeyGeneration) + panicOnMachineKeyGeneration = func() bool { return true } var logf logger.Logf = logger.Discard store := new(mem.Store) diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index c5379953e..042998f7b 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -24,7 +24,7 @@ import ( "tailscale.com/types/tkatype" ) -var networkLockAvailable = envknob.Bool("TS_EXPERIMENTAL_NETWORK_LOCK") +var networkLockAvailable = envknob.RegisterBool("TS_EXPERIMENTAL_NETWORK_LOCK") type tkaState struct { authority *tka.Authority @@ -82,7 +82,7 @@ func (b *LocalBackend) NetworkLockInit(keys []tka.Key) error { if b.tka != nil { return errors.New("network-lock is already initialized") } - if !networkLockAvailable { + if !networkLockAvailable() { return errors.New("this is an experimental feature in your version of tailscale - Please upgrade to the latest to use this.") } if !b.CanSupportNetworkLock() { diff --git a/ipn/localapi/cert.go b/ipn/localapi/cert.go index 1dbe58b23..2458b054a 100644 --- a/ipn/localapi/cert.go +++ b/ipn/localapi/cert.go @@ -73,7 +73,7 @@ func (h *Handler) certDir() (string, error) { return full, nil } -var acmeDebug = envknob.Bool("TS_DEBUG_ACME") +var acmeDebug = envknob.RegisterBool("TS_DEBUG_ACME") func (h *Handler) serveCert(w http.ResponseWriter, r *http.Request) { if !h.PermitWrite && !h.PermitCert { @@ -96,7 +96,7 @@ func (h *Handler) serveCert(w http.ResponseWriter, r *http.Request) { now := time.Now() logf := logger.WithPrefix(h.logf, fmt.Sprintf("cert(%q): ", domain)) traceACME := func(v any) { - if !acmeDebug { + if !acmeDebug() { return } j, _ := json.MarshalIndent(v, "", "\t") diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index e25fa3f86..baf349511 100644 --- a/net/dns/manager_windows.go +++ b/net/dns/manager_windows.go @@ -32,7 +32,7 @@ const ( versionKey = `SOFTWARE\Microsoft\Windows NT\CurrentVersion` ) -var configureWSL = envknob.Bool("TS_DEBUG_CONFIGURE_WSL") +var configureWSL = envknob.RegisterBool("TS_DEBUG_CONFIGURE_WSL") type windowsManager struct { logf logger.Logf @@ -359,7 +359,7 @@ func (m windowsManager) SetDNS(cfg OSConfig) error { // On initial setup of WSL, the restart caused by --shutdown is slow, // so we do it out-of-line. - if configureWSL { + if configureWSL() { go func() { if err := m.wslManager.SetDNS(cfg); err != nil { m.logf("WSL SetDNS: %v", err) // continue diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index 470153f5f..a5729cdbd 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -484,13 +484,13 @@ func (f *forwarder) sendDoH(ctx context.Context, urlBase string, c *http.Client, return res, err } -var verboseDNSForward = envknob.Bool("TS_DEBUG_DNS_FORWARD_SEND") +var verboseDNSForward = envknob.RegisterBool("TS_DEBUG_DNS_FORWARD_SEND") // send sends packet to dst. It is best effort. // // send expects the reply to have the same txid as txidOut. func (f *forwarder) send(ctx context.Context, fq *forwardQuery, rr resolverAndDelay) (ret []byte, err error) { - if verboseDNSForward { + if verboseDNSForward() { f.logf("forwarder.send(%q) ...", rr.name.Addr) defer func() { f.logf("forwarder.send(%q) = %v, %v", rr.name.Addr, len(ret), err) diff --git a/net/dnscache/dnscache.go b/net/dnscache/dnscache.go index 14a8c3e3c..6c98322e0 100644 --- a/net/dnscache/dnscache.go +++ b/net/dnscache/dnscache.go @@ -141,7 +141,7 @@ func (r *Resolver) ttl() time.Duration { return 10 * time.Minute } -var debug = envknob.Bool("TS_DEBUG_DNS_CACHE") +var debug = envknob.RegisterBool("TS_DEBUG_DNS_CACHE") // LookupIP returns the host's primary IP address (either IPv4 or // IPv6, but preferring IPv4) and optionally its IPv6 address, if @@ -167,14 +167,14 @@ func (r *Resolver) LookupIP(ctx context.Context, host string) (ip, v6 netip.Addr } if ip, err := netip.ParseAddr(host); err == nil { ip = ip.Unmap() - if debug { + if debug() { log.Printf("dnscache: %q is an IP", host) } return ip, zaddr, []netip.Addr{ip}, nil } if ip, ip6, allIPs, ok := r.lookupIPCache(host); ok { - if debug { + if debug() { log.Printf("dnscache: %q = %v (cached)", host, ip) } return ip, ip6, allIPs, nil @@ -192,13 +192,13 @@ func (r *Resolver) LookupIP(ctx context.Context, host string) (ip, v6 netip.Addr if res.Err != nil { if r.UseLastGood { if ip, ip6, allIPs, ok := r.lookupIPCacheExpired(host); ok { - if debug { + if debug() { log.Printf("dnscache: %q using %v after error", host, ip) } return ip, ip6, allIPs, nil } } - if debug { + if debug() { log.Printf("dnscache: error resolving %q: %v", host, res.Err) } return zaddr, zaddr, nil, res.Err @@ -206,7 +206,7 @@ func (r *Resolver) LookupIP(ctx context.Context, host string) (ip, v6 netip.Addr r := res.Val return r.ip, r.ip6, r.allIPs, nil case <-ctx.Done(): - if debug { + if debug() { log.Printf("dnscache: context done while resolving %q: %v", host, ctx.Err()) } return zaddr, zaddr, nil, ctx.Err() @@ -250,7 +250,7 @@ func (r *Resolver) lookupTimeoutForHost(host string) time.Duration { func (r *Resolver) lookupIP(host string) (ip, ip6 netip.Addr, allIPs []netip.Addr, err error) { if ip, ip6, allIPs, ok := r.lookupIPCache(host); ok { - if debug { + if debug() { log.Printf("dnscache: %q found in cache as %v", host, ip) } return ip, ip6, allIPs, nil @@ -300,13 +300,13 @@ func (r *Resolver) addIPCache(host string, ip, ip6 netip.Addr, allIPs []netip.Ad if ip.IsPrivate() { // Don't cache obviously wrong entries from captive portals. // TODO: use DoH or DoT for the forwarding resolver? - if debug { + if debug() { log.Printf("dnscache: %q resolved to private IP %v; using but not caching", host, ip) } return } - if debug { + if debug() { log.Printf("dnscache: %q resolved to IP %v; caching", host, ip) } @@ -382,7 +382,7 @@ func (d *dialer) DialContext(ctx context.Context, network, address string) (retC } i4s := v4addrs(allIPs) if len(i4s) < 2 { - if debug { + if debug() { log.Printf("dnscache: dialing %s, %s for %s", network, ip, address) } c, err := dc.dialOne(ctx, ip.Unmap()) @@ -406,7 +406,7 @@ func (d *dialer) shouldTryBootstrap(ctx context.Context, err error, dc *dialCall // Can't try bootstrap DNS if we don't have a fallback function if d.dnsCache.LookupIPFallback == nil { - if debug { + if debug() { log.Printf("dnscache: not using bootstrap DNS: no fallback") } return false @@ -415,7 +415,7 @@ func (d *dialer) shouldTryBootstrap(ctx context.Context, err error, dc *dialCall // We can't retry if the context is canceled, since any further // operations with this context will fail. if ctxErr := ctx.Err(); ctxErr != nil { - if debug { + if debug() { log.Printf("dnscache: not using bootstrap DNS: context error: %v", ctxErr) } return false @@ -423,7 +423,7 @@ func (d *dialer) shouldTryBootstrap(ctx context.Context, err error, dc *dialCall wasTrustworthy := dc.dnsWasTrustworthy() if wasTrustworthy { - if debug { + if debug() { log.Printf("dnscache: not using bootstrap DNS: DNS was trustworthy") } return false diff --git a/net/dnscache/dnscache_test.go b/net/dnscache/dnscache_test.go index 2e64a87e2..c6348266b 100644 --- a/net/dnscache/dnscache_test.go +++ b/net/dnscache/dnscache_test.go @@ -167,10 +167,8 @@ func TestInterleaveSlices(t *testing.T) { func TestShouldTryBootstrap(t *testing.T) { oldDebug := debug - t.Cleanup(func() { - debug = oldDebug - }) - debug = true + t.Cleanup(func() { debug = oldDebug }) + debug = func() bool { return true } type step struct { ip netip.Addr // IP we pretended to dial diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 49cdcf514..8d2be78a3 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -43,7 +43,7 @@ import ( // Debugging and experimentation tweakables. var ( - debugNetcheck = envknob.Bool("TS_DEBUG_NETCHECK") + debugNetcheck = envknob.RegisterBool("TS_DEBUG_NETCHECK") ) // The various default timeouts for things. @@ -210,7 +210,7 @@ func (c *Client) logf(format string, a ...any) { } func (c *Client) vlogf(format string, a ...any) { - if c.Verbose || debugNetcheck { + if c.Verbose || debugNetcheck() { c.logf(format, a...) } } diff --git a/net/netns/netns_linux.go b/net/netns/netns_linux.go index d94920d57..2f5403012 100644 --- a/net/netns/netns_linux.go +++ b/net/netns/netns_linux.go @@ -63,12 +63,12 @@ func socketMarkWorks() bool { return true } -var forceBindToDevice = envknob.Bool("TS_FORCE_LINUX_BIND_TO_DEVICE") +var forceBindToDevice = envknob.RegisterBool("TS_FORCE_LINUX_BIND_TO_DEVICE") // UseSocketMark reports whether SO_MARK is in use. // If it doesn't, we have to use SO_BINDTODEVICE on our sockets instead. func UseSocketMark() bool { - if forceBindToDevice { + if forceBindToDevice() { return false } socketMarkWorksOnce.Do(func() { diff --git a/net/tlsdial/tlsdial.go b/net/tlsdial/tlsdial.go index 35f1c0c4f..f8966ce94 100644 --- a/net/tlsdial/tlsdial.go +++ b/net/tlsdial/tlsdial.go @@ -32,7 +32,7 @@ var counterFallbackOK int32 // atomic // See https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format var sslKeyLogFile = os.Getenv("SSLKEYLOGFILE") -var debug = envknob.Bool("TS_DEBUG_TLS_DIAL") +var debug = envknob.RegisterBool("TS_DEBUG_TLS_DIAL") // Config returns a tls.Config for connecting to a server. // If base is non-nil, it's cloned as the base config before @@ -77,7 +77,7 @@ func Config(host string, base *tls.Config) *tls.Config { opts.Intermediates.AddCert(cert) } _, errSys := cs.PeerCertificates[0].Verify(opts) - if debug { + if debug() { log.Printf("tlsdial(sys %q): %v", host, errSys) } if errSys == nil { @@ -88,7 +88,7 @@ func Config(host string, base *tls.Config) *tls.Config { // or broken, fall back to trying LetsEncrypt at least. opts.Roots = bakedInRoots() _, err := cs.PeerCertificates[0].Verify(opts) - if debug { + if debug() { log.Printf("tlsdial(bake %q): %v", host, err) } if err == nil { @@ -142,7 +142,7 @@ func SetConfigExpectedCert(c *tls.Config, certDNSName string) { opts.Intermediates.AddCert(cert) } _, errSys := certs[0].Verify(opts) - if debug { + if debug() { log.Printf("tlsdial(sys %q/%q): %v", c.ServerName, certDNSName, errSys) } if errSys == nil { @@ -150,7 +150,7 @@ func SetConfigExpectedCert(c *tls.Config, certDNSName string) { } opts.Roots = bakedInRoots() _, err := certs[0].Verify(opts) - if debug { + if debug() { log.Printf("tlsdial(bake %q/%q): %v", c.ServerName, certDNSName, err) } if err == nil { diff --git a/net/tstun/tun.go b/net/tstun/tun.go index 3030112d5..5a9d96ca5 100644 --- a/net/tstun/tun.go +++ b/net/tstun/tun.go @@ -20,14 +20,6 @@ import ( "tailscale.com/types/logger" ) -var tunMTU = DefaultMTU - -func init() { - if mtu, ok := envknob.LookupInt("TS_DEBUG_MTU"); ok { - tunMTU = mtu - } -} - // createTAP is non-nil on Linux. var createTAP func(tapName, bridgeName string) (tun.Device, error) @@ -52,6 +44,10 @@ func New(logf logger.Logf, tunName string) (tun.Device, string, error) { } dev, err = createTAP(tapName, bridgeName) } else { + tunMTU := DefaultMTU + if mtu, ok := envknob.LookupInt("TS_DEBUG_MTU"); ok { + tunMTU = mtu + } dev, err = tun.CreateTUN(tunName, tunMTU) } if err != nil { diff --git a/portlist/portlist.go b/portlist/portlist.go index 9eb35ebfc..e2e911fe4 100644 --- a/portlist/portlist.go +++ b/portlist/portlist.go @@ -74,10 +74,10 @@ func (pl List) String() string { return strings.TrimRight(sb.String(), "\n") } -var debugDisablePortlist = envknob.Bool("TS_DEBUG_DISABLE_PORTLIST") +var debugDisablePortlist = envknob.RegisterBool("TS_DEBUG_DISABLE_PORTLIST") func GetList(prev List) (List, error) { - if debugDisablePortlist { + if debugDisablePortlist() { return nil, nil } pl, err := listPorts() diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index 06b8460dc..f44146099 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -46,9 +46,7 @@ import ( ) var ( - debugPolicyFile = envknob.SSHPolicyFile() - debugIgnoreTailnetSSHPolicy = envknob.SSHIgnoreTailnetPolicy() - sshVerboseLogging = envknob.Bool("TS_DEBUG_SSH_VLOG") + sshVerboseLogging = envknob.RegisterBool("TS_DEBUG_SSH_VLOG") ) type server struct { @@ -384,9 +382,10 @@ func (c *conn) sshPolicy() (_ *tailcfg.SSHPolicy, ok bool) { if nm == nil { return nil, false } - if pol := nm.SSHPolicy; pol != nil && !debugIgnoreTailnetSSHPolicy { + if pol := nm.SSHPolicy; pol != nil && !envknob.SSHIgnoreTailnetPolicy() { return pol, true } + debugPolicyFile := envknob.SSHPolicyFile() if debugPolicyFile != "" { c.logf("reading debug SSH policy file: %v", debugPolicyFile) f, err := os.ReadFile(debugPolicyFile) @@ -769,7 +768,7 @@ type sshSession struct { } func (ss *sshSession) vlogf(format string, args ...interface{}) { - if sshVerboseLogging { + if sshVerboseLogging() { ss.logf(format, args...) } } @@ -952,7 +951,7 @@ func (ss *sshSession) handleSSHAgentForwarding(s ssh.Session, lu *user.User) err // functionality and support off-node streaming. // // TODO(bradfitz,maisem): move this to SSHPolicy. -var recordSSH = envknob.Bool("TS_DEBUG_LOG_SSH") +var recordSSH = envknob.RegisterBool("TS_DEBUG_LOG_SSH") // run is the entrypoint for a newly accepted SSH session. // @@ -1092,7 +1091,7 @@ func (ss *sshSession) shouldRecord() bool { // TODO(bradfitz,maisem): make configurable on SSHPolicy and // support recording non-pty stuff too. _, _, isPtyReq := ss.Pty() - return recordSSH && isPtyReq + return recordSSH() && isPtyReq } type sshConnInfo struct { diff --git a/types/logger/logger.go b/types/logger/logger.go index 4e33f651a..48fdd9373 100644 --- a/types/logger/logger.go +++ b/types/logger/logger.go @@ -129,8 +129,6 @@ type limitData struct { ele *list.Element // list element used to access this string in the cache } -var disableRateLimit = envknob.String("TS_DEBUG_LOG_RATE") == "all" - // rateFree are format string substrings that are exempt from rate limiting. // Things should not be added to this unless they're already limited otherwise // or are critical for generating important stats from the logs. @@ -156,7 +154,7 @@ func RateLimitedFn(logf Logf, f time.Duration, burst int, maxCache int) Logf { // timeNow is a function that returns the current time, used for calculating // rate limits. func RateLimitedFnWithClock(logf Logf, f time.Duration, burst int, maxCache int, timeNow func() time.Time) Logf { - if disableRateLimit { + if envknob.String("TS_DEBUG_LOG_RATE") == "all" { return logf } var ( diff --git a/wgengine/magicsock/debugknobs.go b/wgengine/magicsock/debugknobs.go index 9ae40fe13..7cda8109c 100644 --- a/wgengine/magicsock/debugknobs.go +++ b/wgengine/magicsock/debugknobs.go @@ -11,28 +11,30 @@ import ( "tailscale.com/envknob" ) +const linkDebug = true + // Various debugging and experimental tweakables, set by environment // variable. var ( // debugDisco prints verbose logs of active discovery events as // they happen. - debugDisco = envknob.Bool("TS_DEBUG_DISCO") + debugDisco = envknob.RegisterBool("TS_DEBUG_DISCO") // debugOmitLocalAddresses removes all local interface addresses // from magicsock's discovered local endpoints. Used in some tests. - debugOmitLocalAddresses = envknob.Bool("TS_DEBUG_OMIT_LOCAL_ADDRS") + debugOmitLocalAddresses = envknob.RegisterBool("TS_DEBUG_OMIT_LOCAL_ADDRS") // debugUseDerpRoute temporarily (2020-03-22) controls whether DERP // reverse routing is enabled (Issue 150). - debugUseDerpRoute = envknob.OptBool("TS_DEBUG_ENABLE_DERP_ROUTE") + debugUseDerpRoute = envknob.RegisterOptBool("TS_DEBUG_ENABLE_DERP_ROUTE") // logDerpVerbose logs all received DERP packets, including their // full payload. - logDerpVerbose = envknob.Bool("TS_DEBUG_DERP") + logDerpVerbose = envknob.RegisterBool("TS_DEBUG_DERP") // debugReSTUNStopOnIdle unconditionally enables the "shut down // STUN if magicsock is idle" behavior that normally only triggers // on mobile devices, lowers the shutdown interval, and logs more // verbosely about idle measurements. - debugReSTUNStopOnIdle = envknob.Bool("TS_DEBUG_RESTUN_STOP_ON_IDLE") + debugReSTUNStopOnIdle = envknob.RegisterBool("TS_DEBUG_RESTUN_STOP_ON_IDLE") // debugAlwaysDERP disables the use of UDP, forcing all peer communication over DERP. - debugAlwaysDERP = envknob.Bool("TS_DEBUG_ALWAYS_USE_DERP") + debugAlwaysDERP = envknob.RegisterBool("TS_DEBUG_ALWAYS_USE_DERP") ) // inTest reports whether the running program is a test that set the diff --git a/wgengine/magicsock/debugknobs_stubs.go b/wgengine/magicsock/debugknobs_stubs.go index 47357fa3a..fd951737d 100644 --- a/wgengine/magicsock/debugknobs_stubs.go +++ b/wgengine/magicsock/debugknobs_stubs.go @@ -10,15 +10,15 @@ package magicsock import "tailscale.com/types/opt" // All knobs are disabled on iOS and Wasm. -// Further, they're const, so the toolchain can produce smaller binaries. -const ( - debugDisco = false - debugOmitLocalAddresses = false - debugUseDerpRouteEnv = "" - debugUseDerpRoute opt.Bool = "" - logDerpVerbose = false - debugReSTUNStopOnIdle = false - debugAlwaysDERP = false -) +// +// They're inlinable and the linker can deadcode that's guarded by them to make +// smaller binaries. +func debugDisco() bool { return false } +func debugOmitLocalAddresses() bool { return false } +func logDerpVerbose() bool { return false } +func debugReSTUNStopOnIdle() bool { return false } +func debugAlwaysDERP() bool { return false } +func debugUseDerpRouteEnv() string { return "" } +func debugUseDerpRoute() opt.Bool { return "" } func inTest() bool { return false } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index a12ef1a31..2669b9cad 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -74,7 +74,7 @@ const ( // useDerpRoute reports whether magicsock should enable the DERP // return path optimization (Issue 150). func useDerpRoute() bool { - if b, ok := debugUseDerpRoute.Get(); ok { + if b, ok := debugUseDerpRoute().Get(); ok { return b } ob := controlclient.DERPRouteFlag() @@ -638,18 +638,18 @@ func (c *Conn) updateEndpoints(why string) { // etc) d := tstime.RandomDurationBetween(20*time.Second, 26*time.Second) if t := c.periodicReSTUNTimer; t != nil { - if debugReSTUNStopOnIdle { + if debugReSTUNStopOnIdle() { c.logf("resetting existing periodicSTUN to run in %v", d) } t.Reset(d) } else { - if debugReSTUNStopOnIdle { + if debugReSTUNStopOnIdle() { c.logf("scheduling periodicSTUN to run in %v", d) } c.periodicReSTUNTimer = time.AfterFunc(d, c.doPeriodicSTUN) } } else { - if debugReSTUNStopOnIdle { + if debugReSTUNStopOnIdle() { c.logf("periodic STUN idle") } c.stopPeriodicReSTUNTimerLocked() @@ -1074,7 +1074,7 @@ func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, erro return } addAddr := func(ipp netip.AddrPort, et tailcfg.EndpointType) { - if !ipp.IsValid() || (debugOmitLocalAddresses && et == tailcfg.EndpointLocal) { + if !ipp.IsValid() || (debugOmitLocalAddresses() && et == tailcfg.EndpointLocal) { return } if _, ok := already[ipp]; !ok { @@ -1575,7 +1575,7 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netip.AddrPort, d pkt = m res.n = len(m.Data) res.src = m.Source - if logDerpVerbose { + if logDerpVerbose() { c.logf("magicsock: got derp-%v packet: %q", regionID, m.Data) } // If this is a new sender we hadn't seen before, remember it and @@ -1826,7 +1826,7 @@ func (c *Conn) sendDiscoMessage(dst netip.AddrPort, dstKey key.NodePublic, dstDi pkt = append(pkt, box...) sent, err = c.sendAddr(dst, dstKey, pkt) if sent { - if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco) { + if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco()) { node := "?" if !dstKey.IsZero() { node = dstKey.ShortString() @@ -1890,7 +1890,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke if c.closed { return } - if debugDisco { + if debugDisco() { c.logf("magicsock: disco: got disco-looking frame from %v", sender.ShortString()) } if c.privateKey.IsZero() { @@ -1899,7 +1899,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke return } if c.discoPrivate.IsZero() { - if debugDisco { + if debugDisco() { c.logf("magicsock: disco: ignoring disco-looking frame, no local key") } return @@ -1907,7 +1907,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke if !c.peerMap.anyEndpointForDiscoKey(sender) { metricRecvDiscoBadPeer.Add(1) - if debugDisco { + if debugDisco() { c.logf("magicsock: disco: ignoring disco-looking frame, don't know endpoint for %v", sender.ShortString()) } return @@ -1931,7 +1931,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke // Don't log in normal case. Pass on to wireguard, in case // it's actually a wireguard packet (super unlikely, // but). - if debugDisco { + if debugDisco() { c.logf("magicsock: disco: failed to open naclbox from %v (wrong rcpt?)", sender) } metricRecvDiscoBadKey.Add(1) @@ -1939,7 +1939,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke } dm, err := disco.Parse(payload) - if debugDisco { + if debugDisco() { c.logf("magicsock: disco: disco.Parse = %T, %v", dm, err) } if err != nil { @@ -2094,7 +2094,7 @@ func (c *Conn) handlePingLocked(dm *disco.Ping, src netip.AddrPort, di *discoInf return } - if !likelyHeartBeat || debugDisco { + if !likelyHeartBeat || debugDisco() { pingNodeSrcStr := dstKey.ShortString() if numNodes > 1 { pingNodeSrcStr = "[one-of-multi]" @@ -2381,7 +2381,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) { } ep.wgEndpoint = n.Key.UntypedHexString() ep.initFakeUDPAddr() - if debugDisco { // rather than making a new knob + if debugDisco() { // rather than making a new knob c.logf("magicsock: created endpoint key=%s: disco=%s; %v", n.Key.ShortString(), n.DiscoKey.ShortString(), logger.ArgWriter(func(w *bufio.Writer) { const derpPrefix = "127.3.3.40:" if strings.HasPrefix(n.DERP, derpPrefix) { @@ -2736,7 +2736,7 @@ func (c *Conn) goroutinesRunningLocked() bool { } func maxIdleBeforeSTUNShutdown() time.Duration { - if debugReSTUNStopOnIdle { + if debugReSTUNStopOnIdle() { return 45 * time.Second } return sessionActiveTimeout @@ -2753,7 +2753,7 @@ func (c *Conn) shouldDoPeriodicReSTUNLocked() bool { } if f := c.idleFunc; f != nil { idleFor := f() - if debugReSTUNStopOnIdle { + if debugReSTUNStopOnIdle() { c.logf("magicsock: periodicReSTUN: idle for %v", idleFor.Round(time.Second)) } if idleFor > maxIdleBeforeSTUNShutdown() { @@ -2834,7 +2834,7 @@ func (c *Conn) bindSocket(ruc *RebindingUDPConn, network string, curPortFate cur return nil } - if debugAlwaysDERP { + if debugAlwaysDERP() { c.logf("disabled %v per TS_DEBUG_ALWAYS_USE_DERP", network) ruc.setConnLocked(newBlockForeverConn()) return nil @@ -3626,7 +3626,7 @@ func (de *endpoint) pingTimeout(txid stun.TxID) { if !ok { return } - if debugDisco || !de.bestAddr.IsValid() || mono.Now().After(de.trustBestAddrUntil) { + if debugDisco() || !de.bestAddr.IsValid() || mono.Now().After(de.trustBestAddrUntil) { de.c.logf("[v1] magicsock: disco: timeout waiting for pong %x from %v (%v, %v)", txid[:6], sp.to, de.publicKey.ShortString(), de.discoShort) } de.removeSentPingLocked(txid, sp) diff --git a/wgengine/magicsock/magicsock_linux.go b/wgengine/magicsock/magicsock_linux.go index 71e39cacb..fa0bb167e 100644 --- a/wgengine/magicsock/magicsock_linux.go +++ b/wgengine/magicsock/magicsock_linux.go @@ -28,7 +28,7 @@ const ( ) // Enable/disable using raw sockets to receive disco traffic. -var debugDisableRawDisco = envknob.Bool("TS_DEBUG_DISABLE_RAW_DISCO") +var debugDisableRawDisco = envknob.RegisterBool("TS_DEBUG_DISABLE_RAW_DISCO") // These are our BPF filters that we use for testing packets. var ( @@ -125,7 +125,7 @@ var ( // and BPF filter. // https://github.com/tailscale/tailscale/issues/3824 func (c *Conn) listenRawDisco(family string) (io.Closer, error) { - if debugDisableRawDisco { + if debugDisableRawDisco() { return nil, errors.New("raw disco listening disabled by debug flag") } diff --git a/wgengine/monitor/monitor_linux.go b/wgengine/monitor/monitor_linux.go index 07e3236eb..b6d1188bb 100644 --- a/wgengine/monitor/monitor_linux.go +++ b/wgengine/monitor/monitor_linux.go @@ -20,7 +20,7 @@ import ( "tailscale.com/types/logger" ) -var debugNetlinkMessages = envknob.Bool("TS_DEBUG_NETLINK") +var debugNetlinkMessages = envknob.RegisterBool("TS_DEBUG_NETLINK") // unspecifiedMessage is a minimal message implementation that should not // be ignored. In general, OS-specific implementations should use better @@ -96,7 +96,7 @@ func (c *nlConn) Receive() (message, error) { nip := netaddrIP(rmsg.Attributes.Address) - if debugNetlinkMessages { + if debugNetlinkMessages() { typ := "RTM_NEWADDR" if msg.Header.Type == unix.RTM_DELADDR { typ = "RTM_DELADDR" @@ -125,7 +125,7 @@ func (c *nlConn) Receive() (message, error) { } if addrs[nip] { - if debugNetlinkMessages { + if debugNetlinkMessages() { c.logf("ignored duplicate RTM_NEWADDR for %s", nip) } return ignoreMessage{}, nil @@ -147,7 +147,7 @@ func (c *nlConn) Receive() (message, error) { Addr: nip, Delete: msg.Header.Type == unix.RTM_DELADDR, } - if debugNetlinkMessages { + if debugNetlinkMessages() { c.logf("%+v", nam) } return nam, nil @@ -169,7 +169,7 @@ func (c *nlConn) Receive() (message, error) { (rmsg.Attributes.Table == 255 || rmsg.Attributes.Table == 254) && (dst.Addr().IsMulticast() || dst.Addr().IsLinkLocalUnicast()) { - if debugNetlinkMessages { + if debugNetlinkMessages() { c.logf("%s ignored", typeStr) } @@ -202,7 +202,7 @@ func (c *nlConn) Receive() (message, error) { Dst: dst, Gateway: gw, } - if debugNetlinkMessages { + if debugNetlinkMessages() { c.logf("%+v", nrm) } return nrm, nil @@ -225,7 +225,7 @@ func (c *nlConn) Receive() (message, error) { table: rmsg.Table, priority: rmsg.Attributes.Priority, } - if debugNetlinkMessages { + if debugNetlinkMessages() { c.logf("%+v", rdm) } return rdm, nil diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index 729a96bce..9740a5da6 100644 --- a/wgengine/netstack/netstack.go +++ b/wgengine/netstack/netstack.go @@ -55,7 +55,7 @@ import ( const debugPackets = false -var debugNetstack = envknob.Bool("TS_DEBUG_NETSTACK") +var debugNetstack = envknob.RegisterBool("TS_DEBUG_NETSTACK") var ( magicDNSIP = tsaddr.TailscaleServiceIP() @@ -638,7 +638,7 @@ func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte) { } return } - if debugNetstack { + if debugNetstack() { ns.logf("exec pinged %v in %v", dstIP, time.Since(t0)) } if err := ns.tundev.InjectOutbound(pingResPkt); err != nil { @@ -718,7 +718,7 @@ func netaddrIPFromNetstackIP(s tcpip.Address) netip.Addr { func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) { reqDetails := r.ID() - if debugNetstack { + if debugNetstack() { ns.logf("[v2] TCP ForwarderRequest: %s", stringifyTEI(reqDetails)) } clientRemoteIP := netaddrIPFromNetstackIP(reqDetails.RemoteAddress) @@ -849,7 +849,7 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) { func (ns *Impl) forwardTCP(getClient func() *gonet.TCPConn, clientRemoteIP netip.Addr, wq *waiter.Queue, dialAddr netip.AddrPort) (handled bool) { dialAddrStr := dialAddr.String() - if debugNetstack { + if debugNetstack() { ns.logf("[v2] netstack: forwarding incoming connection to %s", dialAddrStr) } @@ -866,7 +866,7 @@ func (ns *Impl) forwardTCP(getClient func() *gonet.TCPConn, clientRemoteIP netip go func() { select { case <-notifyCh: - if debugNetstack { + if debugNetstack() { ns.logf("[v2] netstack: forwardTCP notifyCh fired; canceling context for %s", dialAddrStr) } case <-done: @@ -919,7 +919,7 @@ func (ns *Impl) forwardTCP(getClient func() *gonet.TCPConn, clientRemoteIP netip func (ns *Impl) acceptUDP(r *udp.ForwarderRequest) { sess := r.ID() - if debugNetstack { + if debugNetstack() { ns.logf("[v2] UDP ForwarderRequest: %v", stringifyTEI(sess)) } var wq waiter.Queue @@ -995,7 +995,7 @@ func (ns *Impl) handleMagicDNSUDP(srcAddr netip.AddrPort, c *gonet.UDPConn) { // proxy to it directly. func (ns *Impl) forwardUDP(client *gonet.UDPConn, wq *waiter.Queue, clientAddr, dstAddr netip.AddrPort) { port, srcPort := dstAddr.Port(), clientAddr.Port() - if debugNetstack { + if debugNetstack() { ns.logf("[v2] netstack: forwarding incoming UDP connection on port %v", port) } @@ -1071,7 +1071,7 @@ func (ns *Impl) forwardUDP(client *gonet.UDPConn, wq *waiter.Queue, clientAddr, } func startPacketCopy(ctx context.Context, cancel context.CancelFunc, dst net.PacketConn, dstAddr net.Addr, src net.PacketConn, logf logger.Logf, extend func()) { - if debugNetstack { + if debugNetstack() { logf("[v2] netstack: startPacketCopy to %v (%T) from %T", dstAddr, dst, src) } go func() { @@ -1096,7 +1096,7 @@ func startPacketCopy(ctx context.Context, cancel context.CancelFunc, dst net.Pac } return } - if debugNetstack { + if debugNetstack() { logf("[v2] wrote UDP packet %s -> %s", srcAddr, dstAddr) } extend() diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index 3dcb4c205..2ea0fa3c8 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -316,7 +316,7 @@ func useAmbientCaps() bool { return distro.DSMVersion() >= 7 } -var forceIPCommand = envknob.Bool("TS_DEBUG_USE_IP_COMMAND") +var forceIPCommand = envknob.RegisterBool("TS_DEBUG_USE_IP_COMMAND") // useIPCommand reports whether r should use the "ip" command (or its // fake commandRunner for tests) instead of netlink. @@ -324,7 +324,7 @@ func (r *linuxRouter) useIPCommand() bool { if r.cmd == nil { panic("invalid init") } - if forceIPCommand { + if forceIPCommand() { return true } // In the future we might need to fall back to using the "ip" diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 224379514..f18997177 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -534,7 +534,7 @@ func (e *userspaceEngine) pollResolver() { } } -var debugTrimWireguard = envknob.OptBool("TS_DEBUG_TRIM_WIREGUARD") +var debugTrimWireguard = envknob.RegisterOptBool("TS_DEBUG_TRIM_WIREGUARD") // forceFullWireguardConfig reports whether we should give wireguard our full // network map, even for inactive peers. @@ -550,7 +550,7 @@ var debugTrimWireguard = envknob.OptBool("TS_DEBUG_TRIM_WIREGUARD") // with these knobs in place. func forceFullWireguardConfig(numPeers int) bool { // Did the user explicitly enable trimmming via the environment variable knob? - if b, ok := debugTrimWireguard.Get(); ok { + if b, ok := debugTrimWireguard().Get(); ok { return !b } if opt := controlclient.TrimWGConfig(); opt != "" {