From cfd596a48935463501d2a87f77eeba7d8940827b Mon Sep 17 00:00:00 2001 From: Sanaei Date: Wed, 9 Sep 2026 00:57:24 +0200 Subject: [PATCH] fix(amneziawg): let a cleared header protection key reach a running device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit amneziawg-go reads an absent UAPI line as "keep the current value", and addressFingerprint keys only on the addresses and MTU, so an obfuscation-only edit reconfigures in place rather than rebuilding. Clearing headerProtectionKey therefore never took effect: the device kept protecting headers with the old key. The stale key also keeps the S1-S4 minimum in force, so lowering S3/S4 in the same edit made every later IpcSet fail with -22 — after replace_peers had already dropped the peers. Send the all-zero key when the field is empty, which is how the UAPI expresses "disabled"; an empty value would be rejected, since it decodes to zero bytes. --- internal/amneziawgnet/device.go | 8 +++-- internal/amneziawgnet/device_test.go | 47 +++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/internal/amneziawgnet/device.go b/internal/amneziawgnet/device.go index 8527f62ed..91e2a7330 100644 --- a/internal/amneziawgnet/device.go +++ b/internal/amneziawgnet/device.go @@ -205,13 +205,17 @@ func buildUAPIConfig(inst amneziawg.Instance, opts DeviceOptions) (string, error writeOptionalLine(&b, "i4", o.I4) writeOptionalLine(&b, "i5", o.I5) + // An omitted line means "unchanged" to amneziawg-go, so a cleared key can + // only reach a live device as the all-zero one that disables the feature. + hpHex := strings.Repeat("0", 64) if opts.HeaderProtectionKey != "" { - hpHex, err := wireguard.KeyToHex(opts.HeaderProtectionKey) + var err error + hpHex, err = wireguard.KeyToHex(opts.HeaderProtectionKey) if err != nil { return "", fmt.Errorf("invalid header protection key: %w", err) } - fmt.Fprintf(&b, "header_protection_key=%s\n", hpHex) } + fmt.Fprintf(&b, "header_protection_key=%s\n", hpHex) if opts.ContentPaddingAddition != "" { fmt.Fprintf(&b, "content_padding_addition=%s\n", opts.ContentPaddingAddition) } diff --git a/internal/amneziawgnet/device_test.go b/internal/amneziawgnet/device_test.go index c5b2231b1..1eda9c71c 100644 --- a/internal/amneziawgnet/device_test.go +++ b/internal/amneziawgnet/device_test.go @@ -191,7 +191,12 @@ func TestBuildUAPIConfigHeaderProtectionAndContentPaddingLines(t *testing.T) { if err != nil { t.Fatalf("buildUAPIConfig with empty options: %v", err) } - if strings.Contains(conf, "header_protection_key=") || strings.Contains(conf, "content_padding_addition=") { + // header_protection_key is the exception: an omitted line reads as + // "unchanged", so clearing the key has to be sent as the all-zero one. + if !strings.Contains(conf, "header_protection_key="+strings.Repeat("0", 64)+"\n") { + t.Fatalf("an unset key must be emitted as the all-zero key, got:\n%s", conf) + } + if strings.Contains(conf, "content_padding_addition=") { t.Fatalf("empty DeviceOptions must not emit AWG 3.0 lines, got:\n%s", conf) } @@ -624,3 +629,43 @@ func TestValidatedObfuscationAlwaysApplies(t *testing.T) { }) } } + +// Clearing HeaderProtectionKey on a running inbound must actually reach the +// device: amneziawg-go treats an absent UAPI line as "keep the current value", +// so an omitted key leaves header protection permanently on. Worse, the stale +// key keeps the S1-S4 minimum alive, so lowering them then fails IpcSet with +// -22 on every reconcile after the peers were already replaced. +func TestBuildUAPIConfigClearedHeaderProtectionKeyIsSentAsZero(t *testing.T) { + priv, _, err := wireguard.GenerateWireguardKeypair() + if err != nil { + t.Fatalf("generate keypair: %v", err) + } + inst := amneziawg.Instance{ + PrivateKey: priv, + Obfuscation: amneziawg.Obfuscation31{S1: 20, S2: 20, S3: 20, S4: 20}, + } + + key, err := wireguard.GenerateWireguardPSK() + if err != nil { + t.Fatalf("generate header protection key: %v", err) + } + withKey, err := buildUAPIConfig(inst, DeviceOptions{HeaderProtectionKey: key}) + if err != nil { + t.Fatalf("buildUAPIConfig with a key: %v", err) + } + cleared, err := buildUAPIConfig(inst, DeviceOptions{}) + if err != nil { + t.Fatalf("buildUAPIConfig with the key cleared: %v", err) + } + if withKey == cleared { + t.Fatal("clearing the key produced an identical UAPI config, so the device would never see the change") + } + + zero := "header_protection_key=" + strings.Repeat("0", 64) + "\n" + if !strings.Contains(cleared, zero) { + t.Fatalf("cleared config must carry the all-zero key, got:\n%s", cleared) + } + if strings.Contains(withKey, zero) { + t.Fatalf("a configured key must not be emitted as zero, got:\n%s", withKey) + } +}