The cloner and viewer code generators didn't handle named types
with basic underlying types (map/slice) that have their own Clone
or View methods. For example, a type like:
type Map map[string]any
func (m Map) Clone() Map { ... }
func (m Map) View() MapView { ... }
When used as a struct field, the cloner would descend into the
underlying map[string]any and fail because it can't clone the any
(interface{}) value type. Similarly, the viewer would try to create
a MapFnOf view and fail.
Fix the cloner to check for a Clone method on the named type
before falling through to the underlying type handling.
Fix the viewer to check for a View method on named map/slice types,
so the type author can provide a purpose-built safe view that
doesn't leak raw any values. Named map/slice types without a View
method fall through to normal handling, which correctly rejects
types like map[string]any as unsupported.
Updates tailscale/corp#39502 (needed by tailscale/corp#39594)
Change-Id: Iaef0192a221e02b4b8e409c99ef8398090327744
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Add a new vet analyzer that checks t.Run subtest names don't contain
characters requiring quoting when re-running via "go test -run". This
enforces the style guide rule: don't use spaces or punctuation in
subtest names.
The analyzer flags:
- Direct t.Run calls with string literal names containing spaces,
regex metacharacters, quotes, or other problematic characters
- Table-driven t.Run(tt.name, ...) calls where tt ranges over a
slice/map literal with bad name field values
Also fix all 978 existing violations across 81 test files, replacing
spaces with hyphens and shortening long sentence-like names to concise
hyphenated forms.
Updates #19242
Change-Id: Ib0ad96a111bd8e764582d1d4902fe2599454ab65
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
When racing multiple upstream DNS resolvers, a REFUSED (RCode 5) response
from a broken or misconfigured resolver could win the race and be returned
to the client before healthier resolvers had a chance to respond with a
valid answer. This caused complete DNS failure in cases where, e.g., a
broken upstream resolver returned REFUSED quickly while a working resolver
(such as 1.1.1.1) was still responding.
Previously, only SERVFAIL (RCode 2) was treated as a soft error. REFUSED
responses were returned as successful bytes and could win the race
immediately. This change also treats REFUSED as a soft error in the UDP
and TCP forwarding paths, so the race continues until a better answer
arrives. If all resolvers refuse, the first REFUSED response is returned
to the client.
Additionally, SERVFAIL responses from upstream resolvers are now returned
verbatim to the client rather than replaced with a locally synthesized
packet. Synthesized SERVFAIL responses were authoritative and guaranteed
to include a question section echoing the original query; upstream
responses carry no such guarantees but may include extended error
information (e.g. RFC 8914 extended DNS errors) that would otherwise
be lost.
Fixes#19024
Signed-off-by: Brendan Creane <bcreane@gmail.com>
If an NRPT rule lists more than one server, those servers should be separated by a semicolon (";"),
rather than a semicolon followed by a space ("; "). Otherwise, Windows fails to parse the created
registry value, and DNS resolution may fail.
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gpnrpt/06088ca3-4cf1-48fa-8837-ca8d853ee1e8Fixes#19040
Updates #15404 (enabled MagicDNS IPv6 by default, adding a second server and triggering the issue)
Signed-off-by: Nick Khyl <nickk@tailscale.com>
I omitted a lot of the min/max modernizers because they didn't
result in more clear code.
Some of it's older "for x := range 123".
Also: errors.AsType, any, fmt.Appendf, etc.
Updates #18682
Change-Id: I83a451577f33877f962766a5b65ce86f7696471c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The new version of app connector (conn25) needs to read DNS responses
for domains it is interested in and store and swap out IP addresses.
Add a hook to dns manager to enable this.
Give the conn25 updated netmaps so that it knows when to assign
connecting addresses and from what pool.
Assign an address when we see a DNS response for a domain we are
interested in, but don't do anything with the address yet.
Updates tailscale/corp#34252
Signed-off-by: Fran Bull <fran@tailscale.com>
This adds a new ControlKnob to make MagicDNS IPv6 registration
(telling systemd/etc) opt-out rather than opt-in.
Updates #15404
Change-Id: If008e1cb046b792c6aff7bb1d7c58638f7d650b1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
In the absence of a better mechanism, writing unqualified hostnames to the hosts file may be required
for MagicDNS to work on some Windows environments, such as domain-joined machines. It can also
improve MagicDNS performance on non-domain joined devices when we are not the device's primary
DNS resolver.
At the same time, updating the hosts file can be slow and expensive, especially when it already contains
many entries, as was previously reported in #14327. It may also have negative side effects, such as interfering
with the system's DNS resolution policies.
Additionally, to fix#18712, we had to extend hosts file usage to domain-joined machines when we are not
the primary DNS resolver. For the reasons above, this change may introduce risk.
To allow customers to disable hosts file updates remotely without disabling MagicDNS entirely, whether on
domain-joined machines or not, this PR introduces the `disable-hosts-file-updates` node attribute.
Updates #18712
Updates #14327
Signed-off-by: Nick Khyl <nickk@tailscale.com>
On domain-joined Windows devices the primary search domain (the one the device is joined to)
always takes precedence over other search domains. This breaks MagicDNS when we are the primary
resolver on the device (see #18712). To work around this Windows behavior, we should write MagicDNS
host names the hosts file just as we do when we're not the primary resolver.
This commit does exactly that.
Fixes#18712
Signed-off-by: Nick Khyl <nickk@tailscale.com>
fixestailscale/tailscale#18436
Queries can still make their way to the forwarder when accept-dns is disabled.
Since we have not configured the forwarder if --accept-dns is false, this errors out
(correctly) but it also generates a persistent health warning. This forwards the
Pref setting all the way through the stack to the forwarder so that we can be more
judicious about when we decide that the forward path is unintentionally missing, vs
simply not configured.
Testing:
tailscale set --accept-dns=false. (or from the GUI)
dig @100.100.100.100 example.com
tailscale status
No dns related health warnings should be surfaced.
Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
The forwarder was not setting the Truncated (TC) flag when UDP DNS
responses exceeded either the EDNS buffer size (if present) or the
RFC 1035 default 512-byte limit. This affected DoH, TCP fallback,
and UDP response paths.
The fix ensures checkResponseSizeAndSetTC is called in all code paths
that return UDP responses, enforcing both EDNS and default UDP size
limits.
Added comprehensive unit tests and consolidated duplicate test helpers.
Updates #18107
Signed-off-by: Brendan Creane <bcreane@gmail.com>
This adds a new node capability 'dns-subdomain-resolve' that signals
that all of hosts' subdomains should resolve to the same IP address.
It allows wildcard matching on any node marked with this capability.
This change also includes an util/dnsname utility function that lets
us access the parent of a full qualified domain name. MagicDNS takes
this function and recursively searchs for a matching real node name.
One important thing to observe is that, in this context, a subdomain
can have multiple sub labels. This means that for a given node named
machine, both my.machine and be.my.machine will be a positive match.
Updates #1196
Signed-off-by: Fernando Serboncini <fserb@tailscale.com>
When tailscaled gets started with userspace networking, it won't
modify your system's network configuration. For this, it creates
a noopManager for DNS management. noopManager correctly observes
that there's no real OS DNS to send queries to. This leads to we
completely dropping any DNS internal resolution from `dns query`
This change alters this so that even without a base config we'll
still allow the internal resolver to handle internal DNS queries
Fixes#18354
Signed-off-by: Fernando Serboncini <fserb@tailscale.com>
Someone asked me if we use DNS-over-HTTPS if the system's resolver is an
IP address that supports DoH and there's no global nameserver set (i.e.
no "Override DNS servers" set). I didn't know the answer offhand, and it
took a while for me to figure it out. The answer is yes, in cases where
we take over the system's DNS configuration and read the base config, we
do upgrade any DoH-capable resolver to use DoH. Here's a test that
verifies this behaviour (and hopefully helps as documentation the next
time someone has this question).
Updates #cleanup
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
This file was never truly necessary and has never actually been used in
the history of Tailscale's open source releases.
A Brief History of AUTHORS files
---
The AUTHORS file was a pattern developed at Google, originally for
Chromium, then adopted by Go and a bunch of other projects. The problem
was that Chromium originally had a copyright line only recognizing
Google as the copyright holder. Because Google (and most open source
projects) do not require copyright assignemnt for contributions, each
contributor maintains their copyright. Some large corporate contributors
then tried to add their own name to the copyright line in the LICENSE
file or in file headers. This quickly becomes unwieldy, and puts a
tremendous burden on anyone building on top of Chromium, since the
license requires that they keep all copyright lines intact.
The compromise was to create an AUTHORS file that would list all of the
copyright holders. The LICENSE file and source file headers would then
include that list by reference, listing the copyright holder as "The
Chromium Authors".
This also become cumbersome to simply keep the file up to date with a
high rate of new contributors. Plus it's not always obvious who the
copyright holder is. Sometimes it is the individual making the
contribution, but many times it may be their employer. There is no way
for the proejct maintainer to know.
Eventually, Google changed their policy to no longer recommend trying to
keep the AUTHORS file up to date proactively, and instead to only add to
it when requested: https://opensource.google/docs/releasing/authors.
They are also clear that:
> Adding contributors to the AUTHORS file is entirely within the
> project's discretion and has no implications for copyright ownership.
It was primarily added to appease a small number of large contributors
that insisted that they be recognized as copyright holders (which was
entirely their right to do). But it's not truly necessary, and not even
the most accurate way of identifying contributors and/or copyright
holders.
In practice, we've never added anyone to our AUTHORS file. It only lists
Tailscale, so it's not really serving any purpose. It also causes
confusion because Tailscalars put the "Tailscale Inc & AUTHORS" header
in other open source repos which don't actually have an AUTHORS file, so
it's ambiguous what that means.
Instead, we just acknowledge that the contributors to Tailscale (whoever
they are) are copyright holders for their individual contributions. We
also have the benefit of using the DCO (developercertificate.org) which
provides some additional certification of their right to make the
contribution.
The source file changes were purely mechanical with:
git ls-files | xargs sed -i -e 's/\(Tailscale Inc &\) AUTHORS/\1 contributors/g'
Updates #cleanup
Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d
Signed-off-by: Will Norris <will@tailscale.com>
When the TS_DEBUG_DNS_FORWARD_SEND envknob is turned on, also log the
source IP:port of the query that tailscaled is forwarding.
Updates tailscale/corp#35374
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
When using the resolve.conf file for setting DNS, it is possible that
some other services will trample the file and overwrite our set DNS
server. Experiments has shown this to be a racy error depending on how
quickly processes start.
Make an attempt to trample back the file a limited number of times if
the file is changed.
Updates #16635
Signed-off-by: Claus Lensbøl <claus@tailscale.com>
I got sidetracked apparently and never finished writing this Clone
code in 316afe7d02 (#17448). (It really should use views instead.)
And then I missed one of the users of "routerChanged" that was broken up
into "routerChanged" vs "dnsChanged".
This broke integration tests elsewhere.
Fixes#17506
Change-Id: I533bf0fcf3da9ac6eb4a6cdef03b8df2c1fb4c8e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 45 KB from the min build, no longer pulling in deephash or
util/hashx, both with unsafe code.
It can actually be more efficient to not use deephash, as you don't
have to walk all bytes of all fields recursively to answer that two
things are not equal. Instead, you can just return false at the first
difference you see. And then with views (as we use ~everywhere
nowadays), the cloning the old value isn't expensive, as it's just a
pointer under the hood.
Updates #12614
Change-Id: I7b08616b8a09b3ade454bb5e0ac5672086fe8aec
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 262 KB so far. I'm sure I missed some places, but shotizam says
these were the low hanging fruit.
Updates #12614
Change-Id: Ia31c01b454f627e6d0470229aae4e19d615e45e3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The callback itself is not removed as it is used in other repos, making
it simpler for those to slowly transition to the eventbus.
Updates #15160
Signed-off-by: Claus Lensbøl <claus@tailscale.com>
Saves 328 KB (2.5%) off the minimal binary.
For IoT devices that don't need MagicDNS (e.g. they don't make
outbound connections), this provides a knob to disable all the DNS
functionality.
Rather than a massive refactor today, this uses constant false values
as a deadcode sledgehammer, guided by shotizam to find the largest DNS
functions which survived deadcode.
A future refactor could make it so that the net/dns/resolver and
publicdns packages don't even show up in the import graph (along with
their imports) but really it's already pretty good looking with just
these consts, so it's not at the top of my list to refactor it more
soon.
Also do the same in a few places with the ACME (cert) functionality,
as I saw those while searching for DNS stuff.
Updates #12614
Change-Id: I8e459f595c2fde68ca16503ff61c8ab339871f97
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It has nothing to do with logtail and is confusing named like that.
Updates #cleanup
Updates #17323
Change-Id: Idd34587ba186a2416725f72ffc4c5778b0b9db4a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
tsnet apps in particular never use the Linux DNS OSManagers, so they don't need
DBus, etc. I started to pull that all out into separate features so tsnet doesn't
need to bring in DBus, but hit this first.
Here you can see that tsnet (and the k8s-operator) no longer pulls in inotify.
Updates #17206
Change-Id: I7af0f391f60c5e7dbeed7a080346f83262346591
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It doesn't really pull its weight: it adds 577 KB to the binary and
is rarely useful.
Also, we now have static IPs and other connectivity paths coming
soon enough.
Updates #5853
Updates #1278
Updates tailscale/corp#32168
Change-Id: If336fed00a9c9ae9745419e6d81f7de6da6f7275
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This fixes a flaky test which has been occasionally timing out in CI.
In particular, this test times out if `watchFile` receives multiple
notifications from inotify before we cancel the test context. We block
processing the second notification, because we've stopped listening to
the `callbackDone` channel.
This patch changes the test so we only send on the first notification.
Testing this locally with `stress` confirms that the test is no longer
flaky.
Fixes#17172
Updates #14699
Signed-off-by: Alex Chan <alexc@tailscale.com>
The Tracker was using direct callbacks to ipnlocal. This PR moves those
to be triggered via the eventbus.
Additionally, the eventbus is now closed on exit from tailscaled
explicitly, and health is now a SubSystem in tsd.
Updates #15160
Signed-off-by: Claus Lensbøl <claus@tailscale.com>
This is step 4 of making syspolicy a build-time feature.
This adds a policyclient.Get() accessor to return the correct
implementation to use: either the real one, or the no-op one. (A third
type, a static one for testing, also exists, so in general a
policyclient.Client should be plumbed around and not always fetched
via policyclient.Get whenever possible, especially if tests need to use
alternate syspolicy)
Updates #16998
Updates #12614
Change-Id: Iaf19670744a596d5918acfa744f5db4564272978
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Step 3 in the series. See earlier cc532efc20 and d05e6dc09e.
This step moves some types into a new leaf "ptype" package out of the
big "settings" package. The policyclient.Client will later get new
methods to return those things (as well as Duration and Uint64, which
weren't done at the time of the earlier prototype).
Updates #16998
Updates #12614
Change-Id: I4d72d8079de3b5351ed602eaa72863372bd474a2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is step 2 of ~4, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.
Step 1 was #16984.
In this second step, the util/syspolicy/policyclient package is added
with the policyclient.Client interface. This is the interface that's
always present (regardless of build tags), and is what code around the
tree uses to ask syspolicy/MDM questions.
There are two implementations of policyclient.Client for now:
1) NoPolicyClient, which only returns default values.
2) the unexported, temporary 'globalSyspolicy', which is implemented
in terms of the global functions we wish to later eliminate.
This then starts to plumb around the policyclient.Client to most callers.
Future changes will plumb it more. When the last of the global func
callers are gone, then we can unexport the global functions and make a
proper policyclient.Client type and constructor in the syspolicy
package, removing the globalSyspolicy impl out of tsd.
The final change will sprinkle build tags in a few more places and
lock it in with dependency tests to make sure the dependencies don't
later creep back in.
Updates #16998
Updates #12614
Change-Id: Ib2c93d15c15c1f2b981464099177cd492d50391c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is step 1 of ~3, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.
In this first (very noisy) step, all the syspolicy string key
constants move to a new constant-only (code-free) package. This will
make future steps more reviewable, without this movement noise.
There are no code or behavior changes here.
The future steps of this series can be seen in #14720: removing global
funcs from syspolicy resolution and using an interface that's plumbed
around instead. Then adding build tags.
Updates #12614
Change-Id: If73bf2c28b9c9b1a408fe868b0b6a25b03eeabd1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Resolver.PreferGo didn't used to work on Windows.
It was fixed in 2022, though. (https://github.com/golang/go/issues/33097)
Updates #5161
Change-Id: I4e1aeff220ebd6adc8a14f781664fa6a2068b48c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
fixestailscale/corp#25612
We now keep track of any dns configurations which we could not
compile. This gives RecompileDNSConfig a configuration to
attempt to recompile and apply when the OS pokes us to indicate
that the interface dns servers have changed/updated. The manager config
will remain unset until we have the required information to compile
it correctly which should eliminate the problematic SERVFAIL
responses (especially on macOS 15).
This also removes the missingUpstreamRecovery func in the forwarder
which is no longer required now that we have proper error handling
and recovery manager and the client.
Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
In this PR, we make DNS registration behavior configurable via the EnableDNSRegistration policy setting.
We keep the default behavior unchanged, but allow admins to either enforce DNS registration and dynamic
DNS updates for the Tailscale interface, or prevent Tailscale from modifying the settings configured in
the network adapter's properties or by other means.
Updates #14917
Signed-off-by: Nick Khyl <nickk@tailscale.com>
In this PR, we make the "user-dial-routes" behavior default on all platforms except for iOS and Android.
It can be disabled by setting the TS_DNS_FORWARD_USE_ROUTES envknob to 0 or false.
Updates #12027
Updates #13837
Signed-off-by: Nick Khyl <nickk@tailscale.com>
Android is Linux, but doesn't use Linux DNS managers (or D-Bus).
Updates #12614
Change-Id: I487802ac74a259cd5d2480ac26f7faa17ca8d1c3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This adds netx.DialFunc, unifying a type we have a bazillion other
places, giving it now a nice short name that's clickable in
editors, etc.
That highlighted that my earlier move (03b47a55c7) of stuff from
nettest into netx moved too much: it also dragged along the memnet
impl, meaning all users of netx.DialFunc who just wanted netx for the
type definition were instead also pulling in all of memnet.
So move the memnet implementation netx.Network into memnet, a package
we already had.
Then use netx.DialFunc in a bunch of places. I'm sure I missed some.
And plenty remain in other repos, to be updated later.
Updates tailscale/corp#27636
Change-Id: I7296cd4591218e8624e214f8c70dab05fb884e95
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Brad Fitzpatrick
Greg Steuck
Brendan Creane
Nick Khyl
Fran Bull
Jonathan Nobels
Fernando Serboncini
Andrew Dunham
Will Norris
Claus Lensbøl
Alex Chan
jishudashu
David Anderson