diff --git a/internal/database/db.go b/internal/database/db.go index 66d29d4e5..0e92b626a 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"} + 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"} for _, name := range seeders { if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil { return err @@ -1719,6 +1719,12 @@ func runSeeders(isUsersEmpty bool) error { } } + if !slices.Contains(seedersHistory, "OwnerlessClientHwidPurge") { + if err := purgeOwnerlessClientHwids(); err != nil { + return err + } + } + if err := seedHostsFromExternalProxy(); err != nil { return err } @@ -1914,6 +1920,18 @@ func clearLegacyProxySettings() 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 { + return db.Transaction(func(tx *gorm.DB) error { + if err := tx.Where("sub_id NOT IN (SELECT sub_id FROM clients WHERE sub_id IS NOT NULL)"). + Delete(&model.ClientHwid{}).Error; err != nil { + return err + } + return tx.Create(&model.HistoryOfSeeders{SeederName: "OwnerlessClientHwidPurge"}).Error + }) +} + // The groups page addresses a group by its trimmed name, so a padded stored name listed as // its own group while every action on it hit the trimmed one. func trimClientGroupNames() error { diff --git a/internal/database/db_seed_test.go b/internal/database/db_seed_test.go index 3f928879c..8ce6d2c16 100644 --- a/internal/database/db_seed_test.go +++ b/internal/database/db_seed_test.go @@ -2,6 +2,7 @@ package database import ( "encoding/json" + "fmt" "path/filepath" "regexp" "testing" @@ -376,3 +377,37 @@ func TestClientGroupNamesTrimmedOnUpgrade(t *testing.T) { t.Fatalf("groups after upgrade = %q, want [vip]", groups) } } + +// Requests naming a subId no client owns stored HWID rows that nothing ever reads; the +// upgrade drops them and keeps the rows of real subscriptions. +func TestOwnerlessClientHwidsPurgedOnUpgrade(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: "a@x", SubID: "owned", Enable: true}).Error; err != nil { + t.Fatalf("seed client: %v", err) + } + for i, sub := range []string{"owned", "junk-1", "junk-2"} { + if err := db.Create(&model.ClientHwid{SubID: sub, HwidHash: fmt.Sprintf("h%d", i), FirstSeen: 1, LastSeen: 1}).Error; err != nil { + t.Fatalf("seed hwid %s: %v", sub, err) + } + } + if err := db.Where("seeder_name = ?", "OwnerlessClientHwidPurge").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) + } + var subs []string + if err := db.Model(&model.ClientHwid{}).Pluck("sub_id", &subs).Error; err != nil { + t.Fatalf("read hwids: %v", err) + } + if len(subs) != 1 || subs[0] != "owned" { + t.Fatalf("hwid rows after upgrade = %q, want [owned]", subs) + } +} diff --git a/internal/sub/hwid_controller_test.go b/internal/sub/hwid_controller_test.go index 76ab4bafd..934b347b7 100644 --- a/internal/sub/hwid_controller_test.go +++ b/internal/sub/hwid_controller_test.go @@ -218,6 +218,27 @@ func TestSubscriptionHwidStatusDoesNotRegisterDevice(t *testing.T) { } } +// A 404 for a subId nobody was issued must not persist anything: the gate runs +// before the lookup, and an attacker can send any number of distinct subIds. +func TestSubscriptionHwidGateIgnoresUnknownSubID(t *testing.T) { + router, _ := initHwidSubRouter(t, 0) + + for _, path := range []string{"/sub/never-issued", "/json/never-issued", "/clash/never-issued"} { + rec := requestSub(t, router, http.MethodGet, path, "device-ghost", "") + if rec.Code != http.StatusNotFound { + t.Fatalf("%s unknown subId status = %d, want 404", path, rec.Code) + } + } + + var count int64 + if err := database.GetDB().Model(&model.ClientHwid{}).Where("sub_id = ?", "never-issued").Count(&count).Error; err != nil { + t.Fatalf("count hwids: %v", err) + } + if count != 0 { + t.Fatalf("client_hwids rows for an unknown subId = %d, want 0", count) + } +} + func TestSubscriptionHwidStatusWithoutLimit(t *testing.T) { router, subID := initHwidSubRouter(t, 0) diff --git a/internal/web/service/client_hwid.go b/internal/web/service/client_hwid.go index 606ea6ac3..6d7b01b97 100644 --- a/internal/web/service/client_hwid.go +++ b/internal/web/service/client_hwid.go @@ -101,6 +101,16 @@ func effectiveHwidLimitForSubID(tx *gorm.DB, subID string) (int, error) { return limit, err } +// subIDHasEnabledClient separates "no device limit" from "no such subscription": +// effectiveHwidLimitForSubID yields 0 for both. +func subIDHasEnabledClient(tx *gorm.DB, subID string) (bool, error) { + var enabled int64 + err := tx.Model(&model.ClientRecord{}). + Where("sub_id = ? AND enable = ?", subID, true). + Count(&enabled).Error + return enabled > 0, err +} + func (s *ClientService) EnforceHwidForSubID(subID string, req HwidRequest) (HwidGateResult, error) { var res HwidGateResult subID = strings.TrimSpace(subID) @@ -117,7 +127,15 @@ func (s *ClientService) EnforceHwidForSubID(subID string, req HwidRequest) (Hwid req = normalizeHwidRequest(req) if limit <= 0 { res.Allowed = true - if len(req.Hwid) >= minHwidLength { + if len(req.Hwid) < minHwidLength { + return res, nil + } + // The gate runs before the sub lookup; a subId no client owns gets no row. + known, err := subIDHasEnabledClient(db, subID) + if err != nil { + return res, err + } + if known { trackUnlimitedHwid(db, subID, req) } return res, nil @@ -206,15 +224,10 @@ func (s *ClientService) HwidSlotStatusForSubID(subID string) (status HwidSlotSta } db := database.GetDB() - var enabled int64 - if err := db.Model(&model.ClientRecord{}). - Where("sub_id = ? AND enable = ?", subID, true). - Count(&enabled).Error; err != nil { + known, err := subIDHasEnabledClient(db, subID) + if err != nil || !known { return status, false, err } - if enabled == 0 { - return status, false, nil - } limit, err := effectiveHwidLimitForSubID(db, subID) if err != nil { diff --git a/internal/web/service/client_hwid_test.go b/internal/web/service/client_hwid_test.go index 2d742d5e8..2d960322a 100644 --- a/internal/web/service/client_hwid_test.go +++ b/internal/web/service/client_hwid_test.go @@ -62,6 +62,54 @@ func TestClientHwidGate(t *testing.T) { } } +// The gate runs before the subscription lookup, so a subId no enabled client +// owns must pass through without leaving a client_hwids row behind. +func TestClientHwidGateDoesNotTrackUnknownSubID(t *testing.T) { + initClientHwidTestDB(t) + svc := &ClientService{} + db := database.GetDB() + rec := seedHwidClient(t, 0) + + countRows := func(subID string) int64 { + t.Helper() + var rows int64 + if err := db.Model(&model.ClientHwid{}).Where("sub_id = ?", subID).Count(&rows).Error; err != nil { + t.Fatalf("count rows for %q: %v", subID, err) + } + return rows + } + + res, err := svc.EnforceHwidForSubID("no-such-sub", HwidRequest{Hwid: "device-ghost", UserAgent: "Happ/1.0"}) + if err != nil || res != (HwidGateResult{Allowed: true}) { + t.Fatalf("unknown subId gate: err=%v res=%+v", err, res) + } + if rows := countRows("no-such-sub"); rows != 0 { + t.Fatalf("client_hwids rows for an unknown subId = %d, want 0", rows) + } + + if err := db.Model(&model.ClientRecord{}).Where("id = ?", rec.Id).UpdateColumn("enable", false).Error; err != nil { + t.Fatalf("disable client: %v", err) + } + res, err = svc.EnforceHwidForSubID(rec.SubID, HwidRequest{Hwid: "device-ghost"}) + if err != nil || res != (HwidGateResult{Allowed: true}) { + t.Fatalf("disabled subId gate: err=%v res=%+v", err, res) + } + if rows := countRows(rec.SubID); rows != 0 { + t.Fatalf("client_hwids rows for a disabled subId = %d, want 0", rows) + } + + if err := db.Model(&model.ClientRecord{}).Where("id = ?", rec.Id).UpdateColumn("enable", true).Error; err != nil { + t.Fatalf("enable client: %v", err) + } + res, err = svc.EnforceHwidForSubID(rec.SubID, HwidRequest{Hwid: "device-ghost"}) + if err != nil || res != (HwidGateResult{Allowed: true}) { + t.Fatalf("known subId gate: err=%v res=%+v", err, res) + } + if rows := countRows(rec.SubID); rows != 1 { + t.Fatalf("client_hwids rows for a known unlimited subId = %d, want 1", rows) + } +} + func TestClientHwidGateRegistersAndBlocks(t *testing.T) { initClientHwidTestDB(t) svc := &ClientService{}