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

Add routeOnly sniffing option #1271

Closed
wants to merge 1 commit into from

Conversation

nekohasekai
Copy link
Member

Allows the sniffed domain to be used for routing only, without overriding the destination address. This improves the routing accuracy of AsIs, and provides the expected connection behavior of the client (not resolving the domain name again on the server side), which adds a degree of privacy protection when connecting to uncontrolled servers.

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2021

Codecov Report

Merging #1271 (600e369) into master (c58a372) will decrease coverage by 0.00%.
The diff coverage is 9.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1271      +/-   ##
==========================================
- Coverage   44.72%   44.72%   -0.01%     
==========================================
  Files         485      485              
  Lines       29465    29482      +17     
==========================================
+ Hits        13179    13186       +7     
- Misses      14884    14896      +12     
+ Partials     1402     1400       -2     
Impacted Files Coverage Δ
app/dispatcher/default.go 27.72% <0.00%> (-0.85%) ⬇️
app/proxyman/config.pb.go 18.34% <0.00%> (-0.17%) ⬇️
app/proxyman/inbound/worker.go 37.60% <0.00%> (-0.46%) ⬇️
common/session/session.go 70.58% <ø> (ø)
features/routing/dns/context.go 41.17% <0.00%> (ø)
infra/conf/v2ray.go 47.77% <0.00%> (-0.14%) ⬇️
transport/internet/config.pb.go 16.34% <ø> (ø)
features/routing/session/context.go 65.00% <50.00%> (-1.67%) ⬇️
transport/internet/kcp/connection.go 72.37% <0.00%> (+0.55%) ⬆️
transport/internet/kcp/listener.go 83.16% <0.00%> (+1.98%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c58a372...600e369. Read the comment docs.

@database64128
Copy link
Contributor

This option can be useful if you want to make sure the target the server is going to dial is the target your client resolver resovled. But please note that it does not provide any "privacy protection", as the server operator can still easily sniff out the SNI or intercept DNS traffic if you're using plain unencrypted DNS. It also does not provide "the expected connection behavior of the client", because you essentially prevented server-side happy eyeballs from happening, and it's impossible to properly implement happy eyeballs from the client side of a proxy.

@wloot
Copy link

wloot commented Sep 16, 2021

当路由策略使用IPIfNonMatch,在域名匹配失败后尝试ip匹配,这里是使用嗅探出的域名来解析ip还是target原本的目标连接ip?
手机上不好查看更改,但似乎GetTargetIPs()还是原来的逻辑,我认为应该改成routeonly情况优先返回target的ip。

Copy link
Contributor

@database64128 database64128 left a comment

Choose a reason for hiding this comment

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

What about always setting RouteTarget no matter RouteOnly is true or false? Then Router would only have to look at RouteTarget when picking a route. This makes the router code cleaner, less likely to break in the future. And it's no longer necessary to modify GetTargetIPs().

@xiaokangwang
Copy link
Contributor

Instead of adding a new field and option, why not just add the fakedns result to the attribute of the connection. We will need to support things like routing by process name in the future, we can make a minor infrastructure change here to make it easier to add more routing attributes later.

@@ -241,7 +245,11 @@ func (d *DefaultDispatcher) Dispatch(ctx context.Context, destination net.Destin
domain := result.Domain()
newError("sniffed domain: ", domain).WriteToLog(session.ExportIDToError(ctx))
destination.Address = net.ParseAddress(domain)
ob.Target = destination
if sniffingRequest.RouteOnly && result.Protocol() != "fakedns" {
Copy link
Contributor

@dyhkwong dyhkwong Dec 18, 2021

Choose a reason for hiding this comment

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

Traffic will be dispatched to fake IPs if fakeDNS enabled, RouteOnly enabled and both meta and content sniffing success.

Suggested change
if sniffingRequest.RouteOnly && result.Protocol() != "fakedns" {
protocol := result.Protocol()
if resultComposite, ok := result.(SnifferResultComposite); ok {
protocol = resultComposite.ProtocolForDomainResult()
}
if sniffingRequest.RouteOnly && protocol != "fakedns" {

@4-FLOSS-Free-Libre-Open-Source-Software
Copy link
Contributor

This option can be useful if a client uses https altsvc. Which leads to wrong IP if destOverride is enabled.

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

7 participants