[GH-ISSUE #201] device: header_protection_key makes the whole config unusable unless all four paddings are >= 12 (error also names a non-existent S0) #95

New issue

Open

opened 2026-10-13 21:03:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

Originally created by @Capricornus007 on GitHub (Oct 8, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/201

Affected upstream source

Repository: https://github.com/amnezia-vpn/amneziawg-go
Branch / commit: master @ b5928ef
Latest release tag checked: v3.1.20260828
Location: device/uapi.go:852-858

if !d.headerProtectionKey.IsZero() {
	paddings := []uint32{d.paddings.init, d.paddings.response, d.paddings.cookie, d.paddings.transport}
	for i, padding := range paddings {
		if padding < HeaderCipherNonceSize {
			return fmt.Errorf("S%d must be more then %d to use headerProtection", i, HeaderCipherNonceSize)
		}
	}
}

Summary

When a header_protection_key is supplied, the UAPI setter requires all four paddings
(s1..s4) to be at least HeaderCipherNonceSize (12). Any padding left at its UAPI default of
0 aborts the entire dev_set transaction, so the device/peer is never configured at all.

Two separate problems in those 7 lines:

The requirement is unconditional over all four paddings. A configuration that sets only
some of s1..s4 — which the UAPI itself permits, since each key is optional and defaults to
0 — cannot enable header protection at all. The caller gets IPC error -22 and the whole
config is rejected, not just the header-protection part.
The error message is off by one and names a key that does not exist. The loop index i is
0-based while the UAPI keys are s1..s4, so a failure on s1 reports S0, and a failure on
s3 reports S2 — pointing the reader at the wrong parameter.

Reproduction

Applied to a device via the normal UAPI set path (ipcSetDevice):

private_key=<base64>
header_protection_key=<32 random bytes, base64>
s1=12
s2=12

(s3/s4 intentionally omitted, i.e. left at the UAPI default 0.)

Actual: the set fails with S2 must be more then 12 to use headerProtection (the value that
actually tripped the check is s3, and S0 is reported when s1 alone is short). The device ends
up unconfigured.

Expected: either the config is accepted and header protection is derived from the paddings that
are set, or the error names the exact offending key (s3) and documents that all four paddings
must be configured together.

Why this matters

Real-world AWG provisioning data frequently carries only a subset of the S* values (server
operators tune one or two message classes, not all four). With this check the client cannot turn on
header protection for those profiles at all, and the failure surfaces as a generic
EINVAL from dev_set, which is very hard to attribute to the padding combination.

Notes on the rework branch

The same logic exists on experimental-session-init (948c555, device/uapi.go:833-838) after the
conceal refactor: the index is corrected to i+1, but the all-four-paddings requirement is still
unconditional, so the behaviour is unchanged.

Suggested direction

Any of these would fix the reported problem; I am not asking for a specific one and would happily
test a patch:

validate only the paddings that are non-zero (treat 0 as "not configured"), or
keep the all-four requirement but return the offending key name (s3, not S2) and document the
coupling in the UAPI key list, or
derive the nonce from the largest configured padding instead of requiring a minimum on each.

Happy to send a PR with tests if you tell me which of the three you would rather have.

Originally created by @Capricornus007 on GitHub (Oct 8, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/201 ## Affected upstream source - Repository: `https://github.com/amnezia-vpn/amneziawg-go` - Branch / commit: `master` @ `b5928ef` - Latest release tag checked: `v3.1.20260828` - Location: `device/uapi.go:852-858` ```go if !d.headerProtectionKey.IsZero() { paddings := []uint32{d.paddings.init, d.paddings.response, d.paddings.cookie, d.paddings.transport} for i, padding := range paddings { if padding < HeaderCipherNonceSize { return fmt.Errorf("S%d must be more then %d to use headerProtection", i, HeaderCipherNonceSize) } } } ``` ## Summary When a `header_protection_key` is supplied, the UAPI setter requires **all four** paddings (`s1`..`s4`) to be at least `HeaderCipherNonceSize` (12). Any padding left at its UAPI default of `0` aborts the entire `dev_set` transaction, so the device/peer is never configured at all. Two separate problems in those 7 lines: 1. **The requirement is unconditional over all four paddings.** A configuration that sets only some of `s1`..`s4` — which the UAPI itself permits, since each key is optional and defaults to `0` — cannot enable header protection at all. The caller gets `IPC error -22` and the whole config is rejected, not just the header-protection part. 2. **The error message is off by one and names a key that does not exist.** The loop index `i` is 0-based while the UAPI keys are `s1`..`s4`, so a failure on `s1` reports `S0`, and a failure on `s3` reports `S2` — pointing the reader at the wrong parameter. ## Reproduction Applied to a device via the normal UAPI set path (`ipcSetDevice`): ``` private_key=<base64> header_protection_key=<32 random bytes, base64> s1=12 s2=12 ``` (`s3`/`s4` intentionally omitted, i.e. left at the UAPI default `0`.) Actual: the set fails with `S2 must be more then 12 to use headerProtection` (the value that actually tripped the check is `s3`, and `S0` is reported when `s1` alone is short). The device ends up unconfigured. Expected: either the config is accepted and header protection is derived from the paddings that are set, or the error names the exact offending key (`s3`) and documents that all four paddings must be configured together. ## Why this matters Real-world AWG provisioning data frequently carries only a subset of the `S*` values (server operators tune one or two message classes, not all four). With this check the client cannot turn on header protection for those profiles at all, and the failure surfaces as a generic `EINVAL` from `dev_set`, which is very hard to attribute to the padding combination. ## Notes on the rework branch The same logic exists on `experimental-session-init` (`948c555`, `device/uapi.go:833-838`) after the `conceal` refactor: the index is corrected to `i+1`, but the all-four-paddings requirement is still unconditional, so the behaviour is unchanged. ## Suggested direction Any of these would fix the reported problem; I am not asking for a specific one and would happily test a patch: - validate only the paddings that are non-zero (treat `0` as "not configured"), or - keep the all-four requirement but return the offending key name (`s3`, not `S2`) and document the coupling in the UAPI key list, or - derive the nonce from the largest configured padding instead of requiring a minimum on each. Happy to send a PR with tests if you tell me which of the three you would rather have.

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#95

No description provided.

Rows
Columns