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

Split Servo into multiple processes, and introduce simple sandboxing #4735

Closed
wants to merge 7 commits into from

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Jan 27, 2015

The sandboxing will be incomplete until the resource task is rewritten. However, it's a good start—no GPU access in the content process, for example.

Do not merge until after the Rust upgrade.

r? @jdm (overall design)
r? @kmcallister (Linux sandbox)
r? @zwarich (Mac stuff)

Review on Reviewable

pcwalton added 7 commits Jan 27, 2015
Until we rewrite the resource task, we have to allow outbound network
connections. This should be done soon, however.
Until we rewrite the resource task, we have to allow outbound network
connections. This should be done soon, however.

I'm not sure how to restrict access to specific files on the filesystem
without root privileges.
@highfive
Copy link

highfive commented Jan 27, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jan 27, 2015

Critic review: https://critic.hoppipolla.co.uk/r/3843

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Feb 10, 2015

Also would like to make sure that this works on Android & Gonk before merging this, as I mentioned in the weekly meeting :)

@jdm
Copy link
Member

jdm commented Mar 2, 2015

Reviewed everything but the two commits implementing sandboxing, which are presumably going to https://github.com/pcwalton/gaol/.

@metajack
Copy link
Contributor

metajack commented Mar 31, 2015

Assigning to @SimonSapin, though it looks like @pcwalton just needs to fix a few things and rebase.

@SimonSapin
Copy link
Member

SimonSapin commented Apr 21, 2015

The code left to review is the sandbox, which I don’t really understand. Reassigning to @kmcallister.

@pcwalton, could you clarify if https://github.com/pcwalton/gaol/ should be used, like Josh suggests?

@SimonSapin SimonSapin assigned kmcallister and unassigned SimonSapin Apr 21, 2015
@kmcallister
Copy link
Contributor

kmcallister commented Apr 21, 2015

I think the plan is to use gaol, and I've reviewed the Linux sandbox portion of that.

@l0kod
Copy link

l0kod commented May 17, 2015

From the Site Isolation Summit, Chrome site isolation is evolving the process-per-site-instance to a new security model with Out-of-Process IFrames (OOPIF): protect sites from each other.

@pcwalton
Copy link
Contributor Author

pcwalton commented May 20, 2015

This is blocked on the rustup because I want to switch to serde.

@metajack
Copy link
Contributor

metajack commented Jul 29, 2015

I assume this is now blocked on e10s landing?

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 29, 2015

Yes. I've got a redone version of this patch locally that's waiting on all
the e10s pieces to land.
On Jul 29, 2015 12:57 PM, "Jack Moffitt" notifications@github.com wrote:

I assume this is now blocked on e10s landing?


Reply to this email directly or view it on GitHub
#4735 (comment).

@jdm
Copy link
Member

jdm commented Oct 28, 2015

Closed in favour of #6884.

@jdm jdm closed this Oct 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
You can’t perform that action at this time.