AppArmor profiles, 2018-01 edition #310

intrigeri · 2018-01-29T08:29:50Z

This branch includes:

some follow-ups to AppArmor: sandbox the content rendering processes (e10s) more strictly (Closes: #278) #280 and Apparmor: avoid breakage with Linux 4.14 #294 to make e10s work fine especially with a Linux 4.14 kernel
silencing all the denial logs I could observe
support for obfs4 and obfs3
various updates, refactoring and clean-ups

Applying this upgrade to a running system requires special care, see the notes to packagers in the commit messages, that should IMO be copied to the release notes.

…tion. We already allow the main browser profile to do that but with e10s plugin-container now needs it as well.

…m signals. With e10s Firefox does not need to ptrace itself anymore but instead it needs to ptrace and kill its child plugin-container processes.

…ent Firefox process. We already allow Firefox to send term signals to plugin-container; this is the receiving counterpart. This requires giving the Firefox profile a proper name (torbrowser_firefox) because this: signal (receive) set=("term") peer=/home/*/.local/share/torbrowser/tbb/{i686,x86_64}/tor-browser_*/Browser/firefox … does not work. Note to package maintainers =========================== (This should probably be copied to the release notes.) Due to the profile renaming, upgrading the /etc/apparmor.d/torbrowser.Browser.firefox file requires special care. The best option is probably to strongly recommend users to reboot their system after this upgrade. Other options I can think of have unacceptable consequences: - if we unload the old profile from the kernel, we will leave any already running Tor Browser's Firefox executable unconfined, which is an unacceptable violation of the user's security expectations; - if we don't unload the old profile from the kernel, surprising behaviour will happen such as: - any already running Tor Browser's Firefox executable will be left confined under the old profile which won't play well with new rules that have peer=torbrowser_firefox; - unpredictable behavior when a new Tor Browser is started, because two profiles matching the Tor Browser's Firefox executable are loaded.

So far we allowed it to do everything in there except a link operation, so let's be consistent.

…es to read. Same rationale as commit 68f502c.

We don't currently allow access to the audio subsystem; let's not let AppArmor spam the logs about it.

This will allow us to handle upgrades more nicely in the future, e.g. when the executable path changes. Besides, this makes the output of aa-status and logs much easier to grasp. Note to packagers: exactly as for the similar change applied to the Tor Browser's Firefox profile, please consider recommending users to reboot their system after the upgrade that applies this change.

This fixes support for obfs4 and obfs3. meek and fte require vastly more extended permissions and thus dedicated child profiles.

This matches how recent dh-apparmor behaves.

ageis · 2018-03-10T13:39:46Z

just making a note/reminder, I can help test this branch this weekend. @micahflee

geor-g · 2018-03-17T20:25:24Z

@intrigeri Thanks for the work on this! Should this be tested?

intrigeri · 2018-03-18T09:03:14Z

@intrigeri Thanks for the work on this! Should this be tested?

Yes, please :)

rogers0 · 2018-03-18T14:32:39Z

@intrigeri I tried the bridge feature, and yes, obfs4 and obfs3 are working after applying your patches. Thank you!

But meek and fte still got the kernel audit error as below.
Could you kindly help to fix these?
BTW. My testing system is debian stretch.

[1296232.653070] audit: type=1400 audit(1521382524.268:121): apparmor="DENIED" operation="exec" profile="torbrowser_tor" name="~/.local/share/torbrowser/tbb/x86_64/tor-browser_en-US/Browser/firefox" pid=26051 comm="meek-client-tor" requested_mask="x" denied_mask="x" fsuid=1000 ouid=1000
[1296832.242735] audit: type=1400 audit(1521383123.883:123): apparmor="DENIED" operation="exec" profile="torbrowser_tor" name="/usr/bin/python2.7" pid=26535 comm="fteproxy.bin" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0

intrigeri · 2018-03-18T14:39:28Z

Roger Shimizu:

But meek and fte still got the kernel audit error as below. Could you kindly help to fix these?

I think meek and fte will need much more work so let's file a dedicated issue and discuss it there, OK?

rogers0 · 2018-03-18T15:33:51Z

@intrigeri sure.
Good to know that you're already aware of this.

support for obfs4 and obfs3

I confirm obfs4 and obfs3 work well after this patchset.
Looking forward to seeing this gets merged.

micahflee · 2018-03-19T08:42:41Z

@intrigeri as we discussed, I've just added you as a collaborator and code owner of /apparmor/: 0e9db70

While it's great to have extra code reviews and testing of the AppArmor profiles, I'm happy with you merging them directly into develop if you think they are ready.

intrigeri · 2018-03-19T09:02:56Z

Micah Lee:

@intrigeri as we discussed, I've just added you as a collaborator and code owner of `/apparmor/`: 0e9db70 While it's great to have extra code reviews and testing of the AppArmor profiles, I'm happy with you merging them directly into develop if you think they are ready.

Excellent, thank you!

rogers0 · 2018-03-22T11:09:11Z

@intrigeri
since the last commit you pushed in this PR

91652b6
We have to amend setup.py a bit.
I have created the patch, enclosed in the commit below:
https://anonscm.debian.org/git/pkg-privacy/packages/torbrowser-launcher.git/commit/?h=rosh/apparmor&id=ae0070a

I'm not sure how to add the patch after your commits in this PR.
Maybe it's better to start a new PR, after you merge this series.

intrigeri · 2018-03-24T07:54:50Z

Hi @rogers0! Roger Shimizu:

since the last commit you pushed in this PR - 91652b6 We have to amend setup.py a bit. I have created the patch, enclosed in the commit below: - https://anonscm.debian.org/git/pkg-privacy/packages/torbrowser-launcher.git/commit/?h=rosh/apparmor&id=ae0070a

I'm afraid I don't understand. My PR empties these files, it does not remove them, and the idea is to keep installing them through `setup.py`. Could you please send me details about the FTBFS? I suspect it's caused by some limitation of the packaging tools we use to maintain `debian/patches/` and won't happen once my PR is part of a new upstream release, but that's just a guess.

micahflee · 2018-03-26T18:41:24Z

I'm getting ready to make the 0.3.0 release and I'd love to include these updated AppArmor profiles in it. The develop branch has now merged in major-refactor.

@rogers0 @intrigeri would you mind checking to make sure this PR works with what's in develop and let me know, and then I can merge this into develop and make the release?

rogers0 · 2018-03-27T13:04:11Z

@intrigeri
please check the log below.
I think the reason is that setup.py simply refuses to copy a null file.

creating <working-dir>/torbrowser-launcher/debian/torbrowser-launcher/etc
creating <working-dir>/torbrowser-launcher/debian/torbrowser-launcher/etc/apparmor.d
copying apparmor/torbrowser.Browser.firefox -> <working-dir>/torbrowser-launcher/debian/torbrowser-launcher/etc/apparmor.d/
copying apparmor/torbrowser.Browser.plugin-container -> <working-dir>/torbrowser-launcher/debian/torbrowser-launcher/etc/apparmor.d/
copying apparmor/torbrowser.Tor.tor -> <working-dir>/torbrowser-launcher/debian/torbrowser-launcher/etc/apparmor.d/
creating <working-dir>/torbrowser-launcher/debian/torbrowser-launcher/etc/apparmor.d/local
error: can't copy 'apparmor/local/torbrowser.Browser.firefox': doesn't exist or not a regular file

@micahflee thanks! I'll let you know when I finish.

intrigeri · 2018-03-29T08:27:49Z

I think the reason is that setup.py simply refuses to copy a null file.

@rogers0 actually when that failure happens the source file does not exist so setup.py cannot copy it. That's because the patch format we use does not allow expressing the distinction between emptying a file and deleting it so when debian/patches/0015-AppArmor-remove-boilerplate-from-local-override-file.patch is applied, those files are deleted. So once that patch makes it into an upstream release and the corresponding quilt patch gets removed, it won't cause a FTBFS anymore => case closed. But still, even then I think we'll want to keep debian/patches/0016-Remove-apparmor-local-path-from-setup.py.patch (albeit with an updated description): let's not treat these local overrides as conffiles and instead let dh-apparmor manage them.

intrigeri · 2018-03-29T09:27:08Z

@rogers0 @intrigeri would you mind checking to make sure this PR works with what's in `develop`

@micahflee, tested, it works fine for me.

and let me know, and then I can merge this into `develop`

Please go ahead (or ask me to merge it myself: apparently I can since this PR only modifies paths to which I now have commit access to :)

micahflee · 2018-04-06T20:18:12Z

Thanks for the merge @intrigeri :)

… to silence logs. We already allow the main browser profile to do that but with e10s plugin-container now needs it as well. This backports part of commit cdb290f from torproject#310.

intrigeri added 14 commits January 29, 2018 08:24

Drop spurious trailing whitespace.

064ad1f

AppArmor: allow plugin-container to read file/app association informa…

cdb290f

…tion. We already allow the main browser profile to do that but with e10s plugin-container now needs it as well.

AppArmor: allow Firefox to ptrace plugin-container and to send it ter…

9c60947

…m signals. With e10s Firefox does not need to ptrace itself anymore but instead it needs to ptrace and kill its child plugin-container processes.

Fix comment.

f409317

AppArmor: allow Firefox to fully manage its fontconfig cache.

936aee5

So far we allowed it to do everything in there except a link operation, so let's be consistent.

AppArmor: grant access to mostly innocuous stuff plugin-container tri…

dedd45b

…es to read. Same rationale as commit 68f502c.

AppArmor: silence denial logs about PulseAudio.

27289e1

We don't currently allow access to the audio subsystem; let's not let AppArmor spam the logs about it.

AppArmor: silence more inherited files access denial.

0109e95

AppArmor: drop support for long-obsolete paths.

b1e082f

AppArmor: refactor thanks to variables defined in tunables/torbrowser.

a9bef63

AppArmor: support some of the included pluggable transports.

850d560

This fixes support for obfs4 and obfs3. meek and fte require vastly more extended permissions and thus dedicated child profiles.

AppArmor: remove boilerplate from local override files.

91652b6

This matches how recent dh-apparmor behaves.

micahflee changed the base branch from master to develop February 27, 2018 05:26

micahflee added this to the 0.3 milestone Mar 26, 2018

intrigeri merged commit 8a76256 into torproject:develop Apr 6, 2018

intrigeri deleted the apparmor-201801-edition branch April 6, 2018 07:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AppArmor profiles, 2018-01 edition #310

AppArmor profiles, 2018-01 edition #310

intrigeri commented Jan 29, 2018

ageis commented Mar 10, 2018

geor-g commented Mar 17, 2018

intrigeri commented Mar 18, 2018 via email

rogers0 commented Mar 18, 2018

intrigeri commented Mar 18, 2018 via email

rogers0 commented Mar 18, 2018

micahflee commented Mar 19, 2018

intrigeri commented Mar 19, 2018 via email

rogers0 commented Mar 22, 2018

intrigeri commented Mar 24, 2018 via email

micahflee commented Mar 26, 2018

rogers0 commented Mar 27, 2018 •

edited

Loading

intrigeri commented Mar 29, 2018

intrigeri commented Mar 29, 2018 via email

micahflee commented Apr 6, 2018

AppArmor profiles, 2018-01 edition #310

AppArmor profiles, 2018-01 edition #310

Conversation

intrigeri commented Jan 29, 2018

ageis commented Mar 10, 2018

geor-g commented Mar 17, 2018

intrigeri commented Mar 18, 2018 via email

rogers0 commented Mar 18, 2018

intrigeri commented Mar 18, 2018 via email

rogers0 commented Mar 18, 2018

micahflee commented Mar 19, 2018

intrigeri commented Mar 19, 2018 via email

rogers0 commented Mar 22, 2018

intrigeri commented Mar 24, 2018 via email

micahflee commented Mar 26, 2018

rogers0 commented Mar 27, 2018 • edited Loading

intrigeri commented Mar 29, 2018

intrigeri commented Mar 29, 2018 via email

micahflee commented Apr 6, 2018

rogers0 commented Mar 27, 2018 •

edited

Loading