Bug 1286600 - prevent errors in calls to the CertBlocklist from causing blocklist sync to fail. r=MattN
MozReview-Commit-ID: 42O2QnPQRuj
--- a/services/common/blocklist-clients.js
+++ b/services/common/blocklist-clients.js
@@ -227,24 +227,29 @@ class BlocklistClient {
* Revoke the appropriate certificates based on the records from the blocklist.
*
* @param {Object} records current records in the local db.
*/
function* updateCertBlocklist(records) {
let certList = Cc["@mozilla.org/security/certblocklist;1"]
.getService(Ci.nsICertBlocklist);
for (let item of records) {
- if (item.issuerName && item.serialNumber) {
- certList.revokeCertByIssuerAndSerial(item.issuerName,
- item.serialNumber);
- } else if (item.subject && item.pubKeyHash) {
- certList.revokeCertBySubjectAndPubKey(item.subject,
- item.pubKeyHash);
- } else {
- throw new Error("Cert blocklist record has incomplete data");
+ try {
+ if (item.issuerName && item.serialNumber) {
+ certList.revokeCertByIssuerAndSerial(item.issuerName,
+ item.serialNumber);
+ } else if (item.subject && item.pubKeyHash) {
+ certList.revokeCertBySubjectAndPubKey(item.subject,
+ item.pubKeyHash);
+ }
+ } catch (e) {
+ // prevent errors relating to individual blocklist entries from
+ // causing sync to fail. At some point in the future, we may want to
+ // accumulate telemetry on these failures.
+ Cu.reportError(e);
}
}
certList.saveEntries();
}
/**
* Write list of records into JSON file, and notify nsBlocklistService.
*
--- a/services/common/tests/unit/test_blocklist_certificates.js
+++ b/services/common/tests/unit/test_blocklist_certificates.js
@@ -100,16 +100,23 @@ add_task(function* test_something(){
yield OneCRLBlocklistClient.maybeSync(3000, Date.now());
// Check the OneCRL check time pref is modified, even if the collection
// hasn't changed
Services.prefs.setIntPref("services.blocklist.onecrl.checked", 0);
yield OneCRLBlocklistClient.maybeSync(3000, Date.now());
let newValue = Services.prefs.getIntPref("services.blocklist.onecrl.checked");
do_check_neq(newValue, 0);
+
+ // Check that a sync completes even when there's bad data in the
+ // collection. This will throw on fail, so just calling maybeSync is an
+ // acceptible test.
+ Services.prefs.setCharPref("services.settings.server",
+ `http://localhost:${server.identity.primaryPort}/v1`);
+ yield OneCRLBlocklistClient.maybeSync(5000, Date.now());
});
function run_test() {
// Ensure that signature verification is disabled to prevent interference
// with basic certificate sync tests
Services.prefs.setBoolPref("services.blocklist.signing.enforced", false);
// Set up an HTTP Server
@@ -178,14 +185,40 @@ function getSampleResponse(req, port) {
"id":"dabafde9-df4a-ddba-2548-748da04cc02c",
"last_modified":4000
},{
"subject":"MCIxIDAeBgNVBAMMF0Fub3RoZXIgVGVzdCBFbmQtZW50aXR5",
"pubKeyHash":"VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=",
"id":"dabafde9-df4a-ddba-2548-748da04cc02d",
"last_modified":4000
}]})
+ },
+ "GET:/v1/buckets/blocklists/collections/certificates/records?_sort=-last_modified&_since=4000": {
+ "sampleHeaders": [
+ "Access-Control-Allow-Origin: *",
+ "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
+ "Content-Type: application/json; charset=UTF-8",
+ "Server: waitress",
+ "Etag: \"5000\""
+ ],
+ "status": {status: 200, statusText: "OK"},
+ "responseBody": JSON.stringify({"data":[{
+ "issuerName":"not a base64 encoded issuer",
+ "serialNumber":"not a base64 encoded serial",
+ "id":"dabafde9-df4a-ddba-2548-748da04cc02e",
+ "last_modified":5000
+ },{
+ "subject":"not a base64 encoded subject",
+ "pubKeyHash":"not a base64 encoded pubKeyHash",
+ "id":"dabafde9-df4a-ddba-2548-748da04cc02f",
+ "last_modified":5000
+ },{
+ "subject":"MCIxIDAeBgNVBAMMF0Fub3RoZXIgVGVzdCBFbmQtZW50aXR5",
+ "pubKeyHash":"VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=",
+ "id":"dabafde9-df4a-ddba-2548-748da04cc02g",
+ "last_modified":5000
+ }]})
}
};
return responses[`${req.method}:${req.path}?${req.queryString}`] ||
responses[req.method];
}