932176 - Add preference to control whether OCSP GET is used, off by default

Status: VERIFIED FIXED

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

Description

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

Attachment: add-gecko-option-to-control-OCSP-GET.patch

We'll enable OCSP GET in bug 871954 once we've got tests for it.

This isn't an ideal patch in that ideally we'd have automated tests for it. However, the reason I wrote this patch is to revert things to the historically-used code path due to lack of tests for the new code path. If I had time to write tests for this, I could just write the OCSP GET tests and we wouldn't need this. But, I don't have said time.

Comment 1

[Camilo Viecco (:cviecco)]

Comment on attachment 823809 [details] [diff] [review]
add-gecko-option-to-control-OCSP-GET.patch

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

Witht the nit and the caveat, r+

::: security/manager/ssl/src/CertVerifier.cpp
@@ +236,4 @@
>      rev.leafTests.cert_rev_flags_per_method[cert_revocation_method_ocsp] =
> +    rev.chainTests.cert_rev_flags_per_method[cert_revocation_method_ocsp]
> +      = revMethodFlags
> +      | (mOCSPGETEnabled ? 0 : CERT_REV_M_FORCE_POST_METHOD_FOR_OCSP);

I dont see CERT_REV_M_FORCE_POST_METHOD_FOR_OCSP on central, am I assuming it was defined on cert.h?

@@ +346,5 @@
>  
>      // ocsp enabled controls network fetching, too
>      | ((mOCSPDownloadEnabled && !localOnly) ?
>          CERT_REV_M_ALLOW_NETWORK_FETCHING : CERT_REV_M_FORBID_NETWORK_FETCHING)
> +    

nit space.

Comment 2

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

I had to rebase this on top of the new patch for bug 928142
https://hg.mozilla.org/integration/mozilla-inbound/rev/8dbd3f432835

Sorry I missed the whitespace nit. My bad. Thanks for the review.

Comment 3

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8dbd3f432835

Comment 4

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

Comment on attachment 823809 [details] [diff] [review]
add-gecko-option-to-control-OCSP-GET.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 436414, Bug 898431
See bug 898431 comment 25 and bug 898431 comment 26.

Comment 5

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

Backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a288dd7ac0bb

Comment 6

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

Relanded:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4061fa7a68c2

See bug 898431 comment 28.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4061fa7a68c2

Comment 8

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

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

Comment 9

[Ada [:adalucinet]]

Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on Firefox 27 beta 1 (Build ID: 20131209204824): the security.OCSP.GET.enabled pref is present in about:config and set to false by default.

Is anything else that QA can manually test here?

Comment 10

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

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #9)
> Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
> Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101
> Firefox/27.0
> 
> Verified as fixed on Firefox 27 beta 1 (Build ID: 20131209204824): the
> security.OCSP.GET.enabled pref is present in about:config and set to false
> by default.
> 
> Is anything else that QA can manually test here?

1. Run wireshark
2. Visit https://bugzilla.mozilla.org
3. look for requests to the server http://ocsp.digicert.com. All of those requests should be POST requests and none of them should be GET requests.

Comment 11

[Ada [:adalucinet]]

Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0

Verified as fixed on Firefox 27 beta 1 (Build ID: 20131209204824) and latest Aurora 28.0a2 (Build ID: 20131212004008) with STR from comment 10: all the requests are POST requests.