Closed Bug 367878 Opened 18 years ago Closed 18 years ago

Mistake in PSM's OCSP pref switching code

Categories

(Core :: Security: PSM, defect)

1.8 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: KaiE)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Based on code inspection, I wonder if the following code found in PSM is wrong.

static void setOCSPOptions(nsIPrefBranch * pref)
{
  switch (ocspEnabled) {
  case 0: // never use OCSP
    CERT_DisableOCSPChecking(CERT_GetDefaultCertDB());
    CERT_DisableOCSPDefaultResponder(CERT_GetDefaultCertDB());
    break;
  case 1: // use OCSP service URLs in certs
    CERT_EnableOCSPChecking(CERT_GetDefaultCertDB());
    break;
  case 2: // always use given OCSP default responder
    CERT_EnableOCSPChecking(CERT_GetDefaultCertDB());
    CERT_SetOCSPDefaultResponder(CERT_GetDefaultCertDB(), url, signingCA);
    CERT_EnableOCSPDefaultResponder(CERT_GetDefaultCertDB());
    break;

Support a user has option 2 set, and changes configuration to 1.

If I understand correctly, NSS will still use the default OCSP responder, and we need to disable it.
Attached patch Patch v1Splinter Review
Bob, do you agree this single line change makes sense?
Attachment #252459 - Flags: review?(rrelyea)
Do you agree that we should clear the SSL session cache when the application enables OCSP, or switches to a different OCSP responder?

My testing was:
- start app, OCSP disabled
- load https://us.etrade.com
- enable OCSP
- reload https://us.etrade.com

Expected behaviour: OCSP request

Actual behaviour: No OCSP request
Attached patch Additional PatchSplinter Review
Attachment #252463 - Attachment is patch: true
Attachment #252463 - Attachment mime type: application/octet-stream → text/plain
Attachment #252463 - Flags: review?(nelson)
Comment on attachment 252463 [details] [diff] [review]
Additional Patch

r=nelson
Attachment #252463 - Flags: review?(nelson) → review+
Comment on attachment 252459 [details] [diff] [review]
Patch v1

r+=rrelyea

yes.
Attachment #252459 - Flags: review?(rrelyea) → review+
both patches checked in to trunk, marking fixed
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: