From 30f17ef5a334fb501c81953710654b93982a3e05 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sun, 1 Nov 2026 01:22:51 +0100 Subject: [PATCH] fix(amneziawg): keep port forwards per inbound and off future relay ports Attach and BulkAttach copied a client record's forwardedPorts onto the target inbound. A client a node reports keeps the forwards the node chose, so attaching it to one of the master's AmneziaWG inbounds opened listeners of the node's choosing on the master, among them the loopback relay port a later AmneziaWG or TUIC inbound derives from its id; that inbound's traffic then went to the forward and Xray failed to bind. A forward is set per inbound and is no longer carried by an attach, and creating an AmneziaWG or TUIC inbound is refused while a peer forward already holds its relay port. --- internal/web/service/client_attach_test.go | 60 +++++++++++++++++++ internal/web/service/client_bulk.go | 2 + internal/web/service/client_crud.go | 3 + internal/web/service/port_conflict.go | 16 ++++- .../service/port_conflict_forwarded_test.go | 55 +++++++++++++++++ 5 files changed, 134 insertions(+), 2 deletions(-) diff --git a/internal/web/service/client_attach_test.go b/internal/web/service/client_attach_test.go index d1ef9b12e..65000a403 100644 --- a/internal/web/service/client_attach_test.go +++ b/internal/web/service/client_attach_test.go @@ -157,3 +157,63 @@ func TestAttachRefusesDefaultRouteNextToHeldAddress(t *testing.T) { } } } + +// A forward is a listener on the inbound's host; carried over from a node-reported +// client, it let the node pick ports on the master, a future relay port among them. +func TestAttachDoesNotCarryForwardedPorts(t *testing.T) { + attaches := map[string]func(*ClientService, *InboundService, string, int) error{ + "Attach": func(s *ClientService, inboundSvc *InboundService, email string, ibId int) error { + _, err := s.AttachByEmail(inboundSvc, email, []int{ibId}) + return err + }, + "BulkAttach": func(s *ClientService, inboundSvc *InboundService, email string, ibId int) error { + result, _, err := s.BulkAttach(inboundSvc, []string{email}, []int{ibId}) + if err == nil && len(result.Errors) > 0 { + err = errors.New(strings.Join(result.Errors, "; ")) + } + return err + }, + } + for name, attach := range attaches { + t.Run(name, func(t *testing.T) { + setupConflictDB(t) + node := 1 + seedInboundConflict(t, "own", "0.0.0.0", 51830, model.AmneziaWG, ``, awgRelayWindowSettings(t, "own")) + seedInboundConflictNode(t, "node", "0.0.0.0", 44301, model.AmneziaWG, ``, `{"clients":[`+ + `{"email":"forwarder","subId":"s-fwd","enable":true,"publicKey":"QkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkI=","allowedIPs":["10.9.1.2/32"],"forwardedPorts":"8022"}]}`, &node) + + s := &ClientService{} + inboundSvc := &InboundService{} + var own model.Inbound + for _, tag := range []string{"node", "own"} { + own = model.Inbound{} + if err := database.GetDB().Where("tag = ?", tag).First(&own).Error; err != nil { + t.Fatalf("read seeded %s row: %v", tag, err) + } + clients, err := inboundSvc.GetClients(&own) + if err != nil { + t.Fatalf("GetClients(%s): %v", tag, err) + } + if err := s.SyncInbound(nil, own.Id, clients); err != nil { + t.Fatalf("SyncInbound(%s): %v", tag, err) + } + } + + if err := attach(s, inboundSvc, "forwarder", own.Id); err != nil { + t.Fatalf("attach: %v", err) + } + if err := database.GetDB().First(&own, own.Id).Error; err != nil { + t.Fatalf("reread own inbound: %v", err) + } + clients, err := inboundSvc.GetClients(&own) + if err != nil { + t.Fatalf("GetClients(own): %v", err) + } + for _, c := range clients { + if c.Email == "forwarder" && c.ForwardedPorts != "" { + t.Fatalf("attach carried forwardedPorts %q onto the master's inbound", c.ForwardedPorts) + } + } + }) + } +} diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index 1f500c8ac..89b241dc8 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -111,6 +111,8 @@ func (s *ClientService) BulkAttach(inboundSvc *InboundService, emails []string, continue } client := *rec.ToClient() + // Forwards are set per inbound; see Attach. + client.ForwardedPorts = "" if flow, ok := flowsByEmail[rec.Email]; ok && flow != "" { client.Flow = flow } diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index 5429167cd..2b336d9b9 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -1172,6 +1172,9 @@ func (s *ClientService) Attach(inboundSvc *InboundService, id int, inboundIds [] return false, fmt.Errorf("inbound %d: %w", ibId, getErr) } copyClient := *clientWire + // A forward is a listener on the target inbound's host, set per inbound: carried over, it + // would let a node-reported client choose ports on this host. + copyClient.ForwardedPorts = "" if !addressesFitAmneziaWGInbound(copyClient.AllowedIPs, inbound) { copyClient.AllowedIPs = nil } diff --git a/internal/web/service/port_conflict.go b/internal/web/service/port_conflict.go index 07282f1d1..59a1eb635 100644 --- a/internal/web/service/port_conflict.go +++ b/internal/web/service/port_conflict.go @@ -553,7 +553,19 @@ func checkAmneziawgnetSocksReverseConflict(db *gorm.DB, id int) (*portConflictDe Transports: transportTCP, }, nil } - return nil, nil + return relayPortForwardOwner(db, relayPort, id) +} + +// relayPortForwardOwner asks the same of the forward listeners, which no port column +// shows: bound on every interface, one would be dialled in Xray's place by id's relay. +func relayPortForwardOwner(db *gorm.DB, relayPort, id int) (*portConflictDetail, error) { + owner, err := amneziawgForwardedPortOwner(db, &model.Inbound{Port: relayPort}, id) + if err != nil || owner == nil { + return nil, err + } + owner.Relay = true + owner.Transports = transportTCP + return owner, nil } func checkTuicSocksConflict(db *gorm.DB, inbound *model.Inbound, ignoreId int, newBits transportBits) (*portConflictDetail, error) { @@ -643,7 +655,7 @@ func checkTuicSocksReverseConflict(db *gorm.DB, id int) (*portConflictDetail, er Transports: transportTCP, }, nil } - return nil, nil + return relayPortForwardOwner(db, relayPort, id) } func sameNode(a, b *int) bool { diff --git a/internal/web/service/port_conflict_forwarded_test.go b/internal/web/service/port_conflict_forwarded_test.go index e8e0b676e..c1b2a76ce 100644 --- a/internal/web/service/port_conflict_forwarded_test.go +++ b/internal/web/service/port_conflict_forwarded_test.go @@ -1,11 +1,14 @@ package service import ( + "fmt" "strings" "testing" + "github.com/mhsanaei/3x-ui/v3/internal/amneziawgnet" "github.com/mhsanaei/3x-ui/v3/internal/database" "github.com/mhsanaei/3x-ui/v3/internal/database/model" + "github.com/mhsanaei/3x-ui/v3/internal/tuic" ) // checkForwardedPortsConflict only ran from the AmneziaWG save path, so an @@ -133,3 +136,55 @@ func TestAddInboundAllowsANodeRowOnALocallyForwardedPort(t *testing.T) { t.Fatalf("a node row does not bind here; the create must be allowed: %v", err) } } + +// A forward bound on every interface holds a relay port no port column shows, so the +// inbound whose id derives that relay port must not be created onto it. +func TestAddInboundRefusesARelayPortAnAmneziaWGPeerForwards(t *testing.T) { + cases := []struct { + name string + relayPort func(id int) int + inbound func(t *testing.T) *model.Inbound + }{ + {"amneziawg", amneziawgnet.SOCKSPortForInbound, func(t *testing.T) *model.Inbound { + return &model.Inbound{ + Tag: "awg-next", Enable: true, Listen: "0.0.0.0", Port: 51821, + Protocol: model.AmneziaWG, Settings: awgRelayWindowSettings(t, "awg-next"), + } + }}, + {"tuic", tuic.SOCKSPortForInbound, func(t *testing.T) *model.Inbound { + return &model.Inbound{ + Tag: "tuic-next", Enable: true, Listen: "0.0.0.0", Port: 8443, + Protocol: model.TUIC, Settings: `{"clients":[]}`, + } + }}, + } + + for _, tc := range cases { + for _, held := range []bool{true, false} { + t.Run(fmt.Sprintf("%s held=%v", tc.name, held), func(t *testing.T) { + setupConflictDB(t) + // The holder takes id 1, so the create below derives its port from id 2. + forwarded := tc.relayPort(2) + if !held { + forwarded = tc.relayPort(3) + } + seedInboundConflict(t, "awg-forward", "0.0.0.0", 51820, model.AmneziaWG, ``, + awgRelayWindowSettingsWithForward(t, "awg-forward", fmt.Sprint(forwarded))) + + _, _, err := (&InboundService{}).AddInbound(tc.inbound(t)) + if !held { + if err != nil { + t.Fatalf("relay port %d is free; the create must be allowed: %v", tc.relayPort(2), err) + } + return + } + if err == nil { + t.Fatalf("relay port %d is forwarded by a peer of another inbound; the create must be refused", forwarded) + } + if !strings.Contains(err.Error(), "relay port") || !strings.Contains(err.Error(), "awg-forward@relay-window") { + t.Fatalf("the refusal must name the relay port and the peer holding it, got %v", err) + } + }) + } + } +}