ocsp checker returns incorrect error code on request with invalid signing cert

RESOLVED FIXED in 3.12.1

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Alexei Volkov, Assigned: Alexei Volkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX)

Attachments

(1 attachment, 2 obsolete attachments)

4.68 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
Verifying cert from www.diginotar.nl(need to download cert with vfyserv and verify with vfychain :( ):

vfychain -s allow-crl-and-ocsp -d . cert.000
Chain is bad, -8048 = Invalid OCSP signing certificate in OCSP response.

Now trying to use libpkix:
vfychain -s allow-crl-and-ocsp -d . -pp cert.000  -t cert.001 
Chain is bad, -8179 = Peer's Certificate issuer is not recognized.

This is the regression in returned codes. Need to fix it for 3.12.1.
(Assignee)

Updated

9 years ago
Whiteboard: PKIX
(Assignee)

Updated

9 years ago
Priority: -- → P1
(Assignee)

Comment 1

9 years ago
Created attachment 331598 [details] [diff] [review]
Patch v1 - fix reported error

The patch fixes the error that is to be set to OCSP_INVALID_SIGNING_CERT if requests signing cert can not be validated(function pkix_pl_OcspResponse_CallCertVerify).

It also fixes ocsp error code delivery mechanism. Prior to the patch, if code in function pkix_pl_OcspResponse_CallCertVerify had received a cert validation error, it threw an error which bypassed most of the error check code that was written to convert one error code to another.

One more fix is about incorrect handling error when signature object was not assign(pkix_pl_OcspResponse_VerifySignature function).
Attachment #331598 - Flags: review?(nelson)
Comment on attachment 331598 [details] [diff] [review]
Patch v1 - fix reported error

In today's review, I think we concluded that 

a) we want to continue to use a separate plContext when calling 
response->verifyFcn() in pkix_pl_OcspResponse_CallCertVerify.

b) It's not necessary to change the signature of 
pkix_pl_OcspResponse_CallCertVerify to achieve the goal of setting
the NSS error code SEC_ERROR_OCSP_INVALID_SIGNING_CERT when the 
cert verification reports failure,

c) detecting a NULL "signature" in pkix_pl_OcspResponse_VerifySignature
is good to do (as this patch does).
Attachment #331598 - Flags: review?(nelson) → review-
(Assignee)

Comment 3

9 years ago
Created attachment 333190 [details] [diff] [review]
Patch v2 - simplifyed version

Set the proper error through PORT_SetError if signature of the response could not be verified.
Attachment #331598 - Attachment is obsolete: true
Attachment #333190 - Flags: review?(nelson)
Comment on attachment 333190 [details] [diff] [review]
Patch v2 - simplifyed version

In pkix_pl_OcspResponse_VerifySignature there is a call to pkix_pl_OcspResponse_CallCertVerify that is made with the macro PKIX_CHECK.  I thought that the "short patch" was going to change that macro to PKIX_CHECK_ONLY_FATAL.  Does this patch achieve its goal without doing that?

This patch seems harmless. :)  It's OK to check in, but I'm not sure it solves the problem(s).
Attachment #333190 - Flags: review?(nelson) → review+
(Assignee)

Comment 5

9 years ago
Created attachment 333284 [details] [diff] [review]
Patch v3

Previous patch was indeed incomplete. We should not only set an error in CallVerifyCert function, but also should not throw errors if a cert was invalid for response signing.
Attachment #333190 - Attachment is obsolete: true
Attachment #333284 - Flags: review?(nelson)
Comment on attachment 333284 [details] [diff] [review]
Patch v3

r=nelson
Attachment #333284 - Flags: review?(nelson) → review+
(Assignee)

Comment 7

9 years ago
(In reply to comment #6)
> (From update of attachment 333284 [details] [diff] [review])
checked in
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.