Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Executor: Create runtime from artifact in-memory rather than on-disk? #13861

Closed
mrcnski opened this issue Apr 9, 2023 · 6 comments · Fixed by #14184
Closed

Executor: Create runtime from artifact in-memory rather than on-disk? #13861

mrcnski opened this issue Apr 9, 2023 · 6 comments · Fixed by #14184
Assignees
Labels
I7-refactor Code needs refactoring.

Comments

@mrcnski
Copy link

mrcnski commented Apr 9, 2023

ISSUE

Overview

Currently on the Polkadot side we call create_runtime_from_artifact, which takes a path to a pre-compiled artifact. However, this runs in its own thread which we want to sandbox for security, and it would be nice if we could disable all file-system access. For that reason I'd like to be able to read the artifact ahead of time, and have a way to pass in the bytes directly to runtime instantiation.

(This would also help with #13860, since we could handle file-not-found on the Polkadot side without relying on un-matchable String errors from Substrate.)

@mrcnski mrcnski added the I7-refactor Code needs refactoring. label Apr 9, 2023
@bkchr
Copy link
Member

bkchr commented Apr 9, 2023

Yes we can do this.

However, this runs in its own thread which we want to sandbox for security, and it would be nice if we could disable all file-system access.

Can you please explain why? We already run the pvf stuff in its own worker, I don't get why you want to run it in its own sandboxed thread?

@mrcnski
Copy link
Author

mrcnski commented Apr 9, 2023

Sure, good question. We are indeed running each PVF job in its own thread on a separate worker process. But AFAIK we have no security protections around the worker, and the PVF is untrusted code (especially so with parathreads). Per paritytech/polkadot-sdk#882, we want to run the thread with at least a full seccomp jail. Moving filesystem stuff outside the thread means less syscalls that we have to let through, and a smaller surface area for attacks.

@bkchr
Copy link
Member

bkchr commented Apr 9, 2023

Per paritytech/polkadot-sdk#882, we want to run the thread with at least a full seccomp jail. Moving filesystem stuff outside the thread means less syscalls that we have to let through, and a smaller surface area for attacks.

AFAIK seccomp is for the entire process and not only for individual threads. We already have the worker process that should run under the same permissions for everything.

@mrcnski
Copy link
Author

mrcnski commented Apr 11, 2023

@bkchr seccomp filters can be applied to a single thread. I have tested and confirmed it.

@bkchr
Copy link
Member

bkchr commented Apr 11, 2023

Okay ty, still the question why we need seccomp filters on thread level? If we already have a worker process that should be running in a secured env.

@mrcnski
Copy link
Author

mrcnski commented Apr 11, 2023

We already are executing the untrusted code in its own thread, and it's better to apply the filter as fine-grained as possible -- then there's less syscalls we have to whitelist.

Or are you asking why we need seccomp at all? (I'm not sure what you mean by secured env.) To my knowledge we don't really secure the worker process in any way; but that is a discussion for paritytech/polkadot-sdk#882 I think.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants