Replace %w verb with %v verb when logging errors.
Use %w only for wrapping errors with fmt.Errorf()
Fixes: #9213
Signed-off-by: Craig Rodrigues <rodrigc@crodrigues.org>
I'm not saying it works, but it compiles.
Updates #5794
Change-Id: I2f3c99732e67fe57a05edb25b758d083417f083e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Upcoming work on incremental netmap change handling will require some
replumbing of which subsystems get notified about what. Done naively,
it could break "tailscale status --json" visibility later. To make sure
I understood the flow of all the updates I was rereading the status code
and realized parts of ipnstate.Status were being populated by the wrong
subsystems.
The engine (wireguard) and magicsock (data plane, NAT traveral) should
only populate the stuff that they uniquely know. The WireGuard bits
were fine but magicsock was populating stuff stuff that LocalBackend
could've better handled, so move it there.
Updates #1909
Change-Id: I6d1b95d19a2d1b70fbb3c875fac8ea1e169e8cb0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
If we don't have the ICMP hint available, such as on Android, we can use
the signal of rx traffic to bias toward a particular endpoint.
We don't want to stick to a particular endpoint for a very long time
without any signals, so the sticky time is reduced to 1 second, which is
large enough to avoid excessive packet reordering in the common case,
but should be small enough that either rx provides a strong signal, or
we rotate in a user-interactive schedule to another endpoint, improving
the feel of failover to other endpoints.
Updates #8999
Co-authored-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
Signed-off-by: James Tucker <james@tailscale.com>
Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
There are cases where we do not detect the non-viability of a route, but
we will instead observe a failure to send. In a Disco path this would
normally be handled as a side effect of Disco, which is not available to
non-Disco WireGuard nodes. In both cases, recognizing the failure as
such will result in faster convergence.
Updates #8999
Signed-off-by: James Tucker <james@tailscale.com>
LastFullPing is now used for disco or wireguard only endpoints. This
change updates the comment to make that clear.
Updates #7826
Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
There are latency values stored in bestAddr and endpointState that are
no longer applicable after a connectivity change and should be cleared
out, following the documented behavior of the function.
Updates #8999
Signed-off-by: James Tucker <james@tailscale.com>
It was in SelfNode.Hostinfo anyway. The redundant copy was just
costing us an allocation per netmap (a Hostinfo.Clone).
Updates #1909
Change-Id: Ifac568aa5f8054d9419828489442a0f4559bc099
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
When sending a ping from the CLI, only accept a pong that is in reply
to the specific CLI ping we sent.
Updates #311
Signed-off-by: Val <valerie@tailscale.com>
Due to the conflict between our nftables implementation and ufw, which is a common utility used
on linux. We now want to take a step back to prevent regression. This will give us more chance to
let users to test our nftables support and heuristic.
Updates: #391
Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>
Now a nodeAttr: ForceBackgroundSTUN, DERPRoute, TrimWGConfig,
DisableSubnetsIfPAC, DisableUPnP.
Kept support for, but also now a NodeAttr: RandomizeClientPort.
Removed: SetForceBackgroundSTUN, SetRandomizeClientPort (both never
used, sadly... never got around to them. But nodeAttrs are better
anyway), EnableSilentDisco (will be a nodeAttr later when that effort
resumes).
Updates #8923
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
To record wether user is using iptables or nftables after we add support to nftables on linux, we
are adding a field FirewallMode to NetInfo in HostInfo to reflect what firewall mode the host is
running, and form metrics. The information is gained from a global constant in hostinfo.go. We
set it when selection heuristic made the decision, and magicsock reports this to control.
Updates: tailscale/corp#13943
Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>
If a node is flapping or otherwise generating lots of STUN endpoints, we
can end up caching a ton of useless values and sending them to peers.
Instead, let's apply a fixed per-Addr limit of endpoints that we cache,
so that we're only sending peers up to the N most recent.
Updates tailscale/corp#13890
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I8079a05b44220c46da55016c0e5fc96dd2135ef8
The current router errors out when neither iptables nor nftables support is present. We
should fall back to the previous behaviour which we creates a dummy iptablesRunner.
Fixes: #8878
Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>
Netcheck no longer performs I/O itself, instead it makes requests via
SendPacket and expects users to route reply traffic to
ReceiveSTUNPacket.
Netcheck gains a Standalone function that stands up sockets and
goroutines to implement I/O when used in a standalone fashion.
Magicsock now unconditionally routes STUN traffic to the netcheck.Client
that it hosts, and plumbs the send packet sink.
The CLI is updated to make use of the Standalone mode.
Fixes#8723
Signed-off-by: James Tucker <james@tailscale.com>
This sets the Don't Fragment flag, for now behind the
TS_DEBUG_ENABLE_PMTUD envknob.
Updates #311.
Signed-off-by: Val <valerie@tailscale.com>
Signed-off-by: salman <salman@tailscale.com>
This commit replaces the TS_DEBUG_USE_NETLINK_NFTABLES envknob with
a TS_DEBUG_FIREWALL_MODE that should be set to either 'iptables' or
'nftables' to select firewall mode manually, other wise tailscaled
will automatically choose between iptables and nftables depending on
environment and system availability.
updates: #319
Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>
This adds the capability to pad disco ping message payloads to reach a
specified size. It also plumbs it through to the tailscale ping -size
flag.
Disco pings used for actual endpoint discovery do not use this yet.
Updates #311.
Signed-off-by: salman <salman@tailscale.com>
Co-authored-by: Val <valerie@tailscale.com>
The nonce value is not read by anything, and di.sharedKey.Seal()
a few lines below generates its own. #cleanup
Signed-off-by: salman <salman@tailscale.com>
Define PeerCapabilty and PeerCapMap as the new way of sending down
inter-peer capability information.
Previously, this was unstructured and you could only send down strings
which got too limiting for certain usecases. Instead add the ability
to send down raw JSON messages that are opaque to Tailscale but provide
the applications to define them however they wish.
Also update accessors to use the new values.
Updates #4217
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This commit adds nftable rule injection for tailscaled. If tailscaled is
started with envknob TS_DEBUG_USE_NETLINK_NFTABLES = true, the router
will use nftables to manage firewall rules.
Updates: #391
Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>
The MacOS client can't set the MTU when creating the tun due to lack
of permissions, so add it to the router config and have MacOS set it
in the callback using a method that it does have permissions for.
Updates #8219
Signed-off-by: Val <valerie@tailscale.com>
This allows sending logs from the "logpolicy" package (and associated
callees) to something other than the log package. The behaviour for
tailscaled remains the same, passing in log.Printf
Updates #8249
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ie1d43b75fa7281933d9225bffd388462c08a5f31
The server hasn't sent it in ages.
Updates #cleanup
Change-Id: I9695ab0f074ec6fb006e11faf3cdfc5ca049fbf8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This change removes the noV4/noV6 check from addrForSendWireGuardLocked.
On Android, the client panics when reaching `rand.Intn()`, likely due to
the candidates list being containing no candidates. The suspicion is
that the `noV4` and the `noV6` are both being triggered causing the
loop to continue.
Updates tailscale/corp#12938
Updates #7826
Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
This change is introducing new netfilterRunner interface and moving iptables manipulation to a lower leveled iptables runner.
For #391
Signed-off-by: KevinLiang10 <kevinliang@tailscale.com>
The netstack code had a bunch of logic to figure out if the LocalBackend should handle an
incoming connection and then would call the function directly on LocalBackend. Move that
logic to LocalBackend and refactor the methods to return conn handlers.
Updates #cleanup
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Switch our best address selection to use a scoring-based approach, where
we boost each address based on whether it's a private IP or IPv6.
For users in cloud environments, this biases endpoint selection towards
using an endpoint that is less likely to cost the user money, and should
be less surprising to users.
This also involves updating the tests to not use private IPv4 addresses;
other than that change, the behaviour should be identical for existing
endpoints.
Updates #8097
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I069e3b399daea28be66b81f7e44fc27b2943d8af
Various BSD-derived operating systems including macOS and FreeBSD
require that ping6 be used for IPv6 destinations. The "ping" command
does not understand an IPv6 destination.
FreeBSD 13.x and later do handle IPv6 in the regular ping command,
but also retain a ping6 command. We use ping6 on all versions of
FreeBSD.
Fixes https://github.com/tailscale/tailscale/issues/8225
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
This is part of an effort to clean up tailscaled initialization between
tailscaled, tailscaled Windows service, tsnet, and the mac GUI.
Updates #8036
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This change introduces address selection for wireguard only endpoints.
If a endpoint has not been used before, an address is randomly selected
to be used based on information we know about, such as if they are able
to use IPv4 or IPv6. When an address is initially selected, we also
initiate a new ICMP ping to the endpoints addresses to determine which
endpoint offers the best latency. This information is then used to
update which endpoint we should be using based on the best possible
route. If the latency is the same for a IPv4 and an IPv6 address, IPv6
will be used.
Updates #7826
Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
Avoid selecting an endpoint as "better" than the current endpoint if the
total latency improvement is less than 1%. This adds some hysteresis to
avoid flapping between endpoints for a minimal improvement in latency.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: If8312e1768ea65c4b4d4e13d8de284b3825d7a73
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>
We're using it in more and more places, and it's not really specific to
our use of Wireguard (and does more just link/interface monitoring).
Also removes the separate interface we had for it in sockstats -- it's
a small enough package (we already pull in all of its dependencies
via other paths) that it's not worth the extra complexity.
Updates #7621
Updates #7850
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
This is a follow-up to #7905 that adds two more linters and fixes the corresponding findings. As per the previous PR, this only flags things that are "obviously" wrong, and fixes the issues found.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I8739bdb7bc4f75666a7385a7a26d56ec13741b7c
This adds an initial and intentionally minimal configuration for
golang-ci, fixes the issues reported, and adds a GitHub Action to check
new pull requests against this linter configuration.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I8f38fbc315836a19a094d0d3e986758b9313f163
TestMonitorMode skips by default, without the --monitor flag, and then
it previously ran forever. This adds an option --monitor-duration flag
that defaults to zero (run forever) but if non-zero bounds how long
the tests runs. This means you can then also use e.g. `go test
--cpuprofile` and capture a CPU/mem profile for a minute or two.
Updates #7621
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Previously, when updating endpoints we would immediately stop
advertising any endpoint that wasn't discovered during
determineEndpoints. This could result in, for example, a case where we
performed an incremental netcheck, didn't get any of our three STUN
packets back, and then dropped our STUN endpoint from the set of
advertised endpoints... which would result in clients falling back to a
DERP connection until the next call to determineEndpoints.
Instead, let's cache endpoints that we've discovered and continue
reporting them to clients until a timeout expires. In the above case
where we temporarily don't have a discovered STUN endpoint, we would
continue reporting the old value, then re-discover the STUN endpoint
again and continue reporting it as normal, so clients never see a
withdrawal.
Updates tailscale/coral#108
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I42de72e7418ab328a6c732bdefc74549708cf8b9
The comment still said *magicsock.Conn implemented wireguard-go conn.Bind.
That wasn't accurate anymore.
A doc #cleanup.
Change-Id: I7fd003b939497889cc81147bfb937b93e4f6865c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
So we're staying within the netip.Addr/AddrPort consistently and
avoiding allocs/conversions to the legacy net addr types.
Updates #5162
Change-Id: I59feba60d3de39f773e68292d759766bac98c917
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
These tests are passing locally and on CI. They had failed earlier in
the day when first fixing up CI, and it is not immediately clear why. I
have cycled IPv6 support locally, but this should not have a substantial
effect.
Updates #7876
Signed-off-by: James Tucker <jftucker@gmail.com>
We accidentally switched to ./tool/go in
4022796484 which resulted in no longer
running Windows builds, as this is attempting to run a bash script.
I was unable to quickly fix the various tests that have regressed, so
instead I've added skips referencing #7876, which we need to back and
fix.
Updates #7262
Updates #7876
Signed-off-by: James Tucker <james@tailscale.com>
This also adds a bunch of tests for this function to ensure that we're
returning the proper IP(s) in all cases.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I0d9d57170dbab5f2bf07abdf78ecd17e0e635399
This makes `omitempty` actually work, and saves bytes in each map response.
Updates tailscale/corp#8020
Signed-off-by: Maisem Ali <maisem@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>
The lazy initialization of the disco key is not necessary, and
contributes to unnecessary locking and state checking.
Updates #cleanup
Signed-off-by: James Tucker <james@tailscale.com>
Running tailscaled with the race detector enabled immediately fires on
this field, as it is updated after first read.
Updates #cleanup
Signed-off-by: James Tucker <james@tailscale.com>
A peer can have IsWireGuardOnly, which means it will not support DERP or
Disco, and it must have Endpoints filled in order to be usable.
In the present implementation only the first Endpoint will be used as
the bestAddr.
Updates tailscale/corp#10351
Co-authored-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
Co-authored-by: James Tucker <james@tailscale.com>
Signed-off-by: James Tucker <james@tailscale.com>
Identified in review in #7821 endpoint.discoKey and endpoint.discoShort
are often accessed without first taking endpoint.mu. The arrangement
with endpoint.mu is inconvenient for a good number of those call-sites,
so it is instead replaced with an atomic pointer to carry both pieces of
disco info. This will also help with #7821 that wants to add explicit
checks/guards to disable disco behaviors when disco keys are missing
which is necessarily implicitly mostly covered by this change.
Updates #7821
Signed-off-by: James Tucker <james@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>
This commit implements UDP offloading for Linux. GSO size is passed to
and from the kernel via socket control messages. Support is probed at
runtime.
UDP GSO is dependent on checksum offload support on the egress netdev.
UDP GSO will be disabled in the event sendmmsg() returns EIO, which is
a strong signal that the egress netdev does not support checksum
offload.
Updates tailscale/corp#8734
Signed-off-by: Jordan Whited <jordan@tailscale.com>
This adds the util/sysresources package, which currently only contains a
function to return the total memory size of the current system.
Then, we modify magicsock to scale the number of buffered DERP messages
based on the system's available memory, ensuring that we never use a
value lower than the previous constant of 32.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ib763c877de4d0d4ee88869078e7d512f6a3a148d
Noted on #5915 TS_DEBUG_MTU was not used consistently everywhere.
Extract the default into a function that can apply this centrally and
use it everywhere.
Added envknob.Lookup{Int,Uint}Sized to make it easier to keep CodeQL
happy when using converted values.
Updates #5915
Signed-off-by: James Tucker <james@tailscale.com>
This adds support to make exit nodes and subnet routers work
when in scenarios where NAT is required.
It also updates the NATConfig to be generated from a `wgcfg.Config` as
that handles merging prefs with the netmap, so it has the required information
about whether an exit node is already configured and whether routes are accepted.
Updates tailscale/corp#8020
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Use the local context on Impl to check for shut down state in order to
drop rather than inject packets after close has begun.
Netstack sets endpoint.dispatcher to nil during shutdown. After the
recent adjustment in 920ec69241 we now
wait for netstack to fully shutdown before we release tests. This means
that we may continue to accept packets and attempt to inject them, which
we must prevent in order to avoid nil pointer panic.
References google/gvisor#8765Fixes#7715
Signed-off-by: James Tucker <james@tailscale.com>
This commit updates the wireguard-go dependency to pull in fixes for
the tun package, specifically 052af4a and aad7fca.
Signed-off-by: Jordan Whited <jordan@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 adds support in tstun to utitilize the SelfNodeV4MasqAddrForThisPeer and
perform the necessary modifications to the packet as it passes through tstun.
Currently this only handles ICMP, UDP and TCP traffic.
Subnet routers and Exit Nodes are also unsupported.
Updates tailscale/corp#8020
Co-authored-by: Melanie Warrick <warrick@tailscale.com>
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This change focuses on the backend log ID, which is the mostly commonly
used in the client. Tests which don't seem to make use of the log ID
just use the zero value.
Signed-off-by: Will Norris <will@tailscale.com>
Previously, it would accept all TCP connections and then close the ones
it did not care about. Make it only ever accept the connections that it
cares about.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This change adds a ringbuffer to each magicsock endpoint that keeps a
fixed set of "changes"–debug information about what updates have been
made to that endpoint.
Additionally, this adds a LocalAPI endpoint and associated
"debug peer-status" CLI subcommand to fetch the set of changes for a given
IP or hostname.
Updates tailscale/corp#9364
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I34f726a71bddd0dfa36ec05ebafffb24f6e0516a
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>
The debug flag on tailscaled isn't available in the macOS App Store
build, since we don't have a tailscaled binary; move it to the
'tailscale debug' CLI that is available on all platforms instead,
accessed over LocalAPI.
Updates #7377
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I47bffe4461e036fab577c2e51e173f4003592ff7
Followup to #7177 to avoid adding extra dependencies to the CLI. We
instead declare an interface for the link monitor.
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
WSL has started to set the eth0 default route interface default to 1280
MTU, which is too low to carry 1280 byte packets from tailscale0 once
wrapped in WireGuard. The change down to 1280 is very likely smaller
than necessary for almost all users. We can not easily determine the
ideal MTU, but if all the preconditions match, we raise the MTU to 1360,
which is just enough for Tailscale traffic to work.
Updates #4833
Updates #7346
Signed-off-by: James Tucker <james@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>
The log ID types were moved to a separate package so that
code that only depend on log ID types do not need to link
in the logic for the logtail client itself.
Not all code need the logtail client.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
To aid in debugging where a customer has static port-forwards set up and
there are issues establishing a connection through that port.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ic5558bcdb40c9119b83f79dcacf2233b07777f2a
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>
The single packet WriteTo() through RebindingUDPConn.WriteBatch() was
not checking for a rebind between loading the PacketConn and writing to
it. Same with ReadFrom()/ReadBatch().
Fixes#6989
Signed-off-by: Jordan Whited <jordan@tailscale.com>
When you hit control-C on a tailscaled (notably in dev mode, but
also on any systemctl stop/restart), there is a flood of messages like:
magicsock: doing cleanup for discovery key d:aa9c92321db0807f
magicsock: doing cleanup for discovery key d:bb0f16aacadbfd46
magicsock: doing cleanup for discovery key d:b5b2d386296536f2
magicsock: doing cleanup for discovery key d:3b640649f6796c91
magicsock: doing cleanup for discovery key d:71d7b1afbcce52cd
magicsock: doing cleanup for discovery key d:315b61d7e0111377
magicsock: doing cleanup for discovery key d:9301f63dce69bf45
magicsock: doing cleanup for discovery key d:376141884d6fe072
....
It can be hundreds or even tens of thousands.
So don't do that. Not a useful log message during shutdown.
Change-Id: I029a8510741023f740877df28adff778246c18e5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
01b90df2fa added SCTP support before
(with explicit parsing for ports) and
69de3bf7bf tried to add support for
arbitrary IP protocols (as long as the ACL permited a port of "*",
since we might not know how to find ports from an arbitrary IP
protocol, if it even has such a concept). But apparently that latter
commit wasn't tested end-to-end enough. It had a lot of tests, but the
tests made assumptions about layering that either weren't true, or
regressed since 1.20. Notably, it didn't remove the (*Filter).pre
bidirectional filter that dropped all "unknown" protocol packets both
leaving and entering, even if there were explicit protocol matches
allowing them in.
Also, don't map all unknown protocols to 0. Keep their IP protocol
number parsed so it's matchable by later layers. Only reject illegal
things.
Fixes#6423
Updates #2162
Updates #2163
Change-Id: I9659b3ece86f4db51d644f9b34df78821758842c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
With a42a594bb3, iOS uses netstack and
hence there are no longer any platforms which use the legacy MagicDNS path. As such, we remove it.
We also normalize the limit for max in-flight DNS queries on iOS (it was 64, now its 256 as per other platforms).
It was 64 for the sake of being cautious about memory, but now we have 50Mb (iOS-15 and greater) instead of 15Mb
so we have the spare headroom.
Signed-off-by: Tom DNetto <tom@tailscale.com>
The macOS client was forgetting to call netstack.Impl.SetLocalBackend.
Change the API so that it can't be started without one, eliminating this
class of bug. Then update all the callers.
Updates #6764
Change-Id: I2b3a4f31fdfd9fdbbbbfe25a42db0c505373562f
Signed-off-by: Claire Wang <claire@tailscale.com>
Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
These aren't handled, but it's not an error to get one.
Fixes#6806
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I1fcb9032ac36420aa72a048bf26f58360b9461f9
The Tailscale logging service has a hard limit on the maximum
log message size that can be accepted.
We want to ensure that netlog messages never exceed
this limit otherwise a client cannot transmit logs.
Move the goroutine for periodically dumping netlog messages
from wgengine/netlog to net/connstats.
This allows net/connstats to manage when it dumps messages,
either based on time or by size.
Updates tailscale/corp#8427
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
It's possible for the 'somethingChanged' callback to be registered and
then trigger before the ctx field is assigned; move the assignment
earlier so this can't happen.
Change-Id: Ia7ee8b937299014a083ab40adf31a8b3e0db4ec5
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
This is temporary while we work to upstream performance work in
https://github.com/WireGuard/wireguard-go/pull/64. A replace directive
is less ideal as it breaks dependent code without duplication of the
directive.
Signed-off-by: Jordan Whited <jordan@tailscale.com>
Adjust the expected system output by removing the unsupported mask
component including and after the slash in expected output like:
fwmask 0xabc/0xdef
This package's tests now pass in an Alpine container when the 'go' and
'iptables' packages are installed (and run as privileged so /dev/net/tun
exists).
Fixes#5928
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Id1a3896282bfa36b64afaec7a47205e63ad88542
This commit updates the wireguard-go dependency and implements the
necessary changes to the tun.Device and conn.Bind implementations to
support passing vectors of packets in tailscaled. This significantly
improves throughput performance on Linux.
Updates #414
Signed-off-by: Jordan Whited <jordan@tailscale.com>
Signed-off-by: James Tucker <james@tailscale.com>
Co-authored-by: James Tucker <james@tailscale.com>
We would replace the existing real implementation of nettype.PacketConn
with a blockForeverConn, but that violates the contract of atomic.Value
(where the type cannot change). Fix by switching to a pointer value
(atomic.Pointer[nettype.PacketConn]).
A longstanding issue, but became more prevalent when we started binding
connections to interfaces on macOS and iOS (#6566), which could lead to
the bind call failing if the interface was no longer available.
Fixes#6641
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Previously, tstun.Wrapper and magicsock.Conn managed their
own statistics data structure and relied on an external call to
Extract to extract (and reset) the statistics.
This makes it difficult to ensure a maximum size on the statistics
as the caller has no introspection into whether the number
of unique connections is getting too large.
Invert the control flow such that a *connstats.Statistics
is registered with tstun.Wrapper and magicsock.Conn.
Methods on non-nil *connstats.Statistics are called for every packet.
This allows the implementation of connstats.Statistics (in the future)
to better control when it needs to flush to ensure
bounds on maximum sizes.
The value registered into tstun.Wrapper and magicsock.Conn could
be an interface, but that has two performance detriments:
1. Method calls on interface values are more expensive since
they must go through a virtual method dispatch.
2. The implementation would need a sync.Mutex to protect the
statistics value instead of using an atomic.Pointer.
Given that methods on constats.Statistics are called for every packet,
we want reduce the CPU cost on this hot path.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Many packages reference the logtail ID types,
but unfortunately pull in the transitive dependencies of logtail.
Fix this problem by putting the log ID types in its own package
with minimal dependencies.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
We use this pattern in a number of places (in this repo and elsewhere)
and I was about to add a fourth to this repo which was crossing the line.
Add this type instead so they're all the same.
Also, we have another Set type (SliceSet, which tracks its keys in
order) in another repo we can move to this package later.
Change-Id: Ibbdcdba5443fae9b6956f63990bdb9e9443cefa9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This patch removes the crappy, half-backed COM initialization used by `go-ole`
and replaces that with the `StartRuntime` function from `wingoes`, a library I
have started which, among other things, initializes COM properly.
In particular, we should always be initializing COM to use the multithreaded
apartment. Every single OS thread in the process becomes implicitly initialized
as part of the MTA, so we do not need to concern ourselves as to whether or not
any particular OS thread has initialized COM. Furthermore, we no longer need to
lock the OS thread when calling methods on COM interfaces.
Single-threaded apartments are designed solely for working with Win32 threads
that have a message pump; any other use of the STA is invalid.
Fixes https://github.com/tailscale/tailscale/issues/3137
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
There aren't any in the wild, other than one we ran on purpose to keep
us honest, but we can bump that one forward to 0.100.
Change-Id: I129e70724b2d3f8edf3b496dc01eba3ac5a2a907
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This renames canP2P in magicsock to canP2PLocked to reflect
expectation of mutex lock, fixes a race we discovered in the meantime,
and updates the current stats.
Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: Jenny Zhang <jz@tailscale.com>
The health package was turning into a rando dumping ground. Make a new
Warnable type instead that callers can request an instance of, and
then Set it locally in their code without the health package being
aware of all the things that are warnable. (For plenty of things the
health package will want to know details of how Tailscale works so it
can better prioritize/suppress errors, but lots of the warnings are
pretty leaf-y and unrelated)
This just moves two of the health warnings. Can probably move more
later.
Change-Id: I51e50e46eb633f4e96ced503d3b18a1891de1452
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Noticed when testing FUS on tailscale-on-macOS, that routing would break
completely when switching between profiles. However, it would start working
again when going back to the original profile tailscaled started with.
Turns out that if we change the addrs on the interface we need to remove and readd
all the routes.
Updates #713
Signed-off-by: Maisem Ali <maisem@tailscale.com>
All IPv6 packets for the self address were doing netip.Prefix.Contains
lookups.
If if we know they're for a self address (which we already previously
computed and have sitting in a bool), then they can't be for a 4via6
range.
Change-Id: Iaaaf1248cb3fecec229935a80548ead0eb4cb892
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Inspired by #6235, let's explicitly test the behaviour of this function
to ensure that we're not processing things we don't expect to.
Change-Id: I158050a63be7410fb99452089ea607aaf89fe91a
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
It was eating TCP packets to peerapi ports to subnet routers. Some of
the TCP flow's packets went onward, some got eaten. So some TCP flows
to subnet routers, if they used an unfortunate TCP port number, got
broken.
Change-Id: Ifea036119ccfb081f4dfa18b892373416a5239f8
Signed-off-by: Brad Fitzpatrick <bradfitz@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>
It's leftover from an earlier Tailscale SSH wiring and I forgot to
delete it apparently.
Change-Id: I14f071f450e272b98d90080a71ce68ba459168d1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Exit node traffic is aggregated to protect the privacy
of those using an exit node. However, it is reasonable to
at least log which nodes are making most use of an exit node.
For a node using an exit node,
the source will be the taiscale IP address of itself,
while the destination will be zeroed out.
For a node that serves as an exit node,
the source will be zeroed out,
while the destination will be tailscale IP address
of the node that initiated the exit traffic.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
In the future this will cause a node to be unable to join the tailnet
if network logging is enabled.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Setting TCP KeepAlives for Tailscale SSH connections results in them
unnecessarily disconnecting. However, we can't turn them off completely
as that would mean we start leaking sessions waiting for a peer to come
back which may have gone away forever (e.g. if the node was deleted from
the tailnet during a session).
Updates #5021
Signed-off-by: Maisem Ali <maisem@tailscale.com>
If the network logging configruation changes (and nothing else)
we will tear down the network logger and start it back up.
However, doing so will lose the router configuration state.
Manually reconfigure it with the routing state.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This is a temporary hack to prevent logtail getting stuck
uploading the same excessive message over and over.
A better solution will be discussed and implemented.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
There is utility in logging traffic statistics that occurs at the physical layer.
That is, in order to send packets virtually to a particular tailscale IP address,
what physical endpoints did we need to communicate with?
This functionality logs IP addresses identical to
what had always been logged in magicsock prior to #5823,
so there is no increase in PII being logged.
ExtractStatistics returns a mapping of connections to counts.
The source is always a Tailscale IP address (without port),
while the destination is some endpoint reachable on WAN or LAN.
As a special case, traffic routed through DERP will use 127.3.3.40
as the destination address with the port being the DERP region.
This entire feature is only enabled if data-plane audit logging
is enabled on the tailnet (by default it is disabled).
Example of type of information logged:
------------------------------------ Tx[P/s] Tx[B/s] Rx[P/s] Rx[B/s]
PhysicalTraffic: 25.80 3.39Ki 38.80 5.57Ki
100.1.2.3 -> 143.11.22.33:41641 15.40 2.00Ki 23.20 3.37Ki
100.4.5.6 -> 192.168.0.100:41641 10.20 1.38Ki 15.60 2.20Ki
100.7.8.9 -> 127.3.3.40:2 0.20 6.40 0.00 0.00
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The netlog.Message type is useful to depend on from other packages,
but doing so would transitively cause gvisor and other large packages
to be linked in.
Avoid this problem by moving all network logging types to a single package.
We also update staticcheck to take in:
003d277bcf
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Intermittently in the wild we are seeing failures when calling
`INetworkConnection::GetNetwork`. It is unclear what the root cause is, but what
is clear is that the error is happening inside the object's `IDispatch` invoker
(as opposed to the method implementation itself).
This patch replaces our wrapper for `INetworkConnection::GetNetwork` with an
alternate implementation that directly invokes the method, instead of using
`IDispatch`. I also replaced the implementations of `INetwork::SetCategory` and
`INetwork::GetCategory` while I was there.
This patch is speculative and tightly-scoped so that we could possibly add it
to a dot-release if necessary.
Updates https://github.com/tailscale/tailscale/issues/4134
Updates https://github.com/tailscale/tailscale/issues/6037
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
TCP selective acknowledgement can improve throughput by an order
of magnitude in the presence of loss.
Signed-off-by: Jordan Whited <jordan@tailscale.com>
We had previously added this to the netcheck report in #5087 but never
copied it into the NetInfo struct. Additionally, add it to log lines so
it's visible to support.
Change-Id: Ib6266f7c6aeb2eb2a28922aeafd950fe1bf5627e
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
Deleting may temporarily result in no addrs on the interface, which results in
all other rules (like routes) to get dropped by the OS.
I verified this fixes the problem.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Sets up new file for separate silent disco goroutine, tentatively named
pathfinder for now.
Updates #540
Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: Jenny Zhang <jz@tailscale.com>
During development of silent disco (#540), an alternate send policy
for magicsock that doesn't wake up the radio frequently with
heartbeats, we want the old & new policies to coexist, like we did
previously pre- and post-disco.
We started to do that earlier in 5c42990c2f but only set up the
env+control knob plumbing to set a bool about which path should be
used.
This starts to add a way for the silent disco code to update the send
path from a separate goroutine. (Part of the effort is going to
de-state-machinify the event based soup that is the current disco
code and make it more Go synchronous style.)
So far this does nothing. (It does add an atomic load on each send
but that should be noise in the grand scheme of things, and a even more
rare atomic store of nil on node config changes.)
Baby steps.
Updates #540
Co-authored-by: Jenny Zhang <jz@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The wireguard-go code unfortunately calls this unconditionally
even when verbose logging is disabled.
Partial revert of #5911.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This field seems seldom used and the documentation is wrong.
It is simpler to just derive its original value dynamically
when endpoint.DstToString is called.
This method is potentially used by wireguard-go,
but not in any code path is performance sensitive.
All calls to it use it in conjunction with fmt.Printf,
which is going to be slow anyways since it uses Go reflection.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
- At high data rates more buffer space is required in order to avoid
packet loss during any cause of delay.
- On slower machines more buffer space is required in order to avoid
packet loss while decryption & tun writing is underway.
- On higher latency network paths more buffer space is required in order
to overcome BDP.
- On Linux set with SO_*BUFFORCE to bypass net.core.{r,w}mem_max.
- 7MB is the current default maximum on macOS 12.6
- Windows test is omitted, as Windows does not support getsockopt for
these options.
Signed-off-by: James Tucker <james@tailscale.com>
Always set the MTU to the Tailscale default MTU. In practice we are
missing applying an MTU for IPv6 on Windows prior to this patch.
This is the simplest patch to fix the problem, the code in here needs
some more refactoring.
Fixes#5914
Signed-off-by: James Tucker <james@tailscale.com>
This sets up Logger to handle statistics at the magicsock layer,
where we can correlate traffic between a particular tailscale IP address
and any number of physical endpoints used to contact the node
that hosts that tailscale address.
We also export Message and TupleCounts to better document the JSON format
that is being sent to the logging infrastructure.
This commit does NOT yet enable the actual logging of magicsock statistics.
That will be a future commit.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
If the wgcfg.Config is specified with network logging arguments,
then Userspace.Reconfig starts up an asynchronous network logger,
which is shutdown either upon Userspace.Close or when Userspace.Reconfig
is called again without network logging or route arguments.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The Logger type managers a logtail.Logger for extracting
statistics from a tstun.Wrapper.
So long as Shutdown is called, it ensures that logtail
and statistic gathering resources are properly cleared up.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The node and domain audit log IDs are provided in the map response,
but are ultimately going to be used in wgengine since
that's the layer that manages the tstun.Wrapper.
Do the plumbing work to get this field passed down the stack.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
And add a CLI/localapi and c2n mechanism to enable it for a fixed
amount of time.
Updates #1548
Change-Id: I71674aaf959a9c6761ff33bbf4a417ffd42195a7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Most visible when using tsnet.Server, but could have resulted in dropped
messages in a few other places too.
Fixes#5743
Signed-off-by: Mihai Parparita <mihai@tailscale.com>
Context: https://github.com/tailscale/tailscale/pull/5588#issuecomment-1260655929
It seems that if the interface at index 1 is down, the rule is not installed. As such,
we increase the range we detect up to 2004 in the hope that at least one of the interfaces
1-4 will be up.
Signed-off-by: Tom DNetto <tom@tailscale.com>
This fixes a race condition which caused `c.muCond.Broadcast()` to
never fire in the `firstDerp` if block. It resulted in `Close()`
hanging forever.
Signed-off-by: Kyle Carberry <kyle@carberry.com>
As the comment in the code says, netstack should always respond to ICMP
echo requests to a 4via6 address, even if the netstack instance isn't
normally processing subnet traffic.
Follow-up to #5709
Change-Id: I504d0776c5824071b2a2e0e687bc33e24f6c4746
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
From 5c42990c2f, not yet released in a stable build.
Caught by existing tests.
Fixes#5685
Change-Id: Ia76bb328809d9644e8b96910767facf627830600
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Baby steps towards turning off heartbeat pings entirely as per #540.
This doesn't change any current magicsock functionality and requires additional
changes to send/disco paths before the flag can be turned on.
Updates #540
Change-Id: Idc9a72748e74145b068d67e6dd4a4ffe3932efd0
Signed-off-by: Jenny Zhang <jz@tailscale.com>
Signed-off-by: Jenny Zhang <jz@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>
This change masks the bitspace used when setting and querying the fwmark on packets. This allows
tailscaled to play nicer with other networking software on the host, assuming the other networking
software is also using fwmarks & a different mask.
IPTables / mark module has always supported masks, so this is safe on the netfilter front.
However, busybox only gained support for parsing + setting masks in 1.33.0, so we make sure we
arent such a version before we add the "/<mask>" syntax to an ip rule command.
Signed-off-by: Tom DNetto <tom@tailscale.com>
Fixes an panic in `(*magicsock.Conn).ServeHTTPDebug` when the
`recentPongs` ring buffer for an endpoint wraps around.
Signed-off-by: Colin Adler <colin1adler@gmail.com>
If we accept a forwarded TCP connection before dialing, we can
erroneously signal to a client that we support IPv6 (or IPv4) without
that actually being possible. Instead, we only complete the client's TCP
handshake after we've dialed the outbound connection; if that fails, we
respond with a RST.
Updates #5425 (maybe fixes!)
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
Incoming disco packets are now dropped unless they match one of the
current bound ports, or have a zero port*.
The BPF filter passes all packets with a disco header to the raw packet
sockets regardless of destination port (in order to avoid needing to
reconfigure BPF on rebind).
If a BPF enabled node has just rebound, due to restart or rebind, it may
receive and reply to disco ping packets destined for ports other than
those which are presently bound. If the pong is accepted, the pinging
node will now assume that it can send WireGuard traffic to the pinged
port - such traffic will not reach the node as it is not destined for a
bound port.
*The zero port is ignored, if received. This is a speculative defense
and would indicate a problem in the receive path, or the BPF filter.
This condition is allowed to pass as it may enable traffic to flow,
however it will also enable problems with the same symptoms this patch
otherwise fixes.
Fixes#5536
Signed-off-by: James Tucker <james@tailscale.com>
Craig Rodrigues