From 39ce7cbc22b737f5ac78ce89071164230083978e Mon Sep 17 00:00:00 2001 From: BlindMaster24 <375291171150z@gmail.com> Date: Mon, 14 Sep 2026 13:30:23 +0300 Subject: [PATCH] fix(xray): migrate the dns outbound off its legacy nonIPQuery and blockTypes (#6519) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(xray): migrate the dns outbound off its legacy nonIPQuery and blockTypes xray-core logs both keys as deprecated on every config load, and refuses them outright next to rules. The panel's own dns outbound card wrote them with defaults until it switched that card to rules, so a panel that ever had one keeps warning at every start, and the card no longer reads them back — saving that outbound from the current UI silently dropped the policy. The seeder converts them into the three rules the core's legacy builder produced, in its order, then drops the keys. * fix(xray): read a dns outbound's null keys and protocol id like the core Two details the seeder got wrong, both found reviewing the diff against the pinned loader. A JSON null is a present key with a nil value here but a nil pointer there, so `nonIPQuery: null` was rewritten into a reject policy the core never built, and `rules: null` hid the legacy pair that the core does still read — dropping the operator's policy on upgrade. And the core lowercases the protocol id before dispatching, so `"protocol": "DNS"` was never migrated and kept warning. --- internal/database/db.go | 157 +++++++- ...dns_outbound_legacy_keys_migration_test.go | 375 ++++++++++++++++++ 2 files changed, 531 insertions(+), 1 deletion(-) create mode 100644 internal/database/dns_outbound_legacy_keys_migration_test.go diff --git a/internal/database/db.go b/internal/database/db.go index 1a46a9fbd..92277ee1e 100644 --- a/internal/database/db.go +++ b/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", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "ResetIpLimitNoFail2ban"} + seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "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 @@ -1371,6 +1371,12 @@ func runSeeders(isUsersEmpty bool) error { } } + if !slices.Contains(seedersHistory, "DNSOutboundLegacyKeysFix") { + if err := migrateDNSOutboundLegacyKeys(); err != nil { + return err + } + } + if !slices.Contains(seedersHistory, "NodeInboundsAdopted") { if err := seedNodeInboundsAdopted(); err != nil { return err @@ -1771,6 +1777,155 @@ var freedomDomainStrategies = map[string]bool{ "forceipv6": true, "forceipv4v6": true, "forceipv6v4": true, } +// migrateDNSOutboundLegacyKeys rewrites stored dns outbounds once, because the +// core logs nonIPQuery/blockTypes as deprecated on every config load. +func migrateDNSOutboundLegacyKeys() 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: "DNSOutboundLegacyKeysFix"}).Error + } + if err != nil { + return err + } + + updated, changed, rErr := rewriteDNSOutboundLegacyKeys(setting.Value) + if rErr != nil { + log.Printf("DNSOutboundLegacyKeysFix: skip (invalid xrayTemplateConfig json): %v", rErr) + return db.Create(&model.HistoryOfSeeders{SeederName: "DNSOutboundLegacyKeysFix"}).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: "DNSOutboundLegacyKeysFix"}).Error + }) +} + +// rewriteDNSOutboundLegacyKeys turns a dns outbound's legacy nonIPQuery and +// blockTypes into rules, in the order the core's legacy builder used. +func rewriteDNSOutboundLegacyKeys(raw string) (string, bool, error) { + if strings.TrimSpace(raw) == "" { + return raw, false, nil + } + var cfg map[string]any + if err := json.Unmarshal([]byte(raw), &cfg); err != nil { + return raw, false, err + } + outbounds, ok := cfg["outbounds"].([]any) + if !ok { + return raw, false, nil + } + changed := false + for _, ob := range outbounds { + obj, ok := ob.(map[string]any) + if !ok { + continue + } + if proto, _ := obj["protocol"].(string); !strings.EqualFold(proto, "dns") { + continue + } + settings, _ := obj["settings"].(map[string]any) + if settings == nil { + continue + } + nonIPQuery, hasMode := settings["nonIPQuery"] + blockTypes, hasTypes := settings["blockTypes"] + // JSON null is absent to the core, which decides on nil pointers. + hasMode = hasMode && nonIPQuery != nil + hasTypes = hasTypes && blockTypes != nil + if !hasMode && !hasTypes { + continue + } + // The core refuses legacy keys next to real rules, so existing rules win. + if rules, hasRules := settings["rules"]; !hasRules || rules == nil { + settings["rules"] = legacyDNSOutboundRules(dnsNonIPQueryMode(nonIPQuery), legacyDNSBlockTypes(blockTypes)) + } + delete(settings, "nonIPQuery") + delete(settings, "blockTypes") + changed = true + } + if !changed { + return raw, false, nil + } + out, err := json.MarshalIndent(cfg, "", " ") + if err != nil { + return raw, false, err + } + return string(out), true, nil +} + +// dnsNonIPQueryMode reports the mode the core resolved: everything but drop and +// skip meant reject, and any other value never loaded in the first place. +func dnsNonIPQueryMode(value any) string { + mode, _ := value.(string) + mode = strings.ToLower(strings.TrimSpace(mode)) + if mode == "drop" || mode == "skip" { + return mode + } + return "reject" +} + +// legacyDNSBlockTypes accepts every shape the old card could save: a list, a +// bare number, or a comma-separated string, minus the qTypes the core rejects. +func legacyDNSBlockTypes(value any) []int { + items, ok := value.([]any) + if !ok && value != nil { + items = []any{value} + } + var out []int + for _, item := range items { + for _, part := range strings.Split(fmt.Sprint(item), ",") { + qType, err := strconv.Atoi(strings.TrimSpace(part)) + if err != nil || qType < 0 || qType > 65535 { + continue + } + out = append(out, qType) + } + } + return out +} + +// legacyDNSOutboundRules mirrors the core's own legacy dns policy: the blocked +// qTypes, then the hijack, then the mode's answer for everything else. +func legacyDNSOutboundRules(mode string, blockTypes []int) []any { + rules := make([]any, 0, 3) + if len(blockTypes) > 0 { + rule := map[string]any{"action": "drop", "qType": dnsQTypeValue(blockTypes)} + if mode == "reject" { + rule["action"] = "return" + rule["rCode"] = 5 + } + rules = append(rules, rule) + } + rules = append(rules, map[string]any{"action": "hijack", "qType": "1,28"}) + fallback := map[string]any{"action": "direct"} + switch mode { + case "reject": + fallback["action"] = "return" + fallback["rCode"] = 5 + case "drop": + fallback["action"] = "drop" + } + return append(rules, fallback) +} + +// dnsQTypeValue keeps a lone qType a number, the way the core marshals one. +func dnsQTypeValue(blockTypes []int) any { + if len(blockTypes) == 1 { + return blockTypes[0] + } + parts := make([]string, 0, len(blockTypes)) + for _, qType := range blockTypes { + parts = append(parts, strconv.Itoa(qType)) + } + return strings.Join(parts, ",") +} + func normalizeSettingPaths() error { pathKeys := []string{"webBasePath", "subPath", "subJsonPath", "subClashPath"} var rows []model.Setting diff --git a/internal/database/dns_outbound_legacy_keys_migration_test.go b/internal/database/dns_outbound_legacy_keys_migration_test.go new file mode 100644 index 000000000..de2a9ff87 --- /dev/null +++ b/internal/database/dns_outbound_legacy_keys_migration_test.go @@ -0,0 +1,375 @@ +package database + +import ( + "encoding/json" + "strings" + "testing" + + corelog "github.com/xtls/xray-core/common/log" + + "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 TestRewriteDNSOutboundLegacyKeys(t *testing.T) { + tests := []struct { + name string + raw string + wantChanged bool + wantOutbound map[string]any + }{ + { + name: "reject keeps the blocked qTypes and answers rCode 5", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"rewriteNetwork":"udp","rewriteAddress":"8.8.8.8","rewritePort":53,"nonIPQuery":"reject","blockTypes":[65,28]}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rewriteNetwork": "udp", "rewriteAddress": "8.8.8.8", "rewritePort": float64(53), + "rules": []any{ + map[string]any{"action": "return", "qType": "65,28", "rCode": float64(5)}, + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "return", "rCode": float64(5)}, + }, + }, + }, + }, + { + name: "drop with no blocked qTypes keeps only the hijack and the answer", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[]}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{ + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "drop"}, + }, + }, + }, + }, + { + name: "skip passes everything else through and keeps a lone qType a number", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"skip","blockTypes":[28]}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{ + map[string]any{"action": "drop", "qType": float64(28)}, + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "direct"}, + }, + }, + }, + }, + { + name: "the num field could hold a bare number or a string", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"reject","blockTypes":"65, 28"}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{ + map[string]any{"action": "return", "qType": "65,28", "rCode": float64(5)}, + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "return", "rCode": float64(5)}, + }, + }, + }, + }, + { + name: "a missing mode answered as reject, the core's default", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"blockTypes":[28]}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{ + map[string]any{"action": "return", "qType": float64(28), "rCode": float64(5)}, + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "return", "rCode": float64(5)}, + }, + }, + }, + }, + { + name: "existing rules win, because the core refuses the mix", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[28],"rules":[{"action":"hijack","qType":1}]}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{map[string]any{"action": "hijack", "qType": float64(1)}}, + }, + }, + }, + { + name: "a null legacy pair is not a legacy config, as in the core", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":null,"blockTypes":null}}]}`, + wantChanged: false, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{"nonIPQuery": nil, "blockTypes": nil}, + }, + }, + { + name: "null rules leave the legacy pair authoritative", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[28],"rules":null}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{ + map[string]any{"action": "drop", "qType": float64(28)}, + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "drop"}, + }, + }, + }, + }, + { + name: "the core lowercases the protocol id it dispatches on", + raw: `{"outbounds":[{"protocol":"DNS","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[]}}]}`, + wantChanged: true, + wantOutbound: map[string]any{ + "protocol": "DNS", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{ + map[string]any{"action": "hijack", "qType": "1,28"}, + map[string]any{"action": "drop"}, + }, + }, + }, + }, + { + name: "a dns outbound already on rules is left alone", + raw: `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"rules":[{"action":"hijack","qType":1}]}}]}`, + wantChanged: false, + wantOutbound: map[string]any{ + "protocol": "dns", "tag": "dns-out", + "settings": map[string]any{ + "rules": []any{map[string]any{"action": "hijack", "qType": float64(1)}}, + }, + }, + }, + { + name: "the same key names on another protocol are left alone", + raw: `{"outbounds":[{"protocol":"freedom","tag":"direct","settings":{"nonIPQuery":"drop","blockTypes":[28]}}]}`, + wantChanged: false, + wantOutbound: map[string]any{ + "protocol": "freedom", "tag": "direct", + "settings": map[string]any{"nonIPQuery": "drop", "blockTypes": []any{float64(28)}}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + updated, changed, err := rewriteDNSOutboundLegacyKeys(tc.raw) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if changed != tc.wantChanged { + t.Fatalf("changed = %v, want %v", changed, tc.wantChanged) + } + var cfg struct { + Outbounds []map[string]any `json:"outbounds"` + } + if err := json.Unmarshal([]byte(updated), &cfg); err != nil { + t.Fatalf("rewritten template is not JSON: %v", err) + } + if len(cfg.Outbounds) != 1 { + t.Fatalf("got %d outbounds, want 1", len(cfg.Outbounds)) + } + got, _ := json.Marshal(cfg.Outbounds[0]) + want, _ := json.Marshal(tc.wantOutbound) + if string(got) != string(want) { + t.Fatalf("outbound = %s, want %s", got, want) + } + }) + } +} + +type dnsCoreLogCapture struct{ msgs []string } + +func (c *dnsCoreLogCapture) Handle(msg corelog.Message) { c.msgs = append(c.msgs, msg.String()) } + +func (c *dnsCoreLogCapture) has(sub string) bool { + return strings.Contains(strings.Join(c.msgs, "\n"), sub) +} + +type dnsDiscardLogHandler struct{} + +func (dnsDiscardLogHandler) Handle(corelog.Message) {} + +func captureDNSCoreLogs(t *testing.T) *dnsCoreLogCapture { + t.Helper() + capture := new(dnsCoreLogCapture) + corelog.RegisterHandler(capture) + t.Cleanup(func() { corelog.RegisterHandler(dnsDiscardLogHandler{}) }) + return capture +} + +// Drives the real core: the legacy keys warn on every load, rules next to them +// are refused outright, and it reads JSON null the way this rewrite has to. +func TestRewriteDNSOutboundLegacyKeysSatisfiesCore(t *testing.T) { + for _, tc := range []struct { + name string + raw string + wantLoadError bool + wantLegacyWarning bool + wantChanged bool + }{ + { + name: "deprecated keys", + raw: `{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"reject","blockTypes":[65,28]}}`, + wantLegacyWarning: true, + wantChanged: true, + }, + { + name: "deprecated keys next to rules", + raw: `{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[28],"rules":[{"action":"hijack","qType":1}]}}`, + wantLoadError: true, + wantChanged: true, + }, + { + name: "a null legacy pair warns about nothing and builds no policy", + raw: `{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":null,"blockTypes":null}}`, + }, + { + name: "null rules keep the legacy pair in charge, and it warns", + raw: `{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[28],"rules":null}}`, + wantLegacyWarning: true, + wantChanged: true, + }, + { + name: "an upper-case protocol id is a dns outbound to the core", + raw: `{"protocol":"DNS","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[]}}`, + wantLegacyWarning: true, + wantChanged: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + capture := captureDNSCoreLogs(t) + + if err := xray.ValidateOutboundConfig([]byte(tc.raw)); (err != nil) != tc.wantLoadError { + t.Fatalf("legacy outbound load error = %v, want error: %v", err, tc.wantLoadError) + } + if capture.has("nonIPQuery") != tc.wantLegacyWarning { + t.Fatalf("legacy warning = %v, want %v: %v", capture.has("nonIPQuery"), tc.wantLegacyWarning, capture.msgs) + } + + updated, changed, err := rewriteDNSOutboundLegacyKeys(`{"outbounds":[` + tc.raw + `]}`) + if err != nil || changed != tc.wantChanged { + t.Fatalf("rewrite: changed=%v want %v err=%v", changed, tc.wantChanged, err) + } + if !changed { + if !strings.Contains(updated, tc.raw) { + t.Fatalf("unchanged outbound was rewritten: %s", updated) + } + return + } + var after struct { + Outbounds []json.RawMessage `json:"outbounds"` + } + if err := json.Unmarshal([]byte(updated), &after); err != nil { + t.Fatal(err) + } + + capture.msgs = nil + if err := xray.ValidateOutboundConfig(after.Outbounds[0]); err != nil { + t.Fatalf("xray-core refused the rewritten outbound: %v", err) + } + if capture.has("nonIPQuery") { + t.Fatalf("rewritten outbound still warns on load: %v", capture.msgs) + } + }) + } +} + +func TestRewriteDNSOutboundLegacyKeysInvalidJSON(t *testing.T) { + _, changed, err := rewriteDNSOutboundLegacyKeys("{not json") + if err == nil { + t.Fatal("expected an error for invalid JSON") + } + if changed { + t.Fatal("invalid JSON must not report a change") + } +} + +func TestMigrateDNSOutboundLegacyKeysRewritesStoredTemplate(t *testing.T) { + t.Setenv("XUI_DB_FOLDER", t.TempDir()) + // A CGO_ENABLED=0 build links a stubbed driver, so this test needs the same + // C compiler the rest of the package's DB tests do. + 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() }) + + legacy := `{"outbounds":[{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[28]}}]}` + seedDNSOutboundTemplate(t, legacy) + if err := db.Where("seeder_name = ?", "DNSOutboundLegacyKeysFix"). + Delete(&model.HistoryOfSeeders{}).Error; err != nil { + t.Fatalf("clear seeder history: %v", err) + } + + if err := migrateDNSOutboundLegacyKeys(); err != nil { + t.Fatalf("migrate: %v", err) + } + + got := storedDNSOutboundTemplate(t) + var cfg struct { + Outbounds []map[string]any `json:"outbounds"` + } + if err := json.Unmarshal([]byte(got), &cfg); err != nil { + t.Fatalf("stored template is not JSON: %v", err) + } + if len(cfg.Outbounds) != 1 { + t.Fatalf("stored outbounds = %d, want 1", len(cfg.Outbounds)) + } + settings, _ := cfg.Outbounds[0]["settings"].(map[string]any) + if _, present := settings["nonIPQuery"]; present { + t.Errorf("stored outbound kept nonIPQuery: %s", got) + } + if _, present := settings["blockTypes"]; present { + t.Errorf("stored outbound kept blockTypes: %s", got) + } + rules, _ := settings["rules"].([]any) + if len(rules) != 3 { + t.Fatalf("stored rules = %s, want the three legacy rules in %s", settings["rules"], got) + } + + // The history gate is what keeps a hand-edited template from being rewritten + // again on every restart, so run the real seeder list over a fresh legacy one. + seedDNSOutboundTemplate(t, legacy) + if err := runSeeders(false); err != nil { + t.Fatalf("runSeeders: %v", err) + } + if got := storedDNSOutboundTemplate(t); got != legacy { + t.Errorf("a completed seeder rewrote the template again: %s", got) + } +} + +func seedDNSOutboundTemplate(t *testing.T, value string) { + t.Helper() + if err := db.Where("key = ?", "xrayTemplateConfig").Delete(&model.Setting{}).Error; err != nil { + t.Fatalf("clear template: %v", err) + } + if err := db.Create(&model.Setting{Key: "xrayTemplateConfig", Value: value}).Error; err != nil { + t.Fatalf("seed template: %v", err) + } +} + +func storedDNSOutboundTemplate(t *testing.T) string { + t.Helper() + var setting model.Setting + if err := db.Where("key = ?", "xrayTemplateConfig").First(&setting).Error; err != nil { + t.Fatalf("reload template: %v", err) + } + return setting.Value +}