diff options
author | rafaelw@chromium.org <rafaelw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-12 20:45:45 +0000 |
---|---|---|
committer | rafaelw@chromium.org <rafaelw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-12 20:45:45 +0000 |
commit | fbcc4030655802213aab5b731f1180062d232f2d (patch) | |
tree | 41da9d881467d61d9ecde52306f5aacebdcf9d33 | |
parent | cdb4266b62202b44eb1fb36877398c2cb5504917 (diff) | |
download | chromium_src-fbcc4030655802213aab5b731f1180062d232f2d.zip chromium_src-fbcc4030655802213aab5b731f1180062d232f2d.tar.gz chromium_src-fbcc4030655802213aab5b731f1180062d232f2d.tar.bz2 |
Verify signed .crx extension installations
This is second try of:
http://codereview.chromium.org/115682
that was comitted in in 18189 and reverted.
BUG=12114
R=erikkay,wtc,aa
Review URL: http://codereview.chromium.org/126014
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18316 0039d316-1c4b-4281-b951-d872f2087c98
37 files changed, 473 insertions, 301 deletions
diff --git a/chrome/browser/extensions/extension_creator.cc b/chrome/browser/extensions/extension_creator.cc index 8c38e17..9abd030 100755 --- a/chrome/browser/extensions/extension_creator.cc +++ b/chrome/browser/extensions/extension_creator.cc @@ -12,6 +12,7 @@ #include "base/file_util.h" #include "base/scoped_handle.h" #include "base/string_util.h" +#include "chrome/browser/extensions/extensions_service.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/zip.h" #include "net/base/base64.h" @@ -20,8 +21,6 @@ namespace { const int kRSAKeySize = 1024; }; -const char ExtensionCreator::kExtensionHeaderMagic[] = "Cr24"; - bool ExtensionCreator::InitializeInput( const FilePath& extension_dir, const FilePath& private_key_path, @@ -167,14 +166,14 @@ bool ExtensionCreator::WriteCRX(const FilePath& zip_path, return false; } - ExtensionCreator::ExtensionHeader header; - memcpy(&header.magic, ExtensionCreator::kExtensionHeaderMagic, - ExtensionCreator::kExtensionHeaderMagicSize); - header.version = kCurrentVersion; + ExtensionsService::ExtensionHeader header; + memcpy(&header.magic, ExtensionsService::kExtensionHeaderMagic, + ExtensionsService::kExtensionHeaderMagicSize); + header.version = ExtensionsService::kCurrentVersion; header.key_size = public_key.size(); header.signature_size = signature.size(); - fwrite(&header, sizeof(ExtensionCreator::ExtensionHeader), 1, + fwrite(&header, sizeof(ExtensionsService::ExtensionHeader), 1, crx_handle.get()); fwrite(&public_key.front(), sizeof(uint8), public_key.size(), crx_handle.get()); diff --git a/chrome/browser/extensions/extension_creator.h b/chrome/browser/extensions/extension_creator.h index febcecd..129e152 100755 --- a/chrome/browser/extensions/extension_creator.h +++ b/chrome/browser/extensions/extension_creator.h @@ -17,28 +17,6 @@ // generated randomly (and optionally written to |output_private_key_path|. class ExtensionCreator { public: - // The size of the magic character sequence at the beginning of each crx file, - // in bytes. This should be a multiple of 4. - static const size_t kExtensionHeaderMagicSize = 4; - - // The magic character sequence at the beginning of each crx file. - static const char kExtensionHeaderMagic[]; - - // The current version of the crx format. - static const uint32 kCurrentVersion = 2; - - // This header is the first data at the beginning of an extension. Its - // contents are purposely 32-bit aligned so that it can just be slurped into - // a struct without manual parsing. - struct ExtensionHeader { - char magic[kExtensionHeaderMagicSize]; - uint32 version; - size_t key_size; // The size of the public key, in bytes. - size_t signature_size; // The size of the signature, in bytes. - // An ASN.1-encoded PublicKeyInfo structure follows. - // The signature follows. - }; - ExtensionCreator() {} bool Run(const FilePath& extension_dir, diff --git a/chrome/browser/extensions/extension_shelf_model_unittest.cc b/chrome/browser/extensions/extension_shelf_model_unittest.cc index 230e2cc..82ce02e 100644 --- a/chrome/browser/extensions/extension_shelf_model_unittest.cc +++ b/chrome/browser/extensions/extension_shelf_model_unittest.cc @@ -15,7 +15,7 @@ namespace { // The extension we're using as our test case. -const char* kExtensionId = "00123456789abcdef0123456789abcdef0123456"; +const char* kExtensionId = "fc6f6ba6693faf6773c13701019f2e7a12f0febe"; }; // namespace diff --git a/chrome/browser/extensions/extension_uitest.cc b/chrome/browser/extensions/extension_uitest.cc index 38a5bb6..12cac46 100644 --- a/chrome/browser/extensions/extension_uitest.cc +++ b/chrome/browser/extensions/extension_uitest.cc @@ -103,7 +103,7 @@ TEST_F(SimpleApiCallExtensionTest, RunTest) { namespace keys = extension_automation_constants; TestWithURL(GURL( - "chrome-extension://77774444789ABCDEF0123456789ABCDEF0123456/test.html")); + "chrome-extension://fc6f6ba6693faf6773c13701019f2e7a12f0febe/test.html")); AutomationProxyForExternalTab* proxy = static_cast<AutomationProxyForExternalTab*>(automation()); ASSERT_GT(proxy->messages_received(), 0); @@ -270,7 +270,7 @@ class RoundtripApiCallExtensionTest #if defined(OS_WIN) TEST_F(RoundtripApiCallExtensionTest, RunTest) { TestWithURL(GURL( - "chrome-extension://66664444789ABCDEF0123456789ABCDEF0123456/test.html")); + "chrome-extension://e5ead92b2c6795c1d2b92df9c5cb37de5582471a/test.html")); RoundtripAutomationProxy* proxy = static_cast<RoundtripAutomationProxy*>(automation()); diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index bf79e78..6d99e84 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -6,6 +6,7 @@ #include "app/l10n_util.h" #include "base/command_line.h" +#include "base/crypto/signature_verifier.h" #include "base/file_util.h" #include "base/gfx/png_encoder.h" #include "base/scoped_handle.h" @@ -20,6 +21,7 @@ #include "chrome/browser/browser_list.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_thread.h" +#include "chrome/browser/extensions/extension_creator.h" #include "chrome/browser/extensions/extension_browser_event_router.h" #include "chrome/browser/extensions/extension_process_manager.h" #include "chrome/browser/extensions/external_extension_provider.h" @@ -38,6 +40,7 @@ #include "chrome/common/url_constants.h" #include "grit/chromium_strings.h" #include "grit/generated_resources.h" +#include "net/base/base64.h" #include "third_party/skia/include/core/SkBitmap.h" #if defined(OS_WIN) @@ -48,6 +51,8 @@ // ExtensionsService. +const char ExtensionsService::kExtensionHeaderMagic[] = "Cr24"; + const char* ExtensionsService::kInstallDirectoryName = "Extensions"; const char* ExtensionsService::kCurrentVersionFileName = "Current Version"; const char* ExtensionsServiceBackend::kTempExtensionName = "TEMP_INSTALL"; @@ -65,8 +70,17 @@ const wchar_t kState[] = L"state"; // A temporary subdirectory where we unpack extensions. const char* kUnpackExtensionDir = "TEMP_UNPACK"; -// The version of the extension package that this code understands. -const uint32 kExpectedVersion = 1; +// Unpacking errors +const char* kBadMagicNumberError = "Bad magic number"; +const char* kBadHeaderSizeError = "Excessively large key or signature"; +const char* kBadVersionNumberError = "Bad version number"; +const char* kInvalidExtensionHeaderError = "Invalid extension header"; +const char* kInvalidPublicKeyError = "Invalid public key"; +const char* kInvalidSignatureError = "Invalid signature"; +const char* kSignatureVerificationFailed = "Signature verification failed"; +const char* kSignatureVerificationInitFailed = + "Signature verification initialization failed. This is most likely " + "caused by a public key in the wrong format (should encode algorithm)."; } // This class coordinates an extension unpack task which is run in a separate @@ -77,11 +91,12 @@ class ExtensionsServiceBackend::UnpackerClient public: UnpackerClient(ExtensionsServiceBackend* backend, const FilePath& extension_path, + const std::string& public_key, const std::string& expected_id, bool from_external) : backend_(backend), extension_path_(extension_path), - expected_id_(expected_id), from_external_(from_external), - got_response_(false) { + public_key_(public_key), expected_id_(expected_id), + from_external_(from_external), got_response_(false) { } // Starts the unpack task. We call back to the backend when the task is done, @@ -146,6 +161,14 @@ class ExtensionsServiceBackend::UnpackerClient void OnUnpackExtensionSucceededImpl( const DictionaryValue& manifest, const ExtensionUnpacker::DecodedImages& images) { + // Add our public key into the parsed manifest. We want it to be saved so + // that we can later refer to it (eg for generating ids, validating + // signatures, etc). + // The const_cast is hacky, but seems like the right thing here, rather than + // making a full copy just to make this change. + const_cast<DictionaryValue*>(&manifest)->SetString( + Extension::kPublicKeyKey, public_key_); + // The extension was unpacked to the temp dir inside our unpacking dir. FilePath extension_dir = temp_extension_path_.DirName().AppendASCII( ExtensionsServiceBackend::kTempExtensionName); @@ -182,6 +205,9 @@ class ExtensionsServiceBackend::UnpackerClient // The path to the crx file that we're installing. FilePath extension_path_; + // The public key of the extension we're installing. + std::string public_key_; + // The path to the copy of the crx file in the temporary directory where we're // unpacking it. FilePath temp_extension_path_; @@ -355,7 +381,7 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) { } void ExtensionsService::OnExtensionInstalled(Extension* extension, - bool update) { + Extension::InstallType install_type) { UpdateExtensionPref(ASCIIToWide(extension->id()), kState, Value::CreateIntegerValue(Extension::ENABLED), false); UpdateExtensionPref(ASCIIToWide(extension->id()), kLocation, @@ -385,7 +411,7 @@ void ExtensionsService::OnExternalExtensionInstalled( Value::CreateIntegerValue(location), true); } -void ExtensionsService::OnExtensionVersionReinstalled(const std::string& id) { +void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) { Extension* extension = GetExtensionByID(id); if (extension && extension->IsTheme()) { NotificationService::current()->Notify( @@ -630,6 +656,21 @@ void ExtensionsServiceBackend::LoadSingleExtension( } } +DictionaryValue* ExtensionsServiceBackend::ReadManifest(FilePath manifest_path, + std::string* error) { + JSONFileValueSerializer serializer(manifest_path); + scoped_ptr<Value> root(serializer.Deserialize(error)); + if (!root.get()) + return NULL; + + if (!root->IsType(Value::TYPE_DICTIONARY)) { + *error = Extension::kInvalidManifestError; + return NULL; + } + + return static_cast<DictionaryValue*>(root.release()); +} + Extension* ExtensionsServiceBackend::LoadExtension( const FilePath& extension_path, Extension::Location location, @@ -641,22 +682,15 @@ Extension* ExtensionsServiceBackend::LoadExtension( return NULL; } - JSONFileValueSerializer serializer(manifest_path); std::string error; - scoped_ptr<Value> root(serializer.Deserialize(&error)); + scoped_ptr<DictionaryValue> root(ReadManifest(manifest_path, &error)); if (!root.get()) { ReportExtensionLoadError(extension_path, error); return NULL; } - if (!root->IsType(Value::TYPE_DICTIONARY)) { - ReportExtensionLoadError(extension_path, Extension::kInvalidManifestError); - return NULL; - } - scoped_ptr<Extension> extension(new Extension(extension_path)); - if (!extension->InitFromValue(*static_cast<DictionaryValue*>(root.get()), - require_id, &error)) { + if (!extension->InitFromValue(*root.get(), require_id, &error)) { ReportExtensionLoadError(extension_path, error); return NULL; } @@ -765,28 +799,36 @@ bool ExtensionsServiceBackend::ReadCurrentVersion(const FilePath& dir, return false; } -bool ExtensionsServiceBackend::CheckCurrentVersion( +Extension::InstallType ExtensionsServiceBackend::CompareToInstalledVersion( + const std::string& id, const std::string& new_version_str, - const std::string& current_version_str, - const FilePath& dest_dir) { + std::string *current_version_str) { + CHECK(current_version_str); + FilePath dir(install_directory_.AppendASCII(id.c_str())); + if (!ReadCurrentVersion(dir, current_version_str)) + return Extension::NEW_INSTALL; + scoped_ptr<Version> current_version( - Version::GetVersionFromString(current_version_str)); + Version::GetVersionFromString(*current_version_str)); scoped_ptr<Version> new_version( - Version::GetVersionFromString(new_version_str)); - if (current_version->CompareTo(*new_version) >= 0) { - // Verify that the directory actually exists. If it doesn't we'll return - // true so that the install code will repair the broken installation. - // TODO(erikkay): A further step would be to verify that the extension - // has actually loaded successfully. - FilePath version_dir = dest_dir.AppendASCII(current_version_str); - if (file_util::PathExists(version_dir)) { - std::string id = WideToASCII(dest_dir.BaseName().ToWStringHack()); - StringToLowerASCII(&id); - ReportExtensionVersionReinstalled(id); - return false; - } - } - return true; + Version::GetVersionFromString(new_version_str)); + int comp = new_version->CompareTo(*current_version); + if (comp > 0) + return Extension::UPGRADE; + else if (comp == 0) + return Extension::REINSTALL; + else + return Extension::DOWNGRADE; +} + +bool ExtensionsServiceBackend::NeedsReinstall(const std::string& id, + const std::string& current_version) { + // Verify that the directory actually exists. + // TODO(erikkay): A further step would be to verify that the extension + // has actually loaded successfully. + FilePath dir(install_directory_.AppendASCII(id.c_str())); + FilePath version_dir(dir.AppendASCII(current_version)); + return !file_util::PathExists(version_dir); } bool ExtensionsServiceBackend::InstallDirSafely(const FilePath& source_dir, @@ -873,13 +915,107 @@ void ExtensionsServiceBackend::InstallExtension( } void ExtensionsServiceBackend::InstallOrUpdateExtension( - const FilePath& extension_path, const std::string& expected_id, + const FilePath& extension_path, + const std::string& expected_id, bool from_external) { - UnpackerClient* client = - new UnpackerClient(this, extension_path, expected_id, from_external); + std::string actual_public_key; + if (!ValidateSignature(extension_path, &actual_public_key)) + return; // Failures reported within ValidateSignature(). + + UnpackerClient* client = new UnpackerClient( + this, extension_path, actual_public_key, expected_id, from_external); client->Start(); } +bool ExtensionsServiceBackend::ValidateSignature(const FilePath& extension_path, + std::string* key_out) { + ScopedStdioHandle file(file_util::OpenFile(extension_path, "rb")); + if (!file.get()) { + ReportExtensionInstallError(extension_path, "Could not open file."); + return NULL; + } + + // Read and verify the header. + ExtensionsService::ExtensionHeader header; + size_t len; + + // TODO(erikkay): Yuck. I'm not a big fan of this kind of code, but it + // appears that we don't have any endian/alignment aware serialization + // code in the code base. So for now, this assumes that we're running + // on a little endian machine with 4 byte alignment. + len = fread(&header, 1, sizeof(ExtensionsService::ExtensionHeader), + file.get()); + if (len < sizeof(ExtensionsService::ExtensionHeader)) { + ReportExtensionInstallError(extension_path, kInvalidExtensionHeaderError); + return false; + } + if (strncmp(ExtensionsService::kExtensionHeaderMagic, header.magic, + sizeof(header.magic))) { + ReportExtensionInstallError(extension_path, kBadMagicNumberError); + return false; + } + if (header.version != ExtensionsService::kCurrentVersion) { + ReportExtensionInstallError(extension_path, kBadVersionNumberError); + return false; + } + if (header.key_size > ExtensionsService::kMaxPublicKeySize || + header.signature_size > ExtensionsService::kMaxSignatureSize) { + ReportExtensionInstallError(extension_path, kBadHeaderSizeError); + return false; + } + + std::vector<uint8> key; + key.resize(header.key_size); + len = fread(&key.front(), sizeof(uint8), header.key_size, file.get()); + if (len < header.key_size) { + ReportExtensionInstallError(extension_path, kInvalidPublicKeyError); + return false; + } + + std::vector<uint8> signature; + signature.resize(header.signature_size); + len = fread(&signature.front(), sizeof(uint8), header.signature_size, + file.get()); + if (len < header.signature_size) { + ReportExtensionInstallError(extension_path, kInvalidSignatureError); + return false; + } + + // Note: this structure is an ASN.1 which encodes the algorithm used + // with its parameters. This is defined in PKCS #1 v2.1 (RFC 3447). + // It is encoding: { OID sha1WithRSAEncryption PARAMETERS NULL } + // TODO(aa): This needs to be factored away someplace common. + const uint8 signature_algorithm[15] = { + 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, + 0xf7, 0x0d, 0x01, 0x01, 0x05, 0x05, 0x00 + }; + + base::SignatureVerifier verifier; + if (!verifier.VerifyInit(signature_algorithm, + sizeof(signature_algorithm), + &signature.front(), + signature.size(), + &key.front(), + key.size())) { + ReportExtensionInstallError(extension_path, + kSignatureVerificationInitFailed); + return false; + } + + unsigned char buf[1 << 12]; + while ((len = fread(buf, 1, sizeof(buf), file.get())) > 0) + verifier.VerifyUpdate(buf, len); + + if (!verifier.VerifyFinal()) { + ReportExtensionInstallError(extension_path, kSignatureVerificationFailed); + return false; + } + + net::Base64Encode(std::string(reinterpret_cast<char*>(&key.front()), + key.size()), key_out); + return true; +} + void ExtensionsServiceBackend::OnExtensionUnpacked( const FilePath& extension_path, const FilePath& temp_extension_dir, @@ -942,11 +1078,25 @@ void ExtensionsServiceBackend::OnExtensionUnpacked( FilePath dest_dir = install_directory_.AppendASCII(extension.id()); std::string version = extension.VersionString(); std::string current_version; - bool was_update = false; - if (ReadCurrentVersion(dest_dir, ¤t_version)) { - if (!CheckCurrentVersion(version, current_version, dest_dir)) + Extension::InstallType install_type = + CompareToInstalledVersion(extension.id(), version, ¤t_version); + + // Do not allow downgrade. + if (install_type == Extension::DOWNGRADE) { + ReportExtensionInstallError(extension_path, + "Error: Attempt to downgrade extension from more recent version."); + return; + } + + if (install_type == Extension::REINSTALL) { + if (NeedsReinstall(extension.id(), current_version)) { + // Treat corrupted existing installation as new install case. + install_type = Extension::NEW_INSTALL; + } else { + // The client may use this as a signal (to switch themes, for instance). + ReportExtensionOverinstallAttempted(extension.id()); return; - was_update = true; + } } // Write our parsed manifest back to disk, to ensure it doesn't contain an @@ -1044,7 +1194,7 @@ void ExtensionsServiceBackend::OnExtensionUnpacked( frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( frontend_, &ExtensionsService::OnExtensionInstalled, extension, - was_update)); + install_type)); // Only one extension, but ReportExtensionsLoaded can handle multiple, // so we need to construct a list. @@ -1073,10 +1223,10 @@ void ExtensionsServiceBackend::ReportExtensionInstallError( ExtensionErrorReporter::GetInstance()->ReportError(message, alert_on_error_); } -void ExtensionsServiceBackend::ReportExtensionVersionReinstalled( +void ExtensionsServiceBackend::ReportExtensionOverinstallAttempted( const std::string& id) { frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod( - frontend_, &ExtensionsService::OnExtensionVersionReinstalled, id)); + frontend_, &ExtensionsService::OnExtensionOverinstallAttempted, id)); } bool ExtensionsServiceBackend::ShouldSkipInstallingExtension( @@ -1224,9 +1374,14 @@ void ExtensionsServiceBackend::OnExternalExtensionFound( bool ExtensionsServiceBackend::ShouldInstall(const std::string& id, const Version* version) { - FilePath dir(install_directory_.AppendASCII(id.c_str())); std::string current_version; - if (ReadCurrentVersion(dir, ¤t_version)) - return CheckCurrentVersion(version->GetString(), current_version, dir); - return true; + Extension::InstallType install_type = + CompareToInstalledVersion(id, version->GetString(), ¤t_version); + + if (install_type == Extension::DOWNGRADE) + return false; + + return (install_type == Extension::UPGRADE || + install_type == Extension::NEW_INSTALL || + NeedsReinstall(id, current_version)); } diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index fb0feaf..7def6f1 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -34,10 +34,43 @@ class UserScriptMaster; typedef std::vector<Extension*> ExtensionList; + // Manages installed and running Chromium extensions. class ExtensionsService : public base::RefCountedThreadSafe<ExtensionsService> { public: + + // TODO(port): Move Crx package definitions to ExtentionCreator. They are + // currently here because ExtensionCreator is excluded on linux & mac. + + // The size of the magic character sequence at the beginning of each crx + // file, in bytes. This should be a multiple of 4. + static const size_t kExtensionHeaderMagicSize = 4; + + // The maximum size the crx parser will tolerate for a public key. + static const size_t kMaxPublicKeySize = 1 << 16; + + // The maximum size the crx parser will tolerate for a signature. + static const size_t kMaxSignatureSize = 1 << 16; + + // The magic character sequence at the beginning of each crx file. + static const char kExtensionHeaderMagic[]; + + // The current version of the crx format. + static const uint32 kCurrentVersion = 2; + + // This header is the first data at the beginning of an extension. Its + // contents are purposely 32-bit aligned so that it can just be slurped into + // a struct without manual parsing. + struct ExtensionHeader { + char magic[kExtensionHeaderMagicSize]; + uint32 version; + size_t key_size; // The size of the public key, in bytes. + size_t signature_size; // The size of the signature, in bytes. + // An ASN.1-encoded PublicKeyInfo structure follows. + // The signature follows. + }; + ExtensionsService(Profile* profile, MessageLoop* frontend_loop, MessageLoop* backend_loop); @@ -117,14 +150,16 @@ class ExtensionsService void OnExtensionsLoaded(ExtensionList* extensions); // Called by the backend when an extensoin hsa been installed. - void OnExtensionInstalled(Extension* extension, bool is_update); + void OnExtensionInstalled(Extension* extension, + Extension::InstallType install_type); // Called by the backend when an external extension has been installed. void OnExternalExtensionInstalled( const std::string& id, Extension::Location location); - // Called by the backend when an extension has been reinstalled. - void OnExtensionVersionReinstalled(const std::string& id); + // Called by the backend when an attempt was made to reinstall the same + // version of an existing extension. + void OnExtensionOverinstallAttempted(const std::string& id); // The name of the directory inside the profile where extensions are // installed to. @@ -221,6 +256,11 @@ class ExtensionsServiceBackend class UnpackerClient; friend class UnpackerClient; + // Utility function to read an extension manifest and return it as a + // DictionaryValue. If it fails, NULL is returned and |error| contains an + // appropriate message. + DictionaryValue* ReadManifest(FilePath manifest_path, std::string* error); + // Load a single extension from |extension_path|, the top directory of // a specific extension where its manifest file lives. Extension* LoadExtension(const FilePath& extension_path, @@ -241,6 +281,10 @@ class ExtensionsServiceBackend const std::string& expected_id, bool from_external); + // Validates the signature of the extension in |extension_path|. Returns true + // and the public key (in |key|) if the signature validates, false otherwise. + bool ValidateSignature(const FilePath& extension_path, std::string* key_out); + // Finish installing an extension after it has been unpacked to // |temp_extension_dir| by our utility process. If |expected_id| is not // empty, it's verified against the extension's manifest before installation. @@ -265,8 +309,9 @@ class ExtensionsServiceBackend void ReportExtensionInstallError(const FilePath& extension_path, const std::string& error); - // Notify the frontend that the extension had already been installed. - void ReportExtensionVersionReinstalled(const std::string& id); + // Notify the frontend that an attempt was made (but not carried out) to + // install the same version of an existing extension. + void ReportExtensionOverinstallAttempted(const std::string& id); // Checks a set of strings (containing id's to ignore) in order to determine // if the extension should be installed. @@ -297,11 +342,14 @@ class ExtensionsServiceBackend // Reads the Current Version file from |dir| into |version_string|. bool ReadCurrentVersion(const FilePath& dir, std::string* version_string); - // Check that the version to be installed is greater than the current - // installed extension. - bool CheckCurrentVersion(const std::string& version, - const std::string& current_version, - const FilePath& dest_dir); + // Look for an existing installation of the extension |id| & return + // an InstallType that would result from installing |new_version_str|. + Extension::InstallType CompareToInstalledVersion(const std::string& id, + const std::string& new_version_str, std::string* current_version_str); + + // Does an existing installed extension need to be reinstalled. + bool NeedsReinstall(const std::string& id, + const std::string& current_version); // Install the extension dir by moving it from |source| to |dest| safely. bool InstallDirSafely(const FilePath& source, diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index 1411d78..ab380fd 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -33,13 +33,13 @@ namespace { // Extension ids used during testing. const char* const all_zero = "0000000000000000000000000000000000000000"; const char* const zero_n_one = "0000000000000000000000000000000000000001"; -const char* const good0 = "00123456789abcdef0123456789abcdef0123456"; -const char* const good1 = "10123456789abcdef0123456789abcdef0123456"; -const char* const good2 = "20123456789abcdef0123456789abcdef0123456"; -const char* const good_crx = "00123456789abcdef0123456789abcdef0123456"; -const char* const page_action = "8a5e4cb023c61b431e9b603a97c293429ce057c8"; -const char* const theme_crx = "f0123456789abcdef0123456789abcdef0126456"; -const char* const theme2_crx = "f0123456789adddef0123456789abcdef0126456"; +const char* const good0 = "fc6f6ba6693faf6773c13701019f2e7a12f0febe"; +const char* const good1 = "e5ead92b2c6795c1d2b92df9c5cb37de5582471a"; +const char* const good2 = "a37fed892f622823f4daaec4426a32fc7f6147dc"; +const char* const good_crx = "b3dd733cd71a98fa83f387455e12f5c5501c519e"; +const char* const page_action = "a4ca7d01469a010acb200568a0b8f4d9b3ac1f91"; +const char* const theme_crx = "80c45f5ae9e0f839d105c6a6d2461a036bc40a04"; +const char* const theme2_crx = "f9f6c52c01efdd5edd7c396b5f995a15fc7ad6d1"; struct ExtensionsOrder { bool operator()(const Extension* a, const Extension* b) { @@ -303,7 +303,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) { } ASSERT_EQ(3u, loaded_.size()); - EXPECT_EQ(std::string(good_crx), loaded_[0]->id()); + EXPECT_EQ(std::string(good0), loaded_[0]->id()); EXPECT_EQ(std::string("My extension 1"), loaded_[0]->name()); EXPECT_EQ(std::string("The first extension that I made."), @@ -313,8 +313,8 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) { EXPECT_EQ(3u, service_->extensions()->size()); ValidatePrefKeyCount(3); - ValidatePref(good_crx, L"state", Extension::ENABLED); - ValidatePref(good_crx, L"location", Extension::INTERNAL); + ValidatePref(good0, L"state", Extension::ENABLED); + ValidatePref(good0, L"location", Extension::INTERNAL); ValidatePref(good1, L"state", Extension::ENABLED); ValidatePref(good1, L"location", Extension::INTERNAL); ValidatePref(good2, L"state", Extension::ENABLED); @@ -466,6 +466,10 @@ TEST_F(ExtensionsServiceTest, InstallExtension) { ValidatePref(page_action, L"state", Extension::ENABLED); ValidatePref(page_action, L"location", Extension::INTERNAL); + // Bad signature. + path = extensions_path.AppendASCII("bad_signature.crx"); + InstallExtension(path, false); + // 0-length extension file. path = extensions_path.AppendASCII("not_an_extension.crx"); InstallExtension(path, false); @@ -476,16 +480,6 @@ TEST_F(ExtensionsServiceTest, InstallExtension) { InstallExtension(path, false); ValidatePrefKeyCount(pref_count); - // Poorly formed JSON. - path = extensions_path.AppendASCII("bad_json.crx"); - InstallExtension(path, false); - ValidatePrefKeyCount(pref_count); - - // Incorrect zip hash. - path = extensions_path.AppendASCII("bad_hash.crx"); - InstallExtension(path, false); - ValidatePrefKeyCount(pref_count); - // TODO(erikkay): add more tests for many of the failure cases. // TODO(erikkay): add tests for upgrade cases. } @@ -627,6 +621,56 @@ TEST_F(ExtensionsServiceTest, Reinstall) { ValidatePref(good_crx, L"location", Extension::INTERNAL); } +// Test upgrading a signed extension. +TEST_F(ExtensionsServiceTest, UpgradeSignedGood) { + FilePath extensions_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); + extensions_path = extensions_path.AppendASCII("extensions"); + + FilePath path = extensions_path.AppendASCII("good.crx"); + service_->InstallExtension(path); + loop_.RunAllPending(); + + ASSERT_TRUE(installed_); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ(0u, GetErrors().size()); + + // Upgrade to version 2.0 + path = extensions_path.AppendASCII("good2.crx"); + service_->InstallExtension(path); + loop_.RunAllPending(); + + ASSERT_TRUE(installed_); + ASSERT_EQ(2u, loaded_.size()); + ASSERT_EQ(0u, GetErrors().size()); +} + +// Test upgrading a signed extension with a bad signature. +TEST_F(ExtensionsServiceTest, UpgradeSignedBad) { + FilePath extensions_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); + extensions_path = extensions_path.AppendASCII("extensions"); + + FilePath path = extensions_path.AppendASCII("good.crx"); + service_->InstallExtension(path); + loop_.RunAllPending(); + + ASSERT_TRUE(installed_); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ(0u, GetErrors().size()); + installed_ = NULL; + + // Try upgrading with a bad signature. This should fail during the unpack, + // because the key will not match the signature. + path = extensions_path.AppendASCII("good2_bad_signature.crx"); + service_->InstallExtension(path); + loop_.RunAllPending(); + + ASSERT_FALSE(installed_); + ASSERT_EQ(1u, loaded_.size()); + ASSERT_EQ(1u, GetErrors().size()); +} + // Tests uninstalling normal extensions TEST_F(ExtensionsServiceTest, UninstallExtension) { FilePath extensions_path; @@ -724,6 +768,8 @@ TEST_F(ExtensionsServiceTest, LoadExtension) { // Tests that we generate IDs when they are not specified in the manifest for // --load-extension. TEST_F(ExtensionsServiceTest, GenerateID) { + Extension::ResetGeneratedIdCounter(); + FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); extensions_path = extensions_path.AppendASCII("extensions"); diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index 5a5b2a2..63257f1 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -1644,6 +1644,7 @@ # '../build/linux/system.gyp:dbus-glib', # '../build/linux/system.gyp:gnome-keyring', '../build/linux/system.gyp:gtk', + '../build/linux/system.gyp:nss', ], 'sources!': [ 'browser/extensions/extension_shelf.cc', diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index bf7d3a5..73eb417 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -5,10 +5,14 @@ #include "chrome/common/extensions/extension.h" #include "app/resource_bundle.h" +#include "base/basictypes.h" #include "base/file_path.h" #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" +#include "base/third_party/nss/blapi.h" +#include "base/third_party/nss/sha256.h" +#include "net/base/base64.h" #include "net/base/net_util.h" #include "chrome/common/extensions/extension_error_reporter.h" #include "chrome/common/extensions/extension_error_utils.h" @@ -33,6 +37,8 @@ namespace { const int kRSAKeySize = 1024; }; +int Extension::id_counter_ = 0; + const char Extension::kManifestFilename[] = "manifest.json"; const wchar_t* Extension::kBackgroundKey = L"background_page"; @@ -40,10 +46,10 @@ const wchar_t* Extension::kContentScriptsKey = L"content_scripts"; const wchar_t* Extension::kCssKey = L"css"; const wchar_t* Extension::kDescriptionKey = L"description"; const wchar_t* Extension::kIconPathKey = L"icon"; -const wchar_t* Extension::kIdKey = L"id"; const wchar_t* Extension::kJsKey = L"js"; const wchar_t* Extension::kMatchesKey = L"matches"; const wchar_t* Extension::kNameKey = L"name"; +const wchar_t* Extension::kPageActionIdKey = L"id"; const wchar_t* Extension::kPageActionsKey = L"page_actions"; const wchar_t* Extension::kPermissionsKey = L"permissions"; const wchar_t* Extension::kPluginsKey = L"plugins"; @@ -71,7 +77,6 @@ const char* Extension::kPageActionTypePermanent = "permanent"; static const wchar_t* kValidThemeKeys[] = { Extension::kDescriptionKey, Extension::kIconPathKey, - Extension::kIdKey, Extension::kNameKey, Extension::kPublicKeyKey, Extension::kSignatureKey, @@ -93,12 +98,12 @@ const char* Extension::kInvalidCssListError = "Required value 'content_scripts[*].css is invalid."; const char* Extension::kInvalidDescriptionError = "Invalid value for 'description'."; -const char* Extension::kInvalidIdError = - "Required value 'id' is missing or invalid."; const char* Extension::kInvalidJsError = "Invalid value for 'content_scripts[*].js[*]'."; const char* Extension::kInvalidJsListError = "Required value 'content_scripts[*].js is invalid."; +const char* Extension::kInvalidKeyError = + "Value 'key' is missing or invalid."; const char* Extension::kInvalidManifestError = "Manifest is missing or invalid."; const char* Extension::kInvalidMatchCountError = @@ -116,6 +121,8 @@ const char* Extension::kInvalidPageActionsListError = "Invalid value for 'page_actions'."; const char* Extension::kInvalidPageActionIconPathError = "Invalid value for 'page_actions[*].icon'."; +const char* Extension::kInvalidPageActionIdError = + "Required value 'id' is missing or invalid."; const char* Extension::kInvalidPageActionTooltipError = "Invalid value for 'page_actions[*].tooltip'."; const char* Extension::kInvalidPageActionTypeValueError = @@ -139,6 +146,8 @@ const char* Extension::kInvalidBackgroundError = "Invalid value for 'background'."; const char* Extension::kInvalidRunAtError = "Invalid value for 'content_scripts[*].run_at'."; +const char* Extension::kInvalidSignatureError = + "Value 'signature' is missing or invalid."; const char* Extension::kInvalidToolstripError = "Invalid value for 'toolstrips[*]'"; const char* Extension::kInvalidToolstripsError = @@ -167,7 +176,8 @@ const char* Extension::kExtensionRegistryPath = "Software\\Google\\Chrome\\Extensions"; #endif -const size_t Extension::kIdSize = 20; // SHA1 (160 bits) == 20 bytes +// first 20 bytes of SHA256 hashed public key. +const size_t Extension::kIdSize = 20; Extension::~Extension() { for (PageActionMap::iterator i = page_actions_.begin(); @@ -230,6 +240,23 @@ Extension::Location Extension::ExternalExtensionInstallType( return Extension::EXTERNAL_PREF; } +bool Extension::GenerateIdFromPublicKey(const std::string& input, + std::string* output) { + CHECK(output); + if (input.length() == 0) + return false; + + const uint8* ubuf = reinterpret_cast<const unsigned char*>(input.data()); + SHA256Context ctx; + SHA256_Begin(&ctx); + SHA256_Update(&ctx, ubuf, input.length()); + uint8 hash[Extension::kIdSize]; + SHA256_End(&ctx, hash, NULL, sizeof(hash)); + *output = StringToLowerASCII(HexEncode(hash, sizeof(hash))); + + return true; +} + // Helper method that loads a UserScript object from a dictionary in the // content_script list of the manifest. bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script, @@ -367,8 +394,8 @@ PageAction* Extension::LoadPageActionHelper( // Read the page action |id|. std::string id; - if (!page_action->GetString(kIdKey, &id)) { - *error = ExtensionErrorUtils::FormatErrorMessage(kInvalidIdError, + if (!page_action->GetString(kPageActionIdKey, &id)) { + *error = ExtensionErrorUtils::FormatErrorMessage(kInvalidPageActionIdError, IntToString(definition_index)); return NULL; } @@ -492,13 +519,14 @@ Extension::Extension(const FilePath& path) { #endif } - // TODO(rafaelw): Move ParsePEMKeyBytes, ProducePEM & FormatPEMForOutput to a // util class in base: // http://code.google.com/p/chromium/issues/detail?id=13572 bool Extension::ParsePEMKeyBytes(const std::string& input, std::string* output) { - CHECK(output); + DCHECK(output); + if (!output) + return false; if (input.length() == 0) return false; @@ -564,33 +592,23 @@ bool Extension::FormatPEMForFileOutput(const std::string input, bool Extension::InitFromValue(const DictionaryValue& source, bool require_id, std::string* error) { - // Initialize id. - if (source.HasKey(kIdKey)) { - if (!source.GetString(kIdKey, &id_)) { - *error = kInvalidIdError; - return false; - } - - // Normalize the string to lowercase, so it can be used as an URL component - // (where GURL will lowercase it). - StringToLowerASCII(&id_); - - // Verify that the id is legal. - if (!IdIsValid(id_)) { - *error = kInvalidIdError; - return false; + if (source.HasKey(kPublicKeyKey)) { + std::string public_key_bytes; + if (!source.GetString(kPublicKeyKey, &public_key_) || + !ParsePEMKeyBytes(public_key_, &public_key_bytes) || + !GenerateIdFromPublicKey(public_key_bytes, &id_)) { + *error = kInvalidKeyError; + return false; } } else if (require_id) { - *error = kInvalidIdError; + *error = kInvalidKeyError; return false; } else { // Generate a random ID - static int counter = 0; - id_ = StringPrintf("%x", counter); - ++counter; + id_ = StringPrintf("%x", NextGeneratedId()); - // pad the string out to 40 chars with zeroes. - id_.insert(0, 40 - id_.length(), '0'); + // pad the string out to kIdSize*2 chars with zeroes. + id_.insert(0, Extension::kIdSize*2 - id_.length(), '0'); } // Initialize the URL. diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index 9a5bfbf..658b57f 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -38,6 +38,13 @@ class Extension { KILLBIT, // Don't install/upgrade (applies to external extensions only). }; + enum InstallType { + DOWNGRADE, + REINSTALL, + UPGRADE, + NEW_INSTALL + }; + // An NPAPI plugin included in the extension. struct PluginInfo { FilePath path; // Path to the plugin. @@ -53,10 +60,10 @@ class Extension { static const wchar_t* kCssKey; static const wchar_t* kDescriptionKey; static const wchar_t* kIconPathKey; - static const wchar_t* kIdKey; static const wchar_t* kJsKey; static const wchar_t* kMatchesKey; static const wchar_t* kNameKey; + static const wchar_t* kPageActionIdKey; static const wchar_t* kPageActionsKey; static const wchar_t* kPermissionsKey; static const wchar_t* kPluginsKey; @@ -87,9 +94,9 @@ class Extension { static const char* kInvalidCssError; static const char* kInvalidCssListError; static const char* kInvalidDescriptionError; - static const char* kInvalidIdError; static const char* kInvalidJsError; static const char* kInvalidJsListError; + static const char* kInvalidKeyError; static const char* kInvalidManifestError; static const char* kInvalidMatchCountError; static const char* kInvalidMatchError; @@ -101,12 +108,14 @@ class Extension { static const char* kInvalidBackgroundError; static const char* kInvalidRunAtError; + static const char* kInvalidSignatureError; static const char* kInvalidToolstripError; static const char* kInvalidToolstripsError; static const char* kInvalidVersionError; static const char* kInvalidPageActionError; static const char* kInvalidPageActionsListError; static const char* kInvalidPageActionIconPathError; + static const char* kInvalidPageActionIdError; static const char* kInvalidPageActionTooltipError; static const char* kInvalidPageActionTypeValueError; static const char* kInvalidPermissionsError; @@ -133,6 +142,11 @@ class Extension { explicit Extension(const FilePath& path); virtual ~Extension(); + // Resets the id counter. This is only useful for unit tests. + static void ResetGeneratedIdCounter() { + id_counter_ = 0; + } + // Checks to see if the extension has a valid ID. static bool IdIsValid(const std::string& id); @@ -173,6 +187,11 @@ class Extension { // Does a simple base64 encoding of |input| into |output|. static bool ProducePEM(const std::string& input, std::string* output); + // Note: The result is coverted to lower-case because the browser enforces + // hosts to be lower-case in omni-bar. + static bool GenerateIdFromPublicKey(const std::string& input, + std::string* output); + // Expects base64 encoded |input| and formats into |output| including // the appropriate header & footer. static bool FormatPEMForFileOutput(const std::string input, @@ -193,6 +212,7 @@ class Extension { // String representation of the version number. const std::string VersionString() const; const std::string& name() const { return name_; } + const std::string& public_key() const { return public_key_; } const std::string& description() const { return description_; } const UserScriptList& content_scripts() const { return content_scripts_; } const PageActionMap& page_actions() const { return page_actions_; } @@ -222,6 +242,14 @@ class Extension { std::set<FilePath> GetBrowserImages(); private: + // Counter used to assign ids to extensions that are loaded using + // --load-extension. + static int id_counter_; + + // Returns the next counter id. Intentionally post-incrementing so that first + // value is 0. + static int NextGeneratedId() { return id_counter_++; } + // Helper method that loads a UserScript object from a // dictionary in the content_script list of the manifest. bool LoadUserScriptHelper(const DictionaryValue* content_script, @@ -281,10 +309,9 @@ class Extension { // Paths to HTML files to be displayed in the toolbar. std::vector<std::string> toolstrips_; - // A SHA1 hash of the contents of the zip file. Note that this key is only - // present in the manifest that's prepended to the zip. The inner manifest - // will not have this key. - std::string zip_hash_; + // The public key ('key' in the manifest) used to sign the contents of the + // crx package ('signature' in the manifest) + std::string public_key_; // A map of resource id's to relative file paths. scoped_ptr<DictionaryValue> theme_images_; diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc index 7f0f288..661bcb6 100644 --- a/chrome/common/extensions/extension_unittest.cc +++ b/chrome/common/extensions/extension_unittest.cc @@ -43,17 +43,6 @@ TEST(ExtensionTest, InitFromValueInvalid) { scoped_ptr<DictionaryValue> input_value; - // Test missing and invalid ids - input_value.reset(static_cast<DictionaryValue*>(valid_value->DeepCopy())); - input_value->Remove(Extension::kIdKey, NULL); - EXPECT_FALSE(extension.InitFromValue(*input_value, true, &error)); - EXPECT_EQ(Extension::kInvalidIdError, error); - - input_value.reset(static_cast<DictionaryValue*>(valid_value->DeepCopy())); - input_value->SetInteger(Extension::kIdKey, 42); - EXPECT_FALSE(extension.InitFromValue(*input_value, true, &error)); - EXPECT_EQ(Extension::kInvalidIdError, error); - // Test missing and invalid versions input_value.reset(static_cast<DictionaryValue*>(valid_value->DeepCopy())); input_value->Remove(Extension::kVersionKey, NULL); @@ -204,22 +193,22 @@ TEST(ExtensionTest, InitFromValueValid) { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif + Extension::ResetGeneratedIdCounter(); + Extension extension(path); std::string error; DictionaryValue input_value; // Test minimal extension - input_value.SetString(Extension::kIdKey, - "00123456789ABCDEF0123456789ABCDEF0123456"); input_value.SetString(Extension::kVersionKey, "1.0.0.0"); input_value.SetString(Extension::kNameKey, "my extension"); - EXPECT_TRUE(extension.InitFromValue(input_value, true, &error)); + EXPECT_TRUE(extension.InitFromValue(input_value, false, &error)); EXPECT_EQ("", error); - EXPECT_EQ("00123456789abcdef0123456789abcdef0123456", extension.id()); + EXPECT_EQ("0000000000000000000000000000000000000000", extension.id()); EXPECT_EQ("1.0.0.0", extension.VersionString()); EXPECT_EQ("my extension", extension.name()); - EXPECT_EQ("chrome-extension://00123456789abcdef0123456789abcdef0123456/", + EXPECT_EQ("chrome-extension://0000000000000000000000000000000000000000/", extension.url().spec()); EXPECT_EQ(path.value(), extension.path().value()); } @@ -232,11 +221,9 @@ TEST(ExtensionTest, GetResourceURLAndPath) { #endif Extension extension(path); DictionaryValue input_value; - input_value.SetString(Extension::kIdKey, - "00123456789ABCDEF0123456789ABCDEF0123456"); input_value.SetString(Extension::kVersionKey, "1.0.0.0"); input_value.SetString(Extension::kNameKey, "my extension"); - EXPECT_TRUE(extension.InitFromValue(input_value, true, NULL)); + EXPECT_TRUE(extension.InitFromValue(input_value, false, NULL)); EXPECT_EQ(extension.url().spec() + "bar/baz.js", Extension::GetResourceURL(extension.url(), "bar/baz.js").spec()); diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc index b5bc919..bae5a7d 100644 --- a/chrome/common/extensions/extension_unpacker.cc +++ b/chrome/common/extensions/extension_unpacker.cc @@ -8,8 +8,6 @@ #include "base/scoped_handle.h" #include "base/scoped_temp_dir.h" #include "base/string_util.h" -#include "base/third_party/nss/blapi.h" -#include "base/third_party/nss/sha256.h" #include "base/thread.h" #include "base/values.h" #include "net/base/file_stream.h" @@ -23,8 +21,6 @@ #include "webkit/glue/image_decoder.h" namespace { -const char kCurrentVersionFileName[] = "Current Version"; - // The name of a temporary directory to install an extension into for // validation before finalizing install. const char kTempExtensionName[] = "TEMP_INSTALL"; @@ -32,27 +28,14 @@ const char kTempExtensionName[] = "TEMP_INSTALL"; // The file to write our decoded images to, relative to the extension_path. const char kDecodedImagesFilename[] = "DECODED_IMAGES"; -// Chromium Extension magic number -// TODO(aa): This should use the one in ExtensionCreator once we transition this -// to ouptut the same format. -const char kExtensionFileMagic[] = "Cr24"; - -struct ExtensionHeader { - char magic[sizeof(kExtensionFileMagic) - 1]; - uint32 version; - size_t header_size; - size_t manifest_size; -}; - -const size_t kZipHashBytes = 32; // SHA-256 -const size_t kZipHashHexBytes = kZipHashBytes * 2; // Hex string is 2x size. +// Errors +const char* kCouldNotCreateDirectoryError = + "Could not create directory for unzipping."; +const char* kCouldNotDecodeImageError = "Could not decode theme image."; +const char* kCouldNotUnzipExtension = "Could not unzip extension."; +const char* kPathNamesMustBeAbsoluteOrLocalError = + "Path names must not be absolute or contain '..'."; -// A marker file to indicate that an extension was installed from an external -// source. -const char kExternalInstallFile[] = "EXTERNAL_INSTALL"; - -// The version of the extension package that this code understands. -const uint32 kExpectedVersion = 1; } // namespace static SkBitmap DecodeImage(const FilePath& path) { @@ -90,74 +73,6 @@ static bool PathContainsParentDirectory(const FilePath& path) { return false; } -// The extension file format is a header, followed by the manifest, followed -// by the zip file. The header is a magic number, a version, the size of the -// header, and the size of the manifest. These ints are 4 byte little endian. -DictionaryValue* ExtensionUnpacker::ReadPackageHeader() { - ScopedStdioHandle file(file_util::OpenFile(extension_path_, "rb")); - if (!file.get()) { - SetError("no such extension file"); - return NULL; - } - - // Read and verify the header. - ExtensionHeader header; - size_t len; - - // TODO(erikkay): Yuck. I'm not a big fan of this kind of code, but it - // appears that we don't have any endian/alignment aware serialization - // code in the code base. So for now, this assumes that we're running - // on a little endian machine with 4 byte alignment. - len = fread(&header, 1, sizeof(ExtensionHeader), file.get()); - if (len < sizeof(ExtensionHeader)) { - SetError("invalid extension header"); - return NULL; - } - if (strncmp(kExtensionFileMagic, header.magic, sizeof(header.magic))) { - SetError("bad magic number"); - return NULL; - } - if (header.version != kExpectedVersion) { - SetError("bad version number"); - return NULL; - } - if (header.header_size > sizeof(ExtensionHeader)) - fseek(file.get(), header.header_size - sizeof(ExtensionHeader), SEEK_CUR); - - char buf[1 << 16]; - std::string manifest_str; - size_t read_size = std::min(sizeof(buf), header.manifest_size); - size_t remainder = header.manifest_size; - while ((len = fread(buf, 1, read_size, file.get())) > 0) { - manifest_str.append(buf, len); - if (len <= remainder) - break; - remainder -= len; - read_size = std::min(sizeof(buf), remainder); - } - - // Verify the JSON - JSONStringValueSerializer json(manifest_str); - std::string error; - scoped_ptr<Value> val(json.Deserialize(&error)); - if (!val.get()) { - SetError(error); - return NULL; - } - if (!val->IsType(Value::TYPE_DICTIONARY)) { - SetError("manifest isn't a JSON dictionary"); - return NULL; - } - DictionaryValue* manifest = static_cast<DictionaryValue*>(val.get()); - - // TODO(erikkay): The manifest will also contain a signature of the hash - // (or perhaps the whole manifest) for authentication purposes. - - // The caller owns val (now cast to manifest). - val.release(); - return manifest; -} - DictionaryValue* ExtensionUnpacker::ReadManifest() { FilePath manifest_path = temp_install_dir_.AppendASCII(Extension::kManifestFilename); @@ -185,34 +100,16 @@ DictionaryValue* ExtensionUnpacker::ReadManifest() { bool ExtensionUnpacker::Run() { LOG(INFO) << "Installing extension " << extension_path_.value(); - // Read and verify the extension. - scoped_ptr<DictionaryValue> header_manifest(ReadPackageHeader()); - if (!header_manifest.get()) { - // ReadPackageHeader has already reported the extension error. - return false; - } - - // TODO(mpcomplete): it looks like this isn't actually necessary. We don't - // use header_extension, and we check that the unzipped manifest is valid. - Extension header_extension; - std::string error; - if (!header_extension.InitFromValue(*header_manifest, - true, // require ID - &error)) { - SetError(error); - return false; - } - // <profile>/Extensions/INSTALL_TEMP/<version> temp_install_dir_ = extension_path_.DirName().AppendASCII(kTempExtensionName); if (!file_util::CreateDirectory(temp_install_dir_)) { - SetError("Couldn't create directory for unzipping."); + SetError(kCouldNotCreateDirectoryError); return false; } if (!Unzip(extension_path_, temp_install_dir_)) { - SetError("Couldn't unzip extension."); + SetError(kCouldNotUnzipExtension); return false; } @@ -220,16 +117,19 @@ bool ExtensionUnpacker::Run() { parsed_manifest_.reset(ReadManifest()); if (!parsed_manifest_.get()) return false; // Error was already reported. - - // Re-read the actual manifest into our extension struct. + + // NOTE: Since the Unpacker doesn't have the extension's public_id, the + // InitFromValue is allowed to generate a temporary id for the extension. + // ANY CODE THAT FOLLOWS SHOULD NOT DEPEND ON THE CORRECT ID OF THIS + // EXTENSION. Extension extension; + std::string error; if (!extension.InitFromValue(*parsed_manifest_, - true, // require ID + false, &error)) { SetError(error); return false; } - // Decode any images that the browser needs to display. std::set<FilePath> image_paths = extension.GetBrowserImages(); for (std::set<FilePath>::iterator it = image_paths.begin(); @@ -271,13 +171,13 @@ bool ExtensionUnpacker::ReadImagesFromFile(const FilePath& extension_path, bool ExtensionUnpacker::AddDecodedImage(const FilePath& path) { // Make sure it's not referencing a file outside the extension's subdir. if (path.IsAbsolute() || PathContainsParentDirectory(path)) { - SetError("Path names must not be absolute or contain '..'."); + SetError(kPathNamesMustBeAbsoluteOrLocalError); return false; } SkBitmap image_bitmap = DecodeImage(temp_install_dir_.Append(path)); if (image_bitmap.isNull()) { - SetError("Could not decode theme image."); + SetError(kCouldNotDecodeImageError); return false; } diff --git a/chrome/common/extensions/extension_unpacker.h b/chrome/common/extensions/extension_unpacker.h index 65af750..aeb72d9 100644 --- a/chrome/common/extensions/extension_unpacker.h +++ b/chrome/common/extensions/extension_unpacker.h @@ -48,10 +48,6 @@ class ExtensionUnpacker { const DecodedImages& decoded_images() { return decoded_images_; } private: - // Parse the header on the front of the extension file and return the manifest - // inside it. Caller takes ownership of return value. - DictionaryValue* ReadPackageHeader(); - // Parse the manifest.json file inside the extension (not in the header). // Caller takes ownership of return value. DictionaryValue* ReadManifest(); diff --git a/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json b/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json index 9799306..b3b3858 100644 --- a/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json +++ b/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json @@ -1,5 +1,6 @@ { - "id": "00123456789ABCDEF0123456789ABCDEF0123456", + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCsaVmPYFjlYABY7y8sEVB5i3/stDIXU8i3bBeQYeaXOg+Jr1vEhaSgKTy3zMY4m3+cnKI7Z/bMpYohS4pQTcwIXS1kEB6taCUUWgm2315zxSH+gUFRyfzBDlS8LZ9tRirFGj8o0R2YQobHgSPyf04Phq4AeMmGSygEJkfGO+Wt8wIDAQAB", + "signature":"kCUJQeO+FZ0hENaTT9bFW90HwiIppJEj/j88gOyXIL3z00ZOn/eck+M8O+HU7ad+U7BG5GK7xnuIvqzjAYK4ihfEYbExnbx3R79GWUWpHegELbp0PvhU9a+xIwTh48kzpKNgunzZGY0oONGvVeoNQXeTTO8z2i6RZfWD3/YdyBk=", "version": "1.0.0.0", "name": "My extension 1", "description": "The first extension that I made.", diff --git a/chrome/test/data/extensions/bad_hash.crx b/chrome/test/data/extensions/bad_hash.crx Binary files differdeleted file mode 100644 index 8add85c..0000000 --- a/chrome/test/data/extensions/bad_hash.crx +++ /dev/null diff --git a/chrome/test/data/extensions/bad_json.crx b/chrome/test/data/extensions/bad_json.crx Binary files differdeleted file mode 100644 index f249711..0000000 --- a/chrome/test/data/extensions/bad_json.crx +++ /dev/null diff --git a/chrome/test/data/extensions/bad_magic.crx b/chrome/test/data/extensions/bad_magic.crx Binary files differindex da66e12..4323013 100644..100755 --- a/chrome/test/data/extensions/bad_magic.crx +++ b/chrome/test/data/extensions/bad_magic.crx diff --git a/chrome/test/data/extensions/bad_signature.crx b/chrome/test/data/extensions/bad_signature.crx Binary files differnew file mode 100755 index 0000000..d5a6033 --- /dev/null +++ b/chrome/test/data/extensions/bad_signature.crx diff --git a/chrome/test/data/extensions/content_script_inject/manifest.json b/chrome/test/data/extensions/content_script_inject/manifest.json index 8214fb6..48e4496 100644 --- a/chrome/test/data/extensions/content_script_inject/manifest.json +++ b/chrome/test/data/extensions/content_script_inject/manifest.json @@ -1,5 +1,4 @@ { - "id": "00123456789ABCDEF0123456789ABCDEF0123456", "version": "1.0.0.0", "name": "User script inject", "content_scripts": [ diff --git a/chrome/test/data/extensions/good.crx b/chrome/test/data/extensions/good.crx Binary files differindex d479794..1da98b0 100644 --- a/chrome/test/data/extensions/good.crx +++ b/chrome/test/data/extensions/good.crx diff --git a/chrome/test/data/extensions/good.pem b/chrome/test/data/extensions/good.pem new file mode 100755 index 0000000..eb40bdaa --- /dev/null +++ b/chrome/test/data/extensions/good.pem @@ -0,0 +1,16 @@ +-----BEGIN PRIVATE KEY----- +MIICeQICAAAwDQYJKoZIhvcNAQEBBQAEggJiMIICXgICAAACgYC8c4fBSPZ6utYoZ +8NiWF/DSaimBhihjwgOsskyleFGaurhi3TDClTVSGPxNkgCzrz0wACML7M4aNjpd0 +5qupdbR2d294jkDuI7caxEGUucpP7GJRRHnm8Sx+y0ury28n8jbN0PnInKKWcxpIX +XmNQyC19HBuO3QIeUq9Dqc+7YFQIFAAABAAECgYEAIMTmIlIRqh27B6OjcgJ0BH1W +eigtOEqq2AN2wPkXByuoVDfvwcqWHdBMsLEDrJlOejC456eTvodc0JwSYrS3hLduh +efVnvyA+rnMyeUNXH3y1k8F7T8hjNOZSZQe5BwDfVTIpPl220GTmccQ5fqDr1zUQW +oQNrAQRsp5pZHziTECQNyRihwhjbaPcFbR/l6RJMJIntWe1ihrUWwg21/sIWQl7h0 +2TUi6nXdRfJHZ88Jg1zR5BTlXYUrXPBWg+ntbaNMCQNq5OyiZxeYG+N568SIrYfno +05zRzyg58IHZCAxcICOEwhwHlZ8cy0grjo371qRdB0MBQNoua1bYt5e0eV63qncCQ +LlbqYfYfsIzqsGbebu5F/4ZjzmQYQLYpTVMK29h/fGumnt8HdiH0zrphNkBI4NvZI +sZRNWaZA3D8R9wB+/QsrcCQQA2vJ5adck53MrRWrEX3QWC9kpm93bBWWagCEFkXnX +IjcPKIffGvvz8jbH6RGkd7w4PLbQeJfnE3S1s8MRi+NHXAkEAWYOtIWr0ZbYAIKGL +Hxk6eiJ7hKlARhYXzoTlf9LUFFRpvMuGo3S5BcB+WmtiPFXdUOyEHrS4IovqyzVod +auibQ== +-----END PRIVATE KEY----- diff --git a/chrome/test/data/extensions/good/extension1/1/manifest.json b/chrome/test/data/extensions/good/extension1/1/manifest.json index f8888c5..e520422 100644 --- a/chrome/test/data/extensions/good/extension1/1/manifest.json +++ b/chrome/test/data/extensions/good/extension1/1/manifest.json @@ -1,5 +1,6 @@ { - "id": "00123456789ABCDEF0123456789ABCDEF0123456", + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCsaVmPYFjlYABY7y8sEVB5i3/stDIXU8i3bBeQYeaXOg+Jr1vEhaSgKTy3zMY4m3+cnKI7Z/bMpYohS4pQTcwIXS1kEB6taCUUWgm2315zxSH+gUFRyfzBDlS8LZ9tRirFGj8o0R2YQobHgSPyf04Phq4AeMmGSygEJkfGO+Wt8wIDAQAB", + "signature":"kCUJQeO+FZ0hENaTT9bFW90HwiIppJEj/j88gOyXIL3z00ZOn/eck+M8O+HU7ad+U7BG5GK7xnuIvqzjAYK4ihfEYbExnbx3R79GWUWpHegELbp0PvhU9a+xIwTh48kzpKNgunzZGY0oONGvVeoNQXeTTO8z2i6RZfWD3/YdyBk=", "version": "1.0.0.0", "name": "My extension 1", "description": "The first extension that I made.", diff --git a/chrome/test/data/extensions/good/extension2/2/manifest.json b/chrome/test/data/extensions/good/extension2/2/manifest.json index 35f01c0..62eea65 100644 --- a/chrome/test/data/extensions/good/extension2/2/manifest.json +++ b/chrome/test/data/extensions/good/extension2/2/manifest.json @@ -1,5 +1,6 @@ { - "id": "10123456789ABCDEF0123456789ABCDEF0123456", + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC4RIIi/YbLZ6dRpdQnWfrQuo2vunkuPJpD9HNvZQ3J5aylSw7Y8ghzom793CbTJK1ZI254yZtkaWZJWOBhPKCaaRac+qfpRN1spl42vpyMn1OrGQm1pwZT6rDpCyIVIx/k2o4puMsQHNUIqxXPu3Oj+KSVdCIGOmabMhz765UjhwIDAQAB", + "signature":"kCUJQeO+FZ0hENaTT9bFW90HwiIppJEj/j88gOyXIL3z00ZOn/eck+M8O+HU7ad+U7BG5GK7xnuIvqzjAYK4ihfEYbExnbx3R79GWUWpHegELbp0PvhU9a+xIwTh48kzpKNgunzZGY0oONGvVeoNQXeTTO8z2i6RZfWD3/YdyBk=", "version": "1.0.0.0", "name": "My extension 2", "plugins": [ diff --git a/chrome/test/data/extensions/good/extension3/1.0/manifest.json b/chrome/test/data/extensions/good/extension3/1.0/manifest.json index 7927104..f162173 100644 --- a/chrome/test/data/extensions/good/extension3/1.0/manifest.json +++ b/chrome/test/data/extensions/good/extension3/1.0/manifest.json @@ -1,5 +1,6 @@ { - "id": "20123456789ABCDEF0123456789ABCDEF0123456", + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDl3hT1ZDijpPbk8wo8TxRGlQSU1zWvQFnY8XewiRMrEjTndGIGUGxXVg7KlBJOQ0DqK+yS8jeK1xsDRre7IzqoA491AnGRydsHXWwGCNHzwtzFjfVGChoqvXdsAoikBzeEv0+ZL1sHid++gLqICDlapcko0/TzrWe+F6bTA7n5oQIDAQAB", + "signature":"kCUJQeO+FZ0hENaTT9bFW90HwiIppJEj/j88gOyXIL3z00ZOn/eck+M8O+HU7ad+U7BG5GK7xnuIvqzjAYK4ihfEYbExnbx3R79GWUWpHegELbp0PvhU9a+xIwTh48kzpKNgunzZGY0oONGvVeoNQXeTTO8z2i6RZfWD3/YdyBk=", "version": "1.0", "name": "My extension 3" } diff --git a/chrome/test/data/extensions/good2.crx b/chrome/test/data/extensions/good2.crx Binary files differindex d475b5e..26a1dfa 100755 --- a/chrome/test/data/extensions/good2.crx +++ b/chrome/test/data/extensions/good2.crx diff --git a/chrome/test/data/extensions/page_action.crx b/chrome/test/data/extensions/page_action.crx Binary files differindex 8b4ec7c..2a397c8 100644 --- a/chrome/test/data/extensions/page_action.crx +++ b/chrome/test/data/extensions/page_action.crx diff --git a/chrome/test/data/extensions/samples/buildbot/manifest.json b/chrome/test/data/extensions/samples/buildbot/manifest.json index c584f8d..71e2326 100644 --- a/chrome/test/data/extensions/samples/buildbot/manifest.json +++ b/chrome/test/data/extensions/samples/buildbot/manifest.json @@ -1,6 +1,5 @@ { - "description": "Buildbot waterfall monitor", - "id": "7868865ECC233B0400ABE5D38D88355C03C4BB1A", + "description": "Buildbot waterfall monitor", "name": "buildbot", "toolstrips": [ "buildbot.html" diff --git a/chrome/test/data/extensions/samples/subscribe/manifest.json b/chrome/test/data/extensions/samples/subscribe/manifest.json index fa98e31..3ec3cd8 100644 --- a/chrome/test/data/extensions/samples/subscribe/manifest.json +++ b/chrome/test/data/extensions/samples/subscribe/manifest.json @@ -9,8 +9,7 @@ ] } ], - "description": "Adds one-click subscription with Google Reader to your toolbar", - "id": "8962A8B1A1063945CEDE08181EA28D2D65124E86", + "description": "Adds one-click subscription with Google Reader to your toolbar", "name": "Subscribe in Reader", "permissions": [ "http://www.google.com/*" diff --git a/chrome/test/data/extensions/theme.crx b/chrome/test/data/extensions/theme.crx Binary files differindex bed2237..e80391d 100644 --- a/chrome/test/data/extensions/theme.crx +++ b/chrome/test/data/extensions/theme.crx diff --git a/chrome/test/data/extensions/theme2.crx b/chrome/test/data/extensions/theme2.crx Binary files differindex 66fd919..313fc8d 100644 --- a/chrome/test/data/extensions/theme2.crx +++ b/chrome/test/data/extensions/theme2.crx diff --git a/chrome/test/data/extensions/theme_with_extension.crx b/chrome/test/data/extensions/theme_with_extension.crx Binary files differindex beb12be..bd0d312 100644 --- a/chrome/test/data/extensions/theme_with_extension.crx +++ b/chrome/test/data/extensions/theme_with_extension.crx diff --git a/chrome/test/data/extensions/theme_with_missing_image.crx b/chrome/test/data/extensions/theme_with_missing_image.crx Binary files differnew file mode 100755 index 0000000..8f9b355 --- /dev/null +++ b/chrome/test/data/extensions/theme_with_missing_image.crx diff --git a/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension1.json b/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension1.json index 1cf00e7..33dba7f 100644 --- a/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension1.json +++ b/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension1.json @@ -1,5 +1,5 @@ { - "id": "00123456789abcdef0123456789abcdef0123456", + "id": "fc6f6ba6693faf6773c13701019f2e7a12f0febe", "version": "1.0.0.0", "name": "My extension 1", "description": "The first extension that I made.", diff --git a/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension2.json b/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension2.json index 73d98ff..c6300a3 100644 --- a/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension2.json +++ b/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension2.json @@ -1,5 +1,5 @@ { - "id": "10123456789abcdef0123456789abcdef0123456", + "id": "e5ead92b2c6795c1d2b92df9c5cb37de5582471a", "version": "1.0.0.0", "name": "My extension 2", "description": "", diff --git a/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension3.json b/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension3.json index 3fd8a09..1125f48 100644 --- a/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension3.json +++ b/chrome/test/data/extensions/ui/create_extension_detail_value_expected_output/good-extension3.json @@ -1,5 +1,5 @@ { - "id": "20123456789abcdef0123456789abcdef0123456", + "id": "a37fed892f622823f4daaec4426a32fc7f6147dc", "version": "1.0", "name": "My extension 3", "description": "", diff --git a/chrome/test/data/extensions/uitest/roundtrip_api_call/manifest.json b/chrome/test/data/extensions/uitest/roundtrip_api_call/manifest.json index b071780..5acde49 100644 --- a/chrome/test/data/extensions/uitest/roundtrip_api_call/manifest.json +++ b/chrome/test/data/extensions/uitest/roundtrip_api_call/manifest.json @@ -1,5 +1,5 @@ { - "id": "66664444789ABCDEF0123456789ABCDEF0123456", + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC4RIIi/YbLZ6dRpdQnWfrQuo2vunkuPJpD9HNvZQ3J5aylSw7Y8ghzom793CbTJK1ZI254yZtkaWZJWOBhPKCaaRac+qfpRN1spl42vpyMn1OrGQm1pwZT6rDpCyIVIx/k2o4puMsQHNUIqxXPu3Oj+KSVdCIGOmabMhz765UjhwIDAQAB", "version": "1.0.0.0", "name": "Roundtrip ApiCall Test Extension", "description": "An extension for an extension UITest." diff --git a/chrome/test/data/extensions/uitest/simple_api_call/manifest.json b/chrome/test/data/extensions/uitest/simple_api_call/manifest.json index b883fdf..b302c0e 100644 --- a/chrome/test/data/extensions/uitest/simple_api_call/manifest.json +++ b/chrome/test/data/extensions/uitest/simple_api_call/manifest.json @@ -1,5 +1,5 @@ { - "id": "77774444789ABCDEF0123456789ABCDEF0123456", + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCsaVmPYFjlYABY7y8sEVB5i3/stDIXU8i3bBeQYeaXOg+Jr1vEhaSgKTy3zMY4m3+cnKI7Z/bMpYohS4pQTcwIXS1kEB6taCUUWgm2315zxSH+gUFRyfzBDlS8LZ9tRirFGj8o0R2YQobHgSPyf04Phq4AeMmGSygEJkfGO+Wt8wIDAQAB", "version": "1.0.0.0", "name": "ApiCall Test Extension", "description": "An extension for an extension UITest." |