diff options
author | skerner@google.com <skerner@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-22 14:47:18 +0000 |
---|---|---|
committer | skerner@google.com <skerner@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-22 14:47:18 +0000 |
commit | 73e4c36f363107b3b625001ebf524ca247c5220c (patch) | |
tree | aeafb62d03121d1881aa36180d7dcab6e047a990 /chrome | |
parent | e4d9c1fb00637f609fb81e00c8e5be5d3c1c8230 (diff) | |
download | chromium_src-73e4c36f363107b3b625001ebf524ca247c5220c.zip chromium_src-73e4c36f363107b3b625001ebf524ca247c5220c.tar.gz chromium_src-73e4c36f363107b3b625001ebf524ca247c5220c.tar.bz2 |
Add external extensions json source in proper mac location.
The old path will be deprecated once developers have migrated.
BUG=67203
TEST=FileUtilTest.IsPathControledByAdmin
Review URL: http://codereview.chromium.org/7718021
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102274 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/extensions/external_extension_provider_impl.cc | 26 | ||||
-rw-r--r-- | chrome/browser/extensions/external_pref_extension_loader.cc | 76 | ||||
-rw-r--r-- | chrome/browser/extensions/external_pref_extension_loader.h | 16 | ||||
-rw-r--r-- | chrome/common/chrome_paths.cc | 29 | ||||
-rw-r--r-- | chrome/common/chrome_paths.h | 7 | ||||
-rw-r--r-- | chrome/common/chrome_paths_internal.h | 3 | ||||
-rw-r--r-- | chrome/common/chrome_paths_mac.mm | 4 |
7 files changed, 135 insertions, 26 deletions
diff --git a/chrome/browser/extensions/external_extension_provider_impl.cc b/chrome/browser/extensions/external_extension_provider_impl.cc index f288af0..2a61c58 100644 --- a/chrome/browser/extensions/external_extension_provider_impl.cc +++ b/chrome/browser/extensions/external_extension_provider_impl.cc @@ -222,14 +222,38 @@ void ExternalExtensionProviderImpl::CreateExternalProviders( VisitorInterface* service, Profile* profile, ProviderCollection* provider_list) { + + // On Mac OS, items in /Library/... should be written by the superuser. + // Check that all components of the path are writable by root only. + ExternalPrefExtensionLoader::Options options; +#if defined(OS_MACOSX) + options = ExternalPrefExtensionLoader::ENSURE_PATH_CONTROLLED_BY_ADMIN; +#else + options = ExternalPrefExtensionLoader::NONE; +#endif + + provider_list->push_back( + linked_ptr<ExternalExtensionProviderInterface>( + new ExternalExtensionProviderImpl( + service, + new ExternalPrefExtensionLoader( + chrome::DIR_EXTERNAL_EXTENSIONS, options), + Extension::EXTERNAL_PREF, + Extension::EXTERNAL_PREF_DOWNLOAD))); + +#if defined(OS_MACOSX) + // Support old path to external extensions file as we migrate to the + // new one. See crbug/67203. provider_list->push_back( linked_ptr<ExternalExtensionProviderInterface>( new ExternalExtensionProviderImpl( service, new ExternalPrefExtensionLoader( - chrome::DIR_EXTERNAL_EXTENSIONS), + chrome::DIR_DEPRICATED_EXTERNAL_EXTENSIONS, + ExternalPrefExtensionLoader::NONE), Extension::EXTERNAL_PREF, Extension::EXTERNAL_PREF_DOWNLOAD))); +#endif #if defined(OS_CHROMEOS) // Chrome OS specific source for OEM customization. diff --git a/chrome/browser/extensions/external_pref_extension_loader.cc b/chrome/browser/extensions/external_pref_extension_loader.cc index 8d90ee7..29739d0 100644 --- a/chrome/browser/extensions/external_pref_extension_loader.cc +++ b/chrome/browser/extensions/external_pref_extension_loader.cc @@ -7,7 +7,9 @@ #include "base/file_path.h" #include "base/file_util.h" #include "base/logging.h" +#include "base/metrics/histogram.h" #include "base/path_service.h" +#include "chrome/common/chrome_paths.h" #include "content/browser/browser_thread.h" #include "content/common/json_value_serializer.h" @@ -34,8 +36,10 @@ DictionaryValue* ExtractPrefs(const FilePath& path, } // namespace -ExternalPrefExtensionLoader::ExternalPrefExtensionLoader(int base_path_key) - : base_path_key_(base_path_key) { +ExternalPrefExtensionLoader::ExternalPrefExtensionLoader(int base_path_key, + Options options) + : base_path_key_(base_path_key), + options_(options){ CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } @@ -55,32 +59,72 @@ void ExternalPrefExtensionLoader::StartLoading() { &ExternalPrefExtensionLoader::LoadOnFileThread)); } -void ExternalPrefExtensionLoader::LoadOnFileThread() { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - +DictionaryValue* ExternalPrefExtensionLoader::ReadJsonPrefsFile() { // TODO(skerner): Some values of base_path_key_ will cause // PathService::Get() to return false, because the path does // not exist. Find and fix the build/install scripts so that // this can become a CHECK(). Known examples include chrome // OS developer builds and linux install packages. // Tracked as crbug.com/70402 . + if (!PathService::Get(base_path_key_, &base_path_)) { + return NULL; + } + + FilePath json_file = base_path_.Append( + FILE_PATH_LITERAL("external_extensions.json")); - scoped_ptr<DictionaryValue> prefs; - if (PathService::Get(base_path_key_, &base_path_)) { - FilePath json_file; - json_file = - base_path_.Append(FILE_PATH_LITERAL("external_extensions.json")); + if (!file_util::PathExists(json_file)) { + // This is not an error. The file does not exist by default. + return NULL; + } - if (file_util::PathExists(json_file)) { - JSONFileValueSerializer serializer(json_file); - prefs.reset(ExtractPrefs(json_file, &serializer)); + if (IsOptionSet(ENSURE_PATH_CONTROLLED_BY_ADMIN)) { +#if defined(OS_MACOSX) + if (!file_util::VerifyPathControlledByAdmin(json_file)) { + LOG(ERROR) << "Can not read external extensions source. The file " + << json_file.value() << " and every directory in its path, " + << "must be owned by root, have group \"admin\", and not be " + << "writable by all users. These restrictions prevent " + << "unprivleged users from making chrome install extensions " + << "on other users' accounts."; + return NULL; } +#else + // The only platform that uses this check is Mac OS. If you add one, + // you need to implement file_util::VerifyPathControlledByAdmin() for + // that platform. + NOTREACHED(); +#endif // defined(OS_MACOSX) } - if (!prefs.get()) - prefs.reset(new DictionaryValue()); + JSONFileValueSerializer serializer(json_file); + return ExtractPrefs(json_file, &serializer); +} + +void ExternalPrefExtensionLoader::LoadOnFileThread() { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - prefs_.reset(prefs.release()); + prefs_.reset(ReadJsonPrefsFile()); + if (!prefs_.get()) + prefs_.reset(new DictionaryValue()); + + // We want to deprecate the external extensions file inside the app + // bundle on mac os. Use a histogram to see how many extensions + // are installed using the deprecated path, and how many are installed + // from the supported path. We can use this data to measure the + // effectiveness of asking developers to use the new path, or any + // automatic migration methods we implement. +#if defined(OS_MACOSX) + // The deprecated path only exists on mac for now. + if (base_path_key_ == chrome::DIR_DEPRICATED_EXTERNAL_EXTENSIONS) { + UMA_HISTOGRAM_COUNTS_100("Extensions.DepricatedExternalJsonCount", + prefs_->size()); + } +#endif // defined(OS_MACOSX) + if (base_path_key_ == chrome::DIR_EXTERNAL_EXTENSIONS) { + UMA_HISTOGRAM_COUNTS_100("Extensions.ExternalJsonCount", + prefs_->size()); + } // If we have any records to process, then we must have // read the .json file. If we read the .json file, then diff --git a/chrome/browser/extensions/external_pref_extension_loader.h b/chrome/browser/extensions/external_pref_extension_loader.h index 8473635..4d48755 100644 --- a/chrome/browser/extensions/external_pref_extension_loader.h +++ b/chrome/browser/extensions/external_pref_extension_loader.h @@ -20,24 +20,38 @@ // thread and they are expecting public method calls from the UI thread. class ExternalPrefExtensionLoader : public ExternalExtensionLoader { public: + enum Options { + NONE = 0, + + // Ensure that only root can force an external install by checking + // that all components of the path to external extensions files are + // owned by root and not writable by any non-root user. + ENSURE_PATH_CONTROLLED_BY_ADMIN = 1 << 0 + }; + // |base_path_key| is the directory containing the external_extensions.json // file. Relative file paths to extension files are resolved relative // to this path. - explicit ExternalPrefExtensionLoader(int base_path_key); + explicit ExternalPrefExtensionLoader(int base_path_key, Options options); virtual const FilePath GetBaseCrxFilePath() OVERRIDE; protected: virtual void StartLoading() OVERRIDE; + bool IsOptionSet(Options option) { + return (options_ & option) != 0; + } private: friend class base::RefCountedThreadSafe<ExternalExtensionLoader>; virtual ~ExternalPrefExtensionLoader() {} + DictionaryValue* ReadJsonPrefsFile(); void LoadOnFileThread(); int base_path_key_; + Options options_; FilePath base_path_; DISALLOW_COPY_AND_ASSIGN(ExternalPrefExtensionLoader); diff --git a/chrome/common/chrome_paths.cc b/chrome/common/chrome_paths.cc index db1f2b5..3b3260f 100644 --- a/chrome/common/chrome_paths.cc +++ b/chrome/common/chrome_paths.cc @@ -334,16 +334,12 @@ bool PathProvider(int key, FilePath* result) { #endif case chrome::DIR_EXTERNAL_EXTENSIONS: #if defined(OS_MACOSX) - if (!PathService::Get(base::DIR_EXE, &cur)) + if (!chrome::GetGlobalApplicationSupportDirectory(&cur)) return false; - // On Mac, built-in extensions are in Contents/Extensions, a sibling of - // the App dir. If there are none, it may not exist. - // TODO(skerner): Reading external extensions from a file inside the - // app budle causes several problems. Change this path to be outside - // the app bundle. crbug/67203 - cur = cur.DirName(); - cur = cur.Append(FILE_PATH_LITERAL("Extensions")); + cur = cur.Append(FILE_PATH_LITERAL("Google")) + .Append(FILE_PATH_LITERAL("Chrome")) + .Append(FILE_PATH_LITERAL("External Extensions")); create_dir = false; #else if (!PathService::Get(base::DIR_MODULE, &cur)) @@ -353,6 +349,22 @@ bool PathProvider(int key, FilePath* result) { create_dir = true; #endif break; + +#if defined(OS_MACOSX) + case DIR_DEPRICATED_EXTERNAL_EXTENSIONS: + // TODO(skerner): Reading external extensions from a file inside the + // app budle causes several problems. Once users have a chance to + // migrate, remove this path. crbug/67203 + if (!PathService::Get(base::DIR_EXE, &cur)) + return false; + + cur = cur.DirName(); + cur = cur.Append(FILE_PATH_LITERAL("Extensions")); + create_dir = false; + + break; +#endif + case chrome::DIR_DEFAULT_APPS: #if defined(OS_MACOSX) cur = base::mac::MainAppBundlePath(); @@ -363,6 +375,7 @@ bool PathProvider(int key, FilePath* result) { cur = cur.Append(FILE_PATH_LITERAL("default_apps")); #endif break; + default: return false; } diff --git a/chrome/common/chrome_paths.h b/chrome/common/chrome_paths.h index 78ca6b1..2462bf6 100644 --- a/chrome/common/chrome_paths.h +++ b/chrome/common/chrome_paths.h @@ -57,6 +57,13 @@ enum { #endif DIR_EXTERNAL_EXTENSIONS, // Directory where installer places .crx files. + +#if defined(OS_MACOSX) + DIR_DEPRICATED_EXTERNAL_EXTENSIONS, // Former home of external extensions. + // We read from the old path for now, + // to give users time to migrate. +#endif + DIR_DEFAULT_APPS, // Directory where installer places .crx files // to be installed when chrome is first run. FILE_RESOURCE_MODULE, // Full path and filename of the module that diff --git a/chrome/common/chrome_paths_internal.h b/chrome/common/chrome_paths_internal.h index baf5777..23c3ec4 100644 --- a/chrome/common/chrome_paths_internal.h +++ b/chrome/common/chrome_paths_internal.h @@ -76,6 +76,9 @@ FilePath GetFrameworkBundlePath(); // Get the local library directory. bool GetLocalLibraryDirectory(FilePath* result); +// Get the global Application Support directory (under /Library/). +bool GetGlobalApplicationSupportDirectory(FilePath* result); + // Returns the NSBundle for the outer browser application, even when running // inside the helper. In unbundled applications, such as tests, returns nil. NSBundle* OuterAppBundle(); diff --git a/chrome/common/chrome_paths_mac.mm b/chrome/common/chrome_paths_mac.mm index 7331d78..9df7cd5 100644 --- a/chrome/common/chrome_paths_mac.mm +++ b/chrome/common/chrome_paths_mac.mm @@ -105,6 +105,10 @@ bool GetUserDocumentsDirectory(FilePath* result) { return base::mac::GetUserDirectory(NSDocumentDirectory, result); } +bool GetGlobalApplicationSupportDirectory(FilePath* result) { + return base::mac::GetLocalDirectory(NSApplicationSupportDirectory, result); +} + void GetUserCacheDirectory(const FilePath& profile_dir, FilePath* result) { // If the profile directory is under ~/Library/Application Support, // use a suitable cache directory under ~/Library/Caches. For |