Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: [app/dns] Support per-client configuration #1977

Merged

Conversation

Vigilans
Copy link
Contributor

@Vigilans Vigilans commented Sep 12, 2022

This PR defines a new way to interpret DNS module and all its name servers:

Each nameserver (the client) could configure their options (e.g. tag, clientIP, queryStrategy) independently, while options specified in the DNS module are used as global default.

That is, a client for now could configure following options individually:

Inbound Tag
A special nameserver could name its inbound with a unique tag, thus made possible for domain-based DNS routing.
E.g. a server resolving geosite:bilibili gets named bilibili, and in routing it could be routed either directly or via HK proxy, to switch between different regions.

Client IP
Nameservers of same address could use different clientIP for different domains.

Fallback Strategy
A special nameserver serving geosite:bilibili may not expect to be used by other domains. Thus could be disabled from fallback individually.
On the other hand, when a domain matching geosite:bilibili, we also do not want the fallback servers to resolve it anymore, such could be set to disabledIfAnyMatch.

Cache Strategy
Each nameserver could disable cache individually.

Query Strategy
We could force certain domains to use only IPv4 or IPv6 address, if either route is preferred or with better quality.
A use case in transparent proxy:
In transparent proxy, we could specify all IPv6 packets go direct, but route all global IPv4 addresses to proxy (since when proxied, it normally doesn't matter whether dest is v4 or v6). Then, with queryStrategy, we could enforce all 1) global geosite domains, and 2) domains hit fallback server expecting non-CN IP to return only IPv4 IP, therefore gets routed to v2ray naturally.
This sorts of works like fakedns, but all resolved IPs are real, and v2ray does not take control of all traffic, only expected ones.

In this PR, following options are deprecated (but keep compatible with new options):

  • disableFallback, disableFallbackIfMatch, skipFallback: use fallbackStrategy.
  • disableCache: use cacheStrategy.

This PR also refactors the DNS module code, majorly to improve the readability and renders it more maintainable.

This PR does not introduce any change to existing features API. They'll be discussed and separated to future PRs.

@Vigilans Vigilans force-pushed the vigilans/dns-per-client-configuration branch 3 times, most recently from 8329c15 to e393017 Compare September 12, 2022 14:47
@Vigilans Vigilans force-pushed the vigilans/dns-per-client-configuration branch from e393017 to 1655224 Compare September 12, 2022 15:25
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2022

Codecov Report

Base: 38.88% // Head: 38.81% // Decreases project coverage by -0.06% ⚠️

Coverage data is based on head (0669047) compared to base (2e0ea88).
Patch coverage: 44.66% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1977      +/-   ##
==========================================
- Coverage   38.88%   38.81%   -0.07%     
==========================================
  Files         616      608       -8     
  Lines       36336    36502     +166     
==========================================
+ Hits        14128    14170      +42     
- Misses      20630    20763     +133     
+ Partials     1578     1569       -9     
Impacted Files Coverage Δ
app/dns/config.pb.go 11.16% <3.09%> (-1.27%) ⬇️
app/dns/nameserver.go 53.84% <53.84%> (+1.30%) ⬆️
infra/conf/synthetic/dns/dns.go 60.28% <56.66%> (-2.05%) ⬇️
app/dns/dns.go 56.27% <59.66%> (-0.61%) ⬇️
app/dns/config.go 80.64% <100.00%> (+12.22%) ⬆️
features/dns/client.go 100.00% <100.00%> (ø)
app/router/command/errors.generated.go 0.00% <0.00%> (-100.00%) ⬇️
common/strmatcher/matchers.go 64.78% <0.00%> (-14.38%) ⬇️
testing/servers/tcp/tcp.go 81.81% <0.00%> (-5.46%) ⬇️
testing/scenarios/common.go 75.00% <0.00%> (-4.29%) ⬇️
... and 39 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment on lines 150 to 168
// DisableCache disables DNS cache
bool disableCache = 8;
// Deprecated. Use cache_strategy.
bool disableCache = 8 [deprecated = true];

// Deprecated. Use fallback_strategy.
bool disableFallback = 10 [deprecated = true];

// Deprecated. Use fallback_strategy.
bool disableFallbackIfMatch = 11 [deprecated = true];

// Default query strategy (IPv4, IPv6, or both) for each name server.
QueryStrategy query_strategy = 9;

bool disableFallback = 10;
// Default cache strategy for each name server.
CacheStrategy cache_strategy = 12;

bool disableFallbackIfMatch = 11;
// Default fallback strategy for each name server.
FallbackStrategy fallback_strategy = 13;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems for now SimplifiedConfig and SimplifiedNameServer is not used in infra/conf yet, should these fields marked deprecated or directly changed to reserved ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think marking deprecated is better.

@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 14, 2022

Future works

Routing Strategy

This strategy aims to unify the three scenarios in routing dns inbound requests:

  • Default: sends to router normally.
  • SkipDNS: do not use internal DNS. Corresponds to session.Content.SkipDNSResolve.
  • Skip: do not use router. Corresponds to schemes like https+local://.

FakeDNS

Work on FakeDNS aims to remove FakeEnable member in dns.IPOption, ipOption and ctx member in DNS, and provide a new user-friendly way to configure FakeDNS.

Currently I may propose two ways and would like to know any of them would be a good idea:

Configure fakedns server objects independently from servers

Since fakedns is only enabled when using dns outbound, putting the fakedns server object along with other normal servers made the rules in servers hard to interpret when fakedns not enabled.

We may configure the fakedns server objects in a field sibling to servers field, and merge them to a full servers list when fakedns is enabled, at a certain priority:

"dns": {
    "tag": "dns",
    "servers": [
      {
        "address": "8.8.8.8",
        "domains": ["geosite:geolocation-!cn"]
      }
    ],
    "fakedns": {...}
}

Configure fakedns as an independent configuration in each nameserver object

As per FakeDNS | V2Fly.org and #789, integrating fakedns into servers is intended for rule-based dns resolution. In such case, it is the nameserver object that holds the rule semantic, but not the target DNS server address.

Therefore, we could integrate fakedns as a configurable option in a nameserver object, which will return fake ip instead of using original nameserver when fakedns is enabled (also a global default fakedns option):

"dns": {
    "tag": "dns",
    "servers": [
      {
        "address": "8.8.8.8",
        "domains": ["geosite:geolocation-!cn"],
        "fakedns": true
        // or 
        "fakedns": "xxx.xxx.xxx.xxx/xx"  // to return ips in a specific subnet?
      }
    ]
}

Geoip supports specifying IPv4 or IPv6 subsets (Backlog)

Currently, the expect IP "All ipv4 address and only CN ipv6 address" are conveyed as

expectIPs: ["0.0.0.0/0", "geoip:cn"] // ipv4 from geoip:cn just included in "0.0.0.0/0"

It would be better if we could express them as ["geoip:any@v4", "geoip:cn@v6"]
(Yes, also a new country code "any", where geoip:any@v4 = [0.0.0.0/0], geoip:any@v6 = [::/0])

Automatically creates a geosite:builtin domain list for all domains written in DNS and Outbound configs (Backlog)

Maintaining a list of all domains in v2ray config will help resolve many issues related to infinite loop.

Dispatch and forward non-A and AAAA requests (Backlog)

Currently, dns outbound only forwards A and AAAA requests to dns app, and forward all other requests to its original destination.

DNS app may also take other type of requests, but only does rules dispatching for them, and forwards the raw dns message to the dispatched destination.

Reserve original destination in DNS outbound (Backlog)

When dns outbound send the dns request to dns app, the latter does not hold any information about the original destination. Sometimes it may help (and simplify the config writing) if dns app has knowledge of the original destination.

@kslr
Copy link
Contributor

kslr commented Sep 16, 2022

cc @xiaokangwang

@AkinoKaede
Copy link
Contributor

AkinoKaede commented Nov 29, 2022

This is a very constructive PR and we should review the code and merge it as soon as possible.

@AkinoKaede
Copy link
Contributor

I reviewed it again.

…disableFallback`, `disableFallbackIfMatch`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants