[PR #184] fix: send first TUN packet with the configured S4 padding #194

New issue

Open

opened 2026-10-13 21:04:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 21:04:27 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amneziawg-go/pull/184
Author: @AYastrebov
Created: 8/25/2026
Status: 🔄 Open

Base: master ← Head: fix/tun-padding-race-on-first-packet

📝 Commits (3)

b84b56f fix: send first TUN packet with the configured S4 padding
0e204de test: cover the S4 padding captured before the interface is configured
fcbeecf test: close devices built before a later one aborts the test

📊 Changes

2 files changed (+176 additions, -2 deletions)

View changed files

📝 device/send.go (+27 -2)
➕ device/send_awg_test.go (+149 -0)

📄 Description

Problem

The first packet an AmneziaWG interface sends is normally dropped by the peer.

RoutineReadFromTUN is started by NewDevice, before IpcSet has applied any interface configuration. It has to choose a read offset before it can read, so it loads S4 and then blocks in tun.Read — capturing, on that first pass, the default S4 of 0 rather than the configured value.

RoutineEncryption writes the transport header at buffer[padding:padding+MessageTransportHeaderSize] and requires the payload to follow it directly, so that datagram goes out with its type field at offset 0 while the peer looks for it at offset S4. The peer decodes ciphertext there, matches no header range, and drops the packet as unknown.

The same window applies to any later s4 change over UAPI, which took effect one batch of packets late.

Evidence

This is what makes TestAWGDevicePing flaky — it passed roughly 1 run in 5 on master. On a second machine (Linux, 20 cores) it is not flaky at all but simply broken: 0 runs out of 6 passed on master, and 6 out of 6 with this change. The window is a scheduling race, so how often it is lost depends on the machine. Each device in that test sends exactly one packet, so the dropped first packet has no chance to recover and the test times out.

Instrumenting the padding each TUN reader captured correlated perfectly with the outcome:

run	dev0	dev1	result
1, 2	0	0	both subtests fail
3	0	25	only `pong` (dev0→dev1) fails
4	25	25	pass

Run 3 is the clincher: dev0 captured 0, and pong is exactly the direction dev0 sends.

Receiver-side instrumentation agreed — the dropped datagram was 64 bytes (16 header + 32 content + 16 tag, i.e. zero crypto padding) carrying a random value at offset 25, against a configured h4 of 32345-32350.

Fix

The offset still has to be chosen before the read, so S4 is re-read once the packets are in hand and they are re-aligned within their buffer if it moved. copy is memmove-safe for overlapping slices; a packet is dropped if a larger S4 leaves no room for it.

Test

The bug is a scheduling race, so the test removes the scheduling from it. A TUN wrapper closes a channel on its first Read before delegating, which proves the reader has already captured its S4 and parked. The test waits on that for both devices, asserts the captured offset still corresponds to S4 = 0 (i.e. the window is genuinely open), and only then applies s1–s4/h1–h4. The ping that follows is the first packet either interface sends.

Without the fix: fails by timeout, 3/3.
With the fix: passes, 5/5.

Verification

go build ./..., gofmt -l device/ — clean
go test ./... — all packages pass
go test -race ./device/ — passes
TestAWGDevicePing — 10/10, previously ~1 in 5 (and 6/6 against 0/6 on a second machine)

go vet still reports the pre-existing copylock warning at device/pools_test.go:67, which is unrelated to this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TGVkr62MPBrFFzTZdXPs7e

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/amnezia-vpn/amneziawg-go/pull/184 **Author:** [@AYastrebov](https://github.com/AYastrebov) **Created:** 8/25/2026 **Status:** 🔄 Open **Base:** `master` ← **Head:** `fix/tun-padding-race-on-first-packet` --- ### 📝 Commits (3) - [`b84b56f`](https://github.com/amnezia-vpn/amneziawg-go/commit/b84b56f7d12be7e95e253df84f0db301aae7e94b) fix: send first TUN packet with the configured S4 padding - [`0e204de`](https://github.com/amnezia-vpn/amneziawg-go/commit/0e204de1425a1c222e7cbb56cf34a201c6ecca85) test: cover the S4 padding captured before the interface is configured - [`fcbeecf`](https://github.com/amnezia-vpn/amneziawg-go/commit/fcbeecfd6d73f37e3160190553700e544f8e9a79) test: close devices built before a later one aborts the test ### 📊 Changes **2 files changed** (+176 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `device/send.go` (+27 -2) ➕ `device/send_awg_test.go` (+149 -0) </details> ### 📄 Description ## Problem The first packet an AmneziaWG interface sends is normally dropped by the peer. `RoutineReadFromTUN` is started by `NewDevice`, before `IpcSet` has applied any interface configuration. It has to choose a read offset before it can read, so it loads S4 and then blocks in `tun.Read` — capturing, on that first pass, the default S4 of `0` rather than the configured value. `RoutineEncryption` writes the transport header at `buffer[padding:padding+MessageTransportHeaderSize]` and requires the payload to follow it directly, so that datagram goes out with its type field at offset 0 while the peer looks for it at offset S4. The peer decodes ciphertext there, matches no header range, and drops the packet as unknown. The same window applies to any later `s4` change over UAPI, which took effect one batch of packets late. ## Evidence This is what makes `TestAWGDevicePing` flaky — it passed roughly 1 run in 5 on master. On a second machine (Linux, 20 cores) it is not flaky at all but simply broken: **0 runs out of 6 passed** on master, and 6 out of 6 with this change. The window is a scheduling race, so how often it is lost depends on the machine. Each device in that test sends exactly one packet, so the dropped first packet has no chance to recover and the test times out. Instrumenting the padding each TUN reader captured correlated perfectly with the outcome: | run | dev0 | dev1 | result | |-----|------|------|--------| | 1, 2 | 0 | 0 | both subtests fail | | 3 | 0 | 25 | only `pong` (dev0→dev1) fails | | 4 | 25 | 25 | pass | Run 3 is the clincher: dev0 captured 0, and `pong` is exactly the direction dev0 sends. Receiver-side instrumentation agreed — the dropped datagram was 64 bytes (16 header + 32 content + 16 tag, i.e. **zero** crypto padding) carrying a random value at offset 25, against a configured `h4` of `32345-32350`. ## Fix The offset still has to be chosen before the read, so S4 is re-read once the packets are in hand and they are re-aligned within their buffer if it moved. `copy` is memmove-safe for overlapping slices; a packet is dropped if a larger S4 leaves no room for it. ## Test The bug is a scheduling race, so the test removes the scheduling from it. A TUN wrapper closes a channel on its first `Read` before delegating, which proves the reader has already captured its S4 and parked. The test waits on that for both devices, asserts the captured offset still corresponds to S4 = 0 (i.e. the window is genuinely open), and only then applies `s1`–`s4`/`h1`–`h4`. The ping that follows is the first packet either interface sends. - Without the fix: fails by timeout, 3/3. - With the fix: passes, 5/5. ## Verification - `go build ./...`, `gofmt -l device/` — clean - `go test ./...` — all packages pass - `go test -race ./device/` — passes - `TestAWGDevicePing` — 10/10, previously ~1 in 5 (and 6/6 against 0/6 on a second machine) `go vet` still reports the pre-existing copylock warning at `device/pools_test.go:67`, which is unrelated to this change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01TGVkr62MPBrFFzTZdXPs7e --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-13 21:04:27 +00:00

bsakaguchi referenced this issue

2026-10-13 21:04:30 +00:00

[PR #194] Backport some wireguard-go upstream fixes #199

bsakaguchi referenced this issue

2026-10-13 21:04:32 +00:00

[PR #205] fix: clear the timer duration under modifyingLock #203