Commit Graph

153 Commits (4dfde7bffc357694a51761c8c11a23db272964c1)

Author SHA1 Message Date
Andrea Gottardo a584d04f8a
dns: increase TimeToVisible before DNS unavailable warning (#13317)
Updates tailscale/tailscale#13314

Some users are reporting 'DNS unavailable' spurious (?) warnings, especially on Android:

https://old.reddit.com/r/Tailscale/comments/1f2ow3w/health_warning_dns_unavailable_on_tailscale/
https://old.reddit.com/r/Tailscale/comments/1f3l2il/health_warnings_dns_unavailable_what_does_it_mean/

I suspect this is caused by having a too low TimeToVisible setting on the Warnable, which triggers the unhealthy state during slow network transitions.

Signed-off-by: Andrea Gottardo <andrea@gottardo.me>
3 months ago
Andrea Gottardo 5cbbb48c2e
health/dns: reduce severity of DNS unavailable warning (#13152)
`DNS unavailable` was marked as a high severity warning. On Android (and other platforms), these trigger a system notification. Here we reduce the severity level to medium. A medium severity warning will still display the warning icon on platforms with a tray icon because of the `ImpactsConnectivity=true` flag being set here, but it won't show a notification anymore. If people enter an area with bad cellular reception, they're bound to receive so many of these notifications and we need to reduce notification fatigue.

Signed-off-by: Andrea Gottardo <andrea@tailscale.com>
3 months ago
Nick Khyl f23932bd98 net/dns/resolver: log forwarded query details when TS_DEBUG_DNS_FORWARD_SEND is enabled
Troubleshooting DNS resolution issues often requires additional information.
This PR expands the effect of the TS_DEBUG_DNS_FORWARD_SEND envknob to forwarder.forwardWithDestChan,
and includes the request type, domain name length, and the first 3 bytes of the domain's SHA-256 hash in the output.

Fixes #13070

Signed-off-by: Nick Khyl <nickk@tailscale.com>
4 months ago
Jonathan Nobels 19b0c8a024
net/dns, health: raise health warning for failing forwarded DNS queries (#12888)
updates tailscale/corp#21823

Misconfigured, broken, or blocked DNS will often present as
"internet is broken'" to the end user.  This  plumbs the health tracker
into the dns manager and forwarder and adds a health warning
with a 5 second delay that is raised on failures in the forwarder and
lowered on successes.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
4 months ago
Jonathan Nobels 4e5ef5b628
net/dns: fix broken dns benchmark tests (#12686)
Updates tailscale/corp#20677

The recover function wasn't getting set in the benchmark
tests.  Default changed to an empty func.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
5 months ago
Brad Fitzpatrick 9766f0e110 net/dns: move mutex before the field it guards
And some misc doc tweaks for idiomatic Go style.

Updates #cleanup

Change-Id: I3ca45f78aaca037f433538b847fd6a9571a2d918
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
5 months ago
Andrew Dunham a475c435ec net/dns/resolver: fix test failure
Updates #cleanup

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I0e815a69ee44ca0ff7c0ea0ca3c6904bbf67ed1f
5 months ago
Jonathan Nobels 27033c6277
net/dns: recheck DNS config on SERVFAIL errors (#12547)
Fixes tailscale/corp#20677

Replaces the original attempt to rectify this (by injecting a netMon
event) which was both heavy handed, and missed cases where the
netMon event was "minor".

On apple platforms, the fetching the interface's nameservers can
and does return an empty list in certain situations.   Apple's API
in particular is very limiting here.  The header hints at notifications
for dns changes which would let us react ahead of time, but it's all
private APIs.

To avoid remaining in the state where we end up with no
nameservers but we absolutely need them, we'll react
to a lack of upstream nameservers by attempting to re-query
the OS.

We'll rate limit this to space out the attempts.   It seems relatively
harmless to attempt a reconfig every 5 seconds (triggered
by an incoming query) if the network is in this broken state.

Missing nameservers might possibly be a persistent condition
(vs a transient error), but that would  also imply that something
out of our control is badly misconfigured.

Tested by randomly returning [] for the nameservers.   When switching
between Wifi networks, or cell->wifi, this will randomly trigger
the bug, and we appear to reliably heal the DNS state.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
5 months ago
Jonathan Nobels 02e3c046aa
net/dns: re-query system resolvers on no-upstream resolver failure on apple platforms (#12398)
Fixes tailscale/corp#20677

On macOS sleep/wake, we're encountering a condition where reconfigure the network
a little bit too quickly - before apple has set the nameservers for our interface.
This results in a persistent condition where we have no upstream resolver and
fail all forwarded DNS queries.

No upstream nameservers is a legitimate configuration, and we have no  (good) way
of determining when Apple is ready - but if we need to forward a query, and we
have no nameservers, then something has gone badly wrong and the network is
very broken.

A simple fix here is to simply inject a netMon event, which will go through the
configuration dance again when we hit the SERVFAIL condition.

Tested by artificially/randomly returning [] for the list of nameservers in the bespoke
ipn-bridge code responsible for getting the nameservers.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
5 months ago
Nick Khyl 4cdc4ed7db net/dns/resolver: return an empty successful response instead of NXDomain when resolving A records for 4via6 domains
As quad-100 is an authoritative server for 4via6 domains, it should always return responses
with a response code of 0 (indicating no error) when resolving records for these domains.
If there's no resource record of the specified type (e.g. A), it should return a response
with an empty answer section rather than NXDomain. Such a response indicates that there
is at least one RR of a different type (e.g., AAAA), suggesting the Windows stub resolver
to look for it.

Fixes tailscale/corp#20767

Signed-off-by: Nick Khyl <nickk@tailscale.com>
5 months ago
Andrea Gottardo dd77111462
xcode/iOS: set MatchDomains when no route requires a custom DNS resolver (#10576)
Updates https://github.com/tailscale/corp/issues/15802.

On iOS exclusively, this PR adds logic to use a split DNS configuration in more cases, with the goal of improving battery life. Acting as the global DNS resolver on iOS should be avoided, as it leads to frequent wakes of IPNExtension.

We try to determine if we can have Tailscale only handle DNS queries for resources inside the tailnet, that is, all routes in the DNS configuration do not require a custom resolver (this is the case for app connectors, for instance).

If so, we set all Routes as MatchDomains. This enables a split DNS configuration which will help preserve battery life. Effectively, for the average Tailscale user who only relies on MagicDNS to resolve *.ts.net domains, this means that Tailscale DNS will only be used for those domains.

This PR doesn't affect users with Override Local DNS enabled. For these users, there should be no difference and Tailscale will continue acting as a global DNS resolver.

Signed-off-by: Andrea Gottardo <andrea@tailscale.com>
6 months ago
Nick Khyl f62e678df8 net/dns/resolver, control/controlknobs, tailcfg: use UserDial instead of SystemDial to dial DNS servers
Now that tsdial.Dialer.UserDial has been updated to honor the configured routes
and dial external network addresses without going through Tailscale, while also being
able to dial a node/subnet router on the tailnet, we can start using UserDial to forward
DNS requests. This is primarily needed for DNS over TCP when forwarding requests
to internal DNS servers, but we also update getKnownDoHClientForProvider to use it.

Updates tailscale/corp#18725

Signed-off-by: Nick Khyl <nickk@tailscale.com>
7 months ago
Andrew Dunham f97d0ac994 net/dns/resolver: add better error wrapping
To aid in debugging exactly what's going wrong, instead of the
not-particularly-useful "dns udp query: context deadline exceeded" error
that we currently get.

Updates #3786
Updates #10768
Updates #11620
(etc.)

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I76334bf0681a8a2c72c90700f636c4174931432c
7 months ago
Brad Fitzpatrick 3672f29a4e net/netns, net/dns/resolver, etc: make netmon required in most places
The goal is to move more network state accessors to netmon.Monitor
where they can be cheaper/cached. But first (this change and others)
we need to make sure the one netmon.Monitor is plumbed everywhere.

Some notable bits:

* tsdial.NewDialer is added, taking a now-required netmon

* because a tsdial.Dialer always has a netmon, anything taking both
  a Dialer and a NetMon is now redundant; take only the Dialer and
  get the NetMon from that if/when needed.

* netmon.NewStatic is added, primarily for tests

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: I877f9cb87618c4eb037cee098241d18da9c01691
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
Andrew Dunham b85c2b2313 net/dns/resolver: use SystemDial in DoH forwarder
This ensures that we close the underlying connection(s) when a major
link change happens. If we don't do this, on mobile platforms switching
between WiFi and cellular can result in leftover connections in the
http.Client's connection pool which are bound to the "wrong" interface.

Updates #10821
Updates tailscale/corp#19124

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ibd51ce2efcaf4bd68e14f6fdeded61d4e99f9a01
7 months ago
Brad Fitzpatrick 7c1d6e35a5 all: use Go 1.22 range-over-int
Updates #11058

Change-Id: I35e7ef9b90e83cac04ca93fd964ad00ed5b48430
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
7 months ago
alexelisenko fe22032fb3
net/dns/{publicdns,resolver}: add start of Control D support
Updates #7946

[@bradfitz fixed up version of #8417]

Change-Id: I1dbf6fa8d525b25c0d7ad5c559a7f937c3cd142a
Signed-off-by: alexelisenko <39712468+alexelisenko@users.noreply.github.com>
Signed-off-by: Alex Paguis <alex@windscribe.com>
8 months ago
James Tucker 8d0d46462b net/dns: timeout DOH requests after 10s without response headers
If a client socket is remotely lost but the client is not sent an RST in
response to the next request, the socket might sit in RTO for extended
lengths of time, resulting in "no internet" for users. Instead, timeout
after 10s, which will close the underlying socket, recovering from the
situation more promptly.

Updates #10967

Signed-off-by: James Tucker <james@tailscale.com>
9 months ago
Andrew Dunham 35c303227a net/dns/resolver: add ID to verbose logs in forwarder
To make it easier to correlate the starting/ending log messages.

Updates #cleanup

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I2802d53ad98e19bc8914bc58f8c04d4443227b26
11 months ago
Andrew Lytvynov 2716250ee8
all: cleanup unused code, part 2 (#10670)
And enable U1000 check in staticcheck.

Updates #cleanup

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
11 months ago
James Tucker b48b7d82d0 appc,ipn/ipnlocal,net/dns/resolver: add App Connector wiring when enabled in prefs
An EmbeddedAppConnector is added that when configured observes DNS
responses from the PeerAPI. If a response is found matching a configured
domain, routes are advertised when necessary.

The wiring from a configuration in the netmap capmap is not yet done, so
while the connector can be enabled, no domains can yet be added.

Updates tailscale/corp#15437

Signed-off-by: James Tucker <james@tailscale.com>
1 year ago
Brad Fitzpatrick 7868393200 net/dns/resolver, ipnlocal: fix ExitDNS on Android and iOS
Advertise it on Android (it looks like it already works once advertised).

And both advertise & likely fix it on iOS. Yet untested.

Updates #9672

Change-Id: If3b7e97f011dea61e7e75aff23dcc178b6cf9123
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
1 year ago
Andrew Dunham 91b9899402 net/dns/resolver: fix flaky test
Updates #cleanup

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I2d073220bb6ac78ba88d8be35085cc23b727d69f
1 year ago
Andrew Dunham 286c6ce27c
net/dns/resolver: race UDP and TCP queries (#9544)
Instead of just falling back to making a TCP query to an upstream DNS
server when the UDP query returns a truncated query, also start a TCP
query in parallel with the UDP query after a given race timeout. This
ensures that if the upstream DNS server does not reply over UDP (or if
the response packet is blocked, or there's an error), we can still make
queries if the server replies to TCP queries.

This also adds a new package, util/race, to contain the logic required for
racing two different functions and returning the first non-error answer.

Updates tailscale/corp#14809

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I4311702016c1093b1beaa31b135da1def6d86316
1 year ago
Andrew Dunham 530aaa52f1 net/dns: retry forwarder requests over TCP
We weren't correctly retrying truncated requests to an upstream DNS
server with TCP. Instead, we'd return a truncated request to the user,
even if the user was querying us over TCP and thus able to handle a
large response.

Also, add an envknob and controlknob to allow users/us to disable this
behaviour if it turns out to be buggy ( DNS ).

Updates #9264

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ifb04b563839a9614c0ba03e9c564e8924c1a2bfd
1 year ago
Brad Fitzpatrick a874f1afd8 all: adjust case of "IPv4" and "IPv6"
Updates #docs

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
1 year ago
Mihai Parparita 7330aa593e all: avoid repeated default interface lookups
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>
2 years ago
Mihai Parparita 4722f7e322 all: move network monitoring from wgengine/monitor to net/netmon
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>
2 years ago
Andrew Dunham f85dc6f97c
ci: add more lints (#7909)
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
2 years ago
Brad Fitzpatrick 10f1c90f4d wgengine/magicsock, types/nettype, etc: finish ReadFromUDPAddrPort netip migration
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>
2 years ago
Mihai Parparita edb02b63f8 net/sockstats: pass in logger to sockstats.WithSockStats
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>
2 years ago
Andrew Dunham 83fa17d26c various: pass logger.Logf through to more places
Updates #7537

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Id89acab70ea678c8c7ff0f44792d54c7223337c6
2 years ago
Mihai Parparita 6ac6ddbb47 sockstats: switch label to enum
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>
2 years ago
Maisem Ali 1a30b2d73f all: use tstest.Replace more
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Mihai Parparita 9cb332f0e2 sockstats: instrument networking code paths
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>
2 years ago
David Anderson 8b2ae47c31 version: unexport all vars, turn Short/Long into funcs
The other formerly exported values aren't used outside the package,
so just unexport them.

Signed-off-by: David Anderson <danderson@tailscale.com>
2 years ago
Mihai Parparita 0e3fb91a39 net/dns/resolver: remove maxDoHInFlight
It was originally added to control memory use on iOS (#2490), but then
was relaxed conditionally when running on iOS 15 (#3098). Now that we
require iOS 15, there's no need for the limit at all, so simplify back
to the original state.

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Andrew Dunham 880a41bfcc net/dns/resolver: add envknob to debug exit node DNS queries on on Windows
Add the envknob TS_DEBUG_EXIT_NODE_DNS_NET_PKG, which enables more
verbose debug logging when calling the handleExitNodeDNSQueryWithNetPkg
function. This function is currently only called on Windows and Android.

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ieb3ca7b98837d7dc69cd9ca47609c1c52e3afd7b
2 years ago
Brad Fitzpatrick b1248442c3 all: update to Go 1.20, use strings.CutPrefix/Suffix instead of our fork
Updates #7123
Updates #5309

Change-Id: I90bcd87a2fb85a91834a0dd4be6e03db08438672
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Will Norris 71029cea2d all: update copyright and license headers
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>
2 years ago
Mihai Parparita 33520920c3 all: use strs.CutPrefix and strs.CutSuffix more
Updates places where we use HasPrefix + TrimPrefix to use the combined
function.

Updates #5309

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Brad Fitzpatrick 66b4a363bd net/dns/resolver: add yet another 4via6 DNS form that's hopefully more robust
$ dig +short @100.100.100.100 aaaa 10-2-5-3-via-7.foo-bar.ts.net
fd7a:115c:a1e0:b1a:0:7:a02:503

$ dig +short @100.100.100.100 aaaa 10-2-5-3-via-7
fd7a:115c:a1e0:b1a:0:7:a02:503

$ ping 10-2-5-3-via-7
PING 10-2-5-3-via-7(fd7a:115c:a1e0:b1a:0:7:a02:503 (fd7a:115c:a1e0:b1a:0:7:a02:503)) 56 data bytes
...

Change-Id: Ice8f954518a6a2fca8b2c04da7f31f61d78cdec4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick da8def8e13 all: remove old +build tags
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>
2 years ago
Josh Soref d4811f11a0 all: fix spelling mistakes
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
2 years ago
Eng Zer Jun f0347e841f refactor: move from io/ioutil to io and os packages
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>
2 years ago
Brad Fitzpatrick 74674b110d envknob: support changing envknobs post-init
Updates #5114

Change-Id: Ia423fc7486e1b3f3180a26308278be0086fae49b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Mihai Parparita 82e82d9b7a net/dns/resolver: remove unused responseTimeout constant
Timeout is now enforced elsewhere, see discussion in https://github.com/tailscale/tailscale/pull/4408#discussion_r970092333.

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
2 years ago
Brad Fitzpatrick e7376aca25 net/dns/resolver: set DNS-over-HTTPS Accept and User-Agent header on requests
Change-Id: I14b821771681e70405a507f43229c694159265ff
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 2aade349fc net/dns, types/dnstypes: update some comments, tests for DoH
Clarify & verify that some DoH URLs can be sent over tailcfg
in some limited cases.

Updates #2452

Change-Id: Ibb25db77788629c315dc26285a1059a763989e24
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 58abae1f83 net/dns/{publicdns,resolver}: add NextDNS DoH support
NextDNS is unique in that users create accounts and then get
user-specific DNS IPs & DoH URLs.

For DoH, the customer ID is in the URL path.

For IPv6, the IP address includes the customer ID in the lower bits.

For IPv4, there's a fragile "IP linking" mechanism to associate your
public IPv4 with an assigned NextDNS IPv4 and that tuple maps to your
customer ID.

We don't use the IP linking mechanism.

Instead, NextDNS is DoH-only. Which means using NextDNS necessarily
shunts all DNS traffic through 100.100.100.100 (programming the OS to
use 100.100.100.100 as the global resolver) because operating systems
can't usually do DoH themselves.

Once it's in Tailscale's DoH client, we then connect out to the known
NextDNS IPv4/IPv6 anycast addresses.

If the control plane sends the client a NextDNS IPv6 address, we then
map it to the corresponding NextDNS DoH with the same client ID, and
we dial that DoH server using the combination of v4/v6 anycast IPs.

Updates #2452

Change-Id: I3439d798d21d5fc9df5a2701839910f5bef85463
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago