Commit graph

844 commits

Author SHA1 Message Date
Jason A. Donenfeld a816e8511e device: fix comment typo and shorten state.mu.Lock to state.Lock
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld 02138f1f81 device: fix typo in comment
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld d7bc7508e5 device: fix alignment on 32-bit machines and test for it
The test previously checked the offset within a substruct, not the
offset within the allocated struct, so this adds the two together.

It then fixes an alignment crash on 32-bit machines.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld d6e76fdbd6 device: do not log on idempotent device state change
Part of being actually idempotent is that we shouldn't penalize code
that takes advantage of this property with a log splat.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld 6ac1240821 device: do not attach finalizer to non-returned object
Before, the code attached a finalizer to an object that wasn't returned,
resulting in immediate garbage collection. Instead return the actual
pointer.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld 4b5d15ec2b device: lock elem in autodraining queue before freeing
Without this, we wind up freeing packets that the encryption/decryption
queues still have, resulting in a UaF.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld 6548a682a9 device: remove listen port race in tests
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 15:37:04 +01:00
Jason A. Donenfeld a60e6dab76 device: generate test keys on the fly
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-09 00:42:39 +01:00
Josh Bleecher Snyder d8dd1f254f device: remove mutex from Peer send/receive
The immediate motivation for this change is an observed deadlock.

1. A goroutine calls peer.Stop. That calls peer.queue.Lock().
2. Another goroutine is in RoutineSequentialReceiver.
   It receives an elem from peer.queue.inbound.
3. The peer.Stop goroutine calls close(peer.queue.inbound),
   close(peer.queue.outbound), and peer.stopping.Wait().
   It blocks waiting for RoutineSequentialReceiver
   and RoutineSequentialSender to exit.
4. The RoutineSequentialReceiver goroutine calls peer.SendStagedPackets().
   SendStagedPackets attempts peer.queue.RLock().
   That blocks forever because the peer.Stop
   goroutine holds a write lock on that mutex.

A background motivation for this change is that it can be expensive
to have a mutex in the hot code path of RoutineSequential*.

The mutex was necessary to avoid attempting to send elems on a closed channel.
This commit removes that danger by never closing the channel.
Instead, we send a sentinel nil value on the channel to indicate
to the receiver that it should exit.

The only problem with this is that if the receiver exits,
we could write an elem into the channel which would never get received.
If it never gets received, it cannot get returned to the device pools.

To work around this, we use a finalizer. When the channel can be GC'd,
the finalizer drains any remaining elements from the channel and
restores them to the device pool.

After that change, peer.queue.RWMutex no longer makes sense where it is.
It is only used to prevent concurrent calls to Start and Stop.
Move it to a more sensible location and make it a plain sync.Mutex.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 13:02:52 -08:00
Josh Bleecher Snyder 57aadfcb14 device: create channels.go
We have a bunch of stupid channel tricks, and I'm about to add more.
Give them their own file. This commit is 100% code movement.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 12:38:19 -08:00
Josh Bleecher Snyder af408eb940 device: print direction when ping transit fails
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 12:01:08 -08:00
Josh Bleecher Snyder 15810daa22 device: separate timersInit from timersStart
timersInit sets up the timers.
It need only be done once per peer.

timersStart does the work to prepare the timers
for a newly running peer. It needs to be done
every time a peer starts.

Separate the two and call them in the appropriate places.
This prevents data races on the peer's timers fields
when starting and stopping peers.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 10:32:07 -08:00
Josh Bleecher Snyder d840445e9b device: don't track device interface state in RoutineTUNEventReader
We already track this state elsewhere. No need to duplicate.
The cost of calling changeState is negligible.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 10:32:07 -08:00
Josh Bleecher Snyder 675ff32e6c device: improve MTU change handling
The old code silently accepted negative MTUs.
It also set MTUs above the maximum.
It also had hard to follow deeply nested conditionals.

Add more paranoid handling,
and make the code more straight-line.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 10:32:07 -08:00
Josh Bleecher Snyder 3516ccc1e2 device: remove device.state.stopping from RoutineTUNEventReader
The TUN event reader does three things: Change MTU, device up, and device down.
Changing the MTU after the device is closed does no harm.
Device up and device down don't make sense after the device is closed,
but we can check that condition before proceeding with changeState.
There's thus no reason to block device.Close on RoutineTUNEventReader exiting.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 10:32:07 -08:00
Josh Bleecher Snyder 0bcb822e5b device: overhaul device state management
This commit simplifies device state management.
It creates a single unified state variable and documents its semantics.

It also makes state changes more atomic.
As an example of the sort of bug that occurred due to non-atomic state changes,
the following sequence of events used to occur approximately every 2.5 million test runs:

* RoutineTUNEventReader received an EventDown event.
* It called device.Down, which called device.setUpDown.
* That set device.state.changing, but did not yet attempt to lock device.state.Mutex.
* Test completion called device.Close.
* device.Close locked device.state.Mutex.
* device.Close blocked on a call to device.state.stopping.Wait.
* device.setUpDown then attempted to lock device.state.Mutex and blocked.

Deadlock results. setUpDown cannot progress because device.state.Mutex is locked.
Until setUpDown returns, RoutineTUNEventReader cannot call device.state.stopping.Done.
Until device.state.stopping.Done gets called, device.state.stopping.Wait is blocked.
As long as device.state.stopping.Wait is blocked, device.state.Mutex cannot be unlocked.
This commit fixes that deadlock by holding device.state.mu
when checking that the device is not closed.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 10:32:07 -08:00
Josh Bleecher Snyder da95677203 device: remove unnecessary zeroing in peer.SendKeepalive
elem.packet is always already nil.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 10:14:17 -08:00
Josh Bleecher Snyder 9c75f58f3d device: remove device.state.stopping from RoutineHandshake
It is no longer necessary.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 08:18:32 -08:00
Josh Bleecher Snyder 84a42aed63 device: remove device.state.stopping from RoutineDecryption
It is no longer necessary, as of 454de6f3e64abd2a7bf9201579cd92eea5280996
(device: use channel close to shut down and drain decryption channel).

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-08 08:18:32 -08:00
Jason A. Donenfeld 4192036acd main: add back version file
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-04 15:33:04 +01:00
Jason A. Donenfeld 9c7bd73be2 tai64n: add string representation for error messages
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-03 17:56:46 +01:00
Jason A. Donenfeld 01e176af3c device: take peer handshake when reinitializing last sent handshake
This papers over other unrelated races, unfortunately.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-03 17:52:31 +01:00
Josh Bleecher Snyder 91617b4c52 device: fix goroutine leak test
The leak test had rare flakes.
If a system goroutine started at just the wrong moment, you'd get a false positive.
Instead of looping until the goroutines look good and then checking,
exit completely as soon as the number of goroutines looks good.
Also, check more frequently, in an attempt to complete faster.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-03 17:45:22 +01:00
Jason A. Donenfeld 7258a8973d device: add up/down stress test
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-03 17:43:41 +01:00
Jason A. Donenfeld d9d547a3f3 device: pass cfg strings around in tests instead of reader
This makes it easier to tag things onto the end manually for quick hacks.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-03 17:29:01 +01:00
Jason A. Donenfeld c3bde5f590 device: benchmark the waitpool to compare it to the prior channels
Here is the old implementation:

    type WaitPool struct {
        c chan interface{}
    }

    func NewWaitPool(max uint32, new func() interface{}) *WaitPool {
        p := &WaitPool{c: make(chan interface{}, max)}
        for i := uint32(0); i < max; i++ {
            p.c <- new()
        }
        return p
    }

    func (p *WaitPool) Get() interface{} {
        return <- p.c
    }

    func (p *WaitPool) Put(x interface{}) {
        p.c <- x
    }

It performs worse than the new one:

    name         old time/op  new time/op  delta
    WaitPool-16  16.4µs ± 5%  15.1µs ± 3%  -7.86%  (p=0.008 n=5+5)

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-03 16:59:29 +01:00
Josh Bleecher Snyder fd63a233c9 device: test that we do not leak goroutines
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-03 00:57:57 +01:00
Josh Bleecher Snyder 8a374a35a0 device: tie encryption queue lifetime to the peers that write to it
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-02-03 00:57:57 +01:00
Jason A. Donenfeld 4846070322 device: use a waiting sync.Pool instead of a channel
Channels are FIFO which means we have guaranteed cache misses.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-02 19:32:13 +01:00
Jason A. Donenfeld a9f80d8c58 device: reduce number of append calls when padding
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-29 20:10:48 +01:00
Jason A. Donenfeld de51129e33 device: use int64 instead of atomic.Value for time stamp
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-29 18:57:03 +01:00
Jason A. Donenfeld beb25cc4fd device: use new model queues for handshakes
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-29 18:24:45 +01:00
Jason A. Donenfeld 9263014ed3 device: simplify peer queue locking
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-29 16:21:53 +01:00
Jason A. Donenfeld f0f27d7fd2 device: reduce nesting when staging packet
Suggested-by: Josh Bleecher Snyder <josh@tailscale.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-28 18:56:58 +01:00
Jason A. Donenfeld d4112d9096 global: bump copyright
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-28 17:52:15 +01:00
Jason A. Donenfeld bf3bb88851 device: remove version string
This is what modules are for, and Go binaries can introspect.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-28 17:23:39 +01:00
Jason A. Donenfeld 6a128dde71 device: do not allow get to run while set runs
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-28 15:26:22 +01:00
Jason A. Donenfeld 34c047c762 device: avoid hex allocations in IpcGet
benchmark               old ns/op     new ns/op     delta
BenchmarkUAPIGet-16     2872          2157          -24.90%

