]> git.ipfire.org Git - thirdparty/wireguard-go.git/log
thirdparty/wireguard-go.git
4 years agoversion: bump snapshot 0.0.20210212
Jason A. Donenfeld [Fri, 12 Feb 2021 17:00:59 +0000 (18:00 +0100)] 
version: bump snapshot

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: remove old version file
Jason A. Donenfeld [Fri, 12 Feb 2021 16:59:50 +0000 (17:59 +0100)] 
device: remove old version file

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agogitignore: remove old hacks
Jason A. Donenfeld [Thu, 11 Feb 2021 14:48:56 +0000 (15:48 +0100)] 
gitignore: remove old hacks

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: use container/list instead of open coding it
Jason A. Donenfeld [Wed, 10 Feb 2021 17:19:11 +0000 (18:19 +0100)] 
device: use container/list instead of open coding it

This linked list implementation is awful, but maybe Go 2 will help
eventually, and at least we're not open coding the hlist any more.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: retry Up() in up/down test
Jason A. Donenfeld [Wed, 10 Feb 2021 00:01:37 +0000 (01:01 +0100)] 
device: retry Up() in up/down test

We're loosing our ownership of the port when bringing the device down,
which means another test process could reclaim it. Avoid this by
retrying for 4 seconds.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agoconn: close old fd before trying again
Jason A. Donenfeld [Tue, 9 Feb 2021 23:43:31 +0000 (00:43 +0100)] 
conn: close old fd before trying again

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: flush peer queues before starting device
Jason A. Donenfeld [Tue, 9 Feb 2021 23:39:28 +0000 (00:39 +0100)] 
device: flush peer queues before starting device

In case some old packets snuck in there before, this flushes before
starting afresh.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: create peer queues at peer creation time
Jason A. Donenfeld [Tue, 9 Feb 2021 23:21:12 +0000 (00:21 +0100)] 
device: create peer queues at peer creation time

Rather than racing with Start(), since we're never destroying these
queues, we just set the variables at creation time.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: return error from Up() and Down()
Jason A. Donenfeld [Tue, 9 Feb 2021 23:12:23 +0000 (00:12 +0100)] 
device: return error from Up() and Down()

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agorwcancel: add an explicit close call
Jason A. Donenfeld [Tue, 9 Feb 2021 19:18:21 +0000 (20:18 +0100)] 
rwcancel: add an explicit close call

This lets us collect FDs even if the GC doesn't do it for us.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agorwcancel: use errors.Is for unwrapping
Jason A. Donenfeld [Tue, 9 Feb 2021 18:54:00 +0000 (19:54 +0100)] 
rwcancel: use errors.Is for unwrapping

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agotun: use errors.Is for unwrapping
Jason A. Donenfeld [Tue, 9 Feb 2021 18:48:27 +0000 (19:48 +0100)] 
tun: use errors.Is for unwrapping

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agoconn: use errors.Is for unwrapping
Jason A. Donenfeld [Tue, 9 Feb 2021 18:46:57 +0000 (19:46 +0100)] 
conn: use errors.Is for unwrapping

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: handshake routine writes into encryption queue
Jason A. Donenfeld [Tue, 9 Feb 2021 18:26:45 +0000 (19:26 +0100)] 
device: handshake routine writes into encryption queue

Since RoutineHandshake calls peer.SendKeepalive(), it potentially is a
writer into the encryption queue, so we need to bump the wg count.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: make RoutineReadFromTUN keep encryption queue alive
Josh Bleecher Snyder [Tue, 9 Feb 2021 17:53:00 +0000 (09:53 -0800)] 
device: make RoutineReadFromTUN keep encryption queue alive

RoutineReadFromTUN can trigger a call to SendStagedPackets.
SendStagedPackets attempts to protect against sending
on the encryption queue by checking peer.isRunning and device.isClosed.
However, those are subject to TOCTOU bugs.

If that happens, we get this:

goroutine 1254 [running]:
golang.zx2c4.com/wireguard/device.(*Peer).SendStagedPackets(0xc000798300)
        .../wireguard-go/device/send.go:321 +0x125
golang.zx2c4.com/wireguard/device.(*Device).RoutineReadFromTUN(0xc000014780)
        .../wireguard-go/device/send.go:271 +0x21c
created by golang.zx2c4.com/wireguard/device.NewDevice
        .../wireguard-go/device/device.go:315 +0x298

Fix this with a simple, big hammer: Keep the encryption queue
alive as long as it might be written to.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agoconn: try harder to have v4 and v6 ports agree
Jason A. Donenfeld [Tue, 9 Feb 2021 17:45:12 +0000 (18:45 +0100)] 
conn: try harder to have v4 and v6 ports agree

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: only allocate peer queues once
Josh Bleecher Snyder [Tue, 9 Feb 2021 17:08:17 +0000 (09:08 -0800)] 
device: only allocate peer queues once

This serves two purposes.

First, it makes repeatedly stopping then starting a peer cheaper.
Second, it prevents a data race observed accessing the queues.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: clarify device.state.state docs (again)
Josh Bleecher Snyder [Tue, 9 Feb 2021 16:27:48 +0000 (08:27 -0800)] 
device: clarify device.state.state docs (again)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: run fewer iterations in TestUpDown
Josh Bleecher Snyder [Tue, 9 Feb 2021 16:20:11 +0000 (08:20 -0800)] 
device: run fewer iterations in TestUpDown

The high iteration count was useful when TestUpDown
was the nexus of new bugs to investigate.

Now that it has stabilized, that's less valuable.
And it slows down running the tests and crowds out other tests.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: run fewer trials in TestWaitPool when race detector enabled
Josh Bleecher Snyder [Tue, 9 Feb 2021 16:18:47 +0000 (08:18 -0800)] 
device: run fewer trials in TestWaitPool when race detector enabled

On a many-core machine with the race detector enabled,
this test can take several minutes to complete.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove nil elem check in finalizers
Josh Bleecher Snyder [Tue, 9 Feb 2021 16:15:21 +0000 (08:15 -0800)] 
device: remove nil elem check in finalizers

This is not necessary, and removing it speeds up detection of UAF bugs.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: rename unsafeRemovePeer to removePeerLocked
Jason A. Donenfeld [Tue, 9 Feb 2021 15:11:33 +0000 (16:11 +0100)] 
device: rename unsafeRemovePeer to removePeerLocked

This matches the new naming scheme of upLocked and downLocked.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: remove deviceStateNew
Jason A. Donenfeld [Tue, 9 Feb 2021 14:39:19 +0000 (15:39 +0100)] 
device: remove deviceStateNew

