Commit Graph

371 Commits (2c94e3c4adef5730c6c3fb8a65240c61554cea19)

Author SHA1 Message Date
Brad Fitzpatrick 2c94e3c4ad wgengine/magicsock: don't unconditionally close DERP connections on rebind
Only if the source address isn't on the currently active interface or
a ping of the DERP server fails.

Updates #3619

Change-Id: I6bf06503cff4d781f518b437c8744ac29577acc8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick c7f5bc0f69 wgengine/magicsock: add metrics for sent disco messages
We only tracked the transport type (UDP vs DERP), not what they were.

Change-Id: Ia4430c1c53afd4634e2d9893d96751a885d77955
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 7b9c7bc42b ipn/ipnstate: remove old deprecated TailAddr IPv4-only field
It's been a bunch of releases now since the TailscaleIPs slice
replacement was added.

Change-Id: I3bd80e1466b3d9e4a4ac5bedba8b4d3d3e430a03
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
David Anderson 33c541ae30 ipn/ipnlocal: populate self status from netmap in ipnlocal, not magicsock.
Fixes #1933

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Josh Bleecher Snyder 758c37b83d net/netns: thread logf into control functions
So that darwin can log there without panicking during tests.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Brad Fitzpatrick 8ec44d0d5f wgengine/magicsock: remove some log spam
Fixes tailscale/corp#3070

Change-Id: Ie50031800ec8669e0596ad6d59d1e329a5c88516
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Josh Bleecher Snyder b3d6704aa3 wgengine/magicsock: fix data race on endpoint.discoKey
endpoint.discoKey is protected by endpoint.mu.
endpoint.sendDiscoMessage was reading it without holding the lock.
This showed up in a CI failure and is readily reproducible locally.

The fix is in two parts.

First, for Conn.enqueueCallMeMaybe, eliminate the one-line helper method endpoint.sendDiscoMessage; call Conn.sendDiscoMessage directly.
This makes it more natural to read endpoint.discoKey in a context
in which endpoint.mu is already held.

Second, for endpoint.sendDiscoPing, explicitly pass the disco key
as an argument. Again, this makes it easier to read endpoint.discoKey
in a context in which endpoint.mu is already held.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 773af7292b wgengine/magicsock: simplify peerMap.upsertEndpoint
We can do the "maybe delete" check unilaterally:
In the case of an insert, both oldDiscoKey
and ep.discoKey will be the zero value.

And since we don't use pi again, we can skip
giving it a name, which makes scoping clearer.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 9da22dac3d wgengine/magicsock: fix bug in peerMap.upsertEndpoint
Found by inspection by David Crawshaw while
investigating tailscale/corp#3016.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 16870cb754 wgengine/magicsock: fix typo in comment
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Brad Fitzpatrick 24ea365d48 netcheck, controlclient, magicsock: add more metrics
Updates #3307

Change-Id: Ibb33425764a75bde49230632f1b472f923551126
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 57b039c51d util/clientmetrics: add new package to add metrics to the client
And annotate magicsock as a start.

And add localapi and debug handlers with the Prometheus-format
exporter.

Updates #3307

Change-Id: I47c5d535fe54424741df143d052760387248f8d3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
David Anderson 0532eb30db all: replace tailcfg.DiscoKey with key.DiscoPublic.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 7e6a1ef4f1 tailcfg: use key.NodePublic in wire protocol types.
Updates #3206.

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 72ace0acba wgengine/magicsock: use key.NodePublic instead of tailcfg.NodeKey.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 84c3a09a8d types/key: export constants for key size, not a method.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 6422789ea0 disco: use key.NodePublic instead of tailcfg.NodeKey.
Updates #3206

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 418adae379 various: use NodePublic.AsNodeKey() instead of tailcfg.NodeKeyFromNodePublic()
Updates #3206

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson ef241f782e wgengine/magicsock: remove uses of tailcfg.DiscoKey.
Updates #3206

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 55b6753c11 wgengine/magicsock: remove use of key.{Public,Private}.
Updates #3206

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson c1d009b9e9 ipn/ipnstate: use key.NodePublic instead of the generic key.Public.
Updates #3206.

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 37c150aee1 derp: use new node key type.
Update #3206

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson c9bf773312 wgengine/magicsock: replace use of wgkey with new node key type.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Brad Fitzpatrick b0b0a80318 net/netcheck: implement netcheck for js/wasm clients
And the derper change to add a CORS endpoint for latency measurement.

And a little magicsock change to cut down some log spam on js/wasm.

Updates #3157

Change-Id: I5fd9e6f5098c815116ddc8ac90cbcd0602098a48
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick ed3fb197ad wgengine/magicsock: fix/disable a few misc things to get js/wasm working
Updates #3157

Change-Id: Ie9e3a772bb9878584080bb257b32150492e26eaf
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick e25afc6656 wgengine/magicsock: don't try to determine endpoints on js/wasm
Avoid netcheck, LocalAddr, etc.

Updates #3157

Change-Id: Ibc875c787c0e101b8076e64833f4fcc809372815
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 6cb2705833 wgengine/magicsock: don't run UDP listeners on js/wasm
Be DERP-only for now. (WebRTC can come later :))

Updates #3157

Change-Id: I56ebb3d914e37e8f4ab651306fd705b817ca381c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick c30fa5903d wgengine/magicsock: remove peerMap.byDiscoKey map
No longer used.

Updates #3088

Change-Id: I0ced3f87baa4053d3838d3c4a828ed0293923825
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
David Crawshaw 3552d86525 wgengine/magicsock: turn down timeouts in tests
Before:

	--- PASS: TestActiveDiscovery (11.78s)
	    --- PASS: TestActiveDiscovery/facing_easy_firewalls (5.89s)
	    --- PASS: TestActiveDiscovery/facing_nats (5.89s)
	    --- PASS: TestActiveDiscovery/simple_internet (0.89s)

After:

	--- PASS: TestActiveDiscovery (1.98s)
	    --- PASS: TestActiveDiscovery/facing_easy_firewalls (0.99s)
	    --- PASS: TestActiveDiscovery/facing_nats (0.99s)
	    --- PASS: TestActiveDiscovery/simple_internet (0.89s)

Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
3 years ago
David Anderson b956139b0c wgengine/magicsock: track IP<>node mappings without relying on discokeys.
Updates #3088.

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Brad Fitzpatrick 7a243ae5b1 wgengine/magicsock: finish TODO to speed up peerMap.forEachEndpointWithDiscoKey
Now that peerMap tracks the set of nodes for a DiscoKey.

Updates #3088

Change-Id: I927bf2bdfd2b8126475f6b6acc44bc799fcb489f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 11fdb14c53 wgengine/magicsock: don't check always-non-nil endpoint for nil-ness
Continuation of 2aa5df7ac1, remove nil
check because it can never be nil. (It previously was able to be nil.)

Change-Id: I59cd9ad611dbdcbfba680ed9b22e841b00c9d5e6
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
David Anderson e7eb46bced wgengine/magicsock: add an explicit else branch to peerMap update.
Clarifies that the replace+delete of peerinfo data is only when peerInfo
already exists.

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 2aa5df7ac1 wgengine/magicsock: document and enforce that peerInfo.ep is non-nil.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 521b44e653 wgengine/magicsock: move discoKey fields to the mutex-protected section.
Fixes #3106

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Brad Fitzpatrick a6d02dc122 wgengine/magicsock: track which NodeKey each DiscoKey was last for
This adds new fields (currently unused) to discoInfo to track what the
last verified (unambiguous) NodeKey a DiscoKey last mapped to, and
when.

Then on CallMeMaybe, Pong and on most Pings, we update the mapping
from DiscoKey to the current NodeKey for that DiscoKey.

Updates #3088

Change-Id: Idc4261972084dec71cf8ec7f9861fb9178eb0a4d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick c759fcc7d3 wgengine/magicsock: fix data race with sync.Pool in error+logging path
Fixes #3122

Change-Id: Ib52e84f9bd5813d6cf2e80ce5b2296912a48e064
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 75a7779b42 disco, wgengine/magicsock: send self node key in disco pings
This lets clients quickly (sub-millisecond within a local LAN) map
from an ambiguous disco key to a node key without waiting for a
CallMeMaybe (over relatively high latency DERP).

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Denton Gentry def650b3e8 wgengine/magicsock: don't Rebind after STUN error if closed.
https://github.com/tailscale/tailscale/pull/3014 added a
rebind on STUN failure, which means there can now be a
tailscale.com/wgengine/magicsock.(*RebindingUDPConn).ReadFromNetaddr
in progress at the end of the test waiting for a STUN
response which will never arrive.

This causes a test flake due to the resource leak in those
cases where the Conn decided to rebind. For whatever reason,
it mostly flakes with Windows.

If the Conn is closed, don't Rebind after a send error.

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
3 years ago
Brad Fitzpatrick f55c2bccf5 wgengine/magicsock: don't call setAddrToDiscoLocked on DERP ping
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 569f70abfd wgengine/magicsock: finish some renamings of discoEndpoint to endpoint
Renames only; continuation of earlier 8049063d35

These kept confusing me while working on #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 695df497ba wgengine/magicsock: delete peerMap.endpointForDiscoKey, remove remaining caller
The one remaining caller of peerMap.endpointForDiscoKey was making the
improper assumption that there's exactly 1 node with a given DiscoKey
in the network. That was the cause of #3088.

Now that all the other callers have been updated to not use
endpointForDiscoKey, there's no need to try to keep maintaining that
prone-to-misuse index.

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 04fd94acd6 wgengine/magicsock: remove endpointForDiscoKey call from handleDiscoMessage
A DiscoKey maps 1:n to endpoints. When we get a disco pong, we don't
necessarily know which endpoint sent it to us. Ask them all. There
will only usually be 1 (and in rare circumstances 2). So it's easier
to ask all two rather than building new maps from the random ping TxID
to its endpoint.

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 151b4415ca wgengine/magicsock: remove endpoint parameter from handlePingLocked
We can reply to a ping without knowing which exact node it's from.  As
long as it's in our netmap, it's safe to reply. If there's more than
one node with that discokey, it doesn't matter who we're relpying to.

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick d86081f353 wgengine/magicsock: add new discoInfo type for DiscoKey state, move some fields
As more prep for removing the false assumption that you're able to
map from DiscoKey to a single peer, move the lastPingFrom and lastPingTime
fields from the endpoint type to a new discoInfo type, effectively upgrading
the old sharedDiscoKey map (which only held a *[32]byte nacl precomputed key
as its value) to discoInfo which then includes that naclbox key.

Then start plumbing it into handlePing in prep for removing the need
for handlePing to take an endpoint parameter.

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick e5779f019e wgengine/magicsock: move temporary endpoint lookup later, add TODO to remove
Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 36a07089ee wgengine/magicsock: remove redundant/wrong sharedDiscoKey delete
The pass just after in this method handles cleaning up sharedDiscoKey.
No need to do it wrong (assuming DiscoKey => 1 node) earlier.

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 3e80806804 wgengine/magicsock: pass src NodeKey to handleDiscoMessage for DERP disco msgs
And then use it to avoid another lookup-by-DiscoKey.

Updates #3088
3 years ago
Brad Fitzpatrick 82fa15fa3b wgengine/magicsock: start removing endpointForDiscoKey
It's not valid to assume that a discokey is globally unique.

This removes the first two of the four callers.

Updates #3088

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Avery Pennarun 0d4a0bf60e magicsock: if STUN failed to send before, rebind before STUNning again.
On iOS (and possibly other platforms), sometimes our UDP socket would
get stuck in a state where it was bound to an invalid interface (or no
interface) after a network reconfiguration. We can detect this by
actually checking the error codes from sending our STUN packets.

If we completely fail to send any STUN packets, we know something is
very broken. So on the next STUN attempt, let's rebind the UDP socket
to try to correct any problems.

This fixes a problem where iOS would sometimes get stuck using DERP
instead of direct connections until the backend was restarted.

Fixes #2994

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
3 years ago