From bf08e1a2fe9e66fa128914463b61bfd3df8564b9 Mon Sep 17 00:00:00 2001 From: BlindMaster24 <375291171150z@gmail.com> Date: Tue, 15 Sep 2026 12:31:44 +0300 Subject: [PATCH] fix(amneziawg): refuse a WireGuard port that is the row's own relay port All three relay checks filter themselves out of the candidates with id != ignoreId, so nothing ever compared an AmneziaWG row's own WireGuard listen port with the relay port its own id derives. Saving a row on that exact port left the embedded device (UDP on the inbound's listen address, amneziawgnet/device.go:137) and its injected relay (TCP and UDP on 127.0.0.1, amneziawgnet/relay.go:47-61) bound to the same UDP port, so whichever loses the race dies -- and when the relay loses it, Xray refuses the whole config and takes every other protocol on the host with it. The first AmneziaWG inbound on port 65101 was enough to reach it: id 1 derives exactly that port. The row now states the rule its three siblings do: it owns the slot its id derives. A node-hosted row still keeps its own port, since it binds no relay on this host. TestAddInbound_AmneziawgRefusesItsOwnRelayPort and TestUpdateInbound_AmneziawgRefusesItsOwnRelayPort fail without this -- both were watched red first -- and pin the two separate call sites, AddInbound's post-Save block and checkPortConflictTx's ignoreId > 0 block. --- internal/web/service/inbound.go | 5 +- .../inbound_amneziawg_relay_window_test.go | 61 +++++++++++++++++++ internal/web/service/port_conflict.go | 17 ++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 1950f1008..31fc22e7c 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -1238,8 +1238,11 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo return err } // The relay port is derived from the id, only known after Save, and only a - // local row owns one: checkPortConflictTx ran neither check with ignoreId==0. + // local row owns one: checkPortConflictTx ran no relay check with ignoreId==0. if inbound.NodeID == nil && inbound.Protocol == model.AmneziaWG { + if self := amneziawgnetSocksSelfConflict(inbound, inbound.Id); self != "" { + return common.NewError(self) + } conflict, cErr := checkAmneziawgnetSocksRelayCollision(tx, inbound.Id) if cErr != nil { return cErr diff --git a/internal/web/service/inbound_amneziawg_relay_window_test.go b/internal/web/service/inbound_amneziawg_relay_window_test.go index 08f4de9c6..41ec5a85d 100644 --- a/internal/web/service/inbound_amneziawg_relay_window_test.go +++ b/internal/web/service/inbound_amneziawg_relay_window_test.go @@ -168,6 +168,55 @@ func TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot(t *testing.T) } } +// The row's own WireGuard port can be the relay port its own id derives, and +// every relay check excludes that id, so nothing else compares the two. +func TestAddInbound_AmneziawgRefusesItsOwnRelayPort(t *testing.T) { + setupConflictDB(t) + + // Read the sequence instead of assuming id 1: the victim's own derived port + // has to be known before it is created. + placeholder := addAmneziaWGInbound(t, "awg-placeholder", 51820, true) + selfPort := amneziawgnet.SOCKSPortForInbound(placeholder.Id + 1) + + _, _, err := (&InboundService{}).AddInbound(&model.Inbound{ + Tag: "awg-self", + Enable: true, + Listen: "0.0.0.0", + Port: selfPort, + Protocol: model.AmneziaWG, + Settings: awgRelayWindowSettings(t, "awg-self"), + }) + if err == nil { + t.Fatalf("WireGuard port %d is inbound #%d's own relay port; the create must be refused", + selfPort, placeholder.Id+1) + } + if !strings.Contains(err.Error(), "relay port") { + t.Fatalf("the refusal must say the port is an automatic relay one, got %v", err) + } +} + +// The edit path knows the id the relay port comes from, so it has to refuse the +// same self-collision -- the reverse check skips the row it computes for. +func TestUpdateInbound_AmneziawgRefusesItsOwnRelayPort(t *testing.T) { + setupConflictDB(t) + created := addAmneziaWGInbound(t, "awg-self-edit", 51820, true) + + edit := *created + edit.Port = amneziawgnet.SOCKSPortForInbound(created.Id) + if edit.Port == created.Port { + t.Fatalf("fixture: inbound #%d already listens on its derived relay port", created.Id) + } + + _, _, err := (&InboundService{}).UpdateInbound(&edit) + if err == nil { + t.Fatalf("WireGuard port %d is inbound #%d's own relay port; the save must be refused", + edit.Port, created.Id) + } + if !strings.Contains(err.Error(), "relay port") { + t.Fatalf("the refusal must say the port is an automatic relay one, got %v", err) + } +} + // A row adopted from a node keeps the protocol it arrived with and its central // id (inbound_node.go:737), but gets no relay -- so its slot can never be taken. func TestCheckPortConflict_NodeAssignedAmneziawgOwnsNoRelaySlot(t *testing.T) { @@ -197,4 +246,16 @@ func TestCheckPortConflict_NodeAssignedAmneziawgOwnsNoRelaySlot(t *testing.T) { t.Fatalf("id %d is node-assigned and binds no relay, so it cannot collide; got %q", collidingID, got.String()) } + + // The same rule covers the row's own port: with no relay on this host, its + // WireGuard port may legitimately BE the port its id would derive. + adopted.Port = amneziawgnet.SOCKSPortForInbound(collidingID) + got, err = (&InboundService{}).checkPortConflict(adopted, collidingID) + if err != nil { + t.Fatalf("checkPortConflict: %v", err) + } + if got != nil { + t.Fatalf("id %d is node-assigned and binds no relay, so its own port is not a conflict; got %q", + collidingID, got.String()) + } } diff --git a/internal/web/service/port_conflict.go b/internal/web/service/port_conflict.go index 8e70af55c..c1377e1f0 100644 --- a/internal/web/service/port_conflict.go +++ b/internal/web/service/port_conflict.go @@ -223,6 +223,9 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po // The reverse direction, only meaningful once the id is known -- AddInbound // runs it after Save. Only a local row owns a relay slot (#6537 review). if inbound.NodeID == nil && inbound.Protocol == model.AmneziaWG && ignoreId > 0 { + if self := amneziawgnetSocksSelfConflict(inbound, ignoreId); self != "" { + return nil, common.NewError(self) + } conflict, err := checkAmneziawgnetSocksRelayCollision(db, ignoreId) if err != nil { return nil, err @@ -330,6 +333,20 @@ func checkAmneziawgnetSocksRelayCollision(db *gorm.DB, id int) (*portConflictDet return nil, nil } +// amneziawgnetSocksSelfConflict: a row's own WireGuard port vs the relay port its +// own id derives -- all three checks below exclude that id, so nothing else does. +func amneziawgnetSocksSelfConflict(inbound *model.Inbound, id int) string { + if id <= 0 || inbound.NodeID != nil || !listenOverlaps("127.0.0.1", inbound.Listen) { + return "" + } + relayPort := amneziawgnet.SOCKSPortForInbound(id) + if inbound.Port != relayPort { + return "" + } + return fmt.Sprintf("WireGuard port %d is inbound #%d's own SOCKS5 relay port on 127.0.0.1; choose a different WireGuard port", + relayPort, id) +} + // checkAmneziawgnetSocksReverseConflict mirrors checkAmneziawgnetSocksConflict: // does id's own derived relay port collide with some other inbound's port. func checkAmneziawgnetSocksReverseConflict(db *gorm.DB, id int) (*portConflictDetail, error) {