From 90a642ef96b6329a9c0cc8d69e359b22e6effdc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Wed, 15 Apr 2026 21:02:40 +0800 Subject: [PATCH] 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. --- adapter/router.go | 6 ++ dns/router.go | 39 ++++++---- dns/router_test.go | 160 ++++++++++++++++++++++++++++++++++++++++- docs/migration.md | 4 +- docs/migration.zh.md | 4 +- route/rule/rule_set.go | 11 +++ 6 files changed, 206 insertions(+), 18 deletions(-) diff --git a/adapter/router.go b/adapter/router.go index 26f461257..24d45af00 100644 --- a/adapter/router.go +++ b/adapter/router.go @@ -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 } diff --git a/dns/router.go b/dns/router.go index b9fc8f977..adde3bac0 100644 --- a/dns/router.go +++ b/dns/router.go @@ -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()) } diff --git a/dns/router_test.go b/dns/router_test.go index 54213b23c..206eae73b 100644 --- a/dns/router_test.go +++ b/dns/router_test.go @@ -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() diff --git a/docs/migration.md b/docs/migration.md index 867f903b6..be26827f0 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -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`. diff --git a/docs/migration.zh.md b/docs/migration.zh.md index 54dec47e4..4d003e1ec 100644 --- a/docs/migration.zh.md +++ b/docs/migration.zh.md @@ -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` 显式匹配。 diff --git a/route/rule/rule_set.go b/route/rule/rule_set.go index 3c2d9eda2..6720e788b 100644 --- a/route/rule/rule_set.go +++ b/route/rule/rule_set.go @@ -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), } }