Commit Graph

705 Commits (6d3c10579e9c5b4f13606b9cb95c6fbd86371c4c)

Author SHA1 Message Date
Adrian Dewhurst cf9f507d47 ipn/ipnlocal: only build allowed suggested node list once
Rather than building a new suggested exit node set every time, compute
it once on first use. Currently, syspolicy ensures that values do not
change without a restart anyway.

Since the set is being constructed in a separate func now, the test code
that manipulates syspolicy can live there, and the TestSuggestExitNode
can now run in parallel with other tests because it does not have global
dependencies.

Updates tailscale/corp#19681

Change-Id: Ic4bb40ccc91b671f9e542bd5ba9c96f942081515
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>
6 months ago
Andrew Lytvynov 379e2bf189
ipn/ipnlocal: stop offline auto-updates on shutdown (#12342)
Clean up the updater goroutine on shutdown, in addition to doing that on
backend state change. This fixes a goroutine leak on shutdown in tests.

Updates #cleanup
6 months ago
Andrew Lytvynov bc4c8b65c7
ipn/ipnlocal: periodically run auto-updates when "offline" (#12118)
When the client is disconnected from control for any reason (typically
just turned off), we should still attempt to update if auto-updates are
enabled. This may help users who turn tailscale on infrequently for
accessing resources.

RELNOTE: Apply auto-updates even if the node is down or disconnected
from the coordination server.

Updates #12117

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
6 months ago
Adrian Dewhurst 3bf2bddbb5 ipn/ipnlocal: improve testability of random node selection
In order to test the sticky last suggestion code, a test was written for
LocalBackend.SuggestExitNode but it contains a random number generator
which makes writing comprehensive tests very difficult. This doesn't
change how the last suggestion works, but it adds some infrastructure to
make that easier in a later PR.

This adds func parameters for the two randomized parts: breaking ties
between DERP regions and breaking ties between nodes. This way tests can
validate the entire list of tied options, rather than expecting a
particular outcome given a particular random seed.

As a result of this, the global random number generator can be used
rather than seeding a local one each time.

In order to see the tied nodes for the location based (i.e. Mullvad)
case, pickWeighted needed to return a slice instead of a single
arbitrary option, so there is a small change in how that works.

Updates tailscale/corp#19681

Change-Id: I83c48a752abdec0f59c58ccfd8bfb3f3f17d0ea8
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>
6 months ago
Claire Wang f1d10c12ac
ipn/ipnlocal: allowed suggested exit nodes policy (#12240)
Updates tailscale/corp#19681

Signed-off-by: Claire Wang <claire@tailscale.com>
6 months ago
Andrew Lytvynov c9179bc261
various: disable stateful filtering by default (#12197)
After some analysis, stateful filtering is only necessary in tailnets
that use `autogroup:danger-all` in `src` in ACLs. And in those cases
users explicitly specify that hosts outside of the tailnet should be
able to reach their nodes. To fix local DNS breakage in containers, we
disable stateful filtering by default.

Updates #12108

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
6 months ago
Brad Fitzpatrick 964282d34f ipn,wgengine: remove vestigial Prefs.AllowSingleHosts
It was requested by the first customer 4-5 years ago and only used
for a brief moment of time. We later added netmap visibility trimming
which removes the need for this.

It's been hidden by the CLI for quite some time and never documented
anywhere else.

This keeps the CLI flag, though, out of caution. It just returns an
error if it's set to anything but true (its default).

Fixes #12058

Change-Id: I7514ba572e7b82519b04ed603ff9f3bdbaecfda7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
6 months ago
Brad Fitzpatrick 1384c24e41 control/controlclient: delete unused Client.Login Oauth2Token field
Updates #12172 (then need to update other repos)

Change-Id: I439f65e0119b09e00da2ef5c7a4f002f93558578
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
6 months ago
Brad Fitzpatrick 8aa5c3534d ipn/ipnlocal: simplify authURL vs authURLSticky, remove interact field
The previous LocalBackend & CLI 'up' changes improved some stuff, but
might've been too aggressive in some edge cases.

This simplifies the authURL vs authURLSticky distinction and removes
the interact field, which seemed to just just be about duplicate URL
suppression in IPN bus, back from when the IPN bus was a single client
at a time. This moves that suppression to a different spot.

Fixes #12119
Updates #12028
Updates #12042

Change-Id: I1f8800b1e82ccc1c8a0d7abba559e7404ddf41e4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Claire Wang e070af7414
ipnlocal, magicsock: add more description to storing last suggested exit (#11998)
node related functions
Updates tailscale/corp#19681

Signed-off-by: Claire Wang <claire@tailscale.com>
7 months ago
Anton Tolchanov 6f4a1dc6bf ipn/ipnlocal: fix another read of keyExpired outside mutex
Updates #12039

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
7 months ago
Brad Fitzpatrick e968b0ecd7 cmd/tailscale,controlclient,ipnlocal: fix 'up', deflake tests more
The CLI's "up" is kinda chaotic and LocalBackend.Start is kinda
chaotic and they both need to be redone/deleted (respectively), but
this fixes some buggy behavior meanwhile. We were previously calling
StartLoginInteractive (to start the controlclient's RegisterRequest)
redundantly in some cases, causing test flakes depending on timing and
up's weird state machine.

We only need to call StartLoginInteractive in the client if Start itself
doesn't. But Start doesn't tell us that. So cheat a bit and a put the
information about whether there's a current NodeKey in the ipn.Status.
It used to be accessible over LocalAPI via GetPrefs as a private key but
we removed that for security. But a bool is fine.

So then only call StartLoginInteractive if that bool is false and don't
do it in the WatchIPNBus loop.

Fixes #12028
Updates #12042

Change-Id: I0923c3f704a9d6afd825a858eb9a63ca7c1df294
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick e5ef35857f ipn/ipnlocal: fix read of keyExpired outside mutex
Fixes #12039

Change-Id: I28c8a282ce12619f17103e9535841f15394ce685
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Maisem Ali 32bc596062 ipn/ipnlocal: acquire b.mu once in Start
We used to Lock, Unlock, Lock, Unlock quite a few
times in Start resulting in all sorts of weird race
conditions. Simplify it all and only Lock/Unlock once.

Updates #11649

Signed-off-by: Maisem Ali <maisem@tailscale.com>
7 months ago
Maisem Ali 9380e2dfc6 ipn/ipnlocal: use lockAndGetUnlock in Start
This removes one of the Lock,Unlock,Lock,Unlock at least in
the Start function. Still has 3 more of these.

Updates #11649

Signed-off-by: Maisem Ali <maisem@tailscale.com>
7 months ago
Maisem Ali e1011f1387 ipn/ipnlocal: call SetNetInfoCallback from NewLocalBackend
Instead of calling it from Start everytime, call it from NewLocalBackend
once.

Updates #11649

Signed-off-by: Maisem Ali <maisem@tailscale.com>
7 months ago
Maisem Ali e67069550b ipn/ipnlocal,net/tstun,wgengine: create and plumb jailed packet filter
This plumbs a packet filter for jailed nodes through to the
tstun.Wrapper; the filter for a jailed node is equivalent to a "shields
up" filter. Currently a no-op as there is no way for control to
tell the client whether a peer is jailed.

Updates tailscale/corp#19623

Co-authored-by: Andrew Dunham <andrew@du.nham.ca>
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Change-Id: I5ccc5f00e197fde15dd567485b2a99d8254391ad
7 months ago
Andrew Lytvynov c28f5767bf
various: implement stateful firewalling on Linux (#12025)
Updates https://github.com/tailscale/corp/issues/19623


Change-Id: I7980e1fb736e234e66fa000d488066466c96ec85

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Co-authored-by: Andrew Dunham <andrew@du.nham.ca>
7 months ago
Brad Fitzpatrick aadb8d9d21 ipn/ipnlocal: don't send an empty BrowseToURL w/ WatchIPNBus NotifyInitialState
I noticed this while working on the following fix to #11962.

Updates #11962

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I4c5894d8899d1ae8c42f54ecfd4d05a4a7ac598c
7 months ago
Nick Khyl caa3d7594f ipn/ipnlocal, net/tsdial: plumb routes into tsdial and use them in UserDial
We'd like to use tsdial.Dialer.UserDial instead of SystemDial for DNS over TCP.
This is primarily necessary to properly dial internal DNS servers accessible
over Tailscale and subnet routes. However, to avoid issues when switching
between Wi-Fi and cellular, we need to ensure that we don't retain connections
to any external addresses on the old interface. Therefore, we need to determine
which dialer to use internally based on the configured routes.

This plumbs routes and localRoutes from router.Config to tsdial.Dialer,
and updates UserDial to use either the peer dialer or the system dialer,
depending on the network address and the configured routes.

Updates tailscale/corp#18725
Fixes #4529

Signed-off-by: Nick Khyl <nickk@tailscale.com>
7 months ago
Brad Fitzpatrick 7e0dd61e61 ipn/ipnlocal, tstest/integration: add panic to catch flaky test in the act
Updates #11962

Change-Id: Ifa24b82f9c76639bfd83278a7c2fe9cf42897bbb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick c3c18027c6 all: make more tests pass/skip in airplane mode
Updates tailscale/corp#19786

Change-Id: Iedc6730fe91c627b556bff5325bdbaf7bf79d8e6
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Maisem Ali fd6ba43b97 types/views: remove duplicate SliceContainsFunc
We already have `(Slice[T]).ContainsFunc`.

Updates #cleanup

Signed-off-by: Maisem Ali <maisem@tailscale.com>
7 months ago
Percy Wegmann 817badf9ca ipn/ipnlocal: reuse transport across Taildrive remotes
This prevents us from opening a new connection on each HTTP
request.

Updates #11967

Signed-off-by: Percy Wegmann <percy@tailscale.com>
7 months ago
Claire Wang 35872e86d2
ipnlocal, magicsock: store last suggested exit node id in local backend (#11959)
Updates tailscale/corp#19681

Signed-off-by: Claire Wang <claire@tailscale.com>
7 months ago
Andrew Dunham e9505e5432 ipn/ipnlocal: plumb health.Tracker into profileManager constructor
Setting the field after-the-fact wasn't working because we could migrate
prefs on creation, which would set health status for auto updates.

Updates #11986

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I41d79ebd61d64829a3a9e70586ce56f62d24ccfd
7 months ago
Brad Fitzpatrick 96712e10a7 health, ipn/ipnlocal: move more health warning code into health.Tracker
In prep for making health warnings rich objects with metadata rather
than a bunch of strings, start moving it all into the same place.

We'll still ultimately need the stringified form for the CLI and
LocalAPI for compatibility but we'll next convert all these warnings
into Warnables that have severity levels and such, and legacy
stringification will just be something each Warnable thing can do.

Updates #4136

Change-Id: I83e189435daae3664135ed53c98627c66e9e53da
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Andrew Dunham fe009c134e ipn/ipnlocal: reset the dialPlan only when the URL is unchanged
Also, reset it in a few more places (e.g. logout, new blank profiles,
etc.) to avoid a few more cases where a pre-existing dialPlan can cause
a new Headscale server take 10+ seconds to connect.

Updates #11938

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I3095173a5a3d9720507afe4452548491e9e45a3e
7 months ago
Shaw Drastin 1fe073098c
Reset dial plan when switching profile (#11933)
When switching profile, the server URL can change (e.g.
because of switching to a self-hosted headscale instance).

If it is not reset here, dial plans returned by old
server (e.g. tailscale control server) will be used to
connect to new server (e.g. self-hosted headscale server),
and the register request will be blocked by it until
timeout, leading to very slow profile switches.

Updates #11938 11938

Signed-off-by: Shaw Drastin <showier.drastic0a@icloud.com>
7 months ago
Fran Bull 6a0fbacc28 appc: setting AdvertiseRoutes explicitly discards app connector routes
This fixes bugs where after using the cli to set AdvertiseRoutes users
were finding that they had to restart tailscaled before the app
connector would advertise previously learned routes again. And seems
more in line with user expectations.

Fixes #11006
Signed-off-by: Fran Bull <fran@tailscale.com>
7 months ago
Fran Bull 1bd1b387b2 appc: add flag shouldStoreRoutes and controlknob for it
When an app connector is reconfigured and domains to route are removed,
we would like to no longer advertise routes that were discovered for
those domains. In order to do this we plan to store which routes were
discovered for which domains.

Add a controlknob so that we can enable/disable the new behavior.

Updates #11008
Signed-off-by: Fran Bull <fran@tailscale.com>
7 months ago
Fran Bull 79836e7bfd appc: add RouteInfo struct and persist it to StateStore
Lays the groundwork for the ability to persist app connectors discovered
routes, which will allow us to stop advertising routes for a domain if
the app connector no longer monitors that domain.

Updates #11008
Signed-off-by: Fran Bull <fran@tailscale.com>
7 months ago
Brad Fitzpatrick b9adbe2002 net/{interfaces,netmon}, all: merge net/interfaces package into net/netmon
In prep for most of the package funcs in net/interfaces to become
methods in a long-lived netmon.Monitor that can cache things.  (Many
of the funcs are very heavy to call regularly, whereas the long-lived
netmon.Monitor can subscribe to things from the OS and remember
answers to questions it's asked regularly later)

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: Ie4e8dedb70136af2d611b990b865a822cd1797e5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 6b95219e3a net/netmon, add: add netmon.State type alias of interfaces.State
... in prep for merging the net/interfaces package into net/netmon.

This is a no-op change that updates a bunch of the API signatures ahead of
a future change to actually move things (and remove the type alias)

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: I477613388f09389214db0d77ccf24a65bff2199c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 3672f29a4e net/netns, net/dns/resolver, etc: make netmon required in most places
The goal is to move more network state accessors to netmon.Monitor
where they can be cheaper/cached. But first (this change and others)
we need to make sure the one netmon.Monitor is plumbed everywhere.

Some notable bits:

* tsdial.NewDialer is added, taking a now-required netmon

* because a tsdial.Dialer always has a netmon, anything taking both
  a Dialer and a NetMon is now redundant; take only the Dialer and
  get the NetMon from that if/when needed.

* netmon.NewStatic is added, primarily for tests

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: I877f9cb87618c4eb037cee098241d18da9c01691
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Jonathan Nobels 71e9258ad9
ipn/ipnlocal: fix null dereference for early suggested exit node queries (#11885)
Fixes tailscale/corp#19558

A request for the suggested exit nodes that occurs too early in the
VPN lifecycle would result in a null deref of the netmap and/or
the netcheck report.  This checks both and errors out.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
7 months ago
Brad Fitzpatrick 745931415c health, all: remove health.Global, finish plumbing health.Tracker
Updates #11874
Updates #4136

Change-Id: I414470f71d90be9889d44c3afd53956d9f26cd61
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick a4a282cd49 control/controlclient: plumb health.Tracker
Updates #11874
Updates #4136

Change-Id: Ia941153bd83523f0c8b56852010f5231d774d91a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 6d69fc137f ipn/{ipnlocal,localapi},wgengine{,/magicsock}: plumb health.Tracker
Down to 25 health.Global users. After this remains controlclient &
net/dns & wgengine/router.

Updates #11874
Updates #4136

Change-Id: I6dd1856e3d9bf523bdd44b60fb3b8f7501d5dc0d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 723c775dbb tsd, ipnlocal, etc: add tsd.System.HealthTracker, start some plumbing
This adds a health.Tracker to tsd.System, accessible via
a new tsd.System.HealthTracker method.

In the future, that new method will return a tsd.System-specific
HealthTracker, so multiple tsnet.Servers in the same process are
isolated. For now, though, it just always returns the temporary
health.Global value. That permits incremental plumbing over a number
of changes. When the second to last health.Global reference is gone,
then the tsd.System.HealthTracker implementation can return a private
Tracker.

The primary plumbing this does is adding it to LocalBackend and its
dozen and change health calls. A few misc other callers are also
plumbed. Subsequent changes will flesh out other parts of the tree
(magicsock, controlclient, etc).

Updates #11874
Updates #4136

Change-Id: Id51e73cfc8a39110425b6dc19d18b3975eac75ce
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick 5b32264033 health: break Warnable into a global and per-Tracker value halves
Previously it was both metadata about the class of warnable item as
well as the value.

Now it's only metadata and the value is per-Tracker.

Updates #11874
Updates #4136

Change-Id: Ia1ed1b6c95d34bc5aae36cffdb04279e6ba77015
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Brad Fitzpatrick ebc552d2e0 health: add Tracker type, in prep for removing global variables
This moves most of the health package global variables to a new
`health.Tracker` type.

But then rather than plumbing the Tracker in tsd.System everywhere,
this only goes halfway and makes one new global Tracker
(`health.Global`) that all the existing callers now use.

A future change will eliminate that global.

Updates #11874
Updates #4136

Change-Id: I6ee27e0b2e35f68cb38fecdb3b2dc4c3f2e09d68
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Anton Tolchanov 31e6bdbc82 ipn/ipnlocal: always stop the engine on auth when key has expired
If seamless key renewal is enabled, we typically do not stop the engine
(deconfigure networking). However, if the node key has expired there is
no point in keeping the connection up, and it might actually prevent
key renewal if auth relies on endpoints routed via app connectors.

Fixes tailscale/corp#5800

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
7 months ago
Andrew Lytvynov b743b85dad
ipn/ipnlocal,ssh/tailssh: reject c2n /update if SSH conns are active (#11820)
Since we already track active SSH connections, it's not hard to
proactively reject updates until those finish. We attempt to do the same
on the control side, but the detection latency for new connections is in
the minutes, which is not fast enough for common short sessions.

Handle a `force=true` query parameter to override this behavior, so that
control can still trigger an update on a server where some long-running
abandoned SSH session is open.

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

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
7 months ago
Andrew Lytvynov bff527622d
ipn/ipnlocal,clientupdate: disallow auto-updates in containers (#11814)
Containers are typically immutable and should be updated as a whole (and
not individual packages within). Deny enablement of auto-updates in
containers.

Also, add the missing check in EditPrefs in LocalAPI, to catch cases
like tailnet default auto-updates getting enabled for nodes that don't
support it.

Updates #11544

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
7 months ago
Percy Wegmann d16c1293e9 ipn/ipnlocal: remove origin and referer headers from Taildrive requests
peerapi does not want these, but rclone includes them.
Removing them allows rclone to work with Taildrive configured
as a WebDAV remote.

Updates #cleanup

Signed-off-by: Percy Wegmann <percy@tailscale.com>
7 months ago
Percy Wegmann 94c0403104 ipn/ipnlocal: strip origin and referer headers from Taildrive requests
peerapi does not want these, but rclone includes them.
Stripping them out allows rclone to work with Taildrive configured
as a WebDAV remote.

Updates #cleanup

Signed-off-by: Percy Wegmann <percy@tailscale.com>
7 months ago
Andrew Lytvynov 22bd506129
ipn/ipnlocal: hold the mutex when in onTailnetDefaultAutoUpdate (#11786)
Turns out, profileManager is not safe for concurrent use and I missed
all the locking infrastructure in LocalBackend, oops.

I was not able to reproduce the race even with `go test -count 100`, but
this seems like an obvious fix.

Fixes #11773

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
7 months ago
Brad Fitzpatrick 7c1d6e35a5 all: use Go 1.22 range-over-int
Updates #11058

Change-Id: I35e7ef9b90e83cac04ca93fd964ad00ed5b48430
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Jonathan Nobels 7e2b4268d6
ipn/{localapi, ipnlocal}: forget the prior exit node when localAPI is used to zero the ExitNodeID (#11681)
Updates tailscale/corp#18724

When localAPI clients directly set ExitNodeID to "", the expected behaviour is that the prior exit node also gets zero'd - effectively setting the UI state back to 'no exit node was ever selected'

The IntenalExitNodePrior has been changed to be a non-opaque type, as it is read by the UI to render the users last selected exit node, and must be concrete. Future-us can either break this, or deprecate it and replace it with something more interesting.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
7 months ago