amnezia-vpn/amneziawg-go

Fork 0

mirror of https://github.com/amnezia-vpn/amneziawg-go.git synced 2026-10-14 05:41:54 +00:00

[PR #129] [MERGED] fix: apply S4 transport padding to keepalive packets #155

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amneziawg-go/pull/129
Author: @wizier
Created: 3/31/2026
Status: ✅ Merged
Merged: 5/13/2026
Merged by: @ygurov

Base: master ← Head: fix/s4-keepalive-padding

📝 Commits (2)

0e03177 fix: apply S4 transport padding to keepalive packets
f19f048 Merge branch 'master' into fix/s4-keepalive-padding

📊 Changes

1 file changed (+8 additions, -9 deletions)

View changed files

📝 device/send.go (+8 -9)

📄 Description

Bug

When S4 (transport padding) is non-zero, keepalive packets are sent without S4 padding, but the receiving side expects all transport packets to include it. This causes keepalives to be silently dropped, preventing the responder from ever calling timersHandshakeComplete().

Root cause

In device/send.go:574-587, RoutineSequentialSender only adds S4 padding inside the if len(elem.packet) != MessageKeepaliveSize branch:

for _, elem := range elemsContainer.elems {
    if len(elem.packet) != MessageKeepaliveSize {
        dataSent = true

        if padding := device.paddings.transport; padding > 0 {
            // ... prepend S4 random bytes ...
        }
    }
    bufs = append(bufs, elem.packet)
}

Keepalive packets (len == MessageKeepaliveSize) skip the padding entirely.

On the receive side (device/receive.go:602-611), DeterminePacketTypeAndPadding unconditionally expects S4 padding on all transport packets:

if size >= padding+MessageTransportHeaderSize {
    data := packet[padding:]
    if header.Validate(binary.LittleEndian.Uint32(data)) {
        return MessageTransportType, padding
    }
}

An unpadded keepalive (32 bytes) passes the size check when S4 is small, but packet[S4:] points into the middle of the real header — H4 validation fails and the packet is classified as MessageUnknownType → dropped.

What happens

Initiator completes handshake → stores keypair as current → calls timersHandshakeComplete() → sends keepalive
Responder sends handshake response (with S2 padding, separate code path) → stores keypair as next (not current) → waits for first transport packet to promote keypair and call timersHandshakeComplete()
Keepalive arrives without S4 padding → DeterminePacketTypeAndPadding fails → packet dropped
Responder's lastHandshakeNano stays at 0 — keypair is never promoted from next to current
Real data packets have S4 padding applied → arrive correctly → handshake completes on responder

When S4 = 0 there is no issue — no padding expected, keepalive passes normally.

Fix

device/send.go:574-587 — dataSent and padding are independent concerns: dataSent controls timersDataSent(), padding is a property of all transport packets. Close the dataSent guard before the padding block so it applies to keepalives too.

_{🔄 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/129 **Author:** [@wizier](https://github.com/wizier) **Created:** 3/31/2026 **Status:** ✅ Merged **Merged:** 5/13/2026 **Merged by:** [@ygurov](https://github.com/ygurov) **Base:** `master` ← **Head:** `fix/s4-keepalive-padding` --- ### 📝 Commits (2) - [`0e03177`](https://github.com/amnezia-vpn/amneziawg-go/commit/0e03177125c48e6e2df72fa2f3af8e3da44cc17c) fix: apply S4 transport padding to keepalive packets - [`f19f048`](https://github.com/amnezia-vpn/amneziawg-go/commit/f19f048b3e52a5247b99f2d72568af670585b423) Merge branch 'master' into fix/s4-keepalive-padding ### 📊 Changes **1 file changed** (+8 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `device/send.go` (+8 -9) </details> ### 📄 Description ## Bug When S4 (transport padding) is non-zero, keepalive packets are sent **without** S4 padding, but the receiving side expects **all** transport packets to include it. This causes keepalives to be silently dropped, preventing the responder from ever calling `timersHandshakeComplete()`. ## Root cause In `device/send.go:574-587`, `RoutineSequentialSender` only adds S4 padding inside the `if len(elem.packet) != MessageKeepaliveSize` branch: ```go for _, elem := range elemsContainer.elems { if len(elem.packet) != MessageKeepaliveSize { dataSent = true if padding := device.paddings.transport; padding > 0 { // ... prepend S4 random bytes ... } } bufs = append(bufs, elem.packet) } ``` Keepalive packets (`len == MessageKeepaliveSize`) skip the padding entirely. On the receive side (`device/receive.go:602-611`), `DeterminePacketTypeAndPadding` unconditionally expects S4 padding on all transport packets: ```go if size >= padding+MessageTransportHeaderSize { data := packet[padding:] if header.Validate(binary.LittleEndian.Uint32(data)) { return MessageTransportType, padding } } ``` An unpadded keepalive (32 bytes) passes the size check when S4 is small, but `packet[S4:]` points into the middle of the real header — H4 validation fails and the packet is classified as `MessageUnknownType` → dropped. ## What happens 1. **Initiator** completes handshake → stores keypair as `current` → calls `timersHandshakeComplete()` → sends keepalive 2. **Responder** sends handshake response (with S2 padding, separate code path) → stores keypair as `next` (not `current`) → waits for first transport packet to promote keypair and call `timersHandshakeComplete()` 3. Keepalive arrives **without S4 padding** → `DeterminePacketTypeAndPadding` fails → packet dropped 4. Responder's `lastHandshakeNano` stays at **0** — keypair is never promoted from `next` to `current` 5. Real data packets have S4 padding applied → arrive correctly → handshake completes on responder **When S4 = 0 there is no issue** — no padding expected, keepalive passes normally. ## Fix `device/send.go:574-587` — `dataSent` and padding are independent concerns: `dataSent` controls `timersDataSent()`, padding is a property of all transport packets. Close the `dataSent` guard before the padding block so it applies to keepalives too. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

amnezia-vpn/amneziawg-go#155

No description provided.

Rows
Columns