Patch 1 (main patch) - Bug 583408 - Notify user when the certificate attribute check fails. r=dtownsend, a=blocking2.0-beta6
authorRobert Strong <robert.bugzilla@gmail.com>
Wed, 01 Sep 2010 16:27:07 -0700
changeset 51863 c9ee9a98f2d4d4db468d593972e985b6e92634a6
parent 51862 556f27f56d1438d6cb6726a26a90c3f202f7c0df
child 51864 3c3c95c37d204897dfef2e050368f2117ea9b097
push id15454
push userrstrong@mozilla.com
push dateWed, 01 Sep 2010 23:27:33 +0000
treeherdermozilla-central@3c3c95c37d20 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdtownsend, blocking2.0-beta6
bugs583408
milestone2.0b6pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Patch 1 (main patch) - Bug 583408 - Notify user when the certificate attribute check fails. r=dtownsend, a=blocking2.0-beta6
browser/app/profile/firefox.js
toolkit/mozapps/shared/CertUtils.jsm
toolkit/mozapps/shared/test/chrome/test_bug544442_checkCert.xul
toolkit/mozapps/update/content/updates.js
toolkit/mozapps/update/content/updates.xul
toolkit/mozapps/update/nsIUpdateService.idl
toolkit/mozapps/update/nsUpdateService.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -85,16 +85,33 @@ pref("app.update.timer", 600000);
 // App-specific update preferences
 
 // The interval to check for updates (app.update.interval) is defined in
 // firefox-branding.js
 
 // Enables some extra Application Update Logging (can reduce performance)
 pref("app.update.log", false);
 
+// When |app.update.cert.requireBuiltIn| is true or not specified the
+// final certificate and all certificates the connection is redirected to before
+// the final certificate for the url specified in the |app.update.url|
+// preference must be built-in.
+pref("app.update.cert.requireBuiltIn", true);
+
+// When |app.update.cert.checkAttributes| is true or not specified the
+// certificate attributes specified in the |app.update.certs.| preference branch
+// are checked against the certificate for the url specified by the
+// |app.update.url| preference.
+pref("app.update.cert.checkAttributes", true);
+
+// The number of certificate attribute check failures to allow for background
+// update checks before notifying the user of the failure. User initiated update
+// checks always notify the user of the certificate attribute check failure.
+pref("app.update.cert.maxErrors", 5);
+
 // The |app.update.certs.| preference branch contains branches that are
 // sequentially numbered starting at 1 that contain attribute name / value
 // pairs for the certificate used by the server that hosts the update xml file
 // as specified in the |app.update.url| preference. When these preferences are
 // present the following conditions apply for a successful update check:
 // 1. the uri scheme must be https
 // 2. the preference name must exist as an attribute name on the certificate and
 //    the value for the name must be the same as the value for the attribute name
--- a/toolkit/mozapps/shared/CertUtils.jsm
+++ b/toolkit/mozapps/shared/CertUtils.jsm
@@ -49,29 +49,32 @@ const Cu = Components.utils;
 /**
  * Checks if the connection must be HTTPS and if so, only allows built-in
  * certificates and validates application specified certificate attribute
  * values.
  * See bug 340198 and bug 544442.
  *
  * @param  aChannel
  *         The nsIChannel that will have its certificate checked.
- * @param  aCerts
+ * @param  aAllowNonBuiltInCerts (optional)
+ *         When true certificates that aren't builtin are allowed. When false
+ *         or not specified the certificate must be a builtin certificate.
+ * @param  aCerts (optional)
  *         An array of JS objects with names / values corresponding to the
- *         channel's expected certificate's attribute names / values. This can
- *         be null or an empty array. If it isn't null the the scheme for the
- *         channel's originalURI must be https.
+ *         channel's expected certificate's attribute names / values. If it
+ *         isn't null or not specified the the scheme for the channel's
+ *         originalURI must be https.
  * @throws NS_ERROR_UNEXPECTED if a certificate is expected and the URI scheme
  *         is not https.
  *         NS_ERROR_ILLEGAL_VALUE if a certificate attribute name from the
  *         cert param does not exist or the value for a certificate attribute
  *         from the aCerts  param is different than the expected value.
  *         NS_ERROR_ABORT if the certificate issuer is not built-in.
  */
