Closed fonqL closed 4 months ago
虽然我自己review了十几次了大概没什么问题但还是简单测试一下比较好,比如会不会崩溃、有没有效果之类的。(我发现这个bug就是因为跑了一周抓了一百多人全都是绝对判定,一个相对判定的也没有)。
检查的过程中发现原来的 AddPeerMap 的实现也有问题... 因为这里的 PeerInfo 是以 IP 标识, 但正确实现还应结合 TorrentHash, 所以可以考虑去除 peerMap, 并把 peerMap 改为通过 torrentMap 实现. torrentMap[torrentHash].TorrentSize 和 torrentMap[torrentHash].Peers 这样会较好, 1. 节省同时存储 torrentMap 的开销; 2. 解决此前的问题;
但若这样操作, 意味着无法检查到一个 IP 在多个 Torrent 的多个端口, 因此, 可在 ipMap 引入 Port, 并将 MaxIPPortCount 检测移入 CheckAllIP.
则最后 CheckAllPeer 也应更名为 CheckAllTorrent, 这些是一个比较大但因为上述 bug 和优化项的存在, 有必要的改动.
Edit: 一个额外问题, 单个 Peer 相同 IP/端口, 相同 TorrentHash, 到客户端同时有多个连接, 这是一个需要避免的罕见问题. 但还没想到好方法.
嗯, 所以这样一来, 需要修复就不只是标题的内容了, 比较多, 见上述内容.
已对之前的部分 Commit 进行构建, 相信成品已经出来了 (https://github.com/Simple-Tracker/qBittorrent-ClientBlocker/actions/runs/8409306315), 不过最终改进完毕后再测试也许比较好.
我们有发布 Public Beta 的习惯, 因此最后合并改动的公开测试都可以于 Beta 中进行. (尽管对于 Docker, 它会取代 latest)
虽然有点离题,但是修一个小bug。 https://github.com/Simple-Tracker/qBittorrent-ClientBlocker/blob/2c0b8ddd66e3a3a5f7d15b9156b285e173934bf0/console.go#L80-L84 对局部变量的修改没有意义。猜测意图是修改map中的值。
没有怎么写过 Go, 所以有太多疏忽和错误了! 此问题可和上述问题一起修复了 (这个 PR 的标题完全也可以进行扩展).
这个 bug 好在不太影响使用, 因为若仍存在很快又会重新进入封禁列表.
对于MaxIPPort检测,维护一个 IP:Ports 映射就好了吧(类型大概是 map[string][int]bool
这样),不用考虑Torrent。维护方式:在双重遍历torrents-peers时,map[peer.ip][peer.port]=true即可。(然后记得定时清理就没什么问题了)
对于全局的IP流量检测,在配置上只要一个ipUpCheckIncrementMB
就行了吧,ipUpCheckPerTorrentRatio
似乎是不需要的。一个ip可以同时下载多个torrent,这个ratio应该乘哪个torrentSize,这个问题思路走不通啊。
所以似乎全局的IP流量检测也不需要torrent信息吧,维护一个IP:int64映射就好了吧。在双重遍历torrents-peers时,map[peer.ip] += peer.uploaded 就好了吧。(然后记得定时清理就没什么问题了)
对于MaxIPPort检测,维护一个 IP:Ports 映射就好了吧(类型大概是
map[string][int]bool
这样),不用考虑Torrent。维护方式:在双重遍历torrents-peers时,直接塞进map即可。(然后定时清理之类的应该没什么问题)对于全局的IP流量检测,在配置上只要一个
ipUpCheckIncrementMB
就行了吧,ipUpCheckPerTorrentRatio
似乎是不需要的。一个ip可以同时下载多个torrent,这个ratio应该乘哪个torrentSize,这个问题思路走不通啊。 所以似乎全局的IP流量检测也不需要torrent信息吧,维护一个IP:int64映射就好了吧。在双重遍历torrents-peers时,map[peer.ip] += peer.uploaded 就好了吧。(然后定时清理之类的应该没什么问题)
理想上的设定是: IP 在任一 Torrent 的倍率超限视为超限, 总的超限也视为超限. 但是的, ipMap 确实不需要 Torrent 信息, 这一操作可移动至 CheckAllTorrent 中完成, 在其中可调用 CheckIPPerTorrentUpload 等.
有一点需要注意的是: 不应该依赖 qB 的实际上传量的统计, 因为恶意操作者可断开连接 (这可能会清空统计). 因此, 这是原有的 peerMapCleanInterval 的存在原因, 而现在应移植到 torrentMap 上.
(注: 我们正在等一个 PR 或在遥远的未来自行实现 Transmission 支持, 因此可以做保守假定.)
不依赖qb汇报的上传量有道理,但是这样的话统计怎么做呢? 比如上次轮询得到上传量10M,下次轮询得到上传量5M,这肯定是断开过连接的。 如果下次轮询得到上传量15M,clientblocker自己维护的统计量应该是15还是10+15呢?
如果 qb汇报上传量 > clientBlocker统计量 则直接=qb的量;否则肯定断开过连接,应+=qb的量。 这样计算?
不依赖qb汇报的上传量有道理,但是这样的话统计怎么做呢? 比如上次轮询得到上传量10M,下次轮询得到上传量5M,这肯定是断开过连接的。 如果下次轮询得到上传量15M,clientblocker自己维护的统计量应该是15还是10+15呢?
如果 qb汇报上传量 > clientBlocker统计量 则直接=qb的量;否则肯定断开过连接,应+=qb的量。 这样计算?
在两次期间用这种逻辑应该会比没有好, 另外若 API 有提供连接日期, 还可以此作判断. 这个更多的属于优化项目.
因此, 这是原有的 peerMapCleanInterval 的存在原因, 而现在应移植到 torrentMap 上.
我对于此设计表示还请三思,应该做必要的隔离。显而易见的是,全局IP流量检测如果维护自己独一份的数据结构,则十分有利于程序的低耦合、少bug、可读性,写起来也简单。对于内存之类的性能问题本来就是有检测、有反馈才会着手优化解决的,而一个隔离性好、低耦合、易理解的程序也方便社区的PR和日后可能的重构。提前优化是万恶之源啊。(碎碎念:实际上我建议应该思考一下怎么分mod了,全塞main mod里然后全局变量飞来飞去实在是太美了,看到GenBlockPeersStr没有参数然后里面引用blockPeerMap看麻了,这不就是个转成字符串吗给我用参数啊喂)
我之所以有点意见是因为,当我尝试修改代码时,我发现追踪peerMap的数据流,即何时读何时写何时修改,是超级困难的一件事情。主要原因就是peerMap即承担了相对检测又承担了MaxIPPort检测的职责,又复制到lastPeerMap上作为参数传走,又要clean。这三又用的是同一个间隔(peerMapCleanInterval)。一个很(不)明显的bug就是,
我不想torrentMap成为第二个peerMap。每个检测配置应该都是不相干的才对。torrentMap就专注于保存上一次的轮询信息就可以了。
因此, 这是原有的 peerMapCleanInterval 的存在原因, 而现在应移植到 torrentMap 上.
我对于此设计表示还请三思,应该做必要的隔离。显而易见的是,全局IP流量检测如果维护自己独一份的数据结构,则十分有利于程序的低耦合、少bug、可读性,写起来也简单。对于内存之类的性能问题本来就是有检测、有反馈才会着手优化解决的,而一个隔离性好、低耦合、易理解的程序也方便社区的PR和日后可能的重构。提前优化是万恶之源啊。(碎碎念:实际上我建议应该思考一下怎么分mod了,全塞main mod里然后全局变量飞来飞去实在是太美了,看到GenBlockPeersStr没有参数然后里面引用blockPeerMap看麻了,这不就是个转成字符串吗给我用参数啊喂)
我之所以有点意见是因为,当我尝试修改代码时,我发现追踪peerMap的数据流,即何时读何时写何时修改,是超级困难的一件事情。主要原因就是peerMap即承担了相对检测又承担了MaxIPPort检测的职责,又复制到lastPeerMap上作为参数传走,又要clean。这三又用的是同一个间隔(peerMapCleanInterval)。一个很(不)明显的bug就是,
- lastPeerMap实际上对peerMap是直接引用,并没有复制。
- peerMapCleanInterval<interval时,行为是,peerMap是有值的,MaxIPPort检测是运作的,但lastPeerMap永远是空的,相对检测完全不运作。从用户角度上来说这太奇怪了.
我不想torrentMap成为第二个peerMap。每个检测配置应该都是不相干的才对。torrentMap就专注于保存上一次的轮询信息就可以了。
全局 IP 流量检测当然是单独的, 但 torrentMap 现在承担了 peer 的存储功能, 因此对单个 Torrent 做的 PerRatio 检测才应移植到其上. 这也是为了避免重复实现. torrentMap 并不需要进行任何改动, 因为 PerRatio 检测是顺便做的, 而不是专门做的, 但为了区分度, 所以才会考虑 CheckAllTorrent 里调用 CheckIPPerTorrentUpload 等以使逻辑清晰. torrentMap 显然也是合理的存储 torrentSize 的地方, 所以结合 torrentSize 和 Uploaded 就可以实现 PerRatio 检测.
相当于实际上是将 ipCheck 的功能拆成两部分, 在适合做这一个功能的位置放入, 可能有点类似于 hook.
嗯, 使用 mod 的思路确实是不错的. 不过还是有必要共享一些全局变量. 对于全局变量, 不用传入参数某种程度上是合理的.
以前这些文件都是在一个 main.go 里的, 更地狱 后面我们在一次 PR 中发现, 这样会影响代码可读性及影响社区 PR, 并极大概率的造成合并冲突, 因此合并后即拆分了相关模块.
现在的逻辑是分模块存放, 不同的模块在不同的文件, 管理它们自己的变量和方法. 其它模块可以互访一些全局变量来共享信息. GenBlockPeersStr 和 blockPeerMap 是这里的一个不恰当使用 (但若于拆分之前, 同模块间互访应该是恰当的), 但对于其它一些方法, 这样的共享可能有助于减少方法本身的定义过多造成的混乱. 而在模块化后, 模块理论上是可以有共同的前置模块, 这个依赖可以通过一系列方法甚至自循环等实现相关变量的维护和调用. (前置模块或称依赖模块, 可以读写变量. 约定只允许依赖该前置模块的模块访问这些变量, 但只读.)
最后, peerMap 和 lastPeerMap 之间并没有使用 & 引用, 因此默认行为应该是复制而非引用? peerMapCleanInterval < interval 是用户配置不当的一个典型实例, 在 InitConfig 时会对一些必要配置强制作出纠正, 而非必要配置则由用户自己发现和纠正.
在此次 PR 的修复和优化完成后, 若仍有想法, 欢迎后续单独再开一个 PR 用于将其逻辑进一步拆分/细化/优化存取逻辑!
lastPeerMap=peerMap
,这对引用类型(go是这么叫的吗)来说是浅拷贝。
lastPeerMap=peerMap
,这对引用类型(go是这么叫的吗)来说是浅拷贝。
看起来这和类型有关, 小小的反了一下直觉. (参考: https://www.ssgeek.com/post/golang-jie-gou-ti-lei-xing-de-shen-qian-kao-bei/#3-结构体的深拷贝)
那就一并修复吧! 这一项目已经破旧不堪了
要修复的是lastIPMap和ipMap吗?
ps. lastTorrentMap := torrentMap
实际上是正确的。因为第二步是torrentMap = make(map[string]TorrentMapStruct)
。然后torrentMap就拿去填充新的值了。所以逻辑是正确的。
要修复的是lastIPMap和ipMap吗? ps.
lastTorrentMap := torrentMap
实际上是正确的。因为第二步是torrentMap = make(map[string]TorrentMapStruct)
。然后torrentMap就拿去填充新的值了。所以逻辑是正确的。
是的, 不太确定问题是不是仅限于此. peerMap 由于需要被替换而废弃则无需进行修复.
然而ipMap似乎为全局IP流量判断的监控所用。这模块也是要被重写的吧,所以也不用改了(?
所以总结一下, 简要的说本次调整.
peerMap 关联于 IP, 原来实现的功能有:
而 torrentMap 的话:
ipMap 关联于 IP, 目前需要实现的功能有:
所以 peerMap/torrentMap/ipMap 分别实现的子功能是互不干涉的, 但 torrentMap 的 IPCheck (PerTorrent) 加上 ipMap 的 IPCheck (Global) 才形成对用户来说完整的 IP 上传检测功能 (单种和全局).
那么接下来说问题:
反正从上述的细节到这里的总结, 都有点乱. 希望能大致传达到具体的问题和目标所在.
上面这部分功能适合作为其中一个 PR, 而在此 PR 完成后, 重构这些又可以成为另一个 PR 进行, 这样就达成了 修复/优化 和 拆分/重构 互不冲突. 先 修复/优化 再 拆分/重构, 单 PR 复杂度会降低很多.
最后提一下 拆分/重构 的小设想:
我们可新建一 mod 文件夹, 放入各种不同的封禁模块 (绝对/相对/IP 等等), 下再放入一文件夹 dep 作为 mod 所需前置依赖, 如 torrentMap/ipMap 等等, dep 同时会声明一些操作方法, mod 可以读 dep 变量, 而其它组件只应通过 dep 内的 func 去写变量.
其后, 我们可以在 console 主逻辑不同部分, 我们在不同的合适位置插入 mod 方法来实现其功能.
最后, 理想上来说, mod 本身还应可向 config 注册其配置项.
假设攻击者以一定间隔轮换 Peer, 若没有一个周期而只是依靠 Interval 极短的间隔可能是无法找到 lastPeer 的.
只要攻击者连接时间大于间隔时间,那么肯定找的到lastpeer从而被相对屏蔽探测到。所以短间隔比长间隔好,相对检测周期理应等同于程序轮询周期。 你说的情况应该是,攻击者遵纪守法的下完一个torrent后,断开连接,等待qb不再记录对他的上传数据,然后再次发起连接,遵纪守法的下完一个torrent,然后重复无限次。 这本来就是跨周期检测(不依赖qb信息的检测),这种高级功能就交给全局IP流量统计就好了。
假设攻击者以一定间隔轮换 Peer, 若没有一个周期而只是依靠 Interval 极短的间隔可能是无法找到 lastPeer 的.
只要攻击者连接时间大于间隔时间,那么肯定找的到lastpeer从而被相对屏蔽探测到。所以短间隔比长间隔好,相对检测周期理应等同于程序轮询周期。 你说的情况应该是,攻击者遵纪守法的下完一个torrent后,断开连接,等待qb不再记录对他的上传数据,然后再次发起连接,遵纪守法的下完一个torrent,然后重复无限次。 这本来就是跨周期检测(不依赖qb信息的检测),这种高级功能就交给全局IP流量统计就好了。
已在最新的 Commit 中实现除本次算法改动 (不包括修复 https://github.com/Simple-Tracker/qBittorrent-ClientBlocker/issues/39#issuecomment-2016484672) 外的上述功能 (大概, https://github.com/Simple-Tracker/qBittorrent-ClientBlocker/pull/45#issuecomment-2016853035), 暂未测试.
如果在一个连接时间内表现出恶意(乱跳进度条),而且被相对机制抓到(即连接持续时间大于轮询间隔),则直接被绝对/相对屏蔽机制杀死。 所以攻击者在一个连接时间内必须“遵纪守法”,除非他连接时间够短,比如连接1s就断开。但他断开后还必须等待qbt忘记他之前存在过(之前记录过他的uploaded)才能再次发起连接。这也有力的削弱了攻击者能力。 所以相对屏蔽间隔理应等于程序轮询间隔
如果在一个连接时间内表现出恶意(乱跳进度条),而且被相对机制抓到(即连接持续时间大于轮询间隔),则直接被绝对/相对屏蔽机制杀死。 所以攻击者在一个连接时间内必须“遵纪守法”,除非他连接时间够短,比如连接1s就断开。但他断开后还必须等待qbt忘记他之前存在过(之前记录过他的uploaded)才能再次发起连接。这也有力的削弱了攻击者能力。 所以相对屏蔽间隔理应等于程序轮询间隔
攻击者可以通过更换 Port 的方式实现快速清空 (我们的判断逻辑不关心 Port), 且不应依赖于 qB 不立即忘记;
qb到底是不是会记住上传量确实是我的臆断了。重新整理一下思路,,, 对同一ip下的多端口轮换下载,首先用maxipport限制住端口数,超出端口数就会ban ip。maxipport是应该有自己的清理间隔的。如果端口数量合格,对反复建立连接的下载行为检测,我称之为跨连接检测。(实际上maxipport也属于这种) 跨连接检测当然是有一定复杂度的,体现在新的数据到来时,他不能立刻替换掉旧有数据,而是要积攒起来。即需要自己的清理间隔。 我的分支代码中,绝对检测和相对检测都是仅针对一个连接下的检测,体现在我直接使用qbt相邻两次汇报量,它们在设想中基于同一个连接。你正在试图实现的是把相对检测改造成跨连接检测:对一个torrent,不直接使用qbt的汇报量,而是自行维护每个peer的上传和进度(维护方式只是往map里写入新值但有效,因为延迟了清理)从而实现peer的追踪。 在概念上的不清晰让我有了些误会抱歉,那我也没什么意见了,但很容易发现你愿景中torrentMap实际上就是一种全局的统计,不过不是针对ip,而是针对torrent下的peer。 另外,需要注意的是,仍然需要在每个程序执行间隔执行相对检查。这与清理间隔无关。
简单描述一下torrentMap和lastTorrentMap行为:遍历新爬下来的torrent.peers,对每个peers查lastmap并执行相对检查。最后把peers信息写入lastmap?实际上我们只需要一个map!(需要对清理间隔小于interval时提醒用户相对检查不起作用,但这符合配置的直觉)。我发现从我的分支仍然可以轻松实现。
我这么理解对吗,但是“我们的判断逻辑不关心 Port”我仍然不知道指的是什么。如果不对请简述一下您预想的torrentmap行为
ps. 周期太短的0进度问题实际上被很多阈值挡住了所以没问题。你看我IsProgressNotMatchUploaded_Relative函数里有一堆判断条件。
攻击者可以通过更换 Port 的方式实现快速清空 (我们的判断逻辑不关心 Port), 且不应依赖于 qB 不立即忘记;
qb到底是不是会记住上传量确实是我的臆断了。重新整理一下思路,,, 对同一ip下的多端口轮换下载,首先用maxipport限制住端口数,超出端口数就会ban ip。maxipport是应该有自己的清理间隔的。如果端口数量合格,对反复建立连接的下载行为检测,我称之为跨连接检测。(实际上maxipport也属于这种) 跨连接检测当然是有一定复杂度的,体现在新的数据到来时,他不能立刻替换掉旧有数据,而是要积攒起来。即需要自己的清理间隔。 我的分支代码中,绝对检测和相对检测都是仅针对一个连接下的检测,体现在我直接使用qbt相邻两次汇报量,它们在设想中基于同一个连接。你正在试图实现的是把相对检测改造成跨连接检测:对一个torrent,不直接使用qbt的汇报量,而是自行维护每个peer的上传和进度(维护方式只是往map里写入新值但有效,因为延迟了清理)从而实现peer的追踪。 在概念上的不清晰让我有了些误会抱歉,那我也没什么意见了,但很容易发现你愿景中torrentMap实际上就是一种全局的统计,不过不是针对ip,而是针对torrent下的peer。 另外,需要注意的是,仍然需要在每个程序执行间隔执行相对检查。这与清理间隔无关。
简单描述一下torrentMap和lastTorrentMap行为:遍历新爬下来的torrent.peers,对每个peers查lastmap并执行相对检查。最后把peers信息写入lastmap?实际上我们只需要一个map!(需要对清理间隔小于interval时提醒用户相对检查不起作用,但这符合配置的直觉)。我发现从我的分支仍然可以轻松实现。
我这么理解对吗,但是“我们的判断逻辑不关心 Port”我仍然不知道指的是什么。如果不对请简述一下您预想的torrentmap行为
ps. 周期太短的0进度问题实际上被很多阈值挡住了所以没问题。你看我IsProgressNotMatchUploaded_Relative函数里有一堆判断条件。
把相对检测改造成跨连接检测
相对检测自一开始就是依赖于自身维护而非 qB 维护的状态, 因此并没有改造, 而是进行了修复.
需要注意的是,仍然需要在每个程序执行间隔执行相对检查。这与清理间隔无关。
周期太短的 0 进度问题实际上被很多阈值挡住了所以没问题
TorrentSize=1000MB, 客户端历史上传 100MB, 大于 BanByRelativePUStartMB = 10. 在随后的 6 秒甚至用户手动配置的 1 秒内上传了 40 MB, 进度变化为 0.00%, 随后客户端符合相对屏蔽规则被封禁. 并没有看出哪里挡住了. BanByRelativePUStartPrecent = 2 是针对实际上传量的百分比变化而非 Peer 报告的进度变化. BanByRelativePUAntiErrorRatio = 3 是无效的, 若 Progress = 0, 与其相乘任何值都会是 0, 下面已代入, 可以确认一下.
func IsProgressNotMatchUploaded_Relative(torrentTotalSize int64, progress float64, lastProgress float64, uploaded int64, lastUploaded int64) bool {
// 与IsProgressNotMatchUploaded保持一致
if config.BanByRelativeProgressUploaded && 1000 > 0 && (0.00-0.00) >= 0 && (140-100) > 0 {
// 若客户端对 Peer 上传已大于 0, 且相对上传量大于起始上传量, 则继续判断.
relativeUploaded := 140-100
relativeDownloaded := int64(float64(1000) * (0.00-0.00))
if relativeUploaded > int64(10) {
// 若相对上传百分比大于起始百分比, 则继续判断.
if relativeUploaded > int64(float64(1000) * float64(2 / 100) {
// 若相对上传百分比大于 Peer 报告进度乘以一定防误判倍率, 则认为 Peer 是有问题的.
if relativeUploaded > relativeDownloaded * int64(3) {
return true
}
}
}
}
return false
}
换言之:
StartMB 量级小, 所以实际上大头往往是 StartPrecent, 而这两个参数的调整激进与保守对 1/2 是互相冲突的, 调小会改善 1 但会影响 2 (反之). 而周期通过一段时间的数据累计来使 1 安全工作, 同时进而使迟滞相比总量之差小到可使 2 安全工作. 通过暴露周期的间隔设置, 用户也可以很轻松的采取激进与否的手段来减小初判断周期.
若客户端在百分比很高的情况下出现了 0 进度变动, 那么绝对屏蔽的开销是超级大和效果低的 (若 Peer 报告下载进度与设置倍率及 Torrent 大小之乘积得到之下载量 比 客户端上传量 还低, 则允许屏蔽 Peer
). 因此, 相对屏蔽依旧要能够处理 0 进度问题.
遍历新爬下来的 torrent.peers
是的, 这可以是一个优化点, 但前提是不累计. 我们希望累计客户端周期内的所有端口, 并用于屏蔽 (只有非 IP 屏蔽不可的情况, IP 屏蔽才应被使用), 考虑 NAT 网络环境. 而且, 这会增加耦合, 使 CheckAllTorrent 必须被拆分.
若使用新式屏蔽方式 (目前未启动, 因增加损耗), 则屏蔽需要的信息即是 IP+端口 (不去揣测 qB 的实际屏蔽行为). 我们未启用的设置项 banAllPort
可以在新式屏蔽方式上改变这一点.
我们的判断逻辑不关心 Port
peer 的 key 均使用 IP, 与 Port 无关, 攻击者更换 Port 可以清空 qB 的上传量, 但不能清空屏蔽器的上传量, 还会使屏蔽器记录 PortCount += 1.
由于本次 PR 改动离上述目标相差很大, 因此可以考虑从最新的 Commit 开始尝试回应用更改, 因已尝试进行上述除算法外的绝大多数改动.
我去,peer的进度报告居然是这样的吗,我还以为传一份数据就报告一下,这是bt协议的弊端了吧,考虑到UDP传输的话甚至是不可靠的,也就是说不仅进度报告的延迟无限高,而且顺序都不确定:有可能peer先报告了5%然后报告10%,但是在网络传输中10%的进度报告数据包比5%的进度报告数据包更先到达我这里,导致我认为peer的进度从10%变成5%怎么往回走了,于是判定为异常行为所以ban掉,即使这是因为网络问题。。 另外,进度报告的高延迟同时也直接使得绝对检测失效,因为在上传量超过BanByProgressUploaded和BanByPUStartPrecent后,如果同样出现超高延迟的进度报告(比如发现还是进度0%)则同样会触发绝对屏蔽,即使这是网络问题。实际上这里有个issue指出情况确实存在,延迟甚至可以达到两分钟。。那边给出的解决方案是综合其他更多信息,而不是单靠progress和uploaded。 世界毁灭了,整个增强机制完全不可用了。
我去,peer的进度报告居然是这样的吗,我还以为传一份数据就报告一下,这是bt协议的弊端了吧,考虑到UDP传输的话甚至是不可靠的,也就是说不仅进度报告的延迟无限高,而且顺序都不确定:有可能peer先报告了5%然后报告10%,但是在网络传输中10%的进度报告数据包比5%的进度报告数据包更先到达我这里,导致我认为peer的进度从10%变成5%怎么往回走了,于是判定为异常行为所以ban掉,即使这是因为网络问题。。 另外,进度报告的高延迟同时也直接使得绝对检测失效,因为在上传量超过BanByProgressUploaded和BanByPUStartPrecent后,如果同样出现超高延迟的进度报告(比如发现还是进度0%)则同样会触发绝对屏蔽,即使这是网络问题。实际上这里有个issue指出情况确实存在,延迟甚至可以达到两分钟。。那边给出的解决方案是综合其他更多信息,而不是单靠progress和uploaded。 世界毁灭了,整个增强机制完全不可用了。
这是我结合观察做的假设.
首先对端 Peer 下载后需要校验, 下载本身需要时间 (但上传却已开始发生变化), 校验即意味着接收到不意味着进度立马增加, 同时校验也需要时间, 最后校验偶尔会失败 (多种原因导致的偶发正常现象). 在校验完成后, 对端 Peer 要更新自己的进度并送回, 这其中又要涉及到一个单向网络延迟的问题 (弱网更明显). 所以应该假设有一定的延迟.
进度报告应该是可靠的 (除非对端客户端伪造, 但这本来就是我们针对的内容), 尽管 UDP 本身无状态, 但建立于上的 uTP 协议应可以控制顺序 (数据排序重组等), 否则将连文件正确性都无法保证.
综合其它信息的前提是: 有其它信息可用. 很显然, 我们并没有其它信息, 因此, 开启增强屏蔽必定有概率误封一些其它客户端, 但我们认为这是可以接受的权衡.
所以我们的做法是: 考虑一般弱网并兼顾大部分客户端, 并且不考虑特殊弱网和 bug 客户端被误封, 同时为用户提供选择上的灵活性.
我还没想好怎么比较满意的应对延迟问题,PR先关了。
39
修复
IsProgressNotMatchUploaded_Relative
的错误实现,进行了最低限度的重构,引入的破坏性更新有:CheckAllPeer()
和peerMap
现在仅用于MaxIPPort(IP端口数限制)的判断。这意味着PeerMapCleanInterval
与相对屏蔽机制无关。现在的相对屏蔽机制是,程序每次轮询下来的数据都会同上一次轮询下来的数据进行对比。在代码上值得注意的更新有:
AddPeerInfo()
现在与BanByRelativeProgressUploaded
无关CheckPeer
现在还需要负责相对性检查,并且不再负责维护其他屏蔽方式所需的信息。(代码上是AddIPInfo
和AddPeerInfo
的调用)。维护责任移交给Task()
。CheckAllPeer()
在端口数过多的情况下没有自增局部变量blockCount。lastPeerMap
,引入torrentMap
和lastTorrentMap
作为增强自动屏蔽_相对需要维护的信息。关于在内存上的担忧,目前觉得没有必要,因为没有看到有关qBittorrent-ClientBlocker内存占用的反馈。此外,为了内存而牺牲正确性实在是捡了芝麻丢了西瓜。