It's never used and we won't have a use for it. Also, move to go-running
stringer, for those without GOPATHs.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: fix comment typo and shorten state.mu.Lock to state.Lock
Jason A. Donenfeld [Tue, 9 Feb 2021 14:35:43 +0000 (15:35 +0100)] 
device: fix comment typo and shorten state.mu.Lock to state.Lock

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: fix typo in comment
Jason A. Donenfeld [Tue, 9 Feb 2021 14:32:55 +0000 (15:32 +0100)] 
device: fix typo in comment

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: fix alignment on 32-bit machines and test for it
Jason A. Donenfeld [Tue, 9 Feb 2021 14:30:32 +0000 (15:30 +0100)] 
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>
4 years agodevice: do not log on idempotent device state change
Jason A. Donenfeld [Tue, 9 Feb 2021 14:25:43 +0000 (15:25 +0100)] 
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>
4 years agodevice: do not attach finalizer to non-returned object
Jason A. Donenfeld [Tue, 9 Feb 2021 14:09:50 +0000 (15:09 +0100)] 
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>
4 years agodevice: lock elem in autodraining queue before freeing
Jason A. Donenfeld [Tue, 9 Feb 2021 14:00:59 +0000 (15:00 +0100)] 
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>
4 years agodevice: remove listen port race in tests
Jason A. Donenfeld [Mon, 8 Feb 2021 23:59:39 +0000 (00:59 +0100)] 
device: remove listen port race in tests

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: generate test keys on the fly
Jason A. Donenfeld [Mon, 8 Feb 2021 23:33:18 +0000 (00:33 +0100)] 
device: generate test keys on the fly

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: remove mutex from Peer send/receive
Josh Bleecher Snyder [Mon, 8 Feb 2021 21:02:52 +0000 (13:02 -0800)] 
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>
4 years agodevice: create channels.go
Josh Bleecher Snyder [Mon, 8 Feb 2021 20:38:19 +0000 (12:38 -0800)] 
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>
4 years agodevice: print direction when ping transit fails
Josh Bleecher Snyder [Mon, 8 Feb 2021 19:36:55 +0000 (11:36 -0800)] 
device: print direction when ping transit fails

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: separate timersInit from timersStart
Josh Bleecher Snyder [Mon, 8 Feb 2021 18:01:35 +0000 (10:01 -0800)] 
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>
4 years agodevice: don't track device interface state in RoutineTUNEventReader
Josh Bleecher Snyder [Thu, 21 Jan 2021 17:26:14 +0000 (09:26 -0800)] 
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>
4 years agodevice: improve MTU change handling
Josh Bleecher Snyder [Thu, 21 Jan 2021 17:23:45 +0000 (09:23 -0800)] 
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>
4 years agodevice: remove device.state.stopping from RoutineTUNEventReader
Josh Bleecher Snyder [Mon, 8 Feb 2021 18:19:28 +0000 (10:19 -0800)] 
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>
4 years agodevice: overhaul device state management
Josh Bleecher Snyder [Tue, 19 Jan 2021 17:02:16 +0000 (09:02 -0800)] 
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>
4 years agodevice: remove unnecessary zeroing in peer.SendKeepalive
Josh Bleecher Snyder [Mon, 8 Feb 2021 17:21:31 +0000 (09:21 -0800)] 
device: remove unnecessary zeroing in peer.SendKeepalive

elem.packet is always already nil.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove device.state.stopping from RoutineHandshake
Josh Bleecher Snyder [Wed, 3 Feb 2021 00:14:54 +0000 (16:14 -0800)] 
device: remove device.state.stopping from RoutineHandshake

It is no longer necessary.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove device.state.stopping from RoutineDecryption
Josh Bleecher Snyder [Tue, 19 Jan 2021 19:10:05 +0000 (11:10 -0800)] 
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>
4 years agomain: add back version file
Jason A. Donenfeld [Thu, 4 Feb 2021 14:33:04 +0000 (15:33 +0100)] 
main: add back version file

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agotai64n: add string representation for error messages
Jason A. Donenfeld [Wed, 3 Feb 2021 16:56:46 +0000 (17:56 +0100)] 
tai64n: add string representation for error messages

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: take peer handshake when reinitializing last sent handshake
Jason A. Donenfeld [Wed, 3 Feb 2021 16:52:31 +0000 (17:52 +0100)] 
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>
4 years agodevice: fix goroutine leak test
Josh Bleecher Snyder [Wed, 3 Feb 2021 16:26:27 +0000 (08:26 -0800)] 
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>
4 years agodevice: add up/down stress test
Jason A. Donenfeld [Wed, 3 Feb 2021 16:43:41 +0000 (17:43 +0100)] 
device: add up/down stress test

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: pass cfg strings around in tests instead of reader
Jason A. Donenfeld [Wed, 3 Feb 2021 16:29:01 +0000 (17:29 +0100)] 
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>
4 years agodevice: benchmark the waitpool to compare it to the prior channels
Jason A. Donenfeld [Wed, 3 Feb 2021 15:54:45 +0000 (16:54 +0100)] 
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>
4 years agodevice: test that we do not leak goroutines
Josh Bleecher Snyder [Tue, 2 Feb 2021 18:41:20 +0000 (10:41 -0800)] 
device: test that we do not leak goroutines

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: tie encryption queue lifetime to the peers that write to it
Josh Bleecher Snyder [Tue, 2 Feb 2021 18:46:34 +0000 (10:46 -0800)] 
device: tie encryption queue lifetime to the peers that write to it

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: use a waiting sync.Pool instead of a channel
Jason A. Donenfeld [Tue, 2 Feb 2021 17:37:49 +0000 (18:37 +0100)] 
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>
4 years agodevice: reduce number of append calls when padding
Jason A. Donenfeld [Fri, 29 Jan 2021 19:10:48 +0000 (20:10 +0100)] 
device: reduce number of append calls when padding

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: use int64 instead of atomic.Value for time stamp
Jason A. Donenfeld [Fri, 29 Jan 2021 17:54:19 +0000 (18:54 +0100)] 
device: use int64 instead of atomic.Value for time stamp

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: use new model queues for handshakes
Jason A. Donenfeld [Fri, 29 Jan 2021 17:24:45 +0000 (18:24 +0100)] 
device: use new model queues for handshakes

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: simplify peer queue locking
Jason A. Donenfeld [Fri, 29 Jan 2021 13:54:11 +0000 (14:54 +0100)] 
device: simplify peer queue locking

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: reduce nesting when staging packet
Jason A. Donenfeld [Thu, 28 Jan 2021 17:56:58 +0000 (18:56 +0100)] 
device: reduce nesting when staging packet

Suggested-by: Josh Bleecher Snyder <josh@tailscale.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agoglobal: bump copyright
Jason A. Donenfeld [Thu, 28 Jan 2021 16:52:15 +0000 (17:52 +0100)] 
global: bump copyright

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: remove version string
Jason A. Donenfeld [Thu, 28 Jan 2021 16:23:39 +0000 (17:23 +0100)] 
device: remove version string

This is what modules are for, and Go binaries can introspect.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: do not allow get to run while set runs
Jason A. Donenfeld [Thu, 28 Jan 2021 14:26:22 +0000 (15:26 +0100)] 
device: do not allow get to run while set runs

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: avoid hex allocations in IpcGet
Jason A. Donenfeld [Wed, 27 Jan 2021 23:49:31 +0000 (00:49 +0100)] 
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>
4 years agodevice: the psk is not a chapoly key
Jason A. Donenfeld [Thu, 28 Jan 2021 13:44:51 +0000 (14:44 +0100)] 
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>
4 years agodevice: get rid of nonce routine
Jason A. Donenfeld [Wed, 27 Jan 2021 17:13:53 +0000 (18:13 +0100)] 
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>
4 years agotun: use %w for errors on linux
Jason A. Donenfeld [Wed, 27 Jan 2021 14:56:49 +0000 (15:56 +0100)] 
tun: use %w for errors on linux

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: avoid deadlock when changing private key and removing self peers
Jason A. Donenfeld [Wed, 27 Jan 2021 14:53:21 +0000 (15:53 +0100)] 
device: avoid deadlock when changing private key and removing self peers

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: use linked list for per-peer allowed-ip traversal
Jason A. Donenfeld [Tue, 26 Jan 2021 22:44:37 +0000 (23:44 +0100)] 
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>
4 years agodevice: combine debug and info log levels into 'verbose'
Jason A. Donenfeld [Tue, 26 Jan 2021 22:05:48 +0000 (23:05 +0100)] 
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>
4 years agodevice: change logging interface to use functions
Josh Bleecher Snyder [Fri, 22 Jan 2021 22:11:17 +0000 (14:11 -0800)] 
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>
4 years agodevice: fix shadowing of err in IpcHandle
Josh Bleecher Snyder [Tue, 26 Jan 2021 18:35:25 +0000 (10:35 -0800)] 
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>
4 years agodevice: remove extra error arg
Josh Bleecher Snyder [Tue, 26 Jan 2021 16:40:15 +0000 (08:40 -0800)] 
device: remove extra error arg

Caught by go vet.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: reduce allocs in Device.IpcGetOperation
Brad Fitzpatrick [Tue, 26 Jan 2021 19:11:58 +0000 (11:11 -0800)] 
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>
4 years agodevice: add benchmark for UAPI Device.IpcGetOperation
Josh Bleecher Snyder [Tue, 26 Jan 2021 19:39:48 +0000 (11:39 -0800)] 
device: add benchmark for UAPI Device.IpcGetOperation

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agoconn: fix interface parameter name in Bind interface docs
Brad Fitzpatrick [Tue, 26 Jan 2021 02:52:42 +0000 (18:52 -0800)] 
conn: fix interface parameter name in Bind interface docs

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
4 years agodevice: allow pipelining UAPI requests
Jason A. Donenfeld [Mon, 25 Jan 2021 18:00:43 +0000 (19:00 +0100)] 
device: allow pipelining UAPI requests

