New issue
Advanced search Search tips

Issue 867547 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 867549

Blocking:
issue 844128



Sign in to add a comment

Extensions Click-to-Script: Allow granting of permissions that weren't explicitly requested

Project Member Reported by rdevlin....@chromium.org, Jul 25

Issue description

The user should be able to grant permissions beyond what was specifically requested by the extension in the manifest.  This is primarily important for the chrome://extensions page, where we want the user to be able to allow the extension on "google.com", rather than "google.com/some-specific-path-the-extension-specified".

However, for security purposes, we should only grant the extension process itself the permissions it requests.  This way, if a user allows an extension on *.google.com and the extension only requests maps.google.com, we don't need to open up *.google.com to that process.  This will be an implementation detail hidden from the user (who will only see they have granted access to *.google.com).
 
Blockedon: 867549
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b492eb36611eb162e5ec0b9df72e9d53958ae29

commit 6b492eb36611eb162e5ec0b9df72e9d53958ae29
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Wed Aug 01 02:37:09 2018

[Extensions Click-to-Script] Update permissions sectioning

Update the logic used to partition permissions between granted and
withheld with click-to-script. This patch addresses a few issues:
- Previously, permissions granted with runtime host permissions were
  added to the active permissions entry in the ExtensionPrefs. This
  meant there was a chance that extensions would continue to be
  affected after the feature was disabled.
- Previously, we would try to grant an entire origin to an extension,
  even if only a certain path was requested. This could result in a
  crash, since we later CHECK'd that all granted permissions were
  strictly contained within the withheld permissions of an extension.
- The user should not need to specify an exact URLPattern at the
  chrome://extensions page in order to grant permission to an extension.
  That is, the user should be able to allow the extension on
  "google.com", rather than
  "google.com/some-specific-path-the-extension-specified".
  (This is dependent on the changes here, but will be fully resolved in
  a followup.)

To fix this, we change runtime granted host permissions to only affect
the runtime granted permissions preference (and not change active
permissions in the preferences). When calculating permissions, we
determine the set of permissions requested by the extension and
intersect it with the permissions in the runtime granted permissions.

This patch also allows extensions to be granted more permissions than
they explicitly request. This means that if an extension requests only
https://google.com/maps, it can be granted https://google.com/*. This is
necessary for runtime host permissions, where permissions granted may be
broader than explicitly requested. For security purposes, even though
the granted permissions may exceed the requested permissions, the
permissions active on the extension object (and in the extension
process) will always be strictly bound by the requested permissions. To
achieve this, we leverage the new intersection logic in URLPatterns
introduced in  https://crbug.com/867549 .

These two changes are interdependent (and cannot be submitted as two
CLs) because by no longer adding runtime-granted permissions to the
active permissions set, we introduce a need to find the semantic
intersection of the runtime-granted permissions and the requested
permissions. Otherwise, because of the way we partitioned permissions,
we would not grant any permissions if an extension requested *://*/*
and the user granted https://google.com/*.

Bug: 867547

Change-Id: I161cc699c95989f86be5b1743aee422ac5b41221
Reviewed-on: https://chromium-review.googlesource.com/1153986
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579678}
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/chrome/browser/extensions/permissions_updater.cc
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/chrome/browser/extensions/permissions_updater.h
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/chrome/browser/extensions/permissions_updater_unittest.cc
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/chrome/browser/extensions/scripting_permissions_modifier.cc
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/chrome/browser/extensions/scripting_permissions_modifier.h
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/chrome/browser/extensions/scripting_permissions_modifier_unittest.cc
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/extensions/common/permissions/permission_set.cc
[modify] https://crrev.com/6b492eb36611eb162e5ec0b9df72e9d53958ae29/extensions/common/permissions/permission_set.h

Sign in to add a comment