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.
This commit is contained in:
MHSanaei
2026-11-01 01:22:51 +01:00
parent 6dbd219a14
commit 30f17ef5a3
5 changed files with 134 additions and 2 deletions
@@ -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)
}
}
})
}
}
+2
View File
@@ -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
}
+3
View File
@@ -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
}
+14 -2
View File
@@ -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 {
@@ -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)
}
})
}
}
}