Packaged (CRX) extensions shouldn't be able to get 'dev' interfaces in NaCl.BUG=100228TEST=manual

Review URL: http://codereview.chromium.org/8301006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105985 0039d316-1c4b-4281-b951-d872f2087c98

chrome/renderer/chrome_content_renderer_client.cc

@@ -417,54 +417,15 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
           IDR_CLICK_TO_PLAY_PLUGIN_HTML, IDS_PLUGIN_LOAD, true, true);
     }
 
-    // Enforce the Chrome WebStore restriction on the Native Client plugin.
+    if (is_nacl_plugin &&
-    if (is_nacl_plugin) {
+        !IsNaClAllowed(plugin,
-      bool allow_nacl = cmd->HasSwitch(switches::kEnableNaCl);
+                       url,
-      if (!allow_nacl) {
+                       actual_mime_type,
-        const char* kNaClPluginMimeType = "application/x-nacl";
+                       cmd->HasSwitch(switches::kEnableNaCl),
-        const char* kNaClPluginManifestAttribute = "nacl";
+                       params)) {
-
-        GURL nexe_url;
-        if (actual_mime_type == kNaClPluginMimeType) {
-          nexe_url = url;  // Normal embedded NaCl plugin.
-        } else {
-          // Content type handling NaCl plugin; the "nacl" param on the
-          // MIME type holds the nexe URL.
-          string16 nacl_attr = ASCIIToUTF16(kNaClPluginManifestAttribute);
-          for (size_t i = 0; i < plugin.mime_types.size(); ++i) {
-            if (plugin.mime_types[i].mime_type == actual_mime_type) {
-              const webkit::WebPluginMimeType& content_type =
-                  plugin.mime_types[i];
-              for (size_t i = 0;
-                  i < content_type.additional_param_names.size(); ++i) {
-                if (content_type.additional_param_names[i] == nacl_attr) {
-                  nexe_url = GURL(content_type.additional_param_values[i]);
-                  break;
-                }
-              }
-              break;
-            }
-          }
-        }
-
-        // Create the NaCl plugin only if the .nexe is part of an extension
-        // that was installed from the Chrome Web Store, or part of a component
-        // extension, or part of an unpacked extension.
-        const Extension* extension =
-            extension_dispatcher_->extensions()->GetByURL(nexe_url);
-        allow_nacl = extension &&
-            (extension->from_webstore() ||
-            extension->location() == Extension::COMPONENT ||
-            extension->location() == Extension::LOAD);
-      }
-
-      if (!allow_nacl) {
-        // TODO(bbudge) Webkit will crash if this is a full-frame plug-in and
-        // we return NULL. Prepare a patch to fix that, and return NULL here.
         return CreatePluginPlaceholder(
             render_view, frame, plugin, params, group.get(),
             IDR_BLOCKED_PLUGIN_HTML, IDS_PLUGIN_BLOCKED, false, false);
-      }
     }
 
     return render_view->CreatePlugin(frame, plugin, params);
@@ -484,6 +445,88 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
   }
 }
 
+bool ChromeContentRendererClient::IsNaClAllowed(
+    const webkit::WebPluginInfo& plugin,
+    const GURL& url,
+    const std::string& actual_mime_type,
+    bool enable_nacl,
+    WebKit::WebPluginParams& params) {
+  const char* kNaClPluginMimeType = "application/x-nacl";
+  const char* kNaClPluginManifestAttribute = "nacl";
+
+  GURL manifest_url;
+  if (actual_mime_type == kNaClPluginMimeType) {
+    manifest_url = url;  // Normal embedded NaCl plugin.
+  } else {
+    // This is a content type handling NaCl plugin; look for the .nexe URL
+    // among the MIME type's additonal parameters.
+    string16 nacl_attr = ASCIIToUTF16(kNaClPluginManifestAttribute);
+    for (size_t i = 0; i < plugin.mime_types.size(); ++i) {
+      if (plugin.mime_types[i].mime_type == actual_mime_type) {
+        const webkit::WebPluginMimeType& content_type =
+            plugin.mime_types[i];
+        for (size_t i = 0;
+            i < content_type.additional_param_names.size(); ++i) {
+          if (content_type.additional_param_names[i] == nacl_attr) {
+            manifest_url = GURL(content_type.additional_param_values[i]);
+            break;
+          }
+        }
+        break;
+      }
+    }
+  }
+
+  // Determine if the manifest URL is part of an extension.
+  const Extension* extension =
+      extension_dispatcher_->extensions()->GetByURL(manifest_url);
+  // Only component, unpacked, and Chrome Web Store extensions are allowed.
+  bool allowed_extension = extension &&
+      (extension->from_webstore() ||
+      extension->location() == Extension::COMPONENT ||
+      extension->location() == Extension::LOAD);
+
+  // Block any other use of NaCl plugin, unless --enable-nacl is set.
+  if (!allowed_extension && !enable_nacl)
+    return false;
+
+  // Allow dev interfaces for non-extension apps.
+  bool allow_dev_interfaces = true;
+  if (allowed_extension) {
+    // Allow dev interfaces for component and unpacked extensions.
+    if (extension->location() != Extension::COMPONENT &&
+        extension->location() != Extension::LOAD) {
+      // Whitelist all other allowed extensions.
+      allow_dev_interfaces =
+          // PDF Viewer plugin
+          (manifest_url.scheme() == "chrome-extension" &&
+          manifest_url.host() == "acadkphlmlegjaadjagenfimbpphcgnh");
+    }
+  }
+
+  WebString dev_attribute = WebString::fromUTF8("@dev");
+  if (allow_dev_interfaces) {
+    std::vector<string16> param_names;
+    std::vector<string16> param_values;
+    param_names.push_back(dev_attribute);
+    param_values.push_back(WebString());
+    AppendParams(
+        param_names,
+        param_values,
+        &params.attributeNames,
+        &params.attributeValues);
+  } else {
+    // If the params somehow contain this special attribute, remove it.
+    size_t attribute_count = params.attributeNames.size();
+    for (size_t i = 0; i < attribute_count; ++i) {
+      if (params.attributeNames[i].equals(dev_attribute))
+        params.attributeNames[i] = WebString();
+    }
+  }
+
+  return true;
+}
+
 WebPlugin* ChromeContentRendererClient::CreatePluginPlaceholder(
     content::RenderView* render_view,
     WebFrame* frame,

chrome/renderer/chrome_content_renderer_client.h

@@ -127,6 +127,14 @@ class ChromeContentRendererClient : public content::ContentRendererClient {
                                const GURL& new_url,
                                bool is_initial_navigation);
 
+  // Returns true if the NaCl plugin can be created. If it returns true, as a
+  // side effect, it may add special attributes to params.
+  bool IsNaClAllowed(const webkit::WebPluginInfo& plugin,
+                     const GURL& url,
+                     const std::string& actual_mime_type,
+                     bool enable_nacl,
+                     WebKit::WebPluginParams& params);
+
   scoped_ptr<ChromeRenderProcessObserver> chrome_observer_;
   scoped_ptr<ExtensionDispatcher> extension_dispatcher_;
   scoped_ptr<RendererHistogramSnapshots> histogram_snapshots_;

ppapi/native_client/src/shared/ppapi_proxy/browser_globals.cc

@@ -56,26 +56,6 @@ bool enable_3d_interfaces = true;
 
 }  // namespace
 
-// By default, disable developer (Dev) interfaces.  To enable developer
-// interfaces, set the environment variable NACL_ENABLE_PPAPI_DEV to 1.
-// Also, the plugin can request whether or not to enable dev interfaces.
-bool AreDevInterfacesEnabled() {
-  static bool first = true;
-  static bool env_dev_enabled = false;
-  if (first) {
-    const char *nacl_enable_ppapi_dev = getenv("NACL_ENABLE_PPAPI_DEV");
-    if (NULL != nacl_enable_ppapi_dev) {
-      int v = strtol(nacl_enable_ppapi_dev, (char **) 0, 0);
-      if (v != 0) {
-        env_dev_enabled = true;
-      }
-    }
-    first = false;
-  }
-  return env_dev_enabled || enable_dev_interfaces;
-}
-
-
 void SetBrowserPppForInstance(PP_Instance instance, BrowserPpp* browser_ppp) {
   // If there was no map, create one.
   if (instance_to_ppp_map == NULL) {
@@ -202,7 +182,7 @@ const void* GetBrowserInterface(const char* interface_name) {
     return NULL;
   }
   // If dev interface is not enabled, reject interfaces containing "(Dev)"
-  if (!AreDevInterfacesEnabled() && strstr(interface_name, "(Dev)") != NULL) {
+  if (!enable_dev_interfaces && strstr(interface_name, "(Dev)") != NULL) {
     return NULL;
   }
   if (!enable_3d_interfaces) {

ppapi/native_client/src/shared/ppapi_proxy/browser_globals.h

@@ -43,7 +43,7 @@ struct NaClSrpcChannel;
 namespace ppapi_proxy {
 
 // These functions handle the browser-side (trusted code) mapping of a browser
-// Instance to instance-specific data, such as the SRPC communication channel.
+// instance to instance-specific data, such as the SRPC communication channel.
 // These functions are called by the in-browser (trusted) plugin code, and are
 // always called from the main (foreground, UI, ...) thread. As such, they are
 // not thread-safe (they do not need to be).
@@ -52,11 +52,6 @@ namespace ppapi_proxy {
 // channel used to talk to the instance.
 class BrowserPpp;
 
-// Returns true if the PPAPI Developer interfaces are enabled.
-// To enable, set the environment variable NACL_ENABLE_PPAPI_DEV=1
-// Note: Developer interfaces are _not_ enabled by default.
-bool AreDevInterfacesEnabled();
-
 // Associate a particular BrowserPpp with a PP_Instance value.  This allows the
 // browser side to look up information it needs to communicate with the stub.
 void SetBrowserPppForInstance(PP_Instance instance,

ppapi/native_client/src/shared/ppapi_proxy/browser_nacl_file_rpc_server.cc

@@ -16,7 +16,6 @@
 #include "ppapi/c/pp_completion_callback.h"
 #include "srpcgen/ppb_rpc.h"
 
-using ppapi_proxy::AreDevInterfacesEnabled;
 using ppapi_proxy::DebugPrintf;
 using ppapi_proxy::LookupBrowserPppForInstance;
 using ppapi_proxy::MakeRemoteCompletionCallback;
@@ -63,8 +62,8 @@ void NaClFileRpcServer::GetFileDesc(
   NaClSrpcClosureRunner runner(done);
   rpc->result = NACL_SRPC_RESULT_APP_ERROR;
 
-  if (AreDevInterfacesEnabled()) {
+  plugin::Plugin* plugin = LookupBrowserPppForInstance(instance)->plugin();
-    plugin::Plugin* plugin = LookupBrowserPppForInstance(instance)->plugin();
+  if (plugin->enable_dev_interfaces()) {
     int32_t posix_file_desc = plugin->GetPOSIXFileDesc(url);
     DebugPrintf("NaClFile::GetFileDesc: posix_file_desc=%"NACL_PRId32"\n",
                 posix_file_desc);

ppapi/native_client/src/trusted/plugin/plugin.cc

@@ -103,6 +103,10 @@ const char* const kNaClManifestAttribute = "nacl";
 // for the null termination character.
 const size_t kNaClManifestMaxFileBytes = 1024 * 1024;
 
+// Define an argument name to enable 'dev' interfaces. To make sure it doesn't
+// collide with any user-defined HTML attribute, make the first character '@'.
+const char* const kDevAttribute = "@dev";
+
 // URL schemes that we treat in special ways.
 const char* const kChromeExtensionUriScheme = "chrome-extension";
 const char* const kDataUriScheme = "data";
@@ -892,6 +896,11 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
       const_cast<char**>(argn),
       const_cast<char**>(argv));
   if (status) {
+    // Look for the developer attribute; if it's present, enable 'dev'
+    // interfaces.
+    const char* dev_settings = LookupArgument(kDevAttribute);
+    enable_dev_interfaces_ = (dev_settings != NULL);
+
     const char* type_attr = LookupArgument(kTypeAttribute);
     if (type_attr != NULL) {
       mime_type_ = nacl::string(type_attr);
@@ -904,7 +913,6 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
     // and 'nacl' will be the URL for the manifest.
     if (IsForeignMIMEType()) {
       manifest_url = LookupArgument(kNaClManifestAttribute);
-      enable_dev_interfaces_ = RequiresDevInterfaces(manifest_url);
     }
     // Use the document URL as the base for resolving relative URLs to find the
     // manifest.  This takes into account the setting of <base> tags that
@@ -1300,21 +1308,6 @@ void Plugin::BitcodeDidTranslateContinuation(int32_t pp_error) {
   }
 }
 
-// Check manifest_url and return whether or not to enable PPAPI Dev interfaces.
-// Returning true here will enable the PPAPI Dev interfaces regardless of
-// the environment variable NACL_ENABLE_PPAPI_DEV.
-bool Plugin::RequiresDevInterfaces(const nacl::string& manifest_url) {
-  const char* extensions[] = {
-      "chrome-extension://acadkphlmlegjaadjagenfimbpphcgnh/",  // PDF
-  };
-  for (size_t i = 0; i < sizeof(extensions) / sizeof(const char*); ++i) {
-    if (manifest_url.find(extensions[i]) == 0) {
-      return true;
-    }
-  }
-  return false;
-}
-
 bool Plugin::StartProxiedExecution(NaClSrpcChannel* srpc_channel,
                                    ErrorInfo* error_info) {
   PLUGIN_PRINTF(("Plugin::StartProxiedExecution (srpc_channel=%p)\n",

ppapi/native_client/src/trusted/plugin/plugin.h

@@ -374,9 +374,6 @@ class Plugin : public pp::InstancePrivate {
 
   MethodInfo* GetMethodInfo(uintptr_t method_id, CallType call_type);
 
-  // Check url and decide if PPAPI Dev interfaces are required.
-  bool RequiresDevInterfaces(const nacl::string& manifest_url);
-
   // Callback used when getting the URL for the .nexe file.  If the URL loading
   // is successful, the file descriptor is opened and can be passed to sel_ldr
   // with the sandbox on.

ppapi/native_client/tests/ppapi_browser/ppb_dev/ppapi_ppb_dev.cc

@@ -16,31 +16,7 @@
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_core.h"
 
-#include "ppapi/c/dev/ppb_buffer_dev.h"
-#include "ppapi/c/dev/ppb_char_set_dev.h"
-#include "ppapi/c/dev/ppb_console_dev.h"
-#include "ppapi/c/dev/ppb_context_3d_dev.h"
-#include "ppapi/c/dev/ppb_context_3d_trusted_dev.h"
-#include "ppapi/c/dev/ppb_crypto_dev.h"
 #include "ppapi/c/dev/ppb_cursor_control_dev.h"
-#include "ppapi/c/dev/ppb_directory_reader_dev.h"
-#include "ppapi/c/dev/ppb_file_chooser_dev.h"
-#include "ppapi/c/dev/ppb_find_dev.h"
-#include "ppapi/c/dev/ppb_font_dev.h"
-#include "ppapi/c/dev/ppb_fullscreen_dev.h"
-#include "ppapi/c/dev/ppb_gles_chromium_texture_mapping_dev.h"
-#include "ppapi/c/dev/ppb_layer_compositor_dev.h"
-#include "ppapi/c/dev/ppb_memory_dev.h"
-#include "ppapi/c/dev/ppb_scrollbar_dev.h"
-#include "ppapi/c/dev/ppb_surface_3d_dev.h"
-#include "ppapi/c/dev/ppb_testing_dev.h"
-#include "ppapi/c/dev/ppb_transport_dev.h"
-#include "ppapi/c/dev/ppb_url_util_dev.h"
-#include "ppapi/c/dev/ppb_var_deprecated.h"
-#include "ppapi/c/dev/ppb_video_decoder_dev.h"
-#include "ppapi/c/dev/ppb_widget_dev.h"
-#include "ppapi/c/dev/ppb_zoom_dev.h"
-
 
 namespace {
 
@@ -49,33 +25,10 @@ namespace {
 ////////////////////////////////////////////////////////////////////////////////
 
 void TestGetDevInterfaces() {
-  // This test is run only w/ NACL_ENABLE_PPAPI_DEV=0, which should
+  // This test is run as a simple embedded .nexe with --enable-nacl. It should
-  // turn off and disable the PPAPI developer interfaces. When they are
+  // have access to all dev interfaces. Only test one to make the test more
-  // disabled, the interface should return NULL.
+  // robust as interfaces change.
-  CHECK(GetBrowserInterface(PPB_BUFFER_DEV_INTERFACE) == NULL);
+  EXPECT(GetBrowserInterface(PPB_CURSOR_CONTROL_DEV_INTERFACE) != NULL);
-  CHECK(GetBrowserInterface(PPB_CHAR_SET_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_CONSOLE_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_CONTEXT_3D_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_CRYPTO_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_CURSOR_CONTROL_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_DIRECTORYREADER_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_FILECHOOSER_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_FIND_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_FONT_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_FULLSCREEN_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(
-      PPB_GLES_CHROMIUM_TEXTURE_MAPPING_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_LAYER_COMPOSITOR_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_MEMORY_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_SCROLLBAR_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_SURFACE_3D_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_TESTING_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_TRANSPORT_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_URLUTIL_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_VAR_DEPRECATED_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_VIDEODECODER_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_WIDGET_DEV_INTERFACE) == NULL);
-  CHECK(GetBrowserInterface(PPB_ZOOM_DEV_INTERFACE) == NULL);
 
   TEST_PASSED;
 }

ppapi/native_client/tests/ppapi_browser/ppb_dev/ppapi_ppb_dev.html

@@ -31,9 +31,7 @@ function setupTests(tester, plugin) {
   </head>
   <body>
     <h1>PPAPI Dev Interface Test</h1>
-  NOTE: requires --enable-pepper-testing.  Also, this test should be run with
+  NOTE: requires --enable-pepper-testing.
-  PPAPI Dev interfaces disabled; the environment variable NACL_ENABLE_PPAPI_DEV
-  should be set to 0 or unset.
   <br><br>
   <table>
   <tr>