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