1016555 - Disable OCSP checking for certificates covered by OneCRL

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

+++ This bug was initially created as a clone of Bug #1010068 +++

Doug tells me that OCSP isn't providing any significant security to our users but is costing us in networking performance.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Patrick, can you tell Finkle what needs to happen in the front end here?

Comment 2

[Patrick McManus [:mcmanus]]

my suggestion was simply that if OCSP was adminstratively disabled, that the green bar could show EV if OCSP was the only reason EV validation failed.

iirc you can't get that granularity of failure out of PSM, so PSM might need to bypass the OCSP requirement if its disabled.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This is doable (it would require some changes in PSM). However, my understanding is we're essentially using the EV indicator as a way of encouraging CAs to improve in various ways. If we basically say, "Oh, well, we're going to display the EV indicator regardless of if we have revocation information on mobile", we lose some of our leverage.
We have been planning to only rely on stapled OCSP responses with the same requirement that fresh revocation information is required to display the EV indicator. This would get us what we want (no OCSP fetching but still only showing the EV indicator when appropriate), but that depends on everyone implementing multi-stapling (which honestly wouldn't be hard, but then everyone has to upgrade their server, etc.)

Comment 4

[Patrick McManus [:mcmanus]]

david - I hear ya.

but here's where we are as I see it
1] ocsp performance is killing mobile - and its a top organizational priority to grow mobile
2] we don't have must-staple code
3] we don't have a must-staple server software
4] we don't even have a must-staple spec
5] ocsp as implemented provides very little if any benefit to the user
6] our mobile footprint is not large enough to sway anyone anyhow - desktop is different

so I propose making the change in comment 2 and after we have better answers on 3-5 putting folks on notice that we're going to enforce the OCSP requirement via must-staple. (and then do it).

I'd be happy to help. what do you think?

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Ok - sounds reasonable. FWIW, I had a look at mobile chrome, and as far as I can tell, they always use a green lock for https, regardless of EV status.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Patrick McManus [:mcmanus] from comment #2)
> my suggestion was simply that if OCSP was adminstratively disabled, that the
> green bar could show EV if OCSP was the only reason EV validation failed.

I can understand most of these words, but I don't know how they apply to the code here:
https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#6642

David - Since you took a look at the chrome code, can you translate?

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This probably actually won't involve a front-end change (other than a pref, if that counts). What we need to do is add a pref that toggles a flag in CertVerifier/NSSCertDBTrustDomain that basically says "OCSP isn't required for EV". Combined with disabling OCSP fetching, this will get what we want here. I can probably write a patch for this next week.

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: patch

Brian, the context for this is OCSP is too much of a performance hit on mobile, so it's being disabled. To get the same visual behavior regarding the EV indicator, this patch adds a mode whereby verifying an EV certificate doesn't cause OCSP requests to be made.

Comment 9

[Gervase Markham [:gerv]]

This proposal for disabling OCSP needs to be seen in the context of our overall plan for revocation, our policies and practices on improving the CA and certificate landscape, and the possible legal effects of not doing any revocation checking. Kathleen is unfortunately on vacation until 21st of July, but she needs to have a voice in this.

In the mean time, I have attempted to write up our current plan for revocation to make sure everyone is on the same page. This shows how we want to get to a place where we can disable online OCSP checking:
https://wiki.mozilla.org/CA:RevocationPlan

Gerv

Comment 10

[Doug Turner (:dougt)]

Gerv,

rbarnes must review your propsoal/plan.

I do not think that should block if the fennec/psm module owners/peer agree.  MUST-STAPLE is a long road, and performance NOW is critical (especially that OCSP is not privacy preserving AND it is soft fail.  It is theater!)

Assuming module owners are happy, Richard or I will call and brief Kathleen as you suggest.

Comment 11

[Richard Barnes [:rbarnes]]

Hey Gerv,

There's already been some work on a draft plan, currently in an etherpad [1], with a goal to move it to a wiki once it's more complete.  Your comments there would be greatly appreciated.  Just so that we have consistent messaging, while we iron this out, would you mind moving the current contents of the wiki to an etherpad, and replacing the wiki page with a link to [1]?  I've added a note to the top of [1] pointing to the wiki page and a new pad [2] for you to put your stuff in.

[1] https://etherpad.mozilla.org/Revocation
[2] https://etherpad.mozilla.org/RevocationPlanAlt

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I'd like to propose an alternative solution: we turn off OCSP fetching for mobile when bug 1024809 lands (which should be soon), which will give us a limited ability to push some revocation information to the browser. That way, we get better performance as well as some actual, useful protection.
More relevantly to this bug, we shouldn't change the meaning of the EV indicator. I have yet to see a convincing argument for why we would want to do so. It would be a form of security theater as well.

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #12)
> I'd like to propose an alternative solution: we turn off OCSP fetching for
> mobile when bug 1024809 lands (which should be soon), which will give us a
> limited ability to push some revocation information to the browser. That
> way, we get better performance as well as some actual, useful protection.
> More relevantly to this bug, we shouldn't change the meaning of the EV
> indicator. I have yet to see a convincing argument for why we would want to
> do so. It would be a form of security theater as well.

Doug, does this plan work for you? If so, we need movement on bug 1024809

Comment 14

[Richard Barnes [:rbarnes]]

I think this plan in general seems sensible.  Our long-term revocation plan is to move away from OCSP anyway, especially for intermediate CA certificates.  
https://wiki.mozilla.org/CA:RevocationPlan

We should make sure that we have some revocation plan for EV end-entity certs (as well as CA certs).  That might mean that we need to extend OneCRL / bug 1024809 to cover EV EE certs.  My data indicate that the number of those certs is not so large that this isn't plausible (~432k certs, of which presumably only a fraction are revoked).  

tl;dr: Yes.

Comment 15

[Richard Barnes [:rbarnes]]

Updating the title of the bug and dependencies to clarify that this is but about making it OK in general to skip OCSP when a cert is covered by OneCRL.  As in Comment 14, it should be possible to cover EV with OneCRL.

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

bug 1024809 has review and is checkin-needed. What's the plan here?

Comment 17

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Next steps probably are:
* land OneCRL, ensure that it sticks (and iron out any issues)
* start actually serving revocation information for it to consume (and iron out any issues)
* when we're confident in the implementation and we're serving a cert blocklist that covers enough of the revocations we care about, we can disable OCSP fetching by default in the browser

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

This is currently being tracked for Firefox 36 (aurora). Is that still a reasonable target?

Comment 19

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

37 would probably be more reasonable, depending on how OneCRL goes.

And, actually comment 17 was not entirely accurate. We probably won't disable OCSP fetching entirely right away - just for the set of certificates covered by OneCRL (this will probably be intermediates to begin with, although if/when we expand to all certificates, we can disable OCSP fetching entirely).

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

David - What's the status of OneCRL and this bug?

Comment 21

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Platform support for OneCRL landed in bug 1024809. Next steps are to determine and build the set of revocation information we want and to start serving it up with the blocklist. Then we can disable OCSP checking for certificates in that set.

Comment 22

[Richard Barnes [:rbarnes]]

To summarize where we stand here:
* OneCRL exists, but IIRC is currently populated by hand (and currently empty)
* The current stated policy is to cover intermediate CA certificates with OneCRL [1]
* We already don't do OCSP checking for CA certs outside of EV [2]

So the only difference between policy and practice right now is that we still do OCSP for EV certs.  But that seems OK to me because I don't think we have yet automated the process of populating OneCRL.

That seems to me to be the crux of the question -- I would only be willing to turn off OCSP in cases where we've got OneCRL automatically populated.  Added a need-info to mgoodwin so he can comment on current status.


[1] https://wiki.mozilla.org/CA:RevocationPlan#OneCRL
[2] https://dxr.mozilla.org/mozilla-central/source/security/certverifier/NSSCertDBTrustDomain.cpp#490

Comment 23

[Richard Barnes [:rbarnes]]

Attachment: bug-1016555.0.patch

It turns out that the required code to bypass OCSP for CA certificates when we have fresh OneCRL data was added in bug 1128607, but pref'ed off, basically by considering OneCRL to always be stale (security.onecrl.max_staleness_in_seconds = 0).

This patch changes that default to be 1.25x the value of extensions.blocklist.interval.  The blocklist currently updates every 24 hours, so OCSP will be skipped if the blocklist has been updated within the last 30 hours.  That level of padding may be a little generous; feedback welcome.

Comment 24

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8620961 [details] [diff] [review]
bug-1016555.0.patch

Review of attachment 8620961 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me - let's not land this before we're ready though.

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/331e489c7534

Comment 26

[:Cykesiopka]

Comment on attachment 8620961 [details] [diff] [review]
bug-1016555.0.patch

Review of attachment 8620961 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the late drive-by, I only noticed this after the patch was pushed to m-i.

::: browser/app/profile/firefox.js
@@ +1773,5 @@
>  // 1 = allow MITM for certificate pinning checks.
>  pref("security.cert_pinning.enforcement_level", 1);
>  
>  // Required blocklist freshness for OneCRL OCSP bypass
> +// (default is 1.25% extensions.blocklist.interval, or 30 hours)

This looks odd... 1.25x maybe?

::: mobile/android/app/mobile.js
@@ +476,5 @@
>  // Enable pinning
>  pref("security.cert_pinning.enforcement_level", 1);
>  
> +// Required blocklist freshness for OneCRL OCSP bypass
> +// (default is 1.25% extensions.blocklist.interval, or 30 hours)

Same here?

Comment 27

[Carsten Book [:Tomcat]]

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e7ab435c8d1e because somehow one of this 2 patches caused perma failures like 

https://treeherder.mozilla.org/logviewer.html#?job_id=12563641&repo=mozilla-inbound

Comment 28

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Yeah, this probably should have gone through try first.

Comment 29

[Mark Goodwin [:mgoodwin]]

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

Comment 30

[Mark Goodwin [:mgoodwin]]

Attachment: MozReview Request: Bug 1016555 - Disable OCSP checking for certificates covered by OneCRL r?keeler

Bug 1016555 - Disable OCSP checking for certificates covered by OneCRL r?keeler
1) Added some comments to firefox.js to explain the relationship between
extensions.blocklist.interval and security.onecrl.maximum_staleness_in_seconds
2) Modified default values in firefox.js and mobile.js to set maximum staleness
to 1.25x blocklist interval
3) modified the tests_ev_certs.js xpcshell test to cope with larger maximum
staleness values to address test failures

Comment 31

[Mark Goodwin [:mgoodwin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94876caa932c

Comment 32

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8658132 [details]
MozReview Request: Bug 1016555 - Disable OCSP checking for certificates covered by OneCRL r?keeler

https://reviewboard.mozilla.org/r/18483/#review16737

Great!

::: security/manager/ssl/tests/unit/test_ev_certs.js:150
(Diff revision 1)
>      // enable OneCRL OCSP skipping - allow staleness of up to 1 day

Maybe update this comment too.

::: security/manager/ssl/tests/unit/test_ev_certs.js:176
(Diff revision 1)
>      // enable OneCRL OCSP skipping - allow staleness of up to 1 day

Similarly with this comment.

Comment 33

[Mark Goodwin [:mgoodwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/06d15f86c1d38bb88206007c8a2c2a16d7bae9be
Bug 1016555 - Disable OCSP checking for certificates covered by OneCRL r=keeler

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/06d15f86c1d3