Commit Graph

966 Commits (f1cddc6ecf4624b7608b1aeb06bd108c24687fef)

Author SHA1 Message Date
Brad Fitzpatrick f1cddc6ecf ipn{,/local},cmd/tailscale: add "sync" flag and pref to disable control map poll
For manual (human) testing, this lets the user disable control plane
map polls with "tailscale set --sync=false" (which survives restarts)
and "tailscale set --sync" to restore.

A high severity health warning is shown while this is active.

Updates #12639
Updates #17945

Change-Id: I83668fa5de3b5e5e25444df0815ec2a859153a6d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 weeks ago
Brad Fitzpatrick 99b06eac49 syncs: add Mutex/RWMutex alias/wrappers for future mutex debugging
Updates #17852

Change-Id: I477340fb8e40686870e981ade11cd61597c34a20
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 weeks ago
Brad Fitzpatrick 653d0738f9 types/netmap: remove PrivateKey from NetworkMap
It's an unnecessary nuisance having it. We go out of our way to redact
it in so many places when we don't even need it there anyway.

Updates #12639

Change-Id: I5fc72e19e9cf36caeb42cf80ba430873f67167c3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 weeks ago
James Tucker a96ef432cf control/controlclient,ipn/ipnlocal: replace State enum with boolean flags
Remove the State enum (StateNew, StateNotAuthenticated, etc.) from
controlclient and replace it with two explicit boolean fields:
- LoginFinished: indicates successful authentication
- Synced: indicates we've received at least one netmap

This makes the state more composable and easier to reason about, as
multiple conditions can be true independently rather than being
encoded in a single enum value.

The State enum was originally intended as the state machine for the
whole client, but that abstraction moved to ipn.Backend long ago.
This change continues moving away from the legacy state machine by
representing state as a combination of independent facts.

Also adds test helpers in ipnlocal that check independent, observable
facts (hasValidNetMap, needsLogin, etc.) rather than relying on
derived state enums, making tests more robust.

Updates #12639

Signed-off-by: James Tucker <james@tailscale.com>
3 weeks ago
James 'zofrex' Sanderson 124301fbb6
ipn/ipnlocal: log prefs changes and reason in Start (#17876)
Updates tailscale/corp#34238

Signed-off-by: James Sanderson <jsanderson@tailscale.com>
3 weeks ago
Brad Fitzpatrick 146ea42822 ipn/ipnlocal: remove all the weird locking (LockedOnEntry, UnlockEarly, etc)
Fixes #11649
Updates #16369

Co-authored-by: James Sanderson <jsanderson@tailscale.com>
Change-Id: I63eaa18fe870ddf81d84b949efac4d1b44c3db86
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 weeks ago
Jonathan Nobels e8d2f96449
ipn/ipnlocal, net/netns: add node cap to disable netns interface binding on netext Apple clients (#17691)
updates tailscale/corp#31571

It appears that on the latest macOS, iOS and tVOS versions, the work
that netns is doing to bind outgoing connections to the default interface (and all
of the trimmings and workarounds in netmon et al that make that work) are
not needed. The kernel is extension-aware and doing nothing, is the right
thing.  This is, however, not the case for tailscaled (which is not a
special process).

To allow us to test this assertion (and where it might break things), we add a
new node cap that turns this behaviour off only for network-extension equipped clients,
making it possible to turn this off tailnet-wide, without breaking any tailscaled
macos nodes.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
3 weeks ago
Brad Fitzpatrick 8ed6bb3198 ipn/ipnlocal: move vipServiceHash etc to serve.go, out of local.go
Updates #12614

Change-Id: I3c16b94fcb997088ff18d5a21355e0279845ed7e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 weeks ago
Brad Fitzpatrick e0e8731130 feature, ipn/ipnlocal: add, use feature.CanSystemdStatus for more DCE
When systemd notification support was omitted from the build, or on
non-Linux systems, we were unnecessarily emitting code and generating
garbage stringifying addresses upon transition to the Running state.

Updates #12614

Change-Id: If713f47351c7922bb70e9da85bf92725b25954b9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 weeks ago
Andrew Lytvynov ae3dff15e4
ipn/ipnlocal: clean up some of the weird locking (#17802)
* lock released early just to call `b.send` when it can call
  `b.sendToLocked` instead
* `UnlockEarly` called to release the lock before trivially fast
  operations, we can wait for a defer there

Updates #11649

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
3 weeks ago
Andrew Lytvynov db7dcd516f
Revert "control/controlclient: back out HW key attestation (#17664)" (#17732)
This reverts commit a760cbe33f.

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
1 month ago
M. J. Fromberger 06b092388e
ipn/ipnlocal: do not stall event processing for appc route updates (#17663)
A follow-up to #17411. Put AppConnector events into a task queue, as they may
take some time to process. Ensure that the queue is stopped at shutdown so that
cleanup will remain orderly.

Because events are delivered on a separate goroutine, slow processing of an
event does not cause an immediate problem; however, a subscriber that blocks
for a long time will push back on the bus as a whole. See
https://godoc.org/tailscale.com/util/eventbus#hdr-Expected_subscriber_behavior
for more discussion.

Updates #17192
Updates #15160

Change-Id: Ib313cc68aec273daf2b1ad79538266c81ef063e3
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
1 month ago
Patrick O'Doherty a760cbe33f
control/controlclient: back out HW key attestation (#17664)
Temporarily back out the TPM-based hw attestation code while we debug
Windows exceptions.

Updates tailscale/corp#31269

Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>
1 month ago
M. J. Fromberger 3dde233cd3
ipn/ipnlocal: use eventbus.SubscribeFunc in LocalBackend (#17524)
This does not change which subscriptions are made, it only swaps them to use
the SubscribeFunc API instead of Subscribe.

Updates #15160
Updates #17487

Change-Id: Id56027836c96942206200567a118f8bcf9c07f64
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
1 month ago
Patrick O'Doherty d8a6d0183c
ipn/ipnlocal: strip AttestationKey in redacted prefs view (#17527)
Updates tailscale/corp#31269

Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>
2 months ago
Patrick O'Doherty e45557afc0
types/persist: add AttestationKey (#17281)
Extend Persist with AttestationKey to record a hardware-backed
attestation key for the node's identity.

Add a flag to tailscaled to allow users to control the use of
hardware-backed keys to bind node identity to individual machines.

Updates tailscale/corp#31269


Change-Id: Idcf40d730a448d85f07f1bebf387f086d4c58be3

Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>
2 months ago
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
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 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 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
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 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
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
Nick Khyl 9781b7c25c ipn/ipnlocal: plumb logf into nodeBackend
Updates #cleanup

Signed-off-by: Nick Khyl <nickk@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 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 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
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 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
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