diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 1950f1008..31fc22e7c 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -1238,8 +1238,11 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo return err } // The relay port is derived from the id, only known after Save, and only a - // local row owns one: checkPortConflictTx ran neither check with ignoreId==0. + // local row owns one: checkPortConflictTx ran no relay check with ignoreId==0. if inbound.NodeID == nil && inbound.Protocol == model.AmneziaWG { + if self := amneziawgnetSocksSelfConflict(inbound, inbound.Id); self != "" { + return common.NewError(self) + } conflict, cErr := checkAmneziawgnetSocksRelayCollision(tx, inbound.Id) if cErr != nil { return cErr diff --git a/internal/web/service/inbound_amneziawg_relay_window_test.go b/internal/web/service/inbound_amneziawg_relay_window_test.go index 08f4de9c6..41ec5a85d 100644 --- a/internal/web/service/inbound_amneziawg_relay_window_test.go +++ b/internal/web/service/inbound_amneziawg_relay_window_test.go @@ -168,6 +168,55 @@ func TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot(t *testing.T) } } +// 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) { @@ -197,4 +246,16 @@ func TestCheckPortConflict_NodeAssignedAmneziawgOwnsNoRelaySlot(t *testing.T) { 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()) + } } diff --git a/internal/web/service/port_conflict.go b/internal/web/service/port_conflict.go index 8e70af55c..c1377e1f0 100644 --- a/internal/web/service/port_conflict.go +++ b/internal/web/service/port_conflict.go @@ -223,6 +223,9 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po // The reverse direction, only meaningful once the id is known -- AddInbound // runs it after Save. Only a local row owns a relay slot (#6537 review). if inbound.NodeID == nil && inbound.Protocol == model.AmneziaWG && ignoreId > 0 { + if self := amneziawgnetSocksSelfConflict(inbound, ignoreId); self != "" { + return nil, common.NewError(self) + } conflict, err := checkAmneziawgnetSocksRelayCollision(db, ignoreId) if err != nil { return nil, err @@ -330,6 +333,20 @@ func checkAmneziawgnetSocksRelayCollision(db *gorm.DB, id int) (*portConflictDet return nil, nil } +// amneziawgnetSocksSelfConflict: a row's own WireGuard port vs the relay port its +// own id derives -- all three checks below exclude that id, so nothing else does. +func amneziawgnetSocksSelfConflict(inbound *model.Inbound, id int) string { + if id <= 0 || inbound.NodeID != nil || !listenOverlaps("127.0.0.1", inbound.Listen) { + return "" + } + relayPort := amneziawgnet.SOCKSPortForInbound(id) + if inbound.Port != relayPort { + return "" + } + return fmt.Sprintf("WireGuard port %d is inbound #%d's own SOCKS5 relay port on 127.0.0.1; choose a different WireGuard port", + relayPort, id) +} + // checkAmneziawgnetSocksReverseConflict mirrors checkAmneziawgnetSocksConflict: // does id's own derived relay port collide with some other inbound's port. func checkAmneziawgnetSocksReverseConflict(db *gorm.DB, id int) (*portConflictDetail, error) {