Skip to content

Commit 6afdbd9

Browse files
nekohasekainekohasekai
committed
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.
1 parent 4891f19 commit 6afdbd9

6 files changed

Lines changed: 206 additions & 18 deletions

File tree

‎adapter/router.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,10 @@ type RuleSetMetadata struct {
7070
ContainsWIFIRule bool
7171
ContainsIPCIDRRule bool
7272
ContainsDNSQueryTypeRule bool
73+
// ContainsNonIPCIDRRule signals that the rule-set carries at least one sub-rule
74+
// with a predicate other than destination ip_cidr / ip_set, so it can contribute
75+
// to DNS pre-response matching. A rule-set where this is false and
76+
// ContainsIPCIDRRule is true is "pure-IP" and matches nothing before a DNS
77+
// response is available.
78+
ContainsNonIPCIDRRule bool
7379
}

‎dns/router.go‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func (r *Router) buildRules(startRules bool) ([]adapter.DNSRule, bool, dnsRuleMo
186186
return nil, false, dnsRuleModeFlags{}, err
187187
}
188188
if !legacyDNSMode {
189-
err = validateLegacyDNSModeDisabledRules(r.rawRules)
189+
err = validateLegacyDNSModeDisabledRules(router, r.rawRules, nil)
190190
if err != nil {
191191
return nil, false, dnsRuleModeFlags{}, err
192192
}
@@ -248,7 +248,7 @@ func (r *Router) ValidateRuleSetMetadataUpdate(tag string, metadata adapter.Rule
248248
return err
249249
}
250250
if !candidateLegacyDNSMode {
251-
return validateLegacyDNSModeDisabledRules(r.rawRules)
251+
return validateLegacyDNSModeDisabledRules(router, r.rawRules, overrides)
252252
}
253253
return nil
254254
}
@@ -258,7 +258,7 @@ func (r *Router) ValidateRuleSetMetadataUpdate(tag string, metadata adapter.Rule
258258
}
259259
if legacyDNSMode {
260260
if !candidateLegacyDNSMode && flags.disabled {
261-
err := validateLegacyDNSModeDisabledRules(r.rawRules)
261+
err := validateLegacyDNSModeDisabledRules(router, r.rawRules, overrides)
262262
if err != nil {
263263
return err
264264
}
@@ -269,7 +269,7 @@ func (r *Router) ValidateRuleSetMetadataUpdate(tag string, metadata adapter.Rule
269269
if candidateLegacyDNSMode {
270270
return E.New(deprecated.OptionLegacyDNSAddressFilter.MessageWithLink())
271271
}
272-
return nil
272+
return validateLegacyDNSModeDisabledRules(router, r.rawRules, overrides)
273273
}
274274

275275
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 {
10251025
return tags
10261026
}
10271027

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

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

1085-
func validateLegacyDNSModeDisabledDefaultRule(rule option.DefaultDNSRule) (bool, error) {
1085+
func validateLegacyDNSModeDisabledDefaultRule(router adapter.Router, rule option.DefaultDNSRule, metadataOverrides map[string]adapter.RuleSetMetadata) (bool, error) {
10861086
hasResponseRecords := hasResponseMatchFields(rule)
10871087
if (hasResponseRecords || len(rule.IPCIDR) > 0 || rule.IPIsPrivate || rule.IPAcceptAny) && !rule.MatchResponse {
10881088
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")
10891089
}
1090-
// Intentionally do not reject rule_set here. A referenced rule set may mix
1091-
// destination-IP predicates with pre-response predicates such as domain items.
1092-
// When match_response is false, those destination-IP branches fail closed during
1093-
// pre-response evaluation instead of consuming DNS response state, while sibling
1094-
// non-response branches remain matchable.
1090+
// rule_set entries are only rejected when every referenced set is pure-IP;
1091+
// mixed sets still fall through because their non-IP branches remain matchable
1092+
// before a DNS response is available.
1093+
if !rule.MatchResponse && len(rule.RuleSet) > 0 {
1094+
for _, tag := range rule.RuleSet {
1095+
metadata, err := lookupDNSRuleSetMetadata(router, tag, metadataOverrides)
1096+
if err != nil {
1097+
return false, err
1098+
}
1099+
if metadata.ContainsIPCIDRRule && !metadata.ContainsNonIPCIDRRule {
1100+
return false, E.New(deprecated.OptionLegacyDNSAddressFilter.MessageWithLink())
1101+
}
1102+
}
1103+
}
10951104
if rule.RuleSetIPCIDRAcceptEmpty { //nolint:staticcheck
10961105
return false, E.New(deprecated.OptionRuleSetIPCIDRAcceptEmpty.MessageWithLink())
10971106
}

‎dns/router_test.go‎

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,8 @@ func TestValidateRuleSetMetadataUpdateAllowsRuleSetThatKeepsNonLegacyDNSMode(t *
761761
require.False(t, router.legacyDNSMode)
762762

763763
err := router.ValidateRuleSetMetadataUpdate("dynamic-set", adapter.RuleSetMetadata{
764-
ContainsIPCIDRRule: true,
764+
ContainsIPCIDRRule: true,
765+
ContainsNonIPCIDRRule: true,
765766
})
766767
require.NoError(t, err)
767768
}
@@ -808,6 +809,163 @@ func TestValidateRuleSetMetadataUpdateAllowsRelaxingLegacyRequirement(t *testing
808809
require.NoError(t, err)
809810
}
810811

812+
func TestInitializeRejectsPureIPRuleSetWhenLegacyDNSModeDisabled(t *testing.T) {
813+
t.Parallel()
814+
815+
fakeSet := &fakeRuleSet{
816+
metadata: adapter.RuleSetMetadata{
817+
ContainsIPCIDRRule: true,
818+
},
819+
}
820+
routerService := &fakeRouter{
821+
ruleSets: map[string]adapter.RuleSet{
822+
"pure-ip": fakeSet,
823+
},
824+
}
825+
ctx := service.ContextWith[adapter.Router](context.Background(), routerService)
826+
router := &Router{
827+
ctx: ctx,
828+
logger: log.NewNOPFactory().NewLogger("dns"),
829+
transport: &fakeDNSTransportManager{},
830+
client: &fakeDNSClient{},
831+
rawRules: make([]option.DNSRule, 0, 2),
832+
rules: make([]adapter.DNSRule, 0, 2),
833+
defaultDomainStrategy: C.DomainStrategyAsIS,
834+
}
835+
err := router.Initialize([]option.DNSRule{
836+
{
837+
Type: C.RuleTypeDefault,
838+
DefaultOptions: option.DefaultDNSRule{
839+
RawDefaultDNSRule: option.RawDefaultDNSRule{
840+
QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeA)},
841+
},
842+
DNSRuleAction: option.DNSRuleAction{
843+
Action: C.RuleActionTypeRoute,
844+
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
845+
},
846+
},
847+
},
848+
{
849+
Type: C.RuleTypeDefault,
850+
DefaultOptions: option.DefaultDNSRule{
851+
RawDefaultDNSRule: option.RawDefaultDNSRule{
852+
RuleSet: badoption.Listable[string]{"pure-ip"},
853+
},
854+
DNSRuleAction: option.DNSRuleAction{
855+
Action: C.RuleActionTypeRoute,
856+
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
857+
},
858+
},
859+
},
860+
})
861+
require.ErrorContains(t, err, "Address Filter Fields")
862+
}
863+
864+
func TestInitializeAllowsMixedRuleSetWhenLegacyDNSModeDisabled(t *testing.T) {
865+
t.Parallel()
866+
867+
fakeSet := &fakeRuleSet{
868+
metadata: adapter.RuleSetMetadata{
869+
ContainsIPCIDRRule: true,
870+
ContainsNonIPCIDRRule: true,
871+
},
872+
}
873+
routerService := &fakeRouter{
874+
ruleSets: map[string]adapter.RuleSet{
875+
"mixed": fakeSet,
876+
},
877+
}
878+
ctx := service.ContextWith[adapter.Router](context.Background(), routerService)
879+
router := newTestRouterWithContext(t, ctx, []option.DNSRule{
880+
{
881+
Type: C.RuleTypeDefault,
882+
DefaultOptions: option.DefaultDNSRule{
883+
RawDefaultDNSRule: option.RawDefaultDNSRule{
884+
QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeA)},
885+
},
886+
DNSRuleAction: option.DNSRuleAction{
887+
Action: C.RuleActionTypeRoute,
888+
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
889+
},
890+
},
891+
},
892+
{
893+
Type: C.RuleTypeDefault,
894+
DefaultOptions: option.DefaultDNSRule{
895+
RawDefaultDNSRule: option.RawDefaultDNSRule{
896+
RuleSet: badoption.Listable[string]{"mixed"},
897+
},
898+
DNSRuleAction: option.DNSRuleAction{
899+
Action: C.RuleActionTypeRoute,
900+
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
901+
},
902+
},
903+
},
904+
}, &fakeDNSTransportManager{
905+
defaultTransport: &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
906+
transports: map[string]adapter.DNSTransport{
907+
"default": &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
908+
"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
909+
},
910+
}, &fakeDNSClient{})
911+
require.False(t, router.legacyDNSMode)
912+
}
913+
914+
func TestValidateRuleSetMetadataUpdateRejectsRuleSetFlippingToPureIP(t *testing.T) {
915+
t.Parallel()
916+
917+
fakeSet := &fakeRuleSet{
918+
metadata: adapter.RuleSetMetadata{
919+
ContainsIPCIDRRule: true,
920+
ContainsNonIPCIDRRule: true,
921+
},
922+
}
923+
routerService := &fakeRouter{
924+
ruleSets: map[string]adapter.RuleSet{
925+
"mixed": fakeSet,
926+
},
927+
}
928+
ctx := service.ContextWith[adapter.Router](context.Background(), routerService)
929+
router := newTestRouterWithContext(t, ctx, []option.DNSRule{
930+
{
931+
Type: C.RuleTypeDefault,
932+
DefaultOptions: option.DefaultDNSRule{
933+
RawDefaultDNSRule: option.RawDefaultDNSRule{
934+
QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeA)},
935+
},
936+
DNSRuleAction: option.DNSRuleAction{
937+
Action: C.RuleActionTypeRoute,
938+
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
939+
},
940+
},
941+
},
942+
{
943+
Type: C.RuleTypeDefault,
944+
DefaultOptions: option.DefaultDNSRule{
945+
RawDefaultDNSRule: option.RawDefaultDNSRule{
946+
RuleSet: badoption.Listable[string]{"mixed"},
947+
},
948+
DNSRuleAction: option.DNSRuleAction{
949+
Action: C.RuleActionTypeRoute,
950+
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
951+
},
952+
},
953+
},
954+
}, &fakeDNSTransportManager{
955+
defaultTransport: &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
956+
transports: map[string]adapter.DNSTransport{
957+
"default": &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
958+
"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
959+
},
960+
}, &fakeDNSClient{})
961+
require.False(t, router.legacyDNSMode)
962+
963+
err := router.ValidateRuleSetMetadataUpdate("mixed", adapter.RuleSetMetadata{
964+
ContainsIPCIDRRule: true,
965+
})
966+
require.ErrorContains(t, err, "Address Filter Fields")
967+
}
968+
811969
func TestCloseWaitsForInFlightLookupUntilContextCancellation(t *testing.T) {
812970
t.Parallel()
813971

‎docs/migration.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ See [ACME](/configuration/shared/certificate-provider/acme/) for fields newly ad
8282
### Migrate address filter fields to response matching
8383

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

8789
In sing-box 1.14.0, use the [`evaluate`](/configuration/dns/rule_action/#evaluate) action
8890
to fetch a DNS response, then match against it explicitly with `match_response`.

‎docs/migration.zh.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ sing-box 1.14.0 新增字段参阅 [ACME](/zh/configuration/shared/certificate-p
8282
### 迁移地址筛选字段到响应匹配
8383

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

8789
在 sing-box 1.14.0 中，请使用 [`evaluate`](/zh/configuration/dns/rule_action/#evaluate) 动作
8890
获取 DNS 响应，然后通过 `match_response` 显式匹配。

‎route/rule/rule_set.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package rule
22

33
import (
44
"context"
5+
"reflect"
56

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

79+
func isNonIPCIDRHeadlessRule(rule option.DefaultHeadlessRule) bool {
80+
ipOnly := option.DefaultHeadlessRule{
81+
IPCIDR: rule.IPCIDR,
82+
IPSet: rule.IPSet,
83+
Invert: rule.Invert,
84+
}
85+
return !reflect.DeepEqual(rule, ipOnly)
86+
}
87+
7888
func buildRuleSetMetadata(headlessRules []option.HeadlessRule) adapter.RuleSetMetadata {
7989
return adapter.RuleSetMetadata{
8090
ContainsProcessRule: HasHeadlessRule(headlessRules, isProcessHeadlessRule),
8191
ContainsWIFIRule: HasHeadlessRule(headlessRules, isWIFIHeadlessRule),
8292
ContainsIPCIDRRule: HasHeadlessRule(headlessRules, isIPCIDRHeadlessRule),
8393
ContainsDNSQueryTypeRule: HasHeadlessRule(headlessRules, isDNSQueryTypeHeadlessRule),
94+
ContainsNonIPCIDRRule: HasHeadlessRule(headlessRules, isNonIPCIDRHeadlessRule),
8495
}
8596
}
8697

0 commit comments

Comments
 (0)