Patch 1 (main patch) - Bug 583408 - Notify user when the certificate attribute check fails. r=dtownsend, a=blocking2.0-beta6

This commit is contained in:
Robert Strong 2010-09-01 16:27:07 -07:00
parent e43b4b2b37
commit dc84ef605a
7 changed files with 182 additions and 48 deletions

View File

@ -90,6 +90,23 @@ pref("app.update.timer", 600000);
// 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

View File

@ -54,11 +54,14 @@ const Cu = Components.utils;
*
* @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
@ -66,7 +69,7 @@ const Cu = Components.utils;
* 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) {
@ -112,6 +115,8 @@ function checkCert(aChannel, aCerts) {
}
}
if (aAllowNonBuiltInCerts === true)
return;
var issuerCert = cert;
while (issuerCert.issuer && !issuerCert.issuer.equals(issuerCert))
@ -136,6 +141,10 @@ function isBuiltinToken(tokenName) {
* 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;

View File

@ -58,9 +58,9 @@ function testXHRError(aEvent) {
SimpleTest.finish();
}
function getCheckCertResult(aChannel, aCerts) {
function getCheckCertResult(aChannel, aAllowNonBuiltIn, aCerts) {
try {
checkCert(aChannel, aCerts);
checkCert(aChannel, aAllowNonBuiltIn, aCerts);
}
catch (e) {
return e.result;
@ -74,18 +74,22 @@ function testXHRLoad(aEvent) {
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");
@ -95,35 +99,46 @@ function testXHRLoad(aEvent) {
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");

View File

@ -51,6 +51,7 @@ 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";
@ -72,6 +73,9 @@ 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;
@ -399,6 +403,12 @@ var gUpdates = {
// 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;
@ -654,7 +664,14 @@ var gCheckingPage = {
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");
}
},
/**
@ -1592,6 +1609,34 @@ var gErrorsPage = {
}
};
/**
* 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.
*/

View File

@ -229,6 +229,22 @@
</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"

View File

@ -251,10 +251,12 @@ interface nsIUpdate : nsISupports
/**
* 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;

View File

@ -54,6 +54,10 @@ 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";
@ -114,6 +118,9 @@ const STATE_FAILED = "failed";
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;
@ -1192,14 +1199,18 @@ UpdateService.prototype = {
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);
@ -1270,8 +1281,23 @@ UpdateService.prototype = {
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);
},
@ -2002,7 +2028,9 @@ Checker.prototype = {
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");
@ -2094,6 +2122,7 @@ Checker.prototype = {
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;
@ -2113,39 +2142,32 @@ Checker.prototype = {
}
}
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;
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;
// 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);
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);
}
@ -2803,6 +2825,14 @@ UpdatePrompt.prototype = {
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");