Add a whitelist check for nacl-nonsfi mode

Also add an explicit check for whitelisting pepper/nacl
permissions based on importing a shared module, so a shared
module can potentially limit allowing import of its resources
and permissions its allowed.

You can still use nonsfi mode for Linux or ChromeOS by
passing in --enable-nacl-nonsfi-mode, but it is on for a
whitelisted set of extensions for ChromeOS without flags.

BUG=355141
TEST=deploy_chrome to daisy, test app which includes whitelisted module

Review URL: https://codereview.chromium.org/264923011

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

chrome/browser/nacl_host/nacl_browser_delegate_impl.cc

@@ -18,6 +18,7 @@
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/logging_chrome.h"
+#include "chrome/common/pepper_permission_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/site_instance.h"
@@ -34,6 +35,13 @@ using extensions::SharedModuleInfo;
 
 namespace {
 
+// These are temporarily needed for testing non-sfi mode on ChromeOS without
+// passing command-line arguments to Chrome.
+const char* const kAllowedNonSfiOrigins[] = {
+    "6EAED1924DB611B6EEF2A664BD077BE7EAD33B8F",  // see http://crbug.com/355141
+    "4EB74897CB187C7633357C2FE832E0AD6A44883A"   // see http://crbug.com/355141
+};
+
 // Handles an extension's NaCl process transitioning in or out of idle state by
 // relaying the state to the extension's process manager.
 //
@@ -108,6 +116,9 @@ NaClBrowserDelegateImpl::NaClBrowserDelegateImpl(
     ProfileManager* profile_manager)
     : profile_manager_(profile_manager), inverse_debug_patterns_(false) {
   DCHECK(profile_manager_);
+  for (size_t i = 0; i < arraysize(kAllowedNonSfiOrigins); ++i) {
+    allowed_nonsfi_origins_.insert(kAllowedNonSfiOrigins[i]);
+  }
 }
 
 NaClBrowserDelegateImpl::~NaClBrowserDelegateImpl() {
@@ -210,15 +221,8 @@ bool NaClBrowserDelegateImpl::MapUrlToLocalFilePath(
     bool use_blocking_api,
     const base::FilePath& profile_directory,
     base::FilePath* file_path) {
-  // Get the profile associated with the renderer.
-  Profile* profile = profile_manager_->GetProfileByPath(profile_directory);
-  if (!profile)
-    return false;
-
   scoped_refptr<extensions::InfoMap> extension_info_map =
-      extensions::ExtensionSystem::Get(profile)->info_map();
-  DCHECK(extension_info_map);
-
+      GetExtensionInfoMap(profile_directory);
   // Check that the URL is recognized by the extension system.
   const extensions::Extension* extension =
       extension_info_map->extensions().GetExtensionOrAppByURL(file_url);
@@ -279,3 +283,23 @@ content::BrowserPpapiHost::OnKeepaliveCallback
 NaClBrowserDelegateImpl::GetOnKeepaliveCallback() {
   return base::Bind(&OnKeepalive);
 }
+
+bool NaClBrowserDelegateImpl::IsNonSfiModeAllowed(
+    const base::FilePath& profile_directory,
+    const GURL& manifest_url) {
+  const extensions::ExtensionSet* extension_set =
+      &GetExtensionInfoMap(profile_directory)->extensions();
+  return chrome::IsExtensionOrSharedModuleWhitelisted(
+      manifest_url, extension_set, allowed_nonsfi_origins_);
+}
+
+scoped_refptr<extensions::InfoMap> NaClBrowserDelegateImpl::GetExtensionInfoMap(
+    const base::FilePath& profile_directory) {
+  // Get the profile associated with the renderer.
+  Profile* profile = profile_manager_->GetProfileByPath(profile_directory);
+  DCHECK(profile);
+  scoped_refptr<extensions::InfoMap> extension_info_map =
+      extensions::ExtensionSystem::Get(profile)->info_map();
+  DCHECK(extension_info_map);
+  return extension_info_map;
+}

chrome/browser/nacl_host/nacl_browser_delegate_impl.h

@@ -5,10 +5,17 @@
 #ifndef CHROME_BROWSER_NACL_HOST_NACL_BROWSER_DELEGATE_IMPL_H_
 #define CHROME_BROWSER_NACL_HOST_NACL_BROWSER_DELEGATE_IMPL_H_
 
+#include <set>
+#include <string>
+
 #include "base/compiler_specific.h"
 #include "components/nacl/browser/nacl_browser_delegate.h"
 #include "extensions/common/url_pattern.h"
 
+namespace extensions {
+class InfoMap;
+}
+
 class ProfileManager;
 
 class NaClBrowserDelegateImpl : public NaClBrowserDelegate {
@@ -34,11 +41,16 @@ class NaClBrowserDelegateImpl : public NaClBrowserDelegate {
   virtual bool URLMatchesDebugPatterns(const GURL& manifest_url) OVERRIDE;
   virtual content::BrowserPpapiHost::OnKeepaliveCallback
       GetOnKeepaliveCallback() OVERRIDE;
+  virtual bool IsNonSfiModeAllowed(const base::FilePath& profile_directory,
+                                   const GURL& manifest_url) OVERRIDE;
 
  private:
+  scoped_refptr<extensions::InfoMap> GetExtensionInfoMap(
+      const base::FilePath& profile_directory);
   ProfileManager* profile_manager_;
   std::vector<URLPattern> debug_patterns_;
   bool inverse_debug_patterns_;
+  std::set<std::string> allowed_nonsfi_origins_;
   DISALLOW_COPY_AND_ASSIGN(NaClBrowserDelegateImpl);
 };
 

chrome/chrome_tests_unit.gypi

@@ -1872,6 +1872,7 @@
         'common/net/url_fixer_upper_unittest.cc',
         'common/net/x509_certificate_model_unittest.cc',
         'common/partial_circular_buffer_unittest.cc',
+        'common/pepper_permission_util_unittest.cc',
         'common/pref_names_util_unittest.cc',
         'common/search_urls_unittest.cc',
         'common/service_process_util_unittest.cc',

chrome/common/pepper_permission_util.cc

@@ -17,6 +17,7 @@
 
 using extensions::Extension;
 using extensions::Manifest;
+using extensions::SharedModuleInfo;
 
 namespace chrome {
 
@@ -49,20 +50,25 @@ bool IsExtensionOrSharedModuleWhitelisted(
   // is whitelisted.
   const Extension* extension = extension_set ? extension_set->GetByID(host)
                                              : NULL;
-  if (extension) {
-    typedef std::vector<extensions::SharedModuleInfo::ImportInfo>
-        ImportInfoVector;
-    const ImportInfoVector& imports =
-        extensions::SharedModuleInfo::GetImports(extension);
-    for (ImportInfoVector::const_iterator it = imports.begin();
-         it != imports.end(); ++it) {
-      const Extension* imported_extension = extension_set->GetByID(
-          it->extension_id);
-      if (imported_extension &&
-          extensions::SharedModuleInfo::IsSharedModule(imported_extension) &&
-          HostIsInSet(it->extension_id, whitelist)) {
-        return true;
-      }
+  if (!extension)
+    return false;
+
+  typedef std::vector<SharedModuleInfo::ImportInfo> ImportInfoVector;
+  const ImportInfoVector& imports = SharedModuleInfo::GetImports(extension);
+  for (ImportInfoVector::const_iterator it = imports.begin();
+       it != imports.end();
+       ++it) {
+    const Extension* imported_extension =
+        extension_set->GetByID(it->extension_id);
+    if (imported_extension &&
+        SharedModuleInfo::IsSharedModule(imported_extension) &&
+        // We check the whitelist explicitly even though the extension should
+        // never have been allowed to be installed in the first place if this
+        // fails.  See SharedModuleService::CheckImports for details.
+        SharedModuleInfo::IsExportAllowedByWhitelist(imported_extension,
+                                                     host) &&
+        HostIsInSet(it->extension_id, whitelist)) {
+      return true;
     }
   }
 

chrome/common/pepper_permission_util.h

@@ -17,6 +17,11 @@ class ExtensionSet;
 namespace chrome {
 
 // Returns true if the extension (or an imported module if any) is whitelisted.
+// Module imports are at most one level deep (ie, a module that exports cannot
+// import another extension).  The extension is identified by the host of |url|
+// (if it is a chrome-extension URL).  |extension_set| is the list of installed
+// and enabled extensions for a given profile.  |whitelist| is a set of
+// (possibly hashed) extension IDs to check against.
 bool IsExtensionOrSharedModuleWhitelisted(
     const GURL& url,
     const extensions::ExtensionSet* extension_set,

chrome/common/pepper_permission_util_unittest.cc

+// Copyright 2014 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 "chrome/common/pepper_permission_util.h"
+
+#include <set>
+#include <string>
+
+#include "chrome/common/extensions/features/feature_channel.h"
+#include "extensions/common/extension_builder.h"
+#include "extensions/common/extension_set.h"
+#include "extensions/common/id_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using chrome::IsExtensionOrSharedModuleWhitelisted;
+
+namespace extensions {
+
+namespace {
+
+// Return an extension with |id| which imports a module with the given
+// |import_id|.
+scoped_refptr<Extension> CreateExtensionImportingModule(
+    const std::string& import_id,
+    const std::string& id) {
+  scoped_ptr<base::DictionaryValue> manifest =
+      DictionaryBuilder()
+          .Set("name", "Has Dependent Modules")
+          .Set("version", "1.0")
+          .Set("manifest_version", 2)
+          .Set("import",
+               ListBuilder().Append(DictionaryBuilder().Set("id", import_id)))
+          .Build();
+
+  return ExtensionBuilder()
+      .SetManifest(manifest.Pass())
+      .AddFlags(Extension::FROM_WEBSTORE)
+      .SetID(id)
+      .Build();
+}
+
+}  // namespace
+
+TEST(PepperPermissionUtilTest, ExtensionWhitelisting) {
+  ScopedCurrentChannel current_channel(chrome::VersionInfo::CHANNEL_UNKNOWN);
+  ExtensionSet extensions;
+  std::string whitelisted_id = id_util::GenerateId("whitelisted_extension");
+  scoped_ptr<base::DictionaryValue> manifest =
+      DictionaryBuilder()
+          .Set("name", "Whitelisted Extension")
+          .Set("version", "1.0")
+          .Set("manifest_version", 2)
+          .Build();
+  scoped_refptr<Extension> ext = ExtensionBuilder()
+                                     .SetManifest(manifest.Pass())
+                                     .SetID(whitelisted_id)
+                                     .Build();
+  extensions.Insert(ext);
+  std::set<std::string> whitelist;
+  std::string url = std::string("chrome-extension://") + whitelisted_id +
+                    std::string("/manifest.nmf");
+  std::string bad_scheme_url =
+      std::string("http://") + whitelisted_id + std::string("/manifest.nmf");
+  std::string bad_host_url =
+      std::string("chrome-extension://") + id_util::GenerateId("bad_host");
+  std::string("/manifest.nmf");
+
+  EXPECT_FALSE(
+      IsExtensionOrSharedModuleWhitelisted(GURL(url), &extensions, whitelist));
+  whitelist.insert(whitelisted_id);
+  EXPECT_TRUE(
+      IsExtensionOrSharedModuleWhitelisted(GURL(url), &extensions, whitelist));
+  EXPECT_FALSE(IsExtensionOrSharedModuleWhitelisted(
+      GURL(bad_scheme_url), &extensions, whitelist));
+  EXPECT_FALSE(IsExtensionOrSharedModuleWhitelisted(
+      GURL(bad_host_url), &extensions, whitelist));
+}
+
+TEST(PepperPermissionUtilTest, SharedModuleWhitelisting) {
+  ScopedCurrentChannel current_channel(chrome::VersionInfo::CHANNEL_UNKNOWN);
+  ExtensionSet extensions;
+  std::string whitelisted_id = id_util::GenerateId("extension_id");
+  std::string bad_id = id_util::GenerateId("bad_id");
+
+  scoped_ptr<base::DictionaryValue> shared_module_manifest =
+      DictionaryBuilder()
+          .Set("name", "Whitelisted Shared Module")
+          .Set("version", "1.0")
+          .Set("manifest_version", 2)
+          .Set("export",
+               DictionaryBuilder()
+                   .Set("resources", ListBuilder().Append("*"))
+                   // Add the extension to the whitelist.  This
+                   // restricts import to |whitelisted_id| only.
+                   .Set("whitelist", ListBuilder().Append(whitelisted_id)))
+          .Build();
+  scoped_refptr<Extension> shared_module =
+      ExtensionBuilder().SetManifest(shared_module_manifest.Pass()).Build();
+
+  scoped_refptr<Extension> ext =
+      CreateExtensionImportingModule(shared_module->id(), whitelisted_id);
+  std::string extension_url =
+      std::string("chrome-extension://") + ext->id() + std::string("/foo.html");
+
+  std::set<std::string> whitelist;
+  // Important: whitelist *only* the shared module.
+  whitelist.insert(shared_module->id());
+
+  extensions.Insert(ext);
+  // This should fail because shared_module is not in the set of extensions.
+  EXPECT_FALSE(IsExtensionOrSharedModuleWhitelisted(
+      GURL(extension_url), &extensions, whitelist));
+  extensions.Insert(shared_module);
+  EXPECT_TRUE(IsExtensionOrSharedModuleWhitelisted(
+      GURL(extension_url), &extensions, whitelist));
+  scoped_refptr<Extension> bad_ext =
+      CreateExtensionImportingModule(shared_module->id(), bad_id);
+  std::string bad_extension_url = std::string("chrome-extension://") +
+                                  bad_ext->id() + std::string("/foo.html");
+
+  extensions.Insert(bad_ext);
+  // This should fail because bad_ext is not whitelisted to use shared_module.
+  EXPECT_FALSE(IsExtensionOrSharedModuleWhitelisted(
+      GURL(bad_extension_url), &extensions, whitelist));
+
+  // Note that the whitelist should be empty after this call, so tests checking
+  // for failure to import will fail because of this.
+  whitelist.erase(shared_module->id());
+  EXPECT_FALSE(IsExtensionOrSharedModuleWhitelisted(
+      GURL(extension_url), &extensions, whitelist));
+}
+
+}  // namespace extensions

components/nacl/browser/nacl_browser_delegate.h

@@ -76,6 +76,10 @@ class NaClBrowserDelegate {
   // Returns a callback that handles NaCl idle state transitions.
   virtual content::BrowserPpapiHost::OnKeepaliveCallback
       GetOnKeepaliveCallback() = 0;
+
+  // Returns whether Non-SFI mode is allowed for a given manifest URL.
+  virtual bool IsNonSfiModeAllowed(const base::FilePath& profile_directory,
+                                   const GURL& manifest_url) = 0;
 };
 
 #endif  // COMPONENTS_NACL_BROWSER_NACL_BROWSER_DELEGATE_H_

components/nacl/browser/nacl_process_host.cc

@@ -27,6 +27,7 @@
 #include "base/win/windows_version.h"
 #include "build/build_config.h"
 #include "components/nacl/browser/nacl_browser.h"
+#include "components/nacl/browser/nacl_browser_delegate.h"
 #include "components/nacl/browser/nacl_host_message_filter.h"
 #include "components/nacl/common/nacl_cmd_line.h"
 #include "components/nacl/common/nacl_host_messages.h"
@@ -430,15 +431,23 @@ void NaClProcessHost::Launch(
   }
 
   if (uses_nonsfi_mode_) {
+    bool nonsfi_mode_forced_by_command_line = false;
+    bool nonsfi_mode_allowed = false;
 #if defined(OS_LINUX)
-    const bool kNonSFIModeSupported = true;
-#else
-    const bool kNonSFIModeSupported = false;
+    nonsfi_mode_forced_by_command_line =
+        cmd->HasSwitch(switches::kEnableNaClNonSfiMode);
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
+    nonsfi_mode_allowed = NaClBrowser::GetDelegate()->IsNonSfiModeAllowed(
+        nacl_host_message_filter->profile_directory(), manifest_url_);
+#endif
 #endif
-    if (!kNonSFIModeSupported ||
-        !cmd->HasSwitch(switches::kEnableNaClNonSfiMode)) {
-      SendErrorToRenderer("NaCl non-SFI mode works only on Linux with"
-                          " --enable-nacl-nonsfi-mode specified");
+    bool nonsfi_mode_enabled =
+        nonsfi_mode_forced_by_command_line || nonsfi_mode_allowed;
+
+    if (!nonsfi_mode_enabled) {
+      SendErrorToRenderer(
+          "NaCl non-SFI mode is not available for this platform"
+          " and NaCl module.");
       delete this;
       return;
     }

components/nacl/browser/test_nacl_browser_delegate.cc

@@ -59,3 +59,9 @@ content::BrowserPpapiHost::OnKeepaliveCallback
 TestNaClBrowserDelegate::GetOnKeepaliveCallback() {
   return content::BrowserPpapiHost::OnKeepaliveCallback();
 }
+
+bool TestNaClBrowserDelegate::IsNonSfiModeAllowed(
+    const base::FilePath& profile_directory,
+    const GURL& manifest_url) {
+  return false;
+}

components/nacl/browser/test_nacl_browser_delegate.h

@@ -40,6 +40,8 @@ class TestNaClBrowserDelegate : public NaClBrowserDelegate {
   virtual bool URLMatchesDebugPatterns(const GURL& manifest_url) OVERRIDE;
   virtual content::BrowserPpapiHost::OnKeepaliveCallback
       GetOnKeepaliveCallback() OVERRIDE;
+  virtual bool IsNonSfiModeAllowed(const base::FilePath& profile_directory,
+                                   const GURL& manifest_url) OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestNaClBrowserDelegate);

components/nacl/renderer/ppb_nacl_private_impl.cc

@@ -524,7 +524,13 @@ int32_t GetNumberOfProcessors() {
 }
 
 PP_Bool IsNonSFIModeEnabled() {
-#if defined(OS_LINUX)
+// Note that this only indicates whether non-sfi mode *can* run for a given
+// platform and if nonsfi manifest entries are preferred.  There can be other
+// restrictions which prevent a particular module from launching.  See
+// NaClProcessHost::Launch which makes the final determination.
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
+  return PP_TRUE;
+#elif defined(OS_LINUX)
   return PP_FromBool(CommandLine::ForCurrentProcess()->HasSwitch(
                          switches::kEnableNaClNonSfiMode));
 #else

extensions/common/manifest_handlers/shared_module_info.cc

@@ -86,6 +86,11 @@ bool SharedModuleInfo::IsExportAllowed(const Extension* extension,
 // static
 bool SharedModuleInfo::IsExportAllowedByWhitelist(const Extension* extension,
                                                   const std::string& other_id) {
+  // Sanity check. In case the caller did not check |extension| to make sure it
+  // is a shared module, we do not want it to appear that the extension with
+  // |other_id| importing |extension| is valid.
+  if (!SharedModuleInfo::IsSharedModule(extension))
+    return false;
   const SharedModuleInfo& info = GetSharedModuleInfo(extension);
   if (info.export_whitelist_.empty())
     return true;