Add nativeMessaging extension permission

Add nativeMessaging extension permission

Now extensions need to have nativeMessaging permission to be able to use 
native messaging API.

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

[Matt Perry, 2013-02-28 20:28:45]

You should also add a call to 
CheckCurrentContextAccessToExtensionAPI(name))in OpenChannelToNativeApp. Having
a check in the renderer is good so we can give the developer an appropriate
error message. (After doing this, you also need to fix
runtime_generated_bindings.js to handle OpenChannelToNativeApp failures.)

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
File chrome/browser/extensions/api/messaging/message_service.cc (right):

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
chrome/browser/extensions/api/messaging/message_service.cc:201: if
(extension_service && extension_service->is_ready()) {
You don't need to check for is_ready.

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
File chrome/browser/extensions/api/messaging/native_messaging_apitest.cc
(right):

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
chrome/browser/extensions/api/messaging/native_messaging_apitest.cc:56:
switches::kEnableExperimentalExtensionApis);
What is this for? APIs that need the experimental flag have been deprecated.

[Sergey Ulanov, 2013-03-01 03:15:30]

Added a check in the renderer. Not sure what needs to be fixed in the JS layer.
CheckCurrentContextAccessToExtensionAPI() throws a JS exception, so I think the
calling extension should be able to catch this extension.

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
File chrome/browser/extensions/api/messaging/message_service.cc (right):

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
chrome/browser/extensions/api/messaging/message_service.cc:201: if
(extension_service && extension_service->is_ready()) {
On 2013/02/28 20:28:45, Matt Perry wrote:
> You don't need to check for is_ready.

Done.

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
File chrome/browser/extensions/api/messaging/native_messaging_apitest.cc
(right):

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
chrome/browser/extensions/api/messaging/native_messaging_apitest.cc:56:
switches::kEnableExperimentalExtensionApis);
On 2013/02/28 20:28:45, Matt Perry wrote:
> What is this for? APIs that need the experimental flag have been deprecated.

It wasn't working without this flag. It refuses to load the extension, I think
because the feature is marked as dev in _permission_features.json . What's the
right way to fix it?

[Matt Perry, 2013-03-01 20:31:13]

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
File chrome/browser/extensions/api/messaging/native_messaging_apitest.cc
(right):

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
chrome/browser/extensions/api/messaging/native_messaging_apitest.cc:56:
switches::kEnableExperimentalExtensionApis);
On 2013/03/01 03:15:30, sergeyu wrote:
> On 2013/02/28 20:28:45, Matt Perry wrote:
> > What is this for? APIs that need the experimental flag have been deprecated.
> 
> It wasn't working without this flag. It refuses to load the extension, I think
> because the feature is marked as dev in _permission_features.json . What's the
> right way to fix it?

What error message does it give you? Features marked as "dev" should be loadable
in "dev" channel or newer, regardless of the experimental flag.

[Sergey Ulanov, 2013-03-01 20:55:19]

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
File chrome/browser/extensions/api/messaging/native_messaging_apitest.cc
(right):

https://codereview.chromium.org/12300041/diff/2001/chrome/browser/extensions/...
chrome/browser/extensions/api/messaging/native_messaging_apitest.cc:56:
switches::kEnableExperimentalExtensionApis);
On 2013/03/01 20:31:13, Matt Perry wrote:
> On 2013/03/01 03:15:30, sergeyu wrote:
> > On 2013/02/28 20:28:45, Matt Perry wrote:
> > > What is this for? APIs that need the experimental flag have been
deprecated.
> > 
> > It wasn't working without this flag. It refuses to load the extension, I
think
> > because the feature is marked as dev in _permission_features.json . What's
the
> > right way to fix it?
> 
> What error message does it give you? Features marked as "dev" should be
loadable
> in "dev" channel or newer, regardless of the experimental flag.

Tried without this flag and it works fine now. I tried it first time more than a
week ago, so maybe something changed during this time. Removed it now.

[Matt Perry, 2013-03-01 21:23:42]

Great, lgtm

[commit-bot: I haz the power, 2013-03-01 22:24:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/12300041/8002

[Sergey Ulanov, 2013-03-02 00:26:40]

There was an issue that was caught by PermissionMessage unittests: the message
for the new permission wasn't specified. Now added a new message for it. PTAL.

[Matt Perry, 2013-03-02 00:33:54]

https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
File chrome/common/extensions/permissions/api_permission.cc (right):

https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
chrome/common/extensions/permissions/api_permission.cc:223:
PermissionMessage::kNativeMessaging },
Let's copy the entry for APIPermission::kPlugin since it gives effectively the
same level of access. Use the same warning message as well.

[Sergey Ulanov, 2013-03-02 01:14:27]

+jschuh
Justin, can you please advice on what would be the permission warning message
for the native messaging API?

https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
File chrome/common/extensions/permissions/api_permission.cc (right):

https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
chrome/common/extensions/permissions/api_permission.cc:223:
PermissionMessage::kNativeMessaging },
On 2013/03/02 00:33:54, Matt Perry wrote:
> Let's copy the entry for APIPermission::kPlugin since it gives effectively the
> same level of access. Use the same warning message as well.

Not sure it's the same level of access: Native messaging is allowed only when
the native host is already installed on the machine and it allows connections
from that particular extension.

[jschuh, 2013-03-04 22:26:06]

On 2013/03/02 01:14:27, sergeyu wrote:
> +jschuh
> Justin, can you please advice on what would be the permission warning message
> for the native messaging API?
> 
>
https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
> File chrome/common/extensions/permissions/api_permission.cc (right):
> 
>
https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
> chrome/common/extensions/permissions/api_permission.cc:223:
> PermissionMessage::kNativeMessaging },
> On 2013/03/02 00:33:54, Matt Perry wrote:
> > Let's copy the entry for APIPermission::kPlugin since it gives effectively
the
> > same level of access. Use the same warning message as well.
> 
> Not sure it's the same level of access: Native messaging is allowed only when
> the native host is already installed on the machine and it allows connections
> from that particular extension.

The exe is already installed and specifies what it will talk to based on the
manifest. So, I'd consider this a much lesser permission than NPAPI.

[Matt Perry, 2013-03-04 22:51:52]

On 2013/03/04 22:26:06, Justin Schuh wrote:
> On 2013/03/02 01:14:27, sergeyu wrote:
> > +jschuh
> > Justin, can you please advice on what would be the permission warning
message
> > for the native messaging API?
> > 
> >
>
https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
> > File chrome/common/extensions/permissions/api_permission.cc (right):
> > 
> >
>
https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
> > chrome/common/extensions/permissions/api_permission.cc:223:
> > PermissionMessage::kNativeMessaging },
> > On 2013/03/02 00:33:54, Matt Perry wrote:
> > > Let's copy the entry for APIPermission::kPlugin since it gives effectively
> the
> > > same level of access. Use the same warning message as well.
> > 
> > Not sure it's the same level of access: Native messaging is allowed only
when
> > the native host is already installed on the machine and it allows
connections
> > from that particular extension.
> 
> The exe is already installed and specifies what it will talk to based on the
> manifest. So, I'd consider this a much lesser permission than NPAPI.

The problem is that we don't know what the native program will do, or what the
messaging interface gives the extension access to. From the point of view of the
extension system, the extension still potentially can "Access all data on your
computer and the websites you visit". I don't think users will understand a
warning like "This extension can talk to other programs".

[jschuh, 2013-03-04 23:33:50]

On 2013/03/04 22:51:52, Matt Perry wrote:
> On 2013/03/04 22:26:06, Justin Schuh wrote:
> > On 2013/03/02 01:14:27, sergeyu wrote:
> > > +jschuh
> > > Justin, can you please advice on what would be the permission warning
> message
> > > for the native messaging API?
> > > 
> > >
> >
>
https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
> > > File chrome/common/extensions/permissions/api_permission.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/12300041/diff/5003/chrome/common/extensions/p...
> > > chrome/common/extensions/permissions/api_permission.cc:223:
> > > PermissionMessage::kNativeMessaging },
> > > On 2013/03/02 00:33:54, Matt Perry wrote:
> > > > Let's copy the entry for APIPermission::kPlugin since it gives
effectively
> > the
> > > > same level of access. Use the same warning message as well.
> > > 
> > > Not sure it's the same level of access: Native messaging is allowed only
> when
> > > the native host is already installed on the machine and it allows
> connections
> > > from that particular extension.
> > 
> > The exe is already installed and specifies what it will talk to based on the
> > manifest. So, I'd consider this a much lesser permission than NPAPI.
> 
> The problem is that we don't know what the native program will do, or what the
> messaging interface gives the extension access to. From the point of view of
the
> extension system, the extension still potentially can "Access all data on your
> computer and the websites you visit". I don't think users will understand a
> warning like "This extension can talk to other programs".

I expect we can improve the string, but we don't want to misrepresent the
threat. The user makes their primary trust decision at the time they install the
native program. So, anything we say at extension installation is more
informative than it is a warning, because if the native program was malicious
then the user has already lost.

Let me think about this for a bit, and see if there's a better way to explain
this to the user.

[Sergey Ulanov, 2013-03-05 22:07:55]

On 2013/03/04 23:33:50, Justin Schuh wrote:
> I expect we can improve the string, but we don't want to misrepresent the
> threat. The user makes their primary trust decision at the time they install
the
> native program. So, anything we say at extension installation is more
> informative than it is a warning, because if the native program was malicious
> then the user has already lost.
> 
> Let me think about this for a bit, and see if there's a better way to explain
> this to the user.

Maybe I can land this CL as is and then we can fix the message later?

[Matt Perry, 2013-03-05 22:10:06]

On 2013/03/05 22:07:55, sergeyu wrote:
> On 2013/03/04 23:33:50, Justin Schuh wrote:
> > I expect we can improve the string, but we don't want to misrepresent the
> > threat. The user makes their primary trust decision at the time they install
> the
> > native program. So, anything we say at extension installation is more
> > informative than it is a warning, because if the native program was
malicious
> > then the user has already lost.
> > 
> > Let me think about this for a bit, and see if there's a better way to
explain
> > this to the user.
> 
> Maybe I can land this CL as is and then we can fix the message later?

I'm still not convinced we should have a new message at all.

[Matt Perry, 2013-03-05 22:24:05]

OK, just talked it over with Justin, and he convinced me that we don't need a
message at all. The fact that the native app (a) was installed by the user
already, and (b) whitelists each extension that can talk to it, means the user
already made the trust decision.

Keep the native_messaging permission, but get rid of the warning message.

[Sergey Ulanov, 2013-03-06 00:47:34]

On 2013/03/05 22:24:05, Matt Perry wrote:
> OK, just talked it over with Justin, and he convinced me that we don't need a
> message at all. The fact that the native app (a) was installed by the user
> already, and (b) whitelists each extension that can talk to it, means the user
> already made the trust decision.
> 
> Keep the native_messaging permission, but get rid of the warning message.

Done.

[commit-bot: I haz the power, 2013-03-06 00:50:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/12300041/22001

[commit-bot: I haz the power, 2013-03-06 03:41:37]

Change committed as 186340