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

NGC: provide a stable peer list for NGC groups #2721

Open
zoff99 opened this issue Mar 2, 2024 · 4 comments
Open

NGC: provide a stable peer list for NGC groups #2721

zoff99 opened this issue Mar 2, 2024 · 4 comments
Labels
enhancement New feature for the user, not a new feature for build script

Comments

@zoff99
Copy link

zoff99 commented Mar 2, 2024

currently in PGC conferences if there are 5 peers in a group, the peerlist will always show those 5 peers.
even if some of them are offline.

in NGC this is not the case. so you never know who is acutally a member of the group currently.

the peer list should be stable and consistent. and show all peers unless they are kicked out or actually exit the group.
going offline should not remove the peer from the list.

@zoff99 zoff99 added bug Bug fix for the user, not a fix to a build script enhancement New feature for the user, not a new feature for build script labels Mar 2, 2024
@JFreegman JFreegman removed the bug Bug fix for the user, not a fix to a build script label Mar 2, 2024
@nurupo
Copy link
Member

nurupo commented Mar 2, 2024

What if a peer exits or gets kicked out while you are offline? Since you didn't observe that event, the peer would still remain in your peer list when they really shouldn't. Sounds rather complex, as this would require peer list synchronization among group chat peers and conflict resolution in order to function properly, and even then some events might get missed by multiple peers you are synchronizing your peer list with and you might still end up with peers being listed in your peer list despite them have exited or being kicked out, having to perform some sort of a peer list cleanup periodically.

Or perhaps it's fine if the peer list is not synchronized with other group chat peers to account for the events that you didn't observe, i.e. if it's just your personal observation peer list? For example, in addition to the currently online peer list, a client would display a list of offline peers that the client has previously observed being online, something like "Peers seen in the last N days" or "Last N offline peers". While toxcore could keep track of offline peers like that, this is something that could be done client-side.

I want to point out that this stable peer list feature also opens up an avenue for a DoS attack that should somehow be addressed. A malicious user could keep rejoining the same group with new group identity, increasing the peer list of everyone in the group chat. This might be less of an issue for an actively moderated group chat, but if the moderators are often afk, someone creating thousands of new peers per second might pose an issue. Even if the moderators in the same timezone and are asleep for 8 hours, that might still pose an issue. The peer list is stored in the RAM as toxcore doesn't do disk storage, so all the attacker would need to do is to create enough peers to make toxcore + the client consume all the available RAM. One way around that would be setting a limit on the number of offline peers toxcore remembers per a group chat to some reasonable amount, allowing the client to change that limit via API.

@zoff99
Copy link
Author

zoff99 commented Mar 3, 2024

NGC groups are limited to 100 peers per default. saved peers are also limited in NGC.
and PGC has this offline peer limit aswell.

#define MAX_GC_PEERS_DEFAULT 100
#define GC_MAX_SAVED_PEERS 100

i get the rest of your concerns though.

i still think it is expected of a messenger today to have this. apart from IRC i don't think i know any messenger which lacks a stable peerlist.

how does PGC keep a stable peer list? does it not? i am not 100% sure how it works in PGC, it appears like a stable synchronized peers list though.

there are a few (random) examples where it helps to have all peers known in a group:

  • you are invited to a private group, and when you join you can only see the peers currently online. you have no idea who else is in the group. when you type a message and hit send, at that very moment 3 peers can come online and read your message. you did not expect those 3 people to read your message since you did not even know that they were already members of the group

  • if a peer enters a group, then posts SPAM, then leaves quickly. there is no way to mute that peer. since you can only demote a peer when that peer is online. (this scenario has actually happend a few times already). now you need a bot again to wait until this peer comes online to mute him for the entire group. with a synced peer list actions like demoting could be implemented for offline peers aswell.

@nurupo
Copy link
Member

nurupo commented Mar 3, 2024

Good to know, didn't know NGC had these limits.

when you type a message and hit send, at that very moment 3 peers can come online and read your message. you did not expect those 3 people to read your message since you did not even know that they were already members of the group

Right, but how is it different from someone joining the group for the first time a moment before you send your message? Your message would be read by people whom you didn't expect to be in the group chat and no stable peer list would save you from new peers joining. That's not a strong argument for a stable peer list.

if a peer enters a group, then posts SPAM, then leaves quickly. there is no way to mute that peer. since you can only demote a peer when that peer is online. (this scenario has actually happend a few times already). now you need a bot again to wait until this peer comes online to mute him for the entire group. with a synced peer list actions like demoting could be implemented for offline peers aswell.

That sounds like something separate from the stable peer list feature. A separate feature requests for NGC to allow sanctioning arbitrary public keys, such that moderators could mute/ban/whatever a peer even if there is no such peer in the group chat.

(While on this topic, I'm surprised NGC doesn't allow blocking of exact IP addresses and IP ranges/subnets, as public key based blocking is rather easy to circumvent. I guess this is hard to do when peer A might be connected directly to peer B, but via a TCP relay to peer C, so B and C see different IP addresses and both would need to get blocked, and blocking TCP relays is kind of bad as other currently online peers might be using it or an offline peer / new gc member might join via it and get unintentionally blocked?).

@JFreegman
Copy link
Member

but if the moderators are often afk, someone creating thousands of new peers per second might pose an issue.

NGC rate limits new peer handshakes, so while this would still be an issue, you could do maybe a few dozen per minute rather than thousands per second.

While on this topic, I'm surprised NGC doesn't allow blocking of exact IP addresses and IP ranges/subnets, as public key based blocking is rather easy to circumvent. I guess this is hard to do when peer A might be connected directly to peer B, but via a TCP relay to peer C, so B and C see different IP addresses and both would need to get blocked, and blocking TCP relays is kind of bad as other currently online peers might be using it or an offline peer / new gc member might join via it and get unintentionally blocked?.

We used to have IP bans but decided to remove that feature because it was effectively useless with the existence of TCP relays. Anyone running tox with UDP disabled would be able to circumvent the block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature for the user, not a new feature for build script
Projects
Status: Todo
Development

No branches or pull requests

3 participants