922756 - Add Process Sandboxing w/ Chromium Sandbox

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Brian R. Bondy [:bbondy]]

The goal of this bug is to Add Process Sandboxing w/ Chromium Sandbox in tree and to create the needed build-config and min-dependencies.

For OSX we don't need anything since there is built-in support (App Sandbox Design Guide).
For Linux we're already using seccomp in-tree via m-c/security/sandbox.

In Windows we'd like to have support for Security Sandboxing which allows: Job object control, integrity level control, using alternate window station, using alternate desktops, having policy control, support for restricted tokens, support forwarding of API calls to a more privileged process.  Basically we want to severely limit the APIs that can be used, and the locations that can be accessed and written to.

Chromium source code already contains re-usable code that simplifies all of the above.  We should use it to save time, money, and development resources.  And by doing so, we'll end up with a better, less buggy solution.

My goal for bringing in the Chromium Sandbox code is:
- Keep changes from Chromium at a minimum to make it easier to update as chromium sandbox source gets updated.
- Make any such changes compartmentalized when possible
- When possible eliminate dependencies, possibly at the cost of changing core chromium sandbo code slightly
- Do not conflict with previous ipc/chromium/src/base code

Initial plan for doing this is:
- The new location for this code would be in a subdirectory of /security/sandbox
- The new code being pulled in will be windows specific and pulled from a subset of <chrom-src>/chrome/src/sandbox and <chrom-src>/base. 
- To avoid linking errors with existing outdated forked code in ipc/chromium, I plan to compile the needed code into its own DLL on Windows.
- I already do have chrome-trunk code linking successfully within xul.dll, but I prefer to keep things separated (for maintainability and avoiding potential bugs caused by using some old stuff in ipc/chromium) even if it means a little bit of overlap.

Comment 1

[Brian R. Bondy [:bbondy]]

Slightly modified plan for doing this is:

Looks like using a DLL for the sandboxing is not possible because some ntddl import function patching and writing of process memory directly which happens from the broker process to the target process. 

I had it working originally by linking a static library directly to the exe. I'm going to go back to that method.  This doesn't conflict with the ipc/chromium code because that's inside xul.dll. 

Any process which wishes to be sandboxed would need to link to this library and browser/app will directly link to it as well.

firefox.exe itself would be able to be sandboxed, or an external process.  The library to link to is the same for both target and brokered processes.

Comment 2

[Masatoshi Kimura [:emk]]

> Any process which wishes to be sandboxed would need to link to this library and browser/app will directly link to it as well.

Will IMEs (implemented as in-process DLL) work in sandboxed processes? We, IME users, were suffered from conflicting with Flash Player's sandbox in the past.

Comment 3

[Brian R. Bondy [:bbondy]]

