OEM folder name support for apps installed from rootfs

+ add was_installed_by_oem support to external extensions json.
+ set OEM apps folder name from default_app_order.json

BUG=357408
TEST=unit_tests
R= asargent@chromium.org, stevenjb@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262847

[Dmitry Polukhin, 2014-03-28 18:41:59]

PTAL

[Dmitry Polukhin, 2014-03-28 22:14:44]

+ stevenjb

[stevenjb, 2014-03-28 23:07:04]

lgtm

[asargent_no_longer_on_chrome, 2014-03-31 19:28:12]

Should this be restricted to only ChromeOS, or do we intentionally want to
support the concept of marking exernal extensions as OEM installs on other
platforms?

Also another question: this only applies to where the app gets shown in the app
launcher or similar UI and doesn't impact more security sensitive things like
permissions, right?

Apart from these questions, this lgtm.

[Dmitry Polukhin, 2014-04-09 00:09:10]

Sorry for delay, I was on vacation.

On 2014/03/31 19:28:12, Antony Sargent wrote:
> Should this be restricted to only ChromeOS, or do we intentionally want to
> support the concept of marking exernal extensions as OEM installs on other
> platforms?

I can make it Chrome OS only but it looks like making this code general won't
add any harm for other platforms and in general I prefer to avoid adding
#ifdef's of there is no real need. So please let me know if you think I should
restrict it to Chrome OS only.

> Also another question: this only applies to where the app gets shown in the
app
> launcher or similar UI and doesn't impact more security sensitive things like
> permissions, right?

This flag won't affect permissions. It will result in putting the app into OEM
folder and it won't prevent user from removing the app or disabling it so it
seems safe.

> Apart from these questions, this lgtm.

[asargent_no_longer_on_chrome, 2014-04-09 16:56:23]

On 2014/04/09 00:09:10, Dmitry Polukhin wrote:
> Sorry for delay, I was on vacation.
> 
> On 2014/03/31 19:28:12, Antony Sargent wrote:
> > Should this be restricted to only ChromeOS, or do we intentionally want to
> > support the concept of marking exernal extensions as OEM installs on other
> > platforms?
> 
> I can make it Chrome OS only but it looks like making this code general won't
> add any harm for other platforms and in general I prefer to avoid adding
> #ifdef's of there is no real need. So please let me know if you think I should
> restrict it to Chrome OS only.
> 
> > Also another question: this only applies to where the app gets shown in the
> app
> > launcher or similar UI and doesn't impact more security sensitive things
like
> > permissions, right?
> 
> This flag won't affect permissions. It will result in putting the app into OEM
> folder and it won't prevent user from removing the app or disabling it so it
> seems safe.
> 
> > Apart from these questions, this lgtm.

My only concern would be if malicious 3rd party software (and not OEMs) on
windows would abuse this support to try and make their force-installed items
(which users often don't want and didn't realize were installed) somehow look
more legitimate. It's hard for me to judge this without knowing a little more
about the UX that users get with this - are there any mocks or design docs
beyond just the crbug entry?

[Dmitry Polukhin, 2014-04-09 17:39:47]

On 2014/04/09 16:56:23, Antony Sargent wrote:
> My only concern would be if malicious 3rd party software (and not OEMs) on
> windows would abuse this support to try and make their force-installed items
> (which users often don't want and didn't realize were installed) somehow look
> more legitimate. It's hard for me to judge this without knowing a little more
> about the UX that users get with this - are there any mocks or design docs
> beyond just the crbug entry?

Issue with screenshots how it looks on Chrome OS
https://code.google.com/p/chromium/issues/detail?id=356378
Steven should know better how this UI could look on Windows if any at all. On
Windows there is no way to change folder name it will be "OEM folder". But I
think using this flag won't add any benefit because it will only add additional
click to launch the app. But perhaps adding #ifdef would be the simplest
solution.

[stevenjb, 2014-04-09 18:17:38]

