peer blacklist

[Kolcha]

added custom peer blacklist support as counterpart of previously added in #238 peer whitelist.

closes #190 , closes #223 , closes #226 , closes #248 , closes #51 ,
very likely few more issues can be listed here, but it is pretty hard to me to find them...

blacklist rules are placed in external file 'peer_blacklist.txt', located in qBittorrent data directory ($HOME/.local/share/qBittorrent in case of Linux). if there is NO such file plugin is not activated. it is completely safe to keep empty file - this just means "nothing blacklisted". the same rules syntax and logic are used as for whitelist, but just for blacklisting.

for example, to blacklist client mentioned in #190 , next line can be added to blacklist:

-GT\d{4}-    .*

in this case .* means "any client name", i.e. "ban peer by id". of course, client name matching pattern (or exact name can be added). to ban peer by client name regardless its id, just place .* instead of peer id matching pattern. beware, .* in both fields means "blacklist everything"!

info about banned peers is saved in the same table in peers.db SQLite database, but only with tag "blacklist".

blacklist can co-exists with whitelists, but this is meaningless. in any case, blacklist rules take precedence. as whitelist, blacklist is applied for all torrents, not only public.

[c0re100]

Ah...
I am creating peer whitelist and drafted peer whitelist description recently, maybe I can slow it down or discard it now lol

[Kolcha]

as I understand correctly from discussion in #190 , whitelist caused some mess. sorry...
so, I decided to add its counterpart, especially when too easy to implement reusing existing code. sorry for such delay and inconvenience...

now users who want just to block something also have such option, not only "drop everything except allowed".

in any case, I think whitelist is useful in cases described by me in PR, but maybe it is for advanced users... if you download/seed torrents from the same source(s), peers base is pretty consistent, and it is very easy to collect statistics who and what connects to you. even observation in a week is enough to get approximate picture. and it is much easy to create whitelist based on such statistics rather than blacklist.

P.S> now I'm plan to refactor these black/white lists implementation. I think log messages about missing filters file per torrent are very annoying, and also some static variables used in code are ugly (they are just a trick). I found more elegant way to do the same, but some time is required for testing and polishing. Nothing will be changed from end user point of view (exception only one log message will be produced only once on startup, instead of message per torrent).
moreover, I tried to implement some "smart" plugin which should detect and drop unfair clients, but unfortunately first try was not so good - it turned out to be very "stupid" and decided to drop a lot of peers...

[SeaHOH]

moreover, I tried to implement some "smart" plugin which should detect and drop unfair clients, but unfortunately first try was not so good - it turned out to be very "stupid" and decided to drop a lot of peers...

Peers who are always choking should be dropped, and make peers' priority down in case very big DL/UL ratio when torrent is not seeding.

[escape0707]

@Kolcha I'm also thinking about doing a blacklist function, but "eh", why I'm figuring my own life and stuffs so slowly and lazily... (I want to wipe my current daily drive laptop and install dual booted win + arch on it. Then figure out a good Docker file to build qB on Win and Linux. Then edit and debug the code based on that. All this without losing my Anki desks and some media files used to create them scattered around my workspace folder.)
Anyway thanks for the works.

A traffic based blocking / UL speed restricting plugin is a lot more complex to implement. Like, you do want to seed an old torrent to some client requesting it if you believe (s)he will continue to seed, even if this could only happen in a week later or so. A reasonable way to do this is to build a credit system, but I don't think we are trying to implement a better "FileCoin" here, or are we?

Would you like to share your initial thought about the algorithm you might implement first?

[Kolcha]

@escape0707 first try to implement "smart" plugin was just to track reported progress and drop peers which don't update it for some time (calculated based on uploaded data). this implementation was targeted only to drop "downloaders" (because I saw a lot of such peers connected to my server). but I see that is not enough... and right now I know too little about torrent internals, I have to read some theory. so, right now I have no any ideas or plans regarding algorithm...

[escape0707]

Oh, I see. So you suggest to first create a plugin to impose a stricter banning rules to avoid "liars" and "malfunctioning clients". That's fair.

[c0re100]

I'm not sure why doesn't exist warning message is duplicate, seems no problem in source code

4/26/2021 6:40 AM - 'peer_whitelist.txt' doesn't exist, do not enabling whitelist plugin
4/26/2021 6:40 AM - 'peer_blacklist.txt' doesn't exist, do not enabling blacklist plugin
4/26/2021 6:40 AM - 'peer_whitelist.txt' doesn't exist, do not enabling whitelist plugin
4/26/2021 6:40 AM - 'peer_blacklist.txt' doesn't exist, do not enabling blacklist plugin

[Kolcha]

it is printed on each added torrent due to implementation... with new implementation I'm working on this will not happen (more correctly will happen only once at startup)

[c0re100]

Thank you~

[escape0707]

@Kolcha

Today I tried your patch and:

in this case .* means "any client name", i.e. "ban peer by id". of course, client name matching pattern (or exact name can be added). to ban peer by client name regardless its id, just place .* instead of peer id matching pattern. beware, .* in both fields means "blacklist everything"!

I found "blacklist ban by just peer client name" won't work. For example:

.* qBittorrent.*

in the peer_whitelist.txt can whitelist peers which client name starts with qBittorrent just fine, but will block every peer if put into the peer_blacklist.txt.

Then I glanced at the code, and think it's because you've adopted the code from the peer_whitelist logic, and it only looks at peer_id to determine whether to block or not at handshake time. This works fine for whitelisting but not blacklisting.

Because in whitelisting, ban = (not id_match) or (not name_match) so short circuit return true on not id_match works. But for blacklisting, ban = id_match and name_match so you can only short circuit return false on not id_match.

Would you like to try reproducing this issue? 😉

[Kolcha]

yes, blacklist was implemented based on whitelist logic. and this is not surprising that there are may be some errors...
thanks for reporting, I'll look into this this weekend

[Kolcha]

@escape0707
fixed! and fix is very simple - #252
now it will work as expected and as you described - ban = id_match and name_match . Thanks!

I tested your example banning qBittorrent:

[escape0707]

@Kolcha

blacklist can co-exists with whitelists, but this is meaningless. in any case, blacklist rules take precedence. as whitelist, blacklist is applied for all torrents, not only public.

Does this means at that case, whitelist will be completely ignored?

I want to whitelist Transmission but not Transmission/2.94 because Motrix is using it. But it seems that if blacklist exist, whitelist is completely not functioning.

I was thinking that these two filters act like "only let peers allowed by both rules (intersection) to connect".

Plus, is it possible and better to disable the whitelist and blacklist plugin for private torrents?

[Kolcha]

@escape0707

Does this means at that case, whitelist will be completely ignored?

no, whitelist rules are still applied, right after blacklist ones, see

qBittorrent-Enhanced-Edition/src/base/bittorrent/peer_filter_session_plugin.hpp

Line 61 in fd04741

bool filter(const lt::peer_info& info, bool handshake, bool* stop_filtering) const

I was thinking that these two filters act like "only let peers allowed by both rules (intersection) to connect".

it should be so. but due to very unstable nature of client name reporting some errors may occur. real client name (reported by client itself) became known only after few bittorrent messages (after "bitfield" message in most cases), but until this happen client name has string guessed by libtorrent, and it may affect filters if it is not replaced in appropriate time. for my personal case (I have slightly different filter built just for my server) I patched libtorrent to disable this guessing and track just empty name (it is empty until client reports something). please note that there are clients which don't report client name at all through whole communication session! but fortunately, usually such clients have suspicious peer IDs or just something I would like to block.

Plus, is it possible and better to disable the whitelist and blacklist plugin for private torrents?

this requires some code changes and recompilation, required check to ignore private torrents must be added here

qBittorrent-Enhanced-Edition/src/base/bittorrent/peer_filter_session_plugin.hpp

Line 52 in fd04741

std::shared_ptr<lt::torrent_plugin> new_torrent(const lt::torrent_handle&, client_data) override

required check looks like

qBittorrent-Enhanced-Edition/src/base/bittorrent/peer_blacklist.hpp

Line 71 in fd04741

if (th.torrent_file() && th.torrent_file()->priv())

sorry for delayed answer

[escape0707]

no, whitelist rules are still applied, right after blacklist ones, see

Then I think there might be a bug, I have:

$ cat peer_blacklist.txt 
-TR2940-       Transmission/2.94

and:

$ cat peer_whitelist.txt 
-qB9999-      .*

And I'm still getting a lot of peers. I think we have to look closer into this.

disable the whitelist and blacklist plugin for private torrents

Maybe I could give a try on it, thanks for the points you've given!

[Kolcha]

slightly strange, will look into it in few days

[escape0707]

@Kolcha I'm still looking at it, a first guess is that *stop_filtering is set to true inside blacklist match but is premature for whitelist matching to happen? Let me dive a bit deeper...

[escape0707]

@Kolcha Oh my god, only a fool like me will spend so much time looking at the older version of this code. And thinking about a pointed outer world storage's state, and then realize that I should look for the source of the bug in the latest version of the code......

I'm completely out of speech at the moment......

Any way, the reason is very straight forward:

qBittorrent-Enhanced-Edition/src/base/bittorrent/peer_filter_session_plugin.hpp

Lines 61 to 77 in fd04741

	bool filter(const lt::peer_info& info, bool handshake, bool* stop_filtering) const
	{
	if (m_blacklist) {
	bool matched = m_blacklist->match_peer(info, false);
	*stop_filtering = !handshake && !matched;
	if (matched)
	peer_logger_singleton::instance().log_peer(info, "blacklist");
	return matched;
	}
	if (m_whitelist) {
	bool matched = m_whitelist->match_peer(info, handshake);
	*stop_filtering = !handshake && matched;
	if (!matched)
	peer_logger_singleton::instance().log_peer(info, "whitelist");
	return !matched;
	}

If blacklist rule file exist, this method will always return prematurely at L68. The whitelist match code block will never be reached.

I think I can file a PR to fix this and also add the logic to bypass PT torrents. What's your thought?

[Kolcha]

@escape0707

Any way, the reason is very straight forward:
If blacklist rule file exist, this method will always return prematurely at L68. The whitelist match code block will never be reached.

ugh... when I looked into it to answer your question I even didn't notice this return in each if block... it was supposed to have both filters enabled if they exist

I think I can file a PR to fix this and also add the logic to bypass PT torrents. What's your thought?

feel free to do this. moreover, original "blacklist" filters are enabled only for public torrents