Commit graph

853 commits

Author SHA1 Message Date
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
Jason A. Donenfeld 18e47795e5 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>
2021-01-25 20:48:28 +01:00
Jason A. Donenfeld a29767dda6 ipc: add missing Windows errno
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-25 20:48:28 +01:00
Josh Bleecher Snyder cecb41515d device: serialize access to IpcSetOperation
Interleaves IpcSetOperations would spell trouble.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-25 09:38:09 -08:00
Josh Bleecher Snyder a9ce4b762c device: simplify handling of IPC set endpoint
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-25 09:37:28 -08:00
Josh Bleecher Snyder d8f2cc87ee 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>
2021-01-25 09:36:53 -08:00
Josh Bleecher Snyder 2b8665f5f9 device: remove unnecessary comment
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-25 09:36:41 -08:00
Josh Bleecher Snyder 674a4675a1 device: introduce new IPC error message for unknown error
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-25 09:36:17 -08:00
Josh Bleecher Snyder 87bdcb2ae4 device: correct IPC error number for I/O errors
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-25 09:35:48 -08:00
Josh Bleecher Snyder 37a239e736 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>
2021-01-25 09:09:24 -08:00
Josh Bleecher Snyder 6252de0db9 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>
2021-01-25 09:09:24 -08:00
Josh Bleecher Snyder a029b942ae 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>
2021-01-25 08:47:48 -08:00
Josh Bleecher Snyder db3fa1409c 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>
2021-01-25 08:47:48 -08:00
Josh Bleecher Snyder 675aae2423 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>
2021-01-25 08:47:48 -08:00
Jason A. Donenfeld fcc8ad05df 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>
2021-01-21 00:25:02 +01:00
Jason A. Donenfeld 1d4eb2727a 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>
2021-01-21 00:16:59 +01:00
Jason A. Donenfeld 294d3bedf9 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>
2021-01-20 20:12:32 +01:00
Josh Bleecher Snyder 86a58b51c0 device: remove unused fields from DummyDatagram and DummyBind
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 20:03:40 +01:00
Josh Bleecher Snyder 6a2ecb581b device: remove unused trie test code
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 20:03:40 +01:00
Josh Bleecher Snyder f07177c762 conn: remove _ method receiver
Minor style fix.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 20:03:40 +01:00
Josh Bleecher Snyder b00b2c2951 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>
2021-01-20 20:03:40 +01:00
Josh Bleecher Snyder 7c5d1e355e device: remove unnecessary zeroing
Newly allocated objects are already zeroed.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 19:57:07 +01:00
Josh Bleecher Snyder a86492a567 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>
2021-01-20 19:57:06 +01:00
Josh Bleecher Snyder 7ee95e053c 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>
2021-01-20 19:57:05 +01:00
Josh Bleecher Snyder 291dbcf1f0 tun/wintun/memmod: gofmt
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 19:57:04 +01:00
Josh Bleecher Snyder abc88c82b1 tun/wintun/memmod: fix format verb
Caught by 'go vet'.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 19:57:02 +01:00
Josh Bleecher Snyder 23642a13be device: check returned errors from NewPeer in TestNoiseHandshake
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 19:57:01 +01:00
Josh Bleecher Snyder 2fe19ce54d 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>
2021-01-20 19:57:00 +01:00
Josh Bleecher Snyder 0cc15e7c7c device: put handshake buffer in pool in FlushPacketQueues
This appears to have been an oversight.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 19:56:59 +01:00
Josh Bleecher Snyder 48c3b87eb8 device: use channel close to shut down and drain decryption channel
This is similar to commit e1fa1cc556,
but for the decryption channel.

It is an alternative fix to f9f655567930a4cd78d40fa4ba0d58503335ae6a.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-20 19:56:54 +01:00
Jason A. Donenfeld 675955de5d tun: add tcpip stack tunnel abstraction
This allows people to initiate connections over WireGuard without any
underlying operating system support.

I'm not crazy about the trash it adds to go.sum, but the code this
actually adds to the binaries seems contained to the gvisor repo.

