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)) }) }