From 4444f297c2c9891ca335a843ca4838206fef34de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Sat, 22 Aug 2026 13:34:45 +0800 Subject: [PATCH] Fix address filter matching for inverted DNS rules Inverted rules whose address filter conditions come from rule sets or logical rules were rejected during pre-lookup matching, so the lookup was never sent and the rules never matched; for plain ip rule sets this was a regression from the rule-set invert fix in 1.12.22. Pre-lookup matching now tracks whether the result depends on deferred address filter conditions and defers such rules to the response phase, while results already determined by evaluated conditions are still decided during pre-lookup. --- adapter/inbound.go | 4 +- route/rule/match_state.go | 18 +- route/rule/rule_abstract.go | 75 ++-- route/rule/rule_dns_address_filter_test.go | 392 +++++++++++++++++++++ route/rule/rule_item_rule_set.go | 4 +- route/rule/rule_set_local.go | 4 +- route/rule/rule_set_remote.go | 4 +- route/rule/rule_set_semantics_test.go | 24 +- 8 files changed, 484 insertions(+), 41 deletions(-) create mode 100644 route/rule/rule_dns_address_filter_test.go diff --git a/adapter/inbound.go b/adapter/inbound.go index f047199e..552e984d 100644 --- a/adapter/inbound.go +++ b/adapter/inbound.go @@ -94,7 +94,7 @@ type InboundContext struct { SourcePortMatch bool DestinationAddressMatch bool DestinationPortMatch bool - DidMatch bool + DefinitiveMatchStates uint16 IgnoreDestinationIPCIDRMatch bool } @@ -109,7 +109,7 @@ func (c *InboundContext) ResetRuleMatchCache() { c.SourcePortMatch = false c.DestinationAddressMatch = false c.DestinationPortMatch = false - c.DidMatch = false + c.DefinitiveMatchStates = 0 } type inboundContextKey struct{} diff --git a/route/rule/match_state.go b/route/rule/match_state.go index 0d2e4b0b..425fe15f 100644 --- a/route/rule/match_state.go +++ b/route/rule/match_state.go @@ -96,11 +96,16 @@ func matchHeadlessRuleStatesWithBase(rule adapter.HeadlessRule, metadata *adapte return matcher.matchStatesWithBase(metadata, base) } if matcher, isStateMatcher := rule.(ruleStateMatcher); isStateMatcher { - return matcher.matchStates(metadata).withBase(base) + stateSet := matcher.matchStates(metadata).withBase(base) + metadata.DefinitiveMatchStates = uint16(ruleMatchStateSet(metadata.DefinitiveMatchStates).withBase(base)) + return stateSet } if rule.Match(metadata) { - return emptyRuleMatchState().withBase(base) + stateSet := emptyRuleMatchState().withBase(base) + metadata.DefinitiveMatchStates = uint16(stateSet) + return stateSet } + metadata.DefinitiveMatchStates = 0 return 0 } @@ -109,10 +114,15 @@ func matchRuleItemStatesWithBase(item RuleItem, metadata *adapter.InboundContext return matcher.matchStatesWithBase(metadata, base) } if matcher, isStateMatcher := item.(ruleStateMatcher); isStateMatcher { - return matcher.matchStates(metadata).withBase(base) + stateSet := matcher.matchStates(metadata).withBase(base) + metadata.DefinitiveMatchStates = uint16(ruleMatchStateSet(metadata.DefinitiveMatchStates).withBase(base)) + return stateSet } if item.Match(metadata) { - return emptyRuleMatchState().withBase(base) + stateSet := emptyRuleMatchState().withBase(base) + metadata.DefinitiveMatchStates = uint16(stateSet) + return stateSet } + metadata.DefinitiveMatchStates = 0 return 0 } diff --git a/route/rule/rule_abstract.go b/route/rule/rule_abstract.go index 8a95fa6d..e02cf290 100644 --- a/route/rule/rule_abstract.go +++ b/route/rule/rule_abstract.go @@ -56,7 +56,7 @@ func (r *abstractDefaultRule) Match(metadata *adapter.InboundContext) bool { } func (r *abstractDefaultRule) destinationIPCIDRMatchesSource(metadata *adapter.InboundContext) bool { - return !metadata.IgnoreDestinationIPCIDRMatch && metadata.IPCIDRMatchSource && len(r.destinationIPCIDRItems) > 0 + return metadata.IPCIDRMatchSource && len(r.destinationIPCIDRItems) > 0 } func (r *abstractDefaultRule) destinationIPCIDRMatchesDestination(metadata *adapter.InboundContext) bool { @@ -77,7 +77,9 @@ func (r *abstractDefaultRule) matchStates(metadata *adapter.InboundContext) rule func (r *abstractDefaultRule) matchStatesWithBase(metadata *adapter.InboundContext, inheritedBase ruleMatchState) ruleMatchStateSet { if len(r.allItems) == 0 { - return emptyRuleMatchState().withBase(inheritedBase) + stateSet := emptyRuleMatchState().withBase(inheritedBase) + metadata.DefinitiveMatchStates = uint16(stateSet) + return stateSet } evaluationBase := inheritedBase if r.invert { @@ -85,59 +87,58 @@ func (r *abstractDefaultRule) matchStatesWithBase(metadata *adapter.InboundConte } baseState := evaluationBase if len(r.sourceAddressItems) > 0 { - metadata.DidMatch = true if matchAnyItem(r.sourceAddressItems, metadata) { baseState |= ruleMatchSourceAddress } } if r.destinationIPCIDRMatchesSource(metadata) && !baseState.has(ruleMatchSourceAddress) { - metadata.DidMatch = true if matchAnyItem(r.destinationIPCIDRItems, metadata) { baseState |= ruleMatchSourceAddress } - } else if r.destinationIPCIDRMatchesSource(metadata) { - metadata.DidMatch = true } if len(r.sourcePortItems) > 0 { - metadata.DidMatch = true if matchAnyItem(r.sourcePortItems, metadata) { baseState |= ruleMatchSourcePort } } if len(r.destinationAddressItems) > 0 { - metadata.DidMatch = true if matchAnyItem(r.destinationAddressItems, metadata) { baseState |= ruleMatchDestinationAddress } } if r.destinationIPCIDRMatchesDestination(metadata) && !baseState.has(ruleMatchDestinationAddress) { - metadata.DidMatch = true if matchAnyItem(r.destinationIPCIDRItems, metadata) { baseState |= ruleMatchDestinationAddress } - } else if r.destinationIPCIDRMatchesDestination(metadata) { - metadata.DidMatch = true } if len(r.destinationPortItems) > 0 { - metadata.DidMatch = true if matchAnyItem(r.destinationPortItems, metadata) { baseState |= ruleMatchDestinationPort } } + var deferredGroups ruleMatchState + if metadata.IgnoreDestinationIPCIDRMatch && !metadata.IPCIDRMatchSource && len(r.destinationIPCIDRItems) > 0 && len(r.destinationAddressItems) == 0 { + deferredGroups |= ruleMatchDestinationAddress + } for _, item := range r.items { - metadata.DidMatch = true if !item.Match(metadata) { - return r.invertedFailure(inheritedBase) + return r.invertedFailure(metadata, inheritedBase) } } - var stateSet ruleMatchStateSet + var stateSet, definitiveStateSet ruleMatchStateSet if r.ruleSetItem != nil { - metadata.DidMatch = true stateSet = matchRuleItemStatesWithBase(r.ruleSetItem, metadata, baseState) + definitiveStateSet = ruleMatchStateSet(metadata.DefinitiveMatchStates) } else { stateSet = singleRuleMatchState(baseState) + definitiveStateSet = stateSet } - stateSet = stateSet.filter(func(state ruleMatchState) bool { + if deferredGroups != 0 { + definitiveStateSet = definitiveStateSet.filter(func(state ruleMatchState) bool { + return deferredGroups&^state == 0 + }) + } + stateFilter := func(state ruleMatchState) bool { if r.requiresSourceAddressMatch(metadata) && !state.has(ruleMatchSourceAddress) { return false } @@ -151,24 +152,31 @@ func (r *abstractDefaultRule) matchStatesWithBase(metadata *adapter.InboundConte return false } return true - }) + } + stateSet = stateSet.filter(stateFilter) + definitiveStateSet = definitiveStateSet.filter(stateFilter) if stateSet.isEmpty() { - return r.invertedFailure(inheritedBase) + return r.invertedFailure(metadata, inheritedBase) } if r.invert { - // DNS pre-lookup defers destination address-limit checks until the response phase. - if metadata.IgnoreDestinationIPCIDRMatch && stateSet == emptyRuleMatchState() && !metadata.DidMatch && len(r.destinationIPCIDRItems) > 0 { + metadata.DefinitiveMatchStates = 0 + if definitiveStateSet.isEmpty() { + // DNS pre-lookup defers destination address-limit checks until the response phase. return emptyRuleMatchState().withBase(inheritedBase) } return 0 } + metadata.DefinitiveMatchStates = uint16(definitiveStateSet) return stateSet } -func (r *abstractDefaultRule) invertedFailure(base ruleMatchState) ruleMatchStateSet { +func (r *abstractDefaultRule) invertedFailure(metadata *adapter.InboundContext, base ruleMatchState) ruleMatchStateSet { if r.invert { - return emptyRuleMatchState().withBase(base) + stateSet := emptyRuleMatchState().withBase(base) + metadata.DefinitiveMatchStates = uint16(stateSet) + return stateSet } + metadata.DefinitiveMatchStates = 0 return 0 } @@ -239,37 +247,52 @@ func (r *abstractLogicalRule) matchStatesWithBase(metadata *adapter.InboundConte if r.invert { evaluationBase = 0 } - var stateSet ruleMatchStateSet + var stateSet, definitiveStateSet ruleMatchStateSet if r.mode == C.LogicalTypeAnd { stateSet = emptyRuleMatchState().withBase(evaluationBase) + definitiveStateSet = stateSet for _, rule := range r.rules { nestedMetadata := *metadata nestedMetadata.ResetRuleCache() nestedStateSet := matchHeadlessRuleStatesWithBase(rule, &nestedMetadata, evaluationBase) if nestedStateSet.isEmpty() { if r.invert { - return emptyRuleMatchState().withBase(base) + invertedStateSet := emptyRuleMatchState().withBase(base) + metadata.DefinitiveMatchStates = uint16(invertedStateSet) + return invertedStateSet } + metadata.DefinitiveMatchStates = 0 return 0 } stateSet = stateSet.combine(nestedStateSet) + definitiveStateSet = definitiveStateSet.combine(ruleMatchStateSet(nestedMetadata.DefinitiveMatchStates)) } } else { for _, rule := range r.rules { nestedMetadata := *metadata nestedMetadata.ResetRuleCache() stateSet = stateSet.merge(matchHeadlessRuleStatesWithBase(rule, &nestedMetadata, evaluationBase)) + definitiveStateSet = definitiveStateSet.merge(ruleMatchStateSet(nestedMetadata.DefinitiveMatchStates)) } if stateSet.isEmpty() { if r.invert { - return emptyRuleMatchState().withBase(base) + invertedStateSet := emptyRuleMatchState().withBase(base) + metadata.DefinitiveMatchStates = uint16(invertedStateSet) + return invertedStateSet } + metadata.DefinitiveMatchStates = 0 return 0 } } if r.invert { + metadata.DefinitiveMatchStates = 0 + if definitiveStateSet.isEmpty() { + // DNS pre-lookup defers destination address-limit checks until the response phase. + return emptyRuleMatchState().withBase(base) + } return 0 } + metadata.DefinitiveMatchStates = uint16(definitiveStateSet) return stateSet } diff --git a/route/rule/rule_dns_address_filter_test.go b/route/rule/rule_dns_address_filter_test.go new file mode 100644 index 00000000..0944bc3f --- /dev/null +++ b/route/rule/rule_dns_address_filter_test.go @@ -0,0 +1,392 @@ +package rule + +import ( + "context" + "net/netip" + "testing" + + "github.com/sagernet/sing-box/adapter" + C "github.com/sagernet/sing-box/constant" + "github.com/sagernet/sing-box/log" + "github.com/sagernet/sing-box/option" + "github.com/sagernet/sing/common/json" + M "github.com/sagernet/sing/common/metadata" + "github.com/sagernet/sing/service" + + "github.com/stretchr/testify/require" +) + +type addressFilterRouter struct { + adapter.Router + ruleSets map[string]adapter.RuleSet +} + +func (r *addressFilterRouter) RuleSet(tag string) (adapter.RuleSet, bool) { + ruleSet, loaded := r.ruleSets[tag] + return ruleSet, loaded +} + +func addressFilterContext(t *testing.T, ruleSetConfigs map[string]string) context.Context { + t.Helper() + router := &addressFilterRouter{ruleSets: make(map[string]adapter.RuleSet)} + ctx := service.ContextWith[adapter.Router](context.Background(), router) + for tag, config := range ruleSetConfigs { + var plainOptions option.PlainRuleSetCompat + err := json.UnmarshalContext(ctx, []byte(config), &plainOptions) + require.NoError(t, err) + ruleSet, err := NewLocalRuleSet(ctx, log.NewNOPFactory().Logger(), option.RuleSet{ + Tag: tag, + Type: C.RuleSetTypeInline, + InlineOptions: plainOptions.Options, + }) + require.NoError(t, err) + router.ruleSets[tag] = ruleSet + } + return ctx +} + +func addressFilterDNSRule(t *testing.T, ctx context.Context, config string) adapter.DNSRule { + t.Helper() + var ruleOptions option.DNSRule + err := json.UnmarshalContext(ctx, []byte(config), &ruleOptions) + require.NoError(t, err) + rule, err := NewDNSRule(ctx, log.NewNOPFactory().NewLogger("test"), ruleOptions, true) + require.NoError(t, err) + require.NoError(t, rule.Start()) + return rule +} + +// addressFilterFlow mirrors dns/router.go: pre-lookup Match under +// IgnoreDestinationIPCIDRMatch, then addressLimitResponseCheck against the +// response addresses. +func addressFilterFlow(rule adapter.DNSRule, domain string, responseAddress string) (preMatched bool, routed bool) { + metadata := adapter.InboundContext{ + Domain: domain, + QueryType: 1, + Source: M.ParseSocksaddrHostPort("192.168.1.10", 5353), + } + metadata.ResetRuleCache() + preMatched = rule.Match(&metadata) + if !preMatched { + return false, false + } + if !rule.WithAddressLimit() { + return true, true + } + checkMetadata := metadata + checkMetadata.DestinationAddresses = []netip.Addr{netip.MustParseAddr(responseAddress)} + return true, rule.MatchAddressLimit(&checkMetadata) +} + +func TestDNSAddressFilterInvert(t *testing.T) { + t.Parallel() + ctx := addressFilterContext(t, map[string]string{ + "mixed": `{"version": 3, "rules": [{"domain_suffix": ["ads.example"]}, {"ip_cidr": ["1.1.1.0/24"]}]}`, + "cn-ip": `{"version": 3, "rules": [{"ip_cidr": ["1.1.1.0/24"]}]}`, + "lan-ip": `{"version": 3, "rules": [{"ip_cidr": ["192.168.0.0/16"]}]}`, + "other-net": `{"version": 3, "rules": [{"ip_cidr": ["10.99.0.0/16"]}]}`, + "cn-domain": `{"version": 3, "rules": [{"domain_suffix": ["cn.example"]}]}`, + }) + testCases := []struct { + name string + rule string + domain string + responseAddress string + expectPreMatch bool + expectRouted bool + }{ + { + name: "direct mixed invert, domain hit", + rule: `{"domain_suffix": ["lookup.example"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + }, + { + name: "direct mixed invert, both miss", + rule: `{"domain_suffix": ["lookup.example"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "other.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "direct mixed invert, ip hit", + rule: `{"domain_suffix": ["lookup.example"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "other.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "ip rule-set invert, ip miss", + rule: `{"rule_set": ["cn-ip"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "ip rule-set invert, ip hit", + rule: `{"rule_set": ["cn-ip"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "mixed rule-set invert, both miss", + rule: `{"rule_set": ["mixed"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "mixed rule-set invert, domain hit skips pre-lookup", + rule: `{"rule_set": ["mixed"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "ads.example", + responseAddress: "8.8.8.8", + }, + { + name: "mixed rule-set invert, ip hit", + rule: `{"rule_set": ["mixed"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "logical invert, ip miss", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"rule_set": ["cn-ip"]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "logical invert, ip hit", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"rule_set": ["cn-ip"]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "logical and with inverted ip rule-set, ip miss", + rule: `{"type": "logical", "mode": "and", "rules": [{"domain_suffix": ["lookup.example"]}, {"rule_set": ["cn-ip"], "invert": true}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "logical and with inverted ip rule-set, ip hit", + rule: `{"type": "logical", "mode": "and", "rules": [{"domain_suffix": ["lookup.example"]}, {"rule_set": ["cn-ip"], "invert": true}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "logical or invert, domain hit skips pre-lookup", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"domain_suffix": ["ads.example"]}, {"rule_set": ["cn-ip"]}], "action": "route", "server": "proxy"}`, + domain: "ads.example", + responseAddress: "8.8.8.8", + }, + { + name: "logical or invert, both miss", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"domain_suffix": ["ads.example"]}, {"rule_set": ["cn-ip"]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "logical or invert, ip hit", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"domain_suffix": ["ads.example"]}, {"rule_set": ["cn-ip"]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "outer domain with ip rule-set invert, domain hit skips pre-lookup", + rule: `{"domain_suffix": ["cn.example"], "rule_set": ["cn-ip"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "cn.example", + responseAddress: "8.8.8.8", + }, + { + name: "outer domain with ip rule-set invert, both miss", + rule: `{"domain_suffix": ["cn.example"], "rule_set": ["cn-ip"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "other.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "query type with ip invert, ip miss", + rule: `{"query_type": ["A"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "query type with ip invert, ip hit", + rule: `{"query_type": ["A"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "source ip with ip invert, source hit ip miss", + rule: `{"source_ip_cidr": ["192.168.1.0/24"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "source ip with ip invert, source hit ip hit", + rule: `{"source_ip_cidr": ["192.168.1.0/24"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "source ip with ip invert, source miss", + rule: `{"source_ip_cidr": ["10.99.0.0/16"], "ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "ip rule-set without invert, ip hit", + rule: `{"rule_set": ["cn-ip"], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "ip rule-set without invert, ip miss", + rule: `{"rule_set": ["cn-ip"], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + }, + { + name: "mixed rule-set without invert, ip hit", + rule: `{"rule_set": ["mixed"], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "mixed rule-set without invert, both miss", + rule: `{"rule_set": ["mixed"], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + }, + { + name: "direct ip invert, ip miss", + rule: `{"ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "direct ip invert, ip hit", + rule: `{"ip_cidr": ["1.1.1.0/24"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "match-source rule-set invert, source in set", + rule: `{"rule_set": ["lan-ip"], "rule_set_ip_cidr_match_source": true, "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + }, + { + name: "match-source rule-set invert, source not in set", + rule: `{"rule_set": ["other-net"], "rule_set_ip_cidr_match_source": true, "invert": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "match-source rule-set, source in set", + rule: `{"rule_set": ["lan-ip"], "rule_set_ip_cidr_match_source": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "match-source rule-set, source not in set", + rule: `{"rule_set": ["other-net"], "rule_set_ip_cidr_match_source": true, "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + }, + { + name: "direct ip with domain rule-set invert, domain hit skips pre-lookup", + rule: `{"ip_cidr": ["1.1.1.0/24"], "rule_set": ["cn-domain"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "cn.example", + responseAddress: "8.8.8.8", + }, + { + name: "direct ip with domain rule-set invert, both miss", + rule: `{"ip_cidr": ["1.1.1.0/24"], "rule_set": ["cn-domain"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "other.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "direct ip with domain rule-set invert, ip hit", + rule: `{"ip_cidr": ["1.1.1.0/24"], "rule_set": ["cn-domain"], "invert": true, "action": "route", "server": "proxy"}`, + domain: "other.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "nested logical invert, ip miss", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"type": "logical", "mode": "or", "rules": [{"rule_set": ["cn-ip"]}]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "nested logical invert, ip hit", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"type": "logical", "mode": "or", "rules": [{"rule_set": ["cn-ip"]}]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + { + name: "nested logical and invert, domain hit ip miss", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"type": "logical", "mode": "and", "rules": [{"domain_suffix": ["lookup.example"]}, {"rule_set": ["cn-ip"]}]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "8.8.8.8", + expectPreMatch: true, + expectRouted: true, + }, + { + name: "nested logical and invert, domain hit ip hit", + rule: `{"type": "logical", "mode": "or", "invert": true, "rules": [{"type": "logical", "mode": "and", "rules": [{"domain_suffix": ["lookup.example"]}, {"rule_set": ["cn-ip"]}]}], "action": "route", "server": "proxy"}`, + domain: "lookup.example", + responseAddress: "1.1.1.5", + expectPreMatch: true, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + rule := addressFilterDNSRule(t, ctx, testCase.rule) + preMatched, routed := addressFilterFlow(rule, testCase.domain, testCase.responseAddress) + require.Equal(t, testCase.expectPreMatch, preMatched, "pre-lookup match") + require.Equal(t, testCase.expectRouted, routed, "routed") + }) + } +} diff --git a/route/rule/rule_item_rule_set.go b/route/rule/rule_item_rule_set.go index 3467843b..b286df60 100644 --- a/route/rule/rule_item_rule_set.go +++ b/route/rule/rule_item_rule_set.go @@ -49,14 +49,16 @@ func (r *RuleSetItem) matchStates(metadata *adapter.InboundContext) ruleMatchSta } func (r *RuleSetItem) matchStatesWithBase(metadata *adapter.InboundContext, base ruleMatchState) ruleMatchStateSet { - var stateSet ruleMatchStateSet + var stateSet, definitiveStateSet ruleMatchStateSet for _, ruleSet := range r.setList { nestedMetadata := *metadata nestedMetadata.ResetRuleMatchCache() nestedMetadata.IPCIDRMatchSource = r.ipCidrMatchSource nestedMetadata.IPCIDRAcceptEmpty = r.ipCidrAcceptEmpty stateSet = stateSet.merge(matchHeadlessRuleStatesWithBase(ruleSet, &nestedMetadata, base)) + definitiveStateSet = definitiveStateSet.merge(ruleMatchStateSet(nestedMetadata.DefinitiveMatchStates)) } + metadata.DefinitiveMatchStates = uint16(definitiveStateSet) return stateSet } diff --git a/route/rule/rule_set_local.go b/route/rule/rule_set_local.go index ee19cc45..16997355 100644 --- a/route/rule/rule_set_local.go +++ b/route/rule/rule_set_local.go @@ -211,11 +211,13 @@ func (s *LocalRuleSet) matchStates(metadata *adapter.InboundContext) ruleMatchSt } func (s *LocalRuleSet) matchStatesWithBase(metadata *adapter.InboundContext, base ruleMatchState) ruleMatchStateSet { - var stateSet ruleMatchStateSet + var stateSet, definitiveStateSet ruleMatchStateSet for _, rule := range s.rules { nestedMetadata := *metadata nestedMetadata.ResetRuleMatchCache() stateSet = stateSet.merge(matchHeadlessRuleStatesWithBase(rule, &nestedMetadata, base)) + definitiveStateSet = definitiveStateSet.merge(ruleMatchStateSet(nestedMetadata.DefinitiveMatchStates)) } + metadata.DefinitiveMatchStates = uint16(definitiveStateSet) return stateSet } diff --git a/route/rule/rule_set_remote.go b/route/rule/rule_set_remote.go index 031f1c33..7eacbaad 100644 --- a/route/rule/rule_set_remote.go +++ b/route/rule/rule_set_remote.go @@ -331,11 +331,13 @@ func (s *RemoteRuleSet) matchStates(metadata *adapter.InboundContext) ruleMatchS } func (s *RemoteRuleSet) matchStatesWithBase(metadata *adapter.InboundContext, base ruleMatchState) ruleMatchStateSet { - var stateSet ruleMatchStateSet + var stateSet, definitiveStateSet ruleMatchStateSet for _, rule := range s.rules { nestedMetadata := *metadata nestedMetadata.ResetRuleMatchCache() stateSet = stateSet.merge(matchHeadlessRuleStatesWithBase(rule, &nestedMetadata, base)) + definitiveStateSet = definitiveStateSet.merge(ruleMatchStateSet(nestedMetadata.DefinitiveMatchStates)) } + metadata.DefinitiveMatchStates = uint16(definitiveStateSet) return stateSet } diff --git a/route/rule/rule_set_semantics_test.go b/route/rule/rule_set_semantics_test.go index f1985015..8e1c3f3e 100644 --- a/route/rule/rule_set_semantics_test.go +++ b/route/rule/rule_set_semantics_test.go @@ -669,19 +669,24 @@ func TestDNSInvertAddressLimitPreLookupRegression(t *testing.T) { require.True(t, rule.MatchAddressLimit(&unmatchedMetadata)) }) } - t.Run("mixed resolved and deferred fields keep old pre lookup false", func(t *testing.T) { + t.Run("mixed resolved and deferred fields defer to response phase", func(t *testing.T) { t.Parallel() - metadata := testMetadata("lookup.example") rule := dnsRuleForTest(func(rule *abstractDefaultRule) { rule.invert = true addOtherItem(rule, NewNetworkItem([]string{N.NetworkTCP})) addDestinationIPCIDRItem(t, rule, []string{"203.0.113.0/24"}) }) - require.False(t, rule.Match(&metadata)) + preLookupMetadata := testMetadata("lookup.example") + require.True(t, rule.Match(&preLookupMetadata)) + matchedMetadata := testMetadata("lookup.example") + matchedMetadata.DestinationAddresses = []netip.Addr{netip.MustParseAddr("203.0.113.1")} + require.False(t, rule.MatchAddressLimit(&matchedMetadata)) + unmatchedMetadata := testMetadata("lookup.example") + unmatchedMetadata.DestinationAddresses = []netip.Addr{netip.MustParseAddr("8.8.8.8")} + require.True(t, rule.MatchAddressLimit(&unmatchedMetadata)) }) - t.Run("ruleset only deferred fields keep old pre lookup false", func(t *testing.T) { + t.Run("ruleset only deferred fields defer to response phase", func(t *testing.T) { t.Parallel() - metadata := testMetadata("lookup.example") ruleSet := newLocalRuleSetForTest("dns-ruleset-ipcidr", headlessDefaultRule(t, func(rule *abstractDefaultRule) { addDestinationIPCIDRItem(t, rule, []string{"203.0.113.0/24"}) })) @@ -689,7 +694,14 @@ func TestDNSInvertAddressLimitPreLookupRegression(t *testing.T) { rule.invert = true addRuleSetItem(rule, &RuleSetItem{setList: []adapter.RuleSet{ruleSet}}) }) - require.False(t, rule.Match(&metadata)) + preLookupMetadata := testMetadata("lookup.example") + require.True(t, rule.Match(&preLookupMetadata)) + matchedMetadata := testMetadata("lookup.example") + matchedMetadata.DestinationAddresses = []netip.Addr{netip.MustParseAddr("203.0.113.1")} + require.False(t, rule.MatchAddressLimit(&matchedMetadata)) + unmatchedMetadata := testMetadata("lookup.example") + unmatchedMetadata.DestinationAddresses = []netip.Addr{netip.MustParseAddr("8.8.8.8")} + require.True(t, rule.MatchAddressLimit(&unmatchedMetadata)) }) }