Browse Source

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.
Sanaei 1 day ago
parent
commit
bc424f0968

+ 2 - 1
frontend/src/lib/xray/outbound-form-adapter.ts

@@ -786,7 +786,8 @@ function dnsRuleToWire(r: DnsRuleForm) {
   const result: Raw = { action };
   const result: Raw = { action };
   const qType = r.qType.trim();
   const qType = r.qType.trim();
   if (qType) {
   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
   const domains = r.domain
     .split(',')
     .split(',')

+ 13 - 0
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 });
     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<string, unknown>).rules as Array<
+      Record<string, unknown>
+    >;
+    expect(rules[0]).toEqual({ action: 'drop', qType: '0' });
+  });
+
   it('dns rules read the legacy qtype wire key for back-compat', () => {
   it('dns rules read the legacy qtype wire key for back-compat', () => {
     const wire = {
     const wire = {
       protocol: 'dns',
       protocol: 'dns',

+ 76 - 3
internal/database/db.go

@@ -1254,7 +1254,7 @@ func runSeeders(isUsersEmpty bool) error {
 	}
 	}
 
 
 	if empty && isUsersEmpty {
 	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 {
 		for _, name := range seeders {
 			if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil {
 			if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil {
 				return err
 				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 !slices.Contains(seedersHistory, "NodeInboundsAdopted") {
 		if err := seedNodeInboundsAdopted(); err != nil {
 		if err := seedNodeInboundsAdopted(); err != nil {
 			return err
 			return err
@@ -1920,9 +1926,10 @@ func legacyDNSOutboundRules(mode string, blockTypes []int) []any {
 	return append(rules, fallback)
 	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 {
 func dnsQTypeValue(blockTypes []int) any {
-	if len(blockTypes) == 1 {
+	if len(blockTypes) == 1 && blockTypes[0] != 0 {
 		return blockTypes[0]
 		return blockTypes[0]
 	}
 	}
 	parts := make([]string, 0, len(blockTypes))
 	parts := make([]string, 0, len(blockTypes))
@@ -1932,6 +1939,72 @@ func dnsQTypeValue(blockTypes []int) any {
 	return strings.Join(parts, ",")
 	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 {
 func normalizeSettingPaths() error {
 	pathKeys := []string{"webBasePath", "subPath", "subJsonPath", "subClashPath"}
 	pathKeys := []string{"webBasePath", "subPath", "subJsonPath", "subClashPath"}
 	var rows []model.Setting
 	var rows []model.Setting

+ 99 - 0
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
+}

+ 4 - 0
internal/web/service/xray_setting.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"strings"
 
 
 	"github.com/mhsanaei/3x-ui/v3/internal/amneziawg"
 	"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/util/common"
 	"github.com/mhsanaei/3x-ui/v3/internal/xray"
 	"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 {
 	if synced, err := EnsureDnsServerRouting(newXraySettings); err == nil {
 		newXraySettings = synced
 		newXraySettings = synced
 	}
 	}
+	if spelled, changed, err := database.RewriteDNSOutboundQTypeZero(newXraySettings); err == nil && changed {
+		newXraySettings = spelled
+	}
 	return s.saveSetting("xrayTemplateConfig", newXraySettings)
 	return s.saveSetting("xrayTemplateConfig", newXraySettings)
 }
 }
 
 

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