mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-05 09:57:14 +00:00
fix(amneziawg): reject obfuscation values amneziawg-go's own UAPI rejects
ValidateObfuscation exists, by its own doc comment, so that a bad manual entry cannot break the embedded device's IpcSet. It was not covering enough to do that. Auditing the panel against amneziawg-go v3.1.20260828's full UAPI surface turned up two holes, both confirmed by driving the values through a real IpcSet: S1 = 70000 upstream parses s1-s4 as uint16 S2 = 70000 (device/uapi.go) Jc = -1 jc/jmin/jmax are uint32, so no negatives Jmin/Jmax = -5/-1 Jc = 5000000000 and nothing wider than uint32 I1 = <rand 100> newObfChain hard-fails on an unknown tag I1 = <r 100 ... and on a missing '>' I1 = <> ... and on an empty one All eight passed validation and were then rejected by the device. Only S3 and S4 were bounded, which is why the asymmetry went unnoticed. The inbound saves, the reconcile fails on every tick, and the interface never comes up with a single log line to say so. Bound the five numeric fields to the widths upstream actually parses, and check the I1-I5 chain's <tag value> structure against a tag set mirroring upstream's own obfBuilders map. Each tag's value grammar stays amneziawg-go's to enforce -- that is eight builders across several files, and duplicating them here would drift. So <r abc> still reaches IpcSet, now as the only remaining class rather than one of four. Mirror the same bounds in the Zod schema, next to the max() that s3 and s4 already carried, so the form rejects the value instead of the save doing it. TestValidatedObfuscationAlwaysApplies pins the contract itself: whatever ValidateObfuscation accepts, a real amneziawg-go device must accept too. It covers the specs the new grammar check deliberately allows, not just the ones it rejects, so the allowlist cannot quietly become stricter than upstream. The rest of the audit found no gaps: all 17 settable device keys reach buildUAPIConfig, ServerSettings, the Zod schema and all three .conf emitters. fwmark and persistent_keepalive_interval remain unemitted, both deliberately -- the panel models no fwmark anywhere, and keepAlive is carried client-side where WireGuard puts it.
This commit is contained in:
Sanaei
@@ -66,11 +66,13 @@ export const AmneziawgServerSchema = z.object({
|
||||
// z.object's default unknown-key stripping doesn't silently drop it from
|
||||
// an existing stored settings blob on the next save.
|
||||
routeThroughXray: z.boolean().default(false).optional(),
|
||||
jc: clearedToDefault(z.number().int().min(0).default(5)),
|
||||
jmin: clearedToDefault(z.number().int().min(0).default(10)),
|
||||
jmax: clearedToDefault(z.number().int().min(0).default(50)),
|
||||
s1: clearedToDefault(z.number().int().min(0).default(30)),
|
||||
s2: clearedToDefault(z.number().int().min(0).default(45)),
|
||||
// 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)),
|
||||
s1: clearedToDefault(z.number().int().min(0).max(65535).default(30)),
|
||||
s2: clearedToDefault(z.number().int().min(0).max(65535).default(45)),
|
||||
s3: clearedToDefault(z.number().int().min(0).max(64).default(10)),
|
||||
s4: clearedToDefault(z.number().int().min(0).max(32).default(5)),
|
||||
h1: z.string().default(''),
|
||||
|
||||
@@ -32,3 +32,40 @@ describe('AmneziawgServerSchema cleared numeric fields', () => {
|
||||
expect(parsed.jc).toBe(5);
|
||||
});
|
||||
});
|
||||
|
||||
// The form must reject what amneziawg-go's UAPI parsers reject (device/uapi.go:
|
||||
// jc/jmin/jmax uint32, s1-s4 uint16), or the save silently outlives the apply.
|
||||
describe('AmneziawgServerSchema obfuscation bounds', () => {
|
||||
const overWidth: Array<[string, number]> = [
|
||||
['s1', 65536],
|
||||
['s2', 70000],
|
||||
['s3', 65],
|
||||
['s4', 33],
|
||||
['jc', 4294967296],
|
||||
['jmin', 4294967296],
|
||||
['jmax', 5000000000],
|
||||
];
|
||||
|
||||
it.each(overWidth)('rejects %s above the width amneziawg-go parses', (field, value) => {
|
||||
expect(AmneziawgServerSchema.safeParse({ [field]: value }).success).toBe(false);
|
||||
});
|
||||
|
||||
const atLimit: Array<[string, number]> = [
|
||||
['s1', 65535],
|
||||
['s2', 65535],
|
||||
['s3', 64],
|
||||
['s4', 32],
|
||||
['jc', 4294967295],
|
||||
];
|
||||
|
||||
it.each(atLimit)('accepts %s exactly at its limit', (field, value) => {
|
||||
const parsed = AmneziawgServerSchema.safeParse({ [field]: value });
|
||||
expect(parsed.success).toBe(true);
|
||||
});
|
||||
|
||||
it('still rejects negatives on every junk and padding field', () => {
|
||||
for (const field of ['jc', 'jmin', 'jmax', 's1', 's2', 's3', 's4']) {
|
||||
expect(AmneziawgServerSchema.safeParse({ [field]: -1 }).success).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"crypto/rand"
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"math"
|
||||
"math/big"
|
||||
"net/netip"
|
||||
"regexp"
|
||||
@@ -130,6 +131,28 @@ 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)
|
||||
}
|
||||
// amneziawg-go parses jc/jmin/jmax as uint32 and s1-s4 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},
|
||||
} {
|
||||
if int64(f.v) < 0 || int64(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)
|
||||
}
|
||||
}
|
||||
if o.S3 < 0 || o.S3 > 64 {
|
||||
return fmt.Errorf("invalid S3 value %d (must be 0..64)", o.S3)
|
||||
}
|
||||
@@ -187,6 +210,40 @@ func ValidateObfuscation(o Obfuscation31) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// obfChainTags mirrors amneziawg-go's own obfBuilders map (device/obf.go): an
|
||||
// unknown tag makes newObfChain fail, and IpcSet then rejects the whole device.
|
||||
var obfChainTags = map[string]bool{
|
||||
"b": true, "t": true, "r": true, "rc": true,
|
||||
"rd": true, "d": true, "ds": true, "dz": true,
|
||||
}
|
||||
|
||||
// validateObfChain checks an I1-I5 signature-packet spec's "<tag value>"
|
||||
// structure. Each tag's own value grammar stays amneziawg-go's to enforce.
|
||||
func validateObfChain(spec string) error {
|
||||
if strings.TrimSpace(spec) == "" {
|
||||
return nil
|
||||
}
|
||||
remaining := spec
|
||||
for {
|
||||
start := strings.IndexByte(remaining, '<')
|
||||
if start == -1 {
|
||||
return nil
|
||||
}
|
||||
end := strings.IndexByte(remaining[start:], '>')
|
||||
if end == -1 {
|
||||
return fmt.Errorf("spec %q is missing an enclosing '>'", spec)
|
||||
}
|
||||
fields := strings.Fields(remaining[start+1 : start+end])
|
||||
if len(fields) == 0 {
|
||||
return fmt.Errorf("spec %q has an empty <> tag", spec)
|
||||
}
|
||||
if !obfChainTags[fields[0]] {
|
||||
return fmt.Errorf("spec %q uses unknown tag <%s>", spec, fields[0])
|
||||
}
|
||||
remaining = remaining[start+end+1:]
|
||||
}
|
||||
}
|
||||
|
||||
// CanonicalizeUintRange stores a pasted "110 - 140" as "110-140", and
|
||||
// collapses a whitespace-only value back to "feature off".
|
||||
func CanonicalizeUintRange(v string) string {
|
||||
|
||||
@@ -377,3 +377,60 @@ func TestValidateConfigValueRejectsControlCharacters(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateObfuscationRejectsOutOfRangeJunkAndPadding pins the widths
|
||||
// amneziawg-go's UAPI actually parses: uint32 for jc/jmin/jmax, uint16 for s1-s4.
|
||||
func TestValidateObfuscationRejectsOutOfRangeJunkAndPadding(t *testing.T) {
|
||||
base := Obfuscation31{Jc: 4, Jmin: 40, Jmax: 70, S1: 20, S2: 30, S3: 20, S4: 20}
|
||||
tests := []struct {
|
||||
name string
|
||||
mut func(*Obfuscation31)
|
||||
}{
|
||||
{"S1 over uint16", func(o *Obfuscation31) { o.S1 = 65536 }},
|
||||
{"S2 over uint16", func(o *Obfuscation31) { o.S2 = 70000 }},
|
||||
{"negative Jc", func(o *Obfuscation31) { o.Jc = -1 }},
|
||||
{"negative Jmin and Jmax", func(o *Obfuscation31) { o.Jmin, o.Jmax = -5, -1 }},
|
||||
{"Jc over uint32", func(o *Obfuscation31) { o.Jc = 5000000000 }},
|
||||
{"negative S1", func(o *Obfuscation31) { o.S1 = -1 }},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
o := base
|
||||
tt.mut(&o)
|
||||
if err := ValidateObfuscation(o); err == nil {
|
||||
t.Fatal("ValidateObfuscation accepted a value amneziawg-go's UAPI parser rejects, so the inbound would save and then fail to apply")
|
||||
}
|
||||
})
|
||||
}
|
||||
if err := ValidateObfuscation(Obfuscation31{Jc: 4, Jmin: 40, Jmax: 70, S1: 65535, S2: 30, S3: 20, S4: 20}); err != nil {
|
||||
t.Fatalf("S1 at the uint16 maximum must stay valid: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateObfuscationRejectsMalformedSignaturePackets covers I1-I5, whose
|
||||
// "<tag value>" chain amneziawg-go parses with newObfChain (device/obf.go).
|
||||
func TestValidateObfuscationRejectsMalformedSignaturePackets(t *testing.T) {
|
||||
base := Obfuscation31{Jc: 4, Jmin: 40, Jmax: 70, S1: 20, S2: 30, S3: 20, S4: 20}
|
||||
|
||||
bad := []string{"<rand 100>", "<r 100", "<>", "< >", "<r 10><nope 2>"}
|
||||
for _, spec := range bad {
|
||||
t.Run("reject "+spec, func(t *testing.T) {
|
||||
o := base
|
||||
o.I1 = spec
|
||||
if err := ValidateObfuscation(o); err == nil {
|
||||
t.Fatalf("ValidateObfuscation accepted I1=%q, which newObfChain rejects", spec)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
good := []string{"", "<r 100>", "<b ff00><r 10>", "<t><rc 5>", "no tags at all"}
|
||||
for _, spec := range good {
|
||||
t.Run("accept "+spec, func(t *testing.T) {
|
||||
o := base
|
||||
o.I5 = spec
|
||||
if err := ValidateObfuscation(o); err != nil {
|
||||
t.Fatalf("ValidateObfuscation rejected valid I5=%q: %v", spec, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -548,3 +548,79 @@ func TestNewDeviceRandomTrailersAndDisableCookiesRoundTrip(t *testing.T) {
|
||||
t.Fatal("timed out waiting for the server side to finish")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidatedObfuscationAlwaysApplies pins the contract ValidateObfuscation
|
||||
// exists for: whatever it accepts, amneziawg-go's own IpcSet must accept too.
|
||||
func TestValidatedObfuscationAlwaysApplies(t *testing.T) {
|
||||
priv, pub, err := wireguard.GenerateWireguardKeypair()
|
||||
if err != nil {
|
||||
t.Fatalf("server keypair: %v", err)
|
||||
}
|
||||
_, peerPub, err := wireguard.GenerateWireguardKeypair()
|
||||
if err != nil {
|
||||
t.Fatalf("peer keypair: %v", err)
|
||||
}
|
||||
base := amneziawg.Obfuscation31{Jc: 4, Jmin: 40, Jmax: 70, S1: 20, S2: 30, S3: 20, S4: 20}
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
mut func(*amneziawg.Obfuscation31)
|
||||
}{
|
||||
{"generated defaults", func(o *amneziawg.Obfuscation31) { *o = amneziawg.GenerateObfuscation31() }},
|
||||
{"S1 over uint16", func(o *amneziawg.Obfuscation31) { o.S1 = 70000 }},
|
||||
{"S2 over uint16", func(o *amneziawg.Obfuscation31) { o.S2 = 70000 }},
|
||||
{"negative Jc", func(o *amneziawg.Obfuscation31) { o.Jc = -1 }},
|
||||
{"negative Jmin and Jmax", func(o *amneziawg.Obfuscation31) { o.Jmin, o.Jmax = -5, -1 }},
|
||||
{"Jc over uint32", func(o *amneziawg.Obfuscation31) { o.Jc = 5000000000 }},
|
||||
{"I1 unknown tag", func(o *amneziawg.Obfuscation31) { o.I1 = "<rand 100>" }},
|
||||
{"I1 missing close", func(o *amneziawg.Obfuscation31) { o.I1 = "<r 100" }},
|
||||
{"I1 empty tag", func(o *amneziawg.Obfuscation31) { o.I1 = "<>" }},
|
||||
// The specs validateObfChain deliberately accepts must really apply.
|
||||
{"I1 chained tags", func(o *amneziawg.Obfuscation31) { o.I1 = "<b ff00><r 10>" }},
|
||||
{"I1 valueless tag", func(o *amneziawg.Obfuscation31) { o.I1 = "<t><rc 5>" }},
|
||||
{"I1 no tags at all", func(o *amneziawg.Obfuscation31) { o.I1 = "plain text" }},
|
||||
}
|
||||
|
||||
for i, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
o := base
|
||||
tc.mut(&o)
|
||||
if err := amneziawg.ValidateObfuscation(o); err != nil {
|
||||
return // rejected before saving, which is the whole point
|
||||
}
|
||||
inst := amneziawg.Instance{
|
||||
Id: 88, InterfaceName: "awgcontract", ListenPort: 58900 + i,
|
||||
PrivateKey: priv, PublicKey: pub,
|
||||
Address: []string{"10.198.0.1/24"}, MTU: 1420,
|
||||
Obfuscation: o,
|
||||
Peers: []amneziawg.Peer{{
|
||||
Email: "contract@example.com", PublicKey: peerPub,
|
||||
AllowedIPs: []string{"10.198.0.2/32"},
|
||||
}},
|
||||
}
|
||||
opts := DeviceOptions{
|
||||
HeaderProtectionKey: o.HeaderProtectionKey,
|
||||
ContentPaddingAddition: o.ContentPaddingAddition,
|
||||
RekeyAfterTime: o.RekeyAfterTime,
|
||||
RekeyTimeout: o.RekeyTimeout,
|
||||
RejectAfterTime: o.RejectAfterTime,
|
||||
KeepaliveTimeout: o.KeepaliveTimeout,
|
||||
MaxHandshakeAttempts: o.MaxHandshakeAttempts,
|
||||
RandomTrailers: o.RandomTrailers,
|
||||
DisableCookies: o.DisableCookies,
|
||||
}
|
||||
dev, err := newUnconfiguredDevice(inst, opts)
|
||||
if err != nil {
|
||||
t.Fatalf("newUnconfiguredDevice: %v", err)
|
||||
}
|
||||
defer dev.Close()
|
||||
conf, err := buildUAPIConfig(inst, opts)
|
||||
if err != nil {
|
||||
t.Fatalf("buildUAPIConfig: %v", err)
|
||||
}
|
||||
if err := dev.IpcSet(conf); err != nil {
|
||||
t.Fatalf("ValidateObfuscation accepted this config but amneziawg-go rejected it: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user