diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 9b90660cc..5b2aa2488 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -1785,7 +1785,8 @@ "type": "string" }, "forwardedPorts": { - "description": "AmneziaWG per-client port-forwarding spec, e.g. \"80,443,8000-8100\"", + "description": "AmneziaWG port-forwarding spec, e.g. \"80,443,8000-8100\"; \"\" withdraws the forwards, omit to keep them", + "nullable": true, "type": "string" }, "group": { diff --git a/frontend/public/openapi.json b/frontend/public/openapi.json index 9b90660cc..5b2aa2488 100644 --- a/frontend/public/openapi.json +++ b/frontend/public/openapi.json @@ -1785,7 +1785,8 @@ "type": "string" }, "forwardedPorts": { - "description": "AmneziaWG per-client port-forwarding spec, e.g. \"80,443,8000-8100\"", + "description": "AmneziaWG port-forwarding spec, e.g. \"80,443,8000-8100\"; \"\" withdraws the forwards, omit to keep them", + "nullable": true, "type": "string" }, "group": { diff --git a/frontend/src/generated/examples.ts b/frontend/src/generated/examples.ts index 6c8c5aca5..384de907c 100644 --- a/frontend/src/generated/examples.ts +++ b/frontend/src/generated/examples.ts @@ -431,7 +431,7 @@ export const EXAMPLES: Record = { "enable": false, "expiryTime": 0, "flow": "", - "forwardedPorts": "", + "forwardedPorts": null, "group": "", "id": "", "keepAlive": 0, diff --git a/frontend/src/generated/schemas.ts b/frontend/src/generated/schemas.ts index 8165b101b..479ec9fdb 100644 --- a/frontend/src/generated/schemas.ts +++ b/frontend/src/generated/schemas.ts @@ -1759,7 +1759,8 @@ export const SCHEMAS: Record = { "type": "string" }, "forwardedPorts": { - "description": "AmneziaWG per-client port-forwarding spec, e.g. \"80,443,8000-8100\"", + "description": "AmneziaWG port-forwarding spec, e.g. \"80,443,8000-8100\"; \"\" withdraws the forwards, omit to keep them", + "nullable": true, "type": "string" }, "group": { diff --git a/frontend/src/generated/types.ts b/frontend/src/generated/types.ts index a7e158e36..c9d50cfc6 100644 --- a/frontend/src/generated/types.ts +++ b/frontend/src/generated/types.ts @@ -426,7 +426,7 @@ export interface Client { enable: boolean; expiryTime: number; flow?: string; - forwardedPorts?: string; + forwardedPorts?: string | null; group?: string; id?: string; keepAlive?: number | string; diff --git a/frontend/src/generated/zod.ts b/frontend/src/generated/zod.ts index 692a2f677..d86fa6391 100644 --- a/frontend/src/generated/zod.ts +++ b/frontend/src/generated/zod.ts @@ -450,7 +450,7 @@ export const ClientSchema = z.object({ enable: z.boolean(), expiryTime: z.number().int(), flow: z.string().optional(), - forwardedPorts: z.string().optional(), + forwardedPorts: z.string().nullable().optional(), group: z.string().optional(), id: z.string().optional(), keepAlive: z.union([z.number().int(), z.string()]).optional(), diff --git a/internal/amneziawg/instance.go b/internal/amneziawg/instance.go index 8c75af6ed..81a58823d 100644 --- a/internal/amneziawg/instance.go +++ b/internal/amneziawg/instance.go @@ -34,7 +34,7 @@ func InstanceFromInbound(ib *model.Inbound) (Instance, bool) { PublicKey: c.PublicKey, PresharedKey: c.PreSharedKey, AllowedIPs: c.AllowedIPs, - ForwardedPorts: c.ForwardedPorts, + ForwardedPorts: c.ForwardedPortsSpec(), }) } if len(peers) == 0 { diff --git a/internal/database/db.go b/internal/database/db.go index 80b680365..f3cbb7c47 100644 --- a/internal/database/db.go +++ b/internal/database/db.go @@ -1530,7 +1530,7 @@ func runSeeders(isUsersEmpty bool) error { } if empty && isUsersEmpty { - seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "UppercaseFreedomFinalRulesFix", "InboundRealityFinalmaskTcpStrip", "ApiTokensHash", "LegacyProxySettingsCleanup", "SessionSecretRotation", "OutboundRemovedKeysFix", "FreedomDomainStrategyFix", "DNSOutboundLegacyKeysFix", "DNSOutboundQTypeZeroFix", "WireguardDomainStrategyFix", "XdnsFinalmaskNamesAddrsFix", "UDPHopClientMaskFix", "HostVerifyPeerCertByNameBoolFix", "HostMihomoX25519Reset", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "NodeDownNotified", "ResetIpLimitNoFail2ban", "ClientGroupNameTrim", "OwnerlessClientHwidPurge"} + seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "UppercaseFreedomFinalRulesFix", "InboundRealityFinalmaskTcpStrip", "ApiTokensHash", "LegacyProxySettingsCleanup", "SessionSecretRotation", "OutboundRemovedKeysFix", "FreedomDomainStrategyFix", "DNSOutboundLegacyKeysFix", "DNSOutboundQTypeZeroFix", "WireguardDomainStrategyFix", "XdnsFinalmaskNamesAddrsFix", "UDPHopClientMaskFix", "HostVerifyPeerCertByNameBoolFix", "HostMihomoX25519Reset", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "NodeDownNotified", "ResetIpLimitNoFail2ban", "ClientGroupNameTrim", "OwnerlessClientHwidPurge", "ClientForwardedPortsBackfill"} for _, name := range seeders { if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil { return err @@ -1725,6 +1725,12 @@ func runSeeders(isUsersEmpty bool) error { } } + if !slices.Contains(seedersHistory, "ClientForwardedPortsBackfill") { + if err := backfillClientForwardedPorts(); err != nil { + return err + } + } + if err := seedHostsFromExternalProxy(); err != nil { return err } @@ -1920,6 +1926,40 @@ func clearLegacyProxySettings() error { }) } +// The client editor fills forwardedPorts from the record, which kept the value it was created +// with; an emptied field now withdraws the forwards, so the record must hold the live spec. +func backfillClientForwardedPorts() error { + return db.Transaction(func(tx *gorm.DB) error { + var inbounds []model.Inbound + if err := tx.Select("id", "settings").Where("protocol = ?", model.AmneziaWG).Find(&inbounds).Error; err != nil { + return err + } + for _, ib := range inbounds { + var parsed struct { + Clients []struct { + Email string `json:"email"` + ForwardedPorts string `json:"forwardedPorts"` + } `json:"clients"` + } + if json.Unmarshal([]byte(ib.Settings), &parsed) != nil { + continue + } + for _, c := range parsed.Clients { + spec := strings.TrimSpace(c.ForwardedPorts) + if c.Email == "" || spec == "" { + continue + } + if err := tx.Model(&model.ClientRecord{}). + Where("email = ? AND (wg_forwarded_ports IS NULL OR wg_forwarded_ports = '')", c.Email). + UpdateColumn("wg_forwarded_ports", spec).Error; err != nil { + return err + } + } + } + return tx.Create(&model.HistoryOfSeeders{SeederName: "ClientForwardedPortsBackfill"}).Error + }) +} + // The HWID gate stored a row for any subId a request named, owned or not; those rows // belong to no subscription and only grow the database. func purgeOwnerlessClientHwids() error { diff --git a/internal/database/db_seed_test.go b/internal/database/db_seed_test.go index 8bf177254..e42be24be 100644 --- a/internal/database/db_seed_test.go +++ b/internal/database/db_seed_test.go @@ -452,3 +452,43 @@ func TestOwnerlessClientHwidsPurgedOnUpgrade(t *testing.T) { t.Fatalf("hwid rows after upgrade = %q, want [owned]", subs) } } + +// The editor fills forwardedPorts from the record, and an emptied field now withdraws the +// forwards: a record left at its creation value would drop live forwards on the next save. +func TestClientForwardedPortsBackfilledOnUpgrade(t *testing.T) { + dbPath := filepath.Join(t.TempDir(), "x-ui.db") + if err := InitDB(dbPath); err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = CloseDB() }) + if err := db.Create(&model.ClientRecord{Email: "fwd@x", Enable: true}).Error; err != nil { + t.Fatalf("seed client: %v", err) + } + if err := db.Create(&model.ClientRecord{Email: "kept@x", Enable: true, ForwardedPorts: "9000"}).Error; err != nil { + t.Fatalf("seed client: %v", err) + } + if err := db.Create(&model.Inbound{ + Tag: "awg", Port: 51820, Protocol: model.AmneziaWG, Enable: true, + Settings: `{"clients":[{"email":"fwd@x","forwardedPorts":"8022"},{"email":"kept@x","forwardedPorts":"9001"}]}`, + }).Error; err != nil { + t.Fatalf("seed inbound: %v", err) + } + if err := db.Where("seeder_name = ?", "ClientForwardedPortsBackfill").Delete(&model.HistoryOfSeeders{}).Error; err != nil { + t.Fatalf("clear seeder history: %v", err) + } + if err := CloseDB(); err != nil { + t.Fatalf("CloseDB: %v", err) + } + if err := InitDB(dbPath); err != nil { + t.Fatalf("reopen InitDB: %v", err) + } + for email, want := range map[string]string{"fwd@x": "8022", "kept@x": "9000"} { + var rec model.ClientRecord + if err := db.Where("email = ?", email).First(&rec).Error; err != nil { + t.Fatalf("read %s: %v", email, err) + } + if rec.ForwardedPorts != want { + t.Errorf("%s forwardedPorts = %q after upgrade, want %q", email, rec.ForwardedPorts, want) + } + } +} diff --git a/internal/database/model/model.go b/internal/database/model/model.go index cf93ba7fe..06120d9f9 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -955,7 +955,7 @@ type Client struct { AllowedIPsByInbound map[int][]string `json:"allowedIPsByInbound,omitempty"` PreSharedKey string `json:"preSharedKey,omitempty"` KeepAlive *KeepAliveValue `json:"keepAlive,omitempty"` // Numeric values or AmneziaWG ranges; 0 disables, omission preserves. - ForwardedPorts string `json:"forwardedPorts,omitempty"` // AmneziaWG per-client port-forwarding spec, e.g. "80,443,8000-8100" + ForwardedPorts *string `json:"forwardedPorts,omitempty"` // AmneziaWG port-forwarding spec, e.g. "80,443,8000-8100"; "" withdraws the forwards, omit to keep them Secret string `json:"secret,omitempty" example:"ee1234567890abcdef1234567890abcd7777772e636c6f7564666c6172652e636f6d"` AdTag string `json:"adTag,omitempty" example:"0123456789abcdef0123456789abcdef"` Email string `json:"email"` // Client email identifier @@ -1273,6 +1273,27 @@ func keepAliveValue(value *KeepAliveValue) KeepAliveValue { return *value } +// ForwardedPortsSpec is the client's port-forwarding spec, "" when unset. +func (c Client) ForwardedPortsSpec() string { + if c.ForwardedPorts == nil { + return "" + } + return strings.TrimSpace(*c.ForwardedPorts) +} + +// ForwardedPortsPtr wraps an explicit spec, "" included: distinct from a nil +// ForwardedPorts, which means the field was never sent. +func ForwardedPortsPtr(spec string) *string { return &spec } + +// nonEmptyForwardedPorts keeps a stored "" nil: a record re-saved as a client must not +// read as a request to withdraw the forwards. +func nonEmptyForwardedPorts(spec string) *string { + if spec == "" { + return nil + } + return ForwardedPortsPtr(spec) +} + func (c *Client) ToRecord() *ClientRecord { rec := &ClientRecord{ Email: c.Email, @@ -1304,7 +1325,7 @@ func (c *Client) ToRecord() *ClientRecord { AllowedIPs: strings.Join(c.AllowedIPs, ","), PreSharedKey: c.PreSharedKey, KeepAlive: keepAliveValue(c.KeepAlive), - ForwardedPorts: c.ForwardedPorts, + ForwardedPorts: c.ForwardedPortsSpec(), Secret: c.Secret, AdTag: c.AdTag, } @@ -1364,7 +1385,7 @@ func (r *ClientRecord) ToClient() *Client { AllowedIPs: splitWireguardAllowedIPs(r.AllowedIPs), PreSharedKey: r.PreSharedKey, KeepAlive: keepAlivePtr(r.KeepAlive), - ForwardedPorts: r.ForwardedPorts, + ForwardedPorts: nonEmptyForwardedPorts(r.ForwardedPorts), Secret: r.Secret, AdTag: r.AdTag, } diff --git a/internal/database/model/model_wireguard_test.go b/internal/database/model/model_wireguard_test.go index f2d7b8898..10c0c7ee9 100644 --- a/internal/database/model/model_wireguard_test.go +++ b/internal/database/model/model_wireguard_test.go @@ -16,6 +16,8 @@ func TestClientToRecordRoundTripWireGuard(t *testing.T) { AllowedIPs: []string{"10.0.0.2/32", "fd00::2/128"}, PreSharedKey: "cHNrLWJhc2U2NC0zMmJ5dGVzLXBsYWNlaG9sZGVyISE=", KeepAlive: KeepAlivePtr(25), + + ForwardedPorts: ForwardedPortsPtr("80,8000-8100"), } rec := c.ToRecord() @@ -32,6 +34,7 @@ func TestClientToRecordRoundTripWireGuard(t *testing.T) { {"PublicKey", c.PublicKey, got.PublicKey}, {"PreSharedKey", c.PreSharedKey, got.PreSharedKey}, {"KeepAlive", c.KeepAliveSeconds(), got.KeepAliveSeconds()}, + {"ForwardedPorts", c.ForwardedPortsSpec(), got.ForwardedPortsSpec()}, } { if f.a != f.b { t.Errorf("%s round-trip = %v, want %v", f.name, f.b, f.a) @@ -59,6 +62,15 @@ func TestClientToClientOmitsUnsetKeepAlive(t *testing.T) { } } +// An explicit "" withdraws the forwards, which a re-saved record must never ask for. +func TestClientToClientOmitsUnsetForwardedPorts(t *testing.T) { + rec := &ClientRecord{Email: "awg@example.test", Enable: true} + + if got := rec.ToClient().ForwardedPorts; got != nil { + t.Fatalf("ForwardedPorts for a record that never set any = %q, want nil", *got) + } +} + func TestClientRecordEmptyAllowedIPs(t *testing.T) { rec := &ClientRecord{Email: "bob@example.test", AllowedIPs: ""} if got := rec.ToClient().AllowedIPs; got != nil { diff --git a/internal/web/service/client_attach_test.go b/internal/web/service/client_attach_test.go index 65000a403..45c123c2b 100644 --- a/internal/web/service/client_attach_test.go +++ b/internal/web/service/client_attach_test.go @@ -210,8 +210,8 @@ func TestAttachDoesNotCarryForwardedPorts(t *testing.T) { t.Fatalf("GetClients(own): %v", err) } for _, c := range clients { - if c.Email == "forwarder" && c.ForwardedPorts != "" { - t.Fatalf("attach carried forwardedPorts %q onto the master's inbound", c.ForwardedPorts) + if c.Email == "forwarder" && c.ForwardedPortsSpec() != "" { + t.Fatalf("attach carried forwardedPorts %q onto the master's inbound", c.ForwardedPortsSpec()) } } }) diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index f6fe94586..80dba5f38 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -113,7 +113,7 @@ func (s *ClientService) BulkAttach(inboundSvc *InboundService, emails []string, } client := *rec.ToClient() // Forwards are set per inbound; see Attach. - client.ForwardedPorts = "" + client.ForwardedPorts = nil if flow, ok := flowsByEmail[rec.Email]; ok && flow != "" { client.Flow = flow } diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index 904f93037..e2878159e 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -213,6 +213,10 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate return false, err } normalizeClientTrafficReset(&client) + // Nothing to withdraw on a new client, so a blank spec is no spec. + if client.ForwardedPortsSpec() == "" { + client.ForwardedPorts = nil + } client.ClientHostRuleIds = model.NormalizeClientHostRuleIds(client.ClientHostRuleIds) if len(payload.InboundIds) == 0 { return false, common.NewError("at least one inbound is required") @@ -938,6 +942,16 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model return needRestart, err } + // The AmneziaWG forwards too: the record merge keeps a stored spec when the settings + // carry none, so a withdrawal is written here; nil (not sent) leaves it alone. + if updated.ForwardedPorts != nil { + if err := database.GetDB().Model(&model.ClientRecord{}). + Where("id = ?", id). + UpdateColumn("wg_forwarded_ports", updated.ForwardedPortsSpec()).Error; err != nil { + return needRestart, err + } + } + // Same shape as the group write above: SyncInbound keeps a stored ad-tag // when the incoming settings carry none, so clearing the override must be // applied here, where the editor always round-trips the field. @@ -1186,7 +1200,7 @@ func (s *ClientService) Attach(inboundSvc *InboundService, id int, inboundIds [] copyClient := *clientWire // A forward is a listener on the target inbound's host, set per inbound: carried over, it // would let a node-reported client choose ports on this host. - copyClient.ForwardedPorts = "" + copyClient.ForwardedPorts = nil if !addressesFitAmneziaWGInbound(copyClient.AllowedIPs, inbound) { copyClient.AllowedIPs = nil } diff --git a/internal/web/service/client_inbound_apply.go b/internal/web/service/client_inbound_apply.go index cc563cdf2..f1142d87f 100644 --- a/internal/web/service/client_inbound_apply.go +++ b/internal/web/service/client_inbound_apply.go @@ -61,7 +61,7 @@ func validateAmneziaWGClientWrite(inbound *model.Inbound, clients []model.Client inst := amneziawg.Instance{Listen: inbound.Listen, IPv6Enabled: settings.Server.IPv6Enabled} for _, client := range clients { inst.Peers = append(inst.Peers, amneziawg.Peer{ - AllowedIPs: client.AllowedIPs, ForwardedPorts: client.ForwardedPorts, + AllowedIPs: client.AllowedIPs, ForwardedPorts: client.ForwardedPortsSpec(), }) } if err := amneziawgkern.ValidateInstanceFeatures(inst, settings.Server.RouteThroughXray); err != nil { @@ -537,7 +537,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model } } if oldInbound.Protocol == model.AmneziaWG { - if hit := inboundSvc.checkForwardedPortsConflict(portCtx, client.ForwardedPorts); hit != "" { + if hit := inboundSvc.checkForwardedPortsConflict(portCtx, client.ForwardedPortsSpec()); hit != "" { return false, common.NewError("amneziawg: forwardedPorts collides with", hit) } } @@ -607,7 +607,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model return pErr } for i := range clients { - if hit := inboundSvc.checkForwardedPortsConflict(txPortCtx, clients[i].ForwardedPorts); hit != "" { + if hit := inboundSvc.checkForwardedPortsConflict(txPortCtx, clients[i].ForwardedPortsSpec()); hit != "" { return common.NewError("amneziawg: forwardedPorts collides with", hit) } } @@ -845,8 +845,8 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo if clients[0].KeepAlive == nil { clients[0].KeepAlive = old.KeepAlive } - // AWG partial edits preserve forwarding specs; kernel mode rejects legacy specs. - if oldInbound.Protocol == model.AmneziaWG && clients[0].ForwardedPorts == "" { + // AWG partial edits (nil: the field was not sent) keep the stored spec; "" withdraws it. + if oldInbound.Protocol == model.AmneziaWG && clients[0].ForwardedPorts == nil { clients[0].ForwardedPorts = old.ForwardedPorts } } @@ -858,7 +858,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo if err != nil { return false, err } - if hit := inboundSvc.checkForwardedPortsConflict(portCtx, clients[0].ForwardedPorts); hit != "" { + if hit := inboundSvc.checkForwardedPortsConflict(portCtx, clients[0].ForwardedPortsSpec()); hit != "" { return false, common.NewError("amneziawg: forwardedPorts collides with", hit) } } @@ -911,10 +911,14 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo if clients[0].KeepAlive != nil { newMap["keepAlive"] = clients[0].KeepAlive } - if oldInbound.Protocol == model.AmneziaWG && clients[0].ForwardedPorts != "" { - newMap["forwardedPorts"] = clients[0].ForwardedPorts + if oldInbound.Protocol == model.AmneziaWG && clients[0].ForwardedPortsSpec() != "" { + newMap["forwardedPorts"] = clients[0].ForwardedPortsSpec() } } + // Withdrawn forwards are stored as no spec at all, whatever the protocol. + if clients[0].ForwardedPortsSpec() == "" { + delete(newMap, "forwardedPorts") + } if oldClientMap != nil && sameClientConfigExceptUpdatedAt(oldClientMap, newMap) { if v, ok2 := oldClientMap["updated_at"]; ok2 { newMap["updated_at"] = v @@ -995,7 +999,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo if pErr != nil { return pErr } - if hit := inboundSvc.checkForwardedPortsConflict(txPortCtx, clients[0].ForwardedPorts); hit != "" { + if hit := inboundSvc.checkForwardedPortsConflict(txPortCtx, clients[0].ForwardedPortsSpec()); hit != "" { return common.NewError("amneziawg: forwardedPorts collides with", hit) } } diff --git a/internal/web/service/client_link.go b/internal/web/service/client_link.go index 912408bb2..fe69c523b 100644 --- a/internal/web/service/client_link.go +++ b/internal/web/service/client_link.go @@ -63,6 +63,11 @@ func applyClientRecordMerge(row *model.ClientRecord, incoming *model.ClientRecor if incoming.KeepAlive != "" { row.KeepAlive = incoming.KeepAlive } + // Guarded like Group: only an AmneziaWG entry is sure to carry the spec, and the record + // is shared with every other inbound of the client. + if incoming.ForwardedPorts != "" { + row.ForwardedPorts = incoming.ForwardedPorts + } row.SubID = incoming.SubID row.LimitIP = incoming.LimitIP row.TotalGB = incoming.TotalGB diff --git a/internal/web/service/inbound_amneziawg.go b/internal/web/service/inbound_amneziawg.go index e4e099ea2..c32064198 100644 --- a/internal/web/service/inbound_amneziawg.go +++ b/internal/web/service/inbound_amneziawg.go @@ -324,7 +324,7 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, old if client.Enable { inst.Peers = append(inst.Peers, amneziawg.Peer{ AllowedIPs: client.AllowedIPs, - ForwardedPorts: client.ForwardedPorts, + ForwardedPorts: client.ForwardedPortsSpec(), }) } } @@ -372,7 +372,7 @@ func (s *InboundService) loadPortConflictContext(db *gorm.DB, nodeID *int) (port // 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) + hit := s.checkForwardedPortsConflict(ctx, c.ForwardedPortsSpec()) if hit == "" { return nil } diff --git a/internal/web/service/inbound_amneziawg_test.go b/internal/web/service/inbound_amneziawg_test.go index 94c057d52..2a2d48df3 100644 --- a/internal/web/service/inbound_amneziawg_test.go +++ b/internal/web/service/inbound_amneziawg_test.go @@ -580,3 +580,137 @@ func TestNormalizeAmneziaWGSettingsGatesHostReadinessOnlyWhenEnablingKernelMode( }) } } + +func inboundForwardedPorts(t *testing.T, inboundSvc *InboundService, ibId int, email string) string { + t.Helper() + ib, err := inboundSvc.GetInbound(ibId) + if err != nil { + t.Fatalf("GetInbound %d: %v", ibId, err) + } + clients, err := inboundSvc.GetClients(ib) + if err != nil { + t.Fatalf("GetClients %d: %v", ibId, err) + } + for i := range clients { + if clients[i].Email == email { + return clients[i].ForwardedPortsSpec() + } + } + t.Fatalf("email %q not found on inbound %d", email, ibId) + return "" +} + +// seedForwardedPortsClient attaches one AmneziaWG client already holding a +// port forward, and returns its inbound and client-record id. +func seedForwardedPortsClient(t *testing.T, email, forwardedPorts string) (*model.Inbound, int) { + t.Helper() + svc := &ClientService{} + + seeded := model.Client{ + Email: email, + SubID: "sub-" + email, + Enable: true, + PublicKey: "QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUE=", + AllowedIPs: []string{"10.8.1.2/32"}, + ForwardedPorts: model.ForwardedPortsPtr(forwardedPorts), + } + ib := mkInbound(t, 51830, model.AmneziaWG, clientsSettings(t, []model.Client{seeded})) + if err := svc.SyncInbound(nil, ib.Id, []model.Client{seeded}); err != nil { + t.Fatalf("seed linkage: %v", err) + } + return ib, lookupClientRecord(t, email).Id +} + +// The client form sends "" once the field was emptied. Carried forward like an +// omitted field, the save was reported as done and the port stayed open. +func TestUpdateCanWithdrawForwardedPorts(t *testing.T) { + setupBulkDB(t) + inboundSvc := &InboundService{} + svc := &ClientService{} + + ib, recId := seedForwardedPortsClient(t, "fp@x", "8022") + if got := inboundForwardedPorts(t, inboundSvc, ib.Id, "fp@x"); got != "8022" { + t.Fatalf("seeded forwardedPorts = %q, want 8022", got) + } + + updated := model.Client{ + Email: "fp@x", + Enable: true, + AllowedIPs: []string{"10.8.1.2/32"}, + ForwardedPorts: model.ForwardedPortsPtr(""), + } + if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil { + t.Fatalf("Update: %v", err) + } + + if got := inboundForwardedPorts(t, inboundSvc, ib.Id, "fp@x"); got != "" { + t.Fatalf("inbound forwardedPorts after an explicit \"\" = %q, want none", got) + } + stored, err := inboundSvc.GetInbound(ib.Id) + if err != nil { + t.Fatalf("GetInbound: %v", err) + } + if strings.Contains(stored.Settings, "forwardedPorts") { + t.Fatalf("withdrawn forwards must leave no key behind: %s", stored.Settings) + } + if got := lookupClientRecord(t, "fp@x").ForwardedPorts; got != "" { + t.Fatalf("stored wg_forwarded_ports after an explicit \"\" = %q, want none", got) + } +} + +// The other half of the contract: a payload that never mentions forwardedPorts +// (a metadata-only edit from the bot or the API) leaves the stored value alone. +func TestUpdateWithoutForwardedPortsPreservesTheStoredValue(t *testing.T) { + setupBulkDB(t) + inboundSvc := &InboundService{} + svc := &ClientService{} + + ib, recId := seedForwardedPortsClient(t, "fp@x", "8022") + + updated := model.Client{ + Email: "fp@x", + Enable: true, + AllowedIPs: []string{"10.8.1.2/32"}, + } + if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil { + t.Fatalf("Update: %v", err) + } + + if got := inboundForwardedPorts(t, inboundSvc, ib.Id, "fp@x"); got != "8022" { + t.Fatalf("inbound forwardedPorts after an edit that omitted it = %q, want 8022", got) + } + if got := lookupClientRecord(t, "fp@x").ForwardedPorts; got != "8022" { + t.Fatalf("stored wg_forwarded_ports after an edit that omitted it = %q, want 8022", got) + } +} + +// The form is filled from the record, and the traffic resets re-save it: a record +// left at the creation-time spec moves the forward back on the next save. +func TestUpdateKeepsTheRecordsForwardedPortsCurrent(t *testing.T) { + setupBulkDB(t) + inboundSvc := &InboundService{} + svc := &ClientService{} + + ib, recId := seedForwardedPortsClient(t, "fp@x", "8021") + + updated := model.Client{ + Email: "fp@x", + Enable: true, + AllowedIPs: []string{"10.8.1.2/32"}, + ForwardedPorts: model.ForwardedPortsPtr("8022"), + } + if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil { + t.Fatalf("Update: %v", err) + } + rec := lookupClientRecord(t, "fp@x") + if rec.ForwardedPorts != "8022" { + t.Fatalf("stored wg_forwarded_ports after 8021 -> 8022 = %q, want 8022", rec.ForwardedPorts) + } + + if _, err := svc.Update(inboundSvc, recId, *rec.ToClient(), 0); err != nil { + t.Fatalf("re-save of the record: %v", err) + } + if got := inboundForwardedPorts(t, inboundSvc, ib.Id, "fp@x"); got != "8022" { + t.Fatalf("inbound forwardedPorts after a re-save of the record = %q, want 8022", got) + } +} diff --git a/internal/web/service/kernel_client_guard_test.go b/internal/web/service/kernel_client_guard_test.go index 7e6d15547..f6c2a5e2b 100644 --- a/internal/web/service/kernel_client_guard_test.go +++ b/internal/web/service/kernel_client_guard_test.go @@ -23,7 +23,7 @@ func TestKernelAWGClientWritesRejectUnsafeValuesAndUnsupportedFeatures(t *testin client model.Client }{ {"public key hook injection", "control characters", model.Client{Email: "bad-key@awg", Enable: true, PublicKey: "peer-key\n[Interface]\nPostDown = touch /tmp/pwned", AllowedIPs: []string{"10.8.1.2/32"}}}, - {"forwarded ports", "port forwarding is not supported", model.Client{Email: "bad-forward@awg", Enable: true, PublicKey: "peer-key", AllowedIPs: []string{"10.8.1.2/32"}, ForwardedPorts: "54321"}}, + {"forwarded ports", "port forwarding is not supported", model.Client{Email: "bad-forward@awg", Enable: true, PublicKey: "peer-key", AllowedIPs: []string{"10.8.1.2/32"}, ForwardedPorts: model.ForwardedPortsPtr("54321")}}, {"IPv6 allowed IP", "per-peer IPv6 egress is not supported", model.Client{Email: "bad-v6@awg", Enable: true, PublicKey: "peer-key", AllowedIPs: []string{"2001:db8::2/128"}}}, } for _, tc := range cases { @@ -70,7 +70,7 @@ func TestKernelAWGClientUpdateRejectsUnsafeValuesAndUnsupportedFeatures(t *testi update model.Client }{ {"preshared key hook injection", "control characters", model.Client{Email: "kept@awg", PreSharedKey: "psk\n[Interface]\nPreDown = touch /tmp/pwned"}}, - {"forwarded ports", "port forwarding is not supported", model.Client{Email: "kept@awg", ForwardedPorts: "54321"}}, + {"forwarded ports", "port forwarding is not supported", model.Client{Email: "kept@awg", ForwardedPorts: model.ForwardedPortsPtr("54321")}}, {"IPv6 allowed IP", "per-peer IPv6 egress is not supported", model.Client{Email: "kept@awg", AllowedIPs: []string{"2001:db8::2/128"}}}, } for _, tc := range cases { @@ -123,7 +123,7 @@ func TestKernelAWGClientFeatureChecksPreserveSupportedPaths(t *testing.T) { t.Fatalf("valid IPv4 kernel peer was rejected: %v", err) } embedded := mkInbound(t, 52003, model.AmneziaWG, amneziawgClientTestSettings) - if _, err := svc.CreateOne(inboundSvc, embedded.Id, model.Client{Email: "forward@embedded", Enable: true, PublicKey: "valid-key", AllowedIPs: []string{"10.8.1.3/32"}, ForwardedPorts: "54321"}); err != nil { + if _, err := svc.CreateOne(inboundSvc, embedded.Id, model.Client{Email: "forward@embedded", Enable: true, PublicKey: "valid-key", AllowedIPs: []string{"10.8.1.3/32"}, ForwardedPorts: model.ForwardedPortsPtr("54321")}); err != nil { t.Fatalf("embedded AWG port forwarding was rejected: %v", err) } } @@ -136,7 +136,7 @@ func TestKernelAWGSharedCreateAndUpdateValidateAllTargetsBeforeWrites(t *testing kernel := mkInbound(t, 52005, model.AmneziaWG, kernelAWGSettings(nil)) embedded := mkInbound(t, 52006, model.AmneziaWG, amneziawgClientTestSettings) _, err := svc.Create(inboundSvc, &ClientCreatePayload{ - Client: model.Client{Email: "shared-create@awg", Enable: true, PublicKey: "valid-key", AllowedIPs: []string{"10.8.1.2/32"}, ForwardedPorts: "54321"}, + Client: model.Client{Email: "shared-create@awg", Enable: true, PublicKey: "valid-key", AllowedIPs: []string{"10.8.1.2/32"}, ForwardedPorts: model.ForwardedPortsPtr("54321")}, InboundIds: []int{embedded.Id, kernel.Id}, }) if err == nil || !strings.Contains(err.Error(), "port forwarding is not supported") { @@ -173,7 +173,7 @@ func TestKernelAWGSharedCreateAndUpdateValidateAllTargetsBeforeWrites(t *testing beforeKernel, _ := inboundSvc.GetInbound(kernel.Id) beforeEmbedded, _ := inboundSvc.GetInbound(embedded.Id) rec := lookupClientRecord(t, seed.Email) - _, err := svc.Update(inboundSvc, rec.Id, model.Client{Email: seed.Email, ForwardedPorts: "54321"}, 0) + _, err := svc.Update(inboundSvc, rec.Id, model.Client{Email: seed.Email, ForwardedPorts: model.ForwardedPortsPtr("54321")}, 0) if err == nil || !strings.Contains(err.Error(), "port forwarding is not supported") { t.Fatalf("Update error = %v, want kernel feature rejection", err) }