Closed Bug 912807 Opened 8 years ago Closed 8 years ago

Use --enable-content-sandbox-reporter on b2g "eng" builds

Categories

(Core :: Security, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
1.3 Sprint 2 - 10/11
Tracking Status
firefox26 --- affected
firefox27 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: jld, Assigned: jld)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently there are two reasons to want --enable-content-sandbox-reporter on b2g:
1) bug 907006, where processes that are supposed to be killed by seccomp instead get stuck in zombie state (possibly due to a bug in our backport of the kernel support from 3.5 to 3.0.x), and
2) the simple fact that we haven't yet caught all of our sandbox program's false positives, so people trying to develop or dogfood on seccomp-enabled devices like keon will encounter issues and should be given some clue as to what broke.

I gather that we'd prefer to not have the sandbox reporter enabled when we ship, in keeping with the principle of minimizing attack surface (killing the process immediately vs. letting it continue running in a presumably compromised state), and under the assumption that all false positives will have been discovered by then — but, for now, I feel that this security/usability tradeoff should favor usability.
Attachment #799898 - Flags: review?(gdestuynder)
We should do this only for ENG builds, not for all builds.
Comment on attachment 799898 [details] [diff] [review]
Enable sandbox reporter by default on b2g.

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

As :briansmith says, I think this would make sense to have on ENG builds only instead of any build. Can you modify the patch for that?
(In reply to Guillaume Destuynder [:kang] from comment #3)
> As :briansmith says, I think this would make sense to have on ENG builds
> only instead of any build. Can you modify the patch for that?

Does that belong in gecko/configure.in rather than gonk-misc/default-gecko-config, at that point?  (Or maybe elsewhere in the mozconfig machinery.)

I don't particularly like the idea of using SECCOMP_RET_KILL at all as long as we're running on kernels where it's known to be broken, but on further thought that might want to be a separate bug.
Attachment #799898 - Attachment is obsolete: true
Attachment #799898 - Flags: review?(gdestuynder)
Summary: Use --enable-content-sandbox-reporter on b2g (until outstanding issues are resolved) → Use --enable-content-sandbox-reporter on b2g "eng" builds
Attachment #800502 - Flags: review?(brian)
Attachment #800502 - Flags: review?(gdestuynder)
Attachment #800502 - Flags: review?(gdestuynder) → review+
Comment on attachment 800502 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gonk-misc/pull/114

Talked to Brian at the MozSummit; he says r=kang is enough.
Attachment #800502 - Flags: review?(brian)
Jed, you marked "affected" for "1.2", does it mean you'd like it uplifted to the 1.2 branch ? If yes I think we should ask koi+ on this (or we can say it's not part of the build because it's only done for eng builds ?)
I'd like for this to be uplifted, if that's allowed.  I doubt we'd consider it koi+, if I understand correctly, because it wouldn't block the 1.2 release.

It is definitely Not Part Of The User Build (or any other build whose variant isn't "eng"), so if that's enough I can make another pull request.
Yep, I agree with this.

If that's not done before I'll land and uplift monday.
Flags: needinfo?(felash)
https://github.com/mozilla-b2g/gonk-misc/commit/5283859444a13b05cb3ce584236022c56a6d726c
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.3 Sprint 2 - 10/11
a=npotb
gonk-misc/v1.2: 610051dd78e520363d3a3cd88df6ce86a6e59274
Flags: needinfo?(felash)
You need to log in before you can comment on or make changes to this bug.