summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
Diffstat (limited to 'chrome')
-rw-r--r--chrome/app/generated_resources.grd6
-rw-r--r--chrome/browser/extensions/extension_creator.cc2
-rw-r--r--chrome/browser/extensions/extension_service_unittest.cc43
-rw-r--r--chrome/common/extensions/extension.cc6
-rw-r--r--chrome/common/extensions/extension.h6
-rw-r--r--chrome/common/extensions/extension_file_util.cc63
-rw-r--r--chrome/common/extensions/extension_file_util.h12
-rw-r--r--chrome/common/extensions/extension_file_util_unittest.cc101
-rw-r--r--chrome/common/extensions/extension_unittest.cc5
-rw-r--r--chrome/common/extensions/extension_unpacker.cc5
10 files changed, 239 insertions, 10 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index b60516c..3814fff 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -4483,6 +4483,12 @@ Update checks have repeatedly failed for the extension "<ph name="EXTENSION_NAME
<message name="IDS_EXTENSION_PACKAGE_INSTALL_ERROR" desc="Error message in case package fails to install because of some problem with the filesystem.">
Could not install package: '<ph name="ERROR_CODE">$1<ex>error</ex></ph>'
</message>
+ <message name="IDS_EXTENSION_CONTAINS_PRIVATE_KEY" desc="Error message when an extension includes a file containing a private key.">
+ This extension includes the key file '<ph name="KEY_PATH">$1<ex>relative/path/to/file.pem</ex></ph>'. You probably don't want to do that.
+ </message>
+ <message name="IDS_EXTENSION_CANNOT_INSTALL_OWN_PRIVATE_KEY" desc="Error message when a user tries to install an extension that contains its own private key.">
+ Cannot install extension that contains its own private key: <ph name="KEY_PATH">$1<ex>relative/path/to/file.pem</ex></ph>
+ </message>
<!-- Extension installed bubble -->
<message name="IDS_EXTENSION_BUNDLE_INSTALLED_HEADING_EXTENSIONS" desc="First line in the content area of the extension bundle installed bubble. Instructs which extensions were installed.">
diff --git a/chrome/browser/extensions/extension_creator.cc b/chrome/browser/extensions/extension_creator.cc
index ea03d3a..61e0134 100644
--- a/chrome/browser/extensions/extension_creator.cc
+++ b/chrome/browser/extensions/extension_creator.cc
@@ -107,7 +107,7 @@ bool ExtensionCreator::ValidateManifest(const FilePath& extension_dir,
extension_dir,
extension_id,
Extension::INTERNAL,
- Extension::FOLLOW_SYMLINKS_ANYWHERE,
+ Extension::FOLLOW_SYMLINKS_ANYWHERE | Extension::ERROR_ON_PRIVATE_KEY,
&error_message_));
return !!extension.get();
}
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 2602c11..6d2bc0e0 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -1750,6 +1750,49 @@ TEST_F(ExtensionServiceTest, PackPunctuatedExtension) {
}
}
+TEST_F(ExtensionServiceTest, PackExtensionContainingKeyFails) {
+ InitializeEmptyExtensionService();
+
+ ScopedTempDir extension_temp_dir;
+ ASSERT_TRUE(extension_temp_dir.CreateUniqueTempDir());
+ FilePath input_directory = extension_temp_dir.path().AppendASCII("ext");
+ ASSERT_TRUE(file_util::CopyDirectory(
+ data_dir_
+ .AppendASCII("good")
+ .AppendASCII("Extensions")
+ .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
+ .AppendASCII("1.0.0.0"),
+ input_directory,
+ /*recursive=*/true));
+
+ ScopedTempDir output_temp_dir;
+ ASSERT_TRUE(output_temp_dir.CreateUniqueTempDir());
+ FilePath output_directory = output_temp_dir.path();
+
+ FilePath crx_path(output_directory.AppendASCII("ex1.crx"));
+ FilePath privkey_path(output_directory.AppendASCII("privkey.pem"));
+
+ // Pack the extension once to get a private key.
+ scoped_ptr<ExtensionCreator> creator(new ExtensionCreator());
+ ASSERT_TRUE(creator->Run(input_directory, crx_path, FilePath(),
+ privkey_path, ExtensionCreator::kNoRunFlags))
+ << creator->error_message();
+ ASSERT_TRUE(file_util::PathExists(crx_path));
+ ASSERT_TRUE(file_util::PathExists(privkey_path));
+
+ file_util::Delete(crx_path, false);
+ // Move the pem file into the extension.
+ file_util::Move(privkey_path,
+ input_directory.AppendASCII("privkey.pem"));
+
+ // This pack should fail because of the contained private key.
+ EXPECT_FALSE(creator->Run(input_directory, crx_path, FilePath(),
+ privkey_path, ExtensionCreator::kNoRunFlags));
+ EXPECT_THAT(creator->error_message(),
+ testing::ContainsRegex(
+ "extension includes the key file.*privkey.pem"));
+}
+
// Test Packaging and installing an extension using an openssl generated key.
// The openssl is generated with the following:
// > openssl genrsa -out privkey.pem 1024
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 29f6827..82816c9 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -400,6 +400,12 @@ const std::string Extension::VersionString() const {
return version()->GetString();
}
+void Extension::AddInstallWarnings(
+ const std::vector<std::string>& new_warnings) {
+ install_warnings_.insert(install_warnings_.end(),
+ new_warnings.begin(), new_warnings.end());
+}
+
// static
bool Extension::IsExtension(const FilePath& file_name) {
return file_name.MatchesExtension(chrome::kExtensionFileExtension);
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 40311440..665043d 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -213,6 +213,10 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
// anywhere in the filesystem, rather than being restricted to the
// extension directory.
FOLLOW_SYMLINKS_ANYWHERE = 1 << 5,
+
+ // |ERROR_ON_PRIVATE_KEY| means that private keys inside an
+ // extension should be errors rather than warnings.
+ ERROR_ON_PRIVATE_KEY = 1 << 6,
};
static scoped_refptr<Extension> Create(const FilePath& path,
@@ -592,6 +596,8 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
const ExtensionPermissionSet* required_permission_set() const {
return required_permission_set_.get();
}
+ // Appends |new_warnings| to install_warnings().
+ void AddInstallWarnings(const std::vector<std::string>& new_warnings);
const std::vector<std::string>& install_warnings() const {
return install_warnings_;
}
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index ec72db7..3a024df 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -16,6 +16,7 @@
#include "base/stringprintf.h"
#include "base/threading/thread_restrictions.h"
#include "base/utf_string_conversions.h"
+#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_action.h"
@@ -151,8 +152,10 @@ scoped_refptr<Extension> LoadExtension(const FilePath& extension_path,
if (!extension.get())
return NULL;
- if (!ValidateExtension(extension.get(), error))
+ std::vector<std::string> warnings;
+ if (!ValidateExtension(extension.get(), error, &warnings))
return NULL;
+ extension->AddInstallWarnings(warnings);
return extension;
}
@@ -191,8 +194,43 @@ DictionaryValue* LoadManifest(const FilePath& extension_path,
return static_cast<DictionaryValue*>(root.release());
}
+std::vector<FilePath> FindPrivateKeyFiles(const FilePath& extension_dir) {
+ std::vector<FilePath> result;
+ // Pattern matching only works at the root level, so filter manually.
+ file_util::FileEnumerator traversal(extension_dir, /*recursive=*/true,
+ file_util::FileEnumerator::FILES);
+ for (FilePath current = traversal.Next(); !current.empty();
+ current = traversal.Next()) {
+ if (!current.MatchesExtension(chrome::kExtensionKeyFileExtension))
+ continue;
+
+ std::string key_contents;
+ if (!file_util::ReadFileToString(current, &key_contents)) {
+ // If we can't read the file, assume it's not a private key.
+ continue;
+ }
+ std::string key_bytes;
+ if (!Extension::ParsePEMKeyBytes(key_contents, &key_bytes)) {
+ // If we can't parse the key, assume it's ok too.
+ continue;
+ }
+
+ // TODO(jyasskin): Use crypto::RSAPrivateKey to get the public key
+ // out of the key file, so the caller can check whether the found
+ // private key is the same one used to sign this extension. This
+ // requires refactoring the RSAPrivateKey implementation a bit,
+ // since at least on Mac, CSSMInitSingleton accesses some files
+ // that aren't available in the utility process where this check
+ // needs to run.
+
+ result.push_back(current);
+ }
+ return result;
+}
+
bool ValidateExtension(const Extension* extension,
- std::string* error) {
+ std::string* error,
+ std::vector<std::string>* warnings) {
// Validate icons exist.
for (ExtensionIconSet::IconMap::const_iterator iter =
extension->icons().map().begin();
@@ -365,6 +403,27 @@ bool ValidateExtension(const Extension* extension,
return false;
}
+ // Check that extensions don't include private key files.
+ // TODO(jyasskin): When ERROR_ON_PRIVATE_KEY is not set, still block
+ // installing an extension that contains its own private key.
+ std::vector<FilePath> private_keys = FindPrivateKeyFiles(extension->path());
+ if (extension->creation_flags() & Extension::ERROR_ON_PRIVATE_KEY) {
+ if (!private_keys.empty()) {
+ // Only print one of the private keys because l10n_util doesn't have a way
+ // to translate a list of strings.
+ *error = l10n_util::GetStringFUTF8(
+ IDS_EXTENSION_CONTAINS_PRIVATE_KEY,
+ private_keys.front().LossyDisplayName());
+ return false;
+ }
+ } else {
+ for (size_t i = 0; i < private_keys.size(); ++i) {
+ warnings->push_back(l10n_util::GetStringFUTF8(
+ IDS_EXTENSION_CONTAINS_PRIVATE_KEY,
+ private_keys[i].LossyDisplayName()));
+ }
+ // Only warn; don't block loading the extension.
+ }
return true;
}
diff --git a/chrome/common/extensions/extension_file_util.h b/chrome/common/extensions/extension_file_util.h
index 96f38fb..0c896d6 100644
--- a/chrome/common/extensions/extension_file_util.h
+++ b/chrome/common/extensions/extension_file_util.h
@@ -60,9 +60,17 @@ base::DictionaryValue* LoadManifest(const FilePath& extension_root,
std::string* error);
// Returns true if the given extension object is valid and consistent.
-// Otherwise, a description of the error is returned in |error|.
+// May also append a series of warning messages to |warnings|, but they
+// should not prevent the extension from running.
+//
+// Otherwise, returns false, and a description of the error is
+// returned in |error|.
bool ValidateExtension(const extensions::Extension* extension,
- std::string* error);
+ std::string* error,
+ std::vector<std::string>* warnings);
+
+// Returns a list of files that contain private keys inside |extension_dir|.
+std::vector<FilePath> FindPrivateKeyFiles(const FilePath& extension_dir);
// Cleans up the extension install directory. It can end up with garbage in it
// if extensions can't initially be removed when they are uninstalled (eg if a
diff --git a/chrome/common/extensions/extension_file_util_unittest.cc b/chrome/common/extensions/extension_file_util_unittest.cc
index e713b58..f6769ba 100644
--- a/chrome/common/extensions/extension_file_util_unittest.cc
+++ b/chrome/common/extensions/extension_file_util_unittest.cc
@@ -14,6 +14,7 @@
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_manifest_constants.h"
#include "grit/generated_resources.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h"
@@ -382,8 +383,11 @@ TEST(ExtensionFileUtil, ValidateThemeUTF8) {
kManifest, temp.path(), Extension::LOAD, 0, &error);
ASSERT_TRUE(extension.get()) << error;
- EXPECT_TRUE(extension_file_util::ValidateExtension(extension, &error)) <<
+ std::vector<std::string> warnings;
+ EXPECT_TRUE(extension_file_util::ValidateExtension(extension,
+ &error, &warnings)) <<
error;
+ EXPECT_EQ(0U, warnings.size());
}
#if defined(OS_WIN)
@@ -406,14 +410,17 @@ TEST(ExtensionFileUtil, MAYBE_BackgroundScriptsMustExist) {
value->Set("background.scripts", scripts);
std::string error;
+ std::vector<std::string> warnings;
scoped_refptr<Extension> extension = LoadExtensionManifest(
value.get(), temp.path(), Extension::LOAD, 0, &error);
ASSERT_TRUE(extension.get()) << error;
- EXPECT_FALSE(extension_file_util::ValidateExtension(extension, &error));
+ EXPECT_FALSE(extension_file_util::ValidateExtension(extension,
+ &error, &warnings));
EXPECT_EQ(l10n_util::GetStringFUTF8(
IDS_EXTENSION_LOAD_BACKGROUND_SCRIPT_FAILED, ASCIIToUTF16("foo.js")),
error);
+ EXPECT_EQ(0U, warnings.size());
scripts->Clear();
scripts->Append(Value::CreateStringValue("http://google.com/foo.js"));
@@ -422,11 +429,99 @@ TEST(ExtensionFileUtil, MAYBE_BackgroundScriptsMustExist) {
0, &error);
ASSERT_TRUE(extension.get()) << error;
- EXPECT_FALSE(extension_file_util::ValidateExtension(extension, &error));
+ warnings.clear();
+ EXPECT_FALSE(extension_file_util::ValidateExtension(extension,
+ &error, &warnings));
EXPECT_EQ(l10n_util::GetStringFUTF8(
IDS_EXTENSION_LOAD_BACKGROUND_SCRIPT_FAILED,
ASCIIToUTF16("http://google.com/foo.js")),
error);
+ EXPECT_EQ(0U, warnings.size());
+}
+
+// Private key, generated by Chrome specifically for this test, and
+// never used elsewhere.
+const char private_key[] =
+ "-----BEGIN PRIVATE KEY-----\n"
+ "MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKt02SR0FYaYy6fpW\n"
+ "MAA+kU1BgK3d+OmmWfdr+JATIjhRkyeSF4lTd/71JQsyKqPzYkQPi3EeROWM+goTv\n"
+ "EhJqq07q63BolpsFmlV+S4ny+sBA2B4aWwRYXlBWikdrQSA0mJMzvEHc6nKzBgXik\n"
+ "QSVbyyBNAsxlDB9WaCxRVOpK3AgMBAAECgYBGvSPlrVtAOAQ2V8j9FqorKZA8SLPX\n"
+ "IeJC/yzU3RB2nPMjI17aMOvrUHxJUhzMeh4jwabVvSzzDtKFozPGupW3xaI8sQdi2\n"
+ "WWMTQIk/Q9HHDWoQ9qA6SwX2qWCc5SyjCKqVp78ye+000kqTJYjBsDgXeAlzKcx2B\n"
+ "4GAAeWonDdkQJBANNb8wrqNWFn7DqyQTfELzcRTRnqQ/r1pdeJo6obzbnwGnlqe3t\n"
+ "KhLjtJNIGrQg5iC0OVLWFuvPJs0t3z62A1ckCQQDPq2JZuwTwu5Pl4DJ0r9O1FdqN\n"
+ "JgqPZyMptokCDQ3khLLGakIu+TqB9YtrzI69rJMSG2Egb+6McaDX+dh3XmR/AkB9t\n"
+ "xJf6qDnmA2td/tMtTc0NOk8Qdg/fD8xbZ/YfYMnVoYYs9pQoilBaWRePDRNURMLYZ\n"
+ "vHAI0Llmw7tj7jv17pAkEAz44uXRpjRKtllUIvi5pUENAHwDz+HvdpGH68jpU3hmb\n"
+ "uOwrmnQYxaMReFV68Z2w9DcLZn07f7/R9Wn72z89CxwJAFsDoNaDes4h48bX7plct\n"
+ "s9ACjmTwcCigZjN2K7AGv7ntCLF3DnV5dK0dTHNaAdD3SbY3jl29Rk2CwiURSX6Ee\n"
+ "g==\n"
+ "-----END PRIVATE KEY-----\n";
+
+TEST(ExtensionFileUtil, FindPrivateKeyFiles) {
+ ScopedTempDir temp;
+ ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+ FilePath src_path = temp.path().AppendASCII("some_dir");
+ ASSERT_TRUE(file_util::CreateDirectory(src_path));
+
+ ASSERT_TRUE(file_util::WriteFile(src_path.AppendASCII("a_key.pem"),
+ private_key, arraysize(private_key)));
+ ASSERT_TRUE(file_util::WriteFile(src_path.AppendASCII("second_key.pem"),
+ private_key, arraysize(private_key)));
+ // Shouldn't find a key with a different extension.
+ ASSERT_TRUE(file_util::WriteFile(src_path.AppendASCII("key.diff_ext"),
+ private_key, arraysize(private_key)));
+ // Shouldn't find a key that isn't parsable.
+ ASSERT_TRUE(file_util::WriteFile(src_path.AppendASCII("unparsable_key.pem"),
+ private_key, arraysize(private_key) - 30));
+ std::vector<FilePath> private_keys =
+ extension_file_util::FindPrivateKeyFiles(temp.path());
+ EXPECT_EQ(2U, private_keys.size());
+ EXPECT_THAT(private_keys,
+ testing::Contains(src_path.AppendASCII("a_key.pem")));
+ EXPECT_THAT(private_keys,
+ testing::Contains(src_path.AppendASCII("second_key.pem")));
+}
+
+TEST(ExtensionFileUtil, WarnOnPrivateKey) {
+ ScopedTempDir temp;
+ ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+ FilePath ext_path = temp.path().AppendASCII("ext_root");
+ ASSERT_TRUE(file_util::CreateDirectory(ext_path));
+
+ const char manifest[] =
+ "{\n"
+ " \"name\": \"Test Extension\",\n"
+ " \"version\": \"1.0\",\n"
+ " \"manifest_version\": 2,\n"
+ " \"description\": \"The first extension that I made.\"\n"
+ "}\n";
+ ASSERT_TRUE(file_util::WriteFile(ext_path.AppendASCII("manifest.json"),
+ manifest, strlen(manifest)));
+ ASSERT_TRUE(file_util::WriteFile(ext_path.AppendASCII("a_key.pem"),
+ private_key, strlen(private_key)));
+
+ std::string error;
+ scoped_refptr<Extension> extension(extension_file_util::LoadExtension(
+ ext_path, "the_id", Extension::EXTERNAL_PREF,
+ Extension::NO_FLAGS, &error));
+ ASSERT_TRUE(extension.get()) << error;
+ EXPECT_THAT(extension->install_warnings(),
+ testing::ElementsAre(
+ testing::ContainsRegex(
+ "extension includes the key file.*ext_root.a_key.pem")));
+
+ // Turn the warning into an error with ERROR_ON_PRIVATE_KEY.
+ extension = extension_file_util::LoadExtension(
+ ext_path, "the_id", Extension::EXTERNAL_PREF,
+ Extension::ERROR_ON_PRIVATE_KEY, &error);
+ EXPECT_FALSE(extension.get());
+ EXPECT_THAT(error,
+ testing::ContainsRegex(
+ "extension includes the key file.*ext_root.a_key.pem"));
}
// TODO(aa): More tests as motivation allows. Maybe steal some from
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 5255c00..4c5f57a 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -189,7 +189,10 @@ TEST(ExtensionTest, GetAbsolutePathNoError) {
"absolute.json");
EXPECT_TRUE(extension.get());
std::string err;
- EXPECT_TRUE(extension_file_util::ValidateExtension(extension.get(), &err));
+ std::vector<std::string> warnings;
+ EXPECT_TRUE(extension_file_util::ValidateExtension(extension.get(),
+ &err, &warnings));
+ EXPECT_EQ(0U, warnings.size());
EXPECT_EQ(extension->path().AppendASCII("test.html").value(),
extension->GetResource("test.html").GetFilePath().value());
diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc
index 5194b4b..7915de1 100644
--- a/chrome/common/extensions/extension_unpacker.cc
+++ b/chrome/common/extensions/extension_unpacker.cc
@@ -190,10 +190,13 @@ bool ExtensionUnpacker::Run() {
return false;
}
- if (!extension_file_util::ValidateExtension(extension.get(), &error)) {
+ std::vector<std::string> warnings;
+ if (!extension_file_util::ValidateExtension(extension.get(),
+ &error, &warnings)) {
SetError(error);
return false;
}
+ extension->AddInstallWarnings(warnings);
// Decode any images that the browser needs to display.
std::set<FilePath> image_paths = extension->GetBrowserImages();
generated by cgit v1.1 at 2025-05-18 07:42:00 +0000