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.
This commit is contained in:
BlindMaster24
2026-09-15 12:55:08 +03:00
parent 35ffba1eea
commit 80eb5712b6
3 changed files with 69 additions and 2 deletions
+5
View File
@@ -1257,6 +1257,11 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo
if conflict != nil {
return common.NewError(conflict.String())
}
// The clients' forward specs were validated while this row had no id,
// so the ports it now derives were never in the guard's context.
if aErr := s.checkAmneziaWGForwardedPorts(tx, inbound.Settings); aErr != nil {
return aErr
}
}
// Emails seeded here (import's ClientStats, e.g. the controller's forced
// Enable=true on every imported stat row) are authoritative for this call
+31 -2
View File
@@ -278,8 +278,8 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, oldS
}
for i := range parsed.Clients {
c := &parsed.Clients[i]
if hit := s.checkForwardedPortsConflict(portCtx, c.ForwardedPorts); hit != "" {
return fmt.Errorf("amneziawg: client %q forwardedPorts collides with %s", c.Email, hit)
if err := s.amneziaWGForwardedPortsConflict(portCtx, c); err != nil {
return err
}
if err := amneziawg.ValidateConfigValue("email", c.Email); err != nil {
return fmt.Errorf("amneziawg: %w", err)
@@ -333,6 +333,35 @@ func (s *InboundService) loadPortConflictContext(db *gorm.DB) (portConflictConte
return ctx, err
}
// amneziaWGForwardedPortsConflict renders one client's ForwardedPorts collision,
// or nil: the single copy both the pre-Save pass and the post-Save re-run use.
func (s *InboundService) amneziaWGForwardedPortsConflict(ctx portConflictContext, c *model.Client) error {
hit := s.checkForwardedPortsConflict(ctx, c.ForwardedPorts)
if hit == "" {
return nil
}
return fmt.Errorf("amneziawg: client %q forwardedPorts collides with %s", c.Email, hit)
}
// checkAmneziaWGForwardedPorts re-runs the guard over one row's stored clients:
// on create it ran before Save, when the row's own ports were not in the context.
func (s *InboundService) checkAmneziaWGForwardedPorts(db *gorm.DB, settings string) error {
var parsed amneziawg.InboundSettings
if err := json.Unmarshal([]byte(settings), &parsed); err != nil {
return nil
}
ctx, err := s.loadPortConflictContext(db)
if err != nil {
return err
}
for i := range parsed.Clients {
if err := s.amneziaWGForwardedPortsConflict(ctx, &parsed.Clients[i]); err != nil {
return err
}
}
return nil
}
// 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 {
@@ -24,6 +24,14 @@ func awgRelayWindowSettings(t *testing.T, tag string) string {
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) {
@@ -168,6 +176,31 @@ func TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot(t *testing.T)
}
}
// 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) {