The root device fetch timed out after one second, but some router firmware only starts answering after just over a second, so UPnP never worked behind it. Allow three seconds.
Updates #21860
Signed-off-by: Aniket <aniketkomailkro@gmail.com>
Retry UDP reads returning EHOSTUNREACH or ENETUNREACH, including their
Winsock equivalents, without returning those errors to wireguard-go.
Count them in magicsock_recv_udp_error_reachability. Other read errors are unchanged.
Updates #20616
Updates #19504
Signed-off-by: James Tucker <james@tailscale.com>
Resolve Server.Dial and Server.HTTPClient names using the tailnet's DNS
configuration, including MagicDNS, search domains, split DNS routes, and
global resolvers. Preserve the dialer's local MagicDNS fast path.
Add Server.LookupIP for direct lookups returning netip.Addr values. Keep
the resolver implementation private to tsnet. Search expansion uses
ndots=1, includes the MagicDNS suffix when enabled, and respects trailing
dots. Each lookup uses the current DNS configuration.
Use the internal resolver before considering host fallback. Configured
upstream failures and authoritative local denials must not fall back to
the host resolver; the host's hosts file is consulted only during fallback.
Distinguish configured routes from ambient cloud-host fallback resolvers.
Cover search expansion, split and global DNS, MagicDNS-disabled operation,
configuration updates, hosts-file precedence, and dialing in tests.
Fixes#5840
Updates #7984
Updates #15260
Updates #4677
Change-Id: I832bc4c68658d2d25c5dc29b79acf611fc11e59e
Signed-off-by: James Tucker <james@tailscale.com>
Similar to #20866 for 4via6, conn25 needs to block host-scoped
destinations. Due to the similarity between the two situations, the
matching is no longer in a 4via6-specific location. Since there is no
backwards compatibility to satisfy, there is no exception list for
conn25.
ipn/ipnlocal (for 4via6) and feature/conn25 now both use the new
package.
Updates tailscale/corp#50118
Updates tailscale/corp#46169
Change-Id: Ic3f9cccd2f5823a2ca48bd78607792b0fd6d7db9
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>
* wgengine/magicsock: implement conn.MultiSocketBind
Bind a group of UDP sockets sharing one local port per address family. A
flow index is plumbed through the send path to pick which socket a batch
leaves from.
Group size is fixed at Options.UDPSockets for the life of the Conn. In
case rebind comes short, the group is padded out with conns that block
until closed.
No caller sets Options.UDPSockets yet, every group holds a single socket
and this is inert.
Additionally, add test coverage for the now exercised net/sockopts paths.
Updates tailscale/corp#48168
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
Change-Id: I253646c753fa46e2efd0ad2158d24c416a6a6964
response when DNS request goes to PeerAPI DoH
To support feature/conn25 and other extensions or consumers needing to
know which PeerAPI node handled a DNS request, e.g. for app connectors
or exit nodes, and needing extra data from the server that can be
transmitted in the headers.
DNS responses from other upstream resolvers do not get any metadata
populated, even if those servers are DoH.
Add a new Response type in net/dns/resolver that includes the raw
response bytes and the optional metadata, and replace the bare bytes
with it in some places.
Pass this new type to to net/dns.ResponseMapper instead of the bare
bytes to the extensions can see the metadata.
Updates tailscale/corp#46169
Signed-off-by: Michael Ben-Ami <mzb@tailscale.com>
A peer can advertise 100.64.0.0/10 or fd7a:115c:a1e0::/48 as a subnet
route. Those prefixes are also the coarse routes that stand in for peers'
individual addresses, so one OS route entry has two reasons to exist.
applyDirty set that entry from one reason at a time, so removing the
advertising peer deleted the /10 while coarsening still wanted it, and
turning coarsening off deleted the /10 while a peer still advertised it.
Set each coarse prefix once, from both inputs, as rebuildAll already does.
Fixes#21890
Signed-off-by: Brendan Creane <bcreane@gmail.com>
Windows socket setup enumerates adapters and the routing table through
GetWindowsDefault for every netns socket. These calls can be expensive,
particularly on systems where security software adds syscall latency.
Cache nonzero default interface indices per monitor and address family,
and use them from netns controlC. Invalidate on native route, address,
and IP interface change notifications.
Coalesce concurrent lookups and retry results invalidated during lookup.
Bound lookup and wait attempts; once the budget is exhausted, fall back
to a single uncached lookup rather than failing socket setup, erring
toward a possibly stale interface index over no socket at all. Perform
fresh lookups for zero indices and errors, and retain the existing
unbound-socket fallback for a fresh zero result.
Use uncached lookups after monitor shutdown or when interface-change
notification registration fails.
Test cache reuse, independent address families, notification invalidation,
concurrent fills, invalidation during lookup, route restoration after a
zero result, churn and wait-budget fallbacks, and shutdown.
Fixes#21885
Signed-off-by: James Tucker <james@tailscale.com>
In #20775 (and the follow-up in #21471), we started installing quad-100
as the OS's primary resolver whenever cfg has hosts not covered by a split
DNS route, so that sandboxed macOS clients could still answer them.
However, that check was added to the generic split DNS gate rather than
to the Apple-specific one, so it also applies on Windows, where dcfg.Hosts
always carries every node's records and MagicDNSHostsUnrouted is set
whenever MagicDNS is disabled. As a result, a Windows node with MagicDNS
off and at least one split DNS route (or with MagicDNS on and a peer shared
in from another tailnet) gets a catch-all NRPT rule pointing at quad-100,
and all DNS traffic on the device is proxied through it instead of only
the configured suffixes.
The check is redundant for Apple platforms: iOS never takes the scoped
path there, and sandboxed macOS only does so via scopeApple, which already
requires !cfg.requiresPrimaryResolver(). This removes it from the generic
gate, restoring the pre-1.104 behavior on Windows, and adds tests.
Updates tailscale/corp#45534Fixes#21863
Signed-off-by: Nick Khyl <nickk@tailscale.com>
We're going to add a second SO_REUSEPORT use in the codebase. Extract
and generalize udprelay mechanism for future reuse. Also address the
Linux flaw of allowing same UID groups to merge if the ephemeral port
selection collides.
Exposes public SetReusePort, RawConn and ControlFunc signatures for
shared use.
Updates tailscale/corp#48168Fixestailscale/corp#49297
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
Change-Id: Iba33142877513d72123c51cf1bcc33cf6a6a6964
Add regression guards for the inbound reply paths (TSMP pong, TSMP
connection reject, and the conn25 unmapped Transit IP reject) that were
made nonblocking via TryInjectOutbound: with the injection queue full
and its reader not draining (as when the reader is blocked behind
wireguard-go's device.peers lock held by RemovePeer), generating a
reply must drop rather than block, because the replying goroutine
must complete for the peer teardown to finish. A blocking injection
there deadlocks peer removal against inbound packet processing, as in
tailscale/corp#45544.
The tests run in synctest bubbles: the full queue is the only blocking
operation on these paths, so after synctest.Wait the reply either
completed or is provably stuck on the queue, and no timeout-based
failure is needed. A regressed, blocking sender is unblocked by
closing the Wrapper before the test fails, so the bubble settles.
Updates tailscale/corp#45544
Updates #21631
Signed-off-by: James Tucker <james@tailscale.com>
Export ErrInjectionQueueFull, returned by TryInjectOutbound when a full
injection queue forced a packet drop, and define it to wrap
syscall.ENOBUFS on platforms that define that errno. Callers can now
test for buffer exhaustion generically with errors.Is(err,
syscall.ENOBUFS) instead of knowing about tstun's error type. plan9 and
tamago have no ENOBUFS in their syscall packages, so there the error is
a plain sentinel; code that must build on those platforms (or on all
platforms, like feature/conn25) can still match the exported sentinel.
Also stop conn25 from logging queue-full TSMP rejections: those drops
are expected under saturation and are already counted as reason="queue_full"
in the outbound usermetric.
Signed-off-by: James Tucker <james@tailscale.com>
Make link queue writes nonblocking: gVisor can generate replies on the
reader needed to drain the queue. A full queue drops only the rejected
packet and reports ErrNoBufferSpace, which gVisor counts and TCP handles
by arming its retransmit timer. UDP sockets do not see the error unless
IP_RECVERR is set, matching Linux. Count WireGuard-bound drops as
reason="queue_full" in tailscaled_outbound_dropped_packets_total, and
per-queue drops in debug and client metrics.
Add Wrapper.TryInjectOutbound for best-effort replies generated while
processing packets (TSMP rejects and pongs, conn25 rejects, fake-mode
echo replies), so those paths drop instead of stalling behind the
injection queue's reader. Its drops are also counted as queue_full.
Make outbound injection waits cancellable and close link queues before
abort-generated replies. Stop the WireGuard injection pump on ErrClosed.
Release injected packets still queued at Close, outside closeOnce to
allow reentrant release callbacks; a sender whose send races with Close
releases the packet itself. Release temporary capture buffers.
Copy injected payload views directly into the WireGuard slab, avoiding
an intermediate allocation and copy.
Test queue saturation, multiqueue progress, cancellation, concurrent
shutdown, reference ownership, fragmented payloads, and GSO.
Performance versus the parent commit, on a 4-core Xeon 6975P-C:
inject+read 64B / 1280B 245ns / 479ns -> 187ns / 195ns, 0 allocs
4 injectors, 1 reader 843ns -> 451ns
64KiB GSO inject+split 10.9us -> 4.4us, 64KiB/op -> 0
link queue write+read 81.4ns -> 81.5ns
tsnet TCP, 1 / 8 streams 448 / 580 MB/s -> 492 / 674 MB/s
BenchmarkTCPQueueBottleneck shows that dropping on full queues keeps
throughput at the bottleneck rate (117 MB/s at 100k packets/s, as with
blocking writes), at the cost of more retransmits with many streams.
Fixes#21631
Updates #21638
Updates #21632
Updates #21639
Signed-off-by: James Tucker <james@tailscale.com>
Updates tailscale/corp#49775
Track authoritative NXDOMAINs for at least 60 seconds on macOS and iOS (where our short SOA ttls are not properly respected), and flush when a cached name becomes resolvable. Compile a no-op tracker elsewhere and avoid duplicate VPN reconfiguration on link changes.
Injected packets now arrive on a queue of their own rather than being
multiplexed onto queue 0. Wrapper.Read is deprecated. It still reads the
first kernel-backed queue, but from now on no single queue of a Wrapper
sees every packet.
Deletes the poll goroutine, the bufferConsumed token, the vectorOutbound
channel, tunVectorReadResult, a full-size slab, and the copy of every
accepted packet between the two slabs.
wireguard-go now runs an extra TUN reader goroutine, a net wash compared
to running our poll routine.
Measured on c8gn.2xlarge, Ubuntu 24.04, Kernel v6.14, tailscaled under
load from two peers, each running iperf3 -P8.
tx mean rx mean cores
main 7.00 8.84 3.49
branch 8.94 8.86 3.95
Bumps wireguard-go pin to tailscale/wireguard-go@e61b6b7811.
Updates tailscale/corp#37878
Change-Id: Ide0e8a9f345371b2747fad59dd2bf963c8af5b63
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
Wrapper presents one read queue per queue of the underlying device, and
forwards the flow it is given on the write side.
pollVector still multiplexes injected packets onto the device's own
read, which is queue 0. Wrapper.Read is now queues[0].Read and behaves
exactly as it did. Locally-terminated traffic stays on flow 0.
This is a no-op change: the Wrapper presents a single queue and
wireguard-go runs the single reader goroutine it ran before.
Updates tailscale/corp#37878
Change-Id: I844e95cf430075e237ad7e83bdbaa8e43e067ccb
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
A TCP retry to the server that just answered can only return the same
rcode, and when that server accepted the connection and never answered,
the query ran the full 10s dnsQueryTimeout and the client got nothing.
An upstream rcode now ends the UDP/TCP race when it arrives, and the
client gets it once every resolver has had udpRaceTimeout to answer
instead of waiting for all of them to report. tcpQueryTimeout is also
set on the conn, since a context cannot interrupt a blocked read, and a
new dns_query_fwd_udp_read_ctx_done counter separates query teardown
from upstream read errors.
Fixes#20826
Signed-off-by: Brendan Creane <bcreane@gmail.com>
When control removes our home DERP region, magicsock drops the
connection, clears its home, and does a ReSTUN (since #3664). But
the netcheck stickiness checks could still pick the removed region
again, because they keyed off the previous report's PreferredDERP and
recent DERP frame activity, both of which outlive the removal.
Ignore the previous preferred region, and ForcePreferredDERP, when
they're no longer in the current DERP map.
Updates #21673
Change-Id: Ib3b7f62b6861a73d0893d6b3011dfac20bc93117
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
logtail's dialer only used dnscache in its fallback path, and built a
new Resolver per dial there, so every new connection to the log server
did a fresh DNS lookup (twice when the first dial failed). On systems
without a caching resolver, that adds up to a lot of queries for
log.tailscale.com.
Share one dnscache.Resolver across all log dialers, and have the
bootstrap path go straight to dnsfallback rather than repeating the
regular lookup. Since the cache TTL is a fixed guess, make
dnscache.Dialer expire a cached entry and look the host up again when
dialing its cached IPs fails, so a moved host isn't stuck on stale IPs.
Updates #15326
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I6a55d72a73ed878faebc15317421a54d9ae293ca
Allow TS_DEBUG_DISABLE_HOSTS_FILE_UPDATES to opt out of Windows hosts entries locally, using the existing tailscaled-env.txt configuration mechanism. Keep the control-plane node attribute authoritative and preserve upstream behavior when the local option is unset or false.
Cover defaults, local opt-out, nil control knobs, and policy precedence with Windows tests. This change is limited to DNS configuration; no route changes or upstream route-fix cherry-pick are included.
Updates tailscale/tailscale#14327
Signed-off-by: Caleb Crome <caleb.crome@flyzipline.com>
Previously, ipconfig /flushdns and ipconfig /registerdns were called
from multiple sites without coordination: the DNS manager's SetDNS
(in a fire-and-forget goroutine), the router's winRouter.Set (synchronously
on every route change), the link change handler, and session unlock.
These could fire concurrently or in rapid succession, spawning multiple
ipconfig processes and putting excessive pressure on the Dnscache service.
In this PR, we introduce a new util/coalescedop.CoalescedOp type that provides
"one in-flight, at most one pending" semantics: Do() is non-blocking,
at most one execution runs at a time, and concurrent/rapid successive
requests coalesce into a single follow-up execution.
We then use CoalescedOp for both ipconfig /flushdns and ipconfig /registerdns,
replacing the fire-and-forget goroutine in windowsManager.SetDNS
and in the session unlock handler in tailscaled.
We also remove the redundant dns.Flush() call from winRouter.Set, since every
meaningful router.Set is followed by dns.Set in Reconfig, which already
flushes.
Updates #21001
Signed-off-by: Nick Khyl <nickk@tailscale.com>
When quad-100 must be installed as the OS primary resolver (Apple Mode
B), its catch-all forwarders were blended from base-config nameserver
IPs alone: non-standard ports, search domains, and DoH/DoT endpoints
were lost, and a base read that returned no resolvers silently
installed a catch-all with no forwarders at all, breaking all public
DNS while reporting a healthy DNS configuration.
Introduce OSConfig.Resolvers, which OSConfigurators able to recover the
underlying resolver configuration at full fidelity populate for the DNS
manager only: it never reaches the OS, and Equal ignores it so
configuration application is unaffected. Blend it into the catch-all
route verbatim, falling back to plain IP:53 resolvers derived from
Nameservers.
Teach the forwarder to dial arbitrary https:// resolvers instead of
only well-known public providers: at their BootstrapResolution
addresses when present, or at the URL's own host when that is an IP
literal, so enterprise DoH endpoints recovered from the OS can be
forwarded to over DoH rather than plaintext DNS.
Reject an empty base config on Apple Mode B: keep the previous
configuration, use the upstream empty-base health warning and medium
severity, and let an extension-triggered recompile retry instead of
installing a catch-all that cannot forward. Expose the same missing-
resolver error to OS configurators so bridge-reported absence uses this
warning too, while genuine configuration-read failures remain distinct.
Extend the Apple mode tests with empty-base and full-fidelity blend
coverage, and update the iOS primary-mode cases to model the real
NetworkExtension base read (LAN resolvers and search domains) rather
than the silently empty catch-all.
RELNOTE: Fix silent public DNS breakage when Tailscale must be the system DNS resolver on Apple clients.
Updates #20341
Updates tailscale/corp#45534
Updates tailscale/corp#48693
Change-Id: Ie7e277b0fd39d2b91b1e770f8f01d688e02b4d49
Signed-off-by: James Tucker <james@tailscale.com>
Apple tunnels derive global search domains from match domains. Scope
simple iOS and sandboxed macOS configurations to the full MagicDNS route
set; keep custom split resolvers and uncovered forward/PTR records behind
a primary resolver. Respect scoping controls and reject unsafe fallback
when base DNS cannot be read.
Add mode-selection and reverse-coverage tests for both Apple platforms.
RELNOTE: Fix MagicDNS routing and split-DNS search scoping on Apple clients.
Updates tailscale/corp#48693
Change-Id: I50b5641db466f77c19f911255748e12a10d0a5fe
Signed-off-by: James Tucker <james@tailscale.com>
Shutting down a socket's read side after its peer's FIN has arrived does
nothing on the wire and fails with ENOTCONN on macOS (and on Linux once
the socket reaches TIME_WAIT), so every cleanly half-closed connection
was logged as a failure and TestTCPHalfClose failed on macOS. Only
half-close the destination after EOF.
When one direction fails, one of the two sockets is dead. If the failure
was writing to it, the other direction is reading from that dead socket
and finishes on its own once it has drained what arrived before the
failure, so leave it alone rather than truncate the data. If the failure
was reading from it, the other direction may be blocked reading the live
peer, so close both connections to unblock it; nothing deliverable is
lost in that case.
Telling the two apart means wrapping the destination, which disables the
kernel splice fast path when both ends are bare TCP connections (about a
third of relay throughput on Linux loopback). That is accepted here: the
common userspace-networking and proxymux configurations wrap the
connections anyway, so they never had the fast path.
The regression was introduced by 027e249fcf (#21359), merged 2026-09-17.
Tested on Linux and on macOS 27 (arm64): the old test failed 49 of 50
runs on macOS; the new tests pass 200 iterations there and pass under
the race detector on Linux.
Fixes#21522
Updates #20883
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I7c2e4a9d1b8f5e3c6a0d2f4b8e1c9a7d3f5b6e2c
Contining the trend of making everything be optional that could
possibly be optional, this makes the magicsock UDP underlay transport
be optional. (mostly replacing a bunch of GOOS != "js" checks in the process)
Omitting udptransport (ts_omit_udptransport) makes magicsock DERP-only: no
UDP sockets, no advertised endpoints, and netcheck measures DERP latency
over HTTPS. Omitting only nattraversal keeps disco ping/pong to peers'
advertised endpoints but drops STUN, call-me-maybe, the peer relay client,
the endpoint tracker, and UDP lifetime probing.
Both are plain linker dead-code elimination via buildfeatures
constants. (as oppposed to moving the code all over into feature
packages and indirecting through hooks) The minimal linux/amd64
tailscaled shrinks by 258 KB without nattraversal and 561 KB without
both.
Updates #12614
Change-Id: Ife10804b48a168d9481e4321c18ed8fc85fa71f1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
WriteBatchTo copied the destination IP and port into the pooled
net.UDPAddr handed to sendmmsg(2), but not the zone. For an IPv6
link-local destination the sockaddr then had no scope ID, and Linux
failed the send with "cannot assign requested address". Disco goes
through WriteToUDPAddrPort, which keeps the zone, so magicsock could
pick a link-local path whose WireGuard packets, sent via WriteBatchTo,
never left the host.
Set the zone on every write, so that a destination without one also
clears the zone left over from an earlier write that reused the
pooled address.
Fixes#21411
Co-Authored-By: Jordan Whited <jordan@tailscale.com>
Signed-off-by: chlee <sourcehatchery@gmail.com>
Signed-off-by: Jordan Whited <jordan@tailscale.com>
On backends that can't do OS-level split DNS, Tailscale forwards the default
route to the system's own resolvers, which it reads out of the OS config. At
boot that config may not be populated yet, because NetworkManager or
systemd-resolved haven't run, and Tailscale took over regardless: it pointed
the OS at 100.100.100.100 but compiled an empty "." route, so every
non-Tailscale name failed to resolve for the rest of the process's life.
Fail compileConfig instead, which leaves the OS resolvers in place, and
retry the last config with a bounded backoff until resolvers appear. A
health warning explains why MagicDNS is inactive in the meantime.
Sandboxed macOS and iOS are exempt. There the network extension reapplies
the config itself when the OS nameservers change, and quad-100 as the
primary resolver is what keeps tailnet names resolving while the base config
is still empty. Dropping the exemption is tracked in tailscale/corp#48962.
This also changes the outcome on an openresolv host with no snippets
registered. Since the fix for #20825, openresolv reports an empty base config
there and tailscaled took over with no upstream, so every public name got
SERVFAIL. Such a host now keeps its resolv.conf and shows the health warning
instead. TestOpenresolvDNS checks that outcome.
TestSplitDNSEmptyBaseConfig covers the boot race end to end in natlab: it
empties resolv.conf on a guest using the "direct" backend, checks that
tailscaled leaves it alone, then adds a resolver and checks that both tailnet
and public names resolve.
Fixes#20341
Signed-off-by: Brendan Creane <bcreane@gmail.com>
Linux is a weak-host stack, so a LAN-adjacent machine can complete a
TCP handshake with a node's peerapi listener by sending a packet to
the node's Tailscale IP, with no credentials and no tailnet
membership.
macOS and iOS already bind the listener to the tunnel interface, and
Windows is protected by its strong host model, so Linux tun mode was
the only platform that leaked.
Bind the Linux listener to the tunnel interface as well, so the
kernel only answers connections that arrive from the tunnel or from
the local host. A natlab VM test verifies that a same-LAN machine can
no longer complete the handshake, while local and peer peerapi keep
working.
FreeBSD has the same weak-host exposure but no per-socket equivalent,
so handling it there with pf is a TODO (#21419).
Updates tailscale/corp#48248
Reported-By: Samuel Keeley (@keeleysam)
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I5f8501b0938c9f7aa39c4c12ebddd988c72e89bf
If people ran derper on a multi-NIC or multi-address host, STUN replies
could go out from the wrong address. The wildcard UDP socket let the
kernel pick the reply's source address by routing to the client, which
means the default route's address rather than the one the request came
in on. With connmark-based policy routing (e.g. DNAT through a tunnel),
the reply then doesn't match the inbound conntrack entry, goes out the
wrong interface, and the client never sees it. DERP over TCP was fine,
since accepted sockets are pinned to the local address.
Add net/pktinfo, a small Linux-only package that uses IP_PKTINFO and
IPV6_RECVPKTINFO to learn each datagram's destination address and to
reply from it, and use it in the STUN server. Only the source address is
pinned; routing still picks the interface. Other platforms are unchanged.
Fixes#21404
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I7b3e9c2d41a8f60e5d9c1b2a3f4e5d6c7b8a9f01
This change improves the initial fuzz seeds to get better coverage. It also includes a fix to the geo fuzzing to avoid a harness induced failure when a NaN input is provided.
No actual code logic changes, test only.
Updates tailscale/corp#46608
Change-Id: I0985b6d3a75603927451eea0a25f4cd72c039795
Signed-off-by: Mike Jensen <mikej@tailscale.com>
If the network comes up during the few milliseconds NewLocalBackend
takes, tailscaled starts with its control client paused and never
unpauses: the interface state snapshot was taken before the netmon
subscription (kept late since #17252), so a change published in between
reached nobody. #21261 hit it on fast-booting NixOS microVMs, and it was
behind the natlab TestEasyEasy CI flake, where a gokrazy node's DHCP
lease landed in that window and "tailscale up" hung at "awaiting
unpause".
Re-read netmon's state after subscribing rather than subscribing before
the snapshot (as #21281 proposed), which would bring back the #17252
data race and let a fresh delta be overwritten by the stale snapshot.
netmon.New and the userspace engine had the same shape of gap between
their snapshot and their subscription; close those too.
Under CPU pressure the hang reproduced in 1 of 22 TestEasyEasy runs
before and 0 of 120 after.
Thanks to @c-vigo for the detailed diagnosis in #21261 and to @zzz-yu
for pointing out this problem and proposing a fix in #21281!
Fixes#21261
Updates #14902
Updates #19126
Updates #deflake
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: Ifa21f8055a85321a5afda7800140fc5045c6ce08
All reads from c.mapping need to take place while the lock is held.
Updates #21127
Change-Id: I4782d5ca027584a9e47dc6241af1970348b18c0a
Signed-off-by: Francois Marier <francois@tailscale.com>
The PCP spec says that when deleting a mapping, both the external
port and the external address must be zero. While PCP servers are
likely to be lenient in practice, we should do the right thing in
case we encounter strict validation.
Resolves#21363
Change-Id: Ice3467d735def5144a810c39e65642988a830972
Signed-off-by: Francois Marier <francois@tailscale.com>
Similar to #16462, when we are acting as a TCP proxy, we need to pass
through half-closes correctly since clients and servers will sometimes
close one direction of the connection and still rely on the other
direction working.
Fixes#20883.
Signed-off-by: Naman Sood <mail@nsood.in>
`decode4` assigned `q.subofs` before validating it against the declared IP total length. A packet with an IHL past the end of the buffer was rejected but left `subofs` dangling there, so a later Transport call would panic.
This change only store `subofs` once validated. As defense in depth an additional bounds check is added in Transport.
Fuzzing was expanded and improved to get better coverage in `packet.go`.
Credit to @Dev-next-gen for finding and reporting.
Fixestailscale/corp#48322
Updates tailscale/corp#46608
Change-Id: I198d06b921add9b188daad79d4ca473aed1e3b66
Signed-off-by: Mike Jensen <mikej@tailscale.com>
Requesting a UDP mapping is the right thing to do since an "all
protocols" (and "all ports") mapping would be akin to requesting
to be a DMZ on that network.
Updates #cleanup
Change-Id: Icc83d4fe14dbec0eb844b093d2d92756d6c05228
Signed-off-by: Francois Marier <francois@tailscale.com>
MakeLookupFunc documents its netMon parameter as optional, but
bootstrapDNSMap passed it straight to netns.NewDialer, which has panicked
on nil since 3672f29a4. Any caller that omitted the monitor and then had a
first dial fail (which is when dnscache consults LookupIPFallback) crashed
with "netns.NewDialer called with nil netMon". control/tsp clients hit this
under Antithesis fault injection.
Use a plain net.Dialer when no monitor is provided and add a regression
test.
Updates tailscale/corp#47865
Change-Id: Ie8255033602dd1f8243c1ced8f453fb7ab9be74d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Payload guards a truncated packet by comparing both length and dataofs
against len(b), but the slice it returns is b[dataofs:length], so what
actually has to hold is dataofs <= length. Those are independent:
length comes from the IPv4 total length header field, while dataofs is
derived from the sub-protocol header, and decode4 never checks that the
declared total length covers the transport header.
A 28-byte IPv4/UDP packet declaring a total length of 20 decodes to
length=20, dataofs=28, len(b)=28, and Payload then evaluates b[28:20].
ICMPv4 and TCP reach the same state with 24- and 40-byte packets.
I found this by fuzzing Decode and then calling the accessors on the
result. I did not find a caller that can be driven into it from the
network: wireguard-go truncates decrypted packets to the declared IP
length, so on the inbound path dataofs > length implies dataofs >
len(b) and the existing guard already catches it.
Add a FuzzParsedPayload target that decodes and then calls Payload,
seeded with valid IPv4/IPv6 packets and with invalid ones, including
the three short total length packets above, and build it in
fuzz/oss-fuzz.sh.
Fixes#21231
Change-Id: Ie5d2100464b79750626b1bfefbe4020c4a42ca91
Signed-off-by: leoca <leo.camus23@gmail.com>
The nonce used in PCP identifies a particular port mapping. It needs
to be generated randomly for the initial mapping, and then saved so
that it can be used to renew or delete a mapping later. This is how
the router checks that we're authorized to change existing mappings.
If we receive a response for a different nonce, we should ignore it
since it's not meant for us (or the router is misbehaving).
Fixes#21127
Change-Id: Ic7874d376a74791807953f2f559c8a14a8eb75e9
Signed-off-by: Francois Marier <francois@tailscale.com>
The forwarder's upstream UDP socket is unconnected, so any host that
could send a datagram to its ephemeral port reached the single
ReadFromUDPAddrPort call that decided the query: a datagram with the
right 16-bit transaction ID was returned to the client as the
resolver's answer no matter what address it came from, and a datagram
with the wrong ID failed the query outright, handing it to the TCP
fallback (or to nothing at all, with TCP retries disabled).
sendUDP now keeps reading until a datagram arrives that could be a
reply to the query it sent: one from the resolver's address and port
carrying the transaction ID of the request. Other datagrams are
dropped and counted, so neither a spoofed reply nor a single stray
datagram can answer or end the query. This is the same source check
the kernel applies to a connected socket; the socket stays unconnected
because its ListenPacket path is what binds the query to a link
(IP_BOUND_IF on macOS, ForwardLinkSelector elsewhere). The read loop
still ends when the query context expires and closeOnCtxDone closes
the conn, as before.
On Windows an oversized datagram is reported as a truncation error
without a source address, so there the transaction ID remains the only
check, as before.
Thanks to Ben Carman for the report!
Updates tailscale/corp#48187
Reported-by: Ben Carman
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: Ibabbec5c82739b66caa6fb943faa376c39adbf64
SetBufferSize sets the socket buffer to the requested size rather than
growing it, so on a host whose net.core.{r,w}mem_default is above the
7MiB the test asks for, the buffers legitimately shrink and the test
fails. Only assert that a buffer we asked to enlarge did not shrink.
Also log curSnd/newSnd, rather than the receive values twice, in the
SO_SNDBUF line.
Fixes#15994
Signed-off-by: aza <DevAza23@users.noreply.github.com>
Ignore malformed D-Bus NameOwnerChanged signals before reading their
body. A short signal previously logged an error but continued indexing
Body[0] and Body[2], which could panic the resolved manager run loop.
Also report the malformed body length correctly and add coverage for
malformed, stopped, and restarted systemd-resolved signals.
Updates #21270
Signed-off-by: alexchang <dragoonchang@gmail.com>
A tailnet peer can remotely crash tailscaled by sending a DERP-sealed
disco CallMeMaybeVia message with an all-zero ServerDisco key. The
decoder accepts the zero key, and the relay manager later hands it to
DiscoPrivate.Shared, which panics on zero keys. The sender only needs
to be a relay-capable peer in the victim's netmap.
Auditing the other DiscoPrivate.Shared call sites reachable from
decoded messages turned up the same bug on the relay server side.
AllocateUDPRelayEndpointRequest.ClientDisco is attacker-chosen: one
slot must match the sender's disco key, and the other can be zero. It
flows unchecked into udprelay.Server.AllocateEndpoint, which calls
Shared on both client keys and panics in its eventbus subscriber
goroutine. AllocateEndpoint now rejects zero client keys with an error,
which its only caller already handles by logging.
Thanks to Ben Carman for the report!
Updates tailscale/corp#48187
Reported-by: Ben Carman
Change-Id: Ifc0f64d8f63270100b22c06e9f759dce624ab811
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This change expands our fuzzing coverage in protocol and parsing logic. No issues discovered from this fuzzing. Wiring into oss-fuzz for continual coverage.
Updates https://github.com/tailscale/corp/issues/46608
Change-Id: I6b5218cb1103ccc5b957c512a10d87f637c4b6e5
Signed-off-by: Mike Jensen <mikej@tailscale.com>
This is a follow-up to #21029 (aa2681ac5f) to make it a bit stricter
and not cache DNS results until they've passed TLS cert validation,
to weed out DNS servers that are lying (like captive portals).
Because this is done via dnscache.TLSDialer we only catch the control
connection, but that's fine. That's all we need to come back alive
if DNS was down because real system DNS is itself over Tailscale.
The DERP connections should come via IPv4/IPv6 fields in the DERPMap.
And the logging connection isn't important; it'll buffer and catch up
later as needed when DNS is back up.
Updates #21028
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I75f176a0222f04ed52c9de1247deeed9b911f92c
* net/dns: export OSConfigurationReadWarnable
A natlab vmtest checks that a node is not reporting this warning. Exporting
the Warnable lets the test take the warning's text from it, instead of
keeping a copy of the wording that could stop matching without failing.
Updates #20825
Updates tailscale/corp#44793
Signed-off-by: Brendan Creane <bcreane@gmail.com>
* tstest/natlab/vmtest: add VM coverage for the openresolv DNS backend
dnsMode() picks one of five Linux DNS backends, and natlab could provision
only systemd-resolved and direct. Add a DNSOpenresolv mode and a VM test for
it, so the backend behind #20825 is covered.
No cloud image ships openresolv and a guest cannot download it, so its two
source files are vendored under testdata and installed with cloud-init's
write_files. openresolv's build is a set of sed substitutions with nothing to
compile, so resolvconf.go does the substitutions in process.
Updates #20825
Updates tailscale/corp#44793
Signed-off-by: Brendan Creane <bcreane@gmail.com>
---------
Signed-off-by: Brendan Creane <bcreane@gmail.com>
TestUDPConcurrent closed its UDP connection as soon as the send loop
finished, and then required that at least one reply had arrived. With
GOMAXPROCS=1 the send loop runs to completion before the proxy's relay
goroutines are scheduled at all, so the close discarded every response.
The test failed on all 20 runs of "go test -race -count=20 -cpu=1".
Wait for the first reply, with a 10 second timeout, before closing the
connection.
This also makes the test catch the race it was written for more often.
With the fix in 0301c7493 reverted, "-cpu=2" reports the race where it
previously reported none.
Fixes#21182
Signed-off-by: Brendan Creane <bcreane@gmail.com>
Aniket