fix(amneziawg): keep a disabled row's relay port reserved for port forwards

loadPortConflictContext filtered its query with enable = true, so a client's
ForwardedPorts spec could claim the relay port a disabled AmneziaWG row's id
derives. That row's relay appears with its first client -- a path that runs no
port check -- and when the relay then loses the loopback bind race to the
forward listener, Xray refuses the whole config instead of losing one forward
(#6542 review, arrived with #6540).

The context now loads every local row and gates only the ordinary-port compare on
enable, which is what a disabled row's own port is worth: free. Its relay slot is
not free, which is the rule #6540 already states for the other two guards.

TestCheckForwardedPortsConflict_DisabledAmneziawgRelayPortIsReserved fails
without this -- watched red first -- and passes with it, while
TestCheckForwardedPortsConflict_IgnoresDisabledInboundPort keeps proving that a
disabled inbound's own port stays available.
This commit is contained in:
BlindMaster24
2026-09-15 12:32:32 +03:00
parent bf08e1a2fe
commit 35ffba1eea
2 changed files with 38 additions and 21 deletions
+10 -21
View File
@@ -313,41 +313,28 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, oldS
return nil
}
// portConflictContext caches the state checkForwardedPortsConflict needs —
// the panel's own port and this host's enabled inbound ports — so validating
// N clients in one save (normalizeAmneziaWGSettings, or a bulk client add)
// costs one query total instead of N. Load it once with
// loadPortConflictContext and pass it to every checkForwardedPortsConflict
// call in that batch.
// portConflictContext caches what checkForwardedPortsConflict needs — the panel's
// own port and this host's inbound rows — so one save costs one query, not N.
type portConflictContext struct {
webPort int
inbounds []*model.Inbound
}
// loadPortConflictContext loads the panel's own port and every enabled
// inbound hosted on THIS panel (node_id IS NULL) — an inbound hosted on a
// different node listens on that node's own host, never this one, so it can
// never collide with a DNAT rule this process installs.
// loadPortConflictContext loads the panel's own port and every inbound hosted on
// THIS panel: a node-hosted one listens on that node's host, never on this one.
func (s *InboundService) loadPortConflictContext(db *gorm.DB) (portConflictContext, error) {
var ctx portConflictContext
if webPort, err := (&SettingService{}).GetPort(); err == nil {
ctx.webPort = webPort
}
err := db.Model(model.Inbound{}).
Where("enable = ? AND node_id IS NULL", true).
Where("node_id IS NULL").
Find(&ctx.inbounds).Error
return ctx, err
}
// checkForwardedPortsConflict reports whether a client's ForwardedPorts spec
// exceeds the cap, covers the panel's own web port, one of this host's own
// enabled inbound listen ports, or an AmneziaWG inbound's own phantom SOCKS5
// relay port (SOCKSPortForInbound -- never a real inbounds row, so the loop
// below can't see it any other way). A collision on the SOCKS5 port would
// let a port-forward listener race Xray's own relay for the bind and, if it
// wins, take down that inbound's entire relay rather than just one forward.
// Returns a human-readable description of the first collision found, or ""
// when there is none.
// checkForwardedPortsConflict names the panel, inbound or AmneziaWG relay port a
// client's ForwardedPorts spec would collide with: a lost bind race kills the relay.
func (s *InboundService) checkForwardedPortsConflict(ctx portConflictContext, forwardedPorts string) string {
if forwardedPorts == "" {
return ""
@@ -359,7 +346,9 @@ func (s *InboundService) checkForwardedPortsConflict(ctx portConflictContext, fo
return fmt.Sprintf("the panel's own port (%d)", ctx.webPort)
}
for _, ib := range ctx.inbounds {
if amneziawg.ForwardedPortsInclude(forwardedPorts, ib.Port) {
// A disabled row's own port is free, but its relay slot is not: the relay
// appears with the first client, and no client path re-checks ports.
if ib.Enable && amneziawg.ForwardedPortsInclude(forwardedPorts, ib.Port) {
name := ib.Remark
if name == "" {
name = ib.Tag
@@ -370,6 +370,34 @@ func TestCheckForwardedPortsConflict_CollidesWithAmneziawgnetSocksPort(t *testin
}
}
// A disabled AmneziaWG row still owns the relay port its id derives (#6540), so
// a client's port-forward spec must not be able to claim that same bind.
func TestCheckForwardedPortsConflict_DisabledAmneziawgRelayPortIsReserved(t *testing.T) {
setupConflictDB(t)
seedInboundConflict(t, "awg-off", "0.0.0.0", 51820, model.AmneziaWG, ``, `{}`)
var off model.Inbound
if err := database.GetDB().Where("tag = ?", "awg-off").First(&off).Error; err != nil {
t.Fatalf("read seeded row: %v", err)
}
if err := database.GetDB().Model(model.Inbound{}).Where("id = ?", off.Id).
Update("enable", false).Error; err != nil {
t.Fatalf("disable the seeded row: %v", err)
}
relayPort := amneziawgnet.SOCKSPortForInbound(off.Id)
svc := &InboundService{}
ctx, err := svc.loadPortConflictContext(database.GetDB())
if err != nil {
t.Fatalf("loadPortConflictContext: %v", err)
}
hit := svc.checkForwardedPortsConflict(ctx, fmt.Sprintf("%d", relayPort))
if !strings.Contains(hit, "SOCKS5") {
t.Fatalf("inbound #%d is disabled but still owns relay port %d; the spec must be refused, got %q",
off.Id, relayPort, hit)
}
}
// A cleared DNS field is meaningful (no DNS line in client configs) and must
// survive the save round-trip instead of resurrecting the frontend defaults.
func TestNormalizeAmneziaWGSettingsKeepsClearedDNS(t *testing.T) {