Parcourir la source

fix(xray): read an outbound protocol id the way the core does (#6521)

* fix(xray): read an outbound protocol id the way the core does

xray-core lowercases a protocol id before it looks up the handler
(infra/conf/loader.go: `id = strings.ToLower(id)`), so a template that
spells the direct outbound "Freedom" runs as freedom. The three config
rewriters compared the id case-sensitively, so such an outbound was
skipped while the seed was recorded as applied: the refused
sockopt.addressPortStrategy stayed and xray-core refused to start.

* fix(xray): match an outbound protocol id case-insensitively

xray-core lowercases a protocol id before looking up its handler, so a
template that spells the direct outbound "Freedom" runs as freedom while
this rewriter skipped it and left the deprecated placement in place.

* fix(xray): re-run the freedom finalRules rewrite where its seeder was gated

Both finalRules seeders recorded their rows before the predicate could see
an outbound spelled "Freedom", and a recorded row is never re-run, so the
corrected predicates alone left the #6037 private-egress hardening
unapplied on every panel that had already run them. The new one-shot
seeder replays both rewrites, and only when the config actually carries a
differently spelled freedom outbound, so stock lowercase configs stay
byte-identical.
BlindMaster24 il y a 1 jour
Parent
commit
4d6db1c961

+ 82 - 5
internal/database/db.go

@@ -1254,7 +1254,7 @@ func runSeeders(isUsersEmpty bool) error {
 	}
 
 	if empty && isUsersEmpty {
-		seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "InboundRealityFinalmaskTcpStrip", "ApiTokensHash", "LegacyProxySettingsCleanup", "OutboundRemovedKeysFix", "FreedomDomainStrategyFix", "DNSOutboundLegacyKeysFix", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "ResetIpLimitNoFail2ban"}
+		seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "UppercaseFreedomFinalRulesFix", "InboundRealityFinalmaskTcpStrip", "ApiTokensHash", "LegacyProxySettingsCleanup", "OutboundRemovedKeysFix", "FreedomDomainStrategyFix", "DNSOutboundLegacyKeysFix", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "ResetIpLimitNoFail2ban"}
 		for _, name := range seeders {
 			if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil {
 				return err
@@ -1347,6 +1347,12 @@ func runSeeders(isUsersEmpty bool) error {
 		}
 	}
 
+	if !slices.Contains(seedersHistory, "UppercaseFreedomFinalRulesFix") {
+		if err := fixUppercaseFreedomFinalRules(); err != nil {
+			return err
+		}
+	}
+
 	if !slices.Contains(seedersHistory, "InboundRealityFinalmaskTcpStrip") {
 		if err := stripRealityFinalmaskTcp(); err != nil {
 			return err
@@ -1625,7 +1631,7 @@ func rewriteRemovedOutboundKeys(raw string) (string, bool, error) {
 			delete(obj, "proxySettings")
 			changed = true
 		}
-		if proto, _ := obj["protocol"].(string); proto == "freedom" {
+		if proto, _ := obj["protocol"].(string); strings.EqualFold(proto, "freedom") {
 			if sockopt := outboundSockopt(obj, false); sockopt != nil {
 				if _, present := sockopt["addressPortStrategy"]; present {
 					delete(sockopt, "addressPortStrategy")
@@ -1711,7 +1717,7 @@ func rewriteFreedomDomainStrategy(raw string) (string, bool, error) {
 		if !ok {
 			continue
 		}
-		if proto, _ := obj["protocol"].(string); proto != "freedom" {
+		if proto, _ := obj["protocol"].(string); !strings.EqualFold(proto, "freedom") {
 			continue
 		}
 		settings, hasSettings := obj["settings"].(map[string]any)
@@ -2144,7 +2150,7 @@ func rewriteFreedomFinalRules(raw string) (string, bool, error) {
 		if !ok {
 			continue
 		}
-		if proto, _ := obj["protocol"].(string); proto != "freedom" {
+		if proto, _ := obj["protocol"].(string); !strings.EqualFold(proto, "freedom") {
 			continue
 		}
 		settings, ok := obj["settings"].(map[string]any)
@@ -2247,7 +2253,7 @@ func rewriteFreedomFinalRulesPrivateEgress(raw string) (string, bool, error) {
 		if !ok {
 			continue
 		}
-		if proto, _ := obj["protocol"].(string); proto != "freedom" {
+		if proto, _ := obj["protocol"].(string); !strings.EqualFold(proto, "freedom") {
 			continue
 		}
 		settings, ok := obj["settings"].(map[string]any)
@@ -2276,6 +2282,77 @@ func rewriteFreedomFinalRulesPrivateEgress(raw string) (string, bool, error) {
 	return string(out), true, nil
 }
 
+func fixUppercaseFreedomFinalRules() error {
+	var setting model.Setting
+	err := db.Model(model.Setting{}).Where("key = ?", "xrayTemplateConfig").First(&setting).Error
+	if errors.Is(err, gorm.ErrRecordNotFound) {
+		return db.Create(&model.HistoryOfSeeders{SeederName: "UppercaseFreedomFinalRulesFix"}).Error
+	}
+	if err != nil {
+		return err
+	}
+
+	updated, changed, rErr := rewriteUppercaseFreedomFinalRules(setting.Value)
+	if rErr != nil {
+		log.Printf("UppercaseFreedomFinalRulesFix: skip (invalid xrayTemplateConfig json): %v", rErr)
+		return db.Create(&model.HistoryOfSeeders{SeederName: "UppercaseFreedomFinalRulesFix"}).Error
+	}
+
+	return db.Transaction(func(tx *gorm.DB) error {
+		if changed {
+			if err := tx.Model(&model.Setting{}).Where("key = ?", "xrayTemplateConfig").
+				Update("value", updated).Error; err != nil {
+				return err
+			}
+		}
+		return tx.Create(&model.HistoryOfSeeders{SeederName: "UppercaseFreedomFinalRulesFix"}).Error
+	})
+}
+
+// Re-runs both finalRules rewrites, because the rows of the two seeders above it
+// already exist on any panel that walked past a differently spelled outbound.
+func rewriteUppercaseFreedomFinalRules(raw string) (string, bool, error) {
+	if !hasNonLowercaseFreedomOutbound(raw) {
+		return raw, false, nil
+	}
+	reversed, reversedChanged, err := rewriteFreedomFinalRules(raw)
+	if err != nil {
+		return raw, false, err
+	}
+	hardened, hardenedChanged, err := rewriteFreedomFinalRulesPrivateEgress(reversed)
+	if err != nil {
+		return raw, false, err
+	}
+	if !reversedChanged && !hardenedChanged {
+		return raw, false, nil
+	}
+	return hardened, true, nil
+}
+
+func hasNonLowercaseFreedomOutbound(raw string) bool {
+	if strings.TrimSpace(raw) == "" {
+		return false
+	}
+	var cfg map[string]any
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		return false
+	}
+	outbounds, ok := cfg["outbounds"].([]any)
+	if !ok {
+		return false
+	}
+	for _, ob := range outbounds {
+		obj, ok := ob.(map[string]any)
+		if !ok {
+			continue
+		}
+		if proto, _ := obj["protocol"].(string); strings.EqualFold(proto, "freedom") && proto != "freedom" {
+			return true
+		}
+	}
+	return false
+}
+
 func stripRealityFinalmaskTcp() error {
 	var inbounds []model.Inbound
 	if err := db.Find(&inbounds).Error; err != nil {

+ 5 - 0
internal/database/freedom_domain_strategy_migration_test.go

@@ -169,6 +169,11 @@ func TestRewriteFreedomDomainStrategySatisfiesCore(t *testing.T) {
 			raw:       `{"protocol":"freedom","tag":"direct","targetStrategy":"ForceIPv6","settings":{}}`,
 			wantValue: `"domainStrategy": "ForceIPv6"`,
 		},
+		{
+			name:      "uppercase protocol spelling",
+			raw:       `{"protocol":"Freedom","tag":"direct","settings":{"domainStrategy":"UseIPv4","finalRules":[{"action":"allow"}]}}`,
+			wantValue: `"domainStrategy": "UseIPv4"`,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			capture := captureCoreLogs(t)

+ 249 - 0
internal/database/rewriter_protocol_case_test.go

@@ -0,0 +1,249 @@
+package database
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/config"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/xray"
+)
+
+func TestRewriteRemovedOutboundKeysSeesAnUppercaseFreedom(t *testing.T) {
+	raw := `{"outbounds":[{"protocol":"Freedom","tag":"direct","settings":{},"streamSettings":{"sockopt":{"addressPortStrategy":"SrvPortOnly"}}}]}`
+	var before struct {
+		Outbounds []json.RawMessage `json:"outbounds"`
+	}
+	if err := json.Unmarshal([]byte(raw), &before); err != nil {
+		t.Fatal(err)
+	}
+	// A refusal here is the proof the core treated it as freedom, not as unknown.
+	if err := xray.ValidateOutboundConfig(before.Outbounds[0]); err == nil {
+		t.Fatal("expected the vendored core to refuse the legacy addressPortStrategy")
+	}
+
+	updated, changed, err := rewriteRemovedOutboundKeys(raw)
+	if err != nil {
+		t.Fatalf("rewrite: %v", err)
+	}
+	if !changed {
+		t.Fatal(`an outbound spelled "Freedom" was left with the key the core refuses`)
+	}
+	var after struct {
+		Outbounds []json.RawMessage `json:"outbounds"`
+	}
+	if err := json.Unmarshal([]byte(updated), &after); err != nil {
+		t.Fatal(err)
+	}
+	if err := xray.ValidateOutboundConfig(after.Outbounds[0]); err != nil {
+		t.Fatalf("rewritten outbound still refused by xray-core: %v", err)
+	}
+}
+
+func TestRewriteUppercaseFreedomFinalRules(t *testing.T) {
+	hardened := []any{
+		map[string]any{"action": "block", "ip": []any{"geoip:private"}},
+		map[string]any{"action": "allow"},
+	}
+	tests := []struct {
+		name        string
+		raw         string
+		wantChanged bool
+		wantRules   []any
+	}{
+		{
+			name:        "stock allow-only rules are hardened",
+			raw:         `{"outbounds":[{"protocol":"Freedom","tag":"direct","settings":{"finalRules":[{"action":"allow"}]}}]}`,
+			wantChanged: true,
+			wantRules:   hardened,
+		},
+		{
+			name:        "legacy private-only allow is hardened",
+			raw:         `{"outbounds":[{"protocol":"FREEDOM","tag":"direct","settings":{"finalRules":[{"action":"allow","ip":["geoip:private"]}]}}]}`,
+			wantChanged: true,
+			wantRules:   hardened,
+		},
+		{
+			name:        "missing finalRules is hardened",
+			raw:         `{"outbounds":[{"protocol":"Freedom","tag":"direct","settings":{}}]}`,
+			wantChanged: true,
+			wantRules:   hardened,
+		},
+		{
+			name:        "customized rules are preserved",
+			raw:         `{"outbounds":[{"protocol":"Freedom","tag":"direct","settings":{"finalRules":[{"action":"block","ip":["1.2.3.4"]},{"action":"allow"}]}}]}`,
+			wantChanged: false,
+		},
+		{
+			name:        "a canonical spelling was already handled by its own seeder",
+			raw:         `{"outbounds":[{"protocol":"freedom","tag":"direct","settings":{"finalRules":[{"action":"allow"}]}}]}`,
+			wantChanged: false,
+		},
+		{
+			name:        "another protocol is ignored",
+			raw:         `{"outbounds":[{"protocol":"blackhole","tag":"blocked","settings":{}}]}`,
+			wantChanged: false,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			updated, changed, err := rewriteUppercaseFreedomFinalRules(tc.raw)
+			if err != nil {
+				t.Fatalf("rewrite: %v", err)
+			}
+			if changed != tc.wantChanged {
+				t.Fatalf("changed = %v, want %v (out: %s)", changed, tc.wantChanged, updated)
+			}
+			if !tc.wantChanged {
+				if updated != tc.raw {
+					t.Fatalf("raw config mutated without change flag:\n%s", updated)
+				}
+				return
+			}
+			var cfg map[string]any
+			if err := json.Unmarshal([]byte(updated), &cfg); err != nil {
+				t.Fatalf("updated config is not valid json: %v", err)
+			}
+			outbounds := cfg["outbounds"].([]any)
+			settings := outbounds[0].(map[string]any)["settings"].(map[string]any)
+			gotRules, _ := json.Marshal(settings["finalRules"])
+			wantRules, _ := json.Marshal(tc.wantRules)
+			if string(gotRules) != string(wantRules) {
+				t.Fatalf("finalRules = %s, want %s", gotRules, wantRules)
+			}
+		})
+	}
+}
+
+// The two earlier seeders recorded their rows before this predicate existed, so
+// this pins the re-run reaching a panel whose history already has both.
+func TestUppercaseFreedomFinalRulesFixReachesHistoryGatedPanels(t *testing.T) {
+	t.Setenv("XUI_DB_FOLDER", t.TempDir())
+	if err := InitDB(config.GetDBPath()); err != nil {
+		if strings.Contains(err.Error(), "CGO_ENABLED=0") {
+			t.Skipf("sqlite needs cgo: %v", err)
+		}
+		t.Fatalf("init db: %v", err)
+	}
+	t.Cleanup(func() { _ = CloseDB() })
+
+	stock := `{"outbounds":[{"protocol":"Freedom","tag":"direct","settings":{"finalRules":[{"action":"allow"}]}}]}`
+	seedTemplate(t, stock)
+	// InitDB pre-seeds a fresh install's rows, so the earlier two are already
+	// recorded and this seeder has to look unapplied for the run to reach it.
+	for _, name := range []string{"FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock"} {
+		var count int64
+		if err := db.Model(&model.HistoryOfSeeders{}).
+			Where("seeder_name = ?", name).Count(&count).Error; err != nil {
+			t.Fatalf("count %s: %v", name, err)
+		}
+		if count == 0 {
+			t.Fatalf("%s is no longer pre-seeded on a fresh install", name)
+		}
+	}
+	if err := db.Where("seeder_name = ?", "UppercaseFreedomFinalRulesFix").
+		Delete(&model.HistoryOfSeeders{}).Error; err != nil {
+		t.Fatalf("clear seeder history: %v", err)
+	}
+
+	if err := runSeeders(false); err != nil {
+		t.Fatalf("runSeeders: %v", err)
+	}
+
+	var cfg struct {
+		Outbounds []map[string]any `json:"outbounds"`
+	}
+	if err := json.Unmarshal([]byte(storedTemplate(t)), &cfg); err != nil {
+		t.Fatalf("stored template is not JSON: %v", err)
+	}
+	settings, _ := cfg.Outbounds[0]["settings"].(map[string]any)
+	if settings["finalRules"] == nil {
+		t.Fatal("the hardening never reached an outbound spelled Freedom")
+	}
+	if proto, _ := cfg.Outbounds[0]["protocol"].(string); proto != "Freedom" {
+		t.Fatalf("the seeder rewrote the protocol id: %q", proto)
+	}
+	if hardened := storedTemplate(t); !strings.Contains(hardened, `"geoip:private"`) {
+		t.Fatalf("stored finalRules are not the hardened pair: %s", hardened)
+	}
+
+	// A second pass must leave the template alone: the row it just wrote gates it.
+	after := storedTemplate(t)
+	if err := runSeeders(false); err != nil {
+		t.Fatalf("second runSeeders: %v", err)
+	}
+	if got := storedTemplate(t); got != after {
+		t.Fatalf("the re-run seed rewrote the template again:\n got %s\nwant %s", got, after)
+	}
+}
+
+// The core lowercases a protocol id before looking up its handler, so a config
+// that runs as freedom must migrate the same whether it says Freedom or freedom.
+func TestRewritersTreatProtocolCaseAlike(t *testing.T) {
+	tests := []struct {
+		name    string
+		rewrite func(string) (string, bool, error)
+		raw     string
+	}{
+		{
+			name:    "removed outbound keys",
+			rewrite: rewriteRemovedOutboundKeys,
+			raw:     `{"outbounds":[{"protocol":"freedom","tag":"direct","settings":{},"streamSettings":{"sockopt":{"addressPortStrategy":"SrvPortOnly"}}}]}`,
+		},
+		{
+			name:    "freedom final rules reverse",
+			rewrite: rewriteFreedomFinalRules,
+			raw:     `{"outbounds":[{"protocol":"freedom","tag":"direct","settings":{"finalRules":[{"action":"allow","ip":["geoip:private"]}]}}]}`,
+		},
+		{
+			name:    "freedom private egress block",
+			rewrite: rewriteFreedomFinalRulesPrivateEgress,
+			raw:     `{"outbounds":[{"protocol":"freedom","tag":"direct","settings":{"finalRules":[]}}]}`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			want, wantChanged, err := tt.rewrite(tt.raw)
+			if err != nil || !wantChanged {
+				t.Fatalf("lowercase rewrite: changed=%v err=%v", wantChanged, err)
+			}
+			upper := strings.Replace(tt.raw, `"protocol":"freedom"`, `"protocol":"Freedom"`, 1)
+			got, changed, err := tt.rewrite(upper)
+			if err != nil {
+				t.Fatalf("uppercase rewrite: %v", err)
+			}
+			if !changed {
+				t.Fatalf(`the rewrite skipped an outbound spelled "Freedom"`)
+			}
+			if loweredGot, loweredWant := lowercaseProtocol(t, got), lowercaseProtocol(t, want); loweredGot != loweredWant {
+				t.Fatalf("uppercase result differs from lowercase:\n got %s\nwant %s", loweredGot, loweredWant)
+			}
+		})
+	}
+}
+
+// The rewriters keep the spelling they were given; only the migrated keys may
+// differ, so the comparison ignores the case of the protocol id itself.
+func lowercaseProtocol(t *testing.T, raw string) string {
+	t.Helper()
+	var cfg map[string]any
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatal(err)
+	}
+	outbounds, _ := cfg["outbounds"].([]any)
+	for _, ob := range outbounds {
+		obj, ok := ob.(map[string]any)
+		if !ok {
+			continue
+		}
+		if proto, ok := obj["protocol"].(string); ok {
+			obj["protocol"] = strings.ToLower(proto)
+		}
+	}
+	out, err := json.Marshal(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return string(out)
+}