Skip to content

Prevent GDScript language server from listening to external hosts by default

Rafał Mikrut requested to merge github/fork/Houkime/gdscript-ls-stop-listen into master

Created by: Houkime

  • Add "bind_ip" property to WebSocketServer class which defaults to "*" (for compatibility with current users)
    • WebSocketServer is the class that GDScript language server relies on
  • Add an editor option to set the bind_ip for the GDScript language server
  • Set GDscript language server to listen only to localhost by default

Problem

Currently GDScript language server is enabled at startup and exposes an open port (6008 by default) to the net accessible by any host unless filtered.

I feel uncomfortable about it considering that

  • I don't even use the language server function.
  • It cannot be turned off and/or bound to localhost from editor settings.

Language server at startup by default?

The most straightforward solution would be to have the Language Server simply turned off at startup by default. However at preliminary discussion @Faless at IRC stated that C# users are unlikely to appreciate if it will be turned off by default. So a solution then to have it on by default, but listen only to localhost, which most likely covers 99% of intended usage anyway.

Why not just make an off switch, so you, Houkime, turn this off, and others have it on by default?

I consider adding an off switch in a separate PR since it is an independent change.
This PR however considers the security of those Godot users who are not likely to look at network settings or scan open ports (which is probably a majority) so it is a bit more important to make early.

Why a property and not an argument for the listen() function?

It was recommended by @Faless for compatibility reasons and because bind_ip is unlikely to change often.

However, in places specific to GDscript Language server (where interfaces are not accessible from GDScript) it is an argument in the line with the underlying TCP_server listen() function. This can be changed however if considered clumsy (though i somehow think that making it a property of a gdscript ptotocol for example is even more odd).

Why touching WSLServer?

WebSocketServer class is more of a virtual thing, which exact implementation depends on whether we're in browser or running natively (in browser it is a dummy). It doesn't implement a listen() function.

ClassDB::instance() when asked for WebSocketServer returns either WSL (native) server or EMWS server. See https://github.com/godotengine/godot/blob/master/modules/websocket/websocket_server.cpp

Merge request reports