timamsk/sing-box
1
0
Fork 0
mirror of https://github.com/SagerNet/sing-box.git synced 2026-04-14 04:38:28 +10:00
Code Issues Packages Projects Releases Wiki Activity

Fix DNS pre-match CIDR fail-closed semantics

Browse Source
This commit is contained in:
世界
2026-03-24 22:26:20 +08:00
parent 04dd0961c4
commit 58334cfd3c
6 changed files with 194 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
adapter/inbound.go
View File

@@ -99,12 +99,11 @@ type InboundContext struct {
IPCIDRMatchSource bool
IPCIDRAcceptEmpty bool
SourceAddressMatch bool
SourcePortMatch bool
DestinationAddressMatch bool
DestinationPortMatch bool
DidMatch bool
IgnoreDestinationIPCIDRMatch bool
SourceAddressMatch bool
SourcePortMatch bool
DestinationAddressMatch bool
DestinationPortMatch bool
DidMatch bool
}
func (c *InboundContext) ResetRuleCache() {

121
dns/_repro_test.go
View File

@@ -1,121 +0,0 @@
package dns
import (
"context"
"errors"
"net/netip"
"testing"
"github.com/sagernet/sing-box/adapter"
C "github.com/sagernet/sing-box/constant"
"github.com/sagernet/sing-box/option"
"github.com/sagernet/sing/common/json/badoption"
mDNS "github.com/miekg/dns"
"github.com/stretchr/testify/require"
)
func TestReproLookupWithRulesIgnoresRouteStrategy(t *testing.T) {
defaultTransport := &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP}
router := newTestRouter(t, []option.DNSRule{
{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{
Domain: badoption.Listable[string]{"example.com"},
},
DNSRuleAction: option.DNSRuleAction{
Action: C.RuleActionTypeEvaluate,
RouteOptions: option.DNSRouteActionOptions{Server: "default"},
},
},
},
{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{
MatchResponse: true,
},
DNSRuleAction: option.DNSRuleAction{
Action: C.RuleActionTypeRoute,
RouteOptions: option.DNSRouteActionOptions{Server: "selected", Strategy: C.DomainStrategyIPv4Only},
},
},
},
}, &fakeDNSTransportManager{
defaultTransport: defaultTransport,
transports: map[string]adapter.DNSTransport{
"default": defaultTransport,
"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
},
}, &fakeDNSClient{
exchange: func(transport adapter.DNSTransport, message *mDNS.Msg) (*mDNS.Msg, error) {
if transport.Tag() == "default" {
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("1.1.1.1")}, 60), nil
}
switch message.Question[0].Qtype {
case mDNS.TypeA:
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("2.2.2.2")}, 60), nil
case mDNS.TypeAAAA:
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("2001:db8::1")}, 60), nil
default:
return nil, errors.New("unexpected qtype")
}
},
})
addrs, err := router.Lookup(context.Background(), "example.com", adapter.DNSQueryOptions{})
require.NoError(t, err)
require.Equal(t, []netip.Addr{netip.MustParseAddr("2.2.2.2")}, addrs)
}
func TestReproLogicalMatchResponseIPCIDR(t *testing.T) {
transportManager := &fakeDNSTransportManager{
defaultTransport: &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
transports: map[string]adapter.DNSTransport{
"upstream": &fakeDNSTransport{tag: "upstream", transportType: C.DNSTypeUDP},
"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
"default": &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
},
}
client := &fakeDNSClient{
exchange: func(transport adapter.DNSTransport, message *mDNS.Msg) (*mDNS.Msg, error) {
switch transport.Tag() {
case "upstream":
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("1.1.1.1")}, 60), nil
case "selected":
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("8.8.8.8")}, 60), nil
default:
return nil, errors.New("unexpected transport")
}
},
}
rules := []option.DNSRule{
{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{Domain: badoption.Listable[string]{"example.com"}},
DNSRuleAction: option.DNSRuleAction{Action: C.RuleActionTypeEvaluate, RouteOptions: option.DNSRouteActionOptions{Server: "upstream"}},
},
},
{
Type: C.RuleTypeLogical,
LogicalOptions: option.LogicalDNSRule{
RawLogicalDNSRule: option.RawLogicalDNSRule{
Mode: C.LogicalTypeOr,
Rules: []option.DNSRule{{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{MatchResponse: true, IPCIDR: badoption.Listable[string]{"1.1.1.0/24"}},
},
}},
},
DNSRuleAction: option.DNSRuleAction{Action: C.RuleActionTypeRoute, RouteOptions: option.DNSRouteActionOptions{Server: "selected"}},
},
},
}
router := newTestRouter(t, rules, transportManager, client)
response, err := router.Exchange(context.Background(), &mDNS.Msg{Question: []mDNS.Question{fixedQuestion("example.com", mDNS.TypeA)}}, adapter.DNSQueryOptions{})
require.NoError(t, err)
require.Equal(t, []netip.Addr{netip.MustParseAddr("8.8.8.8")}, MessageToAddresses(response))
}

142
dns/repro_test.go Normal file
View File

@@ -0,0 +1,142 @@
package dns
import (
"context"
"errors"
"net/netip"
"testing"
"github.com/sagernet/sing-box/adapter"
C "github.com/sagernet/sing-box/constant"
"github.com/sagernet/sing-box/option"
"github.com/sagernet/sing/common/json/badoption"
mDNS "github.com/miekg/dns"
"github.com/stretchr/testify/require"
)
func TestReproLookupWithRulesIgnoresRouteStrategy(t *testing.T) {
t.Parallel()
defaultTransport := &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP}
router := newTestRouter(t, []option.DNSRule{
{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{
Domain: badoption.Listable[string]{"example.com"},
},
DNSRuleAction: option.DNSRuleAction{
Action: C.RuleActionTypeEvaluate,
RouteOptions: option.DNSRouteActionOptions{Server: "default"},
},
},
},
{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{
MatchResponse: true,
},
DNSRuleAction: option.DNSRuleAction{
Action: C.RuleActionTypeRoute,
RouteOptions: option.DNSRouteActionOptions{
Server: "selected",
Strategy: option.DomainStrategy(C.DomainStrategyIPv4Only),
},
},
},
},
}, &fakeDNSTransportManager{
defaultTransport: defaultTransport,
transports: map[string]adapter.DNSTransport{
"default": defaultTransport,
"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
},
}, &fakeDNSClient{
exchange: func(transport adapter.DNSTransport, message *mDNS.Msg) (*mDNS.Msg, error) {
if transport.Tag() == "default" {
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("1.1.1.1")}, 60), nil
}
switch message.Question[0].Qtype {
case mDNS.TypeA:
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("2.2.2.2")}, 60), nil
case mDNS.TypeAAAA:
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("2001:db8::1")}, 60), nil
default:
return nil, errors.New("unexpected qtype")
}
},
})
addrs, err := router.Lookup(context.Background(), "example.com", adapter.DNSQueryOptions{})
require.NoError(t, err)
require.Equal(t, []netip.Addr{netip.MustParseAddr("2.2.2.2")}, addrs)
}
func TestReproLogicalMatchResponseIPCIDR(t *testing.T) {
t.Parallel()
transportManager := &fakeDNSTransportManager{
defaultTransport: &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
transports: map[string]adapter.DNSTransport{
"upstream": &fakeDNSTransport{tag: "upstream", transportType: C.DNSTypeUDP},
"selected": &fakeDNSTransport{tag: "selected", transportType: C.DNSTypeUDP},
"default": &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP},
},
}
client := &fakeDNSClient{
exchange: func(transport adapter.DNSTransport, message *mDNS.Msg) (*mDNS.Msg, error) {
switch transport.Tag() {
case "upstream":
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("1.1.1.1")}, 60), nil
case "selected":
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("8.8.8.8")}, 60), nil
default:
return nil, errors.New("unexpected transport")
}
},
}
rules := []option.DNSRule{
{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{
Domain: badoption.Listable[string]{"example.com"},
},
DNSRuleAction: option.DNSRuleAction{
Action: C.RuleActionTypeEvaluate,
RouteOptions: option.DNSRouteActionOptions{Server: "upstream"},
},
},
},
{
Type: C.RuleTypeLogical,
LogicalOptions: option.LogicalDNSRule{
RawLogicalDNSRule: option.RawLogicalDNSRule{
Mode: C.LogicalTypeOr,
Rules: []option.DNSRule{{
Type: C.RuleTypeDefault,
DefaultOptions: option.DefaultDNSRule{
RawDefaultDNSRule: option.RawDefaultDNSRule{
MatchResponse: true,
IPCIDR: badoption.Listable[string]{"1.1.1.0/24"},
},
},
}},
},
DNSRuleAction: option.DNSRuleAction{
Action: C.RuleActionTypeRoute,
RouteOptions: option.DNSRouteActionOptions{Server: "selected"},
},
},
},
}
router := newTestRouter(t, rules, transportManager, client)
response, err := router.Exchange(context.Background(), &mDNS.Msg{
Question: []mDNS.Question{fixedQuestion("example.com", mDNS.TypeA)},
}, adapter.DNSQueryOptions{})
require.NoError(t, err)
require.Equal(t, []netip.Addr{netip.MustParseAddr("8.8.8.8")}, MessageToAddresses(response))
}

8
route/rule/rule_abstract.go
View File

@@ -56,11 +56,11 @@ 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 {
return !metadata.IgnoreDestinationIPCIDRMatch && !metadata.IPCIDRMatchSource && len(r.destinationIPCIDRItems) > 0
return !metadata.IPCIDRMatchSource && len(r.destinationIPCIDRItems) > 0
}
func (r *abstractDefaultRule) requiresSourceAddressMatch(metadata *adapter.InboundContext) bool {
@@ -156,10 +156,6 @@ func (r *abstractDefaultRule) matchStatesWithBase(metadata *adapter.InboundConte
return r.invertedFailure(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 {
return emptyRuleMatchState().withBase(inheritedBase)
}
return 0
}
return stateSet

1
route/rule/rule_dns.go
View File

@@ -365,7 +365,6 @@ func (r *DefaultDNSRule) matchStatesForMatch(metadata *adapter.InboundContext) r
return r.abstractDefaultRule.matchStates(&matchMetadata)
}
matchMetadata := *metadata
matchMetadata.IgnoreDestinationIPCIDRMatch = true
return r.abstractDefaultRule.matchStates(&matchMetadata)
}

49
route/rule/rule_set_semantics_test.go
View File

@@ -616,6 +616,47 @@ func TestDNSRuleSetSemantics(t *testing.T) {
require.False(t, metadata.IPCIDRMatchSource)
require.False(t, metadata.IPCIDRAcceptEmpty)
})
t.Run("pre lookup ruleset only deferred fields fail closed", func(t *testing.T) {
t.Parallel()
metadata := testMetadata("lookup.example")
ruleSet := newLocalRuleSetForTest("dns-prelookup-ipcidr", headlessDefaultRule(t, func(rule *abstractDefaultRule) {
addDestinationIPCIDRItem(t, rule, []string{"203.0.113.0/24"})
}))
rule := dnsRuleForTest(func(rule *abstractDefaultRule) {
addRuleSetItem(rule, &RuleSetItem{setList: []adapter.RuleSet{ruleSet}})
})
require.False(t, rule.Match(&metadata))
})
t.Run("pre lookup ruleset destination cidr does not fall back to other predicates", func(t *testing.T) {
t.Parallel()
metadata := testMetadata("lookup.example")
ruleSet := newLocalRuleSetForTest("dns-prelookup-network-and-ipcidr", headlessDefaultRule(t, func(rule *abstractDefaultRule) {
addOtherItem(rule, NewNetworkItem([]string{N.NetworkTCP}))
addDestinationIPCIDRItem(t, rule, []string{"203.0.113.0/24"})
}))
rule := dnsRuleForTest(func(rule *abstractDefaultRule) {
addRuleSetItem(rule, &RuleSetItem{setList: []adapter.RuleSet{ruleSet}})
})
require.False(t, rule.Match(&metadata))
})
t.Run("pre lookup mixed ruleset still matches non response branch", func(t *testing.T) {
t.Parallel()
metadata := testMetadata("www.example.com")
ruleSet := newLocalRuleSetForTest(
"dns-prelookup-mixed",
headlessDefaultRule(t, func(rule *abstractDefaultRule) {
addOtherItem(rule, NewNetworkItem([]string{N.NetworkTCP}))
addDestinationIPCIDRItem(t, rule, []string{"203.0.113.0/24"})
}),
headlessDefaultRule(t, func(rule *abstractDefaultRule) {
addDestinationAddressItem(t, rule, nil, []string{"example.com"})
}),
)
rule := dnsRuleForTest(func(rule *abstractDefaultRule) {
addRuleSetItem(rule, &RuleSetItem{setList: []adapter.RuleSet{ruleSet}})
})
require.True(t, rule.Match(&metadata))
})
}
func TestDNSMatchResponseRuleSetDestinationCIDRUsesDNSResponse(t *testing.T) {
@@ -751,7 +792,7 @@ func TestDNSInvertAddressLimitPreLookupRegression(t *testing.T) {
require.True(t, rule.MatchAddressLimit(&unmatchedMetadata, dnsResponseForTest(testCase.unmatchedAddrs...)))
})
}
t.Run("mixed resolved and deferred fields keep old pre lookup false", func(t *testing.T) {
t.Run("mixed resolved and deferred fields invert matches pre lookup", func(t *testing.T) {
t.Parallel()
metadata := testMetadata("lookup.example")
rule := dnsRuleForTest(func(rule *abstractDefaultRule) {
@@ -759,9 +800,9 @@ func TestDNSInvertAddressLimitPreLookupRegression(t *testing.T) {
addOtherItem(rule, NewNetworkItem([]string{N.NetworkTCP}))
addDestinationIPCIDRItem(t, rule, []string{"203.0.113.0/24"})
})
require.False(t, rule.Match(&metadata))
require.True(t, rule.Match(&metadata))
})
t.Run("ruleset only deferred fields keep old pre lookup false", func(t *testing.T) {
t.Run("ruleset only deferred fields invert matches pre lookup", func(t *testing.T) {
t.Parallel()
metadata := testMetadata("lookup.example")
ruleSet := newLocalRuleSetForTest("dns-ruleset-ipcidr", headlessDefaultRule(t, func(rule *abstractDefaultRule) {
@@ -771,7 +812,7 @@ func TestDNSInvertAddressLimitPreLookupRegression(t *testing.T) {
rule.invert = true
addRuleSetItem(rule, &RuleSetItem{setList: []adapter.RuleSet{ruleSet}})
})
require.False(t, rule.Match(&metadata))
require.True(t, rule.Match(&metadata))
})
}