From bc424f096837c7155cc677532423b12837186623 Mon Sep 17 00:00:00 2001 From: Sanaei Date: Tue, 15 Sep 2026 15:45:01 +0200 Subject: [PATCH] fix(xray): stop a lone dns qType 0 from matching every query The core reads a dns rule's qType as a PortList, which drops a bare numeric 0 (infra/conf/common.go: `if number != 0`), and a rule with no qTypes matches every query. A stored `"qType": 0` therefore does not target query type 0: it drops, refuses or hijacks all DNS through that outbound. A qType the panel writes has to be read by the core as exactly the query types it names. Four writers broke that: - DNSOutboundLegacyKeysFix rewrote a lone blockTypes [0] into "qType": 0, so "block type 0" became "block everything" on upgrade. - That seeder shipped in v3.8.0 and is recorded as done, so fixing it does not reach installs that already ran it. DNSOutboundQTypeZeroFix spells any stored numeric qType 0 as "0" once, protocol id matched like the core. - The outbound form adapter turned a typed "0" into the number 0. - The Xray template editor saves raw JSON past that adapter; the save now applies the same rewrite. Each writer is pinned by a test that fails without its part. The rewrite and the repair compare policies as the pinned core builds them, and the repair runs through runSeeders over a database whose legacy-keys seeder already ran, on SQLite and PostgreSQL 16. --- .../src/lib/xray/outbound-form-adapter.ts | 3 +- .../src/test/outbound-form-adapter.test.ts | 13 +++ internal/database/db.go | 79 ++++++++++++++- .../database/dns_outbound_qtype_zero_test.go | 99 +++++++++++++++++++ internal/web/service/xray_setting.go | 4 + .../service/xray_setting_dns_qtype_test.go | 37 +++++++ 6 files changed, 231 insertions(+), 4 deletions(-) create mode 100644 internal/database/dns_outbound_qtype_zero_test.go create mode 100644 internal/web/service/xray_setting_dns_qtype_test.go diff --git a/frontend/src/lib/xray/outbound-form-adapter.ts b/frontend/src/lib/xray/outbound-form-adapter.ts index a1ed479f5..b40c2d334 100644 --- a/frontend/src/lib/xray/outbound-form-adapter.ts +++ b/frontend/src/lib/xray/outbound-form-adapter.ts @@ -786,7 +786,8 @@ function dnsRuleToWire(r: DnsRuleForm) { const result: Raw = { action }; const qType = r.qType.trim(); if (qType) { - result.qType = /^\d+$/.test(qType) ? Number(qType) : qType; + // The core reads a numeric 0 as no qType at all, which matches every query. + result.qType = /^\d+$/.test(qType) && Number(qType) > 0 ? Number(qType) : qType; } const domains = r.domain .split(',') diff --git a/frontend/src/test/outbound-form-adapter.test.ts b/frontend/src/test/outbound-form-adapter.test.ts index 2dbfa00ea..daba54bcc 100644 --- a/frontend/src/test/outbound-form-adapter.test.ts +++ b/frontend/src/test/outbound-form-adapter.test.ts @@ -336,6 +336,19 @@ describe('outbound-form-adapter: round-trip', () => { expect(rules[1]).toEqual({ action: 'return', qType: 28, domain: ['blocked.com'], rCode: 3 }); }); + it('dns rules keep qType 0 a string, since the core reads a numeric 0 as every query', () => { + const back = formValuesToWirePayload( + rawOutboundToFormValues({ + protocol: 'dns', + settings: { rules: [{ action: 'drop', qType: 0 }] }, + }), + ); + const rules = (back.settings as Record).rules as Array< + Record + >; + expect(rules[0]).toEqual({ action: 'drop', qType: '0' }); + }); + it('dns rules read the legacy qtype wire key for back-compat', () => { const wire = { protocol: 'dns', diff --git a/internal/database/db.go b/internal/database/db.go index 390041c71..9d5efd99d 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", "UppercaseFreedomFinalRulesFix", "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", "DNSOutboundQTypeZeroFix", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "ResetIpLimitNoFail2ban"} for _, name := range seeders { if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil { return err @@ -1383,6 +1383,12 @@ func runSeeders(isUsersEmpty bool) error { } } + if !slices.Contains(seedersHistory, "DNSOutboundQTypeZeroFix") { + if err := migrateDNSOutboundQTypeZero(); err != nil { + return err + } + } + if !slices.Contains(seedersHistory, "NodeInboundsAdopted") { if err := seedNodeInboundsAdopted(); err != nil { return err @@ -1920,9 +1926,10 @@ func legacyDNSOutboundRules(mode string, blockTypes []int) []any { return append(rules, fallback) } -// dnsQTypeValue keeps a lone qType a number, the way the core marshals one. +// dnsQTypeValue keeps a lone qType a number the way the core marshals one, except +// 0: the core drops a numeric 0, and a rule with no qTypes matches every query. func dnsQTypeValue(blockTypes []int) any { - if len(blockTypes) == 1 { + if len(blockTypes) == 1 && blockTypes[0] != 0 { return blockTypes[0] } parts := make([]string, 0, len(blockTypes)) @@ -1932,6 +1939,72 @@ func dnsQTypeValue(blockTypes []int) any { return strings.Join(parts, ",") } +// migrateDNSOutboundQTypeZero repairs the numeric qType 0 that 3.8.0's legacy-keys +// seeder stored, which that seeder's own history row keeps it from revisiting. +func migrateDNSOutboundQTypeZero() 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: "DNSOutboundQTypeZeroFix"}).Error + } + if err != nil { + return err + } + + updated, changed, rErr := RewriteDNSOutboundQTypeZero(setting.Value) + if rErr != nil { + log.Printf("DNSOutboundQTypeZeroFix: skip (invalid xrayTemplateConfig json): %v", rErr) + return db.Create(&model.HistoryOfSeeders{SeederName: "DNSOutboundQTypeZeroFix"}).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: "DNSOutboundQTypeZeroFix"}).Error + }) +} + +// RewriteDNSOutboundQTypeZero spells a dns rule's numeric qType 0 as "0", the one +// form the core reads as query type 0 rather than as every query. +func RewriteDNSOutboundQTypeZero(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, _ := cfg["outbounds"].([]any) + changed := false + for _, ob := range outbounds { + obj, _ := ob.(map[string]any) + if proto, _ := obj["protocol"].(string); !strings.EqualFold(proto, "dns") { + continue + } + settings, _ := obj["settings"].(map[string]any) + rules, _ := settings["rules"].([]any) + for _, r := range rules { + rule, _ := r.(map[string]any) + if qType, ok := rule["qType"].(float64); ok && qType == 0 { + rule["qType"] = "0" + 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 +} + func normalizeSettingPaths() error { pathKeys := []string{"webBasePath", "subPath", "subJsonPath", "subClashPath"} var rows []model.Setting diff --git a/internal/database/dns_outbound_qtype_zero_test.go b/internal/database/dns_outbound_qtype_zero_test.go new file mode 100644 index 000000000..0d0a8f79b --- /dev/null +++ b/internal/database/dns_outbound_qtype_zero_test.go @@ -0,0 +1,99 @@ +package database + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/xtls/xray-core/infra/conf" + "github.com/xtls/xray-core/proxy/dns" + "google.golang.org/protobuf/proto" + + "github.com/mhsanaei/3x-ui/v3/internal/config" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// The core drops a lone numeric qType 0 from its PortList, and a rule with no +// qTypes matches every query, so blockTypes [0] must not be written that way. +func TestRewriteDNSOutboundLegacyKeysKeepsQTypeZeroPolicy(t *testing.T) { + for _, mode := range []string{"skip", "reject"} { + t.Run(mode, func(t *testing.T) { + legacy := `{"protocol":"dns","tag":"dns-out","settings":{"nonIPQuery":"` + mode + `","blockTypes":[0]}}` + updated, changed, err := rewriteDNSOutboundLegacyKeys(`{"outbounds":[` + legacy + `]}`) + if err != nil || !changed { + t.Fatalf("rewrite: changed=%v err=%v", changed, err) + } + got := coreDNSOutboundPolicy(t, firstTemplateOutbound(t, updated)) + if want := coreDNSOutboundPolicy(t, []byte(legacy)); !proto.Equal(got, want) { + t.Fatalf("rewritten policy = %v, want the legacy policy %v", got, want) + } + }) + } +} + +// Installs that already ran the legacy-keys seeder store the match-all rule, and +// that seeder never runs again, so the repair has to reach them on its own. +func TestSeedersRepairStoredDNSQTypeZero(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() }) + + // The legacy-keys seeder matched the protocol id without case, so it wrote both. + for _, protocol := range []string{"dns", "DNS"} { + t.Run(protocol, func(t *testing.T) { + legacy := `{"protocol":"` + protocol + `","tag":"dns-out","settings":{"nonIPQuery":"drop","blockTypes":[0]}}` + stored := `{"protocol":"` + protocol + `","tag":"dns-out","settings":{"rules":[{"action":"drop","qType":0},{"action":"hijack","qType":"1,28"},{"action":"drop"}]}}` + seedDNSOutboundTemplate(t, `{"outbounds":[`+stored+`]}`) + if err := db.Where("seeder_name = ?", "DNSOutboundQTypeZeroFix"). + Delete(&model.HistoryOfSeeders{}).Error; err != nil { + t.Fatalf("clear seeder history: %v", err) + } + + if err := runSeeders(false); err != nil { + t.Fatalf("runSeeders: %v", err) + } + + got := coreDNSOutboundPolicy(t, firstTemplateOutbound(t, storedDNSOutboundTemplate(t))) + if want := coreDNSOutboundPolicy(t, []byte(legacy)); !proto.Equal(got, want) { + t.Fatalf("repaired policy = %v, want the legacy policy %v", got, want) + } + }) + } +} + +func firstTemplateOutbound(t *testing.T, template string) []byte { + t.Helper() + var cfg struct { + Outbounds []json.RawMessage `json:"outbounds"` + } + if err := json.Unmarshal([]byte(template), &cfg); err != nil || len(cfg.Outbounds) == 0 { + t.Fatalf("template has no outbound (%v): %s", err, template) + } + return cfg.Outbounds[0] +} + +func coreDNSOutboundPolicy(t *testing.T, raw []byte) *dns.Config { + t.Helper() + var outbound conf.OutboundDetourConfig + if err := json.Unmarshal(raw, &outbound); err != nil { + t.Fatalf("unmarshal outbound: %v", err) + } + handler, err := outbound.Build() + if err != nil { + t.Fatalf("core build: %v", err) + } + instance, err := handler.ProxySettings.GetInstance() + if err != nil { + t.Fatalf("core settings: %v", err) + } + cfg, ok := instance.(*dns.Config) + if !ok { + t.Fatalf("core settings type = %T, want *dns.Config", instance) + } + return cfg +} diff --git a/internal/web/service/xray_setting.go b/internal/web/service/xray_setting.go index 817f4fff9..d129b0832 100644 --- a/internal/web/service/xray_setting.go +++ b/internal/web/service/xray_setting.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/mhsanaei/3x-ui/v3/internal/amneziawg" + "github.com/mhsanaei/3x-ui/v3/internal/database" "github.com/mhsanaei/3x-ui/v3/internal/util/common" "github.com/mhsanaei/3x-ui/v3/internal/xray" ) @@ -40,6 +41,9 @@ func (s *XraySettingService) SaveXraySetting(newXraySettings string) error { if synced, err := EnsureDnsServerRouting(newXraySettings); err == nil { newXraySettings = synced } + if spelled, changed, err := database.RewriteDNSOutboundQTypeZero(newXraySettings); err == nil && changed { + newXraySettings = spelled + } return s.saveSetting("xrayTemplateConfig", newXraySettings) } diff --git a/internal/web/service/xray_setting_dns_qtype_test.go b/internal/web/service/xray_setting_dns_qtype_test.go new file mode 100644 index 000000000..2638ff3a4 --- /dev/null +++ b/internal/web/service/xray_setting_dns_qtype_test.go @@ -0,0 +1,37 @@ +package service + +import ( + "encoding/json" + "testing" +) + +// The template editor saves raw JSON that never passes the outbound form's +// adapter, and the core reads a numeric qType 0 as every query. +func TestSaveXraySettingSpellsDNSQTypeZeroAsString(t *testing.T) { + setupSettingTestDB(t) + svc := &XraySettingService{} + template := `{"outbounds":[{"tag":"dns-out","protocol":"dns","settings":{"rules":[{"action":"drop","qType":0},{"action":"hijack","qType":28}]}}]}` + + if err := svc.SaveXraySetting(template); err != nil { + t.Fatalf("SaveXraySetting: %v", err) + } + + stored, err := svc.GetXrayConfigTemplate() + if err != nil { + t.Fatalf("GetXrayConfigTemplate: %v", err) + } + var cfg struct { + Outbounds []struct { + Settings struct { + Rules []map[string]any `json:"rules"` + } `json:"settings"` + } `json:"outbounds"` + } + if err := json.Unmarshal([]byte(stored), &cfg); err != nil || len(cfg.Outbounds) != 1 { + t.Fatalf("stored template unreadable (%v): %s", err, stored) + } + rules := cfg.Outbounds[0].Settings.Rules + if len(rules) != 2 || rules[0]["qType"] != "0" || rules[1]["qType"] != float64(28) { + t.Fatalf("stored dns rules = %v, want qType \"0\" then the untouched 28", rules) + } +}