mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-17 15:47:14 +00:00
43e64993fc
* fix(amneziawg): refuse a WireGuard port that is the row's own relay port
All three relay checks filter themselves out of the candidates with id !=
ignoreId, so nothing ever compared an AmneziaWG row's own WireGuard listen port
with the relay port its own id derives. Saving a row on that exact port left the
embedded device (UDP on the inbound's listen address, amneziawgnet/device.go:137)
and its injected relay (TCP and UDP on 127.0.0.1, amneziawgnet/relay.go:47-61)
bound to the same UDP port, so whichever loses the race dies -- and when the
relay loses it, Xray refuses the whole config and takes every other protocol on
the host with it. The first AmneziaWG inbound on port 65101 was enough to reach
it: id 1 derives exactly that port.
The row now states the rule its three siblings do: it owns the slot its id
derives. A node-hosted row still keeps its own port, since it binds no relay on
this host.
TestAddInbound_AmneziawgRefusesItsOwnRelayPort and
TestUpdateInbound_AmneziawgRefusesItsOwnRelayPort fail without this -- both were
watched red first -- and pin the two separate call sites, AddInbound's post-Save
block and checkPortConflictTx's ignoreId > 0 block.
* 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.
* fix(amneziawg): re-run the forward guard once a new row has its own ports
normalizeAmneziaWGSettings validates every client's ForwardedPorts before the row
is saved, and loadPortConflictContext then reads the database -- so the new
AmneziaWG row is never a candidate for itself. A client could forward exactly the
relay port the row's own id derives, or its own WireGuard listen port, and the
create was accepted: at runtime the panel's wildcard forward listener and Xray's
127.0.0.1 relay race for the same port, and a lost relay bind makes Xray refuse
the whole generated config (#6544 review, pre-existing).
The post-Save block is the only place the id is known, so it re-runs the guard
there. Both callers now share amneziaWGForwardedPortsConflict, so the collision
message lives in one place instead of two.
TestAddInbound_AmneziawgRefusesAClientForwardingItsOwnRelayPort fails without
this -- watched red first -- and passes with it.
* fix(amneziawg): stop blocking stored forward specs on a disabled row's slot
Round 2 flagged this PR's widening as the one MEDIUM it introduced, and the code
confirms it: UpdateInboundClient carries a stored ForwardedPorts spec forward for
a partial edit (client_inbound_apply.go:763-765) and re-validates it (:772 and
:909), so after an in-place upgrade an edit that never submitted the field -- a
bot enable/expiry toggle -- is refused over a slot the operator did not touch,
for a relay injectAmneziawgnetSocks does not emit while the row is disabled. The
inbound-save path re-validates every stored spec the same way.
The trade does not pay for itself: the slot this reserves is claimable only by a
spec an operator authors onto 65101-65535, while the cost lands on unrelated
operations. The precise fix -- refuse a newly claimed spec rather than a stored
one, and check the enable transition in SetInboundEnable, where the conflict is
actually created -- is larger than the hole, so the slot goes back to a
documented pre-existing item with its own follow-up.
The create-path re-run added in 80eb5712 is unaffected: it reads the settings
submitted in the same request, so it never refuses a stored value, and its test
still passes.
295 lines
11 KiB
Go
295 lines
11 KiB
Go
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"
|
|
wgutil "github.com/mhsanaei/3x-ui/v3/internal/util/wireguard"
|
|
)
|
|
|
|
// awgRelayWindowSettings builds an AmneziaWG settings blob AddInbound accepts:
|
|
// real X25519 keys, one enabled peer, and an email unique to tag.
|
|
func awgRelayWindowSettings(t *testing.T, tag string) string {
|
|
t.Helper()
|
|
_, clientPub, err := wgutil.GenerateWireguardKeypair()
|
|
if err != nil {
|
|
t.Fatalf("generate client keypair: %v", err)
|
|
}
|
|
return `{"server":{"privateKey":"` + awgTestPrivateKey + `","publicKey":"` + awgTestPublicKey +
|
|
`","subnetIp":"10.8.1.0","subnetCidr":24},"clients":[{"email":"` + tag + `@relay-window","enable":true,"publicKey":"` +
|
|
clientPub + `","allowedIPs":["10.8.1.2/32"]}]}`
|
|
}
|
|
|
|
// awgRelayWindowSettingsWithForward is awgRelayWindowSettings with one client's
|
|
// forwardedPorts set, the field the create-time guard validates.
|
|
func awgRelayWindowSettingsWithForward(t *testing.T, tag, forwardedPorts string) string {
|
|
t.Helper()
|
|
settings := awgRelayWindowSettings(t, tag)
|
|
return strings.Replace(settings, `"enable":true`, `"enable":true,"forwardedPorts":"`+forwardedPorts+`"`, 1)
|
|
}
|
|
|
|
// pushInboundIDSequence makes the next inbounds insert land on nextID, standing
|
|
// in for a long-lived database whose AUTOINCREMENT counter has climbed there.
|
|
func pushInboundIDSequence(t *testing.T, nextID int) {
|
|
t.Helper()
|
|
// The counter is a sqlite_sequence row, so this has no PostgreSQL equivalent.
|
|
if database.IsPostgres() {
|
|
t.Skip("the inbounds AUTOINCREMENT counter is a SQLite row")
|
|
}
|
|
res := database.GetDB().Exec("UPDATE sqlite_sequence SET seq = ? WHERE name = ?", nextID-1, "inbounds")
|
|
if res.Error != nil {
|
|
t.Fatalf("push the inbounds sequence to %d: %v", nextID, res.Error)
|
|
}
|
|
if res.RowsAffected != 1 {
|
|
t.Fatalf("inbounds has no AUTOINCREMENT counter row to push (%d rows updated)", res.RowsAffected)
|
|
}
|
|
}
|
|
|
|
func addAmneziaWGInbound(t *testing.T, tag string, port int, enable bool) *model.Inbound {
|
|
t.Helper()
|
|
created, _, err := (&InboundService{}).AddInbound(&model.Inbound{
|
|
Tag: tag,
|
|
Enable: enable,
|
|
Listen: "0.0.0.0",
|
|
Port: port,
|
|
Protocol: model.AmneziaWG,
|
|
Settings: awgRelayWindowSettings(t, tag),
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("AddInbound(%s): %v", tag, err)
|
|
}
|
|
return created
|
|
}
|
|
|
|
// An id past the slot count used to be refused outright, which capped a
|
|
// database at 435 AmneziaWG inbounds for its entire life (#6537).
|
|
func TestAddInbound_AmneziawgPastTheRelayPortWindowStillCreates(t *testing.T) {
|
|
setupConflictDB(t)
|
|
|
|
// Self-check: the low-id path must work, or the assertion below could pass
|
|
// because the fixture never created an AmneziaWG inbound at all.
|
|
addAmneziaWGInbound(t, "awg-low-id", 51820, true)
|
|
|
|
pushInboundIDSequence(t, 70001)
|
|
created := addAmneziaWGInbound(t, "awg-past-window", 51821, true)
|
|
if created.Id < 436 {
|
|
t.Fatalf("fixture: inbound id %d is still inside the old window", created.Id)
|
|
}
|
|
if port := amneziawgnet.SOCKSPortForInbound(created.Id); port < amneziawgnet.SOCKSBasePort+1 || port > 65535 {
|
|
t.Fatalf("inbound %d derived relay port %d, outside %d..65535", created.Id, port, amneziawgnet.SOCKSBasePort+1)
|
|
}
|
|
}
|
|
|
|
// Wrapping ids makes the id -> relay-port map non-injective, so a create can
|
|
// land on a port an existing inbound's relay already owns.
|
|
func TestAddInbound_AmneziawgRefusesAClaimedRelayPort(t *testing.T) {
|
|
for _, blockerEnabled := range []bool{true, false} {
|
|
t.Run(fmt.Sprintf("blocker enabled=%t", blockerEnabled), func(t *testing.T) {
|
|
setupConflictDB(t)
|
|
blocker := addAmneziaWGInbound(t, "awg-blocker", 51820, blockerEnabled)
|
|
|
|
// One slot-window further on is the id that derives the blocker's port.
|
|
collidingID := blocker.Id + 435
|
|
pushInboundIDSequence(t, collidingID)
|
|
|
|
_, _, err := (&InboundService{}).AddInbound(&model.Inbound{
|
|
Tag: "awg-collides",
|
|
Enable: true,
|
|
Listen: "0.0.0.0",
|
|
Port: 51821,
|
|
Protocol: model.AmneziaWG,
|
|
Settings: awgRelayWindowSettings(t, "awg-collides"),
|
|
})
|
|
if err == nil {
|
|
t.Fatalf("inbound %d derives relay port %d, already owned by %q; the create must be refused",
|
|
collidingID, amneziawgnet.SOCKSPortForInbound(blocker.Id), blocker.Tag)
|
|
}
|
|
if !strings.Contains(err.Error(), blocker.Tag) {
|
|
t.Fatalf("the conflict must name the inbound owning the port, got %v", err)
|
|
}
|
|
// The blocker's own port is its WireGuard one, so without this the
|
|
// message reads as if that inbound listened on an unrelated port.
|
|
if !strings.Contains(err.Error(), "relay port") {
|
|
t.Fatalf("the refusal must say the port is an automatic relay one, got %v", err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Wrapping makes id -> relay port non-injective, so an edit landing on a slot a
|
|
// local inbound already owns has to be refused: the create guard never sees it.
|
|
func TestCheckPortConflict_LocalAmneziawgRelayCollisionBlocksTheEdit(t *testing.T) {
|
|
setupConflictDB(t)
|
|
blocker := addAmneziaWGInbound(t, "awg-blocker", 51820, true)
|
|
|
|
local := &model.Inbound{
|
|
Tag: "awg-edited",
|
|
Enable: true,
|
|
Listen: "0.0.0.0",
|
|
Port: 51821,
|
|
Protocol: model.AmneziaWG,
|
|
Settings: awgRelayWindowSettings(t, "awg-edited"),
|
|
}
|
|
collidingID := blocker.Id + 435
|
|
|
|
got, err := (&InboundService{}).checkPortConflict(local, collidingID)
|
|
if err != nil {
|
|
t.Fatalf("checkPortConflict: %v", err)
|
|
}
|
|
if got == nil {
|
|
t.Fatalf("id %d derives relay port %d, already owned by %q; the save must be refused",
|
|
collidingID, amneziawgnet.SOCKSPortForInbound(blocker.Id), blocker.Tag)
|
|
}
|
|
if !strings.Contains(got.String(), blocker.Tag) {
|
|
t.Fatalf("the conflict must name the inbound owning the port, got %q", got.String())
|
|
}
|
|
}
|
|
|
|
// A disabled row still owns the relay slot its id derives: SetInboundEnable
|
|
// flips the column with no port check, so enabling it later would break Xray.
|
|
func TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot(t *testing.T) {
|
|
setupConflictDB(t)
|
|
owner := addAmneziaWGInbound(t, "awg-disabled", 51820, false)
|
|
relayPort := amneziawgnet.SOCKSPortForInbound(owner.Id)
|
|
|
|
got, err := (&InboundService{}).checkPortConflict(&model.Inbound{
|
|
Tag: "takes-the-slot",
|
|
Enable: true,
|
|
Listen: "0.0.0.0",
|
|
Port: relayPort,
|
|
Protocol: model.VLESS,
|
|
Settings: `{"clients":[]}`,
|
|
}, 0)
|
|
if err != nil {
|
|
t.Fatalf("checkPortConflict: %v", err)
|
|
}
|
|
if got == nil {
|
|
t.Fatalf("inbound #%d is disabled but still owns relay port %d; the save must be refused",
|
|
owner.Id, relayPort)
|
|
}
|
|
if !strings.Contains(got.String(), owner.Tag) {
|
|
t.Fatalf("the conflict must name the inbound owning the port, got %q", got.String())
|
|
}
|
|
}
|
|
|
|
// The forwarded-ports guard runs before Save, when the row has no id yet, so a
|
|
// client's spec never saw the relay port the row itself derives.
|
|
func TestAddInbound_AmneziawgRefusesAClientForwardingItsOwnRelayPort(t *testing.T) {
|
|
setupConflictDB(t)
|
|
|
|
placeholder := addAmneziaWGInbound(t, "awg-placeholder", 51820, true)
|
|
ownPort := amneziawgnet.SOCKSPortForInbound(placeholder.Id + 1)
|
|
|
|
_, _, err := (&InboundService{}).AddInbound(&model.Inbound{
|
|
Tag: "awg-forward",
|
|
Enable: true,
|
|
Listen: "0.0.0.0",
|
|
Port: 51821,
|
|
Protocol: model.AmneziaWG,
|
|
Settings: awgRelayWindowSettingsWithForward(t, "awg-forward", fmt.Sprintf("%d", ownPort)),
|
|
})
|
|
if err == nil {
|
|
t.Fatalf("inbound #%d derives relay port %d and its own client forwards that port; the create must be refused",
|
|
placeholder.Id+1, ownPort)
|
|
}
|
|
if !strings.Contains(err.Error(), "forwardedPorts") {
|
|
t.Fatalf("the refusal must come from the forwarded-ports guard, got %v", err)
|
|
}
|
|
}
|
|
|
|
// The row's own WireGuard port can be the relay port its own id derives, and
|
|
// every relay check excludes that id, so nothing else compares the two.
|
|
func TestAddInbound_AmneziawgRefusesItsOwnRelayPort(t *testing.T) {
|
|
setupConflictDB(t)
|
|
|
|
// Read the sequence instead of assuming id 1: the victim's own derived port
|
|
// has to be known before it is created.
|
|
placeholder := addAmneziaWGInbound(t, "awg-placeholder", 51820, true)
|
|
selfPort := amneziawgnet.SOCKSPortForInbound(placeholder.Id + 1)
|
|
|
|
_, _, err := (&InboundService{}).AddInbound(&model.Inbound{
|
|
Tag: "awg-self",
|
|
Enable: true,
|
|
Listen: "0.0.0.0",
|
|
Port: selfPort,
|
|
Protocol: model.AmneziaWG,
|
|
Settings: awgRelayWindowSettings(t, "awg-self"),
|
|
})
|
|
if err == nil {
|
|
t.Fatalf("WireGuard port %d is inbound #%d's own relay port; the create must be refused",
|
|
selfPort, placeholder.Id+1)
|
|
}
|
|
if !strings.Contains(err.Error(), "relay port") {
|
|
t.Fatalf("the refusal must say the port is an automatic relay one, got %v", err)
|
|
}
|
|
}
|
|
|
|
// The edit path knows the id the relay port comes from, so it has to refuse the
|
|
// same self-collision -- the reverse check skips the row it computes for.
|
|
func TestUpdateInbound_AmneziawgRefusesItsOwnRelayPort(t *testing.T) {
|
|
setupConflictDB(t)
|
|
created := addAmneziaWGInbound(t, "awg-self-edit", 51820, true)
|
|
|
|
edit := *created
|
|
edit.Port = amneziawgnet.SOCKSPortForInbound(created.Id)
|
|
if edit.Port == created.Port {
|
|
t.Fatalf("fixture: inbound #%d already listens on its derived relay port", created.Id)
|
|
}
|
|
|
|
_, _, err := (&InboundService{}).UpdateInbound(&edit)
|
|
if err == nil {
|
|
t.Fatalf("WireGuard port %d is inbound #%d's own relay port; the save must be refused",
|
|
edit.Port, created.Id)
|
|
}
|
|
if !strings.Contains(err.Error(), "relay port") {
|
|
t.Fatalf("the refusal must say the port is an automatic relay one, got %v", err)
|
|
}
|
|
}
|
|
|
|
// A row adopted from a node keeps the protocol it arrived with and its central
|
|
// id (inbound_node.go:737), but gets no relay -- so its slot can never be taken.
|
|
func TestCheckPortConflict_NodeAssignedAmneziawgOwnsNoRelaySlot(t *testing.T) {
|
|
setupConflictDB(t)
|
|
blocker := addAmneziaWGInbound(t, "awg-blocker", 51820, true)
|
|
|
|
nodeID := 7
|
|
adopted := &model.Inbound{
|
|
Tag: "awg-adopted",
|
|
Enable: true,
|
|
Listen: "0.0.0.0",
|
|
Port: 51821,
|
|
Protocol: model.AmneziaWG,
|
|
Settings: awgRelayWindowSettings(t, "awg-adopted"),
|
|
NodeID: &nodeID,
|
|
}
|
|
collidingID := blocker.Id + 435
|
|
if amneziawgnet.SOCKSPortForInbound(collidingID) != amneziawgnet.SOCKSPortForInbound(blocker.Id) {
|
|
t.Fatalf("fixture: id %d does not derive the blocker's relay port", collidingID)
|
|
}
|
|
|
|
got, err := (&InboundService{}).checkPortConflict(adopted, collidingID)
|
|
if err != nil {
|
|
t.Fatalf("checkPortConflict: %v", err)
|
|
}
|
|
if got != nil {
|
|
t.Fatalf("id %d is node-assigned and binds no relay, so it cannot collide; got %q",
|
|
collidingID, got.String())
|
|
}
|
|
|
|
// The same rule covers the row's own port: with no relay on this host, its
|
|
// WireGuard port may legitimately BE the port its id would derive.
|
|
adopted.Port = amneziawgnet.SOCKSPortForInbound(collidingID)
|
|
got, err = (&InboundService{}).checkPortConflict(adopted, collidingID)
|
|
if err != nil {
|
|
t.Fatalf("checkPortConflict: %v", err)
|
|
}
|
|
if got != nil {
|
|
t.Fatalf("id %d is node-assigned and binds no relay, so its own port is not a conflict; got %q",
|
|
collidingID, got.String())
|
|
}
|
|
}
|