-function checkCert(aChannel, aCerts) {
+function checkCert(aChannel, aAllowNonBuiltInCerts, aCerts) {
   if (!aChannel.originalURI.schemeIs("https")) {
     // Require https if there are certificate values to verify
     if (aCerts) {
       throw new Ce("SSL is required and URI scheme is not https.",
                    Cr.NS_ERROR_UNEXPECTED);
     }
     return;
   }
@@ -107,16 +110,18 @@ function checkCert(aChannel, aCerts) {
     if (error) {
       const certCheckErr = "Certificate checks failed. See previous errors " +
                            "for details.";
       Cu.reportError(certCheckErr);
       throw new Ce(certCheckErr, Cr.NS_ERROR_ILLEGAL_VALUE);
     }
   }
 
+  if (aAllowNonBuiltInCerts ===  true)
+    return;
 
   var issuerCert = cert;
   while (issuerCert.issuer && !issuerCert.issuer.equals(issuerCert))
     issuerCert = issuerCert.issuer;
 
   const certNotBuiltInErr = "Certificate issuer is not built-in.";
   if (!issuerCert)
     throw new Ce(certNotBuiltInErr, Cr.NS_ERROR_ABORT);
@@ -131,16 +136,20 @@ function checkCert(aChannel, aCerts) {
 function isBuiltinToken(tokenName) {
   return tokenName == "Builtin Object Token";
 }
 
 /**
  * This class implements nsIBadCertListener.  Its job is to prevent "bad cert"
  * security dialogs from being shown to the user.  It is better to simply fail
  * if the certificate is bad. See bug 304286.
+ *
+ * @param  aAllowNonBuiltInCerts (optional)
+ *         When true certificates that aren't builtin are allowed. When false
+ *         or not specified the certificate must be a builtin certificate.
  */
 function BadCertHandler(aAllowNonBuiltInCerts) {
   this.allowNonBuiltInCerts = aAllowNonBuiltInCerts;
 }
 BadCertHandler.prototype = {
 
   // nsIChannelEventSink
   asyncOnChannelRedirect: function(oldChannel, newChannel, flags, callback) {
--- a/toolkit/mozapps/shared/test/chrome/test_bug544442_checkCert.xul
+++ b/toolkit/mozapps/shared/test/chrome/test_bug544442_checkCert.xul
@@ -53,82 +53,97 @@ function testXHRError(aEvent) {
   if (status == 0)
     status = request.channel.QueryInterface(Ci.nsIRequest).status;
 
   ok(false, "XHR onerror called: " + status);
 
   SimpleTest.finish();
 }
 
-function getCheckCertResult(aChannel, aCerts) {
+function getCheckCertResult(aChannel, aAllowNonBuiltIn, aCerts) {
   try {
-    checkCert(aChannel, aCerts);
+    checkCert(aChannel, aAllowNonBuiltIn, aCerts);
   }
   catch (e) {
     return e.result;
   }
   return Cr.NS_OK;
 }
 
 function testXHRLoad(aEvent) {
   ok(true, "Entering testXHRLoad");
 
   var channel = aEvent.target.channel;
 
   var certs = null;
-  is(getCheckCertResult(channel, certs), Cr.NS_ERROR_ABORT,
+  is(getCheckCertResult(channel, false, certs), Cr.NS_ERROR_ABORT,
      "checkCert should throw NS_ERROR_ABORT when the certificate attributes " +
      "array passed to checkCert is null and the certificate is not builtin");
 
+  is(getCheckCertResult(channel, true, certs), Cr.NS_OK,
+     "checkCert should not throw when the certificate attributes array " +
+     "passed to checkCert is null and builtin certificates aren't enforced");
+
   certs = [ { invalidAttribute: "Invalid attribute" } ];
-  is(getCheckCertResult(channel, certs), Cr.NS_ERROR_ILLEGAL_VALUE,
+  is(getCheckCertResult(channel, false, certs), Cr.NS_ERROR_ILLEGAL_VALUE,
      "checkCert should throw NS_ERROR_ILLEGAL_VALUE when the certificate " +
      "attributes array passed to checkCert has an element that has an " +
      "attribute that does not exist on the certificate");
 
   certs = [ { issuerName: "Incorrect issuerName" } ];
-  is(getCheckCertResult(channel, certs), Cr.NS_ERROR_ILLEGAL_VALUE,
+  is(getCheckCertResult(channel, false, certs), Cr.NS_ERROR_ILLEGAL_VALUE,
      "checkCert should throw NS_ERROR_ILLEGAL_VALUE when the certificate " +
      "attributes array passed to checkCert has an element that has an " +
      "issuerName that is not the same as the certificate's");
 
   var cert = channel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider).
              SSLStatus.QueryInterface(Ci.nsISSLStatus).serverCert;
 
   certs = [ { issuerName: cert.issuerName,
               commonName: cert.commonName } ];
-  is(getCheckCertResult(channel, certs), Cr.NS_ERROR_ABORT,
+  is(getCheckCertResult(channel, false, certs), Cr.NS_ERROR_ABORT,
      "checkCert should throw NS_ERROR_ABORT when the certificate attributes " +
      "array passed to checkCert has a single element that has the same " +
      "issuerName and commonName as the certificate's and the certificate is " +
      "not builtin");
 
+  is(getCheckCertResult(channel, true, certs), Cr.NS_OK,
+     "checkCert should not throw when the certificate attributes array " +
+     "passed to checkCert has a single element that has the same issuerName " +
+     "and commonName as the certificate's and and builtin certificates " +
+     "aren't enforced");
+
   certs = [ { issuerName: "Incorrect issuerName",
               invalidAttribute: "Invalid attribute" },
             { issuerName: cert.issuerName,
               commonName: "Invalid Common Name" },
             { issuerName: cert.issuerName,
               commonName: cert.commonName } ];
-  is(getCheckCertResult(channel, certs), Cr.NS_ERROR_ABORT,
+  is(getCheckCertResult(channel, false, certs), Cr.NS_ERROR_ABORT,
      "checkCert should throw NS_ERROR_ABORT when the certificate attributes " +
      "array passed to checkCert has an element that has the same issuerName " +
      "and commonName as the certificate's and the certificate is not builtin");
 
+  is(getCheckCertResult(channel, true, certs), Cr.NS_OK,
+     "checkCert should not throw when the certificate attributes array " +
+     "passed to checkCert has an element that has the same issuerName and " +
+     "commonName as the certificate's and builtin certificates aren't enforced");
+
   var mockChannel = { originalURI: Cc["@mozilla.org/network/io-service;1"].
                                    getService(Ci.nsIIOService).
                                    newURI("http://example.com/", null, null) };
 
   certs = [ ];
-  is(getCheckCertResult(mockChannel, certs), Cr.NS_ERROR_UNEXPECTED,
+  is(getCheckCertResult(mockChannel, false, certs), Cr.NS_ERROR_UNEXPECTED,
      "checkCert should throw NS_ERROR_UNEXPECTED when the certificate " +
      "attributes array passed to checkCert is not null and the channel's " +
      "originalURI is not https");
 
   certs = null;
-  is(getCheckCertResult(mockChannel, certs), Cr.NS_OK,
+  is(getCheckCertResult(mockChannel, false, certs), Cr.NS_OK,
      "checkCert should not throw when the certificate attributes object " +
      "passed to checkCert is null and the the channel's originalURI is not " +
      "https");
 
   SimpleTest.finish();
 }
 
 ]]>
--- a/toolkit/mozapps/update/content/updates.js
+++ b/toolkit/mozapps/update/content/updates.js
@@ -46,16 +46,17 @@ Components.utils.import("resource://gre/
 // so we have to use different names.
 const CoC = Components.classes;
 const CoI = Components.interfaces;
 const CoR = Components.results;
 
 const XMLNS_XUL               = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
 const PREF_APP_UPDATE_BILLBOARD_TEST_URL = "app.update.billboard.test_url";
+const PREF_APP_UPDATE_CERT_ERRORS        = "app.update.cert.errors";
 const PREF_APP_UPDATE_ENABLED            = "app.update.enabled";
 const PREF_APP_UPDATE_LOG                = "app.update.log";
 const PREF_APP_UPDATE_MANUAL_URL         = "app.update.url.manual";
 const PREF_APP_UPDATE_NEVER_BRANCH       = "app.update.never.";
 const PREF_APP_UPDATE_TEST_LOOP          = "app.update.test.loop";
 const PREF_PLUGINS_UPDATEURL             = "plugins.update.url";
 
 const UPDATE_TEST_LOOP_INTERVAL     = 2000;
@@ -67,16 +68,19 @@ const STATE_PENDING           = "pending
 const STATE_APPLYING          = "applying";
 const STATE_SUCCEEDED         = "succeeded";
 const STATE_DOWNLOAD_FAILED   = "download-failed";
 const STATE_FAILED            = "failed";
 
 const SRCEVT_FOREGROUND       = 1;
 const SRCEVT_BACKGROUND       = 2;
 
+const CERT_ATTR_CHECK_FAILED_NO_UPDATE  = 100;
+const CERT_ATTR_CHECK_FAILED_HAS_UPDATE = 101;
+
 var gLogEnabled = false;
 var gUpdatesFoundPageId;
 
 // Notes:
 // 1. use the wizard's goTo method whenever possible to change the wizard
 //    page since it is simpler than most other methods and behaves nicely with
 //    mochitests.
 // 2. using a page's onPageShow method to then change to a different page will
@@ -394,16 +398,22 @@ var gUpdates = {
   getStartPageID: function(aCallback) {
     if ("arguments" in window && window.arguments[0]) {
       var arg0 = window.arguments[0];
       if (arg0 instanceof CoI.nsIUpdate) {
         // If the first argument is a nsIUpdate object, we are notifying the
         // user that the background checking found an update that requires
         // their permission to install, and it's ready for download.
         this.setUpdate(arg0);
+        if (this.update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
+            this.update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE) {
+          aCallback("errorcertcheck");
+          return;
+        }
+
         var p = this.update.selectedPatch;
         if (p) {
           var state = p.state;
           var patchFailed;
           try {
             patchFailed = this.update.getProperty("patchingFailed");
           }
           catch (e) {
@@ -649,17 +659,24 @@ var gCheckingPage = {
     },
 
     /**
      * See nsIUpdateCheckListener
      */
     onError: function(request, update) {
       LOG("gCheckingPage", "onError - proceeding to error page");
       gUpdates.setUpdate(update);
-      gUpdates.wiz.goTo("errors");
+      if (update.errorCode &&
+          (update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
+           update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE )) {
+        gUpdates.wiz.goTo("errorcertcheck");
+      }
+      else {
+        gUpdates.wiz.goTo("errors");
+      }
     },
 
     /**
      * See nsISupports.idl
      */
     QueryInterface: function(aIID) {
       if (!aIID.equals(CoI.nsIUpdateCheckListener) &&
           !aIID.equals(CoI.nsISupports))
@@ -1588,16 +1605,44 @@ var gErrorsPage = {
     var manualURL = Services.urlFormatter.formatURLPref(PREF_APP_UPDATE_MANUAL_URL);
     var errorLinkLabel = document.getElementById("errorLinkLabel");
     errorLinkLabel.value = manualURL;
     errorLinkLabel.setAttribute("url", manualURL);
   }
 };
 
 /**
+ * The page shown when there is a certificate attribute check error.
+ */
+var gErrorCertCheckPage = {
+  /**
+   * Initialize
+   */
+  onPageShow: function() {
+    gUpdates.setButtons(null, null, "okButton", true);
+    gUpdates.wiz.getButton("finish").focus();
+
+    if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_ERRORS))
+      Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_ERRORS);
+
+    if (gUpdates.update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE) {
+      document.getElementById("errorCertAttrHasUpdateLabel").hidden = false;
+    }
+    else {
+      document.getElementById("errorCertCheckNoUpdateLabel").hidden = false;
+      var manualURL = Services.urlFormatter.formatURLPref(PREF_APP_UPDATE_MANUAL_URL);
+      var errorLinkLabel = document.getElementById("errorCertAttrLinkLabel");
+      errorLinkLabel.value = manualURL;
+      errorLinkLabel.setAttribute("url", manualURL);
+      errorLinkLabel.hidden = false;
+    }
+  }
+};
+
+/**
  * The "There was an error applying a partial patch" page.
  */
 var gErrorPatchingPage = {
   /**
    * Initialize
    */
   onPageShow: function() {
     gUpdates.setButtons(null, null, "okButton", true);
--- a/toolkit/mozapps/update/content/updates.xul
+++ b/toolkit/mozapps/update/content/updates.xul
@@ -224,16 +224,32 @@
       <separator/>
       <label id="errorManual">&errorManual.label;</label>
       <hbox>
         <label class="text-link" id="errorLinkLabel" value=""
                onclick="openUpdateURL(event);"/>
       </hbox>
     </vbox>
   </wizardpage>
+
+  <wizardpage id="errorcertcheck" pageid="errorcertcheck"
+              object="gErrorCertCheckPage"
+              onpageshow="gErrorCertCheckPage.onPageShow();">
+    <updateheader label="&error.title;"/>
+    <vbox class="update-content" flex="1">
+      <label id="errorCertAttrHasUpdateLabel"
+             hidden="true">&errorCertAttrHasUpdate.label;</label>
+      <label id="errorCertCheckNoUpdateLabel"
+             hidden="true">&errorCertAttrNoUpdate.label;</label>
+      <hbox>
+        <label id="errorCertAttrLinkLabel" class="text-link" hidden="true"
+               value="" onclick="openUpdateURL(event);"/>
+      </hbox>
+    </vbox>
+  </wizardpage>
   
   <wizardpage id="errorpatching" pageid="errorpatching" next="downloading"
               object="gErrorPatchingPage"
               onpageshow="gErrorPatchingPage.onPageShow();">
     <updateheader label="&error.title;"/>
     <vbox class="update-content" flex="1">
       <label>&errorpatching.intro;</label>
     </vbox>
--- a/toolkit/mozapps/update/nsIUpdateService.idl
+++ b/toolkit/mozapps/update/nsIUpdateService.idl
@@ -246,20 +246,22 @@ interface nsIUpdate : nsISupports
    *   "succeeded"         The update was successfully applied.
    *   "download-failed"   The update failed to be downloaded.
    *   "failed"            The update failed to be applied.
    */
   attribute AString state;
 
   /**
    * A numeric error code that conveys additional information about the state
-   * of a failed update.  If the update is not in the "failed" state, then this
-   * value is zero.
+   * of a failed update or failed certificate attribute check during an update
+   * check. If the update is not in the "failed" state or the certificate
+   * attribute check has not failed the value is zero.
    *
-   * TODO: Define typical error codes (for now, see updater/errors.h)
+   * TODO: Define typical error codes (for now, see updater/errors.h and the
+   *       CERT_ATTR_CHECK_FAILED_* values in nsUpdateService.js)
    */
   attribute long errorCode;
 
   /**
    * The number of patches supplied by this update.
    */
   readonly attribute unsigned long patchCount;
 
--- a/toolkit/mozapps/update/nsUpdateService.js
+++ b/toolkit/mozapps/update/nsUpdateService.js
@@ -49,16 +49,20 @@ Components.utils.import("resource://gre/
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cr = Components.results;
 
 const PREF_APP_UPDATE_AUTO                = "app.update.auto";
 const PREF_APP_UPDATE_BACKGROUND_INTERVAL = "app.update.download.backgroundInterval";
 const PREF_APP_UPDATE_CERTS_BRANCH        = "app.update.certs.";
+const PREF_APP_UPDATE_CERT_CHECKATTRS     = "app.update.cert.checkAttributes";
+const PREF_APP_UPDATE_CERT_ERRORS         = "app.update.cert.errors";
+const PREF_APP_UPDATE_CERT_MAXERRORS      = "app.update.cert.maxErrors";
+const PREF_APP_UPDATE_CERT_REQUIREBUILTIN = "app.update.cert.requireBuiltIn";
 const PREF_APP_UPDATE_CHANNEL             = "app.update.channel";
 const PREF_APP_UPDATE_ENABLED             = "app.update.enabled";
 const PREF_APP_UPDATE_IDLETIME            = "app.update.idletime";
 const PREF_APP_UPDATE_INCOMPATIBLE_MODE   = "app.update.incompatible.mode";
 const PREF_APP_UPDATE_INTERVAL            = "app.update.interval";
 const PREF_APP_UPDATE_LOG                 = "app.update.log";
 const PREF_APP_UPDATE_MODE                = "app.update.mode";
 const PREF_APP_UPDATE_NEVER_BRANCH        = "app.update.never.";
@@ -109,16 +113,19 @@ const STATE_APPLYING        = "applying"
 const STATE_SUCCEEDED       = "succeeded";
 const STATE_DOWNLOAD_FAILED = "download-failed";
 const STATE_FAILED          = "failed";
 
 // From updater/errors.h:
 const WRITE_ERROR        = 7;
 const ELEVATION_CANCELED = 9;
 
+const CERT_ATTR_CHECK_FAILED_NO_UPDATE  = 100;
+const CERT_ATTR_CHECK_FAILED_HAS_UPDATE = 101;
+
 const DOWNLOAD_CHUNK_SIZE           = 300000; // bytes
 const DOWNLOAD_BACKGROUND_INTERVAL  = 600;    // seconds
 const DOWNLOAD_FOREGROUND_INTERVAL  = 0;
 
 const UPDATE_WINDOW_NAME      = "Update:Wizard";
 
 var gLocale     = null;
 
@@ -1187,24 +1194,28 @@ UpdateService.prototype = {
       um.activeUpdate = update;
       Services.prefs.setBoolPref(PREF_APP_UPDATE_POSTUPDATE, true);
       prompter.showUpdateInstalled();
 
       // Done with this update. Clean it up.
       cleanupActiveUpdate();
     }
     else {
-      // If we hit an error, then the error code will be included in the
-      // status string following a colon.  If we had an I/O error, then we
-      // assume that the patch is not invalid, and we restage the patch so
-      // that it can be attempted again the next time we restart.
-      var ary = status.split(": ");
+      // If we hit an error, then the error code will be included in the status
+      // string following a colon and a space. If we had an I/O error, then we
+      // assume that the patch is not invalid, and we re-stage the patch so that
+      // it can be attempted again the next time we restart. This will leave a
+      // space at the beginning of the error code when there is a failure which
+      // will be removed by using parseInt below. This prevents panic which has
+      // occurred numerous times previously (see bug 569642 comment #9 for one
+      // example) when testing releases due to forgetting to include the space.
+      var ary = status.split(":");
       update.state = ary[0];
       if (update.state == STATE_FAILED && ary[1]) {
-        update.errorCode = ary[1];
+        update.errorCode = parseInt(ary[1]);
         if (update.errorCode == WRITE_ERROR) {
           prompter.showUpdateError(update);
           writeStatusFile(getUpdatesDir(), update.state = STATE_PENDING);
           writeVersionFile(getUpdatesDir(), update.appVersion);
           return;
         }
         else if (update.errorCode == ELEVATION_CANCELED) {
           writeStatusFile(getUpdatesDir(), update.state = STATE_PENDING);
@@ -1265,18 +1276,33 @@ UpdateService.prototype = {
       },
 
       /**
        * See nsIUpdateService.idl
        */
       onError: function AUS_notify_onError(request, update) {
         LOG("UpdateService:notify:listener - error during background update: " +
             update.statusText);
-      },
-    }
+
+        if (!update.errorCode ||
+            update.errorCode != CERT_ATTR_CHECK_FAILED_NO_UPDATE &&
+            update.errorCode != CERT_ATTR_CHECK_FAILED_HAS_UPDATE)
+          return;
+
+        var errCount = getPref("getIntPref", PREF_APP_UPDATE_CERT_ERRORS, 0);
+        errCount++;
+        Services.prefs.setIntPref(PREF_APP_UPDATE_CERT_ERRORS, errCount);
+
+        if (errCount >= getPref("getIntPref", PREF_APP_UPDATE_CERT_MAXERRORS, 5)) {
+          var prompter = Cc["@mozilla.org/updates/update-prompt;1"].
+                         createInstance(Ci.nsIUpdatePrompt);
+          prompter.showUpdateError(update);
+        }
+      }
+    };
     this.backgroundChecker.checkForUpdates(listener, false);
   },
 
   /**
    * Determine the update from the specified updates that should be offered.
    * If both valid major and minor updates are available the minor update will
    * be offered.
    * @param   updates
@@ -1997,17 +2023,19 @@ Checker.prototype = {
 
     var url = this.getUpdateURL(force);
     if (!url || (!this.enabled && !force))
       return;
 
     this._request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].
                     createInstance(Ci.nsIXMLHttpRequest);
     this._request.open("GET", url, true);
-    this._request.channel.notificationCallbacks = new gCertUtils.BadCertHandler();
+    var allowNonBuiltIn = !getPref("getBoolPref",
+                                   PREF_APP_UPDATE_CERT_REQUIREBUILTIN, true);
+    this._request.channel.notificationCallbacks = new gCertUtils.BadCertHandler(allowNonBuiltIn);
     this._request.overrideMimeType("text/xml");
     this._request.setRequestHeader("Cache-Control", "no-cache");
 
     var self = this;
     this._request.onerror     = function(event) { self.onError(event);    };
     this._request.onload      = function(event) { self.onLoad(event);     };
     this._request.onprogress  = function(event) { self.onProgress(event); };
 
@@ -2089,16 +2117,17 @@ Checker.prototype = {
    *          The nsIDOMEvent for the load
    */
   onLoad: function UC_onLoad(event) {
     LOG("Checker:onLoad - request completed downloading document");
 
     var prefs = Services.prefs;
     var certs = null;
     if (!prefs.prefHasUserValue(PREF_APP_UPDATE_URL_OVERRIDE) &&
+        getPref("getBoolPref", PREF_APP_UPDATE_CERT_CHECKATTRS, true) &&
         prefs.getBranch(PREF_APP_UPDATE_CERTS_BRANCH).getChildList("").length) {
       certs = [];
       let counter = 1;
       while (true) {
         let prefBranchCert = prefs.getBranch(PREF_APP_UPDATE_CERTS_BRANCH +
                                              counter + ".");
         let prefCertAttrs = prefBranchCert.getChildList("");
         if (prefCertAttrs.length == 0)
@@ -2108,49 +2137,42 @@ Checker.prototype = {
         for each (let prefCertAttr in prefCertAttrs)
           certAttrs[prefCertAttr] = prefBranchCert.getCharPref(prefCertAttr);
 
         certs.push(certAttrs);
         counter++;
       }
     }
 
-    var certAttrCheckFailed = false;
-    var status;
     try {
-      try {
-        gCertUtils.checkCert(this._request.channel, certs);
-      }
-      catch (e) {
-        Components.utils.reportError(e);
-        if (e.result != Cr.NS_ERROR_ILLEGAL_VALUE)
-          throw e;
+      // Analyze the resulting DOM and determine the set of updates.
+      var updates = this._updates;
+      LOG("Checker:onLoad - number of updates available: " + updates.length);
+      var allowNonBuiltIn = !getPref("getBoolPref",
+                                     PREF_APP_UPDATE_CERT_REQUIREBUILTIN, true);
+      gCertUtils.checkCert(this._request.channel, allowNonBuiltIn, certs);
 
-        certAttrCheckFailed = true;
-      }
-
-      // Analyze the resulting DOM and determine the set of updates. If the
-      // certificate attribute check failed treat it as no updates found until
-      // Bug 583408 is fixed.
-      var updates = certAttrCheckFailed ? [] : this._updates;
-
-      LOG("Checker:onLoad - number of updates available: " + updates.length);
+      if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_ERRORS))
+        Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_ERRORS);
 
       // Tell the Update Service about the updates
       this._callback.onCheckComplete(event.target, updates, updates.length);
     }
     catch (e) {
-      LOG("Checker:onLoad - there was a problem with the update service URL " +
-          "specified, either the XML file was malformed or it does not exist " +
-          "at the location specified. Exception: " + e);
+      LOG("Checker:onLoad - there was a problem checking for updates. " +
+          "Exception: " + e);
       var request = event.target;
       var status = this._getChannelStatus(request);
       LOG("Checker:onLoad - request.status: " + status);
       var update = new Update(null);
       update.statusText = getStatusTextFromCode(status, 404);
+      if (e.result == Cr.NS_ERROR_ILLEGAL_VALUE) {
+        update.errorCode = updates[0] ? CERT_ATTR_CHECK_FAILED_HAS_UPDATE
+                                      : CERT_ATTR_CHECK_FAILED_NO_UPDATE;
+      }
       this._callback.onError(request, update);
     }
 
     this._request = null;
   },
 
   /**
    * There was an error of some kind during the XMLHttpRequest
@@ -2798,16 +2820,24 @@ UpdatePrompt.prototype = {
 
   /**
    * See nsIUpdateService.idl
    */
   showUpdateError: function UP_showUpdateError(update) {
     if (!this._enabled)
       return;
 
+    if (update.errorCode &&
+        (update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
+         update.errorCode != CERT_ATTR_CHECK_FAILED_HAS_UPDATE)) {
+      this._showUIWhenIdle(null, URI_UPDATE_PROMPT_DIALOG, null,
+                           UPDATE_WINDOW_NAME, null, update);
+      return;
+    }
+
     // In some cases, we want to just show a simple alert dialog:
     if (update.state == STATE_FAILED && update.errorCode == WRITE_ERROR) {
       var title = gUpdateBundle.GetStringFromName("updaterIOErrorTitle");
       var text = gUpdateBundle.formatStringFromName("updaterIOErrorMsg",
                                                     [Services.appinfo.name,
                                                      Services.appinfo.name], 2);
       Services.ww.getNewPrompter(null).alert(title, text);
     } else {