Closed Bug 351897 Opened 14 years ago Closed 14 years ago

CERT_VerifyCertificate skips OCSP check if requiredUsages includes certificateUsageStatusResponder and other usages

Categories

(NSS :: Libraries, defect, P2)

3.11.3
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.11.4

People

(Reporter: julien.pierre, Assigned: julien.pierre)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.8.1, regression)

Attachments

(2 files)

The OCSP check should be skipped only if the specific usage of certificateUsageStatusResponder is being checked . It is OK to do an OCSP check when checking for other usages. Patch forthcoming.
Blocks: 351873
Priority: -- → P2
Comment on attachment 237423 [details] [diff] [review]
Only skip the OCSP check if certificateUsageStatusResponder is the only usage being checked

I confirm that the effect of this patch is that the code will now skip the OCSP check ONLY when the only requested usage is OCSP status responder usage.

I'm not sure when (under what circumstances, if any) it is appropriate to skip the OCSP check.
Attachment #237423 - Flags: superreview?(nelson) → review+
Blocks: 353530
We should get this patch in ASAP to ensure it gets into FF asap. (too late for FF 2 already, but it should get into a security release)
Comment on attachment 237423 [details] [diff] [review]
Only skip the OCSP check if certificateUsageStatusResponder is the only usage being checked

I actually believe this check can be removed altogether. The purpose is to prevent recursive calls to the OCSP code, however, the OCSP code uses CERT_VerifyCert rather than CERT_VerifyCertificate.
Attachment #237423 - Flags: review?(rrelyea) → review+
Julien, if you don't mind I would like to ahead and check this patch into trunk and 3.11 right away.
Bob,

The OCSP code's implementation may be changed in the future to use CERT_VerifyCertificate or the new libpkix code. So I think it's OK to skip the OCSP check for the specific case of the certificateUsageStatusResponder usage.

Kai,

I checked in this patch to the trunk :

Checking in certvfy.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certvfy.c,v  <--  certvfy.c
new revision: 1.48; previous revision: 1.47
done

And to NSS_3_11_BRANCH :

Checking in certvfy.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certvfy.c,v  <--  certvfy.c
new revision: 1.44.10.4; previous revision: 1.44.10.3
done
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.11.4
*** Bug 351873 has been marked as a duplicate of this bug. ***
No longer blocks: 351873
Keywords: regression
Flags: blocking1.8.1.1?
Comment on attachment 237423 [details] [diff] [review]
Only skip the OCSP check if certificateUsageStatusResponder is the only usage being checked

Do you want to take this security fix? Baking on trunk since 2006-09-26 (check in bug 354378).
Attachment #237423 - Flags: approval1.8.1?
Comment on attachment 237423 [details] [diff] [review]
Only skip the OCSP check if certificateUsageStatusResponder is the only usage being checked

a=beltzner on behalf of drivers, please land on the branch ASAP.
Attachment #237423 - Flags: approval1.8.1? → approval1.8.1+
Patch checked in to MOZILLA_1_8_BRANCH

Checking in certvfy.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certvfy.c,v  <--  certvfy.c
new revision: 1.43.14.3; previous revision: 1.43.14.2
done
Keywords: fixed1.8.1
This is easier to read and what the original code in
CERT_VerifyCert looks like.
Attachment #243227 - Flags: review?(julien.pierre.bugs)
Attachment #243227 - Flags: review?(julien.pierre.bugs) → review+
Comment on attachment 243227 [details] [diff] [review]
Say a != b instead of (! (a == b))

I checked in this patch on the NSS trunk (NSS 3.12).
Flags: blocking1.8.1.1?
You need to log in before you can comment on or make changes to this bug.