The original spec ends with \n\n especially for this reason.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agoipc: add missing Windows errno
Jason A. Donenfeld [Mon, 25 Jan 2021 18:08:41 +0000 (19:08 +0100)] 
ipc: add missing Windows errno

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: serialize access to IpcSetOperation
Josh Bleecher Snyder [Mon, 25 Jan 2021 17:35:35 +0000 (09:35 -0800)] 
device: serialize access to IpcSetOperation

Interleaves IpcSetOperations would spell trouble.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: simplify handling of IPC set endpoint
Josh Bleecher Snyder [Mon, 25 Jan 2021 17:32:09 +0000 (09:32 -0800)] 
device: simplify handling of IPC set endpoint

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove close processing fwmark
Josh Bleecher Snyder [Mon, 25 Jan 2021 17:27:06 +0000 (09:27 -0800)] 
device: remove close processing fwmark

Also, a behavior change: Stop treating a blank value as 0.
It's not in the spec.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove unnecessary comment
Josh Bleecher Snyder [Mon, 25 Jan 2021 17:21:57 +0000 (09:21 -0800)] 
device: remove unnecessary comment

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: introduce new IPC error message for unknown error
Josh Bleecher Snyder [Mon, 25 Jan 2021 17:21:51 +0000 (09:21 -0800)] 
device: introduce new IPC error message for unknown error

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: correct IPC error number for I/O errors
Josh Bleecher Snyder [Mon, 25 Jan 2021 17:21:43 +0000 (09:21 -0800)] 
device: correct IPC error number for I/O errors

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: simplify IpcHandle error handling
Josh Bleecher Snyder [Mon, 25 Jan 2021 16:55:08 +0000 (08:55 -0800)] 
device: simplify IpcHandle error handling

Unify the handling of unexpected UAPI errors.
The comment that says "should never happen" is incorrect;
this could happen due to I/O errors. Correct it.

Change error message capitalization for consistency.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: split IpcSetOperation into parts
Josh Bleecher Snyder [Fri, 15 Jan 2021 22:32:34 +0000 (14:32 -0800)] 
device: split IpcSetOperation into parts

The goal of this change is to make the structure
of IpcSetOperation easier to follow.

IpcSetOperation contains a small state machine:
It starts by configuring the device,
then shifts to configuring one peer at a time.

Having the code all in one giant method obscured that structure.
Split out the parts into helper functions and encapsulate the peer state.

This makes the overall structure more apparent.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: expand IPCError
Josh Bleecher Snyder [Fri, 15 Jan 2021 21:24:38 +0000 (13:24 -0800)] 
device: expand IPCError

Expand IPCError to contain a wrapped error,
and add a helper to make constructing such errors easier.

Add a defer-based "log on returned error" to IpcSetOperation.
This lets us simplify all of the error return paths.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove dead code
Josh Bleecher Snyder [Fri, 15 Jan 2021 21:59:55 +0000 (13:59 -0800)] 
device: remove dead code

If device.NewPeer returns a nil error,
then the returned peer is always non-nil.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: return errors from ipc scanner
Josh Bleecher Snyder [Fri, 15 Jan 2021 21:54:55 +0000 (13:54 -0800)] 
device: return errors from ipc scanner

The code as written will drop any read errors on the floor.
Fix that.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agonetstack: further sequester with own go.mod and go.sum
Jason A. Donenfeld [Wed, 20 Jan 2021 23:24:01 +0000 (00:24 +0100)] 
netstack: further sequester with own go.mod and go.sum

