Persistent conference's offline peer list always grows and never decreases

[nurupo]

I have been looking for changes in v0.2.9 to write a blog post about and noticed that #1266 added API calls for listing offline peers, yet there is no call for deleting an offline peer, which implies that toxcore is the one managing offline peers, which makes one wonder under what condition does toxcore delete offline peers since they are, well, offline, and can be indefinitely offline, never to come back. When asked about when does toxcore delete peers from the offline peer list, @zugz has said that it doesn't, that it grows indefinitely and that its growth is not limited in toxcore, meaning it can be used to DOS a target user or an entire conference.

[nurupo]

As a one possible solution to this, I propose adding a limit on how many offline peers toxcore should store with a sane, non-DOS-enabling, default value, which a client can override with an API call in case the user want to increase or decrease the limit. It would also be nice to add an API function for deleting offline peers, so that users could manually delete offline peers, e.g. ones that haven't been online in ages. Offline peers also need an API call akin to tox_friend_get_last_online(), so that clients could automate deletion of offline peers based on how long they have been offline. The "last online" information might also be useful for the mentioned earlier toxcore's auto-deletion functionality where toxcore removes offline peers when their count exceed a certain number, so that toxcore could delete the oldest ones first (though "last seen" info might be useless for toxcore if it already stored the peers in the last seen order). Though favoring the peers who just recently went offline can be exploited to deny the persistent conference feature, so perhaps toxcore should remove offline peers at random? On the other hand, peers that haven't been online for long are not very likely to come back, so there is less value in keeping them...Not sure that removal strategy is the best.

[zugz]

My plan for dealing with this was to have a hard bound of, say, 128 offline peers per group (which costs ~96KB RAM and 18KB in the save file), and when it's exceeded prefer to keep tox friends over non-friends, and given that prefer to keep more recently seen peers. The point of preferring friends is that these are the peers we're most likely to use to rejoin the group. An API function to change the bound seems reasonable. To keep things simple I wouldn't save the bound, but rather set it on load to max(128,n) where n is the number of saved peers. * Monday, 2019-01-28 at 03:28 -0800 - nurupo <notifications@github.com>:

It would also be nice to add an API function for deleting offline peers

Sure, why not.

Offline peers also need an API call akin to `tox_friend_get_last_online()`,

It's already there: tox_conference_offline_peer_get_last_active().

Though favoring the peers who just recently went offline can be exploited to deny the persistent conference feature, so perhaps toxcore should remove offline peers at random?

Good point. Removing at random only slows down this attack. But preferring friends as suggested above stops this attack doing much damage - it can only purge the frozen list of non-friends.

[nurupo]

prefer to keep tox friends over
non-friends, and given that prefer to keep more recently seen peers.

That sounds better than what I came up with.

An API function to change the bound seems reasonable. To keep things
simple I wouldn't save the bound, but rather set it on load to
max(128,n) where n is the number of saved peers.

This doesn't make sense to me. If a client set the bound to 5, toxcore would save just 5 offline peers into the toxsave, but on the next toxcore startup toxcore will try to load max(128,5) = 128 offline peers from toxsave, even though there are only 5 saved?

[zugz]

* Monday, 2019-01-28 at 14:55 -0800 - nurupo <notifications@github.com>:

>An API function to change the bound seems reasonable. To keep things >simple I wouldn't save the bound, but rather set it on load to >max(128,n) where n is the number of saved peers. This doesn't make sense to me. If a client set the bound to 5, toxcore would save just 5 offline peers into the toxsave, but on the next toxcore startup toxcore will try to load `max(128,5) = 128` offline peers from toxsave, even though there are only 5 saved?

No, I mean it'll load 5 but set the bound back to 128, so if more peers join then it'll save more than 5 when it saves. So if the client still wants the bound to be 5, it'll have to call the API function again at startup.

[nurupo]

Sounds good, that seems to be in line with how toxcore stores other settings, i.e. it doesn't, a client must specify them on every toxcore instance creation, e.g. toxcore doesn't remember tcp vs udp, proxy settings, etc. Though it does remember user name, status message and such. Makes one wonder where it draws the line.

[sudden6]

IMO it makes sense to store status and nickname, because that allows you to easily move your profile between clients.

Also the proposal to prevent endless growth makes sense to me.

[nurupo]

IMO it makes sense to store status and nickname, because that allows you to easily move your profile between clients.

This doesn't explain why it drew the line there, as the same can be said about storing the proxy settings, tcp vs udp settings and so on. The less things you have to re-enter when moving your profile between clients, the easier it is to move. However, toxcore drew the line there and decided not to store those settings, which seems somewhat arbitrary to me.

[zugz]

* Monday, 2019-01-28 at 11:17 +0000 - nurupo <notifications@github.com>:

When asked about when does toxcore delete peers from the offline peer list, @zugz has said that it doesn't, that it grows indefinitely and that its growth is not limited in toxcore, meaning it can be used to DOS a target user or an entire conference.

By the way, it occurs to me that basically the same goes for the online peers list - if you send a load of "new peer" messages with random keys and peer numbers, each existing peer will add these fake peers to their list, potentially exhausting memory.