benchmark               old allocs     new allocs     delta
BenchmarkUAPIGet-16     30             18             -40.00%

benchmark               old bytes     new bytes     delta
BenchmarkUAPIGet-16     737           256           -65.26%

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-28 15:22:34 +01:00
Jason A. Donenfeld d4725bc456 device: the psk is not a chapoly key
It's a separate type of key that gets hashed into the chain.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-28 14:45:53 +01:00
Jason A. Donenfeld 1b092ce584 device: get rid of nonce routine
This moves to a simple queue with no routine processing it, to reduce
scheduler pressure.

This splits latency in half!

benchmark                  old ns/op     new ns/op     delta
BenchmarkThroughput-16     2394          2364          -1.25%
BenchmarkLatency-16        259652        120810        -53.47%

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-27 18:38:27 +01:00
Jason A. Donenfeld a11dec5dc1 tun: use %w for errors on linux
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-27 16:02:42 +01:00
Jason A. Donenfeld ace50a0529 device: avoid deadlock when changing private key and removing self peers
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-27 15:53:21 +01:00
Jason A. Donenfeld 8cc99631d0 device: use linked list for per-peer allowed-ip traversal
This makes the IpcGet method much faster.

We also refactor the traversal API to use a callback so that we don't
need to allocate at all. Avoiding allocations we do self-masking on
insertion, which in turn means that split intermediate nodes require a
copy of the bits.

benchmark               old ns/op     new ns/op     delta
BenchmarkUAPIGet-16     3243          2659          -18.01%

benchmark               old allocs     new allocs     delta
BenchmarkUAPIGet-16     35             30             -14.29%

benchmark               old bytes     new bytes     delta
BenchmarkUAPIGet-16     1218          737           -39.49%

This benchmark is good, though it's only for a pair of peers, each with
only one allowedips. As this grows, the delta expands considerably.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-27 01:48:58 +01:00
Jason A. Donenfeld d669c78c43 device: combine debug and info log levels into 'verbose'
There are very few cases, if any, in which a user only wants one of
these levels, so combine it into a single level.

While we're at it, reduce indirection on the loggers by using an empty
function rather than a nil function pointer. It's not like we have
retpolines anyway, and we were always calling through a function with a
branch prior, so this seems like a net gain.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-26 23:05:48 +01:00
Josh Bleecher Snyder 7139279cd0 device: change logging interface to use functions
This commit overhauls wireguard-go's logging.

The primary, motivating change is to use a function instead
of a *log.Logger as the basic unit of logging.
Using functions provides a lot more flexibility for
people to bring their own logging system.

It also introduces logging helper methods on Device.
These reduce line noise at the call site.
They also allow for log functions to be nil;
when nil, instead of generating a log line and throwing it away,
we don't bother generating it at all.
This spares allocation and pointless work.

This is a breaking change, although the fix required
of clients is fairly straightforward.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-26 22:40:20 +01:00
Josh Bleecher Snyder 37efdcaccf device: fix shadowing of err in IpcHandle
The declaration of err in

	nextByte, err := buffered.ReadByte

shadows the declaration of err in

	op, err := buffered.ReadString('\n')

above. As a result, the assignments to err in

	err = ipcErrorf(ipc.IpcErrorInvalid, "trailing character in UAPI get: %c", nextByte)

and in

	err = device.IpcGetOperation(buffered.Writer)

do not modify the correct err variable.

Found by staticcheck.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-26 22:40:10 +01:00
Josh Bleecher Snyder d3a2b74df2 device: remove extra error arg
Caught by go vet.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-26 22:36:10 +01:00
Brad Fitzpatrick 8114c9db5f device: reduce allocs in Device.IpcGetOperation
Plenty more to go, but a start:

name       old time/op    new time/op    delta
UAPIGet-4    6.37µs ± 2%    5.56µs ± 1%  -12.70%  (p=0.000 n=8+8)

name       old alloc/op   new alloc/op   delta
UAPIGet-4    1.98kB ± 0%    1.22kB ± 0%  -38.71%  (p=0.000 n=10+10)

name       old allocs/op  new allocs/op  delta
UAPIGet-4      42.0 ± 0%      35.0 ± 0%  -16.67%  (p=0.000 n=10+10)

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-01-26 11:51:52 -08:00
Josh Bleecher Snyder e6ec3852a9 device: add benchmark for UAPI Device.IpcGetOperation
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-26 11:40:24 -08:00
Brad Fitzpatrick 23b2790aa0 conn: fix interface parameter name in Bind interface docs
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-01-26 15:20:22 +01:00