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

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Alexei Volkov]

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.

Comment 1

[Alexei Volkov]

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

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 3

[Alexei Volkov]

Attachment: Patch v2 - simplifyed version

Set the proper error through PORT_SetError if signature of the response could not be verified.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 5

[Alexei Volkov]

Attachment: 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.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 333284 [details] [diff] [review]
Patch v3

r=nelson

Comment 7

[Alexei Volkov]

(In reply to comment #6)
> (From update of attachment 333284 [details] [diff] [review])
checked in