Previously, there was no server round trip required to log out, so when
you asked ipnlocal to Logout(), it could clear the netmap immediately
and switch to NeedsLogin state.
In v1.8, we added a true Logout operation. ipn.Logout() would trigger
an async cc.StartLogout() and *also* immediately switch to NeedsLogin.
Unfortunately, some frontends would see NeedsLogin and immediately
trigger a new StartInteractiveLogin() operation, before the
controlclient auth state machine actually acted on the Logout command,
thus accidentally invalidating the entire logout operation, retaining
the netmap, and violating the user's expectations.
Instead, add a new LogoutFinished signal from controlclient
(paralleling LoginFinished) and, upon starting a logout, don't update
the ipn state machine until it's received.
Updates: #1918 (BUG-2)
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Without this, macOS would fail to display its menu state correctly if you
started it while !WantRunning. It relies on the netmap in order to show
the logged-in username.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Track endpoints internally with a new tailcfg.Endpoint type that
includes a typed netaddr.IPPort (instead of just a string) and
includes a type for how that endpoint was discovered (STUN, local,
etc).
Use []tailcfg.Endpoint instead of []string internally.
At the last second, send it to the control server as the existing
[]string for endpoints, but also include a new parallel
MapRequest.EndpointType []tailcfg.EndpointType, so the control server
can start filtering out less-important endpoint changes from
new-enough clients. Notably, STUN-discovered endpoints can be filtered
out from 1.6+ clients, as they can discover them amongst each other
via CallMeMaybe disco exchanges started over DERP. And STUN endpoints
change a lot, causing a lot of MapResposne updates. But portmapped
endpoints are worth keeping for now, as they they work right away
without requiring the firewall traversal extra RTT dance.
End result will be less control->client bandwidth. (despite negligible
increase in client->control bandwidth)
Updates tailscale/corp#1543
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The direct client already logs it in JSON form. Then it's immediately
logged again in an unformatted dump, so this removes that unformatted
one.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This one alone doesn't modify the global dependency map much
(depaware.txt if anything looks slightly worse), but it leave
controlclient as only containing NetworkMap:
bradfitz@tsdev:~/src/tailscale.com/ipn$ grep -F "controlclient." *.go
backend.go: NetMap *controlclient.NetworkMap // new netmap received
fake_test.go: b.notify(Notify{NetMap: &controlclient.NetworkMap{}})
fake_test.go: b.notify(Notify{NetMap: &controlclient.NetworkMap{}})
handle.go: netmapCache *controlclient.NetworkMap
handle.go:func (h *Handle) NetMap() *controlclient.NetworkMap {
Once that goes into a leaf package, then ipn doesn't depend on
controlclient at all, and then the client gets smaller.
Updates #1278
Previously, any change to endpoints or hostinfo (or hostinfo's
netinfo) would result in the long-running map request HTTP stream
being torn down and restarted, losing all compression context along
with it.
This change makes us instead send a lite map request (OmitPeers: true,
Stream: false) that doesn't subscribe to anything, and then the
coordination server knows to not close other streams for that node
when it recives a lite request.
Fixestailscale/corp#797
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is a replacement for the key-related parts
of the wireguard-go wgcfg package.
This is almost a straight copy/paste from the wgcfg package.
I have slightly changed some of the exported functions and types
to avoid stutter, added and tweaked some comments,
and removed some now-unused code.
To avoid having wireguard-go depend on this new package,
wgcfg will keep its key types.
We translate into and out of those types at the last minute.
These few remaining uses will be eliminated alongside
the rest of the wgcfg package.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The previous code read too explicitly like log.Printf("I am here1"),
log.Printf("I am here2"). It still is with this change, but prettier, and
less subject to code rearranging order.
Also, bit of behavior change: on non-nil err but expired context,
don't reset the consecutive failure count. I don't think the old
behavior was intentional.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This code is currently racy due to an incorrect assumption
that goal is never modified in-place, so does not require extra locking.
This change makes the assumption correct.
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
If a test calls log.Printf, 'go test' horrifyingly rearranges the
output to no longer be in chronological order, which makes debugging
virtually impossible. Let's stop that from happening by making
log.Printf panic if called from any module, no matter how deep, during
tests.
This required us to change the default error handler in at least one
http.Server, as well as plumbing a bunch of logf functions around,
especially in magicsock and wgengine, but also in logtail and backoff.
To add insult to injury, 'go test' also rearranges the output when a
parent test has multiple sub-tests (all the sub-test's t.Logf is always
printed after all the parent tests t.Logf), so we need to screw around
with a special Logf that can point at the "current" t (current_t.Logf)
in some places. Probably our entire way of using subtests is wrong,
since 'go test' would probably like to run them all in parallel if you
called t.Parallel(), but it definitely can't because the're all
manipulating the shared state created by the parent test. They should
probably all be separate toplevel tests instead, with common
setup/teardown logic. But that's a job for another time.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
