Commit Graph

523 Commits (0d991249e1330541ba8b25e221d0678a4b82db04)

Author SHA1 Message Date
Brad Fitzpatrick 0d991249e1 types/netmap: remove NetworkMap.{Addresses,MachineStatus}
And convert all callers over to the methods that check SelfNode.

Now we don't have multiple ways to express things in tests (setting
fields on SelfNode vs NetworkMap, sometimes inconsistently) and don't
have multiple ways to check those two fields (often only checking one
or the other).

Updates #9443

Change-Id: I2d7ba1cf6556142d219fae2be6f484f528756e3c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Maisem Ali a61caea911 tailcfg: define a type for NodeCapability
Instead of untyped string, add a type to identify these.

Updates #cleanup

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Brad Fitzpatrick 3d37328af6 wgengine, proxymap: split out port mapping from Engine to new type
(Continuing quest to remove rando stuff from the "Engine")

Updates #cleanup

Change-Id: I77f39902c2194410c10c054b545d70c9744250b0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick db2f37d7c6 ipn/ipnlocal: add some test accessors
Updates tailscale/corp#12990

Change-Id: I82801ac4c003d2c7e1352c514adb908dbf01be87
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 9538e9f970 ipn/ipnlocal: keep internal map updated of latest Nodes post mutations
We have some flaky integration tests elsewhere that have no one place
to ask about the state of the world. This makes LocalBackend be that
place (as it's basically there anyway) but doesn't yet add the ForTest
accessor method.

This adds a LocalBackend.peers map[NodeID]NodeView that is
incrementally updated as mutations arrive. And then we start moving
away from using NetMap.Peers at runtime (UpdateStatus no longer uses
it now). And remove another copy of NodeView in the LocalBackend
nodeByAddr map. Change that to point into b.peers instead.

Future changes will then start streaming whole-node-granularity peer
change updates to WatchIPNBus clients, tracking statefully per client
what each has seen. This will get the GUI clients from receiving less
of a JSON storm of updates all the time.

Updates #1909

Change-Id: I14a976ca9f493bdf02ba7e6e05217363dcf422e5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 926c990a09 types/netmap: start phasing out Addresses, add GetAddresses method
NetworkMap.Addresses is redundant with the SelfNode.Addresses. This
works towards a TODO to delete NetworkMap.Addresses and replace it
with a method.

This is similar to #9389.

Updates #cleanup

Change-Id: Id000509ca5d16bb636401763d41bdb5f38513ba0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick fb5ceb03e3 types/netmap: deprecate NetworkMap.MachineStatus, add accessor method
Step 1 of deleting it, per TODO.

Updates #cleanup

Change-Id: I1d3d0165ae5d8b20610227d60640997b73568733
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 0f3c279b86 ipn/ipnlocal: delete some unused code
Updates #cleanup

Change-Id: I90b46c476f135124d97288e776c2b428b351b8b8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 760b945bc0 ipn/{ipnlocal,ipnstate}: start simplifying UpdateStatus/StatusBuilder
* Remove unnecessary mutexes (there's no concurrency)
* Simplify LocalBackend.UpdateStatus using the StatusBuilder.WantPeers
  field that was added in 0f604923d3, removing passing around some
  method values into func args. And then merge two methods.

More remains, but this is a start.

Updates #9433

Change-Id: Iaf2d7ec6e4e590799f00bae185465a4fd089b822
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
James Tucker f6845b10f6 ipn/ipnlocal: plumb ExitNodeDNSResolvers for IsWireGuardOnly exit nodes
This enables installing default resolvers specified by
tailcfg.Node.ExitNodeDNSResolvers when the exit node is selected.

Updates #9377

Signed-off-by: James Tucker <james@tailscale.com>
9 months ago
Brad Fitzpatrick 99bb355791 wgengine: remove DiscoKey method from Engine interface
It has one user (LocalBackend) which can ask magicsock itself.

Updates #cleanup

Change-Id: I8c03cbb1e5ba57b0b442621b5fa467030c14a2e2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 3c276d7de2 wgengine: remove SetDERPMap method from Engine interface
(continuing the mission of removing rando methods from the Engine
 interface that we don't need anymore)

Updates #cleanup

Change-Id: Id5190917596bf04d7185c3b331a852724a3f5a16
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 67396d716b ipn/ipnlocal: remove defensiveness around not having a magicsock.Conn
We always have one. Stop pretending we might not.

Instead, add one early panic in NewLocalBackend if we actually don't.

Updates #cleanup

Change-Id: Iba4b78ed22cb6248e59c2b01a79355ca7a200ec8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 727b1432a8 wgengine: remove SetNetInfoCallback method from Engine
LocalBackend can talk to magicsock on its own to do this without
the "Engine" being involved.

(Continuing a little side quest of cleaning up the Engine
interface...)

Updates #cleanup

Change-Id: I8654acdca2b883b1bd557fdc0cfb90cd3a418a62
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 343c0f1031 wgengine{,/netstack}: remove AddNetworkMapCallback from Engine interface
It had exactly one user: netstack. Just have LocalBackend notify
netstack when here's a new netmap instead, simplifying the bloated
Engine interface that has grown a bunch of non-Engine-y things.
(plenty of rando stuff remains after this, but it's a start)

Updates #cleanup

Change-Id: I45e10ab48119e962fc4967a95167656e35b141d8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 3af051ea27 control/controlclient, types/netmap: start plumbing delta netmap updates
Currently only the top four most popular changes: endpoints, DERP
home, online, and LastSeen.

Updates #1909

Change-Id: I03152da176b2b95232b56acabfb55dcdfaa16b79
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 7c1ed38ab3 ipn/ipnlocal: fix missing controlknobs.Knobs plumbing
I missed connecting some controlknobs.Knobs pieces in 4e91cf20a8
resulting in that breaking control knobs entirely.

Whoops.

The fix in ipn/ipnlocal (where it makes a new controlclient) but to
atone, I also added integration tests. Those integration tests use
a new "tailscale debug control-knobs" which by itself might be useful
for future debugging.

Updates #9351

Change-Id: Id9c89c8637746d879d5da67b9ac4e0d2367a3f0d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 4e91cf20a8 control/controlknobs, all: add plumbed Knobs type, not global variables
Previously two tsnet nodes in the same process couldn't have disjoint
sets of controlknob settings from control as both would overwrite each
other's global variables.

This plumbs a new controlknobs.Knobs type around everywhere and hangs
the knobs sent by control on that instead.

Updates #9351

Change-Id: I75338646d36813ed971b4ffad6f9a8b41ec91560
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 9a86aa5732 all: depend on zstd unconditionally, remove plumbing to make it optional
All platforms use it at this point, including iOS which was the
original hold out for memory reasons. No more reason to make it
optional.

Updates #9332

Change-Id: I743fbc2f370921a852fbcebf4eb9821e2bdd3086
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Andrew Lytvynov d23b8ffb13
cmd/tailscale/cli,ipn: mention available update in "tailscale status" (#9205)
Cache the last `ClientVersion` value that was received from coordination
server and pass it in the localapi `/status` response.
When running `tailscale status`, print a message if `RunningAsLatest` is
`false`.

Updates #6907

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
9 months ago
Andrew Lytvynov 1073b56e18
ipn/ipnlocal: add logging and locking to c2n /update (#9290)
Log some progress info to make updates more debuggable. Also, track
whether an active update is already started and return an error if
a concurrent update is attempted.

Some planned future PRs:
* add JSON output to `tailscale update`
* use JSON output from `tailscale update` to provide a more detailed
  status of in-progress update (stage, download progress, etc)

Updates #6907

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
9 months ago
Marwan Sulaiman 50990f8931 ipn, ipn/ipnlocal: add Foreground field for ServeConfig
This PR adds a new field to the serve config that can be used to identify which serves are in "foreground mode" and then can also be used to ensure they do not get persisted to disk so that if Tailscaled gets ungracefully shutdown, the reloaded ServeConfig will not have those ports opened.

Updates #8489

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
9 months ago
Marwan Sulaiman 51d3220153 ipn, ipn/ipnlocal: remove log streaming for StreamServe
This PR removes the per request logging to the CLI as the CLI
will not be displaying those logs initially.

Updates #8489

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
9 months ago
Marwan Sulaiman 45eeef244e ipn, ipn/ipnlocal: add Foreground field to ServeConfig
This PR adds a new field to the ServeConfig which maps
WatchIPNBus session ids to foreground serve configs.

The PR also adds a DeleteForegroundSession method to ensure the config
gets cleaned up on sessions ending.

Note this field is not currently used but will be in follow up work.

Updates #8489

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
9 months ago
Maisem Ali cb3b281e98 ipn/ipnlocal: fix race in enterState
It would acquire the lock, calculate `nextState`, relase
the lock, then call `enterState` which would acquire the lock
again. There were obvious races there which could lead to
nil panics as seen in a test in a different repo.

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x70 pc=0x1050f2c7c]

goroutine 42240 [running]:
tailscale.com/ipn/ipnlocal.(*LocalBackend).enterStateLockedOnEntry(0x14002154e00, 0x6)
        tailscale.com/ipn/ipnlocal/local.go:3715 +0x30c
tailscale.com/ipn/ipnlocal.(*LocalBackend).enterState(0x14002154e00?, 0x14002e3a140?)
        tailscale.com/ipn/ipnlocal/local.go:3663 +0x8c
tailscale.com/ipn/ipnlocal.(*LocalBackend).stateMachine(0x14001f5e280?)
        tailscale.com/ipn/ipnlocal/local.go:3836 +0x2c
tailscale.com/ipn/ipnlocal.(*LocalBackend).setWgengineStatus(0x14002154e00, 0x14002e3a190, {0x0?, 0x0?})
        tailscale.com/ipn/ipnlocal/local.go:1193 +0x4d0
tailscale.com/wgengine.(*userspaceEngine).RequestStatus(0x14005d90300)
        tailscale.com/wgengine/userspace.go:1051 +0x80
tailscale.com/wgengine.NewUserspaceEngine.func2({0x14002e3a0a0, 0x2, 0x140025cce40?})
        tailscale.com/wgengine/userspace.go:318 +0x1a0
tailscale.com/wgengine/magicsock.(*Conn).updateEndpoints(0x14002154700, {0x105c13eaf, 0xf})
        tailscale.com/wgengine/magicsock/magicsock.go:531 +0x424
created by tailscale.com/wgengine/magicsock.(*Conn).ReSTUN in goroutine 42077
        tailscale.com/wgengine/magicsock/magicsock.go:2142 +0x3a4
```

Updates tailscale/corp#14480

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Marwan Sulaiman a4aa6507fa ipn, ipn/ipnlocal: add session identifier for WatchIPNBus
This PR adds a SessionID field to the ipn.Notify struct so that
ipn buses can identify a session and register deferred clean up
code in the future. The first use case this is for is to be able to
tie foreground serve configs to a specific watch session and ensure
its clean up when a connection is closed.

Updates #8489

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
9 months ago
Maisem Ali 52a19b5970 ipn/ipnlocal: prevent cc leaks on multiple Start calls
If Start was called multiple times concurrently, it would
create a new client and shutdown the previous one. However
there was a race possible between shutting down the old one
and assigning a new one where the concurent goroutine may
have assigned another one already and it would leak.

Updates tailscale/corp#14471

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Maisem Ali 6bc15f3a73 ipn/ipnlocal: fix startIsNoopLocked
It got broken back when FUS was introduced, but we
never caught it.

Updates tailscale/corp#14471

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Maisem Ali d06a75dcd0 ipn/ipnlocal: fix deadlock in resetControlClientLocked
resetControlClientLocked is called while b.mu was held and
would call cc.Shutdown which would wait for the observer queue
to drain.
However, there may be active callbacks from cc already waiting for
b.mu resulting in a deadlock.

This makes it so that resetControlClientLocked does not call
Shutdown, and instead just returns the value.
It also makes it so that any status received from previous cc
are ignored.

Updates tailscale/corp#12827

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Brad Fitzpatrick 0c1e3ff625 ipn/ipnlocal: avoid calling Start from resetForProfileChangeLockedOnEntry
During Shutdown of an ephemeral node, we called Logout (to best effort
delete the node earlier), which then called back into
resetForProfileChangeLockedOnEntry, which then tried to Start
again. That's all a waste of work during shutdown and complicates
other cleanups coming later.

Updates #cleanup

Change-Id: I0b8648cac492fc70fa97c4ebef919bbe352c5d7b
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick f7b7ccf835 control/controlclient, ipn/ipnlocal: unplumb a bool true literal opt
Updates #cleanup

Change-Id: I664f280a2e06b9875942458afcaf6be42a5e462a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Maisem Ali 96277b63ff ipn/ipnlocal: rename LogoutSync to Logout
Updates #cleanup

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Maisem Ali f52273767f ipn/ipnlocal: fix missing mutex usage for profileManager
It required holding b.mu but was documented incorrectly, fix.

Updates #cleanup

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Maisem Ali 959362a1f4 ipn/ipnlocal,control/controlclient: make Logout more sync
We already removed the async API, make it more sync and remove
the FinishLogout state too.

This also makes the callback be synchronous again as the previous
attempt was trying to work around the logout callback resulting
in a client shutdown getting blocked forever.

Updates #3833

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Maisem Ali 7074a40c06 control/controlclient: run SetControlClientStatus in goroutine
We have cases where the SetControlClientStatus would result in
a Shutdown call back into the auto client that would block
forever. The right thing to do here is to fix the LocalBackend
state machine but thats a different dumpster fire that we
are slowly making progress towards.

This makes it so that the SetControlClientStatus happens in a
different goroutine so that calls back into the auto client
do not block.

Also add a few missing mu.Unlocks in LocalBackend.Start.

Updates #9181

Signed-off-by: Maisem Ali <maisem@tailscale.com>
9 months ago
Brad Fitzpatrick 61ae16cb6f ipn/ipnlocal: add missing mutex unlock in error path
Found while debugging something else.

Updates #cleanup

Change-Id: I73fe55da14bcc3b1ffc39e2dbc0d077bc7f70cf1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 21247f766f ipn/ipnlocal: deflake some tests
* don't try to re-Start (and thus create a new client) during Shutdown
* in tests, wait for controlclient to fully shut down when replacing it
* log a bit more

Updates tailscale/corp#14139
Updates tailscale/corp#13175 etc
Updates #9178 and its flakes.

Change-Id: I3ed2440644dc157aa6e616fe36fbd29a6056846c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 04e1ce0034 control/controlclient: remove unused StartLogout
Updates #cleanup

Co-authored-by: Maisem Ali <maisem@tailscale.com>
Change-Id: I9d052fdbee787f1e8c872124e4bee61c7f04d142
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick f5bfdefa00 control/controlclient: de-pointer Status.PersistView, document more
Updates #cleanup
Updates #1909

Change-Id: I31d91e120e3b299508de2136021eab3b34131a44
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 7053e19562 control/controlclient: delete Status.Log{in,out}Finished
They were entirely redundant and 1:1 with the status field
so this turns them into methods instead.

Updates #cleanup
Updates #1909

Change-Id: I7d939750749edf7dae4c97566bbeb99f2f75adbc
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Val 8ba07aac85 ipn/iplocale: remove unused argument to shouldUseOneCGNATRoute
Remove an unused argument to shouldUseOneCGNATRoute.

Updates #cleanup

Signed-off-by: Val <valerie@tailscale.com>
9 months ago
Brad Fitzpatrick 55bb7314f2 control/controlclient: replace a status func with Observer interface
For now the method has only one interface (the same as the func it's
replacing) but it will grow, eventually with the goal to remove the
controlclient.Status type for most purposes.

Updates #1909

Change-Id: I715c8bf95e3f5943055a94e76af98d988558a2f2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick ea4425d8a9 ipn/ipnlocal, wgengine/magicsock: move UpdateStatus stuff around
Upcoming work on incremental netmap change handling will require some
replumbing of which subsystems get notified about what. Done naively,
it could break "tailscale status --json" visibility later. To make sure
I understood the flow of all the updates I was rereading the status code
and realized parts of ipnstate.Status were being populated by the wrong
subsystems.

The engine (wireguard) and magicsock (data plane, NAT traveral) should
only populate the stuff that they uniquely know. The WireGuard bits
were fine but magicsock was populating stuff stuff that LocalBackend
could've better handled, so move it there.

Updates #1909

Change-Id: I6d1b95d19a2d1b70fbb3c875fac8ea1e169e8cb0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 9089efea06 net/netmon: make ChangeFunc's signature take new ChangeDelta, not bool
Updates #9040

Change-Id: Ia43752064a1a6ecefc8802b58d6eaa0b71cf1f84
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 947def7688 types/netmap: remove redundant Netmap.Hostinfo
It was in SelfNode.Hostinfo anyway. The redundant copy was just
costing us an allocation per netmap (a Hostinfo.Clone).

Updates #1909

Change-Id: Ifac568aa5f8054d9419828489442a0f4559bc099
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Marwan Sulaiman 35ff5bf5a6 cmd/tailscale/cli, ipn/ipnlocal: [funnel] add stream mode
Adds ability to start Funnel in the foreground and stream incoming
connections. When foreground process is stopped, Funnel is turned
back off for the port.

Exampe usage:
```
TAILSCALE_FUNNEL_V2=on tailscale funnel 8080
```

Updates #8489

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
9 months ago
Brad Fitzpatrick 84b94b3146 types/netmap, all: make NetworkMap.SelfNode a tailcfg.NodeView
Updates #1909

Change-Id: I8c470cbc147129a652c1d58eac9b790691b87606
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
9 months ago
Brad Fitzpatrick 165f0116f1 types/netmap: move some mutations earlier, remove, document some fields
And optimize the Persist setting a bit, allocating later and only mutating
fields when there's been a Node change.

Updates #1909

Change-Id: Iaddfd9e88ef76e1d18e8d0a41926eb44d0955312
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
10 months ago
Brad Fitzpatrick 4511e7d64e ipn/ipnstate: add PeerStatus.AltSharerUserID, stop mangling Node.User
In b987b2ab18 (2021-01-12) when we introduced sharing we mapped
the sharer to the userid at a low layer, mostly to fix the display of
"tailscale status" and the client UIs, but also some tests.

The commit earlier today, 7dec09d169, removed the 2.5yo option
to let clients disable that automatic mapping, as clearly we were never
getting around to it.

This plumbs the Sharer UserID all the way to ipnstatus so the CLI
itself can choose to print out the Sharer's identity over the node's
original owner.

Then we stop mangling Node.User and let clients decide how they want
to render things.

To ease the migration for the Windows GUI (which currently operates on
tailcfg.Node via the NetMap from WatchIPNBus, instead of PeerStatus),
a new method Node.SharerOrUser is added to do the mapping of
Sharer-else-User.

Updates #1909
Updates tailscale/corp#1183

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
10 months ago
Brad Fitzpatrick 58a4fd43d8 types/netmap, all: use read-only tailcfg.NodeView in NetworkMap
Updates #8948

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
10 months ago