The Cloudflare "reserved" bytes (1-3) were zeroed unconditionally on
every received datagram across all StdNetBind/WinRingBind receive paths.
AmneziaWG reads its magic header as LittleEndian.Uint32(packet[padding:])
where padding is s1/s2/s4; with small padding (0-3) the magic overlaps
bytes 1-3, so clearing them collapses it out of the ranged h1-h4 window
and every packet is dropped (handshake included) — the AWG endpoint
never comes up. Plain WG (types 1-4, bytes 1-3 already zero) and large
padding are unaffected, which is why it went unnoticed.
Gate all five receive clears (bind_std receiveIP, msgx_darwin
receiveSingle + makeReceiveMsgX, bind_windows receiveIPv4/v6) behind a
new hasReserved() so bytes 1-3 are only touched when a WARP reserved
value is actually configured. Send paths already gate on a per-endpoint
loaded/non-zero check, so they are left unchanged. The reserved map is
populated before the receive goroutines start and never mutated after,
so the lock-free read is safe.
Tests: awg_stdnetbind_reserved_lx_test.go brings up two Devices over
StdNetBind with zero padding (magic in bytes 0-3) and asserts delivery
(red before the fix, green after); reserved_gate_lx_test.go pins the
hasReserved() gate.
Transport padding (s4) crashed the whole process with
"index out of range" in RoutineSequentialSender on the first data
packet: InputPacket/InputPackets sized elem.buffer without headroom
for the in-buffer right-shift that prepends the random prefix.
- send.go: reserve paddings.transport in both injection-path
allocLength computations; replace the manual backward byte loop
with an overlap-safe copy; defensively grow the buffer (pool-backed)
if it still lacks headroom, dropping packets that cannot fit a
single WG message instead of overrunning.
- receive.go: drop the rxBytes/timers block duplicated by the AWG
re-graft (rx accounting was doubled, keepKeyFreshReceiving fired
twice per batch).
- send.go: swap jmin/jmax when configured inverted (UAPI validates
the fields only individually; a swapped pair panicked rand.Int
with a non-positive bound on the first handshake).
- obf*.go: bound obfuscator length args to [0, MaxMessageSize]
(negative panicked slice bounds, huge ones OOMed the handshake).
- magic-header.go: widen to int64 before end-start+1 so a full-range
header cannot wrap to a zero rand.Int bound.
Tests: transport_padding_test.go reproduces the on-device crash
byte-for-byte (red on the previous commit, green now) across both
injection paths and the tun path; obf_guards_test.go pins the
config-value guards.
Rebase of the AWG obf graft (was e5feca7 on v0.0.3) onto v0.0.5
(2c27bbf4f9, 'Add L3 forwarding support'). 15 of 16 graft files
applied clean via 3-way; only send.go conflicted, on a single line
(upstream queuedOutboundPackets backpressure decrement vs a graft
blank line — took upstream).
Key invariant preserved: MessageEncapsulatingTransportSize=0 (graft
zeroes the sagernet encapsulating headroom; AWG obfuscation composes
the prefix itself via SendBuffers, not Bind.Send prepend). Upstream's
InputPacket/InputPackets and the new size-based outbound buffer pool
(GetOutboundBuffer/PutOutboundBuffer) are taken verbatim; the graft's
RoutineEncryption (header at buffer start) and transport-padding shift
in RoutineSequentialSender re-woven around them.
Builds clean on linux/android/windows/darwin (device/conn/tun).
InputPacket/InputPackets used the deprecated trie-only AllowedIPs.Lookup. With tailscale v1.102 a PeerByIPPacketFunc is installed and the trie is no longer populated, so every injected packet was unmatched. Use LookupFromPacket, and size the sequential sender scratch for full input batches instead of capping containers at the device batch size.
Add SetPriorityMessageOnEstablishmentFunc, which registers a
PeerPriorityMessageFunc callback invoked when a peer's session keypair
is established or re-keyed for forward data transmission. The bytes it
returns are transmitted to the peer as a transport message.
The message is "priority" in two senses: it bypasses the staged packet
queue entirely, so it cannot be evicted by TUN-sourced packets, and it
is enqueued ahead of the keepalive/staged packets that follow keypair
establishment.
Updates tailscale/tailscale#20081
Signed-off-by: Jordan Whited <jordan@tailscale.com>
API introduced in a927a66e has two cases of state determination
happening out of critical section for the state value:
(1) expiredSession loads sessionExpiresNano, then releases all locks
and calls noteSessionState(Expired). So a concurrent refresh that
lands in that gap gets clobbered by a stale Expired -- and sticks
until the next re-key.
(2) Likewise in noteSessionHandshakeStopped, hasKeyMaterial check
happens out of the session state lock and races with ZeroAndFlushAll.
Both lead to a wrong state emitted via the device.sessionState.fn,
but are otherwise benign.
This moves the expiry timestamp under a lock to address the former,
and provides a noteSessionStateLocked helper for the latter.
Also changes API semantics to serialize events per-peer, to avoid
sharing a single lock for all timestamps.
Updates tailscale/corp#42874
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
Change-Id: Iee2cdf135375519e58a8e84362349d966a6a6964
e3ac4a0afb4e introduced a lightweight API that can be used instead of UAPI
to reconfigure peers. Peer state created via the new PeerLookupFunc
is not set to expire until the handshake succeeds, making device leak two
goroutines and a set of buffers for each failed handshake.
This change arms the expiry timer before the handshake gets to proceed.
Updates tailscale/tailscale#20183
Change-Id: Ibc0abb6eec97aca0a10f50515dea9e0d6a6a6964
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
In PR #66, we tried to address a memory leak by avoiding
runtime.SetFinalizer for autodrainingInboundQueue and
autodrainingOutboundQueue unless there was something to do.
However, when there is work to be done, these finalizers still leak
memory because they’re still holding on to a cyclical reference to q.
This applies to any platform that relies on a bounded device.WaitPool,
like Android and iOS which both declare PreallocatedBuffersPerPool.
This patch converts this logic to runtime.AddCleanup which is designed
to avoid this problem.
Updates tailscale/corp#42776
Signed-off-by: Simon Law <sfllaw@tailscale.com>
Device-side portion of upstream tailscale/wireguard-go e3ac4a0
(device, cmd/check-lockorder: add static analysis tool for lock
ordering); the analyzer itself is not carried in this fork.
In tailscale/wireguard-go#65, @lkosewsk reproduced a memory leak seen
in prod with lots of wireguard-go instances being created and
destroyed, where they were still being retained forever due to cycles
in the runtime.SetFinalizer reference graph.
Really we shouldn't be using runtime.SetFinalizer anywhere. But we
still use it on mobile platforms in WaitPool. But those platforms
don't have thousands of tsnet.Server instances coming & going, so this
is a half fix: avoid the finalizer registration on Linux, etc where
the queue doesn't need to be drained and there's no WaitPool
accounting. Just let GC handle it, without adding finalizer cycle
complexity.
Updates tailscale/corp#42776
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Discovered by a tool + test that will come in a future change.
Updates tailscale/tailscale#19513
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
On a connected socket sendmsg_x sends a whole batch in one syscall;
msg_name is unsupported there, so batched sends require connecting to
the peer, which loses roaming and is therefore opt-in via
SetSinglePeerMode for single fixed-peer binds. recvmsg_x fills in
per-message source addresses, so batched receive works on unconnected
sockets too. Any unexpected errno permanently falls back to the generic
paths. iOS is excluded: in the Network Extension recvmsg_x on
unconnected UDP sockets delivers no data, and connected sockets stop
passing traffic after a rebind.
coalesceMessages copied every additional datagram into the spare
capacity of the first buffer, which no longer exists now that element
buffers are sized to their packet; append the datagrams as iovecs
instead, which also removes the copy.
Outbound element buffers now come from the sing allocator sized to the
actual packet instead of the bounded MaxMessageSize pool, element and
container pools become plain sync.Pools, and the bounded message buffer
pool serves only the receive path. Packets injected via
InputPacket/InputPackets are dropped before they are copied once a peer
has 2048 packets queued: injection runs on the caller's read loop, which
must never block on pool exhaustion, and the queues are bounded in
containers, so a flood was buffered instead of dropped.
(cherry picked from commit 7c2acadba17cadf8a1df957c49e1333130d460ad)
(cherry picked from commit a7bac1754e7717e1d4009d1ffd2d13330067d631)
(cherry picked from commit 7a2f11c693b49e784318bbf987173095c67b563d)
Kernels below 5.12 are missing this:
commit 98184612aca0a9ee42b8eb0262a49900ee9eef0d
Author: Norman Maurer <norman_maurer@apple.com>
Date: Thu Apr 1 08:59:17 2021
net: udp: Add support for getsockopt(..., ..., UDP_GRO, ..., ...);
Support for UDP_GRO was added in the past but the implementation for
getsockopt was missed which did lead to an error when we tried to
retrieve the setting for UDP_GRO. This patch adds the missing switch
case for UDP_GRO
Fixes: e20cf8d3f1f7 ("udp: implement GRO for plain UDP sockets.")
Signed-off-by: Norman Maurer <norman_maurer@apple.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
That means we can't set the option and then read it back later. Given
how buggy UDP_GRO is in general on odd kernels, just disable it on older
kernels all together.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This is already enforced in receive.go, but if these unmarshallers are
to have error return values anyway, make them as explicit as possible.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
It should be POLLIN because closeFd is read-only file.
Signed-off-by: Kurnia D Win <kurnia.d.win@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Colin's commit went a step further and protected tun.incomingPacket with
a lock on shutdown, but let's see if the tun.stack.Close() call actually
solves that on its own.
Suggested-by: kshangx <hikeshang@hotmail.com>
Suggested-by: Colin Adler <colin1adler@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>