godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.89k stars 20.15k forks source link

Discuss option to disallow propagation of peer connection and targeted RPC messages for authoritative servers. #27932

Closed RyanStein closed 4 years ago

RyanStein commented 5 years ago

Hello Godot team,

While working on an authoritative server (a server which processes only input from clients and communicates the results back), I discovered NetworkedMultiplayerENet sends messages to clients about other clients connecting and disconnecting, as well as blindly relaying RPC calls from one client to another without any apparent way to deny their propagation.

Presuming a world where someone somewhere may make an unofficial client that takes advantage of this blind propagation, it would greatly simplify the code I would have to write to make sure every RPC coming in is from 1 on the clients' side. There is also the small concern some games might want to hide exactly how many other players are currently connected.

I wanted to open this issue for the sake of discussion. Maybe I've misunderstood the networking documentation. Am I using the wrong tool here? Is there a better way to accomplish this? Perhaps StreamPeer? The RPC format is quite convenient though.

For my own purposes, the necessary changes wouldn't be too extensive. It looks like everything that needs changing is under modules/enet/networked_multiplayer_enet.cpp:

https://github.com/godotengine/godot/blob/edc9097bc1604f0bd5ea7136c518699e9d64a731/modules/enet/networked_multiplayer_enet.cpp#L257 https://github.com/godotengine/godot/blob/edc9097bc1604f0bd5ea7136c518699e9d64a731/modules/enet/networked_multiplayer_enet.cpp#L292 https://github.com/godotengine/godot/blob/edc9097bc1604f0bd5ea7136c518699e9d64a731/modules/enet/networked_multiplayer_enet.cpp#L365

fire commented 5 years ago

@Faless This is your area.

Faless commented 5 years ago

Hi @RyanStein , I agree, an option to disable that is definitely needed.

Currently, by defining your RPC as puppet they will only be called on the remote end when the call comes from server (1) by default, (or by the ID assigned via set_network_master).

So the clients cannot exploit that to call RPCs they are not allowed to, but there is no way to tell the server not to relay messages that way. nor to notify the existence of other peers. Again this is something that needs to be added, probably in the NetworkedMultiplayerPeer itself as an option, enforced then inside NetworkedMultiplayerEnet and WebSocketMultiplayer.

TIBI4 commented 5 years ago

Yeah, I think it would be very easy to send those messages ourselves instead of doing this automatically.

I thought I was the only one thinking about this feature proposal, even if I may just ignore the "_on_peer_connected/disconnected" signals, I didn't want it happening in the background.

This "message" is maybe ok for RPG or multiplayer games, but there are other type of online games where we only need to save/load/check.

There are lots of game types where this isn't used, but my low quality english won't let me explain them well.

realkotob commented 4 years ago

Have been thinking a lot about this recently, pretty glad this got changed even before I had time to post an issue!