We were incrementing the sftp metric on regular sessions
too, not just sftp.
Updates #cleanup
Change-Id: I63027a39cffb3e03397c6e4829b1620c10fa3130
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This was mostly already fixed already indirectly in earlier
commits but add a last second length check to this slice so
it can't ever OOB.
Fixes#7860
Change-Id: I31ac17fc93b5808deb09ff34e452fe37c87ddf3a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
io.Writer says you need to write completely on err=nil. (the result
int should be the same as the input buffer length)
We weren't doing that. We used to, but at some point the verbose
filtering was modifying buf before the final return of len(buf).
We've been getting lucky probably, that callers haven't looked at our
results and turned us into a short write error.
Updates #cleanup
Updates tailscale/corp#15664
Change-Id: I01e765ba35b86b759819e38e0072eceb9d10d75c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
In prep for making mapSession's lifetime not be 1:1 with a single HTTP
response's lifetime, this moves the inactivity timer watchdog out of
mapSession and into the caller that owns the streaming HTTP response.
(This is admittedly closer to how it was prior to the mapSession type
existing, but that was before we connected some dots which were
impossible to even see before the mapSession type broke the code up.)
Updates #7175
Change-Id: Ia108dac84a4953db41cbd30e73b1de4a2a676c11
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It was a really a mutable field owned by mapSession that we didn't move
in earlier commits.
Once moved, it's then possible to de-func-ify the code and turn it into
a regular method rather than an installed optional hook.
Noticed while working to move map session lifetimes out of
Direct.sendMapRequest's single-HTTP-connection scope.
Updates #7175
Updates #cleanup
Change-Id: I6446b15793953d88d1cabf94b5943bb3ccac3ad9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3d7fb6c21d dropped the explicit called to (*Client).connect when
its (*Client).WatchConnectionChanges got removed+refactored.
This puts it back, but in RunWatchConnectionLoop, before the call
to the (*Client).ServerPublicKey accessor, which is documented to
return the zero value (which is what broke us) on an unconnected
connection.
Plus some tests.
Fixestailscale/corp#15604
Change-Id: I0f242816f5ee4ad3bb0bf0400abc961dbe9f5fc8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Noticed both while re-reading this code.
Updates #cleanup
Change-Id: I3b70f1d5dc372853fa292ae1adbdee8cfc6a9a7b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
TestNewConn now passes as root on Linux. It wasn't closing the BPF
listeners and their goroutines.
The code is still a mess of two Close overlapping code paths, but that
can be refactored later. For now, make the two close paths more similar.
Updates #9945
Change-Id: I8a3cf5fb04d22ba29094243b8e645de293d9ed85
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It's required as of the recent 5297bd2cff.
Updates #7894
Updates #9394 (sure would be nice)
Change-Id: Id6672408dd8a6c82dba71022c8763e589d789fcd
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It seems to be implicated in a CPU consumption bug that's not yet
understood. Disable it until we understand.
Updates tailscale/corp#15261
Change-Id: Ia6d0c310da6464dda79a70fc3c18be0782812d3f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Move the compilation of everything to its own job too, separate
from test execution.
Updates #7894
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
They're slow. Make them their own job that can run in parallel.
Also, only run them in race mode. No need to run them on 386
or non-race amd64.
Updates #7894
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Regression from c15997511d. The callback could be run multiple times
from different endpoints.
Fixes#9801
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Misc cleanups and things noticed while working on #7894 and pulled out
of a separate change. Submitting them on their own to not distract
from later changes.
Updates #7894
Change-Id: Ie9abc8b88f121c559aeeb7e74db2aa532eb84d3d
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The whois handler was documented as taking IP (e.g. 100.101.102.103)
or IP:port (e.g. usermode 127.0.0.1:1234) but that got broken at some point
and we started requiring a port always. Fix that.
Also, found in the process of adding tests: fix the CapMap lookup in
userspace mode (it was always returning the caps of 127.0.0.1 in
userspace mode). Fix and test that too.
Updates #9714
Change-Id: Ie9a59744286522fa91c4b70ebe89a1e94dbded26
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Advertise it on Android (it looks like it already works once advertised).
And both advertise & likely fix it on iOS. Yet untested.
Updates #9672
Change-Id: If3b7e97f011dea61e7e75aff23dcc178b6cf9123
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This optionally uploads MAC address(es) to control, then adds a
c2n handler so control can ask a node to send a WoL packet.
Updates #306
RELNOTE=now supports waking up peer nodes on your LAN via Wake-on-LAN packets
Change-Id: Ibea1275fcd2048dc61d7059039abfbaf1ad4f465
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The Port.Pid was always more of an implementation detail on some
platforms and isn't necessary on Linux so it was never populated.
(Nothing outside the portlist package ever used it)
But might as well populate it for consistency since we have it in
memory and its absence confused people.
Updates #cleanup
Change-Id: I869768a75c9fedeff242a5452206e2b2947a17cb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Partially reverts 1bd3edbb46 (but keeps part of it)
iptables is almost always required but not strictly needed. Even if
you can technically run Tailscale without it (by manually configuring
nftables or userspace mode), we still now mark this as "Depends"
because our previous experiment in
https://github.com/tailscale/tailscale/issues/9236 of making it only
Recommends caused too many problems. Until our nftables table is more
mature, we'd rather err on the side of wasting a little disk by
including iptables for people who might not need it rather than
handle reports of it being missing.
Updates #9236
Change-Id: I86cc8aa3f78dafa0b4b729f55fb82eef6066be1c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Then use it in tailcfg which had it duplicated a couple times.
I think we have it a few other places too.
And use slices.Equal in wgengine/router too. (found while looking for callers)
Updates #cleanup
Change-Id: If5350eee9b3ef071882a3db29a305081e4cd9d23
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Prevent future problems like we earlier with go.mod replace directives
(e.g. removing our certstore replace in 6d6cf88d82 or wireguard-go
in ea5ee6f87c, both of which were reactions to problems caused by
go.mod replace in non-root modules, often because people are using tsnet
as a library from another module)
Updates #cleanup
Change-Id: I766715cfa7ce7021460ba4933bd2fa977c3081d2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Thanks to @qur and @eric for debugging!
Fixes#6973
Change-Id: Ib2cf8f030cf595cc73dd061c72e78ac19f5fae5d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
For loading testing & profiling the cost of full netmap updates.
Updates #1909
Change-Id: I0afdf5de9967f8d95c7f81d5b531ed1c92c3208f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This would've prevented #9470.
This used to pass, fails as of 9538e9f970, and passes again
once #9472 is in.
Updates #9470
Change-Id: Iab97666f7a318432fb3b6372a177ab50c55d4697
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
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>
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>
* 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>
It became required as of 6e967446e4
Updates #8052
Change-Id: I08d100534254865293c1beca5beff8e529e4e9ac
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It has one user (LocalBackend) which can ask magicsock itself.
Updates #cleanup
Change-Id: I8c03cbb1e5ba57b0b442621b5fa467030c14a2e2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
(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>
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>
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>
I missed this in 343c0f1031 and I guess we don't have integration
tests for wasm. But it compiled! :)
Updates #fixup to a #cleanup
Change-Id: If147b90bab254d144ec851a392e8db10ab97f98e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
This adds a new integration test with two nodes where the first gets a
incremental MapResponse (with only PeersRemoved set) saying that the
second node disappeared.
This extends the testcontrol package to support sending raw
MapResponses to nodes.
Updates #1909
Change-Id: Iea0c25c19cf0d72b52dba5a46d01b5cc87b9b39d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
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>
We're trying to start using that monster type less and eventually get
rid of it.
Updates #1909
Change-Id: I8e1e725bce5324fb820a9be6c7952767863e6542
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I guess we missed this one earlier when we unified the various
copies into set.HandleSet.
Updates #cleanup
Change-Id: I7e6de9ce16e8fc4846abf384dfcc8eaec4d99e60
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is both more efficient (because the knobs' bool is only updated
whenever Node is changed, rarely) and also gets us one step closer to
removing a case of storing a netmap.NetworkMap in
magicsock. (eventually we want to phase out much of the use of that
type internally)
Updates #1909
Change-Id: I37e81789f94133175064fdc09984e4f3a431f1a1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
In prep for incremental netmap update plumbing (#1909), make peerMap
also keyed by NodeID, as all the netmap node mutations passed around
later will be keyed by NodeID.
In the process, also:
* add envknob.InDevMode, as a signal that we can panic more aggressively
in unexpected cases.
* pull two moderately large blocks of code in Conn.SetNetworkMap out
into their own methods
* convert a few more sets from maps to set.Set
Updates #1909
Change-Id: I7acdd64452ba58e9d554140ee7a8760f9043f961
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
If the user's running "go test" by hand, no need to spam stderr with
the sentinel marker. It already calls t.Logf (which only gets output
on actual failure, or verbose mode) which is enough to tell users it's
known flaky. Stderr OTOH always prints out and is distracting to
manual "go test" users.
Updates #cleanup
Change-Id: Ie5e6881bae291787c30f75924fa132f4a28abbb2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It's been implicitly enabled (based on capver) for years.
Updates #cleanup
Change-Id: I8ff1ab844f9ed75c97e866e778dfc0b56cfa98a2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
I didn't clean up the more idiomatic map[T]bool with true values, at
least yet. I just converted the relatively awkward struct{}-valued
maps.
Updates #cleanup
Change-Id: I758abebd2bb1f64bc7a9d0f25c32298f4679c14f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We use it a number of places in different repos. Might as well make
one. Another use is coming.
Updates #cleanup
Change-Id: Ib7ce38de0db35af998171edee81ca875102349a4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I noticed that failed tests were leaving aroudn stray tailscaled processes
on macOS at least.
To repro, add this to tstest/integration:
func TestFailInFewSeconds(t *testing.T) {
t.Parallel()
time.Sleep(3 * time.Second)
os.Exit(1)
t.Fatal("boom")
}
Those three seconds let the other parallel tests (with all their
tailscaled child processes) start up and start running their tests,
but then we violently os.Exit(1) the test driver and all the children
were kept alive (and were spinning away, using all available CPU in
gvisor scheduler code, which is a separate scary issue)
Updates #cleanup
Change-Id: I9c891ed1a1ec639fb2afec2808c04dbb8a460e0e
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We want the overall state (used only for tests) to be computed from
the individual states of each component, rather than moving the state
around by hand in dozens of places.
In working towards that, we found a lot of things to clean up.
Updates #cleanup
Change-Id: Ieaaae5355dfae789a8ec7a56ce212f1d7e3a92db
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
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>
Don't just start goroutines and hope for them to be ordered.
Fixes potential regression from earlier 7074a40c0.
Updates #cleanup
Change-Id: I501a6f3e4e8e6306b958bccdc1e47869991c31f7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It was too aggressive before, as it only had the ill-defined "Major"
bool to work with. Now it can check more precisely.
Updates #9040
Change-Id: I20967283b64af6a9cad3f8e90cff406de91653b8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Drops time by several minutes.
Also, on top of that: skip building variant CLIs on the race builder
(29s), and getting qemu (15s).
Updates #9182
Change-Id: I979e02ab8c0daeebf5200459c9e4458a1f62f728
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Then use the Locked variants in Shutdown while we already hold the lock.
Updates #cleanup
Change-Id: I367d53e6be6f37f783c8f43fc9c4d498d0adf501
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