https://codereview.chromium.org/216763004/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://codereview.chromium.org/216763004/diff/30001/chrome/browser/extension...
chrome/browser/extensions/external_provider_impl.cc:225: if
(extension->GetBoolean(kWasInstalledByOem, &was_installed_by_oem) &&
We only set this property in the Chrome code on Chrome OS. Is it possible for an
external process on non-Chrome OS platforms to set an arbitrary extension
preference? If so, then we should probably make this code OS_CHROMEOS only since
we currently don't have OEM folders on other platforms, and it would be weird if
apps showed up in an "OEM" folder that could not be removed from the folder
(although we do always allow un-installing them).

[asargent_no_longer_on_chrome, 2014-04-09 19:27:59]

On 2014/04/09 18:17:38, stevenjb wrote:
>
https://codereview.chromium.org/216763004/diff/30001/chrome/browser/extension...
> File chrome/browser/extensions/external_provider_impl.cc (right):
> 
>
https://codereview.chromium.org/216763004/diff/30001/chrome/browser/extension...
> chrome/browser/extensions/external_provider_impl.cc:225: if
> (extension->GetBoolean(kWasInstalledByOem, &was_installed_by_oem) &&
> We only set this property in the Chrome code on Chrome OS. Is it possible for
an
> external process on non-Chrome OS platforms to set an arbitrary extension
> preference? If so, then we should probably make this code OS_CHROMEOS only
since
> we currently don't have OEM folders on other platforms, and it would be weird
if
> apps showed up in an "OEM" folder that could not be removed from the folder
> (although we do always allow un-installing them).

I think that bit of code actually can runs on the .json content in external
extensions config files
(https://developer.chrome.com/extensions/external_extensions) as well as any
preferences set from within C++ code that aren't driven by such config files.
Note that |extension| there is not an Extension object, but rather a
base::DictionaryValue. 

But to answer your question, unfortunately it has become very common for
malicious developers not just to use external extensions in the way they were
intended, but to also modify chrome Preferences files directly to prevent users
from getting a chance to opt-in to the install. We're hoping some of the things
we're doing to cut down on instances of force installed extensions (such as the
impending "only from webstore" lockdown for windows, and the search
provider/homepage settings API) will remove some of the incentive here. 

Anyhow, from the screenshots, I guess the only thing I could come up with as a
security risk would be if malicious developers putting their items in there made
users less likely to notice they had been installed. Most abuse we've seen has
been with extensions, not apps, so I think this probably isn't a problem and you
can proceed as-is.

[Dmitry Polukhin, 2014-04-09 19:46:11]

On 2014/04/09 19:27:59, Antony Sargent wrote:
> On 2014/04/09 18:17:38, stevenjb wrote:
> >
>
https://codereview.chromium.org/216763004/diff/30001/chrome/browser/extension...
> > File chrome/browser/extensions/external_provider_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/216763004/diff/30001/chrome/browser/extension...
> > chrome/browser/extensions/external_provider_impl.cc:225: if
> > (extension->GetBoolean(kWasInstalledByOem, &was_installed_by_oem) &&
> > We only set this property in the Chrome code on Chrome OS. Is it possible
for
> an
> > external process on non-Chrome OS platforms to set an arbitrary extension
> > preference? If so, then we should probably make this code OS_CHROMEOS only
> since
> > we currently don't have OEM folders on other platforms, and it would be
weird
> if
> > apps showed up in an "OEM" folder that could not be removed from the folder
> > (although we do always allow un-installing them).
> 
> I think that bit of code actually can runs on the .json content in external
> extensions config files
> (https://developer.chrome.com/extensions/external_extensions) as well as any
> preferences set from within C++ code that aren't driven by such config files.
> Note that |extension| there is not an Extension object, but rather a
> base::DictionaryValue. 
> 
> But to answer your question, unfortunately it has become very common for
> malicious developers not just to use external extensions in the way they were
> intended, but to also modify chrome Preferences files directly to prevent
users
> from getting a chance to opt-in to the install. We're hoping some of the
things
> we're doing to cut down on instances of force installed extensions (such as
the
> impending "only from webstore" lockdown for windows, and the search
> provider/homepage settings API) will remove some of the incentive here. 
> 
> Anyhow, from the screenshots, I guess the only thing I could come up with as a
> security risk would be if malicious developers putting their items in there
made
> users less likely to notice they had been installed. Most abuse we've seen has
> been with extensions, not apps, so I think this probably isn't a problem and
you
> can proceed as-is.

Thank you for review, I'll proceed with the code as is.

[Dmitry Polukhin, 2014-04-09 19:46:18]

The CQ bit was checked by dpolukhin@chromium.org

[commit-bot: I haz the power, 2014-04-09 19:46:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpolukhin@chromium.org/216763004/30001

[commit-bot: I haz the power, 2014-04-09 22:50:07]

Change committed as 262847