Fix a bug where packing an extension with bad private key causes crash.

BUG=263968
TEST=PackExtensionTest.ExtensionWithInvalidKey

Review URL: https://chromiumcodereview.appspot.com/20794003

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

7 files changed, 38 insertions, 3 deletions

diff --git a/chrome/browser/extensions/extension_creator.cc b/chrome/browser/extensions/extension_creator.cc
index e344338..285aa5c 100644
--- a/chrome/browser/extensions/extension_creator.cc
+++ b/chrome/browser/extensions/extension_creator.cc
@@ -228,7 +228,11 @@ bool ExtensionCreator::SignZip(const base::FilePath& zip_path,
   }
   zip_handle.Close();
 
-  signature_creator->Final(signature);
+  if (!signature_creator->Final(signature)) {
+    error_message_ =
+        l10n_util::GetStringUTF8(IDS_EXTENSION_ERROR_WHILE_SIGNING);
+    return false;
+  }
   return true;
 }
 
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index e8f452b..86dd4f1 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -2202,6 +2202,14 @@ TEST_F(ExtensionServiceTest, PackExtension) {
   creator.reset(new ExtensionCreator());
   ASSERT_FALSE(creator->Run(temp_dir2.path(), crx_path, privkey_path,
                             base::FilePath(), ExtensionCreator::kOverwriteCRX));
+
+  // Try packing with a private key that is a valid key, but invalid for the
+  // extension.
+  base::FilePath bad_private_key_dir = data_dir_.AppendASCII("bad_private_key");
+  crx_path = output_directory.AppendASCII("bad_private_key.crx");
+  privkey_path = data_dir_.AppendASCII("bad_private_key.pem");
+  ASSERT_FALSE(creator->Run(bad_private_key_dir, crx_path, base::FilePath(),
+      privkey_path, ExtensionCreator::kOverwriteCRX));
 }
 
 // Test Packaging and installing an extension whose name contains punctuation.
diff --git a/chrome/browser/extensions/pack_extension_unittest.cc b/chrome/browser/extensions/pack_extension_unittest.cc
index dac4608..78bcad3 100644
--- a/chrome/browser/extensions/pack_extension_unittest.cc
+++ b/chrome/browser/extensions/pack_extension_unittest.cc
@@ -17,7 +17,7 @@
 namespace extensions {
 
 // Tests the environment for packing extensions from the command line
-// via the --pack-extension switch.
+// when using the --pack-extension switch.
 class PackExtensionTest : public testing::Test {
  public:
   PackExtensionTest()
diff --git a/chrome/test/data/extensions/bad_private_key.crx b/chrome/test/data/extensions/bad_private_key.crx
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/chrome/test/data/extensions/bad_private_key.crx
diff --git a/chrome/test/data/extensions/bad_private_key.pem b/chrome/test/data/extensions/bad_private_key.pem
new file mode 100644
index 0000000..fbd9fbf
--- /dev/null
+++ b/chrome/test/data/extensions/bad_private_key.pem
@@ -0,0 +1,16 @@
+-----BEGIN PRIVATE KEY-----
+MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAOJDy/d7q0o86928n
+HCOH5TZlC+cpyEFB4G6NMqD9hwVfGdAdnvZfAFf5et+T6+Iv4ezR/F5pT6HGKXD0j
+x9WDcC+LYU2WISkLfm18q6CXaqA9s8B+TrWol0Q3SDwsuRGEwIdZjUPUf63N5nSxr
+QAJrtg+V2bds2vwVIyPhROyenAgMBAAECgYAIOGhXpHGTr+Tt2GVinXc+RC1jEXDs
+/S7Josc6OFMfsBQl+CyV0U/1CNPkm2Evwmsa9gN+HWpIfEYD97pSokcrHvx/KcLhl
+AAiDe/9Cp+Qvy5sG5QjthKenucs2YuUq5q9tV3PJ8iIDPnHvZ8PbvKfriiLYGiAwc
+Xhg0jSQUu/cQJBAPgvRA1z7PhRDArmgBpRP9t5zafaDpiAEW3TPmBJoRUZBpVrhp2
+DSF7t18c6zlh5yzkYrqobu3Y+pNxUvruwGM8CQQDpY9MygZcyyzk8z7+JnPjnW5MC
+JKrBj6tczA7ss09X07Fiv5zZHLe7j+cv1iArLstR6IW4qHFvlFgggq6NDImpAkEAg
+wY5ED0eYu4uxGZyRPKWGuGQIEhvojUifm+io6TbhM7keef9dpTDlVJNs6GpJG6QSo
+FsXOQtMLzbvnKDC4VpYwJAYCF/8kOydXeDgerIJ/SCCTmM2cskkkmM+dILK6/HtJM
+7c0MdZcgQXa8uOGPoKCFEGd6+hYrL/nuJuQpavzo00QJAGjdtxh+IHjdHu02tNIoF
+56UxUy/+TNEC6vnHInpag4x5JHylfZK8iv1hS7skEf0RBbW9DNHmTo0D9/w/mqh28
+w=-
+-----END PRIVATE KEY-----
diff --git a/chrome/test/data/extensions/bad_private_key/manifest.json b/chrome/test/data/extensions/bad_private_key/manifest.json
new file mode 100644
index 0000000..87833c9
--- /dev/null
+++ b/chrome/test/data/extensions/bad_private_key/manifest.json
@@ -0,0 +1,8 @@
+{
+  "manifest_version": 2,
+
+  "name": "Bad extension",
+  "description": "Bad bad!",
+  "version": "1.0"
+
+}
diff --git a/crypto/signature_creator_nss.cc b/crypto/signature_creator_nss.cc
index 6c21efe..5a9ad2e 100644
--- a/crypto/signature_creator_nss.cc
+++ b/crypto/signature_creator_nss.cc
@@ -84,7 +84,6 @@ bool SignatureCreator::Final(std::vector<uint8>* signature) {
   SECItem signature_item;
   SECStatus rv = SGN_End(sign_context_, &signature_item);
   if (rv != SECSuccess) {
-    NOTREACHED();
     return false;
   }
   signature->assign(signature_item.data,