From 425a2bbf08da0605ca49bf03f1690c72a7a67320 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 31 Oct 2026 14:50:18 +0100 Subject: [PATCH] fix(amneziawg): bound what a handshake allocates and the tunnel MTU Every handshake initiation builds Jc junk packets of Jmin..Jmax bytes and the I1-I5 signature packets, and the limits allowed the full uint32 range and lengths of any size, so one connection made the embedded device allocate without bound. The MTU reaches the device as an int32 and a wider value made it panic. Both ended the whole panel at the next connection, after every restart. Jc is now at most 128, Jmin/Jmax and each signature packet at most one 65535-byte datagram, and the MTU at most 65535. Saves refuse more, the device setup refuses rows stored before, and the stored MTU is capped where it is read. The form schemas carry the same limits. --- .../schemas/protocols/inbound/amneziawg.ts | 10 +-- .../schemas/protocols/outbound/amneziawg.ts | 8 +- .../src/test/amneziawg-schema-cleared.test.ts | 11 ++- internal/amneziawg/outbound.go | 3 + internal/amneziawg/params.go | 83 ++++++++++++++++--- internal/amneziawg/params_test.go | 33 ++++++++ internal/amneziawgnet/client_device.go | 4 + internal/amneziawgnet/device.go | 4 + internal/amneziawgnet/device_test.go | 13 +++ internal/web/service/inbound_amneziawg.go | 3 + 10 files changed, 146 insertions(+), 26 deletions(-) diff --git a/frontend/src/schemas/protocols/inbound/amneziawg.ts b/frontend/src/schemas/protocols/inbound/amneziawg.ts index 06576d06a..d3a1f59cb 100644 --- a/frontend/src/schemas/protocols/inbound/amneziawg.ts +++ b/frontend/src/schemas/protocols/inbound/amneziawg.ts @@ -54,7 +54,7 @@ export const AmneziawgServerSchema = z.object({ publicKey: z.string().optional(), subnetIp: z.string().default('10.8.1.0'), subnetCidr: clearedToDefault(z.number().int().min(1).max(32).default(24)), - mtu: optionalClearedInt(z.number().int().min(1)), + mtu: optionalClearedInt(z.number().int().min(1).max(65535)), primaryDns: z.string().default('8.8.8.8'), secondaryDns: z.string().default('8.8.4.4'), externalInterface: z.string().default(''), @@ -71,10 +71,10 @@ export const AmneziawgServerSchema = z.object({ // embedded userspace stack -- no Xray routing, sniffing, IPv6 or port forwarding. kernelMode: z.boolean().default(false).optional(), // Upper bounds match amneziawg-go's own UAPI parsers (device/uapi.go): - // jc/jmin/jmax are uint32, s1-s4 uint16. Wider values make IpcSet fail. - jc: clearedToDefault(z.number().int().min(0).max(4294967295).default(5)), - jmin: clearedToDefault(z.number().int().min(0).max(4294967295).default(10)), - jmax: clearedToDefault(z.number().int().min(0).max(4294967295).default(50)), + // Every handshake allocates jc packets of jmin..jmax bytes; s1-s4 are uint16. + jc: clearedToDefault(z.number().int().min(0).max(128).default(5)), + jmin: clearedToDefault(z.number().int().min(0).max(65535).default(10)), + jmax: clearedToDefault(z.number().int().min(0).max(65535).default(50)), s1: clearedToDefault(z.number().int().min(0).max(1552).default(30)), s2: clearedToDefault(z.number().int().min(0).max(1608).default(45)), s3: clearedToDefault(z.number().int().min(0).max(1636).default(10)), diff --git a/frontend/src/schemas/protocols/outbound/amneziawg.ts b/frontend/src/schemas/protocols/outbound/amneziawg.ts index 924208027..fc0fde1f0 100644 --- a/frontend/src/schemas/protocols/outbound/amneziawg.ts +++ b/frontend/src/schemas/protocols/outbound/amneziawg.ts @@ -14,14 +14,14 @@ export type AmneziaWGOutboundPeer = z.infer; export const AmneziaWGOutboundSettingsSchema = z.object({ // 0 = unset, so the backend derives MTU from S4 (amneziawg.EffectiveMTU); // a pinned 1420 fragments every full-size packet once S4 exceeds 20. - mtu: z.number().int().min(0).default(0), + mtu: z.number().int().min(0).max(65535).default(0), secretKey: z.string().default(''), address: z.array(z.string()).default([]), listenPort: z.number().int().min(0).max(65535).default(0), dns: z.string().default(''), - jc: z.number().int().min(0).default(0), - jmin: z.number().int().min(0).default(40), - jmax: z.number().int().min(0).default(100), + jc: z.number().int().min(0).max(128).default(0), + jmin: z.number().int().min(0).max(65535).default(40), + jmax: z.number().int().min(0).max(65535).default(100), // The remote server sets S1-S3; only amneziawg-go's uint16 UAPI width bounds them here. s1: z.number().int().min(0).max(65535).default(15), s2: z.number().int().min(0).max(65535).default(80), diff --git a/frontend/src/test/amneziawg-schema-cleared.test.ts b/frontend/src/test/amneziawg-schema-cleared.test.ts index 7a70d007a..65041ae6b 100644 --- a/frontend/src/test/amneziawg-schema-cleared.test.ts +++ b/frontend/src/test/amneziawg-schema-cleared.test.ts @@ -42,9 +42,10 @@ describe('AmneziawgServerSchema obfuscation bounds', () => { ['s2', 1609], ['s3', 1637], ['s4', 33], - ['jc', 4294967296], - ['jmin', 4294967296], - ['jmax', 5000000000], + ['jc', 129], + ['jmin', 65536], + ['jmax', 65536], + ['mtu', 65536], ]; it.each(overWidth)('rejects %s past what amneziawg-go can apply or receive', (field, value) => { @@ -56,7 +57,9 @@ describe('AmneziawgServerSchema obfuscation bounds', () => { ['s2', 1608], ['s3', 1636], ['s4', 32], - ['jc', 4294967295], + ['jc', 128], + ['jmax', 65535], + ['mtu', 65535], ]; it.each(atLimit)('accepts %s exactly at its limit', (field, value) => { diff --git a/internal/amneziawg/outbound.go b/internal/amneziawg/outbound.go index b5992ba60..fb780dd5b 100644 --- a/internal/amneziawg/outbound.go +++ b/internal/amneziawg/outbound.go @@ -282,6 +282,9 @@ func ValidateAmneziaWGOutbound(tag string, raw []byte) error { if err := ValidateObfuscation(parsed.Obfuscation()); err != nil { return fmt.Errorf("amneziawg outbound %q: %w", tag, err) } + if err := ValidateMTU(parsed.MTU); err != nil { + return fmt.Errorf("amneziawg outbound %q: %w", tag, err) + } for n, iv := range map[string]string{ "i1": parsed.I1, "i2": parsed.I2, "i3": parsed.I3, "i4": parsed.I4, "i5": parsed.I5, } { diff --git a/internal/amneziawg/params.go b/internal/amneziawg/params.go index f2a30ce3b..14f37652d 100644 --- a/internal/amneziawg/params.go +++ b/internal/amneziawg/params.go @@ -37,11 +37,23 @@ func randInt(min, max int) int { // link, before AmneziaWG's own S4 transport junk is prepended. const DefaultMTU = 1420 +// MaxMTU bounds the tunnel MTU: amneziawg-go keeps it as an int32, and a wider value wraps. +const MaxMTU = 65535 + +// ValidateMTU refuses an MTU past MaxMTU before it is saved; 0 means the default. +func ValidateMTU(mtu int) error { + if mtu < 0 || mtu > MaxMTU { + return fmt.Errorf("invalid mtu %d (must be 0..%d)", mtu, MaxMTU) + } + return nil +} + // EffectiveMTU is the admin's value when set, else DefaultMTU minus S4: s4 junk // is prepended to every transport packet and never clamped against the MTU. func EffectiveMTU(configuredMTU, s4 int) int { if configuredMTU > 0 { - return configuredMTU + // A row saved before ValidateMTU existed may still carry a wider value. + return min(configuredMTU, MaxMTU) } return max(DefaultMTU-max(s4, 0), 1280) } @@ -159,6 +171,35 @@ func ValidateServerObfuscation(o Obfuscation31) error { return nil } +const ( + // maxJunkPackets is the Jc range AmneziaWG documents. + maxJunkPackets = 128 + // maxPreamblePacket is amneziawg-go's MaxSegmentSize: each junk and signature packet is one datagram. + maxPreamblePacket = 65535 +) + +// ValidateHandshakePreamble bounds what every handshake initiation allocates: Jc junk +// packets of Jmin..Jmax bytes and the I1-I5 signature packets. Device setup runs it too. +func ValidateHandshakePreamble(o Obfuscation31) error { + if o.Jmin > o.Jmax { + return fmt.Errorf("invalid Jmin/Jmax: %d must not exceed %d", o.Jmin, o.Jmax) + } + for _, f := range []struct { + name string + v, max int + }{{"Jc", o.Jc, maxJunkPackets}, {"Jmin", o.Jmin, maxPreamblePacket}, {"Jmax", o.Jmax, maxPreamblePacket}} { + if f.v < 0 || f.v > f.max { + return fmt.Errorf("invalid %s value %d (must be 0..%d)", f.name, f.v, f.max) + } + } + for i, spec := range []string{o.I1, o.I2, o.I3, o.I4, o.I5} { + if err := validateObfChain(spec); err != nil { + return fmt.Errorf("invalid I%d: %w", i+1, err) + } + } + return nil +} + // ValidateObfuscation rejects malformed parameters before they are saved, so // a bad manual entry can't break the embedded amneziawg-go device's own // UAPI config apply (internal/amneziawgnet's buildUAPIConfig/IpcSet) or @@ -166,19 +207,16 @@ func ValidateServerObfuscation(o Obfuscation31) error { // are allowed (they fall back to a default); each accepts an integer or a // "100-800" range. func ValidateObfuscation(o Obfuscation31) error { - if o.Jmin > o.Jmax { - return fmt.Errorf("invalid Jmin/Jmax: %d must not exceed %d", o.Jmin, o.Jmax) + if err := ValidateHandshakePreamble(o); err != nil { + return err } - // amneziawg-go parses jc/jmin/jmax as uint32 and s1-s3 as uint16 (device/uapi.go); - // a wider value makes IpcSet reject the whole device. + // amneziawg-go parses s1-s3 as uint16 (device/uapi.go); a wider value makes IpcSet + // reject the whole device. for _, f := range []struct { name string v int max int64 }{ - {"Jc", o.Jc, math.MaxUint32}, - {"Jmin", o.Jmin, math.MaxUint32}, - {"Jmax", o.Jmax, math.MaxUint32}, {"S1", o.S1, math.MaxUint16}, {"S2", o.S2, math.MaxUint16}, {"S3", o.S3, math.MaxUint16}, @@ -187,11 +225,6 @@ func ValidateObfuscation(o Obfuscation31) error { return fmt.Errorf("invalid %s value %d (must be 0..%d)", f.name, f.v, f.max) } } - for i, spec := range []string{o.I1, o.I2, o.I3, o.I4, o.I5} { - if err := validateObfChain(spec); err != nil { - return fmt.Errorf("invalid I%d: %w", i+1, err) - } - } if o.S4 < 0 || o.S4 > 32 { return fmt.Errorf("invalid S4 value %d (must be 0..32)", o.S4) } @@ -263,6 +296,7 @@ func validateObfChain(spec string) error { return nil } remaining := spec + total := 0 for { start := strings.IndexByte(remaining, '<') if start == -1 { @@ -279,6 +313,29 @@ func validateObfChain(spec string) error { if !obfChainTags[fields[0]] { return fmt.Errorf("spec %q uses unknown tag <%s>", spec, fields[0]) } + // The packet is built once per handshake initiation, so what it allocates is bounded. + switch fields[0] { + case "r", "rc", "rd", "dz": + n := -1 + if len(fields) == 2 { + if v, err := strconv.Atoi(fields[1]); err == nil { + n = v + } + } + if n < 0 || n > maxPreamblePacket { + return fmt.Errorf("spec %q: <%s> needs a length of 0..%d", spec, fields[0], maxPreamblePacket) + } + total += n + case "b": + if len(fields) == 2 { + total += len(strings.TrimPrefix(fields[1], "0x")) / 2 + } + case "t": + total += 4 + } + if total > maxPreamblePacket { + return fmt.Errorf("spec %q builds more than %d bytes", spec, maxPreamblePacket) + } remaining = remaining[start+end+1:] } } diff --git a/internal/amneziawg/params_test.go b/internal/amneziawg/params_test.go index 1b800b001..1a3f150b6 100644 --- a/internal/amneziawg/params_test.go +++ b/internal/amneziawg/params_test.go @@ -529,3 +529,36 @@ func TestValidateObfuscationRejectsMalformedSignaturePackets(t *testing.T) { }) } } + +// Each handshake initiation sends Jc junk packets and the I1-I5 packets, allocated from +// these values; unbounded, one connection (an unauthenticated peer) killed the panel. +func TestValidateObfuscationBoundsWhatAHandshakeAllocates(t *testing.T) { + for name, mutate := range map[string]func(*Obfuscation31){ + "jc past the documented range": func(o *Obfuscation31) { o.Jc = maxJunkPackets + 1 }, + "jmax past a datagram": func(o *Obfuscation31) { o.Jmax = maxPreamblePacket + 1 }, + "random bytes past a datagram": func(o *Obfuscation31) { o.I1 = "" }, + "negative length": func(o *Obfuscation31) { o.I2 = "" }, + "spec past a datagram": func(o *Obfuscation31) { o.I3 = "" }, + } { + o := validObfuscation() + mutate(&o) + if err := ValidateObfuscation(o); err == nil { + t.Errorf("%s: accepted", name) + } + } + o := validObfuscation() + o.I1 = "" + if err := ValidateObfuscation(o); err != nil { + t.Fatalf("an ordinary I1 was refused: %v", err) + } +} + +// amneziawg-go stores the MTU as an int32; a wider value wraps and the device panics. +func TestMTUIsBoundedWhereItIsSavedAndWhereItIsRead(t *testing.T) { + if err := ValidateMTU(MaxMTU + 1); err == nil { + t.Fatal("an MTU past 65535 was accepted") + } + if got := EffectiveMTU(1<<31, 0); got != MaxMTU { + t.Fatalf("EffectiveMTU(2^31) = %d, want a stored oversized value capped at %d", got, MaxMTU) + } +} diff --git a/internal/amneziawgnet/client_device.go b/internal/amneziawgnet/client_device.go index 022f4e0fe..3f5c2bbbf 100644 --- a/internal/amneziawgnet/client_device.go +++ b/internal/amneziawgnet/client_device.go @@ -25,6 +25,10 @@ func buildClientUAPIConfig(inst amneziawg.OutboundInstance, opts DeviceOptions) } b.WriteString("replace_peers=true\n") + // Rows saved before these bounds existed would allocate without limit per handshake. + if err := amneziawg.ValidateHandshakePreamble(inst.Obfuscation); err != nil { + return "", err + } o := inst.Obfuscation fmt.Fprintf(&b, "jc=%d\njmin=%d\njmax=%d\n", o.Jc, o.Jmin, o.Jmax) fmt.Fprintf(&b, "s1=%d\ns2=%d\ns3=%d\ns4=%d\n", o.S1, o.S2, o.S3, o.S4) diff --git a/internal/amneziawgnet/device.go b/internal/amneziawgnet/device.go index 5648f4b5a..e6357f747 100644 --- a/internal/amneziawgnet/device.go +++ b/internal/amneziawgnet/device.go @@ -199,6 +199,10 @@ func buildUAPIConfig(inst amneziawg.Instance, opts DeviceOptions) (string, error // dropped instead of lingering from a previous IpcSet call. b.WriteString("replace_peers=true\n") + // Rows saved before these bounds existed would allocate without limit per handshake. + if err := amneziawg.ValidateHandshakePreamble(inst.Obfuscation); err != nil { + return "", err + } o := inst.Obfuscation fmt.Fprintf(&b, "jc=%d\njmin=%d\njmax=%d\n", o.Jc, o.Jmin, o.Jmax) fmt.Fprintf(&b, "s1=%d\ns2=%d\ns3=%d\ns4=%d\n", o.S1, o.S2, o.S3, o.S4) diff --git a/internal/amneziawgnet/device_test.go b/internal/amneziawgnet/device_test.go index 268290168..39e4cd602 100644 --- a/internal/amneziawgnet/device_test.go +++ b/internal/amneziawgnet/device_test.go @@ -679,3 +679,16 @@ func TestBuildUAPIConfigClearedHeaderProtectionKeyIsSentAsZero(t *testing.T) { t.Fatalf("a configured key must not be emitted as zero, got:\n%s", withKey) } } + +// A row stored before the handshake bounds existed must fail its own device setup instead +// of allocating without limit at the next handshake, which ended the whole panel. +func TestBuildUAPIConfigRefusesAStoredUnboundedPreamble(t *testing.T) { + priv, _, err := wireguard.GenerateWireguardKeypair() + if err != nil { + t.Fatalf("generate keypair: %v", err) + } + inst := amneziawg.Instance{PrivateKey: priv, Obfuscation: amneziawg.Obfuscation31{Jc: 1 << 31, Jmin: 10, Jmax: 20}} + if _, err := buildUAPIConfig(inst, DeviceOptions{}); err == nil { + t.Fatal("a stored Jc of 2^31 reached the device") + } +} diff --git a/internal/web/service/inbound_amneziawg.go b/internal/web/service/inbound_amneziawg.go index 578f93426..e4e099ea2 100644 --- a/internal/web/service/inbound_amneziawg.go +++ b/internal/web/service/inbound_amneziawg.go @@ -237,6 +237,9 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, old if err := amneziawg.ValidateServerObfuscation(parsed.Server.Obfuscation()); err != nil { return fmt.Errorf("amneziawg: %w", err) } + if err := amneziawg.ValidateMTU(parsed.Server.MTU); err != nil { + return fmt.Errorf("amneziawg: %w", err) + } if err := amneziawg.ValidateIPv6Subnet(parsed.Server.IPv6Enabled, parsed.Server.IPv6Subnet); err != nil { return fmt.Errorf("amneziawg: %w", err) }