Non-SFI NaCl: Batch-open resource files

components/nacl/renderer/ppb_nacl_private_impl.cc

 // Copyright 2013 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 "components/nacl/renderer/ppb_nacl_private_impl.h"
 
 #include <numeric>
 #include <string>
 #include <vector>
 
@@ skipping 71 matching lines @@
   if (!render_thread)
     return false;
   if (!g_pnacl_resource_host.Get().get()) {
     g_pnacl_resource_host.Get() = new PnaclTranslationResourceHost(
         render_thread->GetIOMessageLoopProxy());
     render_thread->AddFilter(g_pnacl_resource_host.Get().get());
   }
   return true;
 }
 
+bool CheckSecurityOrigin(content::PepperPluginInstance* plugin_instance,

[Mark Seaborn, 2015/03/04 05:07:31]

If the SchemeIs("chrome-extension") check moves here, this could be named
CanOpenViaFastPath(), perhaps?

[Yusuke Sato, 2015/03/04 18:45:49]

Done.

+                         const GURL& gurl) {
+  // IMPORTANT: Make sure the document can request the given URL. If we don't
+  // check, a malicious app could probe the extension system. This enforces a
+  // same-origin policy which prevents the app from requesting resources from
+  // another app.
+  blink::WebSecurityOrigin security_origin =
+      plugin_instance->GetContainer()->element().document().securityOrigin();
+  return security_origin.canRequest(gurl);
+}
+
 // This contains state that is produced by LaunchSelLdr() and consumed
 // by StartPpapiProxy().
 struct InstanceInfo {
   InstanceInfo() : plugin_pid(base::kNullProcessId), plugin_child_id(0) {}
   GURL url;
   ppapi::PpapiPermissions permissions;
   base::ProcessId plugin_pid;
   int plugin_child_id;
   IPC::ChannelHandle channel_handle;
 };
@@ skipping 266 matching lines @@
   // destructed (without passing it to ManifestServiceChannel).
   scoped_ptr<ManifestServiceChannel::Delegate> manifest_service_proxy(
       new ManifestServiceProxy(instance, process_type));
 
   FileDescriptor result_socket;
   IPC::Sender* sender = content::RenderThread::Get();
   DCHECK(sender);
   int routing_id = GetRoutingID(instance);
   NexeLoadManager* load_manager = GetNexeLoadManager(instance);
   DCHECK(load_manager);
-  if (!routing_id || !load_manager) {
+  content::PepperPluginInstance* plugin_instance =
+      content::PepperPluginInstance::Get(instance);
+  DCHECK(plugin_instance);
+  if (!routing_id || !load_manager || !plugin_instance) {
     if (nexe_file_info->handle != PP_kInvalidFileHandle) {
       base::File closer(nexe_file_info->handle);
     }
     ppapi::PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
         FROM_HERE, base::Bind(callback.func, callback.user_data,
                               static_cast<int32_t>(PP_ERROR_FAILED)));
     return;
   }
 
   InstanceInfo instance_info;
   instance_info.url = GURL(alleged_url);
 
   uint32_t perm_bits = ppapi::PERMISSION_NONE;
   // Conditionally block 'Dev' interfaces. We do this for the NaCl process, so
   // it's clearer to developers when they are using 'Dev' inappropriately. We
   // must also check on the trusted side of the proxy.
   if (load_manager->DevInterfacesEnabled())
     perm_bits |= ppapi::PERMISSION_DEV;
   instance_info.permissions =
       ppapi::PpapiPermissions::GetForCommandLine(perm_bits);
   std::string error_message_string;
   NaClLaunchResult launch_result;
 
   IPC::PlatformFileForTransit nexe_for_transit =
       IPC::InvalidPlatformFileForTransit();
+
+  std::vector<std::pair<
+    std::string /*url*/, std::string /*key*/> > resource_files_to_prefetch;
+  if (process_type == kNativeNaClProcessType) {

[Mark Seaborn, 2015/03/04 05:07:31]

This should be conditionalised on Non-SFI mode for now.  Otherwise Chromium will
potentially be doing some unnecessary work to pre-open these files in SFI mode.

(I think it was conditionalised before but that got lost in the refactoring?)

[Yusuke Sato, 2015/03/04 18:45:49]

Done.

+    JsonManifest* manifest = GetJsonManifest(instance);
+    if (manifest)
+      manifest->GetPrefetchableFiles(&resource_files_to_prefetch);
+    for (size_t i = 0; i < resource_files_to_prefetch.size(); ++i) {
+      const GURL gurl(resource_files_to_prefetch[i].first);
+      DCHECK(gurl.SchemeIs("chrome-extension"));

[Mark Seaborn, 2015/03/04 05:07:30]

I had in mind that this check would move into CheckSecurityOrigin() and be
shared.

[Yusuke Sato, 2015/03/04 18:45:50]

Done. Removed.

+      // IMPORTANT SECURITY CHECK. DO NOT REMOVE.

[Mark Seaborn, 2015/03/04 05:07:31]

Nit: doesn't really need caps. :-)

The caps are kind of an implicit way of saying "we don't have test coverage for
this security check, in the style of 'goto fail'". :-/  (Obviously that's not
your fault, and we don't have the infrastructure to test for this
straightforwardly.)

[Yusuke Sato, 2015/03/04 18:45:49]

Done.

+      if (!CheckSecurityOrigin(plugin_instance, gurl)) {
+        resource_files_to_prefetch.clear();
+        break;
+      }
+    }
+  }
+
 #if defined(OS_POSIX)
   if (nexe_file_info->handle != PP_kInvalidFileHandle)
     nexe_for_transit = base::FileDescriptor(nexe_file_info->handle, true);
 #elif defined(OS_WIN)
   // Duplicate the handle on the browser side instead of the renderer.
   // This is because BrokerGetFileForProcess isn't part of content/public, and
   // it's simpler to do the duplication in the browser anyway.
   nexe_for_transit = nexe_file_info->handle;
 #else
 #error Unsupported target platform.
 #endif
   if (!sender->Send(new NaClHostMsg_LaunchNaCl(
           NaClLaunchParams(
               instance_info.url.spec(),
               nexe_for_transit,
               nexe_file_info->token_lo,
               nexe_file_info->token_hi,
+              resource_files_to_prefetch,
               routing_id,
               perm_bits,
               PP_ToBool(uses_nonsfi_mode),
               process_type),
           &launch_result,
           &error_message_string))) {
     ppapi::PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
         FROM_HERE,
         base::Bind(callback.func, callback.user_data,
                    static_cast<int32_t>(PP_ERROR_FAILED)));
@@ skipping 275 matching lines @@
     nacl_plugin_instance->pexe_size = pexe_size;
   }
 }
 
 PP_FileHandle OpenNaClExecutable(PP_Instance instance,
                                  const char* file_url,
                                  uint64_t* nonce_lo,
                                  uint64_t* nonce_hi) {
   // Fast path only works for installed file URLs.
   GURL gurl(file_url);
   if (!gurl.SchemeIs("chrome-extension"))

[Mark Seaborn, 2015/03/04 05:07:30]

Ditto: I had in mind that this check would move into CheckSecurityOrigin() and
be shared.

[Yusuke Sato, 2015/03/04 18:45:49]

Done.

     return PP_kInvalidFileHandle;
 
   NexeLoadManager* load_manager = GetNexeLoadManager(instance);
   DCHECK(load_manager);
   if (!load_manager)
     return PP_kInvalidFileHandle;
 
   content::PepperPluginInstance* plugin_instance =
       content::PepperPluginInstance::Get(instance);
   if (!plugin_instance)
     return PP_kInvalidFileHandle;
-  // IMPORTANT: Make sure the document can request the given URL. If we don't
-  // check, a malicious app could probe the extension system. This enforces a
-  // same-origin policy which prevents the app from requesting resources from
-  // another app.
-  blink::WebSecurityOrigin security_origin =
-      plugin_instance->GetContainer()->element().document().securityOrigin();
-  if (!security_origin.canRequest(gurl))
+
+  // IMPORTANT SECURITY CHECK. DO NOT REMOVE.
+  if (!CheckSecurityOrigin(plugin_instance, gurl))
     return PP_kInvalidFileHandle;
 
   IPC::PlatformFileForTransit out_fd = IPC::InvalidPlatformFileForTransit();
   IPC::Sender* sender = content::RenderThread::Get();
   DCHECK(sender);
   *nonce_lo = 0;
   *nonce_hi = 0;
   base::FilePath file_path;
   if (!sender->Send(
       new NaClHostMsg_OpenNaClExecutable(GetRoutingID(instance),
@@ skipping 906 matching lines @@
   &StreamPexe
 };
 
 }  // namespace
 
 const PPB_NaCl_Private* GetNaClPrivateInterface() {
   return &nacl_interface;
 }
 
 }  // namespace nacl