PS - Adjusting webRequest API for use in Public Sessions

extensions/browser/api/web_request/web_request_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/api/web_request/web_request_api.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "chromeos/login/login_state.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/child_process_host.h"
 #include "extensions/browser/api/activity_log/web_request_constants.h"
 #include "extensions/browser/api/declarative/rules_registry_service.h"
 #include "extensions/browser/api/declarative_webrequest/request_stage.h"
 #include "extensions/browser/api/declarative_webrequest/webrequest_constants.h"
 #include "extensions/browser/api/declarative_webrequest/webrequest_rules_registry.h"
@@ skipping 296 matching lines @@
 // We hide events from the system context as well as sensitive requests.
 bool ShouldHideEvent(void* browser_context,
                      const InfoMap* extension_info_map,
                      const net::URLRequest* request,
                      ExtensionNavigationUIData* navigation_ui_data) {
   return (!browser_context ||
           WebRequestPermissions::HideRequest(extension_info_map, request,
                                              navigation_ui_data));
 }
 
+// Returns true if we're in a Public Session.
+bool IsPublicSession() {
+#if defined(OS_CHROMEOS)
+  if (chromeos::LoginState::IsInitialized()) {
+    return chromeos::LoginState::Get()->IsPublicSessionUser();
+  }
+#endif
+  return false;
+}
+
 }  // namespace
 
 WebRequestAPI::WebRequestAPI(content::BrowserContext* context)
     : browser_context_(context) {
   EventRouter* event_router = EventRouter::Get(browser_context_);
   for (size_t i = 0; i < arraysize(kWebRequestEvents); ++i) {
     // Observe the webRequest event.
     std::string event_name = kWebRequestEvents[i];
     event_router->RegisterObserver(this, event_name);
 
@@ skipping 1082 matching lines @@
         std::find(types.begin(), types.end(), resource_type) == types.end()) {
       continue;
     }
 
     if (!is_web_view_guest) {
       PermissionsData::AccessType access =
           WebRequestPermissions::CanExtensionAccessURL(
               extension_info_map, listener->id.extension_id, url,
               frame_data.tab_id, crosses_incognito,
               WebRequestPermissions::REQUIRE_HOST_PERMISSION);
+
+      // When we are in a Public Session, allow all URL's for webRequests
+      // initiated by a regular extension.
+      if (access != PermissionsData::ACCESS_ALLOWED && extension_info_map) {
+        const extensions::Extension* extension =
+            extension_info_map->extensions().GetByID(listener->id.extension_id);
+        if (IsPublicSession() && extension && extension->is_extension()) {

[Devlin, 2016/11/03 00:33:46]

We want to do this for *all* extensions?  Is it always safe to assume that an
extension in a public session was installed by the admin?

Also, this will allow even restricted urls, like chrome:// urls.  I get the
feeling we don't want to allow that.

[Ivan Šandrk, 2016/11/03 14:45:12]

Yes, for all regular extensions. Yes, users aren't allowed to install
extensions, there is a check in place for this -
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/extensions/devic...
(EXTERNAL_POLICY means that the extension is force installed by policy).

That's a good point about restricted urls. I've got a simple test extension for
testing this stuff out, I couldn't register a chrome:// url. Following the error
message I ended up here
https://cs.chromium.org/chromium/src/extensions/browser/api/web_request/web_r...

Http/https/ftp links are fine, chrome-extension:// links should also be fine
(all extensions are admin installed), and I think file:// links should also be
fine. I don't think people use the browser to browse their local files +
everything is stripped out; for example file:///boot/abi-3.13.0-100-generic is
shown only as file:/// (URLs are stripped down to origin). I could limit the
urls only to http?/ftp, but I think that's just adding unnecessary complexity.
WDYT?

[Devlin, 2016/11/05 06:04:37]

If security/privacy are on board with this, then that's fine.  I was mostly
worried about chrome:// urls.

If all extensions must be installed by policy, can we add a
CHECK(Manifest::IsPolicyLocation(extension->location())); here?  That would make
me feel a bit better.

[Ivan Šandrk, 2016/11/07 10:50:02]

S&P are fine.

Done.

+          access = PermissionsData::ACCESS_ALLOWED;
+        }
+      }
+
       if (access != PermissionsData::ACCESS_ALLOWED) {
         if (access == PermissionsData::ACCESS_WITHHELD &&
             web_request_event_router_delegate_) {
           web_request_event_router_delegate_->NotifyWebRequestWithheld(
               render_process_id, render_frame_id, listener->id.extension_id);
         }
         continue;
       }
     }
 
@@ skipping 667 matching lines @@
             APIPermission::kWebRequestBlocking)) {
       return RespondNow(Error(keys::kBlockingPermissionRequired));
     }
 
     // We allow to subscribe to patterns that are broader than the host
     // permissions. E.g., we could subscribe to http://www.example.com/*
     // while having host permissions for http://www.example.com/foo/* and
     // http://www.example.com/bar/*.
     // For this reason we do only a coarse check here to warn the extension
     // developer if they do something obviously wrong.
-    if (extension->permissions_data()
+    // When we are in a Public Session, allow all URL's for webRequests
+    // initiated by a regular extension.
+    if (!(IsPublicSession() && extension->is_extension()) &&
+        extension->permissions_data()
             ->GetEffectiveHostPermissions()
             .is_empty() &&
         extension->permissions_data()
             ->withheld_permissions()
             .explicit_hosts()
             .is_empty()) {
       return RespondNow(Error(keys::kHostPermissionsRequired));
     }
   }
 
@@ skipping 219 matching lines @@
   // Since EventListeners are segmented by browser_context, check that
   // last, as it is exceedingly unlikely to be different.
   return extension_id == that.extension_id &&
          sub_event_name == that.sub_event_name &&
          web_view_instance_id == that.web_view_instance_id &&
          embedder_process_id == that.embedder_process_id &&
          browser_context == that.browser_context;
 }
 
 }  // namespace extensions