(In reply to Masatoshi Kimura [:emk] from comment #2)
> Will IMEs (implemented as in-process DLL) work in sandboxed processes? We,
> IME users, were suffered from conflicting with Flash Player's sandbox in the
> past.

A lot of things (most unless explicitly allowed) won't work in a fully sandboxed process.  That being said, it is configurable in various different ways, and it's possible to have things live inside, and outside of the sandbox.  The broker process can execute things for the target (sandboxed) process when allowed.

Some things can also be setup before the process goes into lockdown mode, and that allows it to continue to work after the process is in lockdown mode.

Sounds like something to look out for in a future bug that gets posted though, thanks for the heads up.

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch 1 - Build config - rev WIP

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Patch 2 - Unchanged minimum needed Chromium Sandbox code - rev WIP

Comment 6

[Brian R. Bondy [:bbondy]]

Attachment: Patch 3 - Changes to Chromium Sandbox to make buildable and to reduce dependencies - rev WIP

Comment 7

[Brian R. Bondy [:bbondy]]

The work is basically done but I'm going to spend some time cleaning up part 3 and investigating sharing some of the ipc/chromium base code.

There's about ~300 files being added a the moment, reduced from initially 1500.

Comment 8

[Brian R. Bondy [:bbondy]]

I have a local poc app that sandboxes itself too that works, but there's no point in adding that since similar code will be added into plugin_container for e10s soon in another bug.

Comment 9

[Brian R. Bondy [:bbondy]]

I have patches posted for getting the Chromium sandbox in patches in this bug, but it uses its own separate copy of the chromium base library.

We have an old copy of the chromium base library from 2008 in ipc/chromium, so we may want to try to combine them.  Otherwise we'll have 2 copies of ~150 files that are similar but different.  It wouldn't be a problem linking wise because the ipc code goes into xul.dll and the sandbox code gets linked directly to the exe.  But it seems not ideal to have duplicate files in tree.

I spent the last couple days updating the ipc/chromium/src/base code to the newer base library code. But that lead to several hundred custom changes to code in tree.  This is pretty risky.

Another route that I'm looking into is changing the current chromium sandbox code, to use the old base library. This would make the sandbox code itself hard to update, but would be the safest thing regression-wise for our existing ipc code.

I checked with bsmedberg via email to get his preference.

Any of the 3 options will work, but I'm spending some time investigating which one is the best route moving forward.  Getting it right now will save time later on.

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Brian R. Bondy [:bbondy] from comment #9)
> We have an old copy of the chromium base library from 2008 in ipc/chromium,
> so we may want to try to combine them.  Otherwise we'll have 2 copies of
> ~150 files that are similar but different.

> I spent the last couple days updating the ipc/chromium/src/base code to the
> newer base library code. But that lead to several hundred custom changes to
> code in tree.  This is pretty risky.

I suggest we just live with the duplication for now, and file a follow-up bug (unassigned, unprioritized) to de-dedup by removing the old code.

> Another route that I'm looking into is changing the current chromium sandbox
> code, to use the old base library. This would make the sandbox code itself
> hard to update, but would be the safest thing regression-wise for our
> existing ipc code.

This is the worst option. Unless/until we have a pressing need to fork the Chromium sandboxing code, we should try to keep in intact with as few modifications as possible, and ideally with the eventual goal of upstreaming our modifications in some way.

Comment 11

[Brian R. Bondy [:bbondy]]

By the way there is a Google code project for sandboxing from 2010. I used the one from trunk Chromium source instead (2013) since it has a lot of new code including windows 7 and windows 8 stuff. I'm not a lawyer, but the licenses are pretty much identical and basically say that distributions with or without changes are allowed.

Comment 12

[Brian R. Bondy [:bbondy]]

> I suggest we just live with the duplication for now, and file a follow-up 
> bug (unassigned, unprioritized) to de-dedup by removing the old code.

Deferring the merge of chromium/base until later sounds good to me, I'm pretty sure with upgrading ipc/chromium/src/base with hundreds of non-trivial changes in surrounding code, it'd have at least some regressions which may not be obvious.  I'll put a zip of the consolidation WIP inside this bug in case we want to revisit it.

bsmedberg linked me here: :)
http://quotes.burntelectrons.org/1077 

Thanks for the advice bsmith, I knew that was safest but I needed some re-assurance since it's so damn ugly :)

Comment 13

[Brian R. Bondy [:bbondy]]

Posted bug 925471 to potentially do dedupe work in the future of the 2 libraries without holding up the current sandboxing goals.

Comment 14

[Mike Hommey [:glandium]]

(In reply to Brian R. Bondy [:bbondy] from comment #9)
> It wouldn't be a problem linking wise because the ipc code goes into xul.dll and 
> the sandbox code gets linked directly to the exe.

That in itself is probably going to be a problem if you want to avoid ipc/chromium code duplication. Because xul.dll can't depend on symbols from the exe, and vice versa.

Comment 15

[Brian R. Bondy [:bbondy]]

I don't think it's a problem, the base library is statically linked.  The 2008 version is linked to xul.dll and the 2013 version is linked to the exe. The xul.dll one oes not export any of the symbols from the base library.

Comment 16

[Brian R. Bondy [:bbondy]]

Oh I see you're saying, if I want to avoid code duplication. I still don't think that would be a problem either for the same reason.

Comment 17

[Mike Hommey [:glandium]]

(In reply to Brian R. Bondy [:bbondy] from comment #16)
> Oh I see you're saying, if I want to avoid code duplication. I still don't
> think that would be a problem either for the same reason.

You'll have to duplicate the machine code, essentially, even if the source is not duplicated.

Comment 18

[Brian R. Bondy [:bbondy]]

Yep and that doesn't concern me. Source duplication does more.

Comment 19

[Brian R. Bondy [:bbondy]]

And if it was a concern because of file sizes or something like that (download installer size), and we used a common base library, then we could put it into a dll and export everything and then dynamically link that to both binaries.

Comment 20

[Mike Hommey [:glandium]]

Note such a dll already exists: mozglue.

Comment 21

[Brian R. Bondy [:bbondy]]

(In reply to Mike Hommey [:glandium] from comment #20)
> Note such a dll already exists: mozglue.

I think you mean we could use mozglue? 
Currently the files in question are compiled into a library chromium_s and statically linked into xul.dll.

Anyway let's continue discussions on this if needed in bug 925471. Thanks!

Comment 22

[Brian R. Bondy [:bbondy]]

Attachment: Patch 1 - Build config - rev 1

I was able to get things working with >>0<< changes to chromium sandbox and chromium base code. So it'll be fully updatable without having to maintain a separate patch queue.  It works by having a special shim include dir that does any magic needed to make sure the rest compiles.  It also does a force injected sdk declaration include to patch up any missing sdk declarations  since chromium has a higher dependency than we require.

Putting the initial review to you bsmith. After r+ is reached I'll get a super review from bsmedberg.

For this patch in particular, I'll also get a review from a build peer.

Comment 23

[Brian R. Bondy [:bbondy]]

Attachment: Patch 2 - Unchanged minimum needed Chromium Sandbox code - rev 1

Comment 24

[Brian R. Bondy [:bbondy]]

Attachment: Patch 3 - Shim to make Chromium Sandbox buildable - rev 1

Comment 25

[Brian R. Bondy [:bbondy]]

ping, any ETA for initial review bsmith? Thanks :)

Comment 26

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 815986 [details] [diff] [review]
Patch 3 - Shim to make Chromium Sandbox buildable - rev 1

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

Hi Brian - I think Brian is on vacation. I would suggest getting a build peer or two to look at these changes to get their take in the meanwhile. Also, I think :kang and :dougt would be good to get feedback from.

::: security/sandbox/base/shim/metrics/histogram.h
@@ +1,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +// This file is intentionally left blank ;)

I assume some imported chrome files are #including "histogram.h"? Maybe list them here or be more clear about why this file is present but empty.

Comment 27

[Brian R. Bondy [:bbondy]]

Attachment: Patch 1 - Build config - rev 2

Comment 28

[Brian R. Bondy [:bbondy]]

Attachment: Patch 3 - Shim to make Chromium Sandbox buildable - rev 2

Comment 29

[Benjamin Smedberg]

I don't want to be the reviewer for the rest of this. aklotz maybe?

Comment 30

[Brian R. Bondy [:bbondy]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #29)
> I don't want to be the reviewer for the rest of this. aklotz maybe?

No problem, thanks for the build-config review, and thanks for the discussions around implementation.

Comment 31

[(No longer employed by Mozilla) Aaron Klotz]

Sorry for the delay, I'll start reviewing this tomorrow morning.

Comment 32

[Brian R. Bondy [:bbondy]]

No problem at all, I've been on PTO Monday-Tuesday anyway. Thanks very much!

Comment 33

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: OVERRIDE_WIN32_WINNT hack

I'm kind of on the fence about sdkdecls.h. I understand the issues that you're dealing with (I had to deal with this problem when I was first evaluating the feasibility of doing the chromium import).

My concern is that now we are duplicating content from SDK headers (and in the future might need to do maintenance on said duplication). Is it possible to handle this in a more future-proof way?

When I was trying this stuff out, I hacked up a change to mozilla-config.h.in (since the build's _WIN32_WINNT is written to mozilla-config.h by configure) that allows one to selectively override _WIN32_WINNT in the build system for cases where we need the newer SDK definitions (and respecting older OS versions while doing so, of course). Basically you set OVERRIDE_WIN32_WINNT in your DEFINES to the version that you want for that directory.

I don't know if that is necessarily the Right Way to go about this, but I'd like to make sure that we've considered other avenues before going the route of duplicating code from SDK headers.

Thoughts?

Comment 34

[Brian R. Bondy [:bbondy]]

(In reply to Aaron Klotz [:aklotz] from comment #33)
> Created attachment 821293 [details] [diff] [review]
> OVERRIDE_WIN32_WINNT hack
> 
> I'm kind of on the fence about sdkdecls.h. I understand the issues that
> you're dealing with (I had to deal with this problem when I was first
> evaluating the feasibility of doing the chromium import).
> 
> My concern is that now we are duplicating content from SDK headers (and in
> the future might need to do maintenance on said duplication). Is it possible
> to handle this in a more future-proof way?
> 
> When I was trying this stuff out, I hacked up a change to
> mozilla-config.h.in (since the build's _WIN32_WINNT is written to
> mozilla-config.h by configure) that allows one to selectively override
> _WIN32_WINNT in the build system for cases where we need the newer SDK
> definitions (and respecting older OS versions while doing so, of course).
> Basically you set OVERRIDE_WIN32_WINNT in your DEFINES to the version that
> you want for that directory.
> 
> I don't know if that is necessarily the Right Way to go about this, but I'd
> like to make sure that we've considered other avenues before going the route
> of duplicating code from SDK headers.
> 
> Thoughts?

My original thinking was that doing so would make it easy to introduce things into our build that would make it buildable, but not runable on windows XP.  But we have tests running on XP so that's probably not too much of a concern. 

I do think the maintenance would be negligible, but I guess my bigger concern is whether we're allowed to just include our own declarations or not.  I'll play around with it a bit more to see if I can do a forced include to header files that does an undef.  I'm not sure if that forced include would show up before or after mozilla-config.h.in though so this may or may not work.  I did at one point have a similar hack in mozilla-config.h.in by the way.

Comment 35

[Brian R. Bondy [:bbondy]]

Another thing I was worried about is possibly including something only avail in XP SP3 with a blanket define.  Our talos machines test XP SP3, but our min dependency is XP SP2, so it wouldn't be caught there.

Comment 36

[Brian R. Bondy [:bbondy]]

Hi ehsan and bsmedberg,

Vladan/aklotz messaged me on #perf to ask about how to review part 2.
In particular I think the concern is that it would take a very long time to review all of the added code.

I'd like to know if you both are OK with them doing a review along the lines of making sure the code included is indeed chromium sandbox code, and that the important parts of the code is included. 

Otherwise they'd like for the patch to be split up and distributed amongst several people.  I don't think that's the best use of our time though and I think it should be enough to go on the pretense that it's what chromium is using. 

One of the goals here too was to not have changes from the chromium code, so that we can just update the code periodically. Which we accomplished in this bug by having 0 changes to the Chromium sandbox files.

Comment 37

[Brian R. Bondy [:bbondy]]

Private messaged bsmith too:

[ 20:39:00 ] [ bbondy ] if you have an opinion on bug 922756 btw I'd appreciate you weighing in too
[ 20:40:11 ] [ briansmith ] OK, I will do.
[ 20:40:28 ] [ bbondy ] thanks, thought of you just after I posted the comment :)
[ 20:40:29 ] [ briansmith ] I agree with you that it is unreasonable to review all of the chromium code and that we need to take some of it on faith.
[ 20:40:46 ] [ bbondy ] great

Comment 38

[Jim Mathies [:jimm]]

(In reply to Aaron Klotz [:aklotz] from comment #33)
> Created attachment 821293 [details] [diff] [review]
> OVERRIDE_WIN32_WINNT hack
> 
> I'm kind of on the fence about sdkdecls.h. I understand the issues that
> you're dealing with (I had to deal with this problem when I was first
> evaluating the feasibility of doing the chromium import).

This looks like the right way to go about this to me. We've run into this kind of problem in past many times since our target win versions are kept low. The only caveat is that generally we shouldn't copy paste code directly out of an sdk, we should roll our own clean version for checkin.

I we do elevate the win version defines in headers in a few places, but try to keep that isolated to single header/src file rather than an entire chunk of the tree.

Comment 39

[Brian R. Bondy [:bbondy]]

(In reply to Jim Mathies [:jimm] from comment #38)
> (In reply to Aaron Klotz [:aklotz] from comment #33)
> > Created attachment 821293 [details] [diff] [review]
> > OVERRIDE_WIN32_WINNT hack
> > 
> > I'm kind of on the fence about sdkdecls.h. I understand the issues that
> > you're dealing with (I had to deal with this problem when I was first
> > evaluating the feasibility of doing the chromium import).
> 
> This looks like the right way to go about this to me. We've run into this
> kind of problem in past many times since our target win versions are kept
> low. The only caveat is that generally we shouldn't copy paste code directly
> out of an sdk, we should roll our own clean version for checkin.
> 
> I we do elevate the win version defines in headers in a few places, but try
> to keep that isolated to single header/src file rather than an entire chunk
> of the tree.

ok so I'm also fine with this, it's already not a direct copy and paste and it's put in its own defines for only the definitions that are needed.

Comment 40

[(not currently active) Ted Mielczarek]

Historically we've rubber-stamped code imports by simply verifying that the code you're importing matches what you said you imported. (This can be helped by providing a script that does the import, which also helps for updating the code to a newer version later.) Reviewing code from upstream projects is not a good use of anyone's time.

Comment 41

[(no longer active)]

(In reply to Brian R. Bondy [:bbondy] from comment #36)
> Hi ehsan and bsmedberg,
> 
> Vladan/aklotz messaged me on #perf to ask about how to review part 2.
> In particular I think the concern is that it would take a very long time to
> review all of the added code.
> 
> I'd like to know if you both are OK with them doing a review along the lines
> of making sure the code included is indeed chromium sandbox code, and that
> the important parts of the code is included. 

Yep, code imported from upstream projects doesn't require review.

> Otherwise they'd like for the patch to be split up and distributed amongst
> several people.  I don't think that's the best use of our time though and I
> think it should be enough to go on the pretense that it's what chromium is
> using. 

No, don't waste your time doing that!  :-)

> One of the goals here too was to not have changes from the chromium code, so
> that we can just update the code periodically. Which we accomplished in this
> bug by having 0 changes to the Chromium sandbox files.

That may or may not be the ideal path forward.  In my experience, if we treat the code imported from upstream as read-only, sometimes it can be painful to work with it.  For some of the libraries that we have imported (such as the Chromium IPC library, or the Blink Web Audio processing code which I have imported myself) we just declare the code as Mozilla code.  That usually makes sense if you expect few fixes from the upstream project, or if you think that our paths will soon diverge.  For some others (such as hunspell and lots of media libraries that we use), we create update scripts which know how to retrieve code from upstream and apply our own patches on top of it.  That will give us freedom to modify the code and also take upstream changes.  It's best to have _a_ way of updating this code, but how it's going to work should be determined by you guys (since you'll be doing most of the work there.)  Just *please* don't make it a read-only library!

Comment 42

[Brian R. Bondy [:bbondy]]

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #41)
> That may or may not be the ideal path forward.  In my experience, if we
> treat the code imported from upstream as read-only, sometimes it can be
> painful to work with it.  For some of the libraries that we have imported
> (such as the Chromium IPC library, or the Blink Web Audio processing code
> which I have imported myself) we just declare the code as Mozilla code. 
> That usually makes sense if you expect few fixes from the upstream project,
> or if you think that our paths will soon diverge.  For some others (such as
> hunspell and lots of media libraries that we use), we create update scripts
> which know how to retrieve code from upstream and apply our own patches on
> top of it.  That will give us freedom to modify the code and also take
> upstream changes.  It's best to have _a_ way of updating this code, but how
> it's going to work should be determined by you guys (since you'll be doing
> most of the work there.)  Just *please* don't make it a read-only library!

Yep makes sense, just want to minimize unneeded changes in general.

Cancelling needinfo from bsmedberg given we've had a lot of consistent feedback about importing without reviewing the actual imported code.

Comment 43

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 819294 [details] [diff] [review]
Patch 3 - Shim to make Chromium Sandbox buildable - rev 2

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

Okay. r=me with fixed nits.

Can you please add vim and emacs mode lines to these new files?

::: security/sandbox/base/shim/base/logging.cpp
@@ +24,5 @@
> +{
> +}
> +
> +LogMessage::LogMessage(const char* file, int line, std::string* result)
> +    : severity_(LOG_FATAL), file_(file), line_(line) {

nit: this line is indented 4 spaces instead of 2. Also, can you split up the elements of the initialization list onto separate lines?

@@ +29,5 @@
> +}
> +
> +LogMessage::LogMessage(const char* file, int line, LogSeverity severity,
> +                       std::string* result)
> +    : severity_(severity), file_(file), line_(line) {

Same nits as above.

@@ +37,5 @@
> +{
> +}
> +
> +
> +LogMessage::SaveLastError::SaveLastError() : last_error_(::GetLastError()) {

Nit: initialization list formatting. Also, can you please put the opening curly braces for functions on their own line?

@@ +60,5 @@
> +                                           SystemErrorCode err,
> +                                           const char* module)
> +    : err_(err),
> +      module_(module),
> +      log_message_(file, line, severity) {

Nit: indentation

@@ +69,5 @@
> +                                           LogSeverity severity,
> +                                           SystemErrorCode err)
> +    : err_(err),
> +      module_(NULL),
> +      log_message_(file, line, severity) {

Nit: indentation

::: security/sandbox/base/shim/base/strings/string_piece.h
@@ +6,5 @@
> +#define _SECURITY_SANDBOX_SHIM_BASE_STRING_PIECE_H
> +#include "sandbox/base/strings/string_piece.h"
> +
> +namespace {
> +  bool IsStringASCII(const base::StringPiece& ascii)

nit: No indentation necessary inside a namespace block.

::: security/sandbox/base/shim/base/tracked_objects.h
@@ +5,5 @@
> +#ifndef _SECURITY_SANDBOX_TRACKED_OBJECTS_H_
> +#define _SECURITY_SANDBOX_TRACKED_OBJECTS_H_
> +namespace tracked_objects
> +{
> +  class ThreadData

nit: No indentation inside namespace block

::: security/sandbox/base/shim/metrics/histogram.h
@@ +4,5 @@
> +
> +// This file is intentionally left blank beccause histogram.h is included
> +// by the chromium sandbox code, but if the real copy was included, it would
> +// increase the amount of dependencies. In order to minimize changes to the
> +// sandbox code, we just create the file here.

Thanks for writing such a useful comment for this!

::: security/sandbox/base/shim/sdkdecls.h
@@ +1,4 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +

Guard against multiple includes, please!

Comment 44

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 815984 [details] [diff] [review]
Patch 2 - Unchanged minimum needed Chromium Sandbox code - rev 1

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

Importing from upstream, r+.

Comment 45

[Brian R. Bondy [:bbondy]]

Attachment: Patch 3 - Shim to make Chromium Sandbox buildable - rev 3

Implemented review nits, carrying forward r+.

Comment 46

[Brian R. Bondy [:bbondy]]

Attachment: Patch 1 - Build config - rev 3

Minor fix to fix broken b2g build for part of a makefile that should have been inside winnt check. Carrying forward r+.

Comment 47

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/f00c0b788fd7
http://hg.mozilla.org/integration/mozilla-inbound/rev/6eaa8994ea19
http://hg.mozilla.org/integration/mozilla-inbound/rev/3ca982c90f00

Comment 48

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f00c0b788fd7
https://hg.mozilla.org/mozilla-central/rev/6eaa8994ea19
https://hg.mozilla.org/mozilla-central/rev/3ca982c90f00

Comment 49

[u279076]

Brian, is there anything QA needs to be on the lookout for here in Firefox 28?

Comment 50

[Brian R. Bondy [:bbondy]]

Nope nothing to do. When I land a bug about enabling it in builds then we'll test at that point.

Comment 51

[u279076]

(In reply to Brian R. Bondy [:bbondy] from comment #50)
> Nope nothing to do. When I land a bug about enabling it in builds then we'll
> test at that point.

Okay, thanks. Please needinfo me when QA is needed.