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

proxy dns with NOTIMP error #2595

Closed
wants to merge 1 commit into from
Closed

proxy dns with NOTIMP error #2595

wants to merge 1 commit into from

Conversation

robberphex
Copy link

@robberphex robberphex commented Jul 8, 2023

Chromium will send SVCB/HTTPS type dns query, which cause dns leak.

To prvent dns leak,

  • If the dns query type is not implemented, should return NOTIMP error, not just send data to original destination.
  • If the dns query has wrong format domain, should return FORMERR error, not send data to original destination.

@AkinoKaede
Copy link
Contributor

Thanks, I think we need to discuss a more reasonable solution.

@robberphex
Copy link
Author

Thanks, I think we need to discuss a more reasonable solution.

Yes, but how? This has put the user in danger for years.

@dyhkwong
Copy link
Contributor

dyhkwong commented Jul 9, 2023

To prevent DNS leak, why not override destination address and configure proxySettings for dns outbound to make non-A/AAAA traffic routable?

@robberphex
Copy link
Author

To prevent DNS leak, why not override destination address and configure proxySettings for dns outbound to make non-A/AAAA traffic routable?

  1. Is there an example for this?
  2. non-A/AAAA dns request will always to remote server, which is time-costing. And return NOTIMP error is reasonable.
  3. v2ray provide route solution for "customized routing", throw HTTPS dns query will cause client/chromium waiting and use dns response. Returning NOTIMP can make client use dns response from v2ray.

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (5c995d9) 38.29% compared to head (1f28093) 38.29%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2595   +/-   ##
=======================================
  Coverage   38.29%   38.29%           
=======================================
  Files         637      637           
  Lines       38122    38147   +25     
=======================================
+ Hits        14599    14610   +11     
- Misses      21911    21929   +18     
+ Partials     1612     1608    -4     
Impacted Files Coverage Δ
proxy/dns/dns.go 63.75% <0.00%> (-8.31%) ⬇️

... and 8 files with indirect coverage changes

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

@robberphex
Copy link
Author

robberphex commented Jul 16, 2023

@AkinoKaede @dyhkwong proxySettings only has tag and transportLayer field, is not captiable to split flow by domains.

For example, Chromium will send SVCB/HTTPS query for every domain, some chinese domain should only query in local network.

And currently, we don't have much resource to implement SVCB/HTTPS query type, and we cannot implement every query on time in futrue. So let's fllow spec, return NOTIMP.

Is there any other way to prevent this kind of dnsleak, and don't put the user in danger?

@AkinoKaede
Copy link
Contributor

@AkinoKaede @dyhkwong proxySettings only has tag and transportLayer field, is not captiable to split flow by domains.

For example, Chromium will send SVCB/HTTPS query for every domain, some chinese domain should only query in local network.

And currently, we don't have much resource to implement SVCB/HTTPS query type, and we cannot implement every query on time in futrue. So let's fllow spec, return NOTIMP.

Is there any other way to prevent this kind of dnsleak, and don't put the user in danger?

Another developer suggested configuring the proxySettings to route requests not supported by dns app to the blackhole.

@rp-hello
Copy link

@AkinoKaede @dyhkwong proxySettings only has tag and transportLayer field, is not captiable to split flow by domains.
For example, Chromium will send SVCB/HTTPS query for every domain, some chinese domain should only query in local network.
And currently, we don't have much resource to implement SVCB/HTTPS query type, and we cannot implement every query on time in futrue. So let's fllow spec, return NOTIMP.
Is there any other way to prevent this kind of dnsleak, and don't put the user in danger?

Another developer suggested configuring the proxySettings to route requests not supported by dns app to the blackhole.

I meet same problem, Is there any updates for proxySettings? If not, I think we could merge this to keep user safe.

@dyhkwong
Copy link
Contributor

dyhkwong commented Oct 23, 2023 via email

Copy link
Contributor

It has been open 120 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the Stale label Mar 26, 2024
@robberphex robberphex closed this by deleting the head repository Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants