diff --git a/internal/amneziawgnet/portfwd.go b/internal/amneziawgnet/portfwd.go index 25019345d..92e16dc9a 100644 --- a/internal/amneziawgnet/portfwd.go +++ b/internal/amneziawgnet/portfwd.go @@ -134,14 +134,11 @@ func desiredPeerTargets(inst amneziawg.Instance) map[string]netip.Addr { return out } -// desiredPortForwardKeys returns the full set of listener keys inst wants -// right now: one tcpForward and one udpForward key per port in every peer's -// ForwardedPorts spec, for every peer that also has a resolvable target -// (see desiredPeerTargets) -- a key never exists without a target, so -// Reconcile can always resolve one for any key it opens. -func desiredPortForwardKeys(inst amneziawg.Instance) map[portForwardKey]struct{} { - out := map[portForwardKey]struct{}{} +// forwardingPeers is the one gate a host listener comes from: no email, port +// spec and resolvable target (see desiredPeerTargets), no socket. +func forwardingPeers(inst amneziawg.Instance) []amneziawg.Peer { targets := desiredPeerTargets(inst) + out := make([]amneziawg.Peer, 0, len(inst.Peers)) for _, p := range inst.Peers { if p.Email == "" || p.ForwardedPorts == "" { continue @@ -149,6 +146,16 @@ func desiredPortForwardKeys(inst amneziawg.Instance) map[portForwardKey]struct{} if _, ok := targets[p.Email]; !ok { continue } + out = append(out, p) + } + return out +} + +// desiredPortForwardKeys returns every listener key inst wants right now: one +// tcpForward and one udpForward per forwarded port of the forwarding peers. +func desiredPortForwardKeys(inst amneziawg.Instance) map[portForwardKey]struct{} { + out := map[portForwardKey]struct{}{} + for _, p := range forwardingPeers(inst) { for _, port := range amneziawg.ExpandForwardedPorts(p.ForwardedPorts) { out[portForwardKey{email: p.Email, port: port, proto: tcpForward}] = struct{}{} out[portForwardKey{email: p.Email, port: port, proto: udpForward}] = struct{}{} @@ -157,6 +164,19 @@ func desiredPortForwardKeys(inst amneziawg.Instance) map[portForwardKey]struct{} return out } +// ForwardedPortOwner names the peer Reconcile opens a listener on port for -- +// the same peers and expansion as desiredPortForwardKeys, never a silent one. +func ForwardedPortOwner(inst amneziawg.Instance, port int) (string, bool) { + for _, p := range forwardingPeers(inst) { + for _, candidate := range amneziawg.ExpandForwardedPorts(p.ForwardedPorts) { + if candidate == port { + return p.Email, true + } + } + } + return "", false +} + // Reconcile brings the supervisor's open listeners in line with what inst // currently wants: closes anything no longer desired, opens anything newly // desired, leaves everything else untouched. Never returns an error -- diff --git a/internal/web/service/port_conflict.go b/internal/web/service/port_conflict.go index c1377e1f0..21b18b807 100644 --- a/internal/web/service/port_conflict.go +++ b/internal/web/service/port_conflict.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/mhsanaei/3x-ui/v3/internal/amneziawg" "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" @@ -108,8 +109,11 @@ type portConflictDetail struct { Listen string Port int // Relay marks Port as an automatic loopback relay port, not a configured one. - Relay bool - Transports transportBits + Relay bool + // ForwardedBy is the peer whose port forward holds Port, when that is the + // reason for the conflict. + ForwardedBy string + Transports transportBits } // String renders the detail as a single-line, user-facing summary. @@ -134,6 +138,10 @@ func (d *portConflictDetail) String() string { if d.Relay { port = fmt.Sprintf("relay port %d", d.Port) } + if d.ForwardedBy != "" { + return fmt.Sprintf("%s (%s) already forwarded on inbound %s on %s by its client %s", + port, transportTagSuffix(d.Transports), name, listen, d.ForwardedBy) + } return fmt.Sprintf("%s (%s) already used by inbound %s on %s", port, transportTagSuffix(d.Transports), name, listen) } @@ -242,6 +250,17 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po } } + // A forwarded port is not a column and not a relay slot, so the query below + // cannot see it either: the port is bound by the peer's forward listener. + forwardedBy, err := amneziawgForwardedPortOwner(db, inbound, ignoreId) + if err != nil { + return nil, err + } + if forwardedBy != nil { + forwardedBy.Transports = newBits + return forwardedBy, nil + } + var candidates []*model.Inbound q := db.Model(model.Inbound{}).Where("port = ?", inbound.Port) if ignoreId > 0 { @@ -275,6 +294,43 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po return nil, nil } +// amneziawgForwardedPortOwner names the AmneziaWG row on the same host whose peer +// forwards inbound's port -- a bind on every interface that only the AWG side checked. +func amneziawgForwardedPortOwner(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*portConflictDetail, error) { + var rows []*model.Inbound + q := db.Model(model.Inbound{}).Where("protocol = ?", model.AmneziaWG) + if ignoreId > 0 { + q = q.Where("id != ?", ignoreId) + } + if err := q.Find(&rows).Error; err != nil { + return nil, err + } + for _, row := range rows { + if !sameNode(row.NodeID, inbound.NodeID) { + continue + } + instance, ok := amneziawg.InstanceFromInbound(row) + if !ok { + continue + } + email, forwards := amneziawgnet.ForwardedPortOwner(instance, inbound.Port) + if !forwards { + continue + } + return &portConflictDetail{ + InboundID: row.Id, + Remark: row.Remark, + Tag: row.Tag, + // the forward binds :port on every interface, wherever the + // candidate asked to listen. + Listen: "", + Port: inbound.Port, + ForwardedBy: email, + }, nil + } + return nil, nil +} + // checkAmneziawgnetSocksConflict: inbound's port vs the relay port every matching // local row reserves, emitted or not; db keeps it in the caller's transaction (#6225). func checkAmneziawgnetSocksConflict(db *gorm.DB, inbound *model.Inbound, ignoreId int, newBits transportBits) (*portConflictDetail, error) { diff --git a/internal/web/service/port_conflict_forwarded_test.go b/internal/web/service/port_conflict_forwarded_test.go new file mode 100644 index 000000000..e8e0b676e --- /dev/null +++ b/internal/web/service/port_conflict_forwarded_test.go @@ -0,0 +1,135 @@ +package service + +import ( + "strings" + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// checkForwardedPortsConflict only ran from the AmneziaWG save path, so an +// ordinary inbound could take a port a peer forwards on every interface. +func TestAddInboundRefusesAPortAnAmneziaWGPeerForwards(t *testing.T) { + const forwarded = 8443 + cases := []struct { + name string + port int + wantErr bool + }{ + {"the forwarded port", forwarded, true}, + {"a free port", forwarded + 1, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + setupConflictDB(t) + seedInboundConflict(t, "awg-forward", "0.0.0.0", 51820, model.AmneziaWG, ``, + awgRelayWindowSettingsWithForward(t, "awg-forward", "8443")) + + _, _, err := (&InboundService{}).AddInbound(&model.Inbound{ + Tag: "user-inbound", Enable: true, Listen: "0.0.0.0", Port: tc.port, + Protocol: model.VLESS, StreamSettings: `{"network":"tcp"}`, Settings: `{"clients":[]}`, + }) + if !tc.wantErr { + if err != nil { + t.Fatalf("port %d is free; the create must be allowed: %v", tc.port, err) + } + return + } + if err == nil { + t.Fatalf("port %d is forwarded by a peer of another inbound; the create must be refused", tc.port) + } + if !strings.Contains(err.Error(), "awg-forward@relay-window") { + t.Fatalf("the refusal must name the peer holding the port, got %v", err) + } + }) + } +} + +// A peer the forward supervisor opens no listener for holds no port: it has no +// email, or no address the tunnel can route to, and Reconcile skips it either way. +func TestAddInboundAllowsAPortNoPeerCanActuallyForward(t *testing.T) { + cases := []struct { + name string + settings func(t *testing.T) string + }{ + { + name: "a peer with no email", + settings: func(t *testing.T) string { + t.Helper() + return replaceFirst(t, awgRelayWindowSettingsWithForward(t, "awg-forward", "8443"), + `"email":"awg-forward@relay-window"`, `"email":""`) + }, + }, + { + name: "an IPv6-only peer on a row without IPv6", + settings: func(t *testing.T) string { + t.Helper() + return replaceFirst(t, awgRelayWindowSettingsWithForward(t, "awg-forward", "8443"), + `"allowedIPs":["10.8.1.2/32"]`, `"allowedIPs":["fd00::2/128"]`) + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + setupConflictDB(t) + seedInboundConflict(t, "awg-forward", "0.0.0.0", 51820, model.AmneziaWG, ``, tc.settings(t)) + + if _, _, err := (&InboundService{}).AddInbound(&model.Inbound{ + Tag: "user-inbound", Enable: true, Listen: "0.0.0.0", Port: 8443, + Protocol: model.VLESS, StreamSettings: `{"network":"tcp"}`, Settings: `{"clients":[]}`, + }); err != nil { + t.Fatalf("nothing binds 8443 for this peer; the create must be allowed: %v", err) + } + }) + } +} + +// The refusal has to point at where the socket really is: the forward listens on +// every interface, so repeating the candidate's requested address asserts a lie. +func TestForwardedPortRefusalNamesTheWildcardBind(t *testing.T) { + setupConflictDB(t) + seedInboundConflict(t, "awg-forward", "0.0.0.0", 51820, model.AmneziaWG, ``, + awgRelayWindowSettingsWithForward(t, "awg-forward", "8443")) + + _, _, err := (&InboundService{}).AddInbound(&model.Inbound{ + Tag: "user-inbound", Enable: true, Listen: "10.0.0.5", Port: 8443, + Protocol: model.VLESS, StreamSettings: `{"network":"tcp"}`, Settings: `{"clients":[]}`, + }) + if err == nil { + t.Fatal("the port is forwarded on every interface, including 10.0.0.5; the create must be refused") + } + if !strings.Contains(err.Error(), " on * by its client ") { + t.Fatalf("the refusal must place the forward on every interface, got %v", err) + } +} + +func replaceFirst(t *testing.T, s, old, new string) string { + t.Helper() + if !strings.Contains(s, old) { + t.Fatalf("fixture no longer contains %s", old) + } + return strings.Replace(s, old, new, 1) +} + +// The forward listener runs where the AmneziaWG row runs, so a node row sharing +// a local peer's port stays legal -- the scoping every other guard here uses. +func TestAddInboundAllowsANodeRowOnALocallyForwardedPort(t *testing.T) { + setupConflictDB(t) + seedInboundConflict(t, "awg-forward", "0.0.0.0", 51820, model.AmneziaWG, ``, + awgRelayWindowSettingsWithForward(t, "awg-forward", "8443")) + + node := &model.Node{Name: "n1", Address: "127.0.0.1", Port: 2096, Scheme: "https", Enable: true, Status: "online"} + if err := database.GetDB().Create(node).Error; err != nil { + t.Fatalf("seed node: %v", err) + } + if _, _, err := (&InboundService{}).AddInbound(&model.Inbound{ + Tag: "node-inbound", Enable: true, Listen: "0.0.0.0", Port: 8443, + Protocol: model.VLESS, StreamSettings: `{"network":"tcp"}`, Settings: `{"clients":[]}`, + NodeID: &node.Id, + }); err != nil { + t.Fatalf("a node row does not bind here; the create must be allowed: %v", err) + } +}