Reject pure-IP rule-set references without match_response

DNS rules referencing rule-sets that contain only ip_cidr predicates silently stopped matching when legacy DNS mode was disabled, because the IP-CIDR branch cannot match against an in-flight DNS query. The existing validation intentionally let every rule_set through on the premise that mixed sets still work via their non-IP branches, which is only true when such a branch exists. Track whether a rule-set carries any non-IP-CIDR predicate and reject pure-IP references the same way bare ip_cidr fields are already rejected.
2026-05-04 11:32:28 +00:00 · 2026-04-15 21:02:40 +08:00
parent 9bee53225c
commit 90a642ef96
6 changed files with 206 additions and 18 deletions
@@ -70,4 +70,10 @@ type RuleSetMetadata struct {
 	ContainsWIFIRule         bool
 	ContainsIPCIDRRule       bool
 	ContainsDNSQueryTypeRule bool
+	// ContainsNonIPCIDRRule signals that the rule-set carries at least one sub-rule
+	// with a predicate other than destination ip_cidr / ip_set, so it can contribute
+	// to DNS pre-response matching. A rule-set where this is false and
+	// ContainsIPCIDRRule is true is "pure-IP" and matches nothing before a DNS
+	// response is available.
+	ContainsNonIPCIDRRule bool
 }
@@ -186,7 +186,7 @@ func (r *Router) buildRules(startRules bool) ([]adapter.DNSRule, bool, dnsRuleMo
 		return nil, false, dnsRuleModeFlags{}, err
 	}
 	if !legacyDNSMode {
-		err = validateLegacyDNSModeDisabledRules(r.rawRules)
+		err = validateLegacyDNSModeDisabledRules(router, r.rawRules, nil)
 		if err != nil {
 			return nil, false, dnsRuleModeFlags{}, err
 		}
@@ -248,7 +248,7 @@ func (r *Router) ValidateRuleSetMetadataUpdate(tag string, metadata adapter.Rule
 			return err
 		}
 		if !candidateLegacyDNSMode {
-			return validateLegacyDNSModeDisabledRules(r.rawRules)
+			return validateLegacyDNSModeDisabledRules(router, r.rawRules, overrides)
 		}
 		return nil
 	}
