From fee7b1621d652b98cf1f13342ab5635419c5245d Mon Sep 17 00:00:00 2001 From: Aleksey Date: Tue, 3 Nov 2026 17:52:26 +0300 Subject: [PATCH] fix(amneziawg): validate peer subnets before enabling kernel mode (#6965) A kernel-mode AmneziaWG inbound routes and masquerades only its interface subnet, so peers whose AllowedIPs lay outside it (a restored backup, a changed subnet) handshook but got no internet. Enabling kernel mode, saving an active kernel inbound, and adding, enabling or re-addressing a client now refuse out-of-subnet peers with the offending client named. Legacy stray peers still start and can still be edited or disabled. --- internal/amneziawgkern/config.go | 43 ++++++++++ internal/amneziawgkern/config_test.go | 28 +++++++ internal/amneziawgkern/manager_test.go | 33 ++++++++ internal/amneziawgkern/preflight.go | 2 +- internal/web/service/client_crud.go | 4 +- internal/web/service/client_inbound_apply.go | 43 ++++++++-- internal/web/service/inbound_amneziawg.go | 18 +++- .../web/service/inbound_amneziawg_test.go | 40 +++++++++ .../web/service/kernel_client_guard_test.go | 84 +++++++++++++++++++ 9 files changed, 286 insertions(+), 9 deletions(-) diff --git a/internal/amneziawgkern/config.go b/internal/amneziawgkern/config.go index c53107d99..605bc1787 100644 --- a/internal/amneziawgkern/config.go +++ b/internal/amneziawgkern/config.go @@ -257,6 +257,49 @@ func interfaceAddresses(inst amneziawg.Instance) ([]string, error) { return out, nil } +// ValidatePeerSubnets rejects peers the kernel interface cannot route or NAT. +func ValidatePeerSubnets(inst amneziawg.Instance) error { + if len(inst.Peers) == 0 { + return nil + } + addresses, err := interfaceAddresses(inst) + if err != nil { + return err + } + subnets := make([]netip.Prefix, 0, len(addresses)) + for i, address := range addresses { + subnet := netip.MustParsePrefix(address).Masked() + subnets = append(subnets, subnet) + addresses[i] = subnet.String() + } + for _, peer := range inst.Peers { + for _, raw := range peer.AllowedIPs { + prefix, err := netip.ParsePrefix(strings.TrimSpace(raw)) + if err != nil { + address, addressErr := netip.ParseAddr(strings.TrimSpace(raw)) + if addressErr != nil { + return fmt.Errorf("amneziawgkern: peer %q has invalid allowedIPs %q", peer.Email, raw) + } + prefix = netip.PrefixFrom(address, address.BitLen()) + } + if !prefix.Addr().Is4() { + continue + } + covered := false + for _, subnet := range subnets { + if prefix.Bits() >= subnet.Bits() && subnet.Contains(prefix.Masked().Addr()) { + covered = true + break + } + } + if !covered { + return fmt.Errorf("amneziawgkern: peer %q allowedIPs %q is outside kernel interface subnets %v; align the server subnet and client addresses before enabling kernel mode", peer.Email, raw, addresses) + } + } + } + return nil +} + // natChain masquerades each tunnel subnet (not the host address) out of egress. func natChain(addresses []string, egress string) string { var b strings.Builder diff --git a/internal/amneziawgkern/config_test.go b/internal/amneziawgkern/config_test.go index 3b26fc5f4..dd7dc2473 100644 --- a/internal/amneziawgkern/config_test.go +++ b/internal/amneziawgkern/config_test.go @@ -1,6 +1,7 @@ package amneziawgkern import ( + "fmt" "strings" "testing" @@ -118,6 +119,33 @@ func TestBuildSpecMasqueradesSubnetNotHostAddress(t *testing.T) { } } +func TestValidatePeerSubnetsRejectsPeersOutsideNATSubnet(t *testing.T) { + for _, allowedIP := range []string{"10.9.1.2/32", "10.8.0.0/16"} { + t.Run(allowedIP, func(t *testing.T) { + inst := testInstance() + inst.Peers[0].AllowedIPs = []string{allowedIP} + err := ValidatePeerSubnets(inst) + want := fmt.Sprintf("amneziawgkern: peer %q allowedIPs %q is outside kernel interface subnets [10.8.1.0/24]; align the server subnet and client addresses before enabling kernel mode", "alice", allowedIP) + if err == nil || err.Error() != want { + t.Fatalf("ValidatePeerSubnets error = %v, want %q", err, want) + } + }) + } +} + +func TestValidatePeerSubnetsAcceptsPeersCoveredByInterfaceSubnets(t *testing.T) { + for _, allowedIP := range []string{"10.8.1.2/32", "10.8.1.2", "10.8.1.2/31"} { + t.Run(allowedIP, func(t *testing.T) { + inst := testInstance() + inst.Address = []string{"10.8.1.5/24"} + inst.Peers[0].AllowedIPs = []string{allowedIP} + if err := ValidatePeerSubnets(inst); err != nil { + t.Fatalf("covered peer was rejected: %v", err) + } + }) + } +} + // TestBuildSpecRouteThroughXrayOnlyDeletesNAT: a legacy routeThroughXray row // must not leave a masquerade rule behind from an earlier save. func TestBuildSpecRouteThroughXrayOnlyDeletesNAT(t *testing.T) { diff --git a/internal/amneziawgkern/manager_test.go b/internal/amneziawgkern/manager_test.go index 3f5eb560a..2884e6d87 100644 --- a/internal/amneziawgkern/manager_test.go +++ b/internal/amneziawgkern/manager_test.go @@ -250,6 +250,39 @@ func forwardRulesFor(h *fakeHost, name string) []string { return out } +func TestEnsureLegacySubnetMismatchPreservesAvailabilityAndPeerRevocation(t *testing.T) { + for _, coldStart := range []bool{true, false} { + t.Run(fmt.Sprintf("coldStart=%t", coldStart), func(t *testing.T) { + m, h, _ := newTestManager(t) + d := managerDesired(1) + aliceKey := d.Instance.Peers[0].PublicKey + bobKey := d.Instance.Peers[1].PublicKey + if !coldStart { + if err := m.Ensure(d); err != nil { + t.Fatal(err) + } + } + d.Instance.Peers[1].AllowedIPs = []string{"10.9.1.2/32"} + if !coldStart { + d.Instance.Peers = d.Instance.Peers[1:] + } + if err := m.Ensure(d); err != nil { + t.Fatalf("a legacy stray peer must not block startup or peer revocation: %v", err) + } + link := h.links["xui-awg1"] + if link == nil || !link.up || strings.Join(link.addresses, ",") != "10.8.1.1/24" { + t.Fatalf("legacy configuration lost the interface: %+v", link) + } + if strings.Contains(link.conf, "PublicKey = "+aliceKey+"\n") != coldStart { + t.Fatalf("valid peer was lost at startup or remained after revocation: %s", link.conf) + } + if !strings.Contains(link.conf, "PublicKey = "+bobKey+"\n") || !strings.Contains(h.nat["xui_awg1"], "ip saddr 10.8.1.0/24") { + t.Fatalf("legacy runtime configuration was unexpectedly changed: interface=%+v NAT=%s", link, h.nat["xui_awg1"]) + } + }) + } +} + // TestEnsureBuildsInterfaceWithoutShell: every step is a direct exec, so no // client-controlled value ever reaches a shell running as root. func TestEnsureBuildsInterfaceWithoutShell(t *testing.T) { diff --git a/internal/amneziawgkern/preflight.go b/internal/amneziawgkern/preflight.go index d545b00b2..600cde83f 100644 --- a/internal/amneziawgkern/preflight.go +++ b/internal/amneziawgkern/preflight.go @@ -109,7 +109,7 @@ func Preflight(inst amneziawg.Instance, routeThroughXray bool) error { blockers := append([]Blocker(nil), HostReadiness().Blockers...) blockers = append(blockers, instanceFeatureBlockers(inst, routeThroughXray)...) if len(blockers) == 0 { - return nil + return ValidatePeerSubnets(inst) } return fmt.Errorf("kernel mode unavailable: %s", strings.Join(describe(blockers), "; ")) } diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index e2878159e..392fa563a 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -293,7 +293,7 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate // comment on the Attach path. clientForInbound.AllowedIPs = nil } - if err := validateAmneziaWGClientWrite(inbound, []model.Client{clientForInbound}); err != nil { + if err := validateAmneziaWGClientWrite(inbound, []model.Client{clientForInbound}, ""); err != nil { return false, fmt.Errorf("inbound %d: %w", ibId, err) } settingsPayload, mErr := json.Marshal(map[string][]model.Client{"clients": {clientWithInboundFlow(clientForInbound, inbound)}}) @@ -846,7 +846,7 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model // instead. clientForInbound.AllowedIPs = nil } - if err := validateAmneziaWGClientWrite(inbound, []model.Client{clientForInbound}); err != nil { + if err := validateAmneziaWGClientWrite(inbound, []model.Client{clientForInbound}, existing.Email); err != nil { return false, fmt.Errorf("inbound %d: %w", ibId, err) } settingsPayload, mErr := json.Marshal(map[string][]model.Client{"clients": {clientWithInboundFlow(clientForInbound, inbound)}}) diff --git a/internal/web/service/client_inbound_apply.go b/internal/web/service/client_inbound_apply.go index 67b6faebe..c9df9de98 100644 --- a/internal/web/service/client_inbound_apply.go +++ b/internal/web/service/client_inbound_apply.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "maps" + "slices" "strings" "time" @@ -31,7 +32,7 @@ func droppedClientNeedsRestart() bool { return on } -func validateAmneziaWGClientWrite(inbound *model.Inbound, clients []model.Client) error { +func validateAmneziaWGClientWrite(inbound *model.Inbound, clients []model.Client, previousEmail string) error { if inbound.Protocol != model.AmneziaWG { return nil } @@ -59,15 +60,47 @@ func validateAmneziaWGClientWrite(inbound *model.Inbound, clients []model.Client return nil } inst := amneziawg.Instance{Listen: inbound.Listen, IPv6Enabled: settings.Server.IPv6Enabled} + v4Base, _, err := defaultAmneziaWGSubnetBases(inbound.Settings) + if err != nil { + return err + } + inst.Address = []string{v4Base} for _, client := range clients { inst.Peers = append(inst.Peers, amneziawg.Peer{ - AllowedIPs: client.AllowedIPs, ForwardedPorts: client.ForwardedPortsSpec(), + Email: client.Email, AllowedIPs: client.AllowedIPs, ForwardedPorts: client.ForwardedPortsSpec(), }) } if err := amneziawgkern.ValidateInstanceFeatures(inst, settings.Server.RouteThroughXray); err != nil { return err } - return nil + if !inbound.Enable { + return nil + } + inst.Peers = nil + for _, client := range clients { + if !client.Enable { + continue + } + allowedIPs := client.AllowedIPs + for _, old := range settings.Clients { + if previousEmail == "" || !strings.EqualFold(old.Email, previousEmail) { + continue + } + if len(allowedIPs) == 0 { + allowedIPs = old.AllowedIPs + } + // Preserve legacy addresses on active peers; validate newly added + // addresses, or every address when enabling a disabled peer. + if old.Enable { + allowedIPs = slices.DeleteFunc(slices.Clone(allowedIPs), func(ip string) bool { + return slices.Contains(old.AllowedIPs, ip) + }) + } + break + } + inst.Peers = append(inst.Peers, amneziawg.Peer{Email: client.Email, AllowedIPs: allowedIPs}) + } + return amneziawgkern.ValidatePeerSubnets(inst) } func sameClientConfigExceptUpdatedAt(a, b map[string]any) bool { @@ -482,7 +515,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model } } } - if err := validateAmneziaWGClientWrite(oldInbound, clients); err != nil { + if err := validateAmneziaWGClientWrite(oldInbound, clients, ""); err != nil { return false, err } @@ -850,7 +883,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo clients[0].ForwardedPorts = old.ForwardedPorts } } - if err := validateAmneziaWGClientWrite(oldInbound, clients); err != nil { + if err := validateAmneziaWGClientWrite(oldInbound, clients, oldEmail); err != nil { return false, err } if oldInbound.Protocol == model.AmneziaWG { diff --git a/internal/web/service/inbound_amneziawg.go b/internal/web/service/inbound_amneziawg.go index c32064198..1fceae9a5 100644 --- a/internal/web/service/inbound_amneziawg.go +++ b/internal/web/service/inbound_amneziawg.go @@ -312,7 +312,7 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, old // Host readiness gates only a save that starts the kernel datapath: a host that // later loses its module must still let the admin edit, disable or switch it. - if parsed.Server.KernelMode && inbound.NodeID == nil { + if parsed.Server.KernelMode { inst, ok := amneziawg.InstanceFromInbound(inbound) if !ok { inst = amneziawg.Instance{ @@ -323,12 +323,28 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, old for _, client := range parsed.Clients { if client.Enable { inst.Peers = append(inst.Peers, amneziawg.Peer{ + Email: client.Email, AllowedIPs: client.AllowedIPs, ForwardedPorts: client.ForwardedPortsSpec(), }) } } } + if len(inst.Address) == 0 { + v4Base, _, err := defaultAmneziaWGSubnetBases(inbound.Settings) + if err != nil { + return err + } + inst.Address = []string{v4Base} + } + if inbound.Enable { + if err := amneziawgkern.ValidatePeerSubnets(inst); err != nil { + return err + } + } + if inbound.NodeID != nil { + return nil + } check := amneziawgkern.ValidateInstanceFeatures wasRunningKernel := old != nil && old.Enable && amneziawgKernelModeSettings(old.Settings) if inbound.Enable && !wasRunningKernel { diff --git a/internal/web/service/inbound_amneziawg_test.go b/internal/web/service/inbound_amneziawg_test.go index 2a2d48df3..80e9bec5e 100644 --- a/internal/web/service/inbound_amneziawg_test.go +++ b/internal/web/service/inbound_amneziawg_test.go @@ -27,6 +27,46 @@ var awgTestPrivateKey, awgTestPublicKey = func() (string, string) { return priv, pub }() +func TestNormalizeAmneziaWGSettingsRejectsKernelSubnetMismatch(t *testing.T) { + setupConflictDB(t) + for _, remote := range []bool{false, true} { + t.Run(fmt.Sprintf("remote=%t", remote), func(t *testing.T) { + server, err := defaultAmneziaWGServer() + if err != nil { + t.Fatal(err) + } + server.SubnetIP = "10.9.1.0" + client := model.Client{Email: "restored@awg", Enable: true, PublicKey: awgTestPublicKey, AllowedIPs: []string{"10.8.1.2/32"}} + settings := amneziawg.InboundSettings{Server: server, Clients: []model.Client{client}} + marshal := func() string { + b, err := json.Marshal(settings) + if err != nil { + t.Fatal(err) + } + return string(b) + } + old := &model.Inbound{Protocol: model.AmneziaWG, Enable: true, Settings: marshal()} + if remote { + old.NodeID = new(7) + } + if err := (&InboundService{}).normalizeAmneziaWGSettings(old, nil); err != nil { + t.Fatalf("embedded mode must remain editable: %v", err) + } + server.KernelMode = true + inbound := *old + inbound.Settings = marshal() + err = (&InboundService{}).normalizeAmneziaWGSettings(&inbound, old) + if err == nil || !strings.Contains(err.Error(), `peer "restored@awg" allowedIPs "10.8.1.2/32" is outside kernel interface subnets [10.9.1.0/24]`) { + t.Fatalf("kernel-mode save error = %v, want the mismatched peer and server subnet", err) + } + inbound.Enable = false + if err := (&InboundService{}).normalizeAmneziaWGSettings(&inbound, old); err != nil { + t.Fatalf("disabling a broken inbound must remain possible: %v", err) + } + }) + } +} + func TestCheckForwardedPortsConflict_EmptySpecNoConflict(t *testing.T) { setupConflictDB(t) svc := &InboundService{} diff --git a/internal/web/service/kernel_client_guard_test.go b/internal/web/service/kernel_client_guard_test.go index f6c2a5e2b..96229c8bb 100644 --- a/internal/web/service/kernel_client_guard_test.go +++ b/internal/web/service/kernel_client_guard_test.go @@ -25,6 +25,7 @@ func TestKernelAWGClientWritesRejectUnsafeValuesAndUnsupportedFeatures(t *testin {"public key hook injection", "control characters", model.Client{Email: "bad-key@awg", Enable: true, PublicKey: "peer-key\n[Interface]\nPostDown = touch /tmp/pwned", AllowedIPs: []string{"10.8.1.2/32"}}}, {"forwarded ports", "port forwarding is not supported", model.Client{Email: "bad-forward@awg", Enable: true, PublicKey: "peer-key", AllowedIPs: []string{"10.8.1.2/32"}, ForwardedPorts: model.ForwardedPortsPtr("54321")}}, {"IPv6 allowed IP", "per-peer IPv6 egress is not supported", model.Client{Email: "bad-v6@awg", Enable: true, PublicKey: "peer-key", AllowedIPs: []string{"2001:db8::2/128"}}}, + {"outside server subnet", "outside kernel interface subnets", model.Client{Email: "bad-subnet@awg", Enable: true, PublicKey: "peer-key", AllowedIPs: []string{"10.9.1.2/32"}}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -72,6 +73,7 @@ func TestKernelAWGClientUpdateRejectsUnsafeValuesAndUnsupportedFeatures(t *testi {"preshared key hook injection", "control characters", model.Client{Email: "kept@awg", PreSharedKey: "psk\n[Interface]\nPreDown = touch /tmp/pwned"}}, {"forwarded ports", "port forwarding is not supported", model.Client{Email: "kept@awg", ForwardedPorts: model.ForwardedPortsPtr("54321")}}, {"IPv6 allowed IP", "per-peer IPv6 egress is not supported", model.Client{Email: "kept@awg", AllowedIPs: []string{"2001:db8::2/128"}}}, + {"outside server subnet", "outside kernel interface subnets", model.Client{Email: "kept@awg", AllowedIPs: []string{"10.9.1.2/32"}}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -89,6 +91,7 @@ func TestKernelAWGClientUpdateRejectsUnsafeValuesAndUnsupportedFeatures(t *testi t.Fatal(getErr) } update := tc.update + update.Enable = true if len(update.AllowedIPs) > 0 { update.AllowedIPsByInbound = map[int][]string{ib.Id: update.AllowedIPs} } @@ -128,6 +131,87 @@ func TestKernelAWGClientFeatureChecksPreserveSupportedPaths(t *testing.T) { } } +func TestKernelAWGLegacyClientRecovery(t *testing.T) { + cases := []struct { + name, action string + clientEnabled, inboundEnabled, wantReject bool + }{ + {"disable legacy peer", "disable", true, true, false}, + {"renew legacy peer", "renew", true, true, false}, + {"edit legacy peer", "edit", true, true, false}, + {"rename legacy peer", "rename", true, true, false}, + {"edit disabled peer", "edit", false, true, false}, + {"edit disabled inbound", "change", true, false, false}, + {"enable legacy peer", "enable", false, true, true}, + {"change to another invalid address", "change", true, true, true}, + {"repair legacy address", "repair", true, true, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + setupBulkDB(t) + inboundSvc, svc := &InboundService{}, &ClientService{} + ib := mkInbound(t, 52009, model.AmneziaWG, kernelAWGSettings(nil)) + seed := model.Client{Email: "legacy@awg", Enable: tc.clientEnabled, PublicKey: "valid-key", PrivateKey: "valid-private", AllowedIPs: []string{"10.8.1.2/32"}} + if _, err := svc.CreateOne(inboundSvc, ib.Id, seed); err != nil { + t.Fatal(err) + } + rec := lookupClientRecord(t, seed.Email) + ib, _ = inboundSvc.GetInbound(ib.Id) + // Model a restored legacy row without going through save validation. + legacy := strings.Replace(ib.Settings, "10.8.1.0", "10.9.1.0", 1) + if legacy == ib.Settings { + t.Fatal("failed to change legacy subnet") + } + if err := database.GetDB().Model(ib).Updates(map[string]any{"settings": legacy, "enable": tc.inboundEnabled}).Error; err != nil { + t.Fatal(err) + } + update := seed + update.ExpiryTime = 4102444800000 + update.AllowedIPsByInbound = map[int][]string{ib.Id: seed.AllowedIPs} + var err error + switch tc.action { + case "disable": + _, _, err = svc.SetClientEnableByEmail(inboundSvc, seed.Email, false) + case "enable": + _, _, err = svc.SetClientEnableByEmail(inboundSvc, seed.Email, true) + case "renew": + _, err = svc.ResetClientExpiryTimeByEmail(inboundSvc, seed.Email, update.ExpiryTime) + default: + if tc.action == "rename" { + update.Email = "renamed@awg" + } + if tc.action == "change" { + update.AllowedIPsByInbound[ib.Id] = []string{"10.10.1.2/32"} + } + if tc.action == "repair" { + update.AllowedIPsByInbound[ib.Id] = []string{"10.9.1.2/32"} + } + _, err = svc.Update(inboundSvc, rec.Id, update, 0) + } + if tc.wantReject { + if err == nil || !strings.Contains(err.Error(), "outside kernel interface subnets") { + t.Fatalf("error = %v, want subnet rejection", err) + } + } else if err != nil { + t.Fatalf("legacy recovery rejected: %v", err) + } + after, getErr := inboundSvc.GetInbound(ib.Id) + if getErr != nil { + t.Fatal(getErr) + } + if tc.wantReject && after.Settings != legacy { + t.Fatal("rejected update changed inbound settings") + } + if tc.action == "disable" && mustAWGClients(t, after)[0].Enable { + t.Fatal("client remained enabled") + } + if tc.action == "renew" && mustAWGClients(t, after)[0].ExpiryTime != update.ExpiryTime { + t.Fatal("client expiry was not renewed") + } + }) + } +} + func TestKernelAWGSharedCreateAndUpdateValidateAllTargetsBeforeWrites(t *testing.T) { t.Run("create", func(t *testing.T) { setupBulkDB(t)