[GH-ISSUE #185] device: handshake message padding ignores ContentPaddingAddition, unlike transport packets #86

New issue

Open

opened 2026-10-13 21:03:26 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-13 21:03:26 +00:00

Owner

Originally created by @kuzzrus on GitHub (Aug 25, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/185

Affected upstream source

Repository: https://github.com/amnezia-vpn/amneziawg-go
Version: v3.1.20260814

Summary

In device/send.go, transport (data) packets and handshake messages use two
different padding-precedence rules, and I don't think that's intentional.

For transport packets, RoutineEncryption checks ContentPaddingAddition
first and only falls back to RandomTrailers when it's unset:

https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L611-L620

paddingSize := device.randomPaddingAddition(packetSize, mtu)
if paddingSize < 0 {
    paddingSize = elem.peer.randomTrailer(packetSize + MinMessageSize + int(elem.padding))
}

randomPaddingAddition only returns negative when ContentPaddingAddition
itself is unset (addition.IsZero()), and its own output is explicitly
clamped to mtu - packetSize:

https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L533-L552

So on the transport path, configuring ContentPaddingAddition fully
suppresses RandomTrailers, and whichever one does run is bounded by the
tunnel's own MTU.

sendHandshakeInitiation / sendHandshakeResponse don't follow this rule at
all -- they call peer.randomTrailer() unconditionally, with no
ContentPaddingAddition check anywhere on that path:

https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L148-L150
https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L198-L200

peer.randomTrailer() itself is bounded only by peer.udpWindow -- a
per-peer high-water mark of the largest packet ever sent to that peer, which
never shrinks and has no relationship to MTU at all:

https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L565-L575

So a device with ContentPaddingAddition configured -- specifically to keep
padding MTU-safe -- still emits MTU-unbounded, RandomTrailers-sized padding
on every handshake message, and this isn't a one-time startup cost: it
recurs on every rekey (RekeyAfterTime, itself a configurable AWG 3.1
field, commonly well under 3 minutes).

Empirical confirmation

Real handshakes, ContentPaddingAddition=22-95 configured throughout (a
production value, not synthetic), 15 rounds per condition, measuring the
real wire size of the server's MessageResponse packet:

RandomTrailers=false: 15/15 responses at exactly 230 bytes, zero variance
RandomTrailers=true:  235-494 bytes, avg 368 (1.6x), max 494 (2.1x)

Same ContentPaddingAddition value active in both rows. If the transport
path's precedence applied here too, both rows should be identical.

Why this seems worth fixing

ContentPaddingAddition's own clamp-to-MTU logic shows the field exists
specifically to give operators an MTU-safe padding option. An operator who
deliberately sets it to stay MTU-safe has no way to know their handshake
traffic is still being padded by an entirely different, MTU-unbounded
mechanism -- the random_trailers UAPI key doesn't mention handshake
messages, and nothing about ContentPaddingAddition suggests it only
applies to some packet types.

I don't have a proven end-to-end throughput measurement tying this
specifically to a real-world slowdown -- flagging it as a data point, not a
claim -- but the asymmetry itself seems clear and, I'd guess, unintentional
given how carefully the transport path's precedence is implemented.

Possible fix directions

Not sure which of these matches the intended design, so not proposing a
specific diff:

Make the handshake-message path respect the same
ContentPaddingAddition-first precedence RoutineEncryption already
uses, or
At minimum, bound the handshake-path randomTrailer() call by MTU the
same way randomPaddingAddition already is, instead of leaving it keyed
only to the unbounded peer.udpWindow history.

Happy to share the two Go tests that produced the numbers above if useful,
though they live in a downstream project's test suite rather than this
repo's own.

Originally created by @kuzzrus on GitHub (Aug 25, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/185 ## Affected upstream source - Repository: `https://github.com/amnezia-vpn/amneziawg-go` - Version: `v3.1.20260814` ## Summary In `device/send.go`, transport (data) packets and handshake messages use two different padding-precedence rules, and I don't think that's intentional. For transport packets, `RoutineEncryption` checks `ContentPaddingAddition` first and only falls back to `RandomTrailers` when it's unset: https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L611-L620 ```go paddingSize := device.randomPaddingAddition(packetSize, mtu) if paddingSize < 0 { paddingSize = elem.peer.randomTrailer(packetSize + MinMessageSize + int(elem.padding)) } ``` `randomPaddingAddition` only returns negative when `ContentPaddingAddition` itself is unset (`addition.IsZero()`), and its own output is explicitly clamped to `mtu - packetSize`: https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L533-L552 So on the transport path, configuring `ContentPaddingAddition` fully suppresses `RandomTrailers`, and whichever one does run is bounded by the tunnel's own MTU. `sendHandshakeInitiation` / `sendHandshakeResponse` don't follow this rule at all -- they call `peer.randomTrailer()` unconditionally, with no `ContentPaddingAddition` check anywhere on that path: https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L148-L150 https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L198-L200 `peer.randomTrailer()` itself is bounded only by `peer.udpWindow` -- a per-peer high-water mark of the largest packet ever sent to that peer, which never shrinks and has no relationship to MTU at all: https://github.com/amnezia-vpn/amneziawg-go/blob/v3.1.20260814/device/send.go#L565-L575 So a device with `ContentPaddingAddition` configured -- specifically to keep padding MTU-safe -- still emits MTU-unbounded, `RandomTrailers`-sized padding on every handshake message, and this isn't a one-time startup cost: it recurs on every rekey (`RekeyAfterTime`, itself a configurable AWG 3.1 field, commonly well under 3 minutes). ## Empirical confirmation Real handshakes, `ContentPaddingAddition=22-95` configured throughout (a production value, not synthetic), 15 rounds per condition, measuring the real wire size of the server's `MessageResponse` packet: ``` RandomTrailers=false: 15/15 responses at exactly 230 bytes, zero variance RandomTrailers=true: 235-494 bytes, avg 368 (1.6x), max 494 (2.1x) ``` Same `ContentPaddingAddition` value active in both rows. If the transport path's precedence applied here too, both rows should be identical. ## Why this seems worth fixing `ContentPaddingAddition`'s own clamp-to-MTU logic shows the field exists specifically to give operators an MTU-safe padding option. An operator who deliberately sets it to stay MTU-safe has no way to know their handshake traffic is still being padded by an entirely different, MTU-unbounded mechanism -- the `random_trailers` UAPI key doesn't mention handshake messages, and nothing about `ContentPaddingAddition` suggests it only applies to some packet types. I don't have a proven end-to-end throughput measurement tying this specifically to a real-world slowdown -- flagging it as a data point, not a claim -- but the asymmetry itself seems clear and, I'd guess, unintentional given how carefully the transport path's precedence is implemented. ## Possible fix directions Not sure which of these matches the intended design, so not proposing a specific diff: - Make the handshake-message path respect the same `ContentPaddingAddition`-first precedence `RoutineEncryption` already uses, or - At minimum, bound the handshake-path `randomTrailer()` call by MTU the same way `randomPaddingAddition` already is, instead of leaving it keyed only to the unbounded `peer.udpWindow` history. Happy to share the two Go tests that produced the numbers above if useful, though they live in a downstream project's test suite rather than this repo's own.

bsakaguchi commented

2026-10-13 21:03:26 +00:00

Author

Owner

@HeisLuka commented on GitHub (Sep 14, 2026):

I can prepare a focused fix for this, but I want to confirm the intended AWG 3.1 semantics before choosing the patch shape.

Should handshake messages follow the same precedence as the transport path:

ContentPaddingAddition -> suppress RandomTrailers

or should RandomTrailers still apply to handshakes, but be bounded by MTU?

If the transport-path precedence is intentional, I can submit a small PR that applies the same rule to handshake initiation/response and adds regression tests covering both features enabled together. If trailers are intentionally independent for handshakes, I would instead keep them and only add the MTU bound.

I would prefer not to encode one of those protocol semantics without maintainer confirmation.

@HeisLuka commented on GitHub (Sep 14, 2026): I can prepare a focused fix for this, but I want to confirm the intended AWG 3.1 semantics before choosing the patch shape. Should handshake messages follow the same precedence as the transport path: `ContentPaddingAddition` -> suppress `RandomTrailers` or should `RandomTrailers` still apply to handshakes, but be bounded by MTU? If the transport-path precedence is intentional, I can submit a small PR that applies the same rule to handshake initiation/response and adds regression tests covering both features enabled together. If trailers are intentionally independent for handshakes, I would instead keep them and only add the MTU bound. I would prefer not to encode one of those protocol semantics without maintainer confirmation.

bsakaguchi referenced this issue

2026-10-13 21:03:58 +00:00

[PR #86] fix: add i,j params size limitation #139