CopyPoliciesIfUnset() should use the extension ID's SHA-1 |
||
Issue description`ExtensionPolicyMigrator::CopyPoliciesIfUnset()` [1] takes an extension ID as a parameter. Since those extensions can potentially be whitelisted extensions with access to private APIs, we should be careful about putting those extension IDs in the source code directly. We could change `CopyPoliciesIfUnset()` to take the extension ID's SHA-1 [2], instead of taking the extension ID directly. [1] https://cs.chromium.org/chromium/src/components/policy/core/common/extension_policy_migrator.h?l=41&rcl=5869d07ac72eff2bc003036e41ca785c02373d4d [2] https://cs.chromium.org/chromium/src/extensions/common/hashed_extension_id.h?l=17&rcl=b83d07a07ce7d22d429d673cb3c4494537dac754
,
Sep 10
Can you clarify the reason for hiding the identities of whitelisted extensions? My understanding of the previous bar for obfuscating the ID of an extension was to protect the identity of extensions related to unreleased Google products. As a general principle I believe we should be transparent about which extensions have access to private APIs.
,
Sep 10
Thanks for providing a bit more context on the purpose of the obfuscation The reasoning here is: extensions used for CopyPoliciesIfUnset() aren't _necessarily_ public, and we'd rather be safe than to accidentally leak sensitive information. CopyPoliciesIfUnset() is not directly related to private APIs, so it seems my explanation in Comment 1 was off.
,
Sep 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b248a97993e6af4190ec6654946945fbb2d01661 commit b248a97993e6af4190ec6654946945fbb2d01661 Author: Shahbaz Youssefi <syoussefi@chromium.org> Date: Tue Sep 11 15:14:36 2018 Avoid explicit extensions ids for policy migrator `ExtensionPolicyMigrator::CopyPoliciesIfUnset()` now takes the hashed extension id, instead of the extension id itself, and linearly searches in the policy bundle to find a matching policy. Where this function was called with an extension id, the extension id is replaced with the hashed one. Note that HashedExtensionId generates a hash that uses upper-case letters. Bug: 878528 Change-Id: I6290fe894cfcc6e0c440b639908bd1f0866d49ba Reviewed-on: https://chromium-review.googlesource.com/1207891 Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Cr-Commit-Position: refs/heads/master@{#590311} [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/browser/BUILD.gn [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_policy_migrator.cc [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_policy_migrator.h [add] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/browser/policy/chrome_extension_policy_migrator.cc [add] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/browser/policy/chrome_extension_policy_migrator.h [rename] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/browser/policy/chrome_extension_policy_migrator_unittest.cc [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/chrome/test/BUILD.gn [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/components/policy/core/common/BUILD.gn [delete] https://crrev.com/10289ef3c51d3d6045ad062d7926fb2dd18f70de/components/policy/core/common/extension_policy_migrator.cc [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/components/policy/core/common/extension_policy_migrator.h [modify] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/extensions/common/BUILD.gn [add] https://crrev.com/b248a97993e6af4190ec6654946945fbb2d01661/extensions/common/hashed_extension_id_unittest.cc
,
Sep 11
|
||
►
Sign in to add a comment |
||
Comment 1 by syoussefi@chromium.org
, Sep 4