Skip to content

Comments

Refactoring: DNS App#169

Merged
Loyalsoldier merged 3 commits intov2fly:masterfrom
Vigilans:vigilans/dns-refactoring
Dec 18, 2020
Merged

Refactoring: DNS App#169
Loyalsoldier merged 3 commits intov2fly:masterfrom
Vigilans:vigilans/dns-refactoring

Conversation

@Vigilans
Copy link
Contributor

@Vigilans Vigilans commented Sep 8, 2020 •
edited
Loading

DNS模块的实现似乎由于历史原因与Go自身的问题，在耦合度、代码复用上有一些不足之处。这个PR的目标是重构DNS模块，以：

  • 精简server.go中DNS核心代码，避免过长的函数，将功能独立的代码抽出，降低耦合度。
  • 修正部分issues中提及的DNS行为问题。

主要改动介绍如下，详细的改动内容与改动理由将在Review中给出：

Name Server

File Name Change

  • LocalNameServer 从 nameserver.go 中抽出来，放入 nameserver_local.go。
  • udpns.go -> nameserver_udp.go
  • dohdns.go ->nameserver_doh.go

Name Server & Client

Hosts

DNS

Vigilans
Vigilans commented Sep 8, 2020
View reviewed changes
app/dns/nameserver.go Outdated
Comment on lines -18 to 30
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

原Client接口更名为Server接口。Server不再保存clientIP，而是在调用QueryIP时经过参数clientIP传递，以更强调该对象是远程服务器的抽象的概念。

原Client接口的几个实现是：LocalNameServer，ClassicNameServer, DOHNameServer。这几个NameServer类实现的却是Client接口，容易令人混淆。

由于本PR里Client有了新的定义，因此将原Client更名为Server接口。不选择NameServer是因为该名字被proto文件里的Config类占用了。

app/dns/nameserver.go Outdated
Comment on lines 42 to 65
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

server.go中的New -> addNameServer函数实现被移入了NewServer中并得到了简化，以增强可读性和维护性。

app/dns/nameserver.go Outdated
Comment on lines +32 to 38
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

新的Client类将原server.go中与单个Client相关的代码抽象了出来，且成员布局与DNS JSON配置中的ServerObject一致。也即每个ServerObject的信息会对应一个Client对象。

server.go中与config.NameServers, config.NameServer有关的处理逻辑，queryIPTimeout与match函数的实现被移入了Client中。

每个Client中独立保存了clientIP，因此未来可以支持为每个ServerObject独立配置一个Client IP（在TG群里看过这个需求）

Vigilans
Vigilans commented Sep 8, 2020
View reviewed changes
app/dns/hosts.go Outdated
Comment on lines 93 to 107
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

server.go中的lookupStatic迁移到了hosts.go中，并作为StaticHosts提供的公开接口使用。

同时严谨定义StaticHosts.Lookup的行为：

  1. Lookup可能返回nil，此时代表域名没有被收录进StaticHosts中。
  2. Lookup可能返回一个且最多一个域名，此时代表查询到了一个域名替换。Lookup会默认以最多5次展开这个域名，返回展开到的IP或最后无法继续展开的域名。
  3. Lookup可能返回零至多个IP。返回的IP数组一定不为nil，返回空数组代表Lookup有查询结果，但只是被全过滤掉了。
    • lookupInternal返回了多于一个domain也会在这里被处理，经过filterIP后被过滤掉，仍然只有IP返回。关于StaticHosts仍有一段内容需要更新，但不适合放在该重构PR里，会在后面PR补上。

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Sep 8, 2020 •
edited
Loading

希望新增两个配置：

  • 能为每个 DNS 都设置 clientIP (EDNS Client Subnet)，而不是所有 DNS 同一个 clientIP 以实现多次设置同一个 DNS 而配以不同 clientIP 来获取不同地区的解析结果
  • 能设置每个 DNS 是否同时查询 A 类型、AAAA 类型，或者只查询某一个类型

@Vigilans Vigilans mentioned this pull request Sep 8, 2020
Closed
@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 8, 2020 •
edited
Loading

@Loyalsoldier

能为每个 DNS 都设置 clientIP (EDNS Client Subnet)，而不是所有 DNS 同一个 clientIP 以实现多次设置同一个 DNS 而配以不同 clientIP 来获取不同地区的解析结果

目前的 Refactoring 在 App 这边功能上已经有了支持，细节问题有：

  • DNS Client 的 Name() 函数目前直接返回其管理的 Server 的 Name()。是否有必要补充上 clientIP 以在日志中显示 clientIP 来区分不同的 Client ？（可以在 clientIP 为空时不打印）
  • 同服务器地址，不同 clientIP 的 Client 是否应该共享同一份 Server 实例。这从语义上来说是合适的，实现上可能需要将 Server 按不同的 ClientIP 分别维护缓存，以及其他未知细节。这个可以先搁置，观察具体的效果，有了新的需求后再说。

能设置每个 DNS 是否同时查询 A 类型、AAAA 类型，或者只查询某一个类型

这个可以在App端这样实现：Client 维护一个新成员 clientOption IPOption，在调用 Client.QueryIP(..., option) 时，option 与 clientOption 按与计算合并成新的 IPOption，再发给 Server 从而控制查询请求。细节问题是需要 Server 处理好在 IPOption.IPv6Enable 与 IPOption.IPv4Enable 均为 false 时候的行为，从而让 DNS 的轮询能顺利进行下去。

以上是两者在 App 端实现方案的大致设计，Config 端如何设计按你的想法来（面向用户一端应该需要更有经验的人来给提案）。

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Sep 8, 2020 •
edited
Loading

@Vigilans

个人认为，DNS log 是否打印 clientIP 无所谓，因为目前已有对应的 idx。其余实现怎么方便怎么来。

IPOption 似乎是 freedom outbound 用于设置 domainStrategy 的，默认为 AsIs，另外可选项为 UseIP、UseIPv4、UseIPv6。我觉得可以仿照这个作为选项。默认什么都不设置（或设置 UseIP），同时查询 A 和 AAAA；设置 UseIPv4，查询 A 记录；设置 UseIPv6 查询 AAAA 记录；用 queryStrategy 作为 key:

"dns": {
    "servers": [{
        "address": "https+local://223.5.5.5/dns-query",
        "domains": ["geosite:cn", "geosite:icloud"],
        "expectIPs": ["geoip:cn"],
        "queryStrategy": "UseIPv4",
        "clientip": "123.123.123.123"
    }]
}

优先使用 DNS 内设置的 clientIP，否则使用统一设置的 clientIP（如果外层设置了的话）。这样应该能在不破坏已有配置可用性的情况下增加更多细致的选项。

Vigilans
Vigilans commented Sep 8, 2020
View reviewed changes
app/dns/dns.go Outdated
Comment on lines +23 to +32
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

因为nameserver.go中实现了Server接口，原server.go中的Server类直接改名为DNS并迁移到dns.go中。这样与JSON 配置的对应关系也更清晰些。

原Server类中的许多成员也合并到了Client类中，降低了耦合度。这避免了如 #146 这样的问题，其中ipMatcher本应是和Client一比一绑定的关系，但却分成了两个数组，又因两个数组的元素对应不一致而出错。

app/dns/dns.go Outdated
Comment on lines 72 to 100
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

DNS.New()通过以下几种函数创建Client对象：

  • NewClient(...)以新版NameServer配置创建；
  • NewSimpleClient(...)以旧版Endpoint配置创建；
  • NewLocalDNSClient()创建一个维护LocalNameServer的Client。(实现于nameserver_local.go中）

app/dns/dns.go Outdated
Comment on lines +166 to +178
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

Static Hosts 匹配行为改动。参考 #169 (comment) ，对匹配结果addrs的处理流程如下：

  1. addrs为nil时，代表 Static Hosts 查询失败，domain未被收录，继续向下使用 Name Servers 查询。
  2. addrs不为nil，但为空数组时，代表 Static Hosts 收录了 domain，但返回了空结果。此时直接返回dns.ErrEmptyResponse，不继续向下查询 Name Servers。
  3. addrs仅有一个结果且为域名时，代表 Static Hosts 返回了一个替换域名（像是CNAME？），用新域名替换原 domain 继续向下查询 Name Servers。
  4. 默认情况下，均认为 Staic Hosts 返回了 IP 记录，尝试将 addrs 转换为 IP 数组并返回。若转换失败则返回对应的错误。

其中主要的行为变化是条件 2，将nil与空数组作了区分处理。非nil的空数组可能是因为 Static Hosts 中有添加一条 IPv4 记录，但在只有 option.IPv6Enable 为真的情况下被过滤了。这种时候应该认为 Static Hosts 有对应 domain 的记录，但没有收录 IPv6 IP，因此对 IPv6 的查询请求应该返回 Empty Response。这解决了 v2ray/domain-list-community#487 (comment) 中碰到的问题。

app/dns/dns.go Outdated
Comment on lines +180 to +195
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

Name Servers匹配行为改动。原来的先优先查询，后默认轮询被整合为了以下流程：

  1. 按照优先级顺序对 Clients 进行排序。也即待查的 Clients 总数不变，且不会重复查询。
  2. 按照排序后的 Clients 对其进行轮询。

流程 1 解决了 #156 (comment) 中的问题：由于现在查询的是 Clients 经过优先级重排后的结果，因此自然一个 Client 只会被使用一次。
流程 2 解决了 #156 (comment) 的其中一个问题：由于现在是统一在重排后的 Clients 上作轮询了，优先查询与默认轮询的处理逻辑自然地被合并了，而不是使用两份一样的代码。

对于 #156 的本来问题：如何控制允许DNS继续查询的逻辑，这个重构PR没有改动。这部分应留给原 Issue 继续进行讨论，得出共识方案后由他人开PR修正。

app/dns/dns.go Outdated
Comment on lines +200 to +237
Copy link
Contributor Author

@Vigilans Vigilans Sep 8, 2020

Choose a reason for hiding this comment

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

sortClients 的主要逻辑：

  • 先处理优先匹配，后对 Clients 遍历一遍
  • 使用clientUsed数组判断是否重复使用

从而构造一个按优先级排序后的 Clients 数组。

这里对于 domainRules 与 matchedDNS 日志赋予了新的定义：

  • domainRules 数组会打印出匹配的所有规则，尽管某些规则可能对应了同一个 DNS。
  • matchedDNS 变为 clientNames 数组，打印重排后将用于查询的所有 DNS 顺序。若采用了 per-DNS client IP, 这里有必要为 DNS 额外记录 clientIP 以消歧义。

@Vigilans Vigilans marked this pull request as ready for review September 8, 2020 13:50
@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 8, 2020 •
edited
Loading

@kslr @Loyalsoldier 等到周五新版本发布后再考虑这个PR？这样避免合进去后可能会出的问题，也方便跟后面的DNS更新算到一起去。

@kslr
Copy link
Contributor

kslr commented Sep 8, 2020

@kslr @Loyalsoldier 等到周五新版本发布后再考虑这个PR？这样避免合进去后可能会出的问题，也方便跟后面的DNS更新算到一起去。

好的，另外在什么情况下会需要 queryStrategy

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Sep 8, 2020

好的，另外在什么情况下会需要 queryStrategy

就是只希望 DNS 查询 A 记录或者 AAAA 记录

@kslr
Copy link
Contributor

kslr commented Sep 8, 2020 •
edited
Loading

好的，另外在什么情况下会需要 queryStrategy

就是只希望 DNS 查询 A 记录或者 AAAA 记录

这有什么具体场景吗

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Sep 8, 2020

好的，另外在什么情况下会需要 queryStrategy

就是只希望 DNS 查询 A 记录或者 AAAA 记录

这有什么具体场景吗

没有 IPv6 网络，没必要查 AAAA 记录，仅此而已 😂

@CalmLong
Copy link
Contributor

CalmLong commented Sep 12, 2020

提个请求

假设将 V2Ray 放在一台具有公网 IP 的服务器上作为 DNS 服务器使用，那么能否将连接 V2Ray 用户的 IP "自动添加" 到 clientIp，然后再进行 DNS 查询

@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 13, 2020 •
edited
Loading

@CalmLong 判断机器是否有直接的公网IP对我个人来说感觉是比较tricky的问题……怎么知道是公网IP，有两张以上网卡如何处理，感觉要考虑的细节挺多的。我个人是在脚本中curl一些返回IP的公共服务来获取的公网IP，对如何在代码中用成熟的手段处理没有什么经验……（个人也对edns-client-subnet的细节了解不多，以后自己折腾到这里的话可能会来研究下）

如果要实现的话，由于感觉上会额外地与OS产生交互，引入一些额外依赖，感觉不适合放在core中，实现在v2ctl中可能比较合适：在解析JSON配置时通过某个模块获取公网IP，填入解析好的protobuf配置，然后再送给v2ray-core。

@CalmLong
Copy link
Contributor

CalmLong commented Sep 13, 2020

@Vigilans 那 access.log 中的用户的公网 IP 是怎么获取的，不能直接用吗

虽然用户大多都是内网 IP，但是家庭宽带连接带有 V2Ray 的服务器的时候的 IP 是公网 IP 呀

@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 13, 2020 •
edited
Loading

@Vigilans 那 access.log 中的用户的公网 IP 是怎么获取的，不能直接用吗

@CalmLog 我的access.log是这样的（透明代理）：

2020/09/08 08:16:14 192.168.200.9:52817 accepted udp:192.168.200.1:53 [dns-out] 
2020/09/08 08:16:44 192.168.200.7:53798 accepted tcp:58.216.45.242:80 [direct] 
2020/09/11 02:57:11 192.168.200.11:28885 accepted tcp:3.224.18.4:443 [proxy]

源IP还是路由器内网下分配的IP，并没有拿到路由器面向公网的IP。你说的用户的公网IP是指源IP还是目标IP？

@CalmLong
Copy link
Contributor

CalmLong commented Sep 13, 2020

我感觉你没明白我的意思

假设本机 V2Ray 开个的任意门协议，然后监听 53 端口用来接收 DNS 查询的流量，假设本机 IP 是 192.168.1.1

但是目前 clientIp 只能设置一个，在 DNS 查询的时候起作用

假设我用另一台 IP 为 192.168.1.2 的机器 DNS 地址设置为 192.168.1.1

这时候 V2Ray 可以知道是 192.168.1.2 这台机器连接了，然后把 192.168.1.2 这个 IP 当作 clientIp 来用

这里把上述的IP改为公网 IP 就是我想表达的意思

@Vigilans

@CalmLong
Copy link
Contributor

CalmLong commented Sep 13, 2020

源IP

@CalmLong
Copy link
Contributor

CalmLong commented Sep 13, 2020

@Vigilans 那 access.log 中的用户的公网 IP 是怎么获取的，不能直接用吗

@CalmLog 我的access.log是这样的（透明代理）：

2020/09/08 08:16:14 192.168.200.9:52817 accepted udp:192.168.200.1:53 [dns-out] 
2020/09/08 08:16:44 192.168.200.7:53798 accepted tcp:58.216.45.242:80 [direct] 
2020/09/11 02:57:11 192.168.200.11:28885 accepted tcp:3.224.18.4:443 [proxy]

源IP还是路由器内网下分配的IP，并没有拿到路由器面向公网的IP。你说的用户的公网IP是指源IP还是目标IP？

比如上面的 192.168.200.7 这个 IP，当从公网访问 V2Ray 的时候这里就显示公网 IP 而不是内网 IP 了

当前系统的 IP 地址，用于 DNS 查询时，通知服务器客户端的所在位置。不能是私有地址。

对于访问来源是内网 IP 的这种，我觉得反正内网 IP 是有限的，可以识别然后跳过

@kslr
Copy link
Contributor

kslr commented Sep 13, 2020

这个实在太魔法了，你还是单独开一个 issue 再讨论吧

@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 13, 2020 •
edited
Loading

@CalmLong 了解你的意思了……确实我会错意了，之前只往Client角度去想了。

意思是使用了V2Ray的VPS，同时使用V2Ray开放了一个DNS服务，对于从公网送来的查询请求，VPS以请求方的公网IP填入clientIP送给第三方公共DNS查询。

这个想法感觉挺合理的，相对Client开DNS只需要一个Public IP作clientIP，Server的这种可变的clientIP更有意义。内网使用V2Ray DNS查询时，可以fallback到JSON配置里设置的client IP。

在实现上DNS Feature需要重构，调用DNS Lookup时需要能拿到source ip，这个不放在这个DNS App PR做。

@Loyalsoldier 你感觉这个Feature request怎么样？从App角度来说可以实现，需不需要以及面向用户该如何设计配置看你们的讨论了（在另开的Issue回吧）

@CalmLong CalmLong mentioned this pull request Sep 13, 2020
Closed
@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Sep 13, 2020

没意见

@kslr
Copy link
Contributor

kslr commented Sep 14, 2020

我认为不应该把 core 放到公网上当dns服务，太简单了，延伸出的问题更多。

p.s 你准备要合并了吗

@Vigilans
Copy link
Contributor Author

Vigilans commented Sep 15, 2020

@kslr 我想把Routing Stats的几个PR推进了再来考虑DNS这边的PR，因为在此之后DNS Stats或许也可以考虑加上了……

@Vigilans Vigilans force-pushed the vigilans/dns-refactoring branch 3 times, most recently from 8f293f9 to 9e97919 Compare September 27, 2020 06:46
@Vigilans Vigilans force-pushed the vigilans/dns-refactoring branch from 9e97919 to 0df05a1 Compare September 27, 2020 07:06
@1dd9a1f7-36b6
Copy link

1dd9a1f7-36b6 commented Oct 8, 2020

@Vigilans 我有个建议，能否添加并行DNS查询？

@Loyalsoldier Loyalsoldier mentioned this pull request Oct 10, 2020
Closed
@LearZhou
Copy link

LearZhou commented Oct 10, 2020

发现有些域名仅有AAAA，没有A，遇到这种情况若DNS仅查询A的话将没有返回值，此时如何匹配路由规则？

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Oct 13, 2020

#309

@Ardentwheel
Copy link

Ardentwheel commented Oct 22, 2020 •
edited
Loading

好的，另外在什么情况下会需要 queryStrategy

就是只希望 DNS 查询 A 记录或者 AAAA 记录

在路由透明代理里，查询到AAAA记录是不通的，因为ipv6没法不开net的情况下代理。或许可以？但我不知道怎么做

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Dec 2, 2020

#482

@CalmLong
Copy link
Contributor

CalmLong commented Dec 8, 2020

这个PR还不打算合并吗

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Dec 8, 2020

这个PR还不打算合并吗

This is an unfinished PR.

@Loyalsoldier Loyalsoldier force-pushed the vigilans/dns-refactoring branch from 301675e to 043d78c Compare December 18, 2020 08:52
@Loyalsoldier Loyalsoldier force-pushed the vigilans/dns-refactoring branch from 043d78c to 380c018 Compare December 18, 2020 09:13
@Loyalsoldier Loyalsoldier merged commit d8c03f1 into v2fly:master Dec 18, 2020
@kslr
Copy link
Contributor

kslr commented Dec 18, 2020 •
edited
Loading

别啊，怎么给合并了？ 这没有做完也没有充分测试怎么就进master了

@Vigilans
Copy link
Contributor Author

Vigilans commented Dec 18, 2020

😱
Drown in DDLs 😰

@Loyalsoldier
Copy link
Contributor

Loyalsoldier commented Dec 18, 2020

在本地测试了几回，没啥毛病，就先合了（心急如焚

@kslr
Copy link
Contributor

kslr commented Dec 18, 2020 •
edited
Loading

StaticHosts 可能需要测试一下

#529

@Fangliding Fangliding mentioned this pull request Apr 29, 2025
Merged
ttrobot0101 pushed a commit to ttrobot0101/v2ray-core that referenced this pull request Aug 17, 2025
@mzz2017
fix: cannot work on windows/macos v2fly#169
2e390c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Loyalsoldier Loyalsoldier Loyalsoldier approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Vigilans @Loyalsoldier @kslr @CalmLong @1dd9a1f7-36b6 @LearZhou @Ardentwheel