mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-15 19:27:06 +07:00
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:
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user