957667 - De-couple OCSP stapling pref from the OCSP fetching pref, and couple the OCSP required pref to the OCSP fetching pref

Status: VERIFIED FIXED

(Core :: Security: PSM, enhancement)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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

It should be possible to disable OCSP fetching without disabling OCSP stapling support. That will become especially important when we have OCSP must-staple. So, the OCSP stapling pref should be completely independent of the OCSP fetching pref.

However, it isn't reasonable to require OCSP when OCSP fetching is disabled. The Options UI attempts to prevent this from happening by disabling the "required" checkbox when the "fetching" checkbox is disabled (though radio buttons would be a better UI for it). However, it is possible for these to get set inconsistently by directly modifying about:config.

Depends on bug 957665 only to reduce time spent dealing with merge conflicts and rebasing.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: improve-pref-interaction.patch

Comment 2

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

Comment on attachment 8358885 [details] [diff] [review]
improve-pref-interaction.patch

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

It might be worthwhile noting (in a comment or documentation or something) that security.OCSP.enabled really refers to OCSP fetching (as in, a stapled OCSP response saying a certificate is revoked will terminate a connection if security.ssl.enable_ocsp_stapling is true, even if security.OCSP.enabled is false). Anyway, r=me.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/744e91ec69a3

Thanks for the review. I added the comment that you suggested.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: improve-pref-interaction-2.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 700693
User impact if declined: OCSP behavior of Firefox will be confusing and quite different than how we intend to support it long term. Getting this change into developers' hands as soon as is reasonable will make our messaging about OCSP clearer and it will make the transition to insanity::pkix (bug 915930) easier.

Testing completed (on m-c, etc.): Landed on m-c yesterday. Manually tested.

Risk to taking this patch (and alternatives if risky): Users that disabled the OCSP (fetching) pref but not the OCSP stapling pref will now get OCSP stapling support. If they disabled the pref due to problems with OCSP stapling then those problems will return. However, they can "fix" that problem again by disabling the separate OCSP stapling pref. Also, we've implemented some workarounds that should make it unnecessary for users to need to disable OCSP stapling now.

String or IDL/UUID changes made by this patch: none.

Note: I am not going to request this be uplifted to Firefox 27.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/744e91ec69a3

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8359615 [details] [diff] [review]
improve-pref-interaction-2.patch

Thanks for providing the workarounds, should they be needed.  Approving since it seems like we can handle any potential fallout gracefully here.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b15a92ca0ec4

Comment 8

[u279076]

Is there anything QA needs to verify here? Do the changes to these prefs change the way OCSP is supposed to work?

Comment 9

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

It should be possible to enable OCSP stapling independently of OCSP fetching.
Here are some scenarios that should be verified:

security.OCSP.enabled = 1, security.ssl.enable_ocsp_stapling = true -> wireshark shows both OCSP fetching and stapled OCSP responses (try going to tumblr, twitter, or soundcloud)

security.OCSP.enabled = 0, security.ssl.enable_ocsp_stapling = true -> wireshark shows only stapled OCSP responses

security.OCSP.enabled = 0, security.ssl.enable_ocsp_stapling = false -> wireshark shows no OCSP fetching or stapled OCSP responses

security.OCSP.enabled = 1, security.ssl.enable_ocsp_stapling = false -> wireshark shows only OCSP fetching

Comment 10

[u279076]

Matt, can you please verify this based on comment 9?

Comment 11

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

I will verify. If not enough time right now, I will do it post-27 launch.

Comment 12

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

Using keeler's instructions in comment 9, I verified those four scenarios via Wireshark and using tumblr.com as my target site. 

I also verified that the preferences exist and are on by default in 28b4 and 29. 

If more needs to be done, just let me know.