351897 - CERT_VerifyCertificate skips OCSP check if requiredUsages includes certificateUsageStatusResponder and other usages

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Julien Pierre]

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.

Comment 1

[Julien Pierre]

Attachment: Only skip the OCSP check if certificateUsageStatusResponder is the only usage being checked

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 3

[Kai Engert [:KaiE:]]

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 4

[Robert Relyea]

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.

Comment 5

[Kai Engert [:KaiE:]]

Julien, if you don't mind I would like to ahead and check this patch into trunk and 3.11 right away.

Comment 6

[Julien Pierre]

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

Comment 7

[Kai Engert [:KaiE:]]

*** Bug 351873 has been marked as a duplicate of this bug. ***

Comment 8

[Kai Engert [:KaiE:]]

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).

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

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.

Comment 10

[Kai Engert [:KaiE:]]

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

Comment 11

[Wan-Teh Chang]

Attachment: Say a != b instead of (! (a == b))

This is easier to read and what the original code in
CERT_VerifyCert looks like.

Comment 12

[Wan-Teh Chang]

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).