Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(71)

Side by Side Diff: chrome/browser/chromeos/extensions/device_local_account_management_policy_provider.cc

Issue 2367363002: Update Public Session permissions (manifest features part) (Closed)
Patch Set: Added a crbug link to a TODO Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | chrome/browser/chromeos/extensions/device_local_account_management_policy_provider_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/chromeos/extensions/device_local_account_management_pol icy_provider.h" 5 #include "chrome/browser/chromeos/extensions/device_local_account_management_pol icy_provider.h"
6 6
7 #include <stddef.h> 7 #include <stddef.h>
8 8
9 #include <cstddef> 9 #include <cstddef>
10 #include <string> 10 #include <string>
(...skipping 106 matching lines...) Expand 10 before | Expand all | Expand 10 after
117 // Testing extensions: 117 // Testing extensions:
118 "ongnjlefhnoajpbodoldndkbkdgfomlp", // Show Managed Storage 118 "ongnjlefhnoajpbodoldndkbkdgfomlp", // Show Managed Storage
119 "ilnpadgckeacioehlommkaafedibdeob", // Enterprise DeviceAttributes 119 "ilnpadgckeacioehlommkaafedibdeob", // Enterprise DeviceAttributes
120 "oflckobdemeldmjddmlbaiaookhhcngo", // Citrix Receiver QA version 120 "oflckobdemeldmjddmlbaiaookhhcngo", // Citrix Receiver QA version
121 "ljacajndfccfgnfohlgkdphmbnpkjflk", // Chrome Remote Desktop (Dev Build) 121 "ljacajndfccfgnfohlgkdphmbnpkjflk", // Chrome Remote Desktop (Dev Build)
122 }; 122 };
123 123
124 // List of manifest entries from https://developer.chrome.com/apps/manifest. 124 // List of manifest entries from https://developer.chrome.com/apps/manifest.
125 // Unsafe entries are commented out and special cases too. 125 // Unsafe entries are commented out and special cases too.
126 const char* const kSafeManifestEntries[] = { 126 const char* const kSafeManifestEntries[] = {
127 // Special-cased in IsPlatformAppSafeForPublicSession(). 127 emk::kAboutPage,
128
129 // Special-cased in IsSafeForPublicSession().
128 // emk::kApp, 130 // emk::kApp,
129 131
130 // Documented in https://developer.chrome.com/extensions/manifest but not 132 // Documented in https://developer.chrome.com/extensions/manifest but not
131 // implemented anywhere. Still, a lot of apps use it. 133 // implemented anywhere. Still, a lot of apps use it.
132 "author", 134 "author",
133 135
134 // TBD 136 // Allows inspection of page contents, not enabled on stable anyways except
135 // emk::kBluetooth, 137 // for whitelist.
138 // emk::kAutomation,
136 139
137 // TBD 140 // TODO(isandrk): Ask Mattias for comments on entries without a comment.
138 // emk::kCommands, 141 // See crbug.com/650672.
142 "background",
143
144 emk::kBackgroundPageLegacy,
145
146 emk::kBackgroundPersistent,
147
148 emk::kBluetooth,
149
150 emk::kBrowserAction,
151
152 // Allows to replace the search provider which is somewhat risky - need to
153 // double check how the search provider policy behaves in PS.
154 // emk::kSettingsOverride,
155
156 // Custom bookmark managers - I think this is fair game, bookmarks should be
157 // URLs only, and it's restricted to whitelist on stable.
158 emk::kUIOverride,
159
160 // Bookmark manager, history, new tab - should be safe.
161 emk::kChromeURLOverrides,
162
163 // General risk of capturing user input, but key combos must include Ctrl or
164 // Alt, so I think this is safe.
165 emk::kCommands,
166
167 // General risk of capturing user input, but key combos must include Ctrl or
168 // Alt, so I think this is safe.
169 emk::kContentCapabilities,
170
171 // Access to web content.
172 // emk::kContentScripts,
173
174 emk::kContentSecurityPolicy,
175
176 // Access to web content.
177 // emk::kConvertedFromUserScript,
139 178
140 // An implementation detail (actually written by Chrome, not the app 179 // An implementation detail (actually written by Chrome, not the app
141 // author). 180 // author).
142 emk::kCurrentLocale, 181 emk::kCurrentLocale,
143 182
144 // Name of directory containg default strings. 183 // Name of directory containg default strings.
145 emk::kDefaultLocale, 184 emk::kDefaultLocale,
146 185
147 // Just a display string. 186 // Just a display string.
148 emk::kDescription, 187 emk::kDescription,
149 188
150 // TBD, looks unsafe 189 // Access to web content.
190 // emk::kDevToolsPage,
191
192 // Restricted to whitelist already.
193 emk::kDisplayInLauncher,
194
195 emk::kDisplayInNewTabPage,
196
197 // This allows to declaratively filter web requests and content, matching on
198 // content data. Doesn't allow direct access to request/content data. Can be
199 // used to brute-force e.g. cookies (reload with filter rules adjusted to
200 // match all possible cookie values) - but that's equivalent to an
201 // off-device brute-force attack.
202 // Looks safe in general with one exception: There's an action that allows
203 // to insert content scripts on matching content. We can't allow this, need
204 // to check whether there's also a host permission required for this case.
151 // emk::kEventRules, 205 // emk::kEventRules,
152 206
153 // Shared Modules configuration: Allow other extensions to access resources. 207 // Shared Modules configuration: Allow other extensions to access resources.
154 emk::kExport, 208 emk::kExport,
155 209
156 // TBD 210 emk::kExternallyConnectable,
157 // emk::kExternallyConnectable,
158 211
159 // TBD 212 emk::kFileBrowserHandlers,
160 // emk::kFileHandlers,
161 213
162 // TBD 214 // Extension file handlers are restricted to whitelist which only contains
163 // emk::kFileSystemProviderCapabilities, 215 // quickoffice.
216 emk::kFileHandlers,
217
218 emk::kFileSystemProviderCapabilities,
219
220 emk::kHomepageURL,
164 221
165 // Just UX. 222 // Just UX.
166 emk::kIcons, 223 emk::kIcons,
167 224
168 // Shared Modules configuration: Import resources from another extension. 225 // Shared Modules configuration: Import resources from another extension.
169 emk::kImport, 226 emk::kImport,
170 227
228 emk::kIncognito,
229
230 // Keylogging.
231 // emk::kInputComponents,
232
171 // Shared Modules configuration: Specify extension id for development. 233 // Shared Modules configuration: Specify extension id for development.
172 emk::kKey, 234 emk::kKey,
173 235
174 // Descriptive statement about the app. 236 emk::kKiosk,
237
175 emk::kKioskEnabled, 238 emk::kKioskEnabled,
176 239
177 // Contradicts the purpose of running inside a Public Session. 240 // Not useful since it will prevent app from running, but we don't care.
178 // emk::kKioskOnly, 241 emk::kKioskOnly,
179 242
180 // Special-cased in IsPlatformAppSafeForPublicSession(). 243 emk::kKioskRequiredPlatformVersion,
244
245 // Not useful since it will prevent app from running, but we don't care.
246 emk::kKioskSecondaryApps,
247
248 // Whitelisted to only allow Google Now.
249 emk::kLauncherPage,
250
251 // Special-cased in IsSafeForPublicSession().
181 // emk::kManifestVersion, 252 // emk::kManifestVersion,
182 253
183 // Descriptive statement about the app. 254 emk::kMIMETypes,
255
256 // Whitelisted to only allow browser tests and PDF viewer.
257 emk::kMimeTypesHandler,
258
184 emk::kMinimumChromeVersion, 259 emk::kMinimumChromeVersion,
185 260
186 // NaCl modules are bound to app permissions just like the rest of the app 261 // NaCl modules are bound to app permissions just like the rest of the app
187 // and thus should not pose a risk. 262 // and thus should not pose a risk.
188 emk::kNaClModules, 263 emk::kNaClModules,
189 264
190 // Just a display string. 265 // Just a display string.
191 emk::kName, 266 emk::kName,
192 267
193 // TBD, doc missing 268 // Used in conjunction with the identity API - not really used when there's
194 // emk::kOAuth2, 269 // no GAIA user signed in.
270 emk::kOAuth2,
195 271
196 // Descriptive statement about the app. 272 // Generally safe (i.e. only whitelist apps), except for the policy to
273 // whitelist apps for auto-approved token minting (we should just ignore
274 // this in public sessions). Risk is that admin mints OAuth tokens to access
275 // services on behalf of the user silently.
276 // emk::kOAuth2AutoApprove,
277
197 emk::kOfflineEnabled, 278 emk::kOfflineEnabled,
198 279
199 // Special-cased in IsPlatformAppSafeForPublicSession(). 280 // A bit risky as the extensions sees all keystrokes entered into the
281 // omnibox after the search key matches, but generally we deem URLs fair
282 // game.
283 emk::kOmnibox,
284
285 // Special-cased in IsSafeForPublicSession(). Subject to permission
286 // restrictions.
200 // emk::kOptionalPermissions, 287 // emk::kOptionalPermissions,
201 288
202 // Special-cased in IsPlatformAppSafeForPublicSession(). 289 emk::kOptionsPage,
290
291 emk::kOptionsUI,
292
293 emk::kPageAction,
294
295 // Special-cased in IsSafeForPublicSession(). Subject to permission
296 // restrictions.
203 // emk::kPermissions, 297 // emk::kPermissions,
204 298
205 // No constant in manifest_constants.cc. 299 // No constant in manifest_constants.cc. Declared as a feature, but unused.
206 // "platforms", 300 // "platforms",
207 301
208 // Descriptive statement about the app. 302 // N/A on Chrome OS, so we don't care.
303 emk::kPlugins,
304
305 // Stated 3D/WebGL/plugin requirements of an app.
209 emk::kRequirements, 306 emk::kRequirements,
210 307
211 // Execute some pages in a separate sandbox. (Note: Using string literal 308 // Execute some pages in a separate sandbox. (Note: Using string literal
212 // since extensions::manifest_keys only has constants for sub-keys.) 309 // since extensions::manifest_keys only has constants for sub-keys.)
213 "sandbox", 310 "sandbox",
214 311
215 // Just a display string. 312 // Just a display string.
216 emk::kShortName, 313 emk::kShortName,
217 314
218 // TBD, doc missing 315 // Doc missing. Declared as a feature, but unused.
219 // emk::kSignature, 316 // emk::kSignature,
220 317
221 // Network access. 318 // Network access.
222 emk::kSockets, 319 emk::kSockets,
223 320
224 // TBD. (Note: Using string literal since extensions::manifest_keys only 321 // Just provides dictionaries, no access to content.
225 // has constants for sub-keys.) 322 emk::kSpellcheck,
226 // "storage",
227 323
228 // TBD, doc missing 324 // (Note: Using string literal since extensions::manifest_keys only has
229 // emk::kSystemIndicator, 325 // constants for sub-keys.)
326 "storage",
327
328 // Only Hangouts is whitelisted.
329 emk::kSystemIndicator,
330
331 emk::kTheme,
332
333 // Might need this for accessibilty, but has content access. Manual
334 // whitelisting might be reasonable here?
335 // emk::kTtsEngine,
230 336
231 // TODO(tnagel): Ensure that extension updates query UserMayLoad(). 337 // TODO(tnagel): Ensure that extension updates query UserMayLoad().
232 // https://crbug.com/549720 338 // https://crbug.com/549720
233 emk::kUpdateURL, 339 emk::kUpdateURL,
234 340
235 // Apps may intercept navigations to URL patterns for domains for which the 341 // Apps may intercept navigations to URL patterns for domains for which the
236 // app author has proven ownership of to the Web Store. (Chrome starts the 342 // app author has proven ownership of to the Web Store. (Chrome starts the
237 // app instead of fulfilling the navigation.) This is only safe for apps 343 // app instead of fulfilling the navigation.) This is only safe for apps
238 // that have been loaded from the Web Store and thus is special-cased in 344 // that have been loaded from the Web Store and thus is special-cased in
239 // IsPlatformAppSafeForPublicSession(). 345 // IsSafeForPublicSession().
240 // emk::kUrlHandlers, 346 // emk::kUrlHandlers,
241 347
242 // TBD 348 emk::kUsbPrinters,
243 // emk::kUsbPrinters,
244 349
245 // Version string (for app updates). 350 // Version string (for app updates).
246 emk::kVersion, 351 emk::kVersion,
247 352
248 // Just a display string. 353 // Just a display string.
249 emk::kVersionName, 354 emk::kVersionName,
250 355
356 emk::kWebAccessibleResources,
357
251 // Webview has no special privileges or capabilities. 358 // Webview has no special privileges or capabilities.
252 emk::kWebview, 359 emk::kWebview,
253 }; 360 };
254 361
255 // List of permission strings based on [1] and [2]. See |kSafePermissionDicts| 362 // List of permission strings based on [1] and [2]. See |kSafePermissionDicts|
256 // for permission dicts. Since Public Session users may be fully unaware of any 363 // for permission dicts. Since Public Session users may be fully unaware of any
257 // apps being installed, their consent to access any kind of sensitive 364 // apps being installed, their consent to access any kind of sensitive
258 // information cannot be assumed. Therefore only APIs are whitelisted which 365 // information cannot be assumed. Therefore only APIs are whitelisted which
259 // should not leak sensitive data to the caller. Since the privacy boundary is 366 // should not leak sensitive data to the caller. Since the privacy boundary is
260 // drawn at the API level, no safeguards are required to prevent exfiltration 367 // drawn at the API level, no safeguards are required to prevent exfiltration
(...skipping 137 matching lines...) Expand 10 before | Expand all | Expand 10 after
398 // Some permissions take the form of a dictionary. See |kSafePermissionStrings| 505 // Some permissions take the form of a dictionary. See |kSafePermissionStrings|
399 // for permission strings (and for more documentation). 506 // for permission strings (and for more documentation).
400 const char* const kSafePermissionDicts[] = { 507 const char* const kSafePermissionDicts[] = {
401 // TBD 508 // TBD
402 // "fileSystem", 509 // "fileSystem",
403 510
404 // Just another type of connectivity. 511 // Just another type of connectivity.
405 "socket", 512 "socket",
406 }; 513 };
407 514
515 // List of safe entries for the "app" dict in manifest.
516 const char* const kSafeAppStrings[] = {
517 "background",
518 "content_security_policy",
519 "icon_color",
520 "isolation",
521 "launch",
522 "linked_icons",
523 };
524
408 // Return true iff |entry| is contained in |char_array|. 525 // Return true iff |entry| is contained in |char_array|.
409 bool ArrayContainsImpl(const char* const char_array[], 526 bool ArrayContainsImpl(const char* const char_array[],
410 size_t entry_count, 527 size_t entry_count,
411 const std::string& entry) { 528 const std::string& entry) {
412 for (size_t i = 0; i < entry_count; ++i) { 529 for (size_t i = 0; i < entry_count; ++i) {
413 if (entry == char_array[i]) { 530 if (entry == char_array[i]) {
414 return true; 531 return true;
415 } 532 }
416 } 533 }
417 return false; 534 return false;
418 } 535 }
419 536
420 // See http://blogs.msdn.com/b/the1/archive/2004/05/07/128242.aspx for an 537 // See http://blogs.msdn.com/b/the1/archive/2004/05/07/128242.aspx for an
421 // explanation of array size determination. 538 // explanation of array size determination.
422 template <size_t N> 539 template <size_t N>
423 bool ArrayContains(const char* const (&char_array)[N], 540 bool ArrayContains(const char* const (&char_array)[N],
424 const std::string& entry) { 541 const std::string& entry) {
425 return ArrayContainsImpl(char_array, N, entry); 542 return ArrayContainsImpl(char_array, N, entry);
426 } 543 }
427 544
428 // Returns true for platform apps that are considered safe for Public Sessions, 545 // Returns true for extensions that are considered safe for Public Sessions,
429 // which among other things requires the manifest top-level entries to be 546 // which among other things requires the manifest top-level entries to be
430 // contained in the |kSafeManifestEntries| whitelist and all permissions to be 547 // contained in the |kSafeManifestEntries| whitelist and all permissions to be
431 // contained in |kSafePermissionStrings| or |kSafePermissionDicts|. Otherwise 548 // contained in |kSafePermissionStrings| or |kSafePermissionDicts|. Otherwise
432 // returns false and logs all reasons for failure. 549 // returns false and logs all reasons for failure.
433 bool IsPlatformAppSafeForPublicSession(const extensions::Extension* extension) { 550 bool IsSafeForPublicSession(const extensions::Extension* extension) {
434 bool safe = true; 551 bool safe = true;
435 if (extension->GetType() != extensions::Manifest::TYPE_PLATFORM_APP) { 552 if (!extension->is_extension() &&
436 LOG(ERROR) << extension->id() << " is not a platform app."; 553 !extension->is_hosted_app() &&
554 !extension->is_platform_app() &&
555 !extension->is_shared_module() &&
556 !extension->is_theme()) {
557 LOG(ERROR) << extension->id()
558 << " is not of a supported type. Extension type: "
559 << extension->GetType();
437 safe = false; 560 safe = false;
438 } 561 }
439 562
440 for (base::DictionaryValue::Iterator it(*extension->manifest()->value()); 563 for (base::DictionaryValue::Iterator it(*extension->manifest()->value());
441 !it.IsAtEnd(); it.Advance()) { 564 !it.IsAtEnd(); it.Advance()) {
442 if (ArrayContains(kSafeManifestEntries, it.key())) { 565 if (ArrayContains(kSafeManifestEntries, it.key())) {
443 continue; 566 continue;
444 } 567 }
445 568
446 // Permissions must be whitelisted in |kSafePermissionStrings| or 569 // Permissions must be whitelisted in |kSafePermissionStrings| or
(...skipping 49 matching lines...) Expand 10 before | Expand all | Expand 10 after
496 base::CompareCase::SENSITIVE) || 619 base::CompareCase::SENSITIVE) ||
497 base::StartsWith(permission_string, "ftp://", 620 base::StartsWith(permission_string, "ftp://",
498 base::CompareCase::SENSITIVE)) { 621 base::CompareCase::SENSITIVE)) {
499 continue; 622 continue;
500 } 623 }
501 LOG(ERROR) << extension->id() 624 LOG(ERROR) << extension->id()
502 << " requested non-whitelisted permission: " 625 << " requested non-whitelisted permission: "
503 << permission_string; 626 << permission_string;
504 safe = false; 627 safe = false;
505 } 628 }
506 // "app" may only contain "background".
507 } else if (it.key() == emk::kApp) { 629 } else if (it.key() == emk::kApp) {
630 if (!extension->is_hosted_app() &&
631 !extension->is_platform_app()) {
632 LOG(ERROR) << extension->id()
633 << ": app manifest entry is allowed only for hosted_app or "
634 "platform_app extension type. Current extension type: "
635 << extension->GetType();
636 safe = false;
637 }
508 const base::DictionaryValue *dict_value; 638 const base::DictionaryValue *dict_value;
509 if (!it.value().GetAsDictionary(&dict_value)) { 639 if (!it.value().GetAsDictionary(&dict_value)) {
510 LOG(ERROR) << extension->id() << ": app is not a dictionary."; 640 LOG(ERROR) << extension->id() << ": app is not a dictionary.";
511 safe = false; 641 safe = false;
512 continue; 642 continue;
513 } 643 }
514 for (base::DictionaryValue::Iterator it2(*dict_value); 644 for (base::DictionaryValue::Iterator it2(*dict_value);
515 !it2.IsAtEnd(); it2.Advance()) { 645 !it2.IsAtEnd(); it2.Advance()) {
516 if (it2.key() != "background") { 646 if (!ArrayContains(kSafeAppStrings, it2.key())) {
517 LOG(ERROR) << extension->id() 647 LOG(ERROR) << extension->id()
518 << " has non-whitelisted manifest entry: " 648 << " has non-whitelisted manifest entry: "
519 << it.key() << "." << it2.key(); 649 << it.key() << "." << it2.key();
520 safe = false; 650 safe = false;
521 continue; 651 continue;
522 } 652 }
523 } 653 }
524 // Require v2 because that's the only version we understand. 654 // Require v2 because that's the only version we understand.
525 } else if (it.key() == emk::kManifestVersion) { 655 } else if (it.key() == emk::kManifestVersion) {
526 int version; 656 int version;
(...skipping 63 matching lines...) Expand 10 before | Expand all | Expand 10 after
590 if (extension->GetType() == extensions::Manifest::TYPE_HOSTED_APP) { 720 if (extension->GetType() == extensions::Manifest::TYPE_HOSTED_APP) {
591 return true; 721 return true;
592 } 722 }
593 723
594 // Allow extension if its specific ID is whitelisted for use in public 724 // Allow extension if its specific ID is whitelisted for use in public
595 // sessions. 725 // sessions.
596 if (ArrayContains(kPublicSessionWhitelist, extension->id())) { 726 if (ArrayContains(kPublicSessionWhitelist, extension->id())) {
597 return true; 727 return true;
598 } 728 }
599 729
600 // Allow force-installed platform app if all manifest contents are 730 // Allow force-installed extension if all manifest contents are whitelisted.
601 // whitelisted.
602 if ((extension->location() == extensions::Manifest::EXTERNAL_POLICY_DOWNLOAD 731 if ((extension->location() == extensions::Manifest::EXTERNAL_POLICY_DOWNLOAD
603 || extension->location() == extensions::Manifest::EXTERNAL_POLICY) 732 || extension->location() == extensions::Manifest::EXTERNAL_POLICY)
604 && IsPlatformAppSafeForPublicSession(extension)) { 733 && IsSafeForPublicSession(extension)) {
605 return true; 734 return true;
606 } 735 }
607 } else if (account_type_ == policy::DeviceLocalAccount::TYPE_KIOSK_APP) { 736 } else if (account_type_ == policy::DeviceLocalAccount::TYPE_KIOSK_APP) {
608 // For single-app kiosk sessions, allow platform apps, extesions and shared 737 // For single-app kiosk sessions, allow platform apps, extesions and shared
609 // modules. 738 // modules.
610 if (extension->GetType() == extensions::Manifest::TYPE_PLATFORM_APP || 739 if (extension->GetType() == extensions::Manifest::TYPE_PLATFORM_APP ||
611 extension->GetType() == extensions::Manifest::TYPE_SHARED_MODULE || 740 extension->GetType() == extensions::Manifest::TYPE_SHARED_MODULE ||
612 extension->GetType() == extensions::Manifest::TYPE_EXTENSION) { 741 extension->GetType() == extensions::Manifest::TYPE_EXTENSION) {
613 return true; 742 return true;
614 } 743 }
615 } 744 }
616 745
617 // Disallow all other extensions. 746 // Disallow all other extensions.
618 if (error) { 747 if (error) {
619 *error = l10n_util::GetStringFUTF16( 748 *error = l10n_util::GetStringFUTF16(
620 IDS_EXTENSION_CANT_INSTALL_IN_DEVICE_LOCAL_ACCOUNT, 749 IDS_EXTENSION_CANT_INSTALL_IN_DEVICE_LOCAL_ACCOUNT,
621 base::UTF8ToUTF16(extension->name()), 750 base::UTF8ToUTF16(extension->name()),
622 base::UTF8ToUTF16(extension->id())); 751 base::UTF8ToUTF16(extension->id()));
623 } 752 }
624 return false; 753 return false;
625 } 754 }
626 755
627 } // namespace chromeos 756 } // namespace chromeos
OLDNEW
« no previous file with comments | « no previous file | chrome/browser/chromeos/extensions/device_local_account_management_policy_provider_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698