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
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 {