In order to avoid even the flirtation with passing on these dependencies
to ordinary consumers of wireguard-go, this commit makes a new go.mod
that's entirely separate from the root one.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agonetstack: introduce new module for gvisor tcp tun adapter
Jason A. Donenfeld [Wed, 20 Jan 2021 23:02:32 +0000 (00:02 +0100)] 
netstack: introduce new module for gvisor tcp tun adapter

The Go linker isn't smart enough to prevent gvisor from being pulled
into modules that use other parts of tun/, due to the types exposed. So,
we put this into its own standalone module.

We use this as an opportunity to introduce some example code as well.

I'm still not happy that this not only clutters this repo's go.sum, but
all the other projects that consume it, but it seems like making a new
module inside of this repo will lead to even greater confusion.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: allow compiling with Go 1.15
Jason A. Donenfeld [Wed, 20 Jan 2021 19:04:31 +0000 (20:04 +0100)] 
device: allow compiling with Go 1.15

Until we depend on Go 1.16 (which isn't released yet), alias our own
variable to the private member of the net package. This will allow an
easy find replace to make this go away when we eventually switch to
1.16.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
4 years agodevice: remove unused fields from DummyDatagram and DummyBind
Josh Bleecher Snyder [Tue, 19 Jan 2021 21:42:45 +0000 (13:42 -0800)] 
device: remove unused fields from DummyDatagram and DummyBind

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove unused trie test code
Josh Bleecher Snyder [Tue, 19 Jan 2021 21:41:47 +0000 (13:41 -0800)] 
device: remove unused trie test code

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agoconn: remove _ method receiver
Josh Bleecher Snyder [Tue, 19 Jan 2021 21:41:07 +0000 (13:41 -0800)] 
conn: remove _ method receiver

Minor style fix.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agotun: fix fmt.Errorf format strings
Josh Bleecher Snyder [Tue, 19 Jan 2021 21:39:48 +0000 (13:39 -0800)] 
tun: fix fmt.Errorf format strings

Type tcpip.Error is not an error.

I've filed https://github.com/google/gvisor/issues/5314
to fix this upstream.

Until that is fixed, use %v instead of %w,
to keep vet happy.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove unnecessary zeroing
Josh Bleecher Snyder [Tue, 19 Jan 2021 16:33:12 +0000 (08:33 -0800)] 
device: remove unnecessary zeroing

Newly allocated objects are already zeroed.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove QueueInboundElement.dropped
Josh Bleecher Snyder [Sun, 17 Jan 2021 17:49:39 +0000 (09:49 -0800)] 
device: remove QueueInboundElement.dropped

Now that we block when enqueueing to the decryption queue,
there is only one case in which we "drop" a inbound element,
when decryption fails.

We can use a simple, obvious, sync-free sentinel for that, elem.packet == nil.
Also, we can return the message buffer to the pool slightly later,
which further simplifies the code.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove QueueOutboundElement.dropped
Josh Bleecher Snyder [Sun, 17 Jan 2021 17:40:18 +0000 (09:40 -0800)] 
device: remove QueueOutboundElement.dropped

If we block when enqueuing encryption elements to the queue,
then we never drop them.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agotun/wintun/memmod: gofmt
Josh Bleecher Snyder [Sat, 16 Jan 2021 01:25:34 +0000 (17:25 -0800)] 
tun/wintun/memmod: gofmt

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agotun/wintun/memmod: fix format verb
Josh Bleecher Snyder [Sat, 16 Jan 2021 01:25:11 +0000 (17:25 -0800)] 
tun/wintun/memmod: fix format verb

Caught by 'go vet'.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: check returned errors from NewPeer in TestNoiseHandshake
Josh Bleecher Snyder [Wed, 13 Jan 2021 01:27:27 +0000 (17:27 -0800)] 
device: check returned errors from NewPeer in TestNoiseHandshake

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years agodevice: remove selects from encrypt/decrypt/inbound/outbound enqueuing
Josh Bleecher Snyder [Tue, 12 Jan 2021 01:21:16 +0000 (17:21 -0800)] 
device: remove selects from encrypt/decrypt/inbound/outbound enqueuing

Block instead. Backpressure here is fine, probably preferable.
This reduces code complexity.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>