Also remove rebinding logic from the windows router. Magicsock will
instead rebind based on link change signals.
Signed-off-by: David Anderson <danderson@tailscale.com>
Updates #654. See that issue for a discussion of why
this timeout reduces flakiness, and what next steps are.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
For now. Get it working again so it's not stuck on 0.98.
Subnet relay can come later.
Updates #451
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Start of making the IPN state machine react to link changes and down
its DNS & routes if necessary to unblock proxy resolution (e.g. for
transitioning from public to corp networks where the corp network has
mandatory proxies and WPAD PAC files that can't be resolved while
using the DNS/routes configured previously)
This change should be a no-op. Just some callback plumbing.
It turns out that otherwise we don't know what exactly was set.
Also remove the now unused RootDomain config option.
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
Previously, a type AAAA query would be answered with an A record
if only an IPv4 address was available. This is irrelevant for us
while we only use IPv4, but it will be a bug one day,
so it's worth being precise about semantics.
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
Rather than consider bigs jumps in last-received-from activity as a
signal to possibly reconfigure the set of wireguard peers to have
configured, instead just track the set of peers that are currently
excluded from the configuration. Easier to reason about.
Also adds a bit more logging.
This might fix an error we saw on a machine running a recent unstable
build:
2020-08-26 17:54:11.528033751 +0000 UTC: 8.6M/92.6M magicsock: [unexpected] lazy endpoint not created for [UcppE], d:42a770f678357249
2020-08-26 17:54:13.691305296 +0000 UTC: 8.7M/92.6M magicsock: DERP packet received from idle peer [UcppE]; created=false
2020-08-26 17:54:13.691383687 +0000 UTC: 8.7M/92.6M magicsock: DERP packet from unknown key: [UcppE]
If it does happen again, though, we'll have more logs.
Seems to break linux CI builder. Cannot reproduce locally,
so attempting a rollback.
This reverts commit cd7bc02ab1.
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
Without this, a freshly started ipn client will be stuck in the
"Starting" state until something triggers a call to RequestStatus.
Usually a UI does this, but until then we can sit in this state
until poked by an external event, as is evidenced by our e2e tests
locking up when DERP is attached.
(This only recently became a problem when we enabled lazy handshaking
everywhere, otherwise the wireugard tunnel creation would also
trigger a RequestStatus.)
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
The previous approach modifies name in-place in the request slice to avoid an allocation.
This is incorrect: the question section of a DNS request
must be copied verbatim, without any such modification.
Software may rely on it (we rely on other resolvers doing it it in tsdns/forwarder).
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
control/controlclient, wgengine/filter: extract parsePacketFilter to new constructor in wgengine/filter
Signed-off-by: chungdaniel <daniel@tailscale.com>
Consider:
Hard NAT (A) <---> Hard NAT w/ mapped port (B)
If A sends a packet to B's mapped port, A can disco ping B directly,
with low latency, without DERP.
But B couldn't establish a path back to A and needed to use DERP,
despite already logging about A's endpoint and adding a mapping to it
for other purposes (the wireguard conn.Endpoint lookup also needed
it).
This adds the tracking to discoEndpoint too so it'll be used for
finding a path back.
Fixestailscale/corp#556
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
For example:
$ tailscale ping -h
USAGE
ping <hostname-or-IP>
FLAGS
-c 10 max number of pings to send
-stop-once-direct true stop once a direct path is established
-verbose false verbose output
$ tailscale ping mon.ts.tailscale.com
pong from monitoring (100.88.178.64) via DERP(sfo) in 65ms
pong from monitoring (100.88.178.64) via DERP(sfo) in 252ms
pong from monitoring (100.88.178.64) via [2604:a880:2:d1::36:d001]:41641 in 33ms
Fixes#661
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
1) we weren't waking up a discoEndpoint that once existed and
went idle for 5 minutes and then got a disco message again.
2) userspaceEngine.noteReceiveActivity had a buggy check; fixed
and added a test
This removes the atomic bool that tried to track whether we needed to acquire
the lock on a future recursive call back into magicsock. Unfortunately that
hack doesn't work because we also had a lock ordering issue between magicsock
and userspaceEngine (see issue). This documents that too.
Fixes#644
If a node is behind a hard NAT and is using an explicit local port
number, assume they might've mapped a port and add their public IPv4
address with the local tailscaled's port number as a candidate endpoint.
A comparison operator was backwards.
The bad case went:
* device A send packet to B at t=1s
* B gets added to A's wireguard config
* B gets packet
(5 minutes pass)
* some other activity happens, causing B to expire
to be removed from A's network map, since it's
been over 5 minutes since sent or received activity
* device A sends packet to B at t=5m1s
* normally, B would get added back, but the old send
time was not zero (we sent earlier!) and the time
comparison was backwards, so we never regenerated
the wireguard config.
This also refactors the code for legibility and moves constants up
top, with comments.
The OS (tries) to send these but we drop them. No need to worry the
user with spam that we're dropping it.
Fixes#402
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Starting with fe68841dc7, some e2e tests
got flaky. Rather than debug them (they're gnarly), just revert to the old
behavior as far as those tests are concerned. The tests were somehow
using magicsock without a private key and expecting it to do ... something.
My goal with fe68841dc7 was to stop log spam
and unnecessary work I saw on the iOS app when when stopping the app.
Instead, only stop doing that work on any transition from
once-had-a-private-key to no-longer-have-a-private-key. That fixes
what I wanted to fix while still making the mysterious e2e tests
happy.
There is a race in natlab where we might start shutdown while natlab is still running
a goroutine or two to deliver packets. This adds a small grace period to try and receive
it before continuing shutdown.
Signed-off-by: David Anderson <danderson@tailscale.com>
The first packet to transit may take several seconds to do so, because
setup rates in wgengine may result in the initial WireGuard handshake
init to get dropped. So, we have to wait at least long enough for a
retransmit to correct the fault.
Signed-off-by: David Anderson <danderson@tailscale.com>
Active discovery lets us introspect the state of the network stack precisely
enough that it's unnecessary, and dropping the initial DERP packets greatly
slows down tests. Additionally, it's unrealistic since our production network
will never deliver _only_ discovery packets, it'll be all or nothing.
Signed-off-by: David Anderson <danderson@tailscale.com>
Uses natlab only, because the point of this active discovery test is going to be
that it should get through a lot of obstacles.
Signed-off-by: David Anderson <danderson@tailscale.com>
The deadlock was:
* Conn.Close was called, which acquired c.mu
* Then this goroutine scheduled:
if firstDerp {
startGate = c.derpStarted
go func() {
dc.Connect(ctx)
close(c.derpStarted)
}()
}
* The getRegion hook for that derphttp.Client then ran, which also
tries to acquire c.mu.
This change makes that hook first see if we're already in a closing
state and then it can pretend that region doesn't exist.
wireguard-go uses 3 goroutines per peer (with reasonably large stacks
& buffers).
Rather than tell wireguard-go about all our peers, only tell it about
peers we're actively communicating with. That means we need hooks into
magicsock's packet receiving path and tstun's packet sending path to
lazily create a wireguard peer on demand from the network map.
This frees up lots of memory for iOS (where we have almost nothing
left for larger domains with many users).
We should ideally do this in wireguard-go itself one day, but that'd
be a pretty big change.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
sync.Pools should almost always be packate globals, even though in this
case we only have exactly 1 TUN device anyway, so it matters less.
Still, it's unusual to see a Pool that's not a package global, so move it.
We originally picked those numbers somewhat at random, but with the idea
that 8 is a traditionally lucky number in Chinese culture. Unfortunately,
"88" is also neo-nazi shorthand language.
Use 52 instead, because those are the digits above the letters
"TS" (tailscale) on a qwerty keyboard, so we're unlikely to collide with
other users. 5, 2 and 52 are also pleasantly culturally meaningless.
Signed-off-by: David Anderson <danderson@tailscale.com>
Nameserver IP 10.11.12.13 would otherwise get written to resolv.conf as 13.12.11.10, as was happening on my client.
Signed-off-by: Eduardo Kienetz <eduardo@kienetz.com>
Before this patch, the 250ms sleep would not be interrupted by context cancellation,
which would result in the goroutine sometimes lingering in tests (100ms grace period).
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
Very rarely, cancellation occurs between a successful send on derpRecvCh
and a call to copyBuf on the receiving side.
Without this patch, this situation results in <-copyBuf blocking indefinitely.
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
Peers advertising a discovery key know how to speak the discovery
protocol and do their own heartbeats to get through NATs and keep NATs
open. No need for the pinger except for with legacy peers.
The new interface lets implementors more precisely distinguish
local traffic from forwarded traffic, and applies different
forwarding logic within Machines for each type. This allows
Machines to be packet forwarders, which didn't quite work
with the implementation of Inject.
Signed-off-by: David Anderson <danderson@tailscale.com>
This log line looks buggy, even though lacking a filter is expected during bringup.
We already know if we forget to SetFilter: it breaks the magicsock test,
so no useful information is lost.
Resolves#559.
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
The test demonstrates that magicsock can traverse two stateful
firewalls facing each other, that each require localhost to
initiate connections.
Signed-off-by: David Anderson <danderson@tailscale.com>
This change adds to tsdns the ability to delegate lookups to upstream nameservers.
This is crucial for setting Magic DNS as the system resolver.
Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
Never return "nil, nil" anymore. The caller expected a usable
interface now. I missed some of these earlier.
Also, handle address deletion now.
Updates #532
There's a lot of confusion around what tailscale status shows, so make it better:
show region names, last write time, and put stars around DERP too if active.
Now stars are always present if activity, and always somewhere.
* fix tailscale status for peers using discovery
* as part of that, pull out disco address selection into reusable
and testable discoEndpoint.addrForSendLocked
* truncate ping/pong logged hex txids in half to eliminate noise
* move a bunch of random time constants into named constants
with docs
* track a history of per-endpoint pong replies for future use &
status display
* add "send" and " got" prefix to discovery message logging
immediately before the frame type so it's easier to read than
searching for the "<-" or "->" arrows earlier in the line; but keep
those as the more reasily machine readable part for later.
Updates #483
Update the mapping from ip:port to discokey, so when we retrieve a
packet from the network, we can find the same conn.Endpoint that we
gave to wireguard-go previously, without making it think we've
roamed. (We did, but we're not using its roaming.)
Updates #483
Ping messages now go out somewhat regularly, pong replies are sent,
and pong replies are now partially handled enough to upgrade off DERP
to LAN.
CallMeMaybe packets are sent & received over DERP, but aren't yet
handled. That's next (and regular maintenance timers), and then WAN
should work.
Updates #483
Starting at yesterday's e96f22e560 (convering some UDPAddrs to
IPPorts), Conn.ReceiveIPv4 could return a nil addr, which would make
its way through wireguard-go and blow up later. The DERP read path
wasn't initializing the addr result parameter any more, and wgRecvAddr
wasn't checking it either.
Fixes#515
And while plumbing, a bit of discovery work I'll need: the
endpointOfAddr map to map from validated paths to the discoEndpoint.
Not being populated yet.
Updates #483
This adds a new magicsock endpoint type only used when both sides
support discovery (that is, are advertising a discovery
key). Otherwise the old code is used.
So far the new code only communicates over DERP as proof that the new
code paths are wired up. None of the actually discovery messaging is
implemented yet.
Support for discovery (generating and advertising a key) are still
behind an environment variable for now.
Updates #483
The new deepprint package just walks a Go data structure and writes to
an io.Writer. It's not pretty like go-spew, etc.
We then use it to replace the use of UAPI (which we have a TODO to
remove) to generate signatures of data structures to detect whether
anything changed (without retaining the old copy).
This was necessary because the UAPI conversion ends up trying to do
DNS lookups which an upcoming change depends on not happening.
And track known peers.
Doesn't yet do anything with the messages. (nor does it send any yet)
Start of docs on the message format. More will come in subsequent changes.
Updates #483
The NetworkExtension brings up the interface itself and does not have
access to `ifconfig`, which the underlying BSD userspace router attempts
to use when Up is called.
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
If there's been 5 minutes of inactivity, stop doing STUN lookups. That
means NAT mappings will expire, but they can resume later when there's
activity again.
We'll do this for all platforms later.
Updates tailscale/corp#320
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Darwin and FreeBSD are compatible enough to share the userspace router.
The OSX router delegates to the BSD userspace router unless `SetRoutesFunc` is set.
That preserves the mechanism that allows `ipn-go-bridge` to specify its own routing behavior.
Fixes#177
Signed-off-by: Reinaldo de Souza <github@rei.nal.do>
The magicsock derpReader was holding onto 65KB for each DERP
connection forever, just in case.
Make the derp{,http}.Client be in charge of memory instead. It can
reuse its bufio.Reader buffer space.
macOS incorrectly sends packets for the local Tailscale IP
into our tunnel interface. We have to turn the packets around
and send them back to the kernel.
Fixestailscale/corp#189.
Signed-off-by: David Anderson <danderson@tailscale.com>
iproute2 3.16.0-2 from Debian Jessie (oldoldstable) doesn't return
exit code 2 when deleting a non-existent IP rule.
Fixes#434
Signed-off-by: David Anderson <danderson@tailscale.com>
We have a filter in tailscaled itself now, which is more robust
against weird network topologies (such as the one Docker creates).
Signed-off-by: David Anderson <danderson@tailscale.com>
We'll use SO_BINDTODEVICE instead of fancy policy routing. This has
some limitations: for example, we will route all traffic through the
interface that has the main "default" (0.0.0.0/0) route, so machines
that have multiple physical interfaces might have to go through DERP to
get to some peers. But machines with multiple physical interfaces are
very likely to have policy routing (ip rule) support anyway.
So far, the only OS I know of that needs this feature is ChromeOS
(crostini). Fixes#245.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Otherwise iOS/macOS will reconfigure their routing every time anything
minor changes in the netmap (in particular, endpoints and DERP homes),
which is way too often.
Some users reported "network reconfigured" errors from Chrome when this
happens.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
This allows tailscaled's own traffic to bypass Tailscale-managed routes,
so that things like tailscale-provided default routes don't break
tailscaled itself.
Progress on #144.
Signed-off-by: David Anderson <danderson@tailscale.com>
We canceled the pingers in Close, but didn't wait around for their
goroutines to be cleaned up. This caused the ipn/e2e_test to catch
pingers in its resource leak check.
This commit introduces an object, but also simplifies the semantics
around the pinger's cancel functions. They no longer need to be called
while holding the mutex.
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
Specifically, this sequence:
iptables -N ts-forward
iptables -A ts-forward -m mark --mark 0x10000 -j ACCEPT
iptables -A FORWARD -j ts-forward
doesn't work on Debian-9-using-nftables, but this sequence:
iptables -N ts-forward
iptables -A FORWARD -j ts-forward
iptables -A ts-forward -m mark --mark 0x10000 -j ACCEPT
does work.
I'm sure the reason why is totally fascinating, but it's an old version
of iptables and the bug doesn't seem to exist on modern nftables, so
let's refactor our code to add rules in the always-safe order and
pretend this never happened.
Fixes#401.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
On startup, and when switching into =off and =nodivert, we were
deleting netfilter rules even if we weren't the ones that added them.
In order to avoid interfering with rules added by the sysadmin, we have
to be sure to delete rules only in the case that we added them in the
first place.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Instead of retrieving the list of chains, or the list of rules in a
chain, just try deleting the ones we don't want and then adding the
ones we do want. An error in flushing/deleting still means the rule
doesn't exist anymore, so there was no need to check for it first.
This avoids the need to parse iptables output, which avoids the need to
ever call iptables -S, which fixes#403, among other things. It's also
much more future proof in case the iptables command line changes.
Unfortunately the iptables go module doesn't properly pass the iptables
command exit code back up when doing .Delete(), so we can't correctly
check the exit code there. (exit code 1 really means the rule didn't
exist, rather than some other weird problem).
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
This removes the use of suppress_ifgroup and fwmark "x/y" notation,
which are, among other things, not available in busybox and centos6.
We also use the return codes from the 'ip' program instead of trying to
parse its output.
I also had to remove the previous hack that routed all of 100.64.0.0/10
by default, because that would add the /10 route into the 'main' route
table instead of the new table 88, which is no good. It was a terrible
hack anyway; if we wanted to capture that route, we should have
captured it explicitly as a subnet route, not as part of the addr. Note
however that this change affects all platforms, so hopefully there
won't be any surprises elsewhere.
Fixes#405
Updates #320, #144
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
