Selaa lähdekoodia

fix(mtproto): stop persisting a vestigial inbound-level secret

MTProto is multi-client: mtg's [secrets] config and every share link read only the per-client secrets. The old HealMtprotoSecret regenerated an inbound-level secret on every save, and seedMtprotoSecretsToClients only dropped it for legacy single-secret inbounds, so multi-client inbounds kept a dead secret. That value once leaked into stale links imported into Telegram, which mtg then rejected as "incorrect client random".

Replace HealMtprotoSecret with StripMtprotoInboundSecret (removes the key), strip on save in normalizeMtprotoSecret, and add a one-time stripMtprotoInboundSecrets migration that runs after the seeder so a legacy secret is first preserved onto a client before the inbound-level copy is dropped.
MHSanaei 14 tuntia sitten
vanhempi
sitoutus
84b6423020

+ 44 - 0
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()

+ 11 - 17
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

+ 22 - 32
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 _, changed2 := HealMtprotoSecret(out); changed2 {
-		t.Fatal("expected no change for an already-valid secret")
+	if parsed["fakeTlsDomain"] != "a.com" {
+		t.Fatalf("fakeTlsDomain must survive, got %q", out)
 	}
-
-	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)
+	clients, ok := parsed["clients"].([]any)
+	if !ok || len(clients) != 1 {
+		t.Fatalf("clients must survive untouched, got %q", out)
 	}
-	if !strings.HasSuffix(got3, hex.EncodeToString([]byte(newDomain))) {
-		t.Fatalf("suffix not updated for new domain: %q", got3)
+	if clients[0].(map[string]any)["secret"] != "eeaaaa" {
+		t.Fatalf("client secret must survive untouched, got %q", out)
 	}
 
-	if _, changed4 := HealMtprotoSecret(`{"secret":"ee"}`); changed4 {
-		t.Fatal("expected no change when fakeTlsDomain is missing")
+	// 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")
+	}
+	if _, changed3 := StripMtprotoInboundSecret(`{"clients":[]}`); changed3 {
+		t.Fatal("expected no change for settings without a secret key")
 	}
 }
 

+ 138 - 0
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)
+	}
+}

+ 6 - 4
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

+ 4 - 1
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"))) {