Commit Graph

218 Commits (a275b9d7aa89af16b2a14ac9caeb8f2ead0dffdc)

Author SHA1 Message Date
Brad Fitzpatrick 9a346fd8b4 wgengine,magicsock: fix two lazy wireguard config issues
1) we weren't waking up a discoEndpoint that once existed and
   went idle for 5 minutes and then got a disco message again.

2) userspaceEngine.noteReceiveActivity had a buggy check; fixed
   and added a test
4 years ago
Brad Fitzpatrick 41c4560592 control/controlclient: remove unused NetworkMap.UAPI method
And remove last remaining use of wgcfg.ToUAPI in a test's debug
output; replace it with JSON.
4 years ago
Brad Fitzpatrick cff737786e wgengine/magicsock: fix lazy config deadlock, document more lock ordering
This removes the atomic bool that tried to track whether we needed to acquire
the lock on a future recursive call back into magicsock. Unfortunately that
hack doesn't work because we also had a lock ordering issue between magicsock
and userspaceEngine (see issue). This documents that too.

Fixes #644
4 years ago
Brad Fitzpatrick 2bd9ad4b40 wgengine: fix deadlock between engine and magicsock 4 years ago
Brad Fitzpatrick 7c38db0c97 wgengine/magicsock: don't deadlock on pre-disco Endpoints w/ lazy wireguard configs
Fixes tailscale/tailscale#637
4 years ago
Brad Fitzpatrick 4987a7d46c wgengine/magicsock: when hard NAT, add stun-ipv4:static-port as candidate
If a node is behind a hard NAT and is using an explicit local port
number, assume they might've mapped a port and add their public IPv4
address with the local tailscaled's port number as a candidate endpoint.
4 years ago
Brad Fitzpatrick bfcb0aa0be wgengine/magicsock: deflake tests, Close deadlock again
Better fix than 37903a9056

Fixes tailscale/corp#533
4 years ago
Dmytro Shynkevych 28e52a0492
all: dns refactor, add Proxied and PerDomain flags from control (#615)
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
4 years ago
Brad Fitzpatrick cb970539a6 wgengine/magicsock: remove TODO comment that's no longer applicable 4 years ago
Brad Fitzpatrick 915f65ddae wgengine/magicsock: stop disco activity on IPN stop
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
4 years ago
Brad Fitzpatrick c180abd7cf wgengine/magicsock: merge errClosed and errConnClosed 4 years ago
Brad Fitzpatrick d55fdd4669 wgengine/magicsock: update, flesh out a TODO 4 years ago
David Anderson f8e4c75f6b wgengine/magicsock: check slightly less aggressively for connectivity.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
Brad Fitzpatrick 58b721f374 wgengine/magicsock: deflake some tests with an ugly hack
Starting with fe68841dc7, some e2e tests
got flaky. Rather than debug them (they're gnarly), just revert to the old
behavior as far as those tests are concerned. The tests were somehow
using magicsock without a private key and expecting it to do ... something.

My goal with fe68841dc7 was to stop log spam
and unnecessary work I saw on the iOS app when when stopping the app.

Instead, only stop doing that work on any transition from
once-had-a-private-key to no-longer-have-a-private-key. That fixes
what I wanted to fix while still making the mysterious e2e tests
happy.
4 years ago
David Anderson 41d0c81859 wgengine/magicsock: make disco subtest name more precise.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 9beea8b314 wgengine/magicsock: remove unnecessary use of context.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson b62341d308 wgengine/magicsock: add docstring.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 9265296b33 wgengine/magicsock: don't deadlock on shutdown if sending blocks.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 0249236cc0 ipn/ipnstate: record assigned Tailscale IPs.
wgengine/magicsock: use ipnstate to find assigned Tailscale IPs.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson c3958898f1 tstest/natlab: be a bit more lenient during test shutdown.
There is a race in natlab where we might start shutdown while natlab is still running
a goroutine or two to deliver packets. This adds a small grace period to try and receive
it before continuing shutdown.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 7578c815be wgengine/magicsock: give pinger a more generous packet timeout.
The first packet to transit may take several seconds to do so, because
setup rates in wgengine may result in the initial WireGuard handshake
init to get dropped. So, we have to wait at least long enough for a
retransmit to correct the fault.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson c3994fd77c derp: remove OnlyDisco option.
Active discovery lets us introspect the state of the network stack precisely
enough that it's unnecessary, and dropping the initial DERP packets greatly
slows down tests. Additionally, it's unrealistic since our production network
will never deliver _only_ discovery packets, it'll be all or nothing.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 5455c64f1d wgengine/magicsock: add a test for two facing endpoint-independent NATs.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson f794493b4f wgengine/magicsock: explicitly check path discovery, add a firewall test.
The test proves that active discovery can traverse two facing firewalls.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson f582eeabd1 wgengine/magicsock: add a test for active path discovery.
Uses natlab only, because the point of this active discovery test is going to be
that it should get through a lot of obstacles.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
Brad Fitzpatrick 37903a9056 wgengine/magicsock: fix occasional deadlock on Conn.Close on c.derpStarted
The deadlock was:

* Conn.Close was called, which acquired c.mu
* Then this goroutine scheduled:

    if firstDerp {
        startGate = c.derpStarted
        go func() {
            dc.Connect(ctx)
            close(c.derpStarted)
        }()
    }

* The getRegion hook for that derphttp.Client then ran, which also
  tries to acquire c.mu.

This change makes that hook first see if we're already in a closing
state and then it can pretend that region doesn't exist.
4 years ago
Brad Fitzpatrick fe68841dc7 wgengine/magicsock: log better with less spam on transition to stopped state
Required a minor test update too, which now needs a private key to get far
enough to test the thing being tested.
4 years ago
Brad Fitzpatrick e298327ba8 wgengine/magicsock: remove overkill, slow reflect.DeepEqual of NetworkMap
No need to allocate or compare all the fields we don't care about.
4 years ago
David Anderson 3669296cef wgengine/magicsock: refactor twoDevicePing to make stack construction cleaner.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
Brad Fitzpatrick 16a9cfe2f4 wgengine: configure wireguard peers lazily, as needed
wireguard-go uses 3 goroutines per peer (with reasonably large stacks
& buffers).

Rather than tell wireguard-go about all our peers, only tell it about
peers we're actively communicating with. That means we need hooks into
magicsock's packet receiving path and tstun's packet sending path to
lazily create a wireguard peer on demand from the network map.

This frees up lots of memory for iOS (where we have almost nothing
left for larger domains with many users).

We should ideally do this in wireguard-go itself one day, but that'd
be a pretty big change.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
4 years ago
Brad Fitzpatrick 5066b824a6 wgengine/magicsock: don't log about disco ping timeouts if we have a working address
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
4 years ago
Brad Fitzpatrick c06d2a8513 wgengine/magicsock: fix typo in comment 4 years ago
Brad Fitzpatrick bf195cd3d8 wgengine/magicsock: reduce log verbosity of discovery messages
Don't log heartbeat pings & pongs. Track the reason for pings and then
only log the ping/pong traffic if it was for initial path discovery.
4 years ago
Brad Fitzpatrick a6559a8924 wgengine/magicsock: run test DERP in mode where only disco packets allowed
So we don't accidentally pass a NAT traversal test by having DERP pick up our slack
when we really just wanted DERP as an OOB messaging channel.
4 years ago
Brad Fitzpatrick 10ac066013 all: fix vet warnings 4 years ago
Brad Fitzpatrick d74c9aa95b wgengine/magicsock: update comment, fix earlier commit
891898525c had a continue that meant the didCopy synchronization never ran.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
4 years ago
Brad Fitzpatrick c976264bd1 wgengine/magicsock: gofmt 4 years ago
Dmytro Shynkevych f3e2b65637
wgengine/magicsock: time.Sleep -> time.After
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
4 years ago
Dmytro Shynkevych 380ee76d00
wgengine/magicsock: make time.Sleep in runDerpReader respect cancellation.
Before this patch, the 250ms sleep would not be interrupted by context cancellation,
which would result in the goroutine sometimes lingering in tests (100ms grace period).

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
4 years ago
Dmytro Shynkevych 891898525c
wgengine/magicsock: make receive from didCopy respect cancellation.
Very rarely, cancellation occurs between a successful send on derpRecvCh
and a call to copyBuf on the receiving side.
Without this patch, this situation results in <-copyBuf blocking indefinitely.

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
4 years ago
Brad Fitzpatrick a2267aae99 wgengine: only launch pingers for peers predating the discovery protocol
Peers advertising a discovery key know how to speak the discovery
protocol and do their own heartbeats to get through NATs and keep NATs
open. No need for the pinger except for with legacy peers.
4 years ago
David Anderson 45578b47f3 tstest/natlab: refactor PacketHandler into a larger interface.
The new interface lets implementors more precisely distinguish
local traffic from forwarded traffic, and applies different
forwarding logic within Machines for each type. This allows
Machines to be packet forwarders, which didn't quite work
with the implementation of Inject.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
Dmytro Shynkevych 2f15894a10 wgengine/magicsock: wait for derphttp client goroutine to exit
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
4 years ago
David Anderson 88e8456e9b wgengine/magicsock: add a connectivity test for facing firewalls.
The test demonstrates that magicsock can traverse two stateful
firewalls facing each other, that each require localhost to
initiate connections.

Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 1f7b1a4c6c wgengine/magicsock: rearrange TwoDevicePing test for future natlab tests.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
David Anderson 977381f9cc wgengine/magicsock: make trivial natlab test pass.
Signed-off-by: David Anderson <danderson@tailscale.com>
4 years ago
Brad Fitzpatrick 6c74065053 wgengine/magicsock, tstest/natlab: start hooking up natlab to magicsock
Also adds ephemeral port support to natlab.

Work in progress.

Pairing with @danderson.
4 years ago
Brad Fitzpatrick bd59bba8e6 wgengine/magicsock: stop discoEndpoint timers on Close
And add some defensive early returns on c.closed.
4 years ago
Brad Fitzpatrick de875a4d87 wgengine/magicsock: remove DisableSTUNForTesting 4 years ago
Brad Fitzpatrick 5c6d8e3053 netcheck, tailcfg, interfaces, magicsock: survey UPnP, NAT-PMP, PCP
Don't do anything with UPnP, NAT-PMP, PCP yet, but see how common they
are in the wild.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
4 years ago