worker: add option to share resources

[kevinkassimo]

Add an option shareResources (default false) that when true, child worker would be able to share parent resource table and interact with corresponding resource id.

This allows simple connection-based http server clustering using workers (With this PR I have attempted a minimal worker cluster, and seems to show better performance with large amount of parallel connections compared to basic single worker case)

[bartlomieju]

Cool! We had a chat sometime ago with @ry about sharing resources between workers.

We were thinking about inheriting parent resources (behaviour a'la process in POSIX and as implemented in this PR), as well as sharing resources per-resource basis (in that way we could store Arcs to resources which can be copied over to child worker resource table) by sending resource to other worker using postMessage.

That said I think we should defer on implementing sharing resources until we implement workers that are more aligned to spec (#3379, #3396, #3557, #3020, #2444). That refactor is currently blocked by upgrade to newer Tokio (#3418) as well as using rusty_v8 (#3556). Otherwise we'll be exposing non-standard functionality (which is debatable IMHO) before we even have properly working workers.

WDYT?

[kevinkassimo]

@bartlomieju Sounds good to wait for a while.

I am curious what is your plan about sending resources through postMessage? In browsers MessagePort could be forwarded through postMessage which might have some resemblance, but since we rely heavily on rid as a number, it might be slightly problematic (either we might have a number like 1 representing 2 different resources between workers -- making which resource forwarded a bit unclear, or that the resources are always following an ever incrementing counter -- which might be a problem too since this technically leaks information of parent behavior if we were ever to use Worker in a sandboxed way, even if the resource table is not shared) (maybe we can register a resource in parent worker to a global registry that returns an unpredictable string token, and send this string to the child worker so it can use it to retrieve from the registry the same resource while still assigning rid independently starting from 1).

That being said, while I do feel sharing the whole resource table might be too coarse grained (all or none), it seems to me that our current read/write/net perm design is not really better: a forwarded resource still follows our state perm check -- either it is denied op (due to possible Worker perm inheritance or hidden Deno namespace), or the child worker can do the op without the need of the resource (through e.g. open the same file itself). In that sense, resource forwarding is just an optimization technique rather than a security measure.

(We still have the fallback to make the main worker privileged and child worker completely/partially unprivileged, and child request main worker to perform certain privileged op through plain message passing)

[bartlomieju]

@bartlomieju Sounds good to wait for a while.

I am curious what is your plan about sending resources through postMessage? In browsers MessagePort could be forwarded through postMessage which might have some resemblance, but since we rely heavily on rid as a number, it might be slightly problematic (either we might have a number like 1 representing 2 different resources between workers -- making which resource forwarded a bit unclear, or that the resources are always following an ever incrementing counter -- which might be a problem too since this technically leaks information of parent behavior if we were ever to use Worker in a sandboxed way, even if the resource table is not shared) (maybe we can register a resource in parent worker to a global registry that returns an unpredictable string token, and send this string to the child worker so it can use it to retrieve from the registry the same resource while still assigning rid independently starting from 1).

Using random identifier (UUID?) seems like a low-hanging fruit, but definitely this has to be figured out.

That being said, while I do feel sharing the whole resource table might be too coarse grained (all or none), it seems to me that our current read/write/net perm design is not really better: a forwarded resource still follows our state perm check -- either it is denied op (due to possible Worker perm inheritance or hidden Deno namespace), or the child worker can do the op without the need of the resource (through e.g. open the same file itself). In that sense, resource forwarding is just an optimization technique rather than a security measure.
(We still have the fallback to make the main worker privileged and child worker completely/partially unprivileged, and child request main worker to perform certain privileged op through plain message passing)

I don't agree 100%, your original scenario where you accept connections in main worker and forward resources to child workers seems like a good example. Granted, each worker could accept by itself, but it's a great example on how to leverage multiple threads. There is also plan to make child worker permissions very granular (perms option in constructor, where one can specify them as needed, part of making workers completely deterministic). That said, there are a lot of question with not so many answers (yet!), so again, let's just focus on making workers more spec compliant.

[kevinkassimo]

@bartlomieju

I don't agree 100%, your original scenario where you accept connections in main worker and forward resources to child workers seems like a good example. Granted, each worker could accept by itself, but it's a great example on how to leverage multiple threads.

Yeah actually that is true, since technically the existence of an accepted connection can be viewed as a parent worker only state that could be made visible to child only by explicit message.

There is also plan to make child worker permissions very granular (perms option in constructor, where one can specify them as needed, part of making workers completely deterministic). That said, there are a lot of question with not so many answers (yet!), so again, let's just focus on making workers more spec compliant.

When I originally experimented with Worker sandboxing in #2762 (I also privately had an updated version in October, but abandoned as I was busy in grad school), what I did was allowing user to pass whitelists in Worker constructor, which would be compared with parent list and forcefully restrict to a subset. Does that sound close to your idea? (Actually I think I mentioned about some funnier cases here)

Actually I'm kind of worried that perm lists are likely to become quite ugly in many use scenarios, like treating a Worker as a service (telling worker to only open a given filename, and perform some tasks on its behave, and nothing else. Sometimes these filenames are runtime-only information and cannot be predetermined through a whitelist. We will need API for parent to initiate permission upgrades in child). Maybe a whitelist based main worker + unforgeable reference/token for op (like object capability model) in child workers (parent open the file with correct read/write perm and generates a token, and just send this token to the child) would sound more pragmatic. But, alas, I feel this might collide with our current model badly and would be quite some implementation burden.

BTW I am personally not too concerned about "non-standard" behavior over our extra options like noDenoNamespace or shareResources: we are not really violating existing spec here since the option is more of an "extension", just like the existence of Deno namespace itself.

[bartlomieju]

There is also plan to make child worker permissions very granular (perms option in constructor, where one can specify them as needed, part of making workers completely deterministic). That said, there are a lot of question with not so many answers (yet!), so again, let's just focus on making workers more spec compliant.

When I originally experimented with Worker sandboxing in #2762, what I did was allowing user to pass whitelists in Worker constructor, which would be compared with parent list and forcefully restrict to a subset. Does that sound close to your idea? (Actually I think I mentioned about some funnier cases here)

Yes, that's what I had in mind.

Actually I'm kind of worried that perm lists are likely to become quite ugly in many use scenarios, like treating a Worker as a service (telling worker to only open a given filename, and perform some tasks on its behave, and nothing else. Sometimes these filenames are runtime-only information and cannot be predetermined through a whitelist). Maybe a whitelist based main worker + unforgeable reference/token for op (object capability model) in child workers (parent open the file with correct read/write perm and generates a token, and just send this token to the child) would sound more pragmatic. But, alas, I feel this might collide with our current model badly and would be quite some implementation burden.

I think you already opened an issue to handle that (#2620).

BTW I am personally not too concerned about "non-standard" behavior over our extra options like noDenoNamespace or shareResources: we are not really violating existing spec here since the option is more of an "extension", just like the existence of Deno namespace itself.

That's correct... TBD

[kevinkassimo]

@bartlomieju
Yeah that was my idea in #2620, though was mostly module based. I guess there would be less hassle with workers (I was worried about Deno.core API overwrite attack -- not viable cross Workers due to different contexts), but anyways to implement that would require a dual security mechanism (or a complete switch)... I fear too high cost unless all major parties agree it is worth it...

[ry]

Is this necessary? Can you add a comment describing why this needs to be done?

[kevinkassimo]

There is a small problem in our worker design: currently we require calling workerClose() to close a worker. However, it turns out due to structure of workerMain(), it is only effective when workerClose() is called inside onmessage (otherwise it would just get stuck at await getMessage(). Maybe a Promise.race between getMessage and a Promise registered to Rust side about if worker is closed can solve this), which I don't have on the workers here.

I'll work around with this limitation using onmessage in this PR since they are not necessarily related, and that Bartek mentioned about Worker API conforming refactoring (that we are likely be get rid of workerClose soon)

[bartlomieju]

Thanks @kevinkassimo, I'm gonna close it for now, but let's revisit this topic after we figure out workers fully (structured clone, libs).