Commit Graph

1915 Commits (alexc/better-localbackend-logging)

Author SHA1 Message Date
James 'zofrex' Sanderson 2d1014ead1
ipn/ipnlocal: fix data race on captiveCtx in enterStateLockedOnEntry (#17495)
Updates #17491

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
2 months ago
Brad Fitzpatrick 5c1e26b42f ipn/localapi: dead code eliminate unreachable/useless LocalAPI handlers when disabled
Saves ~94 KB from the min build.

Updates #12614

Change-Id: I3b0b8a47f80b9fd3b1038c2834b60afa55bf02c2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Alex Chan a9334576ea ipn/ipnlocal: use named arguments for `mockControl.send()`
Updates #cleanup

Signed-off-by: Alex Chan <alexc@tailscale.com>
2 months ago
James 'zofrex' Sanderson eabc62a9dd
ipn/ipnlocal: don't send LoginFinished unless auth was in progress (#17266)
Before we introduced seamless, the "blocked" state was used to track:

* Whether a login was required for connectivity, and therefore we should
  keep the engine deconfigured until that happened
* Whether authentication was in progress

"blocked" would stop authReconfig from running. We want this when a login is
required: if your key has expired we want to deconfigure the engine and keep
it down, so that you don't keep using exit nodes (which won't work because
your key has expired).

Taking the engine down while auth was in progress was undesirable, so we
don't do that with seamless renewal. However, not entering the "blocked"
state meant that we needed to change the logic for when to send
LoginFinished on the IPN bus after seeing StateAuthenticated from the
controlclient. Initially we changed the "if blocked" check to "if blocked or
seamless is enabled" which was correct in other places.

In this place however, it introduced a bug: we are sending LoginFinished
every time we see StateAuthenticated, which happens even on a down & up, or
a profile switch. This in turn made it harder for UI clients to track when
authentication is complete.

Instead we should only send it out if we were blocked (i.e. seamless is
disabled, or our key expired) or an auth was in progress.

Updates tailscale/corp#31476

Updates tailscale/corp#32645

Fixes #17363

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
2 months ago
Brad Fitzpatrick 316afe7d02 util/checkchange: stop using deephash everywhere
Saves 45 KB from the min build, no longer pulling in deephash or
util/hashx, both with unsafe code.

It can actually be more efficient to not use deephash, as you don't
have to walk all bytes of all fields recursively to answer that two
things are not equal. Instead, you can just return false at the first
difference you see. And then with views (as we use ~everywhere
nowadays), the cloning the old value isn't expensive, as it's just a
pointer under the hood.

Updates #12614

Change-Id: I7b08616b8a09b3ade454bb5e0ac5672086fe8aec
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
M. J. Fromberger 0415a56b6c
ipn/ipnlocal: fix another racy test (#17472)
Some of the test cases access fields of the backend that are supposed to be
locked while the test is running, which can trigger the race detector.  I fixed
a few of these in #17411, but I missed these two cases.

Updates #15160
Updates #17192

Change-Id: I45664d5e34320ecdccd2844e0f8b228145aaf603
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
2 months ago
M. J. Fromberger e0f222b686
appc,ipn/ipnlocal: receive AppConnector updates via the event bus (#17411)
Add subscribers for AppConnector events

Make the RouteAdvertiser interface optional We cannot yet remove it because
the tests still depend on it to verify correctness. We will need to separately
update the test fixtures to remove that dependency.

Publish RouteInfo via the event bus, so we do not need a callback to do that. 
Replace it with a flag that indicates whether to treat the route info the connector 
has as "definitive" for filtering purposes.

Update the tests to simplify the construction of AppConnector values now that a
store callback is no longer required. Also fix a couple of pre-existing racy tests that 
were hidden by not being concurrent in the same way production is.

Updates #15160
Updates #17192

Change-Id: Id39525c0f02184e88feaf0d8a3c05504850e47ee
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
2 months ago
James 'zofrex' Sanderson 7407f404d9
ipn/ipnlocal: fix setAuthURL / setWgengineStatus race condition (#17408)
If we received a wg engine status while processing an auth URL, there was a
race condition where the authURL could be reset to "" immediately after we
set it.

To fix this we need to check that we are moving from a non-Running state to
a Running state rather than always resetting the URL when we "move" into a
Running state even if that is the current state.

We also need to make sure that we do not return from stopEngineAndWait until
the engine is stopped: before, we would return as soon as we received any
engine status update, but that might have been an update already in-flight
before we asked the engine to stop. Now we wait until we see an update that
is indicative of a stopped engine, or we see that the engine is unblocked
again, which indicates that the engine stopped and then started again while
we were waiting before we checked the state.

Updates #17388

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
Co-authored-by: Nick Khyl <nickk@tailscale.com>
2 months ago
Brad Fitzpatrick d816454a88 feature/featuretags: make usermetrics modular
Saves ~102 KB from the min build.

Updates #12614

Change-Id: Ie1d4f439321267b9f98046593cb289ee3c4d6249
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 541a4ed5b4 all: use buildfeatures consts in a few more places
Saves ~25 KB.

Updates #12614

Change-Id: I7b976e57819a0d2692824d779c8cc98033df0d30
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick db65f3fcf8 ipn/ipnlocal: use buildfeature consts in a few more places
Updates #12614

Change-Id: I561d434d9829172a3d7f6933399237924ff80490
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Simon Law 9c3aec58ba
ipn/ipnlocal: remove junk from suggestExitNodeUsingTrafficSteering (#17436)
This patch removes some code that didn’t get removed before merging
the changes in #16580.

Updates #cleanup
Updates #16551

Signed-off-by: Simon Law <sfllaw@tailscale.com>
2 months ago
Brad Fitzpatrick f42be719de all: use buildfeature constants in a few more places
Saves 21 KB.

Updates #12614

Change-Id: I0cd3e735937b0f5c0fcc9f09a24476b1c4ac9a15
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Alex Chan 304dabce17 ipn/ipnauth: fix a null pointer panic in GetConnIdentity
When running integration tests on macOS, we get a panic from a nil
pointer dereference when calling `ci.creds.PID()`.

This panic occurs because the `ci.creds != nil` check is insufficient
after a recent refactoring (c45f881) that changed `ci.creds` from a
pointer to the `PeerCreds` interface. Now `ci.creds` always compares as
non-nil, so we enter this block even when the underlying value is nil.

The integration tests fail on macOS when `peercred.Get()` returns the
error `unix.GetsockoptInt: socket is not connected`. This error isn't
new, and the previous code was ignoring it correctly.

Since we trust that `peercred` returns either a usable value or an error,
checking for a nil error is a sufficient and correct gate to prevent the
method call and avoid the panic.

Fixes #17421

Signed-off-by: Alex Chan <alexc@tailscale.com>
2 months ago
Simon Law cd523eae52
ipn/ipnlocal: introduce the concept of client-side-reachability (#17367)
The control plane will sometimes determine that a node is not online,
while the node is still able to connect to its peers. This patch
doesn’t solve this problem, but it does mitigate it.

This PR introduces the `client-side-reachability` node attribute that
switches the node to completely ignore the online signal from control.

In the future, the client itself should collect reachability data from
active Wireguard flows and Tailscale pings.

Updates #17366
Updates tailscale/corp#30379
Updates tailscale/corp#32686

Signed-off-by: Simon Law <sfllaw@tailscale.com>
2 months ago
Brad Fitzpatrick 24e38eb729 control/controlclient,health,ipn/ipnlocal,health: fix deadlock by deleting health reporting
A recent change (009d702adf) introduced a deadlock where the
/machine/update-health network request to report the client's health
status update to the control plane was moved to being synchronous
within the eventbus's pump machinery.

I started to instead make the health reporting be async, but then we
realized in the three years since we added that, it's barely been used
and doesn't pay for itself, for how many HTTP requests it makes.

Instead, delete it all and replace it with a c2n handler, which
provides much more helpful information.

Fixes tailscale/corp#32952

Change-Id: I9e8a5458269ebfdda1c752d7bbb8af2780d71b04
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick a208cb9fd5 feature/featuretags: add features for c2n, peerapi, advertise/use routes/exit nodes
Saves 262 KB so far. I'm sure I missed some places, but shotizam says
these were the low hanging fruit.

Updates #12614

Change-Id: Ia31c01b454f627e6d0470229aae4e19d615e45e3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 3ae7a351b4 feature/featuretags: make clientmetrics optional
Saves 57 KB

Updates #12614

Change-Id: If7eebec12b3cb30ae6264171d36a258c04b05a70
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
M. J. Fromberger 127a967207
appc,*: publish events for route updates and storage (#17392)
Add and wire up event publishers for these two event types in the AppConnector.
Nothing currently subscribes to them, so this is harmless. Subscribers for
these events will be added in a near-future commit.

As part of this, move the appc.RouteInfo type to the types/appctype package.
It does not contain any package-specific details from appc. Beside it, add
appctype.RouteUpdate to carry route update event state, likewise not specific
to appc.  Update all usage of the appc.* types throughout to use appctype.*
instead, and update depaware files to reflect these changes.

Add a Close method to the AppConnector to make sure the client gets cleaned up
when the connector is dropped (we re-create connectors).

Update the unit tests in the appc package to also check the events published
alongside calls to the RouteAdvertiser.

For now the tests still rely on the RouteAdvertiser for correctness; this is OK
for now as the two methods are always performed together.  In the near future,
we need to rework the tests so not require that, but that will require building
some more test fixtures that we can handle separately.

Updates #15160
Updates #17192

Change-Id: I184670ba2fb920e0d2cb2be7c6816259bca77afe
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
2 months ago
Brad Fitzpatrick c45f8813b4 feature/featuretags, all: add build features, use existing ones in more places
Saves 270 KB.

Updates #12614

Change-Id: I4c3fe06d32c49edb3a4bb0758a8617d83f291cf5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Andrew Lytvynov cca70ddbfc
cmd/tailscaled: default --encrypt-state to true if TPM is available (#17376)
Whenever running on a platform that has a TPM (and tailscaled can access
it), default to encrypting the state. The user can still explicitly set
this flag to disable encryption.

Updates https://github.com/tailscale/corp/issues/32909

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
2 months ago
M. J. Fromberger 67f1081269
appc,ipn/ipnlocal: add a required event bus to the AppConnector type (#17390)
Require the presence of the bus, but do not use it yet.  Check for required
fields and update tests and production use to plumb the necessary arguments.

Updates #15160
Updates #17192

Change-Id: I8cefd2fdb314ca9945317d3320bd5ea6a92e8dcb
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
2 months ago
Claus Lensbøl ce752b8a88
net/netmon: remove usage of direct callbacks from netmon (#17292)
The callback itself is not removed as it is used in other repos, making
it simpler for those to slowly transition to the eventbus.

Updates #15160

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
2 months ago
M. J. Fromberger 6f7ce5eb5d
appc: factor app connector arguments into a Config type (#17389)
Replace the positional arguments to NewAppConnector with a Config struct.
Update the existing uses. Other than the API change, there are no functional
changes in this commit.

Updates #15160
Updates #17192

Change-Id: Ibf37f021372155a4db8aaf738f4b4f2c746bf623
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
2 months ago
Brad Fitzpatrick 05a4c8e839 tsnet: remove AuthenticatedAPITransport (API-over-noise) support
It never launched and I've lost hope of it launching and it's in my
way now, so I guess it's time to say goodbye.

Updates tailscale/corp#4383
Updates #17305

Change-Id: I2eb551d49f2fb062979cc307f284df4b3dfa5956
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick c2f37c891c all: use Go 1.20's errors.Join instead of our multierr package
Updates #7123

Change-Id: Ie9be6814831f661ad5636afcd51d063a0d7a907d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 91fa51ca15 ipn/store, feature/condregister: permit callers to empty import optonal ipn stores
This permits other programs (in other repos) to conditionally
import ipn/store/awsstore and/or ipn/store/kubestore and have them
register themselves, rather than feature/condregister doing it.

Updates tailscale/corp#32922

Change-Id: I2936229ce37fd2acf9be5bf5254d4a262d090ec1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
James Sanderson ebc370e517 ipn/ipnlocal: fail test if more notifies are put than expected
The `put` callback runs on a different goroutine to the test, so calling
t.Fatalf in put had no effect. `drain` is always called when checking what
was put and is called from the test goroutine, so that's a good place to
fail the test if the channel was too full.

Updates #17363

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
2 months ago
Nick Khyl 9781b7c25c ipn/ipnlocal: plumb logf into nodeBackend
Updates #cleanup

Signed-off-by: Nick Khyl <nickk@tailscale.com>
2 months ago
Brad Fitzpatrick 5b09913d64 ipn/ipnlocal, engine: avoid runtime/pprof with two usages of ts_omit_debug
Saves 258 KB.

Updates #12614

Change-Id: I37c2f7f916480e3534883f338de4c64d08f7ef2b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick f7afb9b6ca feature/featuretags, ipn/conffile: make HuJSON support in config files optional
Saves 33 KB.

Updates #12614

Change-Id: Ie701c230e0765281f409f29ed263910b9be9cc77
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 6c6a1d8341 feature/appconnectors: start making it modular
Saves 45 KB.

Updates #12614

Change-Id: Iaeb73e69633878ce0a0f58c986024784bbe218f1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 9386a101d8 cmd/tailscaled, ipn/localapi, util/eventbus: don't link in regexp when debug is omitted
Saves 442 KB. Lock it with a new min test.

Updates #12614

Change-Id: Ia7bf6f797b6cbf08ea65419ade2f359d390f8e91
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick ee034d48fc feature/featuretags: add a catch-all "Debug" feature flag
Saves 168 KB.

Updates #12614

Change-Id: Iaab3ae3efc6ddc7da39629ef13e5ec44976952ba
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 442a3a779d feature, net/tshttpproxy: pull out support for using proxies as a feature
Saves 139 KB.

Also Synology support, which I saw had its own large-ish proxy parsing
support on Linux, but support for proxies without Synology proxy
support is reasonable, so I pulled that out as its own thing.

Updates #12614

Change-Id: I22de285a3def7be77fdcf23e2bec7c83c9655593
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 2c956e30be ipn/ipnlocal: proxy h2c grpc using net/http.Transport instead of x/net/http2
(Kinda related: #17351)

Updates #17305

Change-Id: I47df2612732a5713577164e74652bc9fa3cd14b3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 3f5c560fd4 ipn/ipnlocal: drop h2c package, use net/http's support
In Dec 2021 in d3d503d997 I had grand plans to make exit node DNS
cheaper by using HTTP/2 over PeerAPI, at least on some platforms. I
only did server-side support though and never made it to the client.

In the ~4 years since, some things have happened:

* Go 1.24 got support for http.Protocols (https://pkg.go.dev/net/http#Protocols)
  and doing UnencryptedHTTP2 ("HTTP2 with prior knowledge")
* The old h2c upgrade mechanism was deprecated; see https://github.com/golang/go/issues/63565
  and https://github.com/golang/go/issues/67816
* Go plans to deprecate x/net/http2 and move everything to the standard library.

So this drops our use of the x/net/http2/h2c package and instead
enables h2c (on all platforms now) using the standard library.

This does mean we lose the deprecated h2c Upgrade support, but that's
fine.

If/when we do the h2c client support for ExitDNS, we'll have to probe
the peer to see whether it supports it. Or have it reply with a header
saying that future requests can us h2c. (It's tempting to use capver,
but maybe people will disable that support anyway, so we should
discover it at runtime instead.)

Also do the same in the sessionrecording package.

Updates #17305

Change-Id: If323f5ef32486effb18ed836888aa05c0efb701e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick bcd79b161a feature/featuretags: add option to turn off DNS
Saves 328 KB (2.5%) off the minimal binary.

For IoT devices that don't need MagicDNS (e.g. they don't make
outbound connections), this provides a knob to disable all the DNS
functionality.

Rather than a massive refactor today, this uses constant false values
as a deadcode sledgehammer, guided by shotizam to find the largest DNS
functions which survived deadcode.

A future refactor could make it so that the net/dns/resolver and
publicdns packages don't even show up in the import graph (along with
their imports) but really it's already pretty good looking with just
these consts, so it's not at the top of my list to refactor it more
soon.

Also do the same in a few places with the ACME (cert) functionality,
as I saw those while searching for DNS stuff.

Updates #12614

Change-Id: I8e459f595c2fde68ca16503ff61c8ab339871f97
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick ba76578447 ipn/ipnlocal, feature/posture: pull posture out into a modular feature
Updates #12614

Change-Id: I9d08a1330b9c55e1a23e7979a707e11d8e090d79
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 038cdb4640 feature/clientupdate: move clientupdate to a modular feature, disabled for tsnet
Updates #12614

Change-Id: I5f685dec84a5396b7c2b66f2788ae3d286e1ddc6
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 69c79cb9f3 ipn/store, feature/condregister: move AWS + Kube store registration to condregister
Otherwise they're uselessly imported by tsnet applications, even
though they do nothing. tsnet applications wanting to use these
already had to explicitly import them and use kubestore.New or
awsstore.New and assign those to their tsnet.Server.Store fields.

Updates #12614

Change-Id: I358e3923686ddf43a85e6923c3828ba2198991d4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Fran Bull 65d6c80695 cmd/tailscale/cli,client,ipn: add appc-routes cli command
Allow the user to access information about routes an app connector has
learned, such as how many routes for each domain.

Fixes tailscale/corp#32624

Signed-off-by: Fran Bull <fran@tailscale.com>
2 months ago
Brad Fitzpatrick 976389c0f7 feature/sdnotify: move util/systemd to a modular feature
Updates #12614

Change-Id: I08e714c83b455df7f538cc99cafe940db936b480
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 11b770fbc9 feature/logtail: pull logtail + netlog out to modular features
Removes 434 KB from the minimal Linux binary, or ~3%.

Primarily this comes from not linking in the zstd encoding code.

Fixes #17323

Change-Id: I0a90de307dfa1ad7422db7aa8b1b46c782bfaaf7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 01e645fae1 util/backoff: rename logtail/backoff package to util/backoff
It has nothing to do with logtail and is confusing named like that.

Updates #cleanup
Updates #17323

Change-Id: Idd34587ba186a2416725f72ffc4c5778b0b9db4a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick dd615c8fdd util/linuxfw, feature/buildfeatures: add ts_omit_iptables to make IPTables optional
Updates #12614

Change-Id: Ic0eba982aa8468a55c63e1b763345f032a55b4e2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick d01a0adfa6 types/dnstype: delete unused func, move other one to its sole caller
The dnstype package is used by tailcfg, which tries to be light and
leafy. But it brings in dnstype. So dnstype shouldn't bring in
x/net/dns/dnsmessage.

Updates #12614

Change-Id: I043637a7ce7fed097e648001f13ca1927a781def
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 832e94607e doctor: add ts_omit_doctor support
Updates #12614

Change-Id: I84c166c4b99ca75d70abe4087e5ff3f7d90d4bcc
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 87ee0f4e98 ipn/ipnlocal: move last unconditional gvisor import, complete ts_omit_netstack support
Fixes #17283

Change-Id: Ia84d269683e4a68d7d10562561204934eeaf53bb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Nick Khyl 892f8a9582 various: allow tailscaled shutdown via LocalAPI
A customer wants to allow their employees to restart tailscaled at will, when access rights and MDM policy allow it,
as a way to fully reset client state and re-create the tunnel in case of connectivity issues.

On Windows, the main tailscaled process runs as a child of a service process. The service restarts the child
when it exits (or crashes) until the service itself is stopped. Regular (non-admin) users can't stop the service,
and allowing them to do so isn't ideal, especially in managed or multi-user environments.

In this PR, we add a LocalAPI endpoint that instructs ipnserver.Server, and by extension the tailscaled process,
to shut down. The service then restarts the child tailscaled. Shutting down tailscaled requires LocalAPI write access
and an enabled policy setting.

Updates tailscale/corp#32674
Updates tailscale/corp#32675

Signed-off-by: Nick Khyl <nickk@tailscale.com>
2 months ago
Brad Fitzpatrick 45d635cc98 feature/portlist: pull portlist service porting into extension, use eventbus
And yay: tsnet (and thus k8s-operator etc) no longer depends on
portlist! And LocalBackend is smaller.

Removes 50 KB from the minimal binary.

Updates #12614

Change-Id: Iee04057053dc39305303e8bd1d9599db8368d926
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Nick Khyl bbc5107d7d ipn/ipnlocal: do not reset extHost on (*LocalBackend).Shutdown
We made changes to ipnext callback registration/unregistration/invocation in #15780
that made resetting b.exthost to a nil, no-op host in (*LocalBackend).Shutdown() unnecessary.

But resetting it is also racy: b.exthost must be safe for concurrent use with or without b.mu held,
so it shouldn't be written after NewLocalBackend returns. This PR removes it.

Fixes #17279

Signed-off-by: Nick Khyl <nickk@tailscale.com>
2 months ago
Brad Fitzpatrick 0bd4f4729b ipn/ipnlocal: rename misnamed DisablePortMapperForTest to DisablePortPollerForTest
I think this was originally a brain-o in 9380e2dfc6. It's
disabling the port _poller_, listing what open ports (i.e. services)
are open, not PMP/PCP/UPnP port mapping.

While there, drop in some more testenv.AssertInTest() in a few places.

Updates #cleanup

Change-Id: Ia6f755ad3544f855883b8a7bdcfc066e8649547b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 21dc5f4e21 derp/derpserver: split off derp.Server out of derp into its own package
This exports a number of things from the derp (generic + client) package
to be used by the new derpserver package, as now used by cmd/derper.

And then enough other misc changes to lock in that cmd/tailscaled can
be configured to not bring in tailscale.com/client/local. (The webclient
in particular, even when disabled, was bringing it in, so that's now fixed)

Fixes #17257

Change-Id: I88b6c7958643fb54f386dd900bddf73d2d4d96d5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick b54cdf9f38 all: use buildfeatures.HasCapture const in a handful of places
Help out the linker's dead code elimination.

Updates #12614

Change-Id: I6c13cb44d3250bf1e3a01ad393c637da4613affb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 8fe575409f feature/featuretags: add build tag to remove captive portal detection
This doesn't yet fully pull it out into a feature/captiveportal package.
This is the usual first step, moving the code to its own files within
the same packages.

Updates #17254

Change-Id: Idfaec839debf7c96f51ca6520ce36ccf2f8eec92
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
Brad Fitzpatrick 87ccfbd250 ipn/ipnlocal: fix eventbus data race
Fixes #17252

Change-Id: Id969fca750a48fb43431c53f3e0631bd9bd496d1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 months ago
M. J. Fromberger 1b5201023f
ipn/ipnlocal: use eventbus.Monitor in LocalBackend (#17225)
This commit does not change the order or meaning of any eventbus activity, it
only updates the way the plumbing is set up.

Updates #15160

Change-Id: I06860ac4e43952a9bb4d85366138c9d9a17fd9cd
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
Brad Fitzpatrick 8ec07b5f7f ipn/ipnauth: don't crash on OpenBSD trying to log username of unknown peer
We never implemented the peercred package on OpenBSD (and I just tried
again and failed), but we've always documented that the creds pointer
can be nil for operating systems where we can't map the unix socket
back to its UID. On those platforms, we set the default unix socket
permissions such that only the admin can open it anyway and we don't
have a read-only vs read-write distinction. OpenBSD was always in that
camp, where any access to Tailscale's unix socket meant full access.

But during some refactoring, we broke OpenBSD in that we started
assuming during one logging path (during login) that Creds was non-nil
when looking up an ipnauth.Actor's username, which wasn't relevant (it
was called from a function "maybeUsernameOf" anyway, which threw away
errors).

Verified on an OpenBSD VM. We don't have any OpenBSD integration tests yet.

Fixes #17209
Updates #17221

Change-Id: I473c5903dfaa645694bcc75e7f5d484f3dd6044d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
M. J. Fromberger f9c699812a
ipn/ipnlocal: use eventbus.Monitor in expiryManager (#17204)
This commit does not change the order or meaning of any eventbus activity, it
only updates the way the plumbing is set up.

Updates #15160

Change-Id: I0a175e67e867459daaedba0731bf68bd331e5ebc
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
Anton Tolchanov 2351cc0d0e ipn/ipnlocal: make the redactNetmapPrivateKeys test recursive
Expand TestRedactNetmapPrivateKeys to cover all sub-structs of
NetworkMap and confirm that a) all fields are annotated as private or
public, and b) all private fields are getting redacted.

Updates tailscale/corp#32095

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
3 months ago
Anton Tolchanov 4a04161828 ipn/ipnlocal: add a C2N endpoint for fetching a netmap
For debugging purposes, add a new C2N endpoint returning the current
netmap. Optionally, coordination server can send a new "candidate" map
response, which the client will generate a separate netmap for.
Coordination server can later compare two netmaps, detecting unexpected
changes to the client state.

Updates tailscale/corp#32095

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
3 months ago
Alex Chan cd153aa644 control, ipn, tailcfg: enable seamless key renewal by default
Previously, seamless key renewal was an opt-in feature.  Customers had
to set a `seamless-key-renewal` node attribute in their policy file.

This patch enables seamless key renewal by default for all clients.

It includes a `disable-seamless-key-renewal` node attribute we can set
in Control, so we can manage the rollout and disable the feature for
clients with known bugs.  This new attribute makes the feature opt-out.

Updates tailscale/corp#31479

Signed-off-by: Alex Chan <alexc@tailscale.com>
3 months ago
Claus Lensbøl df362d0a08
net/netmon: make ChangeDelta event not a pointer (#17112)
This makes things work slightly better over the eventbus.

Also switches ipnlocal to use the event over the eventbus instead of the
direct callback.

Updates #15160

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
3 months ago
James Sanderson ddc0cd7e1e ipn/ipnlocal: disconnect and block when key expires even when using seamless
Updates tailscale/corp#31478

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
3 months ago
Brad Fitzpatrick 697098ed6c ipn/ipnlocal: fix a case where ts_omit_ssh was still linking in x/crypto/ssh
And add a test.

Updates #12614

Change-Id: Icb1c77f5890def794a4938583725c1a0886b197d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick e180fc267b feature/featuretags, all: add ts_omit_acme to disable TLS cert support
I'd started to do this in the earlier ts_omit_server PR but
decided to split it into this separate PR.

Updates #17128

Change-Id: Ief8823a78d1f7bbb79e64a5cab30a7d0a5d6ff4b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick 99b3f69126 feature/portmapper: make the portmapper & its debugging tools modular
Starting at a minimal binary and adding one feature back...
    tailscaled tailscale combined (linux/amd64)
     30073135  17451704  31543692 omitting everything
    +  480302 +   10258 +  493896 .. add debugportmapper
    +  475317 +  151943 +  467660 .. add portmapper
    +  500086 +  162873 +  510511 .. add portmapper+debugportmapper

Fixes #17148

Change-Id: I90bd0e9d1bd8cbe64fa2e885e9afef8fb5ee74b1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
M. J. Fromberger 8608e42103
feature,ipn/ipnlocal,wgengine: improve how eventbus shutdown is handled (#17156)
Instead of waiting for a designated subscription to close as a canary for the
bus being stopped, use the bus Client's own signal for closure added in #17118.

Updates #cleanup

Change-Id: I384ea39f3f1f6a030a6282356f7b5bdcdf8d7102
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
Alex Chan 84659b1dc6 ipn: fix the string representation of an empty ipn.Notify
Before: `ipn.Notify}`
After:  `ipn.Notify{}`

Updates #cleanup

Signed-off-by: Alex Chan <alexc@tailscale.com>
3 months ago
Claus Lensbøl 2015ce4081
health,ipn/ipnlocal: introduce eventbus in heath.Tracker (#17085)
The Tracker was using direct callbacks to ipnlocal. This PR moves those
to be triggered via the eventbus.

Additionally, the eventbus is now closed on exit from tailscaled
explicitly, and health is now a SubSystem in tsd.

Updates #15160

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
3 months ago
Brad Fitzpatrick 4cca9f7c67 all: add ts_omit_serve, start making tailscale serve/funnel be modular
tailscaled tailscale combined (linux/amd64)
     29853147  17384418  31412596 omitting everything
    +  621570 +  219277 +  554256 .. add serve

Updates #17128

Change-Id: I87c2c6c3d3fc2dc026c3de8ef7000a813b41d31c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Claus Lensbøl b816fd7117
control/controlclient: introduce eventbus messages instead of callbacks (#16956)
This is a small introduction of the eventbus into controlclient that
communicates with mainly ipnlocal. While ipnlocal is a complicated part
of the codebase, the subscribers here are from the perspective of
ipnlocal already called async.

Updates #15160

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
3 months ago
Brad Fitzpatrick 3a49b7464c all: add ts_omit_tailnetlock as a start of making it build-time modular
Updates #17115

Change-Id: I6b083c0db4c4d359e49eb129d626b7f128f0a9d2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick a1dcf12b67 feature/drive: start factoring out Taildrive, add ts_omit_drive build tag
As of this commit (per the issue), the Taildrive code remains where it
was, but in new files that are protected by the new ts_omit_drive
build tag. Future commits will move it.

Updates #17058

Change-Id: Idf0a51db59e41ae8da6ea2b11d238aefc48b219e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
nikiUppal-TS 88d7db33da
cmd/tailscale: use tailnet display name on cli (#17079)
Updates cli to use tailnet display name

Updates tailscale/corp#32108

Signed-off-by: nikiUppal-TS <nikita@tailscale.com>
3 months ago
Nick O'Neill 77250a301a
ipn/ipnlocal, types: plumb tailnet display name cap through to network profile (#17045)
Updates tailscale/corp#30456

Signed-off-by: Nick O'Neill <nick@tailscale.com>
3 months ago
Brad Fitzpatrick 3e4b0c1516 cmd/tailscale, ipn/ipnlocal: add ts_omit_webclient
Fixes #17063
Updates #12614

Change-Id: I0a189f6a4d1c4558351e3195839867725774fa96
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
James Sanderson 046b8830c7 ipn/ipnlocal: add state change test for key expiry
Updates tailscale/corp#31478

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
3 months ago
Brad Fitzpatrick b034f7cca9 ipn/ipnlocal, util/syspolicy: convert last RegisterWellKnownSettingsForTest caller, remove
Updates #16998

Change-Id: I735d75129a97a929092e9075107e41cdade18944
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Jonathan Nobels a2f2ac6ba1
ipn/local: fix deadlock in initial suggested exit node query (#17025)
updates tailscale/corp#26369

b.mu is locked here.  We need to use suggestExitNodeLocked.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
3 months ago
Alex Chan c9f214e503
ipn: warn about self as the exit node if backend is running (#17018)
Before:

    $ tailscale ip -4
    1.2.3.4

    $ tailscale set --exit-node=1.2.3.4
    no node found in netmap with IP 1.2.3.4

After:

    $ tailscale set --exit-node=1.2.3.4
    cannot use 1.2.3.4 as an exit node as it is a local IP address to this machine; did you mean --advertise-exit-node?

The new error message already existed in the code, but would only be
triggered if the backend wasn't running -- which means, in practice,
it would almost never be triggered.

The old error message is technically true, but could be confusing if you
don't know the distinction between "netmap" and "tailnet" -- it could
sound like the exit node isn't part of your tailnet. A node is never in
its own netmap, but it is part of your tailnet.

This error confused me when I was doing some local dev work, and it's
confused customers before (e.g. #7513). Using the more specific error
message should reduce confusion.

Updates #7513
Updates https://github.com/tailscale/corp/issues/23596

Signed-off-by: Alex Chan <alexc@tailscale.com>
3 months ago
Brad Fitzpatrick d06d9007a6 ipn/ipnlocal: convert more tests to use policytest, de-global-ify
Now that we have policytest and the policyclient.Client interface, we
can de-global-ify many of the tests, letting them run concurrently
with each other, and just removing global variable complexity.

This does ~half of the LocalBackend ones.

Updates #16998

Change-Id: Iece754e1ef4e49744ccd967fa83629d0dca6f66a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick 21f21bd2a2 util/syspolicy: finish adding ts_omit_syspolicy build tags, tests
Fixes #16998
Updates #12614

Change-Id: Idf2b1657898111df4be31f356091b2376d0d7f0b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick 24b8a57b1e util/syspolicy/policytest: move policy test helper to its own package
Updates #16998
Updates #12614

Change-Id: I9fd27d653ebee547951705dc5597481e85b60747
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick 2b3e533048 util/syspolicy: finish plumbing policyclient, add feature/syspolicy, move global impl
This is step 4 of making syspolicy a build-time feature.

This adds a policyclient.Get() accessor to return the correct
implementation to use: either the real one, or the no-op one. (A third
type, a static one for testing, also exists, so in general a
policyclient.Client should be plumbed around and not always fetched
via policyclient.Get whenever possible, especially if tests need to use
alternate syspolicy)

Updates #16998
Updates #12614

Change-Id: Iaf19670744a596d5918acfa744f5db4564272978
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
M. J. Fromberger 9e9bf13063
ipn/ipnlocal: revert some locking changes ahead of release branch cut (#17011) 3 months ago
Brad Fitzpatrick 0d23490e1a ipn/ipnlocal: simplify a test with a new simpler syspolicy client test type
Less indirection.

Updates #16998
Updates #12614

Change-Id: I5a3a3c3f3b195486b2731ec002d2532337b3d211
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick 1ca4ae598a ipn/ipnlocal: use policyclient.Client always, stop using global syspolicy funcs
Step 4 of N. See earlier commits in the series (via the issue) for the
plan.

This adds the missing methods to policyclient.Client and then uses it
everywhere in ipn/ipnlocal and locks it in with a new dep test.

Still plenty of users of the global syspolicy elsewhere in the tree,
but this is a lot of them.

Updates #16998
Updates #12614

Change-Id: I25b136539ae1eedbcba80124de842970db0ca314
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick d05e6dc09e util/syspolicy/policyclient: add policyclient.Client interface, start plumbing
This is step 2 of ~4, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.

Step 1 was #16984.

In this second step, the util/syspolicy/policyclient package is added
with the policyclient.Client interface.  This is the interface that's
always present (regardless of build tags), and is what code around the
tree uses to ask syspolicy/MDM questions.

There are two implementations of policyclient.Client for now:

1) NoPolicyClient, which only returns default values.
2) the unexported, temporary 'globalSyspolicy', which is implemented
   in terms of the global functions we wish to later eliminate.

This then starts to plumb around the policyclient.Client to most callers.

Future changes will plumb it more. When the last of the global func
callers are gone, then we can unexport the global functions and make a
proper policyclient.Client type and constructor in the syspolicy
package, removing the globalSyspolicy impl out of tsd.

The final change will sprinkle build tags in a few more places and
lock it in with dependency tests to make sure the dependencies don't
later creep back in.

Updates #16998
Updates #12614

Change-Id: Ib2c93d15c15c1f2b981464099177cd492d50391c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Brad Fitzpatrick cc532efc20 util/syspolicy/*: move syspolicy keys to new const leaf "pkey" package
This is step 1 of ~3, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.

In this first (very noisy) step, all the syspolicy string key
constants move to a new constant-only (code-free) package. This will
make future steps more reviewable, without this movement noise.

There are no code or behavior changes here.

The future steps of this series can be seen in #14720: removing global
funcs from syspolicy resolution and using an interface that's plumbed
around instead. Then adding build tags.

Updates #12614

Change-Id: If73bf2c28b9c9b1a408fe868b0b6a25b03eeabd1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 months ago
Maisem Ali 882b05fff9 cmd/viewer: add field comments to generated view methods
Extract field comments from AST and include them in generated view
methods. Comments are preserved from the original struct fields to
provide documentation for the view accessors.

Fixes #16958

Signed-off-by: Maisem Ali <3953239+maisem@users.noreply.github.com>
3 months ago
Jonathan Nobels 80f5a00e76
ipn/local: add the suggested exit node to the ipn bus (#16748)
fixes tailscale/corp#26369

The suggested exit node is currently only calculated during a localAPI request.
For older UIs, this wasn't a bad choice - we could just fetch it on-demand when a menu
presented itself.  For newer incarnations however, this is an always-visible field
that needs to react to changes in the suggested exit node's value.

This change recalculates the suggested exit node ID on netmap updates and
broadcasts it on the IPN bus.   The localAPI version of this remains intact for the
time being.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
3 months ago
M. J. Fromberger b411ffb52f ipn/ipnlocal: remove UnlockEarly from doSetHostinfoFilterServices
Pull the lock-bearing code into a closure, and use a clone rather than a
shallow copy of the hostinfo record.

Updates #11649

Change-Id: I4f1d42c42ce45e493b204baae0d50b1cbf82b102
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 9002e5fd6b ipn/ipnlocal: remove an unnecessary unlock shortcut
The early unlock on this branch was required because the "send" method goes on
to acquire the mutex itself. Rather than release the lock just to acquire it
again, call the underlying locked helper directly.

Updates #11649

Change-Id: I50d81864a00150fc41460b7486a9c65655f282f5
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 2fb9472990 ipn/ipnlocal: remove unnecessary usees of lockAndGetUnlock
In places where we are locking the LocakBackend and immediately deferring an
unlock, and where there is no shortcut path in the control flow below the
deferral, we do not need the unlockOnce helper. Replace all these with use of
the lock directly.

Updates #11649

Change-Id: I3e6a7110dfc9ec6c1d38d2585c5367a0d4e76514
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
3 months ago
M. J. Fromberger 16bd60f9ca
ipn,tsnet: update AdvertiseTags documentation (#16931)
Instead of referring to groups, which is a term of art for a different entity,
update the doc comments to more accurately describe what tags are in reference
to the policy document.

Updates #cleanup

Change-Id: Iefff6f84981985f834bae7c6a6c34044f53f2ea2
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
4 months ago
M. J. Fromberger 6c8fef961e
ipn/ipnlocal: replace the LockedOnEntry pattern with conventional lock/unlock discipline (#16925)
There are several methods within the LocalBackend that used an unusual and
error-prone lock discipline whereby they require the caller to hold the backend
mutex on entry, but release it on the way out.

In #11650 we added some support code to make this pattern more visible.
Now it is time to eliminate the pattern (at least within this package).
This is intended to produce no semantic changes, though I am relying on
integration tests and careful inspection to achieve that.

To the extent possible I preserved the existing control flow. In a few places,
however, I replaced this with an unlock/lock closure. This means we will
sometimes reacquire a lock only to release it again one frame up the stack, but
these operations are not performance sensitive and the legibility gain seems
worthwhile.

We can probably also pull some of these out into separate methods, but I did
not do that here so as to avoid other variable scope changes that might be hard
to see. I would like to do some more cleanup separately.

As a follow-up, we could also remove the unlockOnce helper, but I did not do
that here either.

Updates #11649

Change-Id: I4c92d4536eca629cfcd6187528381c33f4d64e20
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
4 months ago
Jordan Whited 86a5292c03
ipn/localapi: make tailscale debug derp STUNOnly-aware (#16927)
Fixes #16926

Signed-off-by: Jordan Whited <jordan@tailscale.com>
4 months ago
M. J. Fromberger 55698c8511
ipn/localapi: plumb an event bus through the localapi.Handler (#16892)
Some of the operations of the local API need an event bus to correctly
instantiate other components (notably including the portmapper).

This commit adds that, and as the parameter list is starting to get a bit long
and hard to read, I took the opportunity to move the arguments to a config
type. Only a few call sites needed to be updated and this API is not intended
for general use, so I did not bother to stage the change.

Updates #15160
Updates #16842

Change-Id: I7b057d71161bd859f5acb96e2f878a34c85be0ef
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
4 months ago