@@ -258,7 +258,7 @@ func (r *Router) ValidateRuleSetMetadataUpdate(tag string, metadata adapter.Rule
 	}
 	if legacyDNSMode {
 		if !candidateLegacyDNSMode && flags.disabled {
-			err := validateLegacyDNSModeDisabledRules(r.rawRules)
+			err := validateLegacyDNSModeDisabledRules(router, r.rawRules, overrides)
 			if err != nil {
 				return err
 			}
@@ -269,7 +269,7 @@ func (r *Router) ValidateRuleSetMetadataUpdate(tag string, metadata adapter.Rule
 	if candidateLegacyDNSMode {
 		return E.New(deprecated.OptionLegacyDNSAddressFilter.MessageWithLink())
 	}
-	return nil
+	return validateLegacyDNSModeDisabledRules(router, r.rawRules, overrides)
 }

 func (r *Router) matchDNS(ctx context.Context, rules []adapter.DNSRule, allowFakeIP bool, ruleIndex int, isAddressQuery bool, options *adapter.DNSQueryOptions) (adapter.DNSTransport, adapter.DNSRule, int) {
@@ -1025,10 +1025,10 @@ func referencedDNSRuleSetTags(rules []option.DNSRule) []string {
 	return tags
 }

-func validateLegacyDNSModeDisabledRules(rules []option.DNSRule) error {
+func validateLegacyDNSModeDisabledRules(router adapter.Router, rules []option.DNSRule, metadataOverrides map[string]adapter.RuleSetMetadata) error {
 	var seenEvaluate bool
 	for i, rule := range rules {
-		requiresPriorEvaluate, err := validateLegacyDNSModeDisabledRuleTree(rule)
+		requiresPriorEvaluate, err := validateLegacyDNSModeDisabledRuleTree(router, rule, metadataOverrides)
 		if err != nil {
 			return E.Cause(err, "validate dns rule[", i, "]")
 		}
@@ -1063,14 +1063,14 @@ func validateEvaluateFakeIPRules(rules []option.DNSRule, transportManager adapte
 	return nil
 }

-func validateLegacyDNSModeDisabledRuleTree(rule option.DNSRule) (bool, error) {
+func validateLegacyDNSModeDisabledRuleTree(router adapter.Router, rule option.DNSRule, metadataOverrides map[string]adapter.RuleSetMetadata) (bool, error) {
 	switch rule.Type {
 	case "", C.RuleTypeDefault:
-		return validateLegacyDNSModeDisabledDefaultRule(rule.DefaultOptions)
+		return validateLegacyDNSModeDisabledDefaultRule(router, rule.DefaultOptions, metadataOverrides)
 	case C.RuleTypeLogical:
 		requiresPriorEvaluate := dnsRuleActionType(rule) == C.RuleActionTypeRespond
 		for i, subRule := range rule.LogicalOptions.Rules {
-			subRequiresPriorEvaluate, err := validateLegacyDNSModeDisabledRuleTree(subRule)
+			subRequiresPriorEvaluate, err := validateLegacyDNSModeDisabledRuleTree(router, subRule, metadataOverrides)
 			if err != nil {
 				return false, E.Cause(err, "sub rule[", i, "]")
 			}
@@ -1082,16 +1082,25 @@ func validateLegacyDNSModeDisabledRuleTree(rule option.DNSRule) (bool, error) {
 	}
 }

-func validateLegacyDNSModeDisabledDefaultRule(rule option.DefaultDNSRule) (bool, error) {
+func validateLegacyDNSModeDisabledDefaultRule(router adapter.Router, rule option.DefaultDNSRule, metadataOverrides map[string]adapter.RuleSetMetadata) (bool, error) {
 	hasResponseRecords := hasResponseMatchFields(rule)
 	if (hasResponseRecords || len(rule.IPCIDR) > 0 || rule.IPIsPrivate || rule.IPAcceptAny) && !rule.MatchResponse {
 		return false, E.New("Response Match Fields (ip_cidr, ip_is_private, ip_accept_any, response_rcode, response_answer, response_ns, response_extra) require match_response to be enabled")
 	}
-	// Intentionally do not reject rule_set here. A referenced rule set may mix
-	// destination-IP predicates with pre-response predicates such as domain items.
-	// When match_response is false, those destination-IP branches fail closed during
-	// pre-response evaluation instead of consuming DNS response state, while sibling
-	// non-response branches remain matchable.
+	// rule_set entries are only rejected when every referenced set is pure-IP;
+	// mixed sets still fall through because their non-IP branches remain matchable
+	// before a DNS response is available.
+	if !rule.MatchResponse && len(rule.RuleSet) > 0 {
+		for _, tag := range rule.RuleSet {
+			metadata, err := lookupDNSRuleSetMetadata(router, tag, metadataOverrides)
+			if err != nil {
+				return false, err
+			}
+			if metadata.ContainsIPCIDRRule && !metadata.ContainsNonIPCIDRRule {
+				return false, E.New(deprecated.OptionLegacyDNSAddressFilter.MessageWithLink())
+			}
+		}
+	}
 	if rule.RuleSetIPCIDRAcceptEmpty { //nolint:staticcheck
 		return false, E.New(deprecated.OptionRuleSetIPCIDRAcceptEmpty.MessageWithLink())
 	}
@@ -761,7 +761,8 @@ func TestValidateRuleSetMetadataUpdateAllowsRuleSetThatKeepsNonLegacyDNSMode(t *
 	require.False(t, router.legacyDNSMode)

 	err := router.ValidateRuleSetMetadataUpdate("dynamic-set", adapter.RuleSetMetadata{
-		ContainsIPCIDRRule: true,
+		ContainsIPCIDRRule:    true,
+		ContainsNonIPCIDRRule: true,
 	})
 	require.NoError(t, err)
 }
@@ -808,6 +809,163 @@ func TestValidateRuleSetMetadataUpdateAllowsRelaxingLegacyRequirement(t *testing
 	require.NoError(t, err)
 }

+func TestInitializeRejectsPureIPRuleSetWhenLegacyDNSModeDisabled(t *testing.T) {
+	t.Parallel()
+
+	fakeSet := &fakeRuleSet{
+		metadata: adapter.RuleSetMetadata{
+			ContainsIPCIDRRule: true,
+		},
+	}
+	routerService := &fakeRouter{
+		ruleSets: map[string]adapter.RuleSet{
+			"pure-ip": fakeSet,
+		},
+	}
+	ctx := service.ContextWith[adapter.Router](context.Background(), routerService)
+	router := &Router{
+		ctx:                   ctx,
+		logger:                log.NewNOPFactory().NewLogger("dns"),
+		transport:             &fakeDNSTransportManager{},
+		client:                &fakeDNSClient{},
+		rawRules:              make([]option.DNSRule, 0, 2),
+		rules:                 make([]adapter.DNSRule, 0, 2),
+		defaultDomainStrategy: C.DomainStrategyAsIS,
+	}
+	err := router.Initialize([]option.DNSRule{
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeA)},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeRoute,
+					RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
+				},
+			},
+		},
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					RuleSet: badoption.Listable[string]{"pure-ip"},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeRoute,
+					RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
+				},
+			},
+		},
+	})
+	require.ErrorContains(t, err, "Address Filter Fields")
+}
+
+func TestInitializeAllowsMixedRuleSetWhenLegacyDNSModeDisabled(t *testing.T) {
+	t.Parallel()
+
+	fakeSet := &fakeRuleSet{
+		metadata: adapter.RuleSetMetadata{
+			ContainsIPCIDRRule:    true,
+			ContainsNonIPCIDRRule: true,
+		},
+	}
+	routerService := &fakeRouter{
+		ruleSets: map[string]adapter.RuleSet{
+			"mixed": fakeSet,
+		},
+	}
+	ctx := service.ContextWith[adapter.Router](context.Background(), routerService)
+	router := newTestRouterWithContext(t, ctx, []option.DNSRule{
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeA)},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeRoute,
+					RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
+				},
+			},
+		},
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					RuleSet: badoption.Listable[string]{"mixed"},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeRoute,
+					RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
+				},
+			},
+		},
+	}, &fakeDNSTransportManager{
+		defaultTransport: &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
+		transports: map[string]adapter.DNSTransport{
+			"default":  &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
+			"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
+		},
+	}, &fakeDNSClient{})
+	require.False(t, router.legacyDNSMode)
+}
+
+func TestValidateRuleSetMetadataUpdateRejectsRuleSetFlippingToPureIP(t *testing.T) {
+	t.Parallel()
+
+	fakeSet := &fakeRuleSet{
+		metadata: adapter.RuleSetMetadata{
+			ContainsIPCIDRRule:    true,
+			ContainsNonIPCIDRRule: true,
+		},
+	}
+	routerService := &fakeRouter{
+		ruleSets: map[string]adapter.RuleSet{
+			"mixed": fakeSet,
+		},
+	}
+	ctx := service.ContextWith[adapter.Router](context.Background(), routerService)
+	router := newTestRouterWithContext(t, ctx, []option.DNSRule{
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeA)},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeRoute,
+					RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
+				},
+			},
+		},
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					RuleSet: badoption.Listable[string]{"mixed"},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeRoute,
+					RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
+				},
+			},
+		},
+	}, &fakeDNSTransportManager{
+		defaultTransport: &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
+		transports: map[string]adapter.DNSTransport{
+			"default":  &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
+			"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
+		},
+	}, &fakeDNSClient{})
+	require.False(t, router.legacyDNSMode)
+
+	err := router.ValidateRuleSetMetadataUpdate("mixed", adapter.RuleSetMetadata{
+		ContainsIPCIDRRule: true,
+	})
+	require.ErrorContains(t, err, "Address Filter Fields")
+}
+
 func TestCloseWaitsForInFlightLookupUntilContextCancellation(t *testing.T) {
 	t.Parallel()

@@ -82,7 +82,9 @@ See [ACME](/configuration/shared/certificate-provider/acme/) for fields newly ad
 ### Migrate address filter fields to response matching

 Legacy Address Filter Fields (`ip_cidr`, `ip_is_private` without `match_response`) in DNS rules are deprecated,
-along with the Legacy `rule_set_ip_cidr_accept_empty` DNS rule item.
+along with the Legacy `rule_set_ip_cidr_accept_empty` DNS rule item. A DNS rule that references a rule-set
+containing only `ip_cidr` items (for example, a GeoIP rule-set) without `match_response` is also rejected
+at startup when legacy DNS mode is disabled.

 In sing-box 1.14.0, use the [`evaluate`](/configuration/dns/rule_action/#evaluate) action
 to fetch a DNS response, then match against it explicitly with `match_response`.
@@ -82,7 +82,9 @@ sing-box 1.14.0 新增字段参阅 [ACME](/zh/configuration/shared/certificate-p
 ### 迁移地址筛选字段到响应匹配

 旧版地址筛选字段（不使用 `match_response` 的 `ip_cidr`、`ip_is_private`）已废弃，
-旧版 `rule_set_ip_cidr_accept_empty` DNS 规则项也已废弃。
+旧版 `rule_set_ip_cidr_accept_empty` DNS 规则项也已废弃。当旧版 DNS 模式被禁用时，
+引用仅包含 `ip_cidr` 项的规则集（例如 GeoIP 规则集）且未设置 `match_response` 的 DNS 规则
+也将在启动时被拒绝。

 在 sing-box 1.14.0 中，请使用 [`evaluate`](/zh/configuration/dns/rule_action/#evaluate) 动作
 获取 DNS 响应，然后通过 `match_response` 显式匹配。
@@ -2,6 +2,7 @@ package rule

 import (
 	"context"
+	"reflect"

 	"github.com/sagernet/sing-box/adapter"
 	C "github.com/sagernet/sing-box/constant"
@@ -75,12 +76,22 @@ func isDNSQueryTypeHeadlessRule(rule option.DefaultHeadlessRule) bool {
 	return len(rule.QueryType) > 0
 }

+func isNonIPCIDRHeadlessRule(rule option.DefaultHeadlessRule) bool {
+	ipOnly := option.DefaultHeadlessRule{
+		IPCIDR: rule.IPCIDR,
+		IPSet:  rule.IPSet,
+		Invert: rule.Invert,
+	}
+	return !reflect.DeepEqual(rule, ipOnly)
+}
+
 func buildRuleSetMetadata(headlessRules []option.HeadlessRule) adapter.RuleSetMetadata {
 	return adapter.RuleSetMetadata{
 		ContainsProcessRule:      HasHeadlessRule(headlessRules, isProcessHeadlessRule),
 		ContainsWIFIRule:         HasHeadlessRule(headlessRules, isWIFIHeadlessRule),
 		ContainsIPCIDRRule:       HasHeadlessRule(headlessRules, isIPCIDRHeadlessRule),
 		ContainsDNSQueryTypeRule: HasHeadlessRule(headlessRules, isDNSQueryTypeHeadlessRule),
+		ContainsNonIPCIDRRule:    HasHeadlessRule(headlessRules, isNonIPCIDRHeadlessRule),
 	}
 }