The derphttp client automatically reconnects upon failure.
RunWatchConnectionLoop called derphttp.Client.WatchConnectionChanges
once, but that wrapper method called the underlying
derp.Client.WatchConnectionChanges exactly once on derphttp.Client's
currently active connection. If there's a failure, we need to re-subscribe
upon all reconnections.
This removes the derphttp.Client.WatchConnectionChanges method, which
was basically impossible to use correctly, and changes it to be a
boolean field on derphttp.Client alongside MeshKey and IsProber. Then
it moves the call to the underlying derp.Client.WatchConnectionChanges
to derphttp's client connection code, so it's resubscribed on any
reconnect.
Some paranoia is then added to make sure people hold the API right,
not calling derphttp.Client.RunWatchConnectionLoop on an
already-started Client without having set the bool to true. (But still
auto-setting it to true if that's the first method that's been called
on that derphttp.Client, as is commonly the case, and prevents
existing code from breaking)
Fixestailscale/corp#9916
Supercedes tailscale/tailscale#9719
Co-authored-by: Val <valerie@tailscale.com>
Co-authored-by: Irbe Krumina <irbe@tailscale.com>
Co-authored-by: Anton Tolchanov <anton@tailscale.com>
Signed-off-by: Brad Fitzpatrick <brad@danga.com>
When trying to set up multiple derper instances meshing with each
other, it turned out that while one can specify an alternative
listening port using the -a flag, the TLS hostname gets incorrectly
determined and includes the set alternative listening port as part of
the hostname. Thus, the TLS hostname validation always fails when the
-mesh-with values have ports.
Updates #9949
Signed-off-by: Thomas Kosiewski <thomas.kosiewski@loft.sh>
This makes wsconn.Conns somewhat present reasonably when they are
the client of an http.Request, rather than just put a placeholder
in that field.
Updates tailscale/corp#13777
Signed-off-by: David Anderson <danderson@tailscale.com>
Named result meant error paths assigned that variable to nil.
But a goroutine was concurrently using that variable.
Don't use a named result for that first parameter. Then then return
paths don't overwrite it.
Fixes#9129
Change-Id: Ie57f99d40ca8110085097780686d9bd620aaf160
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Like net/http.Server.BaseContext, this lets callers specify a base
context for dials.
Updates tailscale/corp#12702
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
~97% of the log messages derper outputs are related to the normal
non-error state of a client disconnecting in some manner. Add a
verbose logging feature that only logs these messages when enabled.
Fixes#8024
Signed-off-by: Val <valerie@tailscale.com>
On some platforms (notably macOS and iOS) we look up the default
interface to bind outgoing connections to. This is both duplicated
work and results in logspam when the default interface is not available
(i.e. when a phone has no connectivity, we log an error and thus cause
more things that we will try to upload and fail).
Fixed by passing around a netmon.Monitor to more places, so that we can
use its cached interface state.
Fixes#7850
Updates #7621
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Using log.Printf may end up being printed out to the console, which
is not desirable. I noticed this when I was investigating some client
logs with `sockstats: trace "NetcheckClient" was overwritten by another`.
That turns to be harmless/expected (the netcheck client will fall back
to the DERP client in some cases, which does its own sockstats trace).
However, the log output could be visible to users if running the
`tailscale netcheck` CLI command, which would be needlessly confusing.
Updates tailscale/corp#9230
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Make developing derp easier by:
1. Creating an envknob telling clients to use HTTP to connect to derp
servers, so devs don't have to acquire a valid TLS cert.
2. Creating an envknob telling clients which derp server to connect
to, so devs don't have to edit the ACLs in the admin console to add a
custom DERP map.
3. Explaining how the -dev and -a command lines args to derper
interact.
To use this:
1. Run derper with -dev.
2. Run tailscaled with TS_DEBUG_USE_DERP_HTTP=1 and
TS_DEBUG_USE_DERP_ADDR=localhost
This will result in the client connecting to derp via HTTP on port
3340.
Fixes#7700
Signed-off-by: Val <valerie@tailscale.com>
* wgengine/magicsock: add envknob to send CallMeMaybe to non-existent peer
For testing older client version responses to the PeerGone packet format change.
Updates #4326
Signed-off-by: Val <valerie@tailscale.com>
* derp: remove dead sclient struct member replaceLimiter
Leftover from an previous solution to the duplicate client problem.
Updates #2751
Signed-off-by: Val <valerie@tailscale.com>
* derp, derp/derphttp, wgengine/magicsock: add new PeerGone message type Not Here
Extend the PeerGone message type by adding a reason byte. Send a
PeerGone "Not Here" message when an endpoint sends a disco message to
a peer that this server has no record of.
Fixes#4326
Signed-off-by: Val <valerie@tailscale.com>
---------
Signed-off-by: Val <valerie@tailscale.com>
This allows tracking packet flow via logs for prober clients. Note that
the new sclient.debug() function is called on every received packet, but
will do nothing for most clients.
I have adjusted sclient logging to print public keys in short format
rather than full. This takes effect even for existing non-debug logging
(mostly client disconnect messages).
Example logs for a packet being sent from client [SbsJn] (connected to
derper [dM2E3]) to client [10WOo] (connected to derper [AVxvv]):
```
derper [dM2E3]:
derp client 10.0.0.1:35470[SbsJn]: register single client mesh("10.0.1.1"): 4 peers
derp client 10.0.0.1:35470[SbsJn]: read frame type 4 len 40 err <nil>
derp client 10.0.0.1:35470[SbsJn]: SendPacket for [10WOo], forwarding via <derphttp_client.Client [AVxvv] url=https://10.0.1.1/derp>: <nil>
derp client 10.0.0.1:35470[SbsJn]: read frame type 0 len 0 err EOF
derp client 10.0.0.1:35470[SbsJn]: read EOF
derp client 10.0.0.1:35470[SbsJn]: sender failed: context canceled
derp client 10.0.0.1:35470[SbsJn]: removing connection
derper [AVxvv]:
derp client 10.0.1.1:50650[10WOo]: register single client
derp client 10.0.1.1:50650[10WOo]: received forwarded packet from [SbsJn] via [dM2E3]
derp client 10.0.1.1:50650[10WOo]: sendPkt attempt 0 enqueued
derp client 10.0.1.1:50650[10WOo]: sendPacket from [SbsJn]: <nil>
derp client 10.0.1.1:50650[10WOo]: read frame type 0 len 0 err EOF
derp client 10.0.1.1:50650[10WOo]: read EOF
derp client 10.0.1.1:50650[10WOo]: sender failed: context canceled
derp client 10.0.1.1:50650[10WOo]: removing connection
```
Signed-off-by: Anton Tolchanov <anton@tailscale.com>
Makes it cheaper/simpler to persist values, and encourages reuse of
labels as opposed to generating an arbitrary number.
Updates tailscale/corp#9230
Updates #3363
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Uses the hooks added by tailscale/go#45 to instrument the reads and
writes on the major code paths that do network I/O in the client. The
convention is to use "<package>.<type>:<label>" as the annotation for
the responsible code path.
Enabled on iOS, macOS and Android only, since mobile platforms are the
ones we're most interested in, and we are less sensitive to any
throughput degradation due to the per-I/O callback overhead (macOS is
also enabled for ease of testing during development).
For now just exposed as counters on a /v0/sockstats PeerAPI endpoint.
We also keep track of the current interface so that we can break out
the stats by interface.
Updates tailscale/corp#9230
Updates #3363
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Updates #7123
Updates #6257 (more to do in other repos)
Change-Id: I073e2a6d81a5d7fbecc29caddb7e057ff65239d0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Update all code generation tools, and those that check for license
headers to use the new standard header.
Also update copyright statement in LICENSE file.
Fixes#6865
Signed-off-by: Will Norris <will@tailscale.com>
This updates all source files to use a new standard header for copyright
and license declaration. Notably, copyright no longer includes a date,
and we now use the standard SPDX-License-Identifier header.
This commit was done almost entirely mechanically with perl, and then
some minimal manual fixes.
Updates #6865
Signed-off-by: Will Norris <will@tailscale.com>
Instead of iterating over the map to determine the preferred forwarder
on every packet (which could happen concurrently with map mutations),
store it separately in an atomic variable.
Fixes#6445
Signed-off-by: Anton Tolchanov <anton@tailscale.com>
The //go:build syntax was introduced in Go 1.17:
https://go.dev/doc/go1.17#build-lines
gofmt has kept the +build and go:build lines in sync since
then, but enough time has passed. Time to remove them.
Done with:
perl -i -npe 's,^// \+build.*\n,,' $(git grep -l -F '+build')
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We removed it in #4806 in favor of the built-in functionality from the
nhooyr.io/websocket package. However, it has an issue with deadlines
that has not been fixed yet (see nhooyr/websocket#350). Temporarily
go back to using a custom wrapper (using the fix from our fork) so that
derpers will stop closing connections too aggressively.
Updates #5921
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Periodically poll the TCP RTT metric from all open TCP connections and
update a (bucketed) histogram metric.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I6214902196b05bf7829c9d0ea501ce0e13d984cf
NewNetcheckClient only initializes a subset of fields of derphttp.Client,
and the Close() call added by #5707 was result in a nil pointer dereference.
Make Close() safe to call when using NewNetcheckClient() too.
Fixes#5919
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
The io/ioutil package has been deprecated as of Go 1.16 [1]. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Reference: https://golang.org/doc/go1.16#ioutil
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
We can't do Noise-over-HTTP in Wasm/JS (because we don't have bidirectional
communication), but we should be able to do it over WebSockets. Reuses
derp WebSocket support that allows us to turn a WebSocket connection
into a net.Conn.
Updates #3157
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
In cases where tailscale is operating behind a MITM proxy, we need to consider
that a lot more of the internals of our HTTP requests are visible and may be
used as part of authorization checks. As such, we need to 'behave' as closely
as possible to ideal.
- Some proxies do authorization or consistency checks based the on Host header
or HTTP URI, instead of just the IP/hostname/SNI. As such, we need to
construct a `*http.Request` with a valid URI everytime HTTP is going to be
used on the wire, even if its over TLS.
Aside from the singular instance in net/netcheck, I couldn't find anywhere
else a http.Request was constructed incorrectly.
- Some proxies may deny requests, typically by returning a 403 status code. We
should not consider these requests as a valid latency check, so netcheck
semantics have been updated to consider >299 status codes as a failed probe.
Signed-off-by: Tom DNetto <tom@tailscale.com>
This allows tests to verfiy that a DERP connection was actually
established.
Related to #4326
Updates tailscale/corp#2579
Signed-off-by: Maisem Ali <maisem@tailscale.com>
A new package can also later record/report which knobs are checked and
set. It also makes the code cleaner & easier to grep for env knobs.
Change-Id: Id8a123ab7539f1fadbd27e0cbeac79c2e4f09751
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Only if the source address isn't on the currently active interface or
a ping of the DERP server fails.
Updates #3619
Change-Id: I6bf06503cff4d781f518b437c8744ac29577acc8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
So magicsock can later ask a DERP connection whether its source IP
would've changed if it reconnected.
Updates #3619
Change-Id: Ibc8810340c511d6786b60c78c1a61c09f5800e40
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Continuing work in 434af15a04, to make it possible for magicsock to
probe whether a DERP server is still there.
Updates #3619
Change-Id: I366a77c27e93b876734e64f445b85ef01eb590f2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
In prep for a future change to have client ping derp connections
when their state is questionable, rather than aggressively tearing
them down and doing a heavy reconnect when their state is unknown.
We already support ping/pong in the other direction (servers probing
clients) so we already had the two frame types, but I'd never finished
this direction.
Updates #3619
Change-Id: I024b815d9db1bc57c20f82f80f95fb55fc9e2fcc
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Otherwise random browser requests to /derp cause log spam.
Change-Id: I7bdf991d2106f0323868e651156c788a877a90d5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We still try the host's x509 roots first, but if that fails (like if
the host is old), we fall back to using LetsEncrypt's root and
retrying with that.
tlsdial was used in the three main places: logs, control, DERP. But it
was missing in dnsfallback. So added it there too, so we can run fine
now on a machine with no DNS config and no root CAs configured.
Also, move SSLKEYLOGFILE support out of DERP. tlsdial is the logical place
for that support.
Fixes#1609
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
On about 1 out of 500 runs, TestSendFreeze failed:
derp_test.go:416: bob: unexpected message type derp.PeerGoneMessage
Closing alice before bob created a race.
If bob closed promptly, the test passed.
If bob closed slowly, and alice's disappearance caused
bob to receive a PeerGoneMessage before closing, the test failed.
Deflake the test by closing bob first.
With this fix, the test passed 12,000 times locally.
Fixes#2668
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
A public key should only have max one connection to a given
DERP node (or really: one connection to a node in a region).
But if people clone their machine keys (e.g. clone their VM, Raspbery
Pi SD card, etc), then we can get into a situation where a public key
is connected multiple times.
Originally, the DERP server handled this by just kicking out a prior
connections whenever a new one came. But this led to reconnect fights
where 2+ nodes were in hard loops trying to reconnect and kicking out
their peer.
Then a909d37a59 tried to add rate
limiting to how often that dup-kicking can happen, but empirically it
just doesn't work and ~leaks a bunch of goroutines and TCP
connections, tying them up for hour+ while more and more accumulate
and waste memory. Mostly because we were doing a time.Sleep forever
while not reading from their TCP connections.
Instead, just accept multiple connections per public key but track
which is the most recent. And if two both are writing back & forth,
then optionally disable them both. That last part is only enabled in
tests for now. The current default policy is just last-sender-wins
while we gather the next round of stats.
Updates #2751
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
If a peer is connected to multiple nodes in a region (so
multiForwarder is in use) and then a node restarts and re-sends all
its additions, this bug about whether an element is in the
multiForwarder could cause a one-time flip in the which peer node we
forward to. Note a huge deal, but not written as intended.
Thanks to @lewgun for the bug report in #2141.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It was once believed that it might be useful. It wasn't. We never used it.
Remove it so we don't slowly leak memory.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The DERPTestPort int meant two things before: which port to use, and
whether to disable TLS verification. Users would like to set the port
without disabling TLS, so break it into two options.
Updates #1264
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
After allowing for custom DERP maps, it's convenient to be able to see their latency in
netcheck. This adds a query to the local tailscaled for the current DERPMap.
Updates #1264
Signed-off-by: julianknodt <julianknodt@gmail.com>
This adds a flag to the DERP server which specifies to verify clients through a local
tailscaled. It is opt-in, so should not affect existing clients, and is mainly intended for
users who want to run their own DERP servers. It assumes there is a local tailscaled running and
will attempt to hit it for peer status information.
Updates #1264
Signed-off-by: julianknodt <julianknodt@gmail.com>
Before it was using the local address and port, so fix that.
The fields in the response from `ss` are:
State, Recv-Q, Send-Q, Local Address:Port, Peer Address:Port, Process
Signed-off-by: julianknodt <julianknodt@gmail.com>
This adds a handler on the DERP server for logging bytes send and received by clients of the
server, by holding open a connection and recording if there is a difference between the number
of bytes sent and received. It sends a JSON marshalled object if there is an increase in the
number of bytes.
Signed-off-by: julianknodt <julianknodt@gmail.com>
It would be useful to know the time that packets spend inside of a queue before they are sent
off, as that can be indicative of the load the server is handling (and there was also an
existing TODO). This adds a simple exponential moving average metric to track the average packet
queue duration.
Changes during review:
Add CAS loop for recording queue timing w/ expvar.Func, rm snake_case, annotate in milliseconds,
convert
Signed-off-by: julianknodt <julianknodt@gmail.com>
