diff options
| -rw-r--r-- | chrome/browser/chromeos/app_mode/kiosk_app_data.cc | 7 | ||||
| -rw-r--r-- | chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc | 9 | ||||
| -rw-r--r-- | chrome/browser/extensions/crx_installer.cc | 15 | ||||
| -rw-r--r-- | chrome/browser/extensions/startup_helper.cc | 12 | ||||
| -rw-r--r-- | extensions/browser/DEPS | 1 | ||||
| -rw-r--r-- | extensions/browser/sandboxed_unpacker.cc | 202 | ||||
| -rw-r--r-- | extensions/browser/sandboxed_unpacker.h | 102 | ||||
| -rw-r--r-- | extensions/browser/sandboxed_unpacker_unittest.cc | 58 | ||||
| -rw-r--r-- | extensions/common/extension_utility_messages.h | 6 | ||||
| -rw-r--r-- | extensions/utility/utility_handler.cc | 38 | ||||
| -rw-r--r-- | extensions/utility/utility_handler.h | 5 |
11 files changed, 319 insertions, 136 deletions
diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc index 22d73d3..1f88f65 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc @@ -147,10 +147,9 @@ class KioskAppData::CrxLoader : public extensions::SandboxedUnpackerClient { scoped_refptr<extensions::SandboxedUnpacker> unpacker( new extensions::SandboxedUnpacker( - extensions::CRXFileInfo(crx_file_), extensions::Manifest::INTERNAL, - extensions::Extension::NO_FLAGS, temp_dir_.path(), - task_runner_.get(), this)); - unpacker->Start(); + extensions::Manifest::INTERNAL, extensions::Extension::NO_FLAGS, + temp_dir_.path(), task_runner_.get(), this)); + unpacker->StartWithCrx(extensions::CRXFileInfo(crx_file_)); } void NotifyFinishedOnBlockingPool() { diff --git a/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc b/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc index b71ab94..d4409a9 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc @@ -29,12 +29,11 @@ KioskExternalUpdateValidator::~KioskExternalUpdateValidator() { void KioskExternalUpdateValidator::Start() { scoped_refptr<extensions::SandboxedUnpacker> unpacker( new extensions::SandboxedUnpacker( - crx_file_, extensions::Manifest::EXTERNAL_PREF, - extensions::Extension::NO_FLAGS, crx_unpack_dir_, - backend_task_runner_.get(), this)); + extensions::Manifest::EXTERNAL_PREF, extensions::Extension::NO_FLAGS, + crx_unpack_dir_, backend_task_runner_.get(), this)); if (!backend_task_runner_->PostTask( - FROM_HERE, - base::Bind(&extensions::SandboxedUnpacker::Start, unpacker.get()))) { + FROM_HERE, base::Bind(&extensions::SandboxedUnpacker::StartWithCrx, + unpacker.get(), crx_file_))) { NOTREACHED(); } } diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc index 2effb1a..295fe34 100644 --- a/chrome/browser/extensions/crx_installer.cc +++ b/chrome/browser/extensions/crx_installer.cc @@ -183,18 +183,15 @@ void CrxInstaller::InstallCrxFile(const CRXFileInfo& source_file) { source_file_ = source_file.path; - scoped_refptr<SandboxedUnpacker> unpacker( - new SandboxedUnpacker(source_file, - install_source_, - creation_flags_, - install_directory_, - installer_task_runner_.get(), - this)); + scoped_refptr<SandboxedUnpacker> unpacker(new SandboxedUnpacker( + install_source_, creation_flags_, install_directory_, + installer_task_runner_.get(), this)); if (!installer_task_runner_->PostTask( - FROM_HERE, - base::Bind(&SandboxedUnpacker::Start, unpacker.get()))) + FROM_HERE, base::Bind(&SandboxedUnpacker::StartWithCrx, + unpacker.get(), source_file))) { NOTREACHED(); + } } void CrxInstaller::InstallUserScript(const base::FilePath& source_file, diff --git a/chrome/browser/extensions/startup_helper.cc b/chrome/browser/extensions/startup_helper.cc index 49068c4..53cd444 100644 --- a/chrome/browser/extensions/startup_helper.cc +++ b/chrome/browser/extensions/startup_helper.cc @@ -133,14 +133,10 @@ class ValidateCrxHelper : public SandboxedUnpackerClient { scoped_refptr<base::SingleThreadTaskRunner> file_task_runner = BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE); - scoped_refptr<SandboxedUnpacker> unpacker( - new SandboxedUnpacker(crx_file_, - Manifest::INTERNAL, - 0, /* no special creation flags */ - temp_dir_, - file_task_runner.get(), - this)); - unpacker->Start(); + scoped_refptr<SandboxedUnpacker> unpacker(new SandboxedUnpacker( + Manifest::INTERNAL, 0, /* no special creation flags */ + temp_dir_, file_task_runner.get(), this)); + unpacker->StartWithCrx(crx_file_); } // The file being validated. diff --git a/extensions/browser/DEPS b/extensions/browser/DEPS index b20ccf8..abf85d9 100644 --- a/extensions/browser/DEPS +++ b/extensions/browser/DEPS @@ -24,6 +24,7 @@ include_rules = [ "+third_party/leveldatabase", "+third_party/re2", "+third_party/WebKit/public/web", + "+third_party/zlib/google", ] specific_include_rules = { diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc index 7b27e85..965de48 100644 --- a/extensions/browser/sandboxed_unpacker.cc +++ b/extensions/browser/sandboxed_unpacker.cc @@ -212,26 +212,18 @@ SandboxedUnpackerClient::SandboxedUnpackerClient() } SandboxedUnpacker::SandboxedUnpacker( - const CRXFileInfo& file, Manifest::Location location, int creation_flags, const base::FilePath& extensions_dir, const scoped_refptr<base::SequencedTaskRunner>& unpacker_io_task_runner, SandboxedUnpackerClient* client) - : crx_path_(file.path), - package_hash_(base::ToLowerASCII(file.expected_hash)), - check_crx_hash_(false), - client_(client), + : client_(client), extensions_dir_(extensions_dir), got_response_(false), location_(location), creation_flags_(creation_flags), - unpacker_io_task_runner_(unpacker_io_task_runner) { - if (!package_hash_.empty()) { - check_crx_hash_ = base::CommandLine::ForCurrentProcess()->HasSwitch( - extensions::switches::kEnableCrxHashCheck); - } -} + unpacker_io_task_runner_(unpacker_io_task_runner), + utility_wrapper_(new UtilityHostWrapper) {} bool SandboxedUnpacker::CreateTempDirectory() { CHECK(unpacker_io_task_runner_->RunsTasksOnCurrentThread()); @@ -256,15 +248,21 @@ bool SandboxedUnpacker::CreateTempDirectory() { return true; } -void SandboxedUnpacker::Start() { +void SandboxedUnpacker::StartWithCrx(const CRXFileInfo& crx_info) { // We assume that we are started on the thread that the client wants us to do // file IO on. CHECK(unpacker_io_task_runner_->RunsTasksOnCurrentThread()); - unpack_start_time_ = base::TimeTicks::Now(); + crx_unpack_start_time_ = base::TimeTicks::Now(); + std::string expected_hash; + if (!crx_info.expected_hash.empty() && + base::CommandLine::ForCurrentProcess()->HasSwitch( + extensions::switches::kEnableCrxHashCheck)) { + expected_hash = base::ToLowerASCII(crx_info.expected_hash); + } PATH_LENGTH_HISTOGRAM("Extensions.SandboxUnpackInitialCrxPathLength", - crx_path_); + crx_info.path); if (!CreateTempDirectory()) return; // ReportFailure() already called. @@ -274,15 +272,16 @@ void SandboxedUnpacker::Start() { extension_root_); // Extract the public key and validate the package. - if (!ValidateSignature()) + if (!ValidateSignature(crx_info.path, expected_hash)) return; // ValidateSignature() already reported the error. // Copy the crx file into our working directory. - base::FilePath temp_crx_path = temp_dir_.path().Append(crx_path_.BaseName()); + base::FilePath temp_crx_path = + temp_dir_.path().Append(crx_info.path.BaseName()); PATH_LENGTH_HISTOGRAM("Extensions.SandboxUnpackTempCrxPathLength", temp_crx_path); - if (!base::CopyFile(crx_path_, temp_crx_path)) { + if (!base::CopyFile(crx_info.path, temp_crx_path)) { // Failed to copy extension file to temporary directory. ReportFailure( FAILED_TO_COPY_EXTENSION_FILE_TO_TEMP_DIRECTORY, @@ -309,10 +308,35 @@ void SandboxedUnpacker::Start() { link_free_crx_path); BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(&SandboxedUnpacker::StartProcessOnIOThread, + base::Bind(&SandboxedUnpacker::StartUnzipOnIOThread, this, link_free_crx_path)); } +void SandboxedUnpacker::StartWithDirectory(const std::string& extension_id, + const std::string& public_key, + const base::FilePath& directory) { + extension_id_ = extension_id; + public_key_ = public_key; + if (!CreateTempDirectory()) + return; // ReportFailure() already called. + + extension_root_ = temp_dir_.path().AppendASCII(kTempExtensionName); + + if (!base::Move(directory, extension_root_)) { + LOG(ERROR) << "Could not move " << directory.value() << " to " + << extension_root_.value(); + ReportFailure( + DIRECTORY_MOVE_FAILED, + l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_INSTALL_ERROR, + ASCIIToUTF16("DIRECTORY_MOVE_FAILED"))); + return; + } + + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(&SandboxedUnpacker::StartUnpackOnIOThread, + this, extension_root_)); +} + SandboxedUnpacker::~SandboxedUnpacker() { // To avoid blocking shutdown, don't delete temporary directory here if it // hasn't been cleaned up or passed on to another owner yet. @@ -322,11 +346,15 @@ SandboxedUnpacker::~SandboxedUnpacker() { bool SandboxedUnpacker::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(SandboxedUnpacker, message) - IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnpackExtension_Succeeded, - OnUnpackExtensionSucceeded) - IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnpackExtension_Failed, - OnUnpackExtensionFailed) - IPC_MESSAGE_UNHANDLED(handled = false) + IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnzipToDir_Succeeded, + OnUnzipToDirSucceeded) + IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnzipToDir_Failed, + OnUnzipToDirFailed) + IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnpackExtension_Succeeded, + OnUnpackExtensionSucceeded) + IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnpackExtension_Failed, + OnUnpackExtensionFailed) + IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() return handled; } @@ -346,23 +374,57 @@ void SandboxedUnpacker::OnProcessCrashed(int exit_code) { l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALL_PROCESS_CRASHED)); } -void SandboxedUnpacker::StartProcessOnIOThread( - const base::FilePath& temp_crx_path) { - UtilityProcessHost* host = - UtilityProcessHost::Create(this, unpacker_io_task_runner_.get()); - host->SetName( - l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_EXTENSION_UNPACKER_NAME)); - // Grant the subprocess access to the entire subdir the extension file is - // in, so that it can unpack to that dir. - host->SetExposedDir(temp_crx_path.DirName()); - host->Send(new ExtensionUtilityMsg_UnpackExtension( - temp_crx_path, extension_id_, location_, creation_flags_)); +void SandboxedUnpacker::StartUnzipOnIOThread(const base::FilePath& crx_path) { + if (!utility_wrapper_->StartIfNeeded(temp_dir_.path(), this, + unpacker_io_task_runner_)) { + ReportFailure( + COULD_NOT_START_UTILITY_PROCESS, + l10n_util::GetStringFUTF16( + IDS_EXTENSION_PACKAGE_INSTALL_ERROR, + FailureReasonToString16(COULD_NOT_START_UTILITY_PROCESS))); + return; + } + DCHECK(crx_path.DirName() == temp_dir_.path()); + base::FilePath unzipped_dir = + crx_path.DirName().AppendASCII(kTempExtensionName); + utility_wrapper_->host()->Send( + new ExtensionUtilityMsg_UnzipToDir(crx_path, unzipped_dir)); +} + +void SandboxedUnpacker::StartUnpackOnIOThread( + const base::FilePath& directory_path) { + if (!utility_wrapper_->StartIfNeeded(temp_dir_.path(), this, + unpacker_io_task_runner_)) { + ReportFailure( + COULD_NOT_START_UTILITY_PROCESS, + l10n_util::GetStringFUTF16( + IDS_EXTENSION_PACKAGE_INSTALL_ERROR, + FailureReasonToString16(COULD_NOT_START_UTILITY_PROCESS))); + return; + } + DCHECK(directory_path.DirName() == temp_dir_.path()); + utility_wrapper_->host()->Send(new ExtensionUtilityMsg_UnpackExtension( + directory_path, extension_id_, location_, creation_flags_)); +} + +void SandboxedUnpacker::OnUnzipToDirSucceeded(const base::FilePath& directory) { + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&SandboxedUnpacker::StartUnpackOnIOThread, this, directory)); +} + +void SandboxedUnpacker::OnUnzipToDirFailed(const std::string& error) { + got_response_ = true; + utility_wrapper_ = nullptr; + ReportFailure(UNZIP_FAILED, + l10n_util::GetStringUTF16(IDS_EXTENSION_PACKAGE_UNZIP_ERROR)); } void SandboxedUnpacker::OnUnpackExtensionSucceeded( const base::DictionaryValue& manifest) { CHECK(unpacker_io_task_runner_->RunsTasksOnCurrentThread()); got_response_ = true; + utility_wrapper_ = nullptr; scoped_ptr<base::DictionaryValue> final_manifest( RewriteManifestFile(manifest)); @@ -411,6 +473,7 @@ void SandboxedUnpacker::OnUnpackExtensionSucceeded( void SandboxedUnpacker::OnUnpackExtensionFailed(const base::string16& error) { CHECK(unpacker_io_task_runner_->RunsTasksOnCurrentThread()); got_response_ = true; + utility_wrapper_ = nullptr; ReportFailure( UNPACKER_CLIENT_FAILED, l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_ERROR_MESSAGE, error)); @@ -495,6 +558,13 @@ base::string16 SandboxedUnpacker::FailureReasonToString16( case CRX_HASH_VERIFICATION_FAILED: return ASCIIToUTF16("CRX_HASH_VERIFICATION_FAILED"); + case UNZIP_FAILED: + return ASCIIToUTF16("UNZIP_FAILED"); + case DIRECTORY_MOVE_FAILED: + return ASCIIToUTF16("DIRECTORY_MOVE_FAILED"); + case COULD_NOT_START_UTILITY_PROCESS: + return ASCIIToUTF16("COULD_NOT_START_UTILITY_PROCESS"); + case NUM_FAILURE_REASONS: NOTREACHED(); return base::string16(); @@ -509,14 +579,14 @@ void SandboxedUnpacker::FailWithPackageError(FailureReason reason) { FailureReasonToString16(reason))); } -bool SandboxedUnpacker::ValidateSignature() { +bool SandboxedUnpacker::ValidateSignature(const base::FilePath& crx_path, + const std::string& expected_hash) { CrxFile::ValidateError error = CrxFile::ValidateSignature( - crx_path_, check_crx_hash_ ? package_hash_ : std::string(), &public_key_, - &extension_id_, nullptr); + crx_path, expected_hash, &public_key_, &extension_id_, nullptr); switch (error) { case CrxFile::ValidateError::NONE: { - if (check_crx_hash_) + if (!expected_hash.empty()) UMA_HISTOGRAM_BOOLEAN("Extensions.SandboxUnpackHashCheck", true); return true; } @@ -558,7 +628,7 @@ bool SandboxedUnpacker::ValidateSignature() { case CrxFile::ValidateError::CRX_HASH_VERIFICATION_FAILED: // We should never get this result unless we had specifically asked for // verification of the crx file's hash. - CHECK(check_crx_hash_ && !package_hash_.empty()); + CHECK(!expected_hash.empty()); UMA_HISTOGRAM_BOOLEAN("Extensions.SandboxUnpackHashCheck", false); FailWithPackageError(CRX_HASH_VERIFICATION_FAILED); break; @@ -568,10 +638,12 @@ bool SandboxedUnpacker::ValidateSignature() { void SandboxedUnpacker::ReportFailure(FailureReason reason, const base::string16& error) { + utility_wrapper_ = nullptr; UMA_HISTOGRAM_ENUMERATION("Extensions.SandboxUnpackFailureReason", reason, NUM_FAILURE_REASONS); - UMA_HISTOGRAM_TIMES("Extensions.SandboxUnpackFailureTime", - base::TimeTicks::Now() - unpack_start_time_); + if (!crx_unpack_start_time_.is_null()) + UMA_HISTOGRAM_TIMES("Extensions.SandboxUnpackFailureTime", + base::TimeTicks::Now() - crx_unpack_start_time_); Cleanup(); CrxInstallError error_info(reason == CRX_HASH_VERIFICATION_FAILED @@ -585,10 +657,14 @@ void SandboxedUnpacker::ReportFailure(FailureReason reason, void SandboxedUnpacker::ReportSuccess( const base::DictionaryValue& original_manifest, const SkBitmap& install_icon) { + utility_wrapper_ = nullptr; UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1); - RecordSuccessfulUnpackTimeHistograms( - crx_path_, base::TimeTicks::Now() - unpack_start_time_); + if (!crx_unpack_start_time_.is_null()) + RecordSuccessfulUnpackTimeHistograms( + crx_path_for_histograms_, + base::TimeTicks::Now() - crx_unpack_start_time_); + DCHECK(!temp_dir_.path().empty()); // Client takes ownership of temporary directory and extension. client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, @@ -601,6 +677,7 @@ base::DictionaryValue* SandboxedUnpacker::RewriteManifestFile( // Add the public key extracted earlier to the parsed manifest and overwrite // the original manifest. We do this to ensure the manifest doesn't contain an // exploitable bug that could be used to compromise the browser. + DCHECK(!public_key_.empty()); scoped_ptr<base::DictionaryValue> final_manifest(manifest.DeepCopy()); final_manifest->SetString(manifest_keys::kPublicKey, public_key_); @@ -631,6 +708,7 @@ base::DictionaryValue* SandboxedUnpacker::RewriteManifestFile( } bool SandboxedUnpacker::RewriteImageFiles(SkBitmap* install_icon) { + DCHECK(!temp_dir_.path().empty()); DecodedImages images; if (!ReadImagesFromFile(temp_dir_.path(), &images)) { // Couldn't read image data from disk. @@ -810,4 +888,42 @@ void SandboxedUnpacker::Cleanup() { } } +SandboxedUnpacker::UtilityHostWrapper::UtilityHostWrapper() {} + +bool SandboxedUnpacker::UtilityHostWrapper::StartIfNeeded( + const base::FilePath& exposed_dir, + const scoped_refptr<UtilityProcessHostClient>& client, + const scoped_refptr<base::SequencedTaskRunner>& client_task_runner) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + if (!utility_host_) { + utility_host_ = + UtilityProcessHost::Create(client, client_task_runner)->AsWeakPtr(); + utility_host_->SetName( + l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_EXTENSION_UNPACKER_NAME)); + + // Grant the subprocess access to our temp dir so it can write out files. + DCHECK(!exposed_dir.empty()); + utility_host_->SetExposedDir(exposed_dir); + if (!utility_host_->StartBatchMode()) { + utility_host_.reset(); + return false; + } + } + return true; +} + +content::UtilityProcessHost* SandboxedUnpacker::UtilityHostWrapper::host() + const { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + return utility_host_.get(); +} + +SandboxedUnpacker::UtilityHostWrapper::~UtilityHostWrapper() { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + if (utility_host_) { + utility_host_->EndBatchMode(); + utility_host_.reset(); + } +} + } // namespace extensions diff --git a/extensions/browser/sandboxed_unpacker.h b/extensions/browser/sandboxed_unpacker.h index 17156df..826d939 100644 --- a/extensions/browser/sandboxed_unpacker.h +++ b/extensions/browser/sandboxed_unpacker.h @@ -2,14 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_EXTENSIONS_SANDBOXED_UNPACKER_H_ -#define CHROME_BROWSER_EXTENSIONS_SANDBOXED_UNPACKER_H_ +#ifndef EXTENSIONS_BROWSER_SANDBOXED_UNPACKER_H_ +#define EXTENSIONS_BROWSER_SANDBOXED_UNPACKER_H_ #include <string> #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/ref_counted_delete_on_message_loop.h" +#include "base/memory/weak_ptr.h" #include "base/time/time.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/utility_process_host_client.h" @@ -24,6 +25,10 @@ class DictionaryValue; class SequencedTaskRunner; } +namespace content { +class UtilityProcessHost; +} + namespace crypto { class SecureHash; } @@ -64,10 +69,11 @@ class SandboxedUnpackerClient virtual ~SandboxedUnpackerClient() {} }; -// SandboxedUnpacker unpacks extensions from the CRX format into a -// directory. This is done in a sandboxed subprocess to protect the browser -// process from parsing complex formats like JPEG or JSON from untrusted -// sources. +// SandboxedUnpacker does work to optionally unpack and then validate/sanitize +// an extension, either starting from a crx file or an already unzipped +// directory (eg from differential update). This is done in a sandboxed +// subprocess to protect the browser process from parsing complex formats like +// JPEG or JSON from untrusted sources. // // Unpacking an extension using this class makes minor changes to its source, // such as transcoding all images to PNG, parsing all message catalogs @@ -87,19 +93,25 @@ class SandboxedUnpackerClient // NOTE: This class should only be used on the file thread. class SandboxedUnpacker : public content::UtilityProcessHostClient { public: - // Unpacks the extension in |crx_path| into a temporary directory and calls - // |client| with the result. If |run_out_of_process| is provided, unpacking - // is done in a sandboxed subprocess. Otherwise, it is done in-process. + // Creates a SanboxedUnpacker that will do work to unpack an extension, + // passing the |location| and |creation_flags| to Extension::Create. The + // |extensions_dir| parameter should specify the directory under which we'll + // create a subdirectory to write the unpacked extension contents. SandboxedUnpacker( - const CRXFileInfo& file, Manifest::Location location, int creation_flags, const base::FilePath& extensions_dir, const scoped_refptr<base::SequencedTaskRunner>& unpacker_io_task_runner, SandboxedUnpackerClient* client); - // Start unpacking the extension. The client is called with the results. - void Start(); + // Start processing the extension, either from a CRX file or already unzipped + // in a directory. The client is called with the results. The directory form + // requires the id and base64-encoded public key (for insertion into the + // 'key' field of the manifest.json file). + void StartWithCrx(const CRXFileInfo& crx_info); + void StartWithDirectory(const std::string& extension_id, + const std::string& public_key_base64, + const base::FilePath& directory); private: class ProcessHostClient; @@ -163,6 +175,10 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient { // SandboxedUnpacker::ValidateSignature() CRX_HASH_VERIFICATION_FAILED, + UNZIP_FAILED, + DIRECTORY_MOVE_FAILED, + COULD_NOT_START_UTILITY_PROCESS, + NUM_FAILURE_REASONS }; @@ -181,16 +197,19 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient { // Validates the signature of the extension and extract the key to // |public_key_|. Returns true if the signature validates, false otherwise. - bool ValidateSignature(); + bool ValidateSignature(const base::FilePath& crx_path, + const std::string& expected_hash); - // Starts the utility process that unpacks our extension. - void StartProcessOnIOThread(const base::FilePath& temp_crx_path); + void StartUnzipOnIOThread(const base::FilePath& crx_path); + void StartUnpackOnIOThread(const base::FilePath& directory_path); // UtilityProcessHostClient bool OnMessageReceived(const IPC::Message& message) override; void OnProcessCrashed(int exit_code) override; // IPC message handlers. + void OnUnzipToDirSucceeded(const base::FilePath& directory); + void OnUnzipToDirFailed(const std::string& error); void OnUnpackExtensionSucceeded(const base::DictionaryValue& manifest); void OnUnpackExtensionFailed(const base::string16& error_message); @@ -211,14 +230,41 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient { // Cleans up temp directory artifacts. void Cleanup(); - // The path to the CRX to unpack. - base::FilePath crx_path_; - - // The package SHA256 hash sum that was reported from the Web Store. - std::string package_hash_; + // This is a helper class to make it easier to keep track of the lifecycle of + // a UtilityProcessHost, including automatic begin and end of batch mode. + class UtilityHostWrapper : public base::RefCountedThreadSafe< + UtilityHostWrapper, + content::BrowserThread::DeleteOnIOThread> { + public: + UtilityHostWrapper(); + + // Start up the utility process if it is not already started, putting it + // into batch mode and giving it access to |exposed_dir|. This should only + // be called on the IO thread. Returns false if there was an error starting + // the utility process or putting it into batch mode. + bool StartIfNeeded( + const base::FilePath& exposed_dir, + const scoped_refptr<UtilityProcessHostClient>& client, + const scoped_refptr<base::SequencedTaskRunner>& client_task_runner); + + // This should only be called on the IO thread. + content::UtilityProcessHost* host() const; + + private: + friend struct content::BrowserThread::DeleteOnThread< + content::BrowserThread::IO>; + friend class base::DeleteHelper<UtilityHostWrapper>; + ~UtilityHostWrapper(); + + // Should only be used on the IO thread. + base::WeakPtr<content::UtilityProcessHost> utility_host_; + + DISALLOW_COPY_AND_ASSIGN(UtilityHostWrapper); + }; - // Whether we need to check the .crx hash sum. - bool check_crx_hash_; + // If we unpacked a crx file, we hold on to the path for use in various + // histograms. + base::FilePath crx_path_for_histograms_; // Our client. scoped_refptr<SandboxedUnpackerClient> client_; @@ -245,8 +291,9 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient { // header. std::string extension_id_; - // Time at which unpacking started. Used to compute the time unpacking takes. - base::TimeTicks unpack_start_time_; + // If we unpacked a .crx file, the time at which unpacking started. Used to + // compute the time unpacking takes. + base::TimeTicks crx_unpack_start_time_; // Location to use for the unpacked extension. Manifest::Location location_; @@ -257,8 +304,13 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient { // Sequenced task runner where file I/O operations will be performed at. scoped_refptr<base::SequencedTaskRunner> unpacker_io_task_runner_; + + // Used for sending tasks to the utility process. + scoped_refptr<UtilityHostWrapper> utility_wrapper_; + + DISALLOW_COPY_AND_ASSIGN(SandboxedUnpacker); }; } // namespace extensions -#endif // CHROME_BROWSER_EXTENSIONS_SANDBOXED_UNPACKER_H_ +#endif // EXTENSIONS_BROWSER_SANDBOXED_UNPACKER_H_ diff --git a/extensions/browser/sandboxed_unpacker_unittest.cc b/extensions/browser/sandboxed_unpacker_unittest.cc index 02fceaf..a66b2fa 100644 --- a/extensions/browser/sandboxed_unpacker_unittest.cc +++ b/extensions/browser/sandboxed_unpacker_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/base64.h" #include "base/bind.h" #include "base/command_line.h" #include "base/files/file_util.h" @@ -11,6 +12,7 @@ #include "base/strings/string_util.h" #include "base/thread_task_runner_handle.h" #include "base/values.h" +#include "components/crx_file/id_util.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_utils.h" #include "extensions/browser/extensions_test.h" @@ -21,6 +23,7 @@ #include "extensions/common/switches.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/skia/include/core/SkBitmap.h" +#include "third_party/zlib/google/zip.h" namespace extensions { @@ -69,6 +72,10 @@ class SandboxedUnpackerTest : public ExtensionsTest { new content::InProcessUtilityThreadHelper); // It will delete itself. client_ = new MockSandboxedUnpackerClient; + + sandboxed_unpacker_ = new SandboxedUnpacker( + Manifest::INTERNAL, Extension::NO_FLAGS, extensions_dir_.path(), + base::ThreadTaskRunnerHandle::Get(), client_); } void TearDown() override { @@ -79,21 +86,38 @@ class SandboxedUnpackerTest : public ExtensionsTest { ExtensionsTest::TearDown(); } + base::FilePath GetCrxFullPath(const std::string& crx_name) { + base::FilePath full_path; + EXPECT_TRUE(PathService::Get(extensions::DIR_TEST_DATA, &full_path)); + full_path = full_path.AppendASCII("unpacker").AppendASCII(crx_name); + EXPECT_TRUE(base::PathExists(full_path)) << full_path.value(); + return full_path; + } + void SetupUnpacker(const std::string& crx_name, const std::string& package_hash) { - base::FilePath original_path; - ASSERT_TRUE(PathService::Get(extensions::DIR_TEST_DATA, &original_path)); - original_path = original_path.AppendASCII("unpacker").AppendASCII(crx_name); - ASSERT_TRUE(base::PathExists(original_path)) << original_path.value(); + base::FilePath crx_path = GetCrxFullPath(crx_name); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, + base::Bind( + &SandboxedUnpacker::StartWithCrx, sandboxed_unpacker_, + extensions::CRXFileInfo(std::string(), crx_path, package_hash))); + client_->WaitForUnpack(); + } - sandboxed_unpacker_ = new SandboxedUnpacker( - extensions::CRXFileInfo(std::string(), original_path, package_hash), - Manifest::INTERNAL, Extension::NO_FLAGS, extensions_dir_.path(), - base::ThreadTaskRunnerHandle::Get(), client_); + void SetupUnpackerWithDirectory(const std::string& crx_name) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + base::FilePath crx_path = GetCrxFullPath(crx_name); + ASSERT_TRUE(zip::Unzip(crx_path, temp_dir.path())); + std::string fake_id = crx_file::id_util::GenerateId(crx_name); + std::string fake_public_key; + base::Base64Encode(std::string(2048, 'k'), &fake_public_key); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, - base::Bind(&SandboxedUnpacker::Start, sandboxed_unpacker_.get())); + FROM_HERE, base::Bind(&SandboxedUnpacker::StartWithDirectory, + sandboxed_unpacker_.get(), fake_id, + fake_public_key, temp_dir.Take())); client_->WaitForUnpack(); } @@ -119,6 +143,13 @@ TEST_F(SandboxedUnpackerTest, NoCatalogsSuccess) { EXPECT_FALSE(base::PathExists(install_path)); } +TEST_F(SandboxedUnpackerTest, FromDirNoCatalogsSuccess) { + SetupUnpackerWithDirectory("no_l10n.crx"); + // Check that there is no _locales folder. + base::FilePath install_path = GetInstallPath().Append(kLocaleFolder); + EXPECT_FALSE(base::PathExists(install_path)); +} + TEST_F(SandboxedUnpackerTest, WithCatalogsSuccess) { SetupUnpacker("good_l10n.crx", ""); // Check that there is _locales folder. @@ -126,6 +157,13 @@ TEST_F(SandboxedUnpackerTest, WithCatalogsSuccess) { EXPECT_TRUE(base::PathExists(install_path)); } +TEST_F(SandboxedUnpackerTest, FromDirWithCatalogsSuccess) { + SetupUnpackerWithDirectory("good_l10n.crx"); + // Check that there is _locales folder. + base::FilePath install_path = GetInstallPath().Append(kLocaleFolder); + EXPECT_TRUE(base::PathExists(install_path)); +} + TEST_F(SandboxedUnpackerTest, FailHashCheck) { base::CommandLine::ForCurrentProcess()->AppendSwitch( extensions::switches::kEnableCrxHashCheck); diff --git a/extensions/common/extension_utility_messages.h b/extensions/common/extension_utility_messages.h index 1d14047..cfaf273 100644 --- a/extensions/common/extension_utility_messages.h +++ b/extensions/common/extension_utility_messages.h @@ -47,10 +47,10 @@ IPC_MESSAGE_CONTROL2(ExtensionUtilityMsg_UnzipToDir, base::FilePath /* zip_file */, base::FilePath /* dir */) -// Tells the utility process to unpack the given extension file in its -// directory and verify that it is valid. +// Tells the utility process to validate and sanitize the extension in a +// directory. IPC_MESSAGE_CONTROL4(ExtensionUtilityMsg_UnpackExtension, - base::FilePath /* extension_filename */, + base::FilePath /* directory_path */, std::string /* extension_id */, int /* Manifest::Location */, int /* InitFromValue flags */) diff --git a/extensions/utility/utility_handler.cc b/extensions/utility/utility_handler.cc index f3c8a8c..4f4ce47 100644 --- a/extensions/utility/utility_handler.cc +++ b/extensions/utility/utility_handler.cc @@ -91,38 +91,22 @@ void UtilityHandler::OnUnzipToDir(const base::FilePath& zip_path, ReleaseProcessIfNeeded(); } -void UtilityHandler::OnUnpackExtension( - const base::FilePath& extension_path, - const std::string& extension_id, - int location, - int creation_flags) { +void UtilityHandler::OnUnpackExtension(const base::FilePath& directory_path, + const std::string& extension_id, + int location, + int creation_flags) { CHECK_GT(location, Manifest::INVALID_LOCATION); CHECK_LT(location, Manifest::NUM_LOCATIONS); DCHECK(ExtensionsClient::Get()); content::UtilityThread::Get()->EnsureBlinkInitialized(); - base::FilePath working_dir = extension_path.DirName(); - base::FilePath unzipped_dir = working_dir.AppendASCII(kTempExtensionName); - base::string16 error; - if (!base::CreateDirectory(unzipped_dir)) { - Send(new ExtensionUtilityHostMsg_UnpackExtension_Failed( - l10n_util::GetStringFUTF16( - IDS_EXTENSION_PACKAGE_DIRECTORY_ERROR, - base::i18n::GetDisplayStringInLTRDirectionality( - unzipped_dir.LossyDisplayName())))); - } else if (!zip::Unzip(extension_path, unzipped_dir)) { - Send(new ExtensionUtilityHostMsg_UnpackExtension_Failed( - l10n_util::GetStringUTF16(IDS_EXTENSION_PACKAGE_UNZIP_ERROR))); + Unpacker unpacker(directory_path.DirName(), directory_path, extension_id, + static_cast<Manifest::Location>(location), creation_flags); + if (unpacker.Run()) { + Send(new ExtensionUtilityHostMsg_UnpackExtension_Succeeded( + *unpacker.parsed_manifest())); } else { - Unpacker unpacker(working_dir, unzipped_dir, extension_id, - static_cast<Manifest::Location>(location), - creation_flags); - if (unpacker.Run()) { - Send(new ExtensionUtilityHostMsg_UnpackExtension_Succeeded( - *unpacker.parsed_manifest())); - } else { - Send(new ExtensionUtilityHostMsg_UnpackExtension_Failed( - unpacker.error_message())); - } + Send(new ExtensionUtilityHostMsg_UnpackExtension_Failed( + unpacker.error_message())); } ReleaseProcessIfNeeded(); } diff --git a/extensions/utility/utility_handler.h b/extensions/utility/utility_handler.h index d3f0b6c..b32707f 100644 --- a/extensions/utility/utility_handler.h +++ b/extensions/utility/utility_handler.h @@ -34,9 +34,10 @@ class UtilityHandler { // IPC message handlers. void OnParseUpdateManifest(const std::string& xml); void OnUnzipToDir(const base::FilePath& zip_path, const base::FilePath& dir); - void OnUnpackExtension(const base::FilePath& extension_path, + void OnUnpackExtension(const base::FilePath& directory_path, const std::string& extension_id, - int location, int creation_flags); + int location, + int creation_flags); DISALLOW_COPY_AND_ASSIGN(UtilityHandler); }; |