Commit graph

923 commits

Author SHA1 Message Date
Jason A. Donenfeld 4e9e5dad09 main: check that code is formatted in unit test
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-10 17:48:26 +02:00
Jason A. Donenfeld 39e0b6dade tun: format
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 12:21:27 +02:00
Jason A. Donenfeld 7121927b87 device: add ID to repeated routines
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 12:21:21 +02:00
Jason A. Donenfeld 326aec10af device: remove unusual ... in messages
We dont use ... in any other present progressive messages except these.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 12:17:41 +02:00
Jason A. Donenfeld efb8818550 device: avoid verbose log line during ordinary shutdown sequence
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 09:39:06 +02:00
Jason A. Donenfeld 69b39db0b4 tun: windows: set event before waiting
In 097af6e ("tun: windows: protect reads from closing") we made sure no
functions are running when End() is called, to avoid a UaF. But we still
need to kick that event somehow, so that Read() is allowed to exit, in
order to release the lock. So this commit calls SetEvent, while moving
the closing boolean to be atomic so it can be modified without locks,
and then moves to a WaitGroup for the RCU-like pattern.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 09:26:24 +02:00
Jason A. Donenfeld db733ccd65 tun: windows: rearrange struct to avoid alignment trap on 32bit
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 09:19:00 +02:00
Jason A. Donenfeld a7aec4449f tun: windows: check alignment in unit test
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-07 09:15:50 +02:00
Josh Bleecher Snyder 60a26371f4 device: log all errors received by RoutineReceiveIncoming
When debugging, it's useful to know why a receive func exited.

We were already logging that, but only in the "death spiral" case.
Move the logging up, to capture it always.
Reduce the verbosity, since it is not an error case any more.
Put the receive func name in the log line.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-05-06 11:22:13 +02:00
Jason A. Donenfeld a544776d70 tun/netstack: update go mod and remove GSO argument
Reported-by: John Xiong <xiaoyang1258@yeah.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-06 11:07:26 +02:00
Jason A. Donenfeld 69a42a4eef tun: windows: send MTU update when forced MTU changes
Otherwise the padding doesn't get updated.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-05 11:42:45 +02:00
Jason A. Donenfeld 097af6e135 tun: windows: protect reads from closing
The code previously used the old errors channel for checking, rather
than the simpler boolean, which caused issues on shutdown, since the
errors channel was meaningless. However, looking at this exposed a more
basic problem: Close() and all the other functions that check the closed
boolean can race. So protect with a basic RW lock, to ensure that
Close() waits for all pending operations to complete.

Reported-by: Joshua Sjoding <joshua.sjoding@scjalliance.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-26 22:22:45 -04:00
Jason A. Donenfeld 8246d251ea conn: windows: do not error out when receiving UDP jumbogram
If we receive a large UDP packet, don't return an error to receive.go,
which then terminates the receive loop. Instead, simply retry.

Considering Winsock's general finickiness, we might consider other
places where an attacker on the wire can generate error conditions like
this.

Reported-by: Sascha Dierberg <sascha.dierberg@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-26 22:07:03 -04:00
Jason A. Donenfeld c9db4b7aaa version: bump snapshot
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-24 13:07:27 -04:00
Jason A. Donenfeld 3625f8d284 tun: freebsd: avoid OOB writes
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-19 15:10:23 -06:00
Jason A. Donenfeld 0687dc06c8 tun: freebsd: become controlling process when reopening tun FD
When we pass the TUN FD to the child, we have to call TUNSIFPID;
otherwise when we close the device, we get a splat in dmesg.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-19 15:02:44 -06:00
Jason A. Donenfeld 71aefa374d tun: freebsd: restructure and cleanup
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-19 14:54:59 -06:00
Jason A. Donenfeld 3d3e30beb8 tun: freebsd: remove horrific hack for getting tunnel name
As of FreeBSD 12.1, there's TUNGIFNAME.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-19 12:03:16 -06:00
Jason A. Donenfeld b0e5b19969 tun: freebsd: set IFF_MULTICAST for routing daemons
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-18 20:09:04 -06:00
Jason A. Donenfeld 3988821442 main: print kernel warning on OpenBSD and FreeBSD too
More kernels!

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-15 23:35:45 -06:00
Jason A. Donenfeld c7cd2c9eab device: don't defer unlocking from loop
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-12 16:19:35 -06:00
Jason A. Donenfeld 54dbe2471f conn: reconstruct v4 vs v6 receive function based on symtab
This is kind of gross but it's better than the alternatives.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-12 15:35:32 -06:00
Kristupas Antanavičius d2fd0c0cc0 device: allocate new buffer in receive death spiral
Note: this bug is "hidden" by avoiding "death spiral" code path by
6228659 ("device: handle broader range of errors in RoutineReceiveIncoming").

If the code reached "death spiral" mechanism, there would be multiple
double frees happening. This results in a deadlock on iOS, because the
pools are fixed size and goroutine might stop until somebody makes
space in the pool.

This was almost 100% repro on the new ARM Macbooks:

- Build with 'ios' tag for Mac. This will enable bounded pools.
- Somehow call device.IpcSet at least couple of times (update config)
- device.BindUpdate() would be triggered
- RoutineReceiveIncoming would enter "death spiral".
- RoutineReceiveIncoming would stall on double free (pool is already
  full)
- The stuck routine would deadlock 'device.closeBindLocked()' function
  on line 'netc.stopping.Wait()'

Signed-off-by: Kristupas Antanavičius <kristupas.antanavicius@nordsec.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-12 11:14:53 -06:00
Jason A. Donenfeld 5f6bbe4ae8 conn: windows: reset ring to starting position after free
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-09 18:09:41 -06:00
Jason A. Donenfeld 75526d6071 conn: windows: compare head and tail properly
By not comparing these with the modulo, the ring became nearly never
full, resulting in completion queue buffers filling up prematurely.

Reported-by: Joshua Sjoding <joshua.sjoding@scjalliance.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-09 14:26:08 -06:00
Jason A. Donenfeld fbf97502cf winrio: test that IOCP-based RIO is supported
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-09 14:26:08 -06:00
Josh Bleecher Snyder 10533c3e73 all: make conn.Bind.Open return a slice of receive functions
Instead of hard-coding exactly two sources from which
to receive packets (an IPv4 source and an IPv6 source),
allow the conn.Bind to specify a set of sources.

Beneficial consequences:

* If there's no IPv6 support on a system,
  conn.Bind.Open can choose not to return a receive function for it,
  which is simpler than tracking that state in the bind.
  This simplification removes existing data races from both
  conn.StdNetBind and bindtest.ChannelBind.
* If there are more than two sources on a system,
  the conn.Bind no longer needs to add a separate muxing layer.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-04-02 11:07:08 -06:00
Jason A. Donenfeld 8ed83e0427 conn: winrio: pass key parameter into struct
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-04-02 10:36:41 -06:00
Josh Bleecher Snyder 6228659a91 device: handle broader range of errors in RoutineReceiveIncoming
RoutineReceiveIncoming exits immediately on net.ErrClosed,
but not on other errors. However, for errors that are known
to be permanent, such as syscall.EAFNOSUPPORT,
we may as well exit immediately instead of retrying.

This considerably speeds up the package device tests right now,
because the Bind sometimes (incorrectly) returns syscall.EAFNOSUPPORT
instead of net.ErrClosed.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-03-30 12:41:43 -07:00
Josh Bleecher Snyder 517f0703f5 conn: document retry loop in StdNetBind.Open
It's not obvious on a first read what the loop is doing.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-03-30 12:09:38 -07:00
Josh Bleecher Snyder 204140016a conn: use local ipvN vars in StdNetBind.Open
This makes it clearer that they are fresh on each attempt,
and avoids the bookkeeping required to clearing them on failure.

Also, remove an unnecessary err != nil.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-03-30 12:09:38 -07:00
Josh Bleecher Snyder 822f5a6d70 conn: unify code in StdNetBind.Send
The sending code is identical for ipv4 and ipv6;
select the conn, then use it.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-03-30 12:09:32 -07:00
Josh Bleecher Snyder 02e419ed8a device: rename unsafeCloseBind to closeBindLocked
And document a bit.
This name is more idiomatic.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
2021-03-30 12:07:12 -07:00
Jason A. Donenfeld bc69a3fa60 version: bump snapshot 2021-03-23 13:07:19 -06:00
Jason A. Donenfeld 12ce53271b tun: freebsd: use broadcast mode instead of PPP mode
It makes the routing configuration simpler.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-23 12:41:34 -06:00
Jason A. Donenfeld 5f0c8b942d device: signal to close device in separate routine
Otherwise we wind up deadlocking.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-11 09:29:10 -07:00
Jason A. Donenfeld c5f382624e tun: linux: do not spam events every second from hack listener
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-11 09:23:11 -07:00
Kay Diam 6005c573e2 tun: freebsd: allow empty names
This change allows omitting the tun interface name setting. When the
name is not set, the kernel automatically picks up the tun name and
index.

Signed-off-by: Kay Diam <kay.diam@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-08 21:32:27 -07:00
Jason A. Donenfeld 82f3e9e2af winpipe: move syscalls into x/sys
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-08 21:32:27 -07:00
Jason A. Donenfeld 4885e7c954 memmod: use resource functions from x/sys
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-08 21:04:09 -07:00
Jason A. Donenfeld 497ba95de7 memmod: do not use IsBadReadPtr
It should be enough to check for the trailing zero name.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-08 21:04:09 -07:00
Jason A. Donenfeld 0eb7206295 conn: linux: unexport mutex
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-08 21:04:09 -07:00
Jason A. Donenfeld 20714ca472 mod: bump x/sys
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-08 21:04:09 -07:00
Jason A. Donenfeld c1e09f1927 mod: rename COPYING to LICENSE
Otherwise the netstack module doesn't show up on the package site.

https://github.com/golang/go/issues/43817#issuecomment-764987580

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-06 09:09:21 -07:00
Jason A. Donenfeld 79611c64e8 tun/netstack: bump deps and api
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-06 08:48:14 -07:00
Jason A. Donenfeld 593658d975 device: get rid of peers.empty boolean in timersActive
There's no way for len(peers)==0 when a current peer has
isRunning==false.

This requires some struct reshuffling so that the uint64 pointer is
aligned.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-03-06 08:44:38 -07:00
Jason A. Donenfeld 3c11c0308e conn: implement RIO for fast Windows UDP sockets
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-25 15:08:08 +01:00
Jason A. Donenfeld f9dac7099e global: remove TODO name graffiti
Googlers have a habit of graffiting their name in TODO items that then
are never addressed, and other people won't go near those because
they're marked territory of another animal. I've been gradually cleaning
these up as I see them, but this commit just goes all the way and
removes the remaining stragglers.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-23 20:00:57 +01:00
Jason A. Donenfeld 9a29ae267c device: test up/down using virtual conn
This prevents port clashing bugs.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-23 20:00:57 +01:00
Jason A. Donenfeld 6603c05a4a device: cleanup unused test components
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-02-23 20:00:57 +01:00