Flag and whitelist to allow crxfs api in NaCl

chrome/browser/renderer_host/pepper/pepper_crx_file_system_message_filter.cc

Index: chrome/browser/renderer_host/pepper/pepper_crx_file_system_message_filter.cc
diff --git a/chrome/browser/renderer_host/pepper/pepper_crx_file_system_message_filter.cc b/chrome/browser/renderer_host/pepper/pepper_crx_file_system_message_filter.cc
index 0a95a17a9dff2a5b32b50de8d92018bfe626fdd7..341e95a669c48c0d44247a91ed37f9fee7d9e3b2 100644
--- a/chrome/browser/renderer_host/pepper/pepper_crx_file_system_message_filter.cc
+++ b/chrome/browser/renderer_host/pepper/pepper_crx_file_system_message_filter.cc
@@ -7,11 +7,15 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/pepper_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_ppapi_host.h"
 #include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/site_instance.h"
 #include "extensions/common/constants.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/host/dispatch_host_message.h"
@@ -22,29 +26,43 @@
 
 namespace chrome {
 
+namespace {
+
+const char* kPredefinedAllowedCrxFsOrigins[] = {
+  "6EAED1924DB611B6EEF2A664BD077BE7EAD33B8F"  // see crbug.com/234789
+};
+
+}  // namespace
+
 // static
 PepperCrxFileSystemMessageFilter* PepperCrxFileSystemMessageFilter::Create(
     PP_Instance instance, content::BrowserPpapiHost* host) {
   int render_process_id;
-  int unused_render_view_id;
+  int render_view_id;
   if (!host->GetRenderViewIDsForInstance(instance,
                                          &render_process_id,
-                                         &unused_render_view_id)) {
+                                         &render_view_id)) {
     return NULL;
   }
   return new PepperCrxFileSystemMessageFilter(
       render_process_id,
+      render_view_id,
       host->GetProfileDataDirectory(),
       host->GetDocumentURLForInstance(instance));
 }
 
 PepperCrxFileSystemMessageFilter::PepperCrxFileSystemMessageFilter(
     int render_process_id,
+    int render_view_id,
     const base::FilePath& profile_directory,
     const GURL& document_url)
     : render_process_id_(render_process_id),
+      render_view_id_(render_view_id),
       profile_directory_(profile_directory),
       document_url_(document_url) {
+  for (size_t i = 0; i < arraysize(kPredefinedAllowedCrxFsOrigins); ++i)
+    allowed_crxfs_origins_.insert(kPredefinedAllowedCrxFsOrigins[i]);
+

[yzshen1, 2013/05/22 18:14:23]

nit: unnecessary empty line.

[victorhsieh, 2013/05/22 19:47:33]

Done.

 }
 
 PepperCrxFileSystemMessageFilter::~PepperCrxFileSystemMessageFilter() {
@@ -68,13 +86,14 @@ int32_t PepperCrxFileSystemMessageFilter::OnResourceMessageReceived(
   return PP_ERROR_FAILED;
 }
 
-std::string PepperCrxFileSystemMessageFilter::CreateIsolatedFileSystem() {
+Profile* PepperCrxFileSystemMessageFilter::GetProfile() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!document_url_.SchemeIs(extensions::kExtensionScheme))
-    return std::string();
-
   ProfileManager* profile_manager = g_browser_process->profile_manager();
-  Profile* profile = profile_manager->GetProfile(profile_directory_);
+  return profile_manager->GetProfile(profile_directory_);
+}
+
+std::string PepperCrxFileSystemMessageFilter::CreateIsolatedFileSystem(
+    Profile* profile) {
   extensions::ExtensionSystem* extension_system =
       extensions::ExtensionSystem::Get(profile);
   if (!extension_system)
@@ -98,9 +117,35 @@ std::string PepperCrxFileSystemMessageFilter::CreateIsolatedFileSystem() {
                                 &kFirstLevelDirectory);
 }
 
+bool PepperCrxFileSystemMessageFilter::CanUseCrxFsAPI(Profile* profile) const {
+  content::RenderViewHost* render_view_host =
+      content::RenderViewHost::FromID(render_process_id_, render_view_id_);
+  if (!render_view_host)
+    return false;
+  content::SiteInstance* site_instance = render_view_host->GetSiteInstance();

[yzshen1, 2013/05/22 18:14:23]

Why do we need site_instance here?

[victorhsieh, 2013/05/22 19:47:33]

Done.

[yzshen1, 2013/05/22 20:12:47]

Now we can remove render_view_id_ entirely.

On 2013/05/22 19:47:33, victorhsieh wrote:

[victorhsieh, 2013/05/22 20:52:22]

Done.

+  if (!site_instance)
+    return false;
+  if (!IsExtensionOrSharedModuleWhitelisted(profile,
+                                            document_url_,
+                                            allowed_crxfs_origins_,
+                                            switches::kAllowNaClCrxFsAPI)) {
+    LOG(ERROR) << "Host " << document_url_.host()
+               << " cannot use CrxFs API or destination is not allowed";

[yzshen1, 2013/05/22 18:14:23]

What is the meaning of 'destination' here?

[victorhsieh, 2013/05/22 19:47:33]

Revised.  I thought it's about origin when copying the message from
pepper_socket_utils.cc, but it's not.

+    return false;
+  }
+  return true;
+}
+
 int32_t PepperCrxFileSystemMessageFilter::OnOpenFileSystem(
     ppapi::host::HostMessageContext* context) {
-  const std::string fsid = CreateIsolatedFileSystem();
+  if (!document_url_.SchemeIs(extensions::kExtensionScheme))
+    return PP_ERROR_NOTSUPPORTED;

[yzshen1, 2013/05/22 18:14:23]

(1) Is it better to use PP_ERROR_NOACCESS for this and below?

(2) I think this scheme check is also performed by
IsExtensionOrSharedModuleWhitelisted(), maybe we could remove it?

[victorhsieh, 2013/05/22 19:47:33]

Removed.

+
+  Profile* profile = GetProfile();
+  if (!CanUseCrxFsAPI(profile))
+    return PP_ERROR_NOTSUPPORTED;
+
+  const std::string fsid = CreateIsolatedFileSystem(profile);
   if (fsid.empty()) {
     context->reply_msg =
         PpapiPluginMsg_Ext_CrxFileSystem_BrowserOpenReply(std::string());