For the TCP/IP implementation, it uses gvisor. And it borrows some
internals from the Go standard library's resolver in order to bring Dial
and DialContext to tun_net, along with the LookupHost helper function.
This allows for things like HTTP2-over-TLS to work quite well:

    package main

    import (
        "io"
        "log"
        "net"
        "net/http"

        "golang.zx2c4.com/wireguard/device"
        "golang.zx2c4.com/wireguard/tun"
    )

    func main() {
        tun, tnet, err := tun.CreateNetTUN([]net.IP{net.ParseIP("192.168.4.29")}, []net.IP{net.ParseIP("8.8.8.8"), net.ParseIP("8.8.4.4")}, 1420)
        if err != nil {
            log.Panic(err)
        }
        dev := device.NewDevice(tun, &device.Logger{log.Default(), log.Default(), log.Default()})
        dev.IpcSet(`private_key=a8dac1d8a70a751f0f699fb14ba1cff7b79cf4fbd8f09f44c6e6a90d0369604f
    public_key=25123c5dcd3328ff645e4f2a3fce0d754400d3887a0cb7c56f0267e20fbf3c5b
    endpoint=163.172.161.0:12912
    allowed_ip=0.0.0.0/0
    `)
        dev.Up()

        client := http.Client{
            Transport: &http.Transport{
                DialContext: tnet.DialContext,
            },
        }
        resp, err := client.Get("https://www.zx2c4.com/ip")
        if err != nil {
            log.Panic(err)
        }
        body, err := io.ReadAll(resp.Body)
        if err != nil {
            log.Panic(err)
        }
        log.Println(string(body))
    }

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-13 16:33:40 +01:00
Jason A. Donenfeld ea6c1cd7e6 device: receive: do not exit immediately on transient UDP receive errors
Some users report seeing lines like:

> Routine: receive incoming IPv4 - stopped

Popping up unexpectedly. Let's sleep and try again before failing, and
also log the error, and perhaps we'll eventually understand this
situation better in future versions.

Because we have to distinguish between the socket being closed
explicitly and whatever error this is, we bump the module to require Go
1.16.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-08 14:30:04 +01:00
Jason A. Donenfeld 3b3de758ec conn: linux: do not allow ReceiveIPvX to race with Close
If Close is called after ReceiveIPvX, then ReceiveIPvX will block on an
invalid or potentially reused fd.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-07 17:08:58 +01:00
Jason A. Donenfeld 29b0477585 device: receive: drain decryption queue before exiting RoutineDecryption
It's possible for RoutineSequentialReceiver to try to lock an elem after
RoutineDecryption has exited. Before this meant we didn't then unlock
the elem, so the whole program deadlocked.

As well, it looks like the flush code (which is now potentially
unnecessary?) wasn't properly dropping the buffers for the
not-already-dropped case.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-07 17:08:41 +01:00
Josh Bleecher Snyder 85b4950579 device: add latency and throughput benchmarks
These obviously don't perfectly capture real world performance,
in which syscalls and network links have a significant impact.
Nevertheless, they capture some of the internal performance factors,
and they're easy and convenient to work with.

Hat tip to Avery Pennarun for help designing the throughput benchmark.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-07 14:49:44 +01:00
Josh Bleecher Snyder 8a30415555 device: use LogLevelError for benchmarking
This keeps the output minimal and focused on the benchmark results.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-07 14:49:44 +01:00
Josh Bleecher Snyder cdaf4e9a76 device: make test infrastructure usable with benchmarks
Switch from *testing.T to testing.TB.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-07 14:49:44 +01:00
Jason A. Donenfeld 3d83df9bf3 memmod: apply explicit build tags to _32 and _64 files
Since _32 and _64 aren't valid goarchs, they don't match _GOOS_GOARCH,
and so the existing tags wind up not being restricted to windows-only.
This fixes the problem by adding windows to the tags explicitly. We
could also fix it by calling the files _32_windows or _64_windows, but
that changes the convention with the other single-arch files.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-07 14:49:44 +01:00
Jason A. Donenfeld d664444928 tun: make customization of WintunPool and requested GUID more obvious
Persnickety consumers can now do:

    func init() {
        tun.WintunPool, _ = wintun.MakePool("Flurp")
        tun.WintunStaticRequestedGUID, _ = windows.GUIDFromString("{5ae2716f-0b3e-4dc4-a8b5-48eba11a6e16}")
    }

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-07 14:49:44 +01:00
Josh Bleecher Snyder 1481e72107 all: use ++ to increment
Make the code slightly more idiomatic. No functional changes.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-07 14:49:44 +01:00
Josh Bleecher Snyder d0f8e9477c device: remove unnecessary zeroing
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-07 14:49:44 +01:00
Josh Bleecher Snyder b42e32047d device: call wg.Add outside the goroutine
One of the first rules of WaitGroups is that you call wg.Add
outside of a goroutine, not inside it. Fix this embarrassing mistake.

This prevents an extremely rare race condition (2 per 100,000 runs)
which could occur when attempting to start a new peer
concurrently with shutting down a device.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-01-07 14:49:44 +01:00