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

Sandboxing #67

Open
lofcz opened this issue Mar 16, 2023 · 13 comments
Open

Sandboxing #67

lofcz opened this issue Mar 16, 2023 · 13 comments

Comments

@lofcz
Copy link

lofcz commented Mar 16, 2023

I've summarily checked the codebase and it seems there aren't many options to sandbox the scripts. Is this in the scope of this project?
If so, it would be great to be able to do the following (in descending priority):

  • limit CLR interop on a whitelist basis
  • limit the number of instructions to be executed
  • limit total memory allocated
  • limit the recursion depth
  • limit the total execution time
@ackava
Copy link
Contributor

ackava commented Mar 18, 2023

@lofcz YantraJS currently isn't designed to run in a browser like sandboxing. The purpose was to create Node like environment to create native apps and server apps where scripts are trusted.

  1. Limiting CLR interop on a whitelist basis is in development.
  2. Limiting the number of instructions to be executed - this one I have no idea how to do it as we have no control over CLR, however whielisting CLR interop will give you enough control on what to expose and what not to.
  3. Memory is again not in our control, unless some CLR API provides this, abstracting memory and providing memory control will slow down every execution and will defeat the purpose of making faster engine. Process isolation can provide memory control and it is better suited for browser like execution.
  4. Limting recursion might be an easy thing to implement. However it is not in current scope as we are working for debugger and other runtime features.
  5. Limiting execution time is an important feature, but .Net core has removed ability to abort the thread, thus checking timeout on every loop will slow the execution. We are searching some easy or faster ways to do it.

We hope more features will come into YantraJS as it gets more popular and we get some help from open source community.

@lofcz
Copy link
Author

lofcz commented Oct 4, 2023

@ackava would you please update this issue if any of the points get implemented? We are still interested in YantraJS but sandboxing is a showstopper for us.

@ackava
Copy link
Contributor

ackava commented Oct 5, 2023

@lofcz Hi, Can you give an example of how you want to sandbox the execution?

Currently, if you disable CLR, and if you create your own class derived from JSObject and your own JSFunction instances in global context, scripts will not be able to access anything. JSContext has no access to CLR, JSModuleContext has ability to turn off CLR.

Once you provide the example, I will provide you source code to achieve it, or else I will add improvement in JSContext/JSModuleContext to support it.

@lofcz
Copy link
Author

lofcz commented Oct 5, 2023

Thanks for the fast and kind reply, currently the CLR integration is on an enable/disable basis, for our use case, we need to enable CLR for a subset of classes in the calling assembly, essentially point (1) in your previous answer. This is the bare minimum needed for us. I imagine I'd provide an ICollection or a similar interface of classes to enable CLR interop onto the JSModuleContext, that would be the optimal solution for the end-user.

We are also interested in whether there is any new know-how on points (4) and (5) above, but these are nice to haves that would be most welcome but are not strictly necessary for the first version of sandboxing.

@ackava
Copy link
Contributor

ackava commented Oct 5, 2023

@lofcz

Point 1 was implemented as decorating [JSExport] on members of a class derived from JavaScriptObject.

https://github.com/yantrajs/yantra/blob/main/YantraJS.Core.Tests/ClrObjects/CustomObject.cs

(4) I haven't thought about it yet, but yes, it is not that difficult to implement, I have created a task for it and will try to do it over weekend.

(5) There is no way to Abort thread in .NET core, so this is kind of deadend. Unless we find some workaround, we have no way to track this. However you can use cancellation token and Task.Any to find out which one finished first, but there is no way to stop execution. We are still constantly looking for alternative.

@lofcz
Copy link
Author

lofcz commented Oct 5, 2023

Thanks! As for (1), I wonder whether it would be possible to also implement the proposed way of whitelisting in my previous comment as we need to whitelist different classes based on the authentication of the end user. That shouldn't be that hard I imagine?

@ackava
Copy link
Contributor

ackava commented Oct 5, 2023

@lofcz That is not possible as the IL is generated only first time, and it runs as native code. IL remains same throughout the lifetime of process. And there are some upcoming optimizations which will break any conditional IL generation such as tail call optimization, reuse same IL if the code is same without any closures, cache results of methods and loop optimizations.

I would recommend using source generator to create proxy on top of what you want to expose which can enable/disable access based on authentication.

@lofcz
Copy link
Author

lofcz commented Oct 5, 2023

Thanks, I have only limited experience with source generators as they are fairly new feature to the language. Is this something simple? If it is, could you write a minimal example, please?

@ackava
Copy link
Contributor

ackava commented Oct 5, 2023

@lofcz No, I don't think source generator will help. Sorry for misdirecting towards it, you will have to perform checks before start of method if user has access or not as shown below.

   class JSFile : JavaScriptObject {

        [JSExport]
        public JSFile WriteText(string text) {
               ThrowIfAccessDenied(... );
        }
   }

@lofcz
Copy link
Author

lofcz commented Oct 5, 2023

Thanks, got it. Seems like a lot of boilerplate, the checks themselves should be just a dictionary lookup in our case. One more idea, I'm not familiar with YantraJS CLR interop code but isn't it by chance possible to implement something like:

when the .NET method is resolved in a CLR interop layer before we invoke it, we call another method provided say to JSModuleContext as a Func<MethodInfo, bool> (given this delegate is not null, else we skip this new logic). The method would essentially wrap the resolved method and if we return false from the wrapper, we skip the inner method invocation.

Now if this is possible it's a costly way to do things but we could improve this approach and hopefully reduce it to a dictionary lookup.

@ackava
Copy link
Contributor

ackava commented Oct 5, 2023

@lofcz No it is not possible, because IL generator will have to do dictionary look up for every methods.

But it is still not correct way, because same method will be used for some different user's context and if it works for first user, it may not be accessible to second user. Whether user has access to resource or not should be checked in beginning of method, not when the method is compiled.

Even when you pass new instance of method, JavaScript class remains same, the function called is same.

@lofcz
Copy link
Author

lofcz commented Oct 5, 2023

Alright, understood. I very much appreciate the time you've given me. I'll look into how the boilerplate could be avoided, probably with weaving but that's outside of the scope of this project. Once again, thanks ❤️

@ackava
Copy link
Contributor

ackava commented Oct 21, 2023

@lofcz I have released new version with support of stack overflow. However, compared to V8, stack size available is only 1/8th, that is due to lot of extra calls placed in .net code generation, which will improve in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants