diff --git a/internal/database/db.go b/internal/database/db.go index 02cf5c519..25a5450e8 100644 --- a/internal/database/db.go +++ b/internal/database/db.go @@ -477,6 +477,42 @@ func seedMtprotoSecretsToClients() error { }) } +// stripMtprotoInboundSecrets removes the vestigial inbound-level `secret` from +// every mtproto inbound. seedMtprotoSecretsToClients already drops it while +// converting legacy single-secret inbounds, but inbounds that already had clients +// kept the dead field, and the old HealMtprotoSecret regenerated it on every +// save. mtg and every share link read only per-client secrets, so the +// inbound-level value is dead data that once leaked into stale, unusable links. +// One-time, self-gated on the "StripMtprotoInboundSecrets" seeder row. +func stripMtprotoInboundSecrets() error { + var history []string + if err := db.Model(&model.HistoryOfSeeders{}).Pluck("seeder_name", &history).Error; err != nil { + return err + } + if slices.Contains(history, "StripMtprotoInboundSecrets") { + return nil + } + + var inbounds []model.Inbound + if err := db.Where("protocol = ?", string(model.MTProto)).Find(&inbounds).Error; err != nil { + return err + } + + return db.Transaction(func(tx *gorm.DB) error { + for _, inbound := range inbounds { + stripped, ok := model.StripMtprotoInboundSecret(inbound.Settings) + if !ok { + continue + } + if err := tx.Model(&model.Inbound{}).Where("id = ?", inbound.Id). + Update("settings", stripped).Error; err != nil { + return err + } + } + return tx.Create(&model.HistoryOfSeeders{SeederName: "StripMtprotoInboundSecrets"}).Error + }) +} + // mtprotoInboundClientEmail derives a stable, unique client email for a migrated // mtproto inbound from its remark. func mtprotoInboundClientEmail(remark string, used map[string]struct{}) string { @@ -925,6 +961,14 @@ func runSeeders(isUsersEmpty bool) error { return err } + // Self-gated on the "StripMtprotoInboundSecrets" row. Must run after the + // seeder above so legacy single-secret inbounds are first converted to a + // client (which preserves the secret) before the inbound-level copy is + // dropped from every mtproto inbound. + if err := stripMtprotoInboundSecrets(); err != nil { + return err + } + // Idempotent, not seeder-gated: bad values can re-enter via a restored // backup, so re-check on every start. return normalizeSettingPaths() diff --git a/internal/database/model/model.go b/internal/database/model/model.go index 3e9bcaeaa..d063413e5 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -425,7 +425,8 @@ func HealShadowsocksClientMethods(settings string) (string, bool) { // GenerateFakeTLSSecret builds an MTProto FakeTLS secret for the given domain: // the "ee" FakeTLS marker, 16 random bytes, then the domain encoded as hex. -// This single value is what mtg's config and the client tg:// link both use. +// MTProto is multi-client, so this value belongs to one client: mtg's [secrets] +// config and that client's tg:// link both read it per client. func GenerateFakeTLSSecret(domain string) string { return "ee" + mtprotoRandomMiddle() + hex.EncodeToString([]byte(domain)) } @@ -455,13 +456,13 @@ func mtprotoSecretMiddle(secret string) string { return mtprotoRandomMiddle() } -// HealMtprotoSecret normalises an mtproto inbound's settings JSON before the -// value leaves for the mtg sidecar or a share link: it rebuilds `secret` so it -// is always a valid FakeTLS secret whose trailing domain matches -// `fakeTlsDomain`, generating the random middle when one is missing and -// rewriting the domain suffix when the domain changed. Returns the rewritten -// settings and true when anything changed. -func HealMtprotoSecret(settings string) (string, bool) { +// StripMtprotoInboundSecret removes the vestigial inbound-level `secret` from an +// mtproto inbound's settings JSON. MTProto is multi-client: every secret lives on +// a client, and mtg's [secrets] config plus every share link read only the +// per-client secrets. A lingering inbound-level secret is dead data — it once +// leaked into stale links that mtg rejected as "incorrect client random". Returns +// the rewritten settings and true when a `secret` key was removed. +func StripMtprotoInboundSecret(settings string) (string, bool) { if settings == "" { return settings, false } @@ -469,17 +470,10 @@ func HealMtprotoSecret(settings string) (string, bool) { if err := json.Unmarshal([]byte(settings), &parsed); err != nil { return settings, false } - domain, _ := parsed["fakeTlsDomain"].(string) - domain = strings.TrimSpace(domain) - if domain == "" { + if _, ok := parsed["secret"]; !ok { return settings, false } - secret, _ := parsed["secret"].(string) - expected := "ee" + mtprotoSecretMiddle(secret) + hex.EncodeToString([]byte(domain)) - if secret == expected { - return settings, false - } - parsed["secret"] = expected + delete(parsed, "secret") out, err := json.MarshalIndent(parsed, "", " ") if err != nil { return settings, false diff --git a/internal/database/model/model_mtproto_test.go b/internal/database/model/model_mtproto_test.go index fcb953620..797235abc 100644 --- a/internal/database/model/model_mtproto_test.go +++ b/internal/database/model/model_mtproto_test.go @@ -25,48 +25,38 @@ func TestGenerateFakeTLSSecret(t *testing.T) { } } -func TestHealMtprotoSecret(t *testing.T) { - domain := "example.com" - suffix := hex.EncodeToString([]byte(domain)) - - in := `{"fakeTlsDomain":"example.com","secret":""}` - out, changed := HealMtprotoSecret(in) +func TestStripMtprotoInboundSecret(t *testing.T) { + // A multi-client inbound that still carries a dead inbound-level secret has + // it removed while the clients (and every other key) survive untouched. + in := `{"fakeTlsDomain":"a.com","secret":"eedeadbeef","clients":[{"email":"x","secret":"eeaaaa"}]}` + out, changed := StripMtprotoInboundSecret(in) if !changed { - t.Fatal("expected heal to populate an empty secret") + t.Fatal("expected the inbound-level secret to be stripped") } var parsed map[string]any if err := json.Unmarshal([]byte(out), &parsed); err != nil { - t.Fatalf("healed settings not valid json: %v", err) + t.Fatalf("stripped settings not valid json: %v", err) } - got, _ := parsed["secret"].(string) - if !strings.HasPrefix(got, "ee") || !strings.HasSuffix(got, suffix) { - t.Fatalf("healed secret malformed: %q", got) + if _, ok := parsed["secret"]; ok { + t.Fatalf("inbound-level secret should be gone, got %q", out) + } + if parsed["fakeTlsDomain"] != "a.com" { + t.Fatalf("fakeTlsDomain must survive, got %q", out) + } + clients, ok := parsed["clients"].([]any) + if !ok || len(clients) != 1 { + t.Fatalf("clients must survive untouched, got %q", out) + } + if clients[0].(map[string]any)["secret"] != "eeaaaa" { + t.Fatalf("client secret must survive untouched, got %q", out) } - if _, changed2 := HealMtprotoSecret(out); changed2 { - t.Fatal("expected no change for an already-valid secret") + // Nothing to strip when there is no inbound-level secret. + if _, changed2 := StripMtprotoInboundSecret(out); changed2 { + t.Fatal("expected no change when there is no inbound-level secret") } - - mid := got[2:34] - newDomain := "telegram.org" - in3 := `{"fakeTlsDomain":"telegram.org","secret":"` + got + `"}` - out3, changed3 := HealMtprotoSecret(in3) - if !changed3 { - t.Fatal("expected heal to rewrite the domain suffix") - } - if err := json.Unmarshal([]byte(out3), &parsed); err != nil { - t.Fatalf("healed settings not valid json: %v", err) - } - got3, _ := parsed["secret"].(string) - if got3[2:34] != mid { - t.Fatalf("random middle should be preserved on domain change: %q vs %q", got3[2:34], mid) - } - if !strings.HasSuffix(got3, hex.EncodeToString([]byte(newDomain))) { - t.Fatalf("suffix not updated for new domain: %q", got3) - } - - if _, changed4 := HealMtprotoSecret(`{"secret":"ee"}`); changed4 { - t.Fatal("expected no change when fakeTlsDomain is missing") + if _, changed3 := StripMtprotoInboundSecret(`{"clients":[]}`); changed3 { + t.Fatal("expected no change for settings without a secret key") } } diff --git a/internal/database/mtproto_migration_test.go b/internal/database/mtproto_migration_test.go new file mode 100644 index 000000000..5faabc909 --- /dev/null +++ b/internal/database/mtproto_migration_test.go @@ -0,0 +1,138 @@ +package database + +import ( + "encoding/json" + "path/filepath" + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +func initMtprotoMigrationDB(t *testing.T) { + t.Helper() + dbDir := t.TempDir() + t.Setenv("XUI_DB_FOLDER", dbDir) + if err := InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil { + t.Fatalf("InitDB failed: %v", err) + } + t.Cleanup(func() { _ = CloseDB() }) +} + +func createMtprotoInbound(t *testing.T, remark string, port int, settings map[string]any) *model.Inbound { + t.Helper() + raw, err := json.Marshal(settings) + if err != nil { + t.Fatalf("marshal settings: %v", err) + } + in := &model.Inbound{ + UserId: 1, + Remark: remark, + Port: port, + Protocol: model.MTProto, + Settings: string(raw), + Tag: remark, + } + if err := db.Create(in).Error; err != nil { + t.Fatalf("create mtproto inbound: %v", err) + } + return in +} + +func clearSeederHistory(t *testing.T, name string) { + t.Helper() + if err := db.Where("seeder_name = ?", name).Delete(&model.HistoryOfSeeders{}).Error; err != nil { + t.Fatalf("clear %s history: %v", name, err) + } +} + +func TestStripMtprotoInboundSecretsRemovesDeadSecret(t *testing.T) { + initMtprotoMigrationDB(t) + in := createMtprotoInbound(t, "mt-mc", 8443, map[string]any{ + "fakeTlsDomain": "www.cloudflare.com", + "secret": "eedeadbeef7777772e636c6f7564666c6172652e636f6d", + "clients": []any{ + map[string]any{"email": "alice", "secret": "eeaaaa7777772e636c6f7564666c6172652e636f6d", "enable": true, "subId": "s1"}, + }, + }) + clearSeederHistory(t, "StripMtprotoInboundSecrets") + + if err := stripMtprotoInboundSecrets(); err != nil { + t.Fatalf("stripMtprotoInboundSecrets: %v", err) + } + + settings := reloadInboundSettings(t, in.Id) + if _, ok := settings["secret"]; ok { + t.Fatalf("inbound-level secret must be removed, got %v", settings) + } + clients, ok := settings["clients"].([]any) + if !ok || len(clients) != 1 { + t.Fatalf("clients must be preserved, got %v", settings["clients"]) + } + if clients[0].(map[string]any)["secret"] != "eeaaaa7777772e636c6f7564666c6172652e636f6d" { + t.Fatalf("client secret must be preserved, got %v", clients[0]) + } + if settings["fakeTlsDomain"] != "www.cloudflare.com" { + t.Fatalf("fakeTlsDomain must be preserved, got %v", settings) + } +} + +func TestStripMtprotoInboundSecretsIsGated(t *testing.T) { + initMtprotoMigrationDB(t) + in := createMtprotoInbound(t, "mt-gate", 8444, map[string]any{ + "fakeTlsDomain": "a.com", + "secret": "eebeef61", + "clients": []any{map[string]any{"email": "x", "secret": "eeaa61", "enable": true}}, + }) + + if err := stripMtprotoInboundSecrets(); err != nil { + t.Fatalf("first run: %v", err) + } + if _, ok := reloadInboundSettings(t, in.Id)["secret"]; ok { + t.Fatal("secret should be stripped on the first run") + } + + var raw model.Inbound + if err := db.First(&raw, in.Id).Error; err != nil { + t.Fatalf("reload: %v", err) + } + raw.Settings = `{"fakeTlsDomain":"a.com","secret":"eereintroduced","clients":[]}` + if err := db.Save(&raw).Error; err != nil { + t.Fatalf("reintroduce secret: %v", err) + } + if err := stripMtprotoInboundSecrets(); err != nil { + t.Fatalf("second run (history gate): %v", err) + } + if _, ok := reloadInboundSettings(t, in.Id)["secret"]; !ok { + t.Fatal("second run must be a no-op: the seeder is one-time, so a re-added secret is left alone") + } +} + +func TestSeedThenStripPreservesLegacySecretOnClient(t *testing.T) { + initMtprotoMigrationDB(t) + const legacy = "eedeadbeefdeadbeefdeadbeefdeadbe7777772e636c6f7564666c6172652e636f6d" + in := createMtprotoInbound(t, "mt-legacy", 8445, map[string]any{ + "fakeTlsDomain": "www.cloudflare.com", + "secret": legacy, + }) + clearSeederHistory(t, "MtprotoSecretsToClients") + clearSeederHistory(t, "StripMtprotoInboundSecrets") + + if err := seedMtprotoSecretsToClients(); err != nil { + t.Fatalf("seedMtprotoSecretsToClients: %v", err) + } + if err := stripMtprotoInboundSecrets(); err != nil { + t.Fatalf("stripMtprotoInboundSecrets: %v", err) + } + + settings := reloadInboundSettings(t, in.Id) + if _, ok := settings["secret"]; ok { + t.Fatalf("inbound-level secret must be gone after seed+strip, got %v", settings) + } + clients, ok := settings["clients"].([]any) + if !ok || len(clients) != 1 { + t.Fatalf("seed should have created exactly one client, got %v", settings["clients"]) + } + if got := clients[0].(map[string]any)["secret"]; got != legacy { + t.Fatalf("the legacy secret must survive on the seeded client, got %v want %s", got, legacy) + } +} diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 69d6d8f55..248df1c81 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -531,14 +531,16 @@ func (s *InboundService) normalizeStreamSettings(inbound *model.Inbound) { } // normalizeMtprotoSecret rebuilds every mtproto client's FakeTLS secret so it is -// always valid before the row is persisted. It also heals a legacy inbound-level -// secret for any inbound that predates the multi-client migration. +// always valid before the row is persisted, and drops the vestigial inbound-level +// secret: MTProto is multi-client, so mtg and every share link read only the +// per-client secrets. Leaving an inbound-level secret behind is what produced +// stale links that failed with "incorrect client random". func (s *InboundService) normalizeMtprotoSecret(inbound *model.Inbound) { if inbound.Protocol != model.MTProto { return } - if healed, ok := model.HealMtprotoSecret(inbound.Settings); ok { - inbound.Settings = healed + if stripped, ok := model.StripMtprotoInboundSecret(inbound.Settings); ok { + inbound.Settings = stripped } if healed, ok := model.HealMtprotoClientSecrets(inbound.Settings); ok { inbound.Settings = healed diff --git a/internal/web/service/inbound_mtproto_test.go b/internal/web/service/inbound_mtproto_test.go index e5b05a11d..cd657774e 100644 --- a/internal/web/service/inbound_mtproto_test.go +++ b/internal/web/service/inbound_mtproto_test.go @@ -128,13 +128,16 @@ func TestFillProtocolDefaultsMtproto(t *testing.T) { func TestNormalizeMtprotoSecretHealsClients(t *testing.T) { s := &InboundService{} - ib := &model.Inbound{Protocol: model.MTProto, Settings: `{"fakeTlsDomain":"a.com","clients":[{"email":"x","secret":""}]}`} + ib := &model.Inbound{Protocol: model.MTProto, Settings: `{"fakeTlsDomain":"a.com","secret":"eedeadbeef","clients":[{"email":"x","secret":""}]}`} s.normalizeMtprotoSecret(ib) var parsed map[string]any if err := json.Unmarshal([]byte(ib.Settings), &parsed); err != nil { t.Fatalf("healed settings not valid json: %v", err) } + if _, ok := parsed["secret"]; ok { + t.Fatalf("the vestigial inbound-level secret must be stripped, got %q", ib.Settings) + } clients := parsed["clients"].([]any) got := clients[0].(map[string]any)["secret"].(string) if !strings.HasPrefix(got, "ee") || !strings.HasSuffix(got, hex.EncodeToString([]byte("a.com"))) {