Closed Bug 1339105 Opened 3 years ago Closed 3 years ago

Implement Windows Level 3 content process sandbox

Categories

(Core :: Security: Process Sandboxing, enhancement)

All
Windows
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

References

(Blocks 1 open bug)

Details

(Whiteboard: sbwc2)

Attachments

(3 files)

Implement Level 3 content process sandbox, which should be the same as level 2, but with access token level USER_LIMITED and job level JOB_RESTRICTED.

This will mean adding rules to allow read access to chrome and extensions profile directories.
Also, write access to content temp directory.
Whiteboard: sbwc3
Whiteboard: sbwc3 → sbwc2
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c42f806b634c28c557bc8d42b34269c714f5449c

xpcshell failure there is because it registers a directory provider after we've already called for the file in question and so it's already cached.

I have a fix that undefines and sets the property on the directory service instead, not sure if there will be other instances of this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=248f8fe7f003554d265ab9998063d4e14c45d898
MozReview-Commit-ID: L8wcVhdLvFe
Attachment #8869429 - Flags: review?(jmathies)
This also fixes the cleanup to fully remove the fake path and put back original if it existed.
Attachment #8869430 - Flags: review?(mak77)
This also removes a rule that was added for sandboxing the Java plugin,
which we never did and we now only allow Flash anyway.
Attachment #8869431 - Flags: review?(jmathies)
Attachment #8869429 - Flags: review?(jmathies) → review+
Attachment #8869431 - Flags: review?(jmathies) → review+
Blocks: 1366694
Comment on attachment 8869430 [details] [diff] [review]
Part 2: Fix registerFakePath in head_migration.js to work when the key has already been accessed

Review of attachment 8869430 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/migration/tests/unit/head_migration.js
@@ +68,5 @@
> +    // To clean up just undefine.
> +    do_register_cleanup(() => {
> +      dirsvc.undefine(key);
> +    });
> +  }

nit: I think it would be a little bit more readable and compact as:
let originalFile;
try {
  // If a file is already cached undefine to set ours.
  originalFile = dirsvc.get(key, Ci.nsIFile);
  dirsvc.undefine(key);
} catch (e) { /* file doesn't exist * / }

// Set our file.
dirsvc.set(key, file);

do_register_cleanup(() => {
  dirsvc.undefine(key);
  if (originalFile)
    dirsvc.set(key, originalFile);
});
Attachment #8869430 - Flags: review?(mak77) → review+
Thanks both for the reviews.

(In reply to Marco Bonardo [::mak] from comment #5)
> Comment on attachment 8869430 [details] [diff] [review]
> Part 2: Fix registerFakePath in head_migration.js to work when the key has
> already been accessed
...
> nit: I think it would be a little bit more readable and compact as:
...

Yeah, I toyed with doing it that way, so happy to change it.
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/50bf4c923818
Part 1: Implement Windows Level 3 content process sandbox policy. r=jimm
https://hg.mozilla.org/integration/mozilla-inbound/rev/964b6ee8ec32
Part 2: Fix registerFakePath in head_migration.js to work when the key has already been accessed. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/367734cc9370
Part 3: Move NPAPI windows process sandbox file rules into SandboxBroker. r=jimm
Backed out for Windows bustage: calling protected constructor of class 'nsAString' at sandboxBroker.cpp(208,11):

https://hg.mozilla.org/integration/mozilla-inbound/rev/3df269387c8cf2b969b928f20c1c5a9d542073ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f389e643ba52e15c1c0a6c5deaa75bd764e4b9d
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5145aab4731280a9713896c08ea94c2e5dace30

Push with failures (only look at the Windows ones): https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=367734cc9370f2528dc564921e3d678cb352f514&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=100929062&repo=mozilla-inbound

13:47:13     INFO -  z:/task_1495457102/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp(208,11):  error: calling a protected constructor of class 'nsAString'
13:47:13     INFO -            aRelativePath, aAccess);
13:47:13     INFO -            ^
13:47:13     INFO -  z:/task_1495457102/build/src/obj-firefox/dist/include/nsTSubstring.h(1107,3):  note: declared protected here
13:47:13     INFO -    nsTSubstring_CharT(const self_type& aStr)
13:47:13     INFO -    ^
13:47:13     INFO -  z:/task_1495457102/build/src/obj-firefox/dist/include/string-template-def-unichar.h(15,45):  note: expanded from macro 'nsTSubstring_CharT'
13:47:13     INFO -  #define nsTSubstring_CharT                  nsAString
13:47:13     INFO -                                              ^
13:47:13     INFO -  z:/task_1495457102/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp(208,11):  error: cannot pass object of non-trivial type 'const nsAString' through variadic function; call will abort at runtime [-Wnon-pod-varargs]
13:47:13     INFO -            aRelativePath, aAccess);
13:47:13     INFO -            ^
13:47:13     INFO -  z:/task_1495457102/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp(221,28):  error: cannot pass object of non-trivial type 'nsAutoString' through variadic function; call will abort at runtime [-Wnon-pod-varargs]
13:47:13     INFO -            result, aAccess, rulePath);
13:47:13     INFO -                             ^
13:47:13     INFO -  3 errors generated.
13:47:13     INFO -  z:/task_1495457102/build/src/config/rules.mk:1060: recipe for target 'sandboxBroker.obj' failed
13:47:13     INFO -  mozmake.EXE[5]: *** [sandboxBroker.obj] Error 1
13:47:13     INFO -  mozmake.EXE[5]: Leaving directory 'z:/task_1495457102/build/src/obj-firefox/security/sandbox/win/src/sandboxbroker'
13:47:13     INFO -  z:/task_1495457102/build/src/config/recurse.mk:73: recipe for target 'security/sandbox/win/src/sandboxbroker/target' failed
13:47:13     INFO -  mozmake.EXE[4]: *** [security/sandbox/win/src/sandboxbroker/target] Error 2
Flags: needinfo?(bobowencode)
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3fb60fbc326
Part 1: Implement Windows Level 3 content process sandbox policy. r=jimm
https://hg.mozilla.org/integration/mozilla-inbound/rev/445875fbf13b
Part 2: Fix registerFakePath in head_migration.js to work when the key has already been accessed. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/431267ab28de
Part 3: Move NPAPI windows process sandbox file rules into SandboxBroker. r=jimm
Pushed by philringnalda@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76376445ed04
Part 1: Implement Windows Level 3 content process sandbox policy. r=jimm
https://hg.mozilla.org/integration/mozilla-inbound/rev/4dbcfc90a5c2
Part 2: Fix registerFakePath in head_migration.js to work when the key has already been accessed. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/31e7b206282d
Part 3: Move NPAPI windows process sandbox file rules into SandboxBroker. r=jimm
Not your leak.
Flags: needinfo?(bobowencode)
You need to log in before you can comment on or make changes to this bug.