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) }