Closed Bug 932176 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox26 --- unaffected
firefox27 + verified

People

(Reporter: briansmith, Assigned: briansmith)

References

Details

Attachments

(1 file)

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.
Attachment #823809 - Flags: review?(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.
Attachment #823809 - Flags: review?(cviecco) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/8dbd3f432835
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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.
Attachment #823809 - Flags: approval-mozilla-aurora?
Backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a288dd7ac0bb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/4061fa7a68c2
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Attachment #823809 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
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?
Flags: needinfo?(brian)
(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.
Flags: needinfo?(brian)
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.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.