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.
This commit is contained in:
+24
-15
@@ -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())
|
||||
}
|
||||
|
||||
+159
-1
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user