Force update checks to use SSL for store-hosted extensions/apps.

chrome/browser/extensions/updater/extension_downloader.cc

Index: chrome/browser/extensions/updater/extension_downloader.cc
diff --git a/chrome/browser/extensions/updater/extension_downloader.cc b/chrome/browser/extensions/updater/extension_downloader.cc
index c9aca17928425e3bcb25ace00eb0dbbef5c8e235..918731f488dbcef619357880ed1050be37f84de3 100644
--- a/chrome/browser/extensions/updater/extension_downloader.cc
+++ b/chrome/browser/extensions/updater/extension_downloader.cc
@@ -150,8 +150,13 @@ bool ExtensionDownloader::AddExtension(const Extension& extension) {
   if (!extension.UpdatesFromGallery())
     update_url_data = delegate_->GetUpdateUrlData(extension.id());
 
+  // Make sure we use SSL for store-hosted extensions.
+  GURL update_url = extension.update_url();
+  if (extension.UpdatesFromGallery() && !update_url.SchemeIsSecure())
+    update_url = extension_urls::GetWebstoreUpdateUrl();
+
   return AddExtensionData(extension.id(), *extension.version(),
-                          extension.GetType(), extension.update_url(),
+                          extension.GetType(), update_url,
                           update_url_data);
 }
 
@@ -187,7 +192,8 @@ void ExtensionDownloader::StartBlacklistUpdate(
   // url here to avoid DNS hijacking of the blacklist, which is not validated
   // by a public key signature like .crx files are.
   ManifestFetchData* blacklist_fetch =
-      new ManifestFetchData(extension_urls::GetWebstoreUpdateUrl(true));
+      new ManifestFetchData(extension_urls::GetWebstoreUpdateUrl());
+  DCHECK(blacklist_fetch->base_url().SchemeIsSecure());
   blacklist_fetch->AddExtension(kBlacklistAppID, version, &ping_data, "",
                                 kDefaultInstallSource);
   StartUpdateCheck(blacklist_fetch);
@@ -205,6 +211,15 @@ bool ExtensionDownloader::AddExtensionData(const std::string& id,
     return false;
   }
 
+  // Double-check that we're using https for webstore urls.
+  if (extension_urls::IsWebstoreUpdateUrl(update_url) &&
+      !update_url.SchemeIsSecure() &&
+      extension_urls::GetWebstoreUpdateUrl().SchemeIsSecure()) {
+    NOTREACHED() << "Refusing to send non-secure update check for " << id
+                 << " (" << update_url.spec() << ")";
+    return false;
+  }
+
   // Skip extensions with empty IDs.
   if (id.empty()) {
     LOG(WARNING) << "Found extension with empty ID";
@@ -216,9 +231,7 @@ bool ExtensionDownloader::AddExtensionData(const std::string& id,
   } else if (update_url.is_empty()) {
     url_stats_.no_url_count++;
     // Fill in default update URL.
-    //
-    // TODO(akalin): Figure out if we should use the HTTPS version.
-    update_url = extension_urls::GetWebstoreUpdateUrl(false);
+    update_url = extension_urls::GetWebstoreUpdateUrl();
   } else {
     url_stats_.other_url_count++;
   }
@@ -247,7 +260,7 @@ bool ExtensionDownloader::AddExtensionData(const std::string& id,
   // webstore update URL.
   if (!extension_urls::IsWebstoreUpdateUrl(update_url) &&
       MetricsServiceHelper::IsMetricsReportingEnabled()) {
-    update_urls.push_back(extension_urls::GetWebstoreUpdateUrl(false));
+    update_urls.push_back(extension_urls::GetWebstoreUpdateUrl());
   }
 
   for (size_t i = 0; i < update_urls.size(); ++i) {
@@ -432,17 +445,27 @@ void ExtensionDownloader::HandleManifestResults(
     const UpdateManifest::Result* update = &(results->list.at(updates[i]));
     const std::string& id = update->extension_id;
     not_updated.erase(id);
+
+    GURL crx_url = update->crx_url;
     if (id != kBlacklistAppID) {
       NotifyUpdateFound(update->extension_id);
     } else {
       // The URL of the blacklist file is returned by the server and we need to
       // be sure that we continue to be able to reliably detect whether a URL
       // references a blacklist file.
-      DCHECK(extension_urls::IsBlacklistUpdateUrl(update->crx_url))
-          << update->crx_url;
+      DCHECK(extension_urls::IsBlacklistUpdateUrl(crx_url)) << crx_url;
+
+      // Force https (crbug.com/129587).
+      if (!crx_url.SchemeIsSecure()) {

[Aaron Boodman, 2012/05/24 23:17:41]

Will this affect non-store updates?

[asargent_no_longer_on_chrome, 2012/05/24 23:23:53]

No, this is in the else block for the "if (id != kBlacklistAppID) {" check
above. This is handling the results for the blacklist contents - we already
forced the update check for that to be SSL, but this is just ensuring that even
if the server said to use plain http to fetch the blacklist contents, we
override that. A patch is going into the server to make sure they always send a
response with an SSL url for the blacklist. So this is just defense in depth.
(Note that we already ensure that the blacklist contents match a hash the server
told us about in the response to the SSL update check)

+        url_canon::Replacements<char> replacements;
+        std::string scheme("https");
+        replacements.SetScheme(scheme.c_str(),
+                               url_parse::Component(0, scheme.size()));
+        crx_url = crx_url.ReplaceComponents(replacements);
+      }
     }
-    FetchUpdatedExtension(update->extension_id, update->crx_url,
-                          update->package_hash, update->version);
+    FetchUpdatedExtension(update->extension_id, crx_url, update->package_hash,
+                          update->version);
   }
 
   // If the manifest response included a <daystart> element, we want to save