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

Server-provided client-side scripting #5393

Open
octacian opened this issue Mar 14, 2017 · 140 comments
Open

Server-provided client-side scripting #5393

octacian opened this issue Mar 14, 2017 · 140 comments
Labels
@ Client Script API Feature request Issues that request the addition or enhancement of a feature High priority Non-trivial A large amount of work is required to address this (sometimes to the point of being infeasible) SSCSM

Comments

@octacian
Copy link
Contributor

The title says it all. What are the issues with doing this? Would security be an issue?

I would see allowing server-side mods to inject temporary client-side mods as being very useful, e.g. allowing a protection mod to make things more smooth for players. If the protection mod client-side portion were to be manually installed by the user, they could modify it so that they could get around protection requiring the protection mod to have to keep server side validation. Instead, if the server could inject the client side mod at load, this wouldn't be an issue.

@nerzhul
Copy link
Contributor

nerzhul commented Mar 14, 2017

We are thinking about it but it's a very very low priority, the more important is to have pure client features at this moments, mod store, etc

@nerzhul nerzhul mentioned this issue Mar 14, 2017
27 tasks
@raymoo
Copy link
Contributor

raymoo commented Mar 14, 2017

The server should keep server-side validation in either case since an attacker could ignore the mod and send digs anyway if they make a modified version of Minetest. For protection, the client-side mod would be just for prediction.

@octacian
Copy link
Contributor Author

This is true. However, being able to inject the clientmod would still be very useful. One could also make it harder to avoid using a clientmod by forcing the client to provide a list of mods to the server. Though the client could be modified to provide a bad list, it makes things a bit harder.

@BluebirdGreycoat
Copy link
Contributor

BluebirdGreycoat commented Mar 16, 2017

I do not understand why this is considered low priority. Is this not the exact reason for having client-side mods in the first place? The ability to inject code into the client isn't even really a big thing -- I mean, websites do it to your browser all the time. If Minetest servers cannot do the same thing to Minetest clients despite supposedly being able to support it, then a huge opportunity for enhanced content delivery is lost. Minetest might as well not have client-side mods at all, if this important feature is left out.

Edit (need to clarify something -- and I've probably been too harsh):

The chief problem I see with supporting client-side mods, but not letting the server upload the mod code dynamically, is that this would require all players who want a given client-side mod to download it manually, and hope its communication channel/version is compatible with its counterpart mod that runs on the server. But in my experience, mod code frequently changes. I think it would be better if the client got a fresh version from the server every time, so that the user doesn't have to worry about versioning, compatibility, etc., and server operators who also manage all their own mod code can update their mods without having to ask their players to download a new version.

@nerzhul
Copy link
Contributor

nerzhul commented Mar 16, 2017

@BluebirdGreycoat before making server sending code, you don't think we need API calls to run code ?
Also sending code should be very very very controled, and needs to change the current client to server connection to accept CSM and non CSM client, it's not easy. First have calls to call, second enhance them, permit to run code from client and at least allow server to send code, with very very careful protocol

@BluebirdGreycoat
Copy link
Contributor

@nerzhul I understand there are huge complexities (esp. security) that need to be overcome before this can even be possible. However, I have difficulty understanding (going just by the title of this issue) why this would be considered as anything other than the end goal of the effort toward client-side mods.

@celeron55
Copy link
Member

I think this is very important. It's in line with Minetest 0.4's original goal of server-defined games.

@rubenwardy
Copy link
Contributor

rubenwardy commented Apr 1, 2017

I agree that this is very important. For me, server mods being able to make GUIs better is the best thing about csm

Maybe we do need to add new client functions first, but it shouldn't be "very very low priority" just a more medium/long term goal

@nerzhul
Copy link
Contributor

nerzhul commented Apr 1, 2017

for GUI better, personnaly please justify which feature you need exactly, because we already send formspec to client and it sends events to server, in 99% cases it's normal because you need to handle server side things

@raymoo
Copy link
Contributor

raymoo commented Apr 1, 2017

@nerzhul
With client-side Lua code, you could have a GUI system that allows for custom GUI elements with custom behavior. It will also decrease the response time of GUIs on a laggy connection.

An example for the first thing: I want to make a mod where players can design their own spells, which are defined as some kind of tree of spell shapes and spell effects. Players can fit these together graphically similar to programming in Scratch. For example, an "area of effect" spell shape node would have slots for spell effects.

An example for the second thing: Same example as the first, but assuming I don't have custom GUI elements so I use some system other than drag-and-drop. I don't want to have to wait for a roundtrip to the server between every click if there is 200ms+ latency. I can send the server the completed spell once the player decides to save it, because there's no need to validate the intermediate steps as long as the finished product, which is what will actually affect the world, is validated.

In the same line of examples, it would also allow the player to save spells for use on any server with the mod. The server would just have to validate the spells when imported from the client.

@Ferk
Copy link
Contributor

Ferk commented Apr 17, 2017

for GUI better, personnaly please justify which feature you need exactly, because we already send formspec to client and it sends events to server

Creative GUI, for once, is slow on heavy servers.
Client-side scripting would make viable new features in the formspecs like autocompletion in fields, and updating the formspec on the fly as you type (to search for nodes) without having to communicate heavily with the server.

And I'm sure creative wouldn't be the only server mod that could benefit from injecting client-side GUI.
There's even a mod that adds a tetris arcade game made with formspecs.

That's not counting things like preventing showing the "undo lag" from protection mods (yes, you still need the server check, but this would reduce traffic and be much more usable).
Or having an ambient sounds / weather effects server mod that actually uses sounds provided by the server and applies in specific circumstances for that server but whose logic for checking if the circumstances apply and running the effects is executed client-side.

Honestly, most of the good uses of client-side scripting involve server mod dependencies.
IMHO, having "clientmods/" has much less uses (and more mis-uses by cheaters) than having client scripts provided by server mods. Personally I would have rather only done the latter.

@sofar
Copy link
Contributor

sofar commented Apr 18, 2017

I'm removing the "low priority" label because as people listed, it's not low priority.

However, just because this is not low priority, it doesn't mean that other things don't need to go in first.

@rubenwardy
Copy link
Contributor

#3440

@ghost
Copy link

ghost commented Oct 4, 2017

Phew! Sorry about the dupe. Thanks for this issue, can't wait until this is possible. Here's what I wrote:

Right now we have Server-Side Mods (SSM), mod channels, and Client-Side Mods (CSM). However, in order for this to be entirely practical for the server's needs, it must know that the client has connected with the appropriate CSM loaded.

I suggest a setting, csm_autodownloads, default false, that when enabled will allow the client to download the necessary CSM from the server. The server will then know it can use mod channels to interact between SSM and CSM.

<+sfan5> csm is sandboxed, so servers sending lua code should not be a problem
<+Calinou> jas_: client-side mods can't be sent from server to client, that would be too insecure

@Ferk
Copy link
Contributor

Ferk commented Oct 4, 2017

<+sfan5> csm is sandboxed, so servers sending lua code should not be a problem
<+Calinou> jas_: client-side mods can't be sent from server to client, that would be too insecure

Aren't these two comments contradicting each other?
Is there a log of the full conversation? was there any agreement?

@paramat
Copy link
Contributor

paramat commented Oct 4, 2017

http://irc.minetest.net/minetest-hub/2017-10-04#i_5097521
Calinou is wrong, we intend to add server-sent CSM once it can be made secure enough.
Server-sent CSM was the original plan documented in the dev wiki and is the more useful aspect of CSM.

@nerzhul
Copy link
Contributor

nerzhul commented Dec 11, 2018

@ClobberXD wait for 5.0.0 to be released and we can discuss that feature together if you want to code it, and define a proper exchange format, with the correct unittests

@ClobberXD
Copy link
Contributor

Been waiting for a long time, but I understand. I can continue waiting, no problem... :)

@paramat
Copy link
Contributor

paramat commented Dec 18, 2018

nerzhul wrote:

and no the network compat breakage cannot prevent CSM, you are totally wrong, because CSM is purely client side (except mod channels) then you can prevent servers from detecting client, like any C++ rogue code.

Ok, but a 0.4.x client won't work on a 5.0.0 server, so only a 5.0.0 client can be used, which would not have the CSM code.
Obviously a 5.0.0 client could be hacked to have CSM code (so i know absolute prevention is impossible) but that would not be easy to do.

I'm comparing the complete removal of CSM before or after 5.0.0 release, if it was removed afterwards in 5.1.0 then a 5.0.0 client could easily be used by anyone, without hacking, to continue using CSM.
So, the network breakage would make using CSM much more difficult.

This is rather theoretical now anyway because it looks more likely CSM will be completed. But a while back it looked possible all CSM was going to be removed. If that was going to happen it would be best to do it before 5.0.0 not after.

@nerzhul
Copy link
Contributor

nerzhul commented Dec 18, 2018

what scenario i want:

  • audit the API to ensure problematic calls for server owners are under restrictions (it's server owner role to list which API can be problematic for them, most of them has already been identified)
  • ensure we have a restriction to disable the client lua stack entierely from server side if needed (you mentioned it but i can't remember if it's possible or need some different handling). This is not bypassable except if you recompile a rogue client (as difficult to bypass as if you hardcode the CSM disable but you keep the possibility to make player use it)
  • have the good CSM restriction flag by default (all restrictions except disable CSM entierelement sounds reasonable and permit server owners to remove it if they want, but permit some others to use it and start to think about CSM mods they can use in 5.1 and push to clients)
  • release 5.0.0 with new restrictions (if new restrictions found)
  • do the 5.1.0 (short dev cycle, when SSCSM is ready and no major waited feature, feature freeze + release)

@nerzhul
Copy link
Contributor

nerzhul commented Dec 19, 2018

I started #8002 technical discussion about this need for the next roadmap.

@nerzhul nerzhul modified the milestones: SSCSM, 5.1.0 Dec 20, 2018
@paramat paramat removed this from the 5.1.0 milestone Sep 12, 2019
@paramat
Copy link
Contributor

paramat commented Jun 16, 2020

Just a quick update.

Client-Provided CSM was released and usable in MT 0.4.16, so Server-Sent CSM is now just over 3 years overdue. I am counting the years =)
It somewhat makes me wonder how many years it would take for us to give up on CSM and admit it will never be completed. In the meantime we are stuck in a situation that was not intended, planned or supported, and is considered crazy by celeron55. It should be SSCSM or nothing at all, as originally intended.

Therefore i think that it is wrong that CPCSM is usable before SSCSM is implemented, CSM should be disabled until then.

As i have commented elsewhere, although i am not particularly sure about this, i somewhat feel it would be best for MT to give up on CSM soon and remove it completely. Due to the inevitable problems of SSCSM, the workload, the security concerns, combined with the perpetual lack of core dev time.
More simplicity would be very valuable to MT.
If MT was sold for a price and we were a professional development company with far more developer time perhaps i would feel different.

I know my opinion will be unpopular with many, but sometimes the right thing to do is an unpopular thing.
Please keep in mind i do not feel very strongly about this.
EDIT: And do not worry, the chances of CSM being removed, or disabled until complete, are very small.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Jun 17, 2020

I actually agree. Keeping this weird half-arsed non-solution is clearly the worst of all possible worlds. Kill it. I won't miss it.

@ghost
Copy link

ghost commented Jun 17, 2020 via email

@LoneWolfHT
Copy link
Contributor

LoneWolfHT commented Jun 17, 2020

  • SSCSM has been implemented in lua twice already.

  • If CSM is removed completely: When SSCSM is finally added for real, the person doing it will have to re-add all of the API.

  • I've heard someone say they were going to take a jab at SSCSM eventually. If they didn't I probably would after I got more familiar with Minetest's C++ (Which I'm already working at)

  • Instead of spending a bunch of core dev time removing it why not start on implementing SSCSM? Once you've spent the time you would have spent removing it you can stop and let someone who's interested pick it up

@ClobberXD
Copy link
Contributor

Plain CSMs (non-server-sent) also have their uses as utilitarian mods. Chat colouring, chat notification bell, etc.

@nerzhul
Copy link
Contributor

nerzhul commented Jun 17, 2020

totally, also please note one coredev is complaining about it for 3 years, but he can try to implement it ? :)

@NA0341
Copy link

NA0341 commented Jun 25, 2024

if the server could inject the client side mod at load, this wouldn't be an issue.

Good idea.
If you add a checksum, you can even store them on disk since changes will just be overridden if they are detected.

@appgurueu appgurueu added the Non-trivial A large amount of work is required to address this (sometimes to the point of being infeasible) label Sep 3, 2024
@appgurueu
Copy link
Contributor

Here is my current SRCSM (the R stands for required) proposal: https://gist.github.com/appgurueu/4f419f4b4c10dc850cc4049962cb101a.

My plan notably deliberately argues for the exclusion of process sandboxing for the first version of SRCSM, both to avoid some of the limitations that brings and to make it more realistic that SRCSM happen in a reasonable timeframe.

@Desour
Copy link
Member

Desour commented Dec 16, 2024

Other proposal (SSCSMCPB only (a much cooler abbreviation)): https://gist.github.com/Desour/0fe4f4586b9213c40db6b04b6e0bd899
(I think I have proposed the same already at least once. But here it is formally, and less buried.)

Btw, linking this PR: #15246

@sfan5
Copy link
Collaborator

sfan5 commented Dec 16, 2024

(glossary: SSCSM = "server-sent" CSM = SPCSM = the thing we're talking about here
CPCSM = "client-provided" CSM = what we have right now)

responding to @appgurueu's proposal:

I think we should view Luanti as more of a game engine / platform than a "browser".

I think not taking an absolutist stance on security is reasonable but "oh you need to trust the game authors" would not be. I want to be able to join a random server without fearing that something could get access to my private data. (You don't seem to disagree)

Servers (via the serverlist) advertise the CSMs they require, identified as author/name:version

This is a totally unnecessary dependency (and SPOF) on the server list.

Mods should obtain references to the public API tables of each other via local mod = require"mod"

How does this even work if other mods run in a different Lua env?
I think we can forego this entirely by declaring CPCSM a dead end (it kinda is), since of the server mods are all of the same trust level.

First, let me suggest that we don't need that security. It can't be a "must have", otherwise SSMs as they are would be unacceptable.

I think the crucial point is that SSMs are installed voluntarily, while the initial design of SSCSM foresaw that they are automatically downloaded and executed on the client just like a browser does.
As I understand you intend to mitigate that by forcing CSMs to be open-source and on ContentDB.

That raises some questions:

  • Would CSMs that just act as loaders for code sent by the server be forbidden?
  • Can we rely on reviewers reliably spotting this?

Additionally, the Luanti community has, to my knowledge, hardly ever seen any attempts at (sophisticated) "malicious" mods. [...]

Sorry but no. "Nobody is seriously trying to attack us" cannot be part of your threat model.

What is the realistic attacker model supposed to be here that justifies the concerns? I simply don't see any bigger actors attacking Luanti anytime soon.

Me neither. To pull up the browser analogy again, modern malware often relies on ads to either directly run JS or lead the user to a site they didn't mean to go to. Nothing even close exists in the Luanti ecosystem.
But the threat model is simply: The user is on the attacker's server, the attacker can run arbitrary code in CSM. The engine should protect the integrity of the user's computer and data.

Second, I do not think that we stand to gain much security by adding a process sandbox.

We still have SSMs, and users happily downloading them from CDB and running them. If there is a malicious mod which escapes the (SSM) sandbox or exploits a Lua implementation bug, I would assume the vast majority of users to already be vulnerable this way. (Also, realistically, who would waste a Lua implementation exploit on Luanti? Furthermore, isn't Luanti much more likely to be vulnerable than both PUC Lua 5.1 (+ patches) and LuaJIT, [...]?)

Notably, a process-level sandbox can not protect against any kind of vulnerability that is triggered by malicious data / commands being processed in the client C++ code, of which I assume there will still be a fair amount of.

Note: point about SSMs somewhat addressed above.

Without a process sandbox the attack surface being the Lua interpreter / JIT as well as (rightly pointed out) all the API-facing Luanti code, which includes various memory management which we have and still do get wrong.
With a process sandbox the attack surface is the OS sandbox itself, the IPC protocol and its clearly defined entry points.
Nothing beats careful coding, but this definitely moves a big part out of the equation.

We probably have a decent attack surface already via our current networking API and dubious file readers already (though those have improved since sfan's fuzzing efforts a while ago).

This is not wrong but given modern mitigations attack in the style of "load this malicious file and we pwn you via a buffer overflow" are almost extinct.
However having a turing machine available increases the attack possibilities by magnitudes, so having that is much more risky.
To substantiate this point here's a recent PS4/PS5 exploit chain based on unsandboxed Lua access: https://github.com/shahrilnet/remote_lua_loader
(impressive chain, but the base for it is just Lua bytecode being untrusted by principle)
((getting more off-topic but this and related articles are really interesting https://googleprojectzero.blogspot.com/2022/03/forcedentry-sandbox-escape.html))

However, we should probably be additionally authenticating servers. This is possible if we identify them with HTTPS websites.

Aside from introducing a huge amount of complexity on many fronts, which problem specifically is this supposed to solve?

We also have CDB as another layer of defense in this scenario. If a CSM was found to be malicious, it could be pulled from CDB rather quickly, limiting the damage.

Except the client does not automatically uninstall mods when they disappear from CDB.
Do you want to introduce a "mod kill switch"? I think many people would rightfully point out that this is not very well-behaved for a FOSS application.

If however CSMs were limited to 1000 or 100 sequential API calls per second, that would be very noticeable depending on the CSM.

I think you are overestimating the impact.
More than 10 years ago someone on StackOverflow reported achieving less 8 microseconds by simple means in Java (and this isn't even using shared memory)
Doubling that to account for round-trips is 62500 API calls per second.

Also: consider that if IPC is good enough for browsers to sandbox their web processes, then it will be good for enough us.

I would still estimate that a "Client API over IPC" will be order(s) of magnitude slower if you issue many sequential API calls (of which you need to get the results back) in a "ping-pong" fashion.

This is obviously correct, but what exactly are we expecting to support? A CSM re-calculating pathfinding for 50 entities every client step? I don't think so.
Worrying about this is extreme pre-mature optimization.

CSM restriction flags are the best you can do in open source software and must be good enough. If people want to cheat, they can already download readily available cheat clients that don't have CSM restrictions [...]

Cheating is only a concern if we blindly add trusted APIs to the current state of client-controlled CSM.

@Lazerbeak12345
Copy link

Lazerbeak12345 commented Dec 16, 2024

If unsandboxed code is ever executed by the client, I suggest a security warning/popup for every instance.

Could be either an itemized list of things to allow each time, or a separated popup per. Must be opt-in.

If a user chooses to not opt in, I suppose that should be up to the SSMs to handle, but default behavior could be to kick.

@sfan5
Copy link
Collaborator

sfan5 commented Dec 17, 2024

Also following current IRC discussions¹ it very much looks like the "we can relax the security stance on lua" instead brings in a whole lot of complexity regarding how to implement CSM revocation in a secure, privacy-preserving, auditable way without an always-online requirement.
I think it's obvious what I'm getting at but maybe it's really easier to make the sandbox as secure as possible and skip the "code DRM"?

¹: https://irc.minetest.net/minetest/2024-12-16 and https://irc.minetest.net/minetest/2024-12-17

@Desour
Copy link
Member

Desour commented Dec 17, 2024

Here's a start: #15568
(Very simple. The most complicated part is the PR description.)

@appgurueu
Copy link
Contributor

A lot of good points here. I may write a more elaborate reply later on. I ran some basic benchmarks (get_node vs IPC RTT) and indeed it seems like my performance concerns are highly likely to be unjustified at present, at least giving current scripting API overheads, and it also turns out that we do not save that much development work by skipping the sandbox (and we would end up with a design which might inhibit our possibility to add a sandbox in the future). Hence I'm not opposed to a process sandbox anymore 👍

I may try to give feedback on the specific implementations over the holidays, I'll see. I appreciate the work you're doing @Desour.

@kromka-chleba
Copy link
Contributor

Not sure if this was discussed but it would be good if the client could allow/refuse CSMs sent by the server based on their license. Servers that would attempt sending a CSM not verified by CDB should provide a license identifier for the CSM, then the client could pick to reject CSMs under licenses that don't comply with the definitions of free/open source software by the FSF and OSI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Client Script API Feature request Issues that request the addition or enhancement of a feature High priority Non-trivial A large amount of work is required to address this (sometimes to the point of being infeasible) SSCSM
Projects
None yet
Development

No branches or pull requests