1286600 - Malformed revocations cause the new OneCRL client to choke on sync.

Status: VERIFIED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Mark Goodwin [:mgoodwin]]

updateCertBlocklist in blocklist-clients.js does not correctly handle failure of the blocklist.revoke* functions.

Comment 1

[Mark Goodwin [:mgoodwin]]

To be clear, the correct behavior here is what happens in the nsBlocklistService implementation of the oneCRL client (e.g. https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#983); if single calls to the revocation functions fail the errors are caught so that other revocations are attempted.

Comment 2

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 1286600 - prevent errors in calls to the CertBlocklist from causing blocklist sync to fail.

Review commit: https://reviewboard.mozilla.org/r/64262/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64262/

Comment 3

[Mark Goodwin [:mgoodwin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e8ed6746b2e

Comment 4

[Matthew N. [:MattN] (PM me if requests are blocking you)]

Comment on attachment 8770980 [details]
Bug 1286600 - prevent errors in calls to the CertBlocklist from causing blocklist sync to fail.

https://reviewboard.mozilla.org/r/64262/#review61328

::: services/common/blocklist-clients.js:243
(Diff revision 1)
> +    } 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.
> +    }

Please add a `Cu.reportError(e)` for this case so they aren't silently ignored. Double-check if that causes a failure on try though in your test.

Comment 5

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8770980 [details]
Bug 1286600 - prevent errors in calls to the CertBlocklist from causing blocklist sync to fail.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64262/diff/1-2/

Comment 6

[Mark Goodwin [:mgoodwin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=35597c7e6058

Comment 7

[Mark Goodwin [:mgoodwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1333efcac5064aeb246e977676df99ac2428c0ec
Bug 1286600 - prevent errors in calls to the CertBlocklist from causing blocklist sync to fail. r=MattN

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1333efcac506

Comment 10

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified fixed. I staged bad data on AMO staging, along with existing good entries. I made sure to put good entries before and after the bad one. Created new Fx profile in today's Nightly, then forced blocklist update. Verified that all sites on the list were blocked correctly, even after restart. Verified that revocations.txt file exists and contains blocklist entries - except the bad one. Also saw an error thrown in the browser console for the bad entry it received.