To be honest I'm not fond of Golden Bytes tests like this, but
not so much as to want to rewrite the whole test. The DNS byte
format is essentially immutable at this point, the encoded bytes
aren't going to change. The rest of the test assumptions about
hostnames might, but we can fix that when it comes.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
eccc167 introduced closeHandle which opened the handle,
but never closed it.
Windows handles should be closed.
Updates #921
Signed-off-by: Alex Brainman <alex.brainman@gmail.com>
Previously the client had heuristics to calculate which DNS search domains
to set, based on the peers' names. Unfortunately that prevented us from
doing some things we wanted to do server-side related to node sharing.
So, bump MapRequest.Version to 9 to signal that the client only uses the
explicitly configured DNS search domains and doesn't augment it with its own
list.
Updates tailscale/corp#1026
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is a replacement for the key-related parts
of the wireguard-go wgcfg package.
This is almost a straight copy/paste from the wgcfg package.
I have slightly changed some of the exported functions and types
to avoid stutter, added and tweaked some comments,
and removed some now-unused code.
To avoid having wireguard-go depend on this new package,
wgcfg will keep its key types.
We translate into and out of those types at the last minute.
These few remaining uses will be eliminated alongside
the rest of the wgcfg package.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The windows key timeout is longer than the wgengine watchdog timeout,
which means we never reach the timeout, instead the process exits.
Reduce the timeout so if we do hit it, at least the process continues.
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
On Win10, there's a hardcoded GUID and this works.
On Win7, this GUID changes and we need to ask the tun for its
LUID and convert that from the GUID.
This commit uses the computed GUID that is placed in InterfaceName.
Diagnosed by Jason Donnenfeld. (Thanks!)
Lazy wg configuration now triggers if a peer has only endpoint
addresses (/32 for IPv4, /128 for IPv6). Subnet routers still
trigger eager configuration to avoid the need for a CIDR match
in the hot packet path.
Signed-off-by: David Anderson <danderson@tailscale.com>
The previous code used a lot of whole-function variables and shared
behavior that only triggered based on prior action from a single codepath.
Instead of that, move the small amounts of "shared" code into each switch
case.
Signed-off-by: David Anderson <danderson@tailscale.com>
Before, tailscaled would log every 10 seconds when the periodic noteRecvActivity
call happens. This is noisy, but worse it's misleading, because the message
suggests that the disco code is starting a lazy config run for a missing peer,
whereas in fact it's just an internal piece of keepalive logic.
With this change, we still log when going from 0->1 tunnel for the peer, but
not every 10s thereafter.
Signed-off-by: David Anderson <danderson@tailscale.com>
While the code was correct, I broke it during a refactoring and
tests didn't detect it. This fixes that glitch.
Signed-off-by: David Anderson <danderson@tailscale.com>
Doesn't materially affect benchmarks, but shrinks match6 by 30 instructions
and halves memory loads.
Part of #19.
Signed-off-by: David Anderson <danderson@tailscale.com>
Part of #19.
name old time/op new time/op delta
Filter/icmp4-8 32.2ns ± 3% 32.5ns ± 2% ~ (p=0.524 n=10+8)
Filter/icmp6-8 49.7ns ± 6% 43.1ns ± 4% -13.12% (p=0.000 n=9+10)
Signed-off-by: David Anderson <danderson@tailscale.com>
The packet filter still rejects all IPv6, but decodes enough from v6
packets to do something smarter in a followup.
name time/op
Decode/tcp4-8 28.8ns ± 2%
Decode/tcp6-8 20.6ns ± 1%
Decode/udp4-8 28.2ns ± 1%
Decode/udp6-8 20.0ns ± 6%
Decode/icmp4-8 21.7ns ± 2%
Decode/icmp6-8 14.1ns ± 2%
Decode/unknown-8 9.43ns ± 2%
Signed-off-by: David Anderson <danderson@tailscale.com>
In tests, we force binding to localhost to avoid OS firewall warning
dialogs.
But for IPv6, we were trying (and failing) to bind to 127.0.0.1.
You'd think we'd just say "localhost", but that's apparently ill
defined. See
https://tools.ietf.org/html/draft-ietf-dnsop-let-localhost-be-localhost
and golang/go#22826. (It's bitten me in the past, but I can't
remember specific bugs.)
So use "::1" explicitly for "udp6", which makes the test quieter.
The goal is to move some of the shenanigans we have elsewhere into the filter
package, so that all the weird things to do with poking at the filter is in
a single place, behind clean APIs.
Signed-off-by: David Anderson <danderson@tailscale.com>
We still use the packet.* alloc-free types in the data path, but
the compilation from netaddr to packet happens within the filter
package.
Signed-off-by: David Anderson <danderson@tailscale.com>
os.IsNotExist doesn't unwrap errors. errors.Is does.
The ioutil.ReadFile ones happened to be fine but I changed them so
we're consistent with the rule: if the error comes from os, you can
use os.IsNotExist, but from any other package, use errors.Is.
(errors.Is always would also work, but not worth updating all the code)
The motivation here was that we were logging about failure to migrate
legacy relay node prefs file on startup, even though the code tried
to avoid that.
See golang/go#41122
Amazingly, there doesn't seem to be a documented way of updating network
configuration programmatically in a way that Windows takes notice of.
The naturopathic remedy for this is to invoke ipconfig /registerdns, which
does a variety of harmless things and also invokes the private API that
tells windows to notice new adapter settings. This makes our DNS config
changes stick within a few seconds of us setting them.
If we're invoking a shell command anyway, why futz with the registry at
all? Because netsh has no command for changing the DNS suffix list, and
its commands for setting resolvers requires parsing its output and
keeping track of which server is in what index. Amazingly, twiddling
the registry directly is the less painful option.
Fixes#853.
Signed-off-by: David Anderson <danderson@tailscale.com>
Updating the Windows firewall is usually reasonably fast, but
sometimes blocks for 20 seconds, 4 minutes, etc. Not sure why.
Until we understand that's happening, configure it in the background
without blocking the normal control flow.
Updates #785
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The RusagePrefixLog is rarely useful, hasn't been useful in a long
time, is rarely the measurement we need, and is pretty spammy (and
syscall-heavy). Disable it by default. We can enable it when we're
debugging memory.
At startup the client doesn't yet have the DERP map so can't do STUN
queries against DERP servers, so it only knows it local interface
addresses, not its STUN-mapped addresses.
We were reporting the interface-local addresses to control, getting
the DERP map, and then immediately reporting the full set of
updates. That was an extra HTTP request to control, but worse: it was
an extra broadcast from control out to all the peers in the network.
Now, skip the initial update if there are no stun results and we don't
have a DERP map.
More work remains optimizing start-up requests/map updates, but this
is a start.
Updates tailscale/corp#557
There was a bug with the lazy wireguard config code where, if the
minimum set of peers to tell wireguard didn't change, we skipped
calling userspaceEngine.updateActivityMapsLocked which updated
the various data structures that matched incoming traffic to later
reconfigure the minimum config.
That meant if an idle peer restarted and changed discovery keys, we
skipped updating our maps of disco keys/IPs that would caused us to
lazily inflate the config for that peer later if/when it did send
traffic.
Use golang.zx2c4.com/wireguard/windows/tunnel/winipcfg
instead of github.com/tailscale/winipcfg-go package.
Updates #760
Signed-off-by: Alex Brainman <alex.brainman@gmail.com>
Due to a copy/paste-o, we were monitoring address changes twice, and
not monitoring route changes at all.
Verified with 'tailscale debug --monitor' that this actually works now (while
running 'route add 10.3.0.0 mask 255.255.0.0 10.0.0.1' and 'route delete (same)'
back and forth in cmd.exe)
In practice route changes are accompanied by address changes and this
doesn't fix any known issues. I just noticed this while reading this
code again. But at least the code does what it was trying to do now.
This function is only called in fake mode, which won't do anything more
with the packet after we respond to it anyway, so dropping it in the
prefilter is not necessary. And it's kinda semantically wrong: we did
not reject it, so telling the upper layer that it was rejected produces
an ugly error message.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
At some point faketun got implemented as a loopback (put a packet in
from wireguard, the same packet goes back to wireguard) which is not
useful. It's supposed to be an interface that just sinks all packets,
and then wgengine adds *only* and ICMP Echo responder as a layer on
top.
This caused extremely odd bugs on darwin, where the special case that
reinjects packets from local->local was filling the loopback channel
and creating an infinite loop (which became jammed since the reader and
writer were in the same goroutine).
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
If no interfaces are up, calm down and stop spamming so much. It was
noticed as especially bad on Windows, but probably was bad
everywhere. I just have the best network conditions testing on a
Windows VM.
Updates #604
Gracefully skips touching the v6 NAT table on systems that don't have
it, and doesn't configure IPv6 at all if IPv6 is globally disabled.
Signed-off-by: David Anderson <danderson@tailscale.com>
When the network link changes, existing UDP sockets fail immediately
and permanently on macOS.
The forwarder set up a single UDP conn and never changed it.
As a result, any time there was a network link change,
all forwarded DNS queries failed.
To fix this, create a new connection when send requests
fail because of network unreachability.
This change is darwin-only, although extended it to other platforms
should be straightforward.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
While we're here, parseQuery into a plain function.
This is helpful for fuzzing. (Which I did a bit of. Didn't find anything.)
And clean up a few minor things.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Part of unforking our winipcfg-go and using upstream (#760), move our
additions into our repo. (We might upstream them later if upstream has
interest)
Originally these were:
@apenwarr: "Add ifc.SyncAddresses() and SyncRoutes()."
609dcf2df5
@bradfitz: "winipcfg: make Interface.AddRoutes do as much as possible, return combined error"
e9f93d53f3
@bradfitz: "prevent unnecessary Interface.SyncAddresses work; normalize IPNets in deltaNets"
decb9ee8e1
Might fix it. I've spent too much time failing to reproduce the issue. This doesn't
seem to make it worse, though (it still runs for me), so I'll include this and
see if it helps others while I still work on a reliable way to reproduce it.
Updates tailscale/corp#474
