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

Project: Script Engine Upgrade #1180

Open
digisomni opened this issue Apr 16, 2021 · 15 comments
Open

Project: Script Engine Upgrade #1180

digisomni opened this issue Apr 16, 2021 · 15 comments
Labels
enhancement New feature or request scripting api change Change is made in the scripting API Severity: Medium Important functionality is affected, but a workaround exists stale Issue / PR has not had activity

Comments

@digisomni
Copy link
Member

digisomni commented Apr 16, 2021

This issue should serve as the central discussion for notes / needs for the script engine upgrade.

Mandatory Requirements for the New Scripting Engine:

  • Security Context
    • Be able to give/take away permissions for scripts by targeting their source.
    • ...
  • Security Sandboxing
    • Scripts can have a small form of storage on the filesystem and network stores.
    • (elaborate...)
  • Script Versioning
    • every script should define its version, if they do not have a version we assume it's a legacy script. This allows us to have the old scripting API and a newer one. With each round of updates we upgrade the script version and so if scripts want to use the API in different ways, they will have to now define that at the top of the file.
    • If there are incompatibilities that are too drastic between API versions (1.0 to 3.3) where it might cause some breakage in how things are handled on a domain... a domain can enforce that only certain script versions can / should be used on it. Using a custom client that rejects that and causes issue can/would be seen as griefing.
@digisomni digisomni added the enhancement New feature or request label Apr 16, 2021
@JulianGro
Copy link
Contributor

JulianGro commented Apr 17, 2021

My understanding is that all the mandatory requirements you list wouldn't need to be part of the actual script engine.

Security Context: The scripting engine will not know what kind of contexts we would need and what api functions would be allowed in what context. If the scripting engine has no functionality for defining what is and what isn't allowed, we can just do that on the api side.

Security Sandboxing: I don't see why the scripting engine would need proper sandboxing functionality. My understanding is that we wouldn't allow anything to leave the script engine except api calls. Of course it would be useful if we can just disable functions that access stuff like the filesystem or the internet, but my understanding is that we can just not give its thread access to the internet or the filesystem. Even if that is not possible, my understanding is that it would be trivial to disable functions which access filesystem or networking inside the script engine source code.

Script Versioning: If the script engine had no way of getting a version like that, we have several alternatives like checking the version before giving the script to the script engine, or defining the version in a call to the api and have the api handle that.

That being said, if we don't want to go with QJSEngine it would probably make sense to use a scripting engine that allows better upstreaming of changes/fixes than Qt does.
Alternatives I have found while looking around:

  • KJS https://invent.kde.org/frameworks/kjs
    From KDE. Has been around and used for 21 years. It worked fine when I was personally using it as part of Konqueror with KHTML at some point. Apple has based some javascript engines on KJS. Packages seem to be available in every major distribution.

    Has been more or less deprecated. It is not supposed to be used for modern KDE applications.
  • v8 https://v8.dev
    From Google. While it is apparently used by node.js, I assume it will be just as horrible to deal with as Crashpad and WebRTC when it comes to actually building the thing and getting any kind of support for it.
    The Debian package is basically a redirect to the nodejs package. Knowing how much the Debian people like to pull software apart into as many tiny libraries as they can, this doesn't make v8 easier to build.
  • SpiderMonkey https://spidermonkey.dev
    From Mozilla. Apparently it is almost exclusively used as part of Firefox and using it externally is not well supported (at least by Mozilla). They specifically say that they try to keep it easy to embed into other applications and accept patched that improve this functioniality. Debian and Ubuntu repositories have newer versions than Mozillas latest "release". It might be easier to work with the Debian repository than getting it out of Firefox ourselves. https://salsa.debian.org/gnome-team/mozjs

@daleglass
Copy link
Contributor

I think the first step would be to refactor the current code to make sure more than one scripting engine can exist.

This is because no matter what, this will be a big project. There's no doubt problems will arise. And given that everything depends on a working scripting engine, we can't just rip it out and spend half a year trying to integrate a new one. Everything would grind to a screeching halt.

That already exists to some extent, so it shouldn't be a huge project. I suggest the following milestones:

  1. Cleanup and document. There's a fair amount of documentation in ScriptEngine already.
  2. Present the current state of the system. As in somebody actually makes a presentation explaining what is where, what the parts are, how they interact, what design issues exist.
  3. Discuss how to refactor
  4. Refactor. The old script engine should remain just as functional as before.
  5. Implement a minimal, proof of concept second engine. QJSEngine probably would be easiest. By minimal I mean it can run a "hello world" -- this is a quick demo of how multiple engines work.
  6. Complete implementation of QJSEngine. I think it's the easiest one we could possibly do, so it's probably worthwhile even if it turns out to suck. Out of this we can draw some conclusions -- is it good enough? Is it lacking something? Did we screw up the refactoring somewhere?
  7. If we're unhappy with something at this point, only now consider V8 and alternatives.

My current preliminary thoughts:

  • ScriptEngine currently inherits from QScriptEngine. This obviously can't remain.
  • ScriptEngine should be the general interface the rest of the code uses for scripting. "Hey, run this script. Connect this API to the scripting system".
  • ScriptEngine currently exposes a scripting interface. I'm not sure whether this should be moved elsewhere or not.
  • ScriptEngine shouldn't inherit from any engine. Instead there should be a backend system. Doing this would allow ScriptEngine to take decisions like whether to spawn a thread per script, how to manage contexts, and perhaps handle some of the security issues, gather stats, even select which backend to use.

@daleglass
Copy link
Contributor

Thoughts on script security:

It's complicated because we don't have proper user accounts like SL does, and there's a lot that needs doing to keep things safe but usable. But here's my rough idea on the model.

Every API function call should begin with a call to the security system that evaluates the situation and returns a result: do we proceed, or not.

Here's a list of things we can currently look at:

  • What kind of script is it? Interface, entity, etc.
  • The script's URL. Some will get an automatic ok, such as scripts shipped with the interface. The rest can be parsed for applying more granular rules (eg, per web server/site).
  • What scripting engine is being used. There might be a need for some per-engine considerations, like if an engine is hard to properly secure, or is experimental and we don't want people exploiting its flaws meanwhile.
  • Presence of a signature. I think it makes a lot of sense to consider code signing.
  • Object identifier
  • Object owner
  • Type of thing being acted on (avatar, object, script, etc)
  • Identifier of the thing being acted on
  • Owner of the thing being acted on

Based on this we formulate a base policy. What is the default set of things that's okay to do? For instance:

  • Scripts included with interface have full control over you and anything UI related.
  • Doing something to yourself is always okay. You can change your own avatar. Other people can't.
  • Scripts owned by a domain admin are allowed to ban people from it.

Anything not permitted fails immediately without any questions. The security API would come with a "request permission" function that could be called first, to obtain a permanent or temporary exemption. Perhaps there also may be implicit permission grants, where the user doing something may grant permission for it to be done to them, for user friendliness.

There should be some way for grouping up permission requests to present a bunch at once, including for a whole domain. Like if a domain creates a game, which needs to rearrange your menus and to control your camera, this could all be done upfront.

@digisomni
Copy link
Member Author

My understanding is that all the mandatory requirements you list wouldn't need to be part of the actual script engine.

Essentially, they are considerations to keep in mind when implementing the new one. It may not necessarily be the case that the new scripting environment will have these features implemented from go, but it is a requirement that it should support their implementation right off the bat when we do get around to it. We want to make sure that whatever is chosen and however we implement it is considering the things that we've long missed with our current implementation.

@daleglass daleglass added Severity: Medium Important functionality is affected, but a workaround exists scripting api change Change is made in the scripting API labels Apr 17, 2021
@ctrlaltdavid
Copy link
Collaborator

A good first step could be to refactor the current scripting implementation in conjunction with adding Qt's new script engine as a (compile time?) alternative - so that the new script engine can be tested against the old script engine, and to provide the framework for inserting other scripting engines.

@digisomni
Copy link
Member Author

Yes, I suppose getting another one "pulled in" to experiment against whilst trying to separate things out will probably be step 1. Unless it's QJSEngine, in which case I'm pretty sure we can just write that code outright since it comes with Qt.

Though, I don't think QJSEngine will cut it just from what I know about it (being slow) vs scripting in mainstream game engines like Unity3D. We expect speed and control.

So, if it's easier to develop a parallel structure using QJSEngine as the fill in first, then cool. If it's easier to import V8 (or something like it) at the base and then use that to start testing out a parallel structure, also cool.

I think that many core pieces will probably change (especially in the way security would be handled) so maybe not making the parallel structure with QJS makes sense. Why? Because the way QJSEngine works would be fundamentally different from how the real implementation would work, no?

@JulianGro
Copy link
Contributor

The question to me would be: In what way would QJSEngine be too slow and in what way would there not be enough control.
I mean all really heavy calculations should probably not be done inside the scripting engine, but realistically speaking why would there be any real speed differences compared to any other JavaScript engine.
What would a game even do inside a scripting engine that would require extreme performance that a mortal scripting engine cannot do?

I mean of course the engine shouldn't be extremely slow, but we are talking about Qt not some random dude on the Internet who coded the thing as a university project.

@daleglass
Copy link
Contributor

@JulianGro Regarding performance, I agree in that it's probably a very secondary concern for the time being. Given that we control the entire system, if we need something performance intensive we could just make an API for it, such as cryptographic functions.

Regarding "not enough control", for instance QJSEngine doesn't have the debugging and monitoring support that QScriptEngine does, so for instance, PR #1156 uses an API that's not available in QJSEngine.

We probably need control more than performance, for instance for monitoring execution and being able to deal with misbehaving ones. For example we'd definitely want to set some sort of resource usage limits, and QJSEngine doesn't seem to provide any way for doing so. That's not surprising, it's an engine definitely not planned for this use case.

I think QJSEngine still should be implemented, because it's almost certain to be the easiest thing to try by far. Then based on that we can see how much trouble we run into practice, and how important it would be to spend the time on something else. And most anything is going to be a good deal trickier because we're going to need some sort of Qt/JS translation mechanism. The ones included in Qt have the enormous advantage of already having one.

It also might make sense to consider other options. For instance perhaps WebAssembly would be an easier thing to deal with because I imagine that if compilation of code can be dealt with separately, it makes for a smaller and simpler library that may be less troublesome for us.

@odysseus654
Copy link
Contributor

Yes, with all the stuff I've abandoned i've been thinking about this (specifically V8, as I have the most experience with this engine)

Currently there is a single isolate generated for each domain and each interface. This means that all scripts are in the same namespace (and also makes it much more difficult for us to isolate and prevent memory leaks). Global variables set in one script will be available to any other script that the server/client is running at the time.

My thoughts are to make much more use of isolates, but hopefully in such a way that we don't blow memory requirements out of the water (especially if we have a script actively trying to crash the system). There is no obvious limit to the number of isolates that can exist although we should keep things down to a "sane" number ( https://stackoverflow.com/questions/65088028/v8-isolates-is-there-a-limit-to-the-number-of-instances-crashes-at-4-7k-insta )

My current thoughts are:

  • By default each script URL will be loaded into its own isolate. Each script can of course manage an unlimited number of entities.
  • There will be a maximum limit of isolates per URL (prior to any ? ). Beyond this limit isolates will be used for multiple scripts. This is to limit the amount of load something like http://nowhere.com/script/script.js?125235 can use
  • There will be a maximum limit of isolates per TLD
  • There will be a maximum limit of isolates per "context"

For "context" I'm thinking of the following, which also defines the APIs that scripts have access to:

  • Server entity scripts running within the domain
  • Client entity scripts running within the domain
  • Client entity scripts running locally within the user's UI.

@stale
Copy link

stale bot commented Nov 11, 2021

Hello! Is this still an issue?

@stale stale bot added the stale Issue / PR has not had activity label Nov 11, 2021
@daleglass
Copy link
Contributor

This will take a while, but still a thing

@stale stale bot removed the stale Issue / PR has not had activity label Jan 27, 2022
@stale
Copy link

stale bot commented Jul 27, 2022

Hello! Is this still an issue?

@stale stale bot added the stale Issue / PR has not had activity label Jul 27, 2022
@odysseus654
Copy link
Contributor

odysseus654 commented Jul 27, 2022 via email

@stale stale bot removed the stale Issue / PR has not had activity label Jul 27, 2022
@digisomni
Copy link
Member Author

It is still indeed, an issue. 🙃

@stale
Copy link

stale bot commented Jan 23, 2023

Hello! Is this still an issue?

@stale stale bot added the stale Issue / PR has not had activity label Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request scripting api change Change is made in the scripting API Severity: Medium Important functionality is affected, but a workaround exists stale Issue / PR has not had activity
Projects
None yet
Development

No branches or pull requests

5 participants