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

AppArmor: sandbox the content rendering processes (e10s) more strictly (Closes: #278) #280

Merged
merged 18 commits into from
Jan 28, 2018

Conversation

intrigeri
Copy link
Collaborator

I've been using this on my system for a few days, and the (adjusted) version we have in Tails passes our test suite.

…es (plugin-container).

This profile was copied as-is from torbrowser.Browser.firefox, and I updated the
name of the profile and the corresponding local include only.
… starting it, i.e. don't inherit Firefox' confinement.

We will later remove credentials plugin-container doesn't need, in order to
confine it more strictly. Such effort would be worthless if we kept inheriting
the permissions we grant the parent Firefox process.
Otherwise at least printing is broken.
…ory.

Otherwise at least printing to a PDF file in that directory fails.
…es that help making sound work.

Apparently these permissions are now needed by plugin-container, not by the
master firefox process.
…ory.

Otherwise e.g. printing to a PDF file fails.
@intrigeri
Copy link
Collaborator Author

Hold on, this seems to break downloading files.

@intrigeri
Copy link
Collaborator Author

False alert, I was running an older version with a bug that's fixed in this branch. Please review and merge :)

@intrigeri
Copy link
Collaborator Author

A month later, here's a gentle ping about this PR :)

@intrigeri
Copy link
Collaborator Author

Hi Micah!

Another 1.5 month later: ping? I've now been running Tor Browser with these changes for 2.5 months on my personal system, and didn't notice any regression.

Background: I would love to have this in Tails 3.2, whose code freeze is on September 14, but as usual I would greatly prefer not diverging from upstream :)

…cache.

Apparently it needs that to use & manage the cache.
With systemd (at least on current Debian sid), /run/shm is a symlink to
/dev/shm, so "owner /dev/shm/org.chromium.* rw," is enough. With sysvinit,
apparently things are set up differently (perhaps the symlinks are in the
opposite direction?) so Firefox tries to access /run/shm/org.chromium.*,
which was rejected.

Let's support both!

Thanks to gregor herrmann <gregoa@debian.org> for the bug report:
https://bugs.debian.org/874383

Note that this problem happens with pristine 0.2.8 profiles,
without the changes brought by my apparmor-e10s branch.
@intrigeri
Copy link
Collaborator Author

Hold on, this branch will need updates for Linux 4.14. Whenever you're in the mood for reviewing PRs please focus on #290, #291 and #294 first.

@micahflee
Copy link
Collaborator

Sorry about ignoring this for many, many, months @intrigeri. Can you verify that these these AppArmor changes are still current with Tor Browser 7.5?

@micahflee
Copy link
Collaborator

I just tested this, along with the other AppArmor PRs, with Tor Browser 7.5 and they all appear to work fine.

@micahflee micahflee merged commit 72d385f into torproject:master Jan 28, 2018
@intrigeri
Copy link
Collaborator Author

I just tested this, along with the other AppArmor PRs, with Tor Browser 7.5 and they all appear to work fine.

Strange. As said in #280 (comment) I think this branch needed updates especially once the PR for #294 is merged. I'll send a new PR about these updates ASAP.

@intrigeri intrigeri deleted the apparmor-e10s branch January 28, 2018 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants