1379803 - [Mac] Use vnode-type predicate in content sandbox to limit what type of files content can create

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement)

Description

[Haik Aftandilian [:haik]]

Content processes have permission to write to a few directories. For example, for printing. On the Mac, we can use the vnode-type predicate with (allow file-write-create ...) rules to prevent content from creating symlinks, directories, and ttys which we don't expect content to need to do. There are instances of this in /System/Library/Sandbox/Profiles, but otherwise it's not documented.

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

Attachment: Bug 1379803 - on macOS, only allow the creation of regular files and directories in writable directories;

This specifically disallows the creation of ttys, directories, and symlinks.

Review commit: https://reviewboard.mozilla.org/r/156140/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/156140/

Comment 2

[Haik Aftandilian [:haik]]

Comment on attachment 8885272 [details]
Bug 1379803 - on macOS, only allow the creation of regular files and directories in writable directories;

https://reviewboard.mozilla.org/r/156140/#review161258

r+

s/writably/writable in commit message.

::: security/sandbox/test/browser_content_sandbox_fs.js:224
(Diff revision 1)
>      // content process successfully created the file, now remove it
>      homeFile.remove(false);
>    }
>  }
>  
>  // Test if the content process can create a temp file, should pass

Update comment

::: security/sandbox/test/browser_content_sandbox_fs.js:235
(Diff revision 1)
>    // now delete the file
>    let fileDeleted = await ContentTask.spawn(browser, path, deleteFile);
>    if (isMac()) {
>      // On macOS we do not allow file deletion - it is not needed by the content
>      // process itself, and macOS uses a different permission to control access
>      // to revoking it is easy.

s/to/so

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

dom/plugins/test/mochitest/test_pluginstream_asfile.html appear to be a legit failing test, looks like for plugins we lazilly create a plugin temp directory (dom/plugins/base/nsPluginHost.cpp line ~693). For now I think the easiest thing to do is allow directory creation -- unless there's an alternate suggestion.

Comment 4

[Haik Aftandilian [:haik]]

(In reply to Alex Gaynor [:Alex_Gaynor] from comment #3)
> dom/plugins/test/mochitest/test_pluginstream_asfile.html appear to be a
> legit failing test, looks like for plugins we lazilly create a plugin temp
> directory (dom/plugins/base/nsPluginHost.cpp line ~693). For now I think the
> easiest thing to do is allow directory creation -- unless there's an
> alternate suggestion.

Allowing directory creation sounds fine for now, but I wonder if pluginstream is something we don't support anymore given our trajectory with plugins (yes, wishful thinking) similar to what happened with bug 1360223. :bsmedberg, is this something you're familiar with?

Comment 5

[Benjamin Smedberg]

Yes, bug 1352567.

Comment 6

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8885272 [details]
Bug 1379803 - on macOS, only allow the creation of regular files and directories in writable directories;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/156140/diff/1-2/

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

Thanks :bsmedberg. I've uploaded a patch which adds directories to the allowed vnode-types, after this is landed I'll file a bug to remove directories, blocked on that bug.

Comment 8

[Alex Gaynor [:Alex_Gaynor]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=24fd6a724e445e6c7a023515d4c236c1b0c7e96b

Looks good!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/566036f11442
on macOS, only allow the creation of regular files and directories in writable directories; r=haik

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/566036f11442