mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-17 15:47:14 +00:00
fix(amneziawg): reserve the relay port before an AmneziaWG inbound has a peer (#6542)
* test(amneziawg): pin that a peerless inbound still owns its relay port checkAmneziawgnetSocksConflict skips a candidate whose settings yield no qualifying peer, and normalizeAmneziaWGSettings writes Clients: [] for a fresh AmneziaWG inbound -- so a newly created row reserves nothing, an ordinary inbound can take its derived port, and adding that row's first client then puts two inbounds on 127.0.0.1:65101. The client paths run no port check. Expected red on this head; the fix follows. * fix(amneziawg): reserve the relay port before the first peer is added checkAmneziawgnetSocksConflict skipped a candidate whose settings yield no qualifying peer (amneziawg.InstanceFromInbound), and normalizeAmneziaWGSettings writes Clients: [] for a fresh AmneziaWG inbound. A newly created row therefore reserved nothing, an ordinary inbound could be saved onto the port that row derives, and adding its first client generated the relay next to it: two inbounds on 127.0.0.1:65101, which makes Xray refuse the whole config and take every other protocol on the host down with it. Nothing re-checked it later either -- only AddInbound and UpdateInbound run checkPortConflictTx, and the client paths that create the first peer run no port check at all. Ownership now follows the row, so the check states the same rule as its two siblings, which key on protocol and node_id IS NULL alone. The amneziawg import goes with the guard. TestCheckPortConflict_AmneziawgnetSocksRelayReservedBeforeTheFirstPeer fails without this, on a test-only head whose go-test run failed on exactly that test, and passes with it. * docs(amneziawg): stop the forward check's doc block claiming every row gets a relay Round-1 LOW: the block's justification clause read "every one of them gets a relay inbound", which is false for exactly the rows this change newly reserves for -- injectAmneziawgnetSocks skips a row with no peer email, and that is the row whose port must stay reserved. A reader following the cross-reference landed on the guard this branch removes and read it as the rule. Replaced by the two facts that are true, which also brings the block under CLAUDE.md's two-line cap instead of twelve lines over it. The peerless reason stays where it is load-bearing, in the two-line comment above the candidate loop.
This commit is contained in:
@@ -5,7 +5,6 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/mhsanaei/3x-ui/v3/internal/amneziawg"
|
|
||||||
"github.com/mhsanaei/3x-ui/v3/internal/amneziawgnet"
|
"github.com/mhsanaei/3x-ui/v3/internal/amneziawgnet"
|
||||||
"github.com/mhsanaei/3x-ui/v3/internal/database"
|
"github.com/mhsanaei/3x-ui/v3/internal/database"
|
||||||
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
||||||
@@ -273,18 +272,8 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po
|
|||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// checkAmneziawgnetSocksConflict reports whether inbound's own port
|
// checkAmneziawgnetSocksConflict: inbound's port vs the relay port every matching
|
||||||
// collides with an existing local AmneziaWG inbound's automatic
|
// local row reserves, emitted or not; db keeps it in the caller's transaction (#6225).
|
||||||
// Xray SOCKS5 relay port. Unlike the retired kernel-module bridge this
|
|
||||||
// checks every qualifying AmneziaWG inbound unconditionally: the embedded
|
|
||||||
// relay has no RouteThroughXray-style opt-in, every one of them gets a
|
|
||||||
// relay inbound (see injectAmneziawgnetSocks). ignoreId excludes one inbound
|
|
||||||
// id from the AmneziaWG candidates, the same way the general DB-backed
|
|
||||||
// conflict query above excludes the inbound being edited from matching
|
|
||||||
// itself. Takes db rather than fetching its own handle so it runs inside the
|
|
||||||
// same serialized transaction as the rest of checkPortConflictTx (#6225) --
|
|
||||||
// otherwise two concurrent AmneziaWG creates could both pass this check
|
|
||||||
// before either row commits.
|
|
||||||
func checkAmneziawgnetSocksConflict(db *gorm.DB, inbound *model.Inbound, ignoreId int, newBits transportBits) (*portConflictDetail, error) {
|
func checkAmneziawgnetSocksConflict(db *gorm.DB, inbound *model.Inbound, ignoreId int, newBits transportBits) (*portConflictDetail, error) {
|
||||||
// A disabled row still owns the slot its id derives: SetInboundEnable flips
|
// A disabled row still owns the slot its id derives: SetInboundEnable flips
|
||||||
// the column with no port check, so enabling it later must not collide.
|
// the column with no port check, so enabling it later must not collide.
|
||||||
@@ -296,10 +285,9 @@ func checkAmneziawgnetSocksConflict(db *gorm.DB, inbound *model.Inbound, ignoreI
|
|||||||
if err := q.Find(&candidates).Error; err != nil {
|
if err := q.Find(&candidates).Error; err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
// Ownership does not depend on the peers: the relay appears when the first
|
||||||
|
// client is added, and the client paths run no port check at all.
|
||||||
for _, c := range candidates {
|
for _, c := range candidates {
|
||||||
if _, ok := amneziawg.InstanceFromInbound(c); !ok {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
if amneziawgnet.SOCKSPortForInbound(c.Id) != inbound.Port {
|
if amneziawgnet.SOCKSPortForInbound(c.Id) != inbound.Port {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -846,12 +846,13 @@ func TestCheckPortConflict_AmneziawgnetSocksRelayReservedRegardlessOfLegacyRoute
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// A qualifying AmneziaWG inbound with no enabled/valid peer at all never
|
// A local AmneziaWG inbound owns its relay port from the row, not from its first
|
||||||
// gets a relay inbound (amneziawg.InstanceFromInbound returns ok=false), so
|
// peer: the relay appears when a client is added, and that path runs no port check.
|
||||||
// its port isn't reserved.
|
func TestCheckPortConflict_AmneziawgnetSocksRelayReservedBeforeTheFirstPeer(t *testing.T) {
|
||||||
func TestCheckPortConflict_AmneziawgnetSocksRelayIgnoredWhenNoQualifyingPeer(t *testing.T) {
|
|
||||||
setupConflictDB(t)
|
setupConflictDB(t)
|
||||||
seedInboundConflict(t, "awg-1", "0.0.0.0", 51820, model.AmneziaWG, ``, `{}`)
|
// The shape normalizeAmneziaWGSettings writes for a fresh AmneziaWG inbound.
|
||||||
|
seedInboundConflict(t, "awg-1", "0.0.0.0", 51820, model.AmneziaWG, ``,
|
||||||
|
`{"server":{"privateKey":"priv","publicKey":"pub","subnetIp":"10.8.1.0","subnetCidr":24},"clients":[]}`)
|
||||||
|
|
||||||
var awgInbound model.Inbound
|
var awgInbound model.Inbound
|
||||||
if err := database.GetDB().Where("tag = ?", "awg-1").First(&awgInbound).Error; err != nil {
|
if err := database.GetDB().Where("tag = ?", "awg-1").First(&awgInbound).Error; err != nil {
|
||||||
@@ -866,8 +867,15 @@ func TestCheckPortConflict_AmneziawgnetSocksRelayIgnoredWhenNoQualifyingPeer(t *
|
|||||||
Port: relayPort,
|
Port: relayPort,
|
||||||
Protocol: model.VLESS,
|
Protocol: model.VLESS,
|
||||||
}
|
}
|
||||||
if got, err := svc.checkPortConflict(candidate, 0); err != nil || got != nil {
|
got, err := svc.checkPortConflict(candidate, 0)
|
||||||
t.Fatalf("an AmneziaWG inbound with no qualifying peer must not reserve its relay port; got=%v err=%v", got, err)
|
if err != nil {
|
||||||
|
t.Fatalf("checkPortConflict: %v", err)
|
||||||
|
}
|
||||||
|
if got == nil {
|
||||||
|
t.Fatalf("an AmneziaWG inbound with no peer yet still owns relay port %d; the save must be refused", relayPort)
|
||||||
|
}
|
||||||
|
if !strings.Contains(got.String(), "awg-1") {
|
||||||
|
t.Fatalf("the conflict must name the inbound owning the port, got %